Add dashboard chat agent to add and patch plugins on dashboard - #151
Open
romer8 wants to merge 90 commits into
Open
Add dashboard chat agent to add and patch plugins on dashboard#151romer8 wants to merge 90 commits into
romer8 wants to merge 90 commits into
Conversation
… refetch event listener in DashboardLoader
…g; update visualization tile dimensions for better layout
…d_id and chat_command functions
…ES in chat agent and related functions
…BotMessage, integrate Chatbox component, and update chat message endpoint.
…at functionality to trigger refetch event after sending messages
…inal_prompt and fix available_plugins function name
…posal handling and visualization staging
…n + list-plugins
Small local models (qwen3:0.6b-1.7b) route unreliably across the map
tool surface; the map path (6 approval-gated add_*_map tools, spec-table
factory) is preserved on feat/map-agent for revival on a model upgrade.
- remove chat/{map_agent,pending,actions,geocode}.py and tools/map_tools.py
- router back to 3 candidates (add_visualization, list_available_plugins,
out_of_scope_reply)
- controllers: approval-gate block removed from chat_message
- viz_agent renamed to plugin_agent (kept from the map branch work)
…tion - Implement unit tests for the ChatSettings component to verify loading, saving, and displaying chat settings. - Introduce authorization checks for mutating actions in the chat agent, ensuring only dashboard owners can perform certain actions. - Update the router to conditionally expose router candidates based on ownership. - Enhance the add_visualization_from_plugin tool to enforce ownership checks and return appropriate error messages. - Add integration tests for chat provider settings to ensure correct database interactions. - Remove outdated unit tests for agent tools and replace them with updated tests for the new plugin system. - Add tests for chat configuration and history sanitization to ensure robustness.
…or enhanced user suggestions
- drop tethys-agents from dependencies (unpublished; superseded by pydantic-ai which is already declared) - add propTypes validation to all chat components - use findByDisplayValue instead of waitFor+getByDisplayValue in ChatSettings test
[chat-stream]/[chat] console logs trace the fetch reader (response, each chunk, each parsed line, close) and each onEvent; a server-side print marks when each NDJSON line is flushed. Lets us see whether events arrive incrementally or buffered, and pinpoint backend vs proxy vs browser. Revert once diagnosed.
The chat/free-text path now streams the model's reply token-by-token instead
of returning it whole. LLMRouter._stream_chat wraps chat_agent.run_stream and
pushes each text delta onto the request stream as a new 'delta' event; the
joined text is still returned as the final reply.
- utils: emit_delta ({type: delta}) alongside emit_progress, sharing _emit.
- frontend: useChatState accumulates delta events (append) while progress
milestones only show until the first token arrives.
- authz tests updated with a run_stream stub; new delta-accumulation test.
Verified end-to-end against the live Ollama model: deltas stream, join to the
full answer. Only the chat path streams tokens; add/patch stay milestone-based
(little text to stream). pydantic-ai debounces deltas at 100ms by default.
…l errors
Three root causes found via /ce-debug:
1. Buffering (all events arrived at once): a SYNC generator in
StreamingHttpResponse is buffered until it finishes under ASGI; only ASYNC
generators stream chunk-by-chunk. Rewrote chat_message to stream an async
generator - route() runs as an asyncio task on the loop and a small
_AsyncSink bridges sync emit_progress/emit_delta onto an asyncio queue.
Proven: a tick-every-0.5s probe buffered as a sync gen, streamed as async
(through the dev proxy too - the proxy was never the problem).
2. Simple prompts ('where is bolivia?') failed with retry exhaustion:
chat_agent was the only agent with reasoning_effort:none commented out, so
qwen3 burned its token budget on <think> and produced no answer -> retries
exhausted. Enabled reasoning_effort:none like every other agent.
3. Failures were invisible: the retry-exhausted fallbacks swallowed the real
exception. Added _log_llm_error so the router/specialist/chat-stream paths
print the actual model error server-side.
Verified end-to-end against live Ollama: 'where is bolivia?' now answers and
streams token deltas ~0.1s apart. test_chat_streaming rewritten for the sink
registry contract (emit_progress/emit_delta -> registered sink).
chat_message was ~135 lines doing ten jobs with inline comments and a nested async pyramid. Split it: - new chatbot/streaming.py owns the NDJSON machinery: _AsyncSink, _reply_text, _log_request_error, _run_router, _chat_events, and stream_chat_response. - controllers.py keeps request concerns: _parse_chat_request (validation, raising _ChatRequestError with an HTTP status), _build_chat_deps (ChatDeps + ownership), and a ~12-line chat_message that parses, builds deps, and delegates to stream_chat_response. - validated fields travel in a _ChatRequest NamedTuple; errors travel as one exception type instead of scattered early-return JsonResponses. - dropped the dead 'api_view(["POST"])' line (no decorator, was a no-op). Docstrings only, one purpose per function, DRY. Behavior unchanged: verified compile + ruff clean, 27 chat unit tests pass, and an end-to-end run streams progress then answer deltas exactly as before.
Remove the [chat-stream] fetch-reader logs and the [chat] onEvent log added to diagnose the streaming/buffering issue. Server-side _log_llm_error / _log_request_error stay - they surface real model errors, not debug noise.
…strings Five readability/DRY passes over this branch's code: 1. Deduplicate agent instructions: recent_conversation (x4) and available_plugins (x2) were copy-pasted across agents. Extracted to agents/instructions.py and registered with agent.instructions(fn). 2. Split LLMRouter (+ _log_llm_error, _RETRY_EXHAUSTED_MESSAGE) out of models.py into chatbot/routing.py; models.py is now pure data classes. 3. Fix ChatDeps docstring: it still described the removed WebSocket; now describes the NDJSON stream sink. history typed list | None. 4. Replace the 'chat'/'fallback' magic strings with INTENT_CHAT / INTENT_FALLBACK constants in registry.py; Route and RoutedResponse now build their Literals from the shared constants. 5. Frontend: extract recentHistory() and streamIntoBubble() from useChatState.send, shrinking send to orchestration. Verified: import chain clean (no cycles), ruff clean, agents build with shared instructions, Route/RoutedResponse Literals resolve, 37 backend + 4 frontend chat tests pass.
Root cause (found via live-model repro): the numbered-index interface asked the 2B model to (a) map a description to a 1-based number and (b) judge ambiguity. It could do neither - it conflated 1-based/0-based/values (passed candidates [1,2] for tiles at [1,3]; 'visualization #2 is actually at index 0'), and over-triggered disambiguation on unambiguous prompts. So patch_visualization never landed on multi-tile dashboards -> 'update keeps failing'. Single-tile dashboards worked, which made it look intermittent. Fix - move counting and ambiguity-judgment out of the model into deterministic code, keeping only what the model does reliably (extract source + new value): - patch_visualization(source, args, where=None): matches tiles by source name (normalized, fuzzy). 0 -> ModelRetry listing real sources; 1 -> patch; 2+ -> the tool asks by current value (accurate, code-generated) - no numbers. - _auto_select: when several tiles share a source, if the user's prompt already names one tile's current value, pick it deterministically. Resolves the disambiguation follow-up without the model having to fill 'where'. - remove ask_which_visualization (the tool owns ambiguity now); patch_agent output is NativeOutput([patch_visualization]); instructions forbid numbers. Validated against live Ollama: unambiguous -> patches directly; ambiguous -> asks by value; 'the forecast viewer with river_id 111' -> auto-selects + patches. Tests rewritten for source targeting (15 patch tests); 39 chat tests pass.
When several tiles share a source, _auto_select picked the one match whose
current value the user named. But the user's message also contains the NEW
value, and when that new value already equals other tiles' current values
(e.g. set river_id to 441057380 while 3 tiles are already 441057380), it
counted as a selector for all of them -> multiple hits -> None -> the same
disambiguation list every turn ('looks cached, never updates'), even when the
user clearly named the distinguishing value.
Exclude the args values from the auto-select scan so only genuine selector
values count. 'change the forecast viewer with river_id 710462910 to 441057380'
now resolves and updates that tile; a prompt with no distinguishing value still
asks. Verified against live Ollama with the transcript's 5-tile dashboard.
Root-cause fix (plan units 1-2) for identical dashboard tiles that made patch disambiguation impossible: - Normalize each arg value to the plugin's declared type from visualization_args on write (river_id is declared 'text'; the plugin already str()s it, so this is behavior-safe). Equal logical values (441057380 int vs '441057380' str) now store one way. - Dedupe on add: skip a tile whose (source, normalized args) already exists on the active tab (or repeats within the batch), and tell the user it already exists. Existing tiles are type-normalized when compared, so int/str dupes are caught too. Prevents the duplicate/identical-tile accumulation at its source. 15 add-path tests pass (10 existing + 5 new: normalize, skip-identical, int/str dedupe, batch dedupe, new-value-not-dup); 50 chat unit tests pass overall.
Plan units 3-6. When patch finds 2+ same-source tiles it now shows a numbered
list and stores a short-lived server-side pending record; the next user reply is
resolved deterministically before the router/LLM.
- disambiguation.py (new): Django-cache-backed pending record (keyed by
dashboard+user, 10m TTL) + resolve_pending. Classifies a whole-message reply
as a number / 'all' / 'cancel'; anything else falls through so real requests
('change all the rivers to X', 'add 3 tiles') aren't hijacked. Numbers are
code-generated and code-resolved against the record's ordered (tab,item)
identities - the model never handles an index, so this doesn't reintroduce the
removed model-index failure. Re-validates a dashboard fingerprint (fail-closed
re-ask on drift), reuses patch's ownership + arg-validation + no-op guards, and
handles empty/out-of-range/non-owner/invalid-args.
- patch.py: numbered _disambiguation_reply; writes the pending record at the 2+
branch; shared check_args / candidate_signature / _pairs; clears the record on
a successful single-tile patch.
- streaming.py: stream_immediate() one-shot NDJSON so a resolved reply rides the
same envelope streamIntoBubble expects.
- controllers.py: resolve_pending pre-check hop before stream_chat_response.
Numbers finally crack the identical-tiles case value/all can't. Verified
end-to-end against live Ollama (ambiguous -> numbered list + record; '2' ->
updates the 2nd, clears). 75 chat unit tests pass; import chain clean.
From the parallel review of the patch/disambiguation work:
- plugins.py _normalize_arg_value: only canonicalize scalars (leave date-range
and other dict/list args intact - str()-coercing them silently blanked the
value in the UI); reject NaN/Infinity (float('nan')/('inf') would serialize to
bare NaN/Infinity tokens = invalid JSON that breaks the whole dashboard load).
- patch.py _auto_select: exclude a candidate value that is a SUBSTRING of the new
value, not just an exact match - '300' inside new value '3005' was auto-picking
the wrong tile with no disambiguation prompt.
- patch.py candidate_signature: bind to tile uuid so a delete + content-identical
add during the pending window is detected as drift (position+content alone
collided).
- disambiguation.py get_pending: fall back to None on a stale cached record shape
(TypeError) so a rolling deploy can't 500 a mid-disambiguation request.
- disambiguation.py 'all' branch: validate args per tile (not just the first) so
a heterogeneous fuzzy-source match can't blast an unvalidated arg into a
differently-specced tile.
+4 regression tests (nan/date-range passthrough, substring auto-select, clear on
success). 74 chat unit tests pass.
… 500 The deterministic pre-check ran outside any try/except, unlike the rest of the chat pipeline (streaming _run_router, routing.route). A DB/cache error there raised a raw 500 instead of degrading to the normal chat stream. Wrap it: log and fall through on any exception.
- #1 hijack guard: resolve_pending only fires when the previous assistant turn actually posed the disambiguation ask (DISAMBIGUATION_MARKER) - #2 tile_ops extraction: shared helpers module breaks the patch <-> disambiguation import cycle - #3 clear scoping: a successful patch clears only a pending record whose source matches the patched tile, not unrelated ones - #4 single-read dedupe: append_new_tiles does one read-modify-write, closing the add-path TOCTOU that let duplicate tiles slip in - #5 Literal typing: intents declared as string literals (PEP 586) instead of constants inside Literal[...] - #6 tests: controller pre-check hop, stream_immediate, "all" mixed noop/changed, where-filter in resolve, and set_pending supersede 88 chat tests pass; ruff clean.
Correctness: - Scope the post-patch pending-clear to the just-patched tile's (tab,item) identity, not a fuzzy source-name match - patching chart_widget no longer wipes an unanswered "which one?" for a substring-related source like chart. - Stringify non-finite floats in _normalize_arg_value so a raw NaN/Infinity can't serialize to invalid JSON and break the dashboard load. DRY / typing: - router.py reuses IntentName | Literal["chat"] instead of re-listing the intent literals, keeping the set single-sourced in agents/registry.py. - Extract utils.log_chat_error; routing.py and controllers.py use it instead of two more copies of the print+traceback block. - plugins.py reuses tile_ops._tile_args instead of a byte-identical _safe_tile_args copy; append_new_tiles gains full type hints and its docstring no longer overclaims a cross-request concurrency guarantee. Reliability: - clear_pending is best-effort so a post-save cache blip can't propagate and replay the selection through the router against an already-patched tile. Tests (+4): add-path single dashboard read; "all" rejects when any matched tile's args are invalid; candidate_signature uuid-swap drift detection; get_pending stale-shape recovery. Realign the keeps-unrelated-pending test to a realistic record whose candidate position differs from the patched tile. Inline comments folded into docstrings (docstrings-only convention). 92 chat tests pass; ruff clean.
…terministic matching
…ncy and accessibility
…nents for enhanced user input
…ance props handling
…val for UI display
romer8
changed the title
…e plugin catalog formatting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a complete, self-contained Docker Compose stack for TethysDash, enabling local development and deployment with SQLite (no Postgres), Valkey (Redis-compatible), and Ollama for chatbot LLMs. It includes all necessary Dockerfiles, configuration, and provisioning scripts, as well as documentation and plugin management. Additionally, it adds new React dependencies and comprehensive unit tests for chatbot components.
Docker stack and provisioning:
docker/docker-compose.yml) with services for TethysDash, Valkey, Ollama, and provisioning, using SQLite for all persistent stores and supporting local LLMs via Ollama.docker/Dockerfile) that builds the React frontend, installs TethysDash and user-selected plugins, and sets up persistent storage and app workspace/media directories.docker/init.d/10-tethysdash-stores.sh) to automate persistent store setup and linking, ensuring idempotent initialization.docker/.env.example), plugin selection (docker/plugins.txt), and portal settings (docker/portal_config.yml) for easy customization. [1] [2] [3].dockerignoreanddocker/.gitignoreto keep build contexts and secrets clean. [1] [2]Documentation:
docker/README.md) with quickstart, plugin management, troubleshooting, and data reset instructions for the stack.Frontend and testing improvements:
markdown-to-jsxdependency for improved markdown rendering in the React frontend (package.json).reactapp/__tests__/components/chat/slashTemplates.test.js,reactapp/__tests__/components/chat/useChatState.test.js). [1] [2]Python dependencies:
pydantic-aidependency to the Python project for improved AI integration.