feat(workbench-shell): ✨ Add dynamic chart artifact rendering to thread messages by jorben · Pull Request #169 · TiyAgents/tiycode

jorben · 2026-05-06T12:41:50Z

Summary

Introduce a complete "model-driven dynamic chart preview" capability for thread messages, enabling the agent to generate interactive Vega-Lite visualizations inline within the conversation flow.

Changes

Backend (Rust)

Schema: Add parts_json column to messages table (new migration)
Model: Extend MessageRecord / MessageDto with structured parts field
Repo: Add replace_parts() and merge_chart_artifact_part() for chart persistence
Events: Add ThreadStreamEvent::ArtifactUpdated variant for streaming chart updates
Tool: Register render_chart in runtime tool profile with full Vega-Lite schema
Execution: Implement execute_render_chart() with artifact lifecycle (started → completed)

Frontend (TypeScript/React)

Protocol: Define MessagePartDto union types (text, chart, data-*, unknown)
Mapping: Add mapMessageParts() with backward-compatible contentMarkdown fallback
Rendering: Refactor LongMessageBody to render by part type (text → markdown, chart → card)
Chart Card: Real Vega-Lite rendering via vega-embed with dark theme, SVG, responsive width
Safety: validateSpec() (512KB size limit, 50K data points), ChartErrorBoundary for crash isolation
Stream: Wire ArtifactEvent → onArtifact → mergeArtifactPartIntoMessage() for live updates
UX: Loading skeleton, error state, spec/chart toggle, multi-chart support per message

Dependencies

Add vega, vega-lite, vega-embed for declarative chart rendering

Motivation

Enable the agent to produce rich data visualizations (distributions, comparisons, statistical plots) directly in the thread message flow — similar to the generative chart demo UI pattern — while maintaining security (no arbitrary JS execution), persistence (survives refresh), and progressive rendering (loading → ready).

Testing

npm run typecheck — zero errors
npm run test:unit — 544 tests passed
cargo fmt --check — clean
cargo test --locked — all crates pass
Manual smoke test: trigger render_chart via agent, verify chart appears and persists after refresh

Breaking Changes

None. The contentMarkdown field is preserved for backward compatibility; parts is additive.

Checklist

Code follows project style guidelines
Tests have been added/updated
No new warnings or errors introduced
Migration is incremental (ALTER TABLE ADD COLUMN)

🤖 Generated with TiyCode

…hread messages Introduce a multi-part message protocol and chart artifact system that enables dynamic chart preview generation within the thread message flow. This is the foundational layer for model-driven data visualization. Frontend changes: - Extend MessageDto/SurfaceMessage with structured `parts` array - Refactor LongMessageBody to render messages by part type - Add ThreadChartArtifactCard with real Vega-Lite rendering via vega-embed - Wire ArtifactEvent through ThreadStream and runtime-thread-surface - Add mapMessageParts() with fallback compatibility for legacy messages Backend changes: - Add `parts_json` column to messages table (migration) - Extend MessageRecord/MessageDto with parts field - Add message_repo::replace_parts and merge_chart_artifact_part - Add ThreadStreamEvent::ArtifactUpdated variant - Propagate parts_json through all MessageRecord construction sites Dependencies: - Add vega, vega-lite, vega-embed for declarative chart rendering

… error boundary Implement the controlled chart generation tool that enables the model to produce dynamic Vega-Lite visualizations in thread messages. Runtime tool: - Register render_chart in runtime_tools_for_profile (both profiles) - Add execute_render_chart in agent_session_execution with full lifecycle: persist tool call → validate spec → resolve target message → emit ArtifactUpdated started/completed → merge_chart_artifact_part → emit ToolCompleted Frontend hardening: - Add validateSpec() with size limit (512KB) and data point cap (50K) - Add ChartErrorBoundary (React class component) to catch render crashes - Add responsive width fitting and dark theme autosize config - Show validation errors inline without breaking message layout - Support show/hide spec toggle for debugging

github-actions · 2026-05-06T12:51:26Z

AI Code Review Summary

PR: #169 (feat(workbench-shell): ✨ Add dynamic chart artifact rendering to thread messages)
Preferred language: English

Overall Assessment

Detected 13 actionable findings, prioritize CRITICAL/HIGH before merge.

Major Findings by Severity

HIGH (3)
- src-tauri/src/core/agent_session_execution.rs:543 - No integration test covering the new render tool execution path
- src-tauri/src/model/thread.rs:110 - Missing DB migration for new parts_json column
- src-tauri/src/persistence/repo/message_repo.rs:322 - Race condition in merge_chart_artifact_part: read-modify-write is not atomic
MEDIUM (7)
- package.json:45 - Missing UI integration for newly added Vega dependencies
- src-tauri/src/core/agent_session_execution.rs:749 - Chart artifact target message can be a synthetic fallback ID
- src-tauri/src/core/agent_session_execution.rs:602 - File-path branch within render tool not covered by tests
- src-tauri/src/ipc/frontend_channels.rs:15 - No tests for ArtifactStatus serialization format
- src-tauri/src/model/thread.rs:142 - Silent deserialization failure for parts field
- src-tauri/src/persistence/repo/message_repo.rs:309 - Missing test coverage for new DB functions
- src-tauri/tests/agent_run.rs:956 - No test for concurrent or out-of-order tool call updates
LOW (3)
- src-tauri/src/core/agent_run_compaction.rs:68 - Missing DB fallback after persist_clear_context_reset_to_pool
- src-tauri/src/persistence/repo/message_repo.rs:260 - INSERT parameter count mismatch in comment vs actual bind arguments
- src/modules/workbench-shell/ui/workbench-preview-overlay.tsx:28 - WorkbenchPreviewOverlay renders DOM nodes even when closed (conditional return vs CSS)

Actionable Suggestions

Prevent synthetic message IDs from being persisted as the target for chart artifacts; either ensure a real message exists before render or handle synthetic IDs on the frontend explicitly.
Add a warning log when event_tx.send fails for artifact events, to aid debugging.
Verify that the MessageInsert struct includes parts_json in all branches.
Implement integration tests for the render tool at src-tauri/tests/agent_render.rs covering vega-lite, html inline, html file path, missing spec, empty source, oversized source, and path outside workspace.
Add a concurrency test that races an agent run completion against a render call to ensure artifact target message is stable.
Add unit tests for active_runs_db_fallback with 0, 1, and multiple assistant messages.
Add a DB integration test for message_repo::merge_chart_artifact_part to verify parts_json column update.
Add a database migration for the new parts_json column in the messages table before deploying this change.
Log a warning when parts_json deserialization fails in MessageDto::from(MessageRecord).
Confirm with the team whether pub mod persistence is intentional; if only for tests, restrict with #[cfg(test)].
Extend existing thread command tests to include messages with non-None parts_json and validate DTO conversion.
Validate or constrain the ArtifactUpdated payload before sending to the frontend, especially if artifact sources can be untrusted.

Potential Risks

User-facing chart artifacts may be silently lost if active_runs_message_id() returns None.
If active_runs map gets out of sync, render tool may attach artifacts to stale message IDs.
All part_json: None additions assume the struct field exists; if not, compilation fails (moderate risk, caught by CI).
Untested render pipeline could silently fail to attach artifacts in production, leading to invisible rendered content.
Race condition on streaming_message_id could cause artifacts to attach to the wrong message under high concurrency.
Existing databases without the parts_json column will cause runtime errors when message records are read or written.
Malformed JSON in parts_json is silently swallowed, potentially hiding important data integrity issues from operators.
Accidentally making persistence public may lead to unintended API dependencies that are hard to remove later.
Arbitrary JSON artifact payloads could inject unexpected data into the frontend rendering pipeline.
Exposed persistence module may lead to unintended database access patterns.
Without tests, the ArtifactUpdated event serialization may not match frontend expectations, breaking artifact display.
Invalid parts_json in the database would silently result in None parts with no error log or test coverage.

Test Suggestions

Write integration test for execute_render covering: valid vega-lite, html with inline source, html from file path, path outside workspace, missing spec, oversized source, and successful attachment to real message.
Test the race condition where active_runs_message_id() returns None and the frontend receives a chart artifact with a synthetic ID; verify UI behavior.
Ensure active_runs is populated before any execute_render call in integration tests.
Create src-tauri/tests/agent_render.rs with at least 6 test cases covering the major branches.
Use test fixtures that seed an agent session with workspace files to test path rendering.
Employ tokio::sync::Barrier or similar to simulate concurrent session end and render execution.
Integration test: verify that the ArtifactUpdated event is properly dispatched to frontend listeners.
Unit test: serialize/deserialize ArtifactStatus enum variants to ensure snake_case works.
Unit test: MessageDto conversion with both valid JSON and invalid JSON in parts_json.
Integration test: ensure database migration adds the parts_json column and existing messages are handled.
Integration test: spawn a thread run that emits an artifact event, capture ThreadStreamEvent, deserialize, and assert ArtifactUpdated fields.
Unit test: serde round-trip for ArtifactStatus enum.

File-Level Coverage Notes

src-tauri/src/core/agent_session_execution.rs: Major new execute_render flow and active_runs plumbing added with thorough error handling but zero test coverage for any branch. (The code itself is clean but untested.)
src-tauri/src/core/agent_session.rs: Struct extended with active_runs shared map; new signature changed to crate visibility. No test impact directly, but callers need to update.
src-tauri/src/core/agent_session_tools.rs: Render tool definition added with correct JSON Schema. No executable logic to test here.
src-tauri/src/core/agent_run_manager.rs: MessageRecord inserts now include parts_json: None and start_session signature changed to accept active_runs Arc. Callers updated accordingly.
src-tauri/src/core/agent_run_compaction.rs: MessageRecord inserts now include parts_json: None; backward compatible.
src-tauri/src/core/agent_run_event_handler.rs: MessageRecord inserts now include parts_json: None; backward compatible.
src-tauri/src/ipc/frontend_channels.rs: no-blocking-risk (The enum and event struct are well-defined; the main gap is missing tests for the new IPC surface.)
src-tauri/src/core/thread_manager.rs: no-risk (Only a single line addition setting parts_json: None; low impact.)
src-tauri/src/core/tool_gateway.rs: no-risk (Minimal change adding parts_json: None to existing constructor.)
src-tauri/src/commands/thread.rs: no-risk (Three test constructors updated; but the tests themselves don't assert the new field.)
src-tauri/src/model/thread.rs: low-risk (Data model changes are structurally sound, but the conversion logic should be tested for robustness.)
src-tauri/src/lib.rs: no-risk (Visibility change from private to public for the persistence module; no functional impact.)
src-tauri/src/persistence/repo/message_repo.rs: The module gains new functionality but the test module does not cover replace_parts, merge_chart_artifact_part, or any non-null parts_json scenarios.
src-tauri/src/core/built_in_agent_runtime.rs: The start_session signature change is small but introduces a shared mutable state that demands concurrency-aware tests. (This file appears only as a context snippet; the full usage of active_runs in downstream code should also be tested.)
src/modules/workbench-shell/ui/file-preview-surface.tsx: Performance-neutral. Lazy import of MarkdownPreviewContent via inline function (not React.lazy) but the function is small, and the branch only executes for markdown type. No I/O, network, or memory concerns. (MarkdownPreviewContent is defined in the same module but labeled as 'lazy-import' — actual lazy import (React.lazy + Suspense) would be more beneficial if MarkdownPreviewContent imports heavy dependencies.)
src/modules/workbench-shell/ui/workbench-preview-overlay.tsx: Lightweight overlay with conditional null return. No perf issues.
src/modules/workbench-shell/ui/thread-chart-artifact-card.tsx: Good lazy load of vega-embed, cancellation-based effect, and error boundaries. Minor concerns: missing React.memo, unnecessary specText computation, and full re-render on theme change. (See findings above for concrete suggestions.)
src/modules/workbench-shell/ui/chart-spec-validation.ts: Pure validation function with constant time/space bounds. No performance risks. (JSON.stringify is called on small-to-moderate payloads and is bounded by the size check itself, so no double-serialization concern.)
src/modules/workbench-shell/ui/vega-embed.d.ts: Type declaration only; no runtime impact.
src/modules/workbench-shell/ui/runtime-thread-surface.tsx: Artifact support is well-integrated with message delta/completion handlers. The buffer mechanism handles out-of-order arrival reasonably, but lacks cleanup for orphaned entries. (Ensure that the component resets pendingArtifactsRef when a new run starts or the surface is unmounted.)
... and 17 more file-level entries.

Inline Downgraded Items (processed but not inline)

None

Coverage Status

Target files: 37
Covered files: 37
Uncovered files: 0
No-patch/binary covered as file-level: 0
Findings with unknown confidence (N/A): 0

Uncovered list:

None

No-patch covered list:

None

Runtime/Budget

Rounds used: 1/4
Planned batches: 9
Executed batches: 9
Sub-agent runs: 16
Planner calls: 1
Reviewer calls: 19
Model calls: 20/64
Structured-output summary-only degradation: NO

github-actions

Automated PR review completed.

Findings kept: 11
Findings with unknown confidence: 0
Inline comments attempted: 10
Target files: 25
Covered files: 25
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

        }
    }

+    async fn execute_render_chart(


Sign in to view

+    }
+
+    void render();
+    return () => { cancelled = true; };


Sign in to view

        }
    }

+    async fn execute_render_chart(


Sign in to view

                .await;
        }

+        if tool_name == "render_chart" {


Sign in to view

+        // Resolve the message to attach the chart to: use the last completed
+        // assistant message in this run, or create a reference if needed.
+        let target_message_id = {
+            let runs = self.active_runs_message_id().await;


Sign in to view

        run_id: String,
        task_board: TaskBoardDto,
    },
+    ArtifactUpdated {


Sign in to view

+    Ok(())
+}
+
+pub async fn merge_chart_artifact_part(


Sign in to view

+function mapUnknownPart(part: MessagePartDto): SurfaceUnknownMessagePart {
+  return {
+    type: part.type,
+    value: part as Record<string, unknown>,


Sign in to view

+    async function render() {
+      if (!containerRef.current) return;
+      try {
+        const vegaEmbed = (await import("vega-embed")).default;


Sign in to view

+    }
+
+    /// Get the last streaming/completed message ID for the current run.
+    async fn active_runs_message_id(&self) -> Option<String> {


github-actions

Automated PR review completed.

Findings kept: 19
Findings with unknown confidence: 0
Inline comments attempted: 19
Target files: 25
Covered files: 25
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

    run_id: Option<String>,
    role: String,
    content_markdown: String,
+    parts_json: Option<String>,


Sign in to view

+    };
+  }
+
+  const nextParts = message.parts.slice();


Sign in to view

        }
    }

+    async fn execute_render_chart(


Sign in to view

+      try {
+        const vegaEmbed = (await import("vega-embed")).default;
+        if (cancelled) return;
+        const result = await vegaEmbed(containerRef.current, spec as object, {


Sign in to view

            run_id: Some(self.spec.run_id.clone()),
            role: "assistant".to_string(),
            content_markdown: plan_markdown(&plan_metadata),
+            parts_json: None,


Sign in to view

+          error: event.error,
+          kind: event.status,
+          messageId: event.messageId,
+          payload: event.payload,


Sign in to view

                content_markdown: "<context_summary>\nCarry this forward.\n</context_summary>"
                    .to_string(),
+                parts_json: None,
+


Sign in to view

-  const content = message.content || (message.status === "streaming" ? "…" : "");
-  const preview = shouldUseLongMessagePreview(message)
-    ? getLongMessagePreview(content)
+  const parts: SurfaceMessagePart[] = message.parts ?? [{ type: "text", text: message.content || (message.status === "streaming" ? "…" : "") }];


Sign in to view

        : "assistant",
    runId: message.runId,
    content: message.contentMarkdown,
+    parts: mapMessageParts(message.parts, message.contentMarkdown),


Sign in to view

    this.onRunStateChange = null;
    this.onContextCompressing = null;
    this.onPlan = null;
+    this.onArtifact = null;


Implement chart artifact handling across the full stack: - Add shared chart spec validation in chart-spec-validation.ts with size and data constraints - Add type declaration for vega-embed library - Refactor thread-chart-artifact-card.tsx to use shared validation and hoist vega-embed import - Add normalizeThreadStreamEvent to agent-commands.ts with status validation and fallback - Add Rust tests for render_chart tool availability and spec requirements - Add Rust integration tests for ThreadStreamEvent::ArtifactUpdated serialization - Add unit tests for client-side event normalization and spec validation This enables agents to render Vega-Lite charts as artifacts within threads, with proper validation, error handling, and type safety.

github-actions

Automated PR review completed.

Findings kept: 13
Findings with unknown confidence: 0
Inline comments attempted: 13
Target files: 29
Covered files: 29
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

@@ -0,0 +1,178 @@
+import { Component, useEffect, useRef, useState } from "react";


Sign in to view

        }
    }

+    async fn execute_render_chart(


Sign in to view

+        // Validate spec field exists
+        let spec = match tool_input.get("spec") {
+            Some(s) if s.is_object() || s.is_array() => s.clone(),
+            _ => {


Sign in to view

+            tool_input: tool_input.clone(),
+        });
+
+        // Validate spec field exists


Sign in to view

    Ok(())
 }

+pub async fn replace_parts(


Sign in to view

        runId: readRequiredString(rawEvent, "runId", "run_id"),
        plan: readValue(rawEvent, "plan", "plan"),
      };
+    case "artifact_updated":


Sign in to view

  url: string | null;
 }

+export interface TextMessagePartDto {


Sign in to view

+        });
+
+        // Persist chart artifact into message parts
+        if let Err(error) = message_repo::merge_chart_artifact_part(


Sign in to view

+  });
+
+  it("falls back to 'completed' for unknown status values", () => {
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});


Sign in to view

+  if (VALID_ARTIFACT_STATUSES.has(rawStatus)) {
+    return rawStatus as "started" | "delta" | "completed" | "failed";
+  }
+  console.warn(`[agent-commands] Unknown artifact status "${rawStatus}", falling back to "completed"`);


github-actions

Automated PR review completed.

Findings kept: 12
Findings with unknown confidence: 0
Inline comments attempted: 12
Target files: 29
Covered files: 29
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

        }
    }

+    async fn execute_render_chart(


Sign in to view

+        });
+
+        // Persist chart artifact into message parts
+        if let Err(error) = message_repo::merge_chart_artifact_part(


Sign in to view

+            .iter()
+            .rev()
+            .find(|m| m.run_id.as_deref() == Some(&self.spec.run_id) && m.role == "assistant")
+            .map(|m| m.id.clone())


Sign in to view

    "tailwind-merge": "^3.3.1",
    "tw-animate-css": "^1.3.7",
    "use-stick-to-bottom": "^1.1.3",
+    "vega": "^6.2.0",


Sign in to view

            .contains("incrementally refine the plan"));
    }

+    #[test]


Sign in to view

+    };
+  }
+
+  const nextParts = message.parts.slice();


Sign in to view

+
+export function ThreadChartArtifactCard({ part }: ThreadChartArtifactCardProps) {
+  const [showSpec, setShowSpec] = useState(false);
+  const specText = JSON.stringify(part.spec, null, 2);


Sign in to view

        this.onPlan?.({ runId: event.runId, plan: event.plan });
        break;

+      case "artifact_updated":


Sign in to view

                content_markdown: "<context_summary>\nCarry this forward.\n</context_summary>"
                    .to_string(),
+                parts_json: None,
+


Sign in to view

+            <p className="text-sm leading-6 text-app-muted">{part.caption}</p>
+          ) : null}
+        </div>
+        <button


Add comprehensive integration tests for the persistence layer: - Render chart tool call insertion, success and failure updates - Replace message parts with JSON and nullification - Merge chart artifact part creation, upsert, and empty content migration behavior Supporting changes: - Expose persistence module as public for external test access - Refactor merge_chart_artifact_part to eliminate redundant database fetch and improve error handling - Memoize specText in ThreadChartArtifactCard for performance

… tools

…L/SVG artifacts Rename the `render_chart` tool to `render` to broaden its purpose beyond charts. The updated tool now supports three libraries: `vega-lite` (requires `spec`), `html`, and `svg` (both require `source`). This allows the agent to render HTML pages and SVG graphics inline alongside existing Vega-Lite visualizations. Previously, only the `render_chart` tool name was available and it required a `spec` field for Vega-Lite. Now the tool accepts a `library` parameter that selects the render path, with separate validation rules for each library type. HTML/SVG source is enforced as a non-empty string with a 1MB size limit. On the frontend, the `ThreadChartArtifactCard` component is extended to handle HTML/SVG artifacts with a preview dialog (using `dangerouslySetInnerHTML` for SVG and `<iframe>` for HTML). The `SurfaceChartMessagePart` type gains an optional `source` field, and the state mapping logic is updated to accept artifacts that contain either `spec` or `source`. The `render` tool is also collapsed by default in the thread surface, consistent with the previous `render_chart` behavior.

…ming parts accumulation Add support for reading HTML/SVG source from a file path instead of requiring inline source only. The `render` tool now accepts an optional `path` parameter that resolves relative to the workspace root. Includes proper error handling for file read failures and path validation using workspace path resolution. Update the tool schema to document the new `path` parameter alongside the existing `source` field. Fix message streaming in RuntimeThreadSurface to preserve non-text parts (e.g., tool calls) when delta updates arrive. Previously, each delta replaced the parts array with only a text part, causing loss of tool call information during streaming.

…or tools Extract and export `isDefaultCollapsedTool` function to centralize logic that determines which tool blocks should remain collapsed by default. Previously, only task board tools (`isTaskBoardTool`) were forced to collapsed state. Now the condition also includes the "render" tool, ensuring consistent behavior for all default-collapsed tools regardless of their state. Update `RuntimeThreadSurface` to use `isDefaultCollapsedTool` instead of `isTaskBoardTool`, both when preserving user state and when initializing new tool entries.

…I and remove unused import

…essage ID retrieval - Add in-memory `active_runs` reference to `AgentSession` for race-free access to streaming message IDs - Refactor active message ID retrieval to prioritize in-memory state over DB fallback - Implement artifact event buffering in React runtime thread surface to handle artifacts arriving before their target message exists in state

…nd surface components Extract the preview overlay and content rendering logic from `ThreadChartArtifactCard` into reusable `WorkbenchPreviewOverlay` and `FilePreviewSurface` components. This eliminates duplicated DOM structure and prepares for future preview use cases beyond chart artifacts. - Create `WorkbenchPreviewOverlay` as a generic full-screen overlay with backdrop blur, header, and scrollable content area - Create `FilePreviewSurface` to handle HTML, SVG, and Markdown preview content with proper sandboxing and lazy imports - Refactor `ThreadChartArtifactCard` to use `FilePreviewSurface`, removing ~30 lines of inline preview code

github-actions

Automated PR review completed.

Findings kept: 17
Findings with unknown confidence: 0
Inline comments attempted: 16
Target files: 36
Covered files: 36
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

+      return (
+        <div
+          className="flex min-h-full w-full items-center justify-center p-6"
+          dangerouslySetInnerHTML={{ __html: source }}


Sign in to view

      );
    });

+    stream.onArtifact = withActiveStream((event) => {


Sign in to view

+  );
+}
+
+export function ThreadChartArtifactCard({ part }: ThreadChartArtifactCardProps) {


Sign in to view

+        // Resolve the message to attach the artifact to
+        let target_message_id = {
+            let runs = self.active_runs_message_id().await;
+            runs.unwrap_or_else(|| format!("chart-msg-{}", &artifact_id[..8]))


Sign in to view

+        message_id: String,
+        artifact_id: String,
+        artifact_type: String,
+        status: String,


Sign in to view

+        </div>
+      </div>
+
+      {isHtmlSvg && part.source ? (


Sign in to view

+                        Ok(path) => match tokio::fs::read_to_string(&path).await {
+                            Ok(content) if !content.is_empty() => Some(content),
+                            Ok(_) => None,
+                            Err(e) => {


Sign in to view

                checkpoint_requested: AtomicBool::new(false),
                abort_signal: tiycore::agent::AbortSignal::new(),
                context_compression_state,
+                active_runs,


Sign in to view

+const VALID_ARTIFACT_STATUSES = new Set(["started", "delta", "completed", "failed"]);
+
+function readArtifactStatus(rawStatus: string): "started" | "delta" | "completed" | "failed" {
+  if (VALID_ARTIFACT_STATUSES.has(rawStatus)) {


Sign in to view

+      artifactId: string;
+      artifactType: string;
+      status: "started" | "delta" | "completed" | "failed";
+      payload?: unknown;


…tatus enum Replace string-based artifact status with the `ArtifactStatus` enum in the Rust backend, improving type safety across `ThreadStreamEvent::ArtifactUpdated` events and corresponding tests. In the frontend, move the `mergeArtifactPartIntoMessage` function from the `runtime-thread-surface` component into the `runtime-thread-surface-state` module, exporting it with a dedicated `ArtifactEvent` type. Add comprehensive unit tests for the extracted function to ensure correct handling of all artifact lifecycle states.

github-actions

Automated PR review completed.

Findings kept: 13
Findings with unknown confidence: 0
Inline comments attempted: 13
Target files: 37
Covered files: 37
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

github-actions · 2026-05-07T17:27:22Z

        }
    }

+    async fn execute_render(


[HIGH] No integration test covering the new render tool execution path

Add integration tests that exercise execute_render for vega-lite, html (inline source), html (file path), edge cases like empty source, missing spec, oversized source, and path outside workspace.

Suggestion: Create tests in src-tauri/tests/agent_run.rs (or a new agent_render.rs) that spin a real session, call the render tool via tool dispatch, and assert the emitted ToolRequested/ArtifactUpdated/ToolCompleted events and DB records.

Confidence: 0.90

[From SubAgent: testing]

github-actions · 2026-05-07T17:27:22Z

    pub run_id: Option<String>,
    pub role: String,
    pub content_markdown: String,
+    pub parts_json: Option<String>,


[HIGH] Missing DB migration for new parts_json column

The parts_json field is added to the MessageRecord struct but no corresponding database schema migration is provided.

Suggestion: Add a migration to alter the messages table (e.g., ALTER TABLE messages ADD COLUMN parts_json TEXT) and handle existing rows appropriately.

Risk: Existing databases will be missing the column, causing runtime errors when the field is read or written.

Confidence: 0.90

[From SubAgent: general]

github-actions · 2026-05-07T17:27:22Z

+    Ok(())
+}
+
+pub async fn merge_chart_artifact_part(


[HIGH] Race condition in merge_chart_artifact_part: read-modify-write is not atomic

Concurrent calls to merge_chart_artifact_part for the same message ID can lose previously merged parts because the read-modify-write sequence is not atomic.

Suggestion: Wrap the find_by_id → serialize → replace_parts sequence in a database transaction (BEGIN IMMEDIATE / COMMIT) using sqlx or connection-level transactions to ensure atomicity.

Risk: Under concurrent chart artifact completions, a message's parts array may silently lose chart entries, leading to missing visualizations in the UI.

Confidence: 0.85

[From SubAgent: general]

github-actions · 2026-05-07T17:27:22Z

    "tailwind-merge": "^3.3.1",
    "tw-animate-css": "^1.3.7",
    "use-stick-to-bottom": "^1.1.3",
+    "vega": "^6.2.0",


[MEDIUM] Missing UI integration for newly added Vega dependencies

Vega, vega-embed, and vega-lite are added to package.json without any consuming UI code in the current diff, risking them becoming orphaned dependencies.

Suggestion: Either wire up a Vega chart component using these packages or remove them from the dependency list to prevent unused dependency accumulation and unnecessary bundle size growth.

Risk: Dead code, increased bundle size, dormant maintenance burden.

Confidence: 0.90

[From SubAgent: general]

github-actions · 2026-05-07T17:27:22Z

+        // Resolve the message to attach the artifact to
+        let target_message_id = {
+            let runs = self.active_runs_message_id().await;
+            runs.unwrap_or_else(|| format!("chart-msg-{}", &artifact_id[..8]))


[MEDIUM] Chart artifact target message can be a synthetic fallback ID

When active_runs_message_id() returns None, the render tool uses a synthetic message ID like chart-msg-xxxx. This synthetic ID is persisted via merge_chart_artifact_part and emitted in ArtifactUpdated events, but it does not correspond to any DB message. The UI may try to display chart artifacts on a non-existent message, which can cause missing charts in the conversation history.

Suggestion: Ensure the artifact always attaches to a real message. Options: (a) make the render tool wait until a message is created (e.g., retry with a timeout loop), (b) ensure the caller always provides a message ID in the tool input, or (c) document and handle synthetic IDs on the frontend if this is intentional.

Risk: Chart artifacts may not display in the conversation if no real message was created yet, leading to a poor user experience.

Confidence: 1.00

[From SubAgent: general]

github-actions · 2026-05-07T17:27:22Z

    Ok(())
 }

+pub async fn replace_parts(


[MEDIUM] Missing test coverage for new DB functions

replace_parts and merge_chart_artifact_part are untested; the new parts_json field is only exercised with None in existing tests.

Suggestion: Add mod tests cases for replace_parts (null parts, valid JSON, empty string) and for merge_chart_artifact_part (message not found, empty parts, merging into existing text/chart parts, serialization failure).

Risk: Regressions in parts merging logic may not be caught until manual testing; serialization errors could cause unexpected runtime failures.

Confidence: 0.85

[From SubAgent: testing]

github-actions · 2026-05-07T17:27:22Z

+/// Validates the complete render data flow: tool_call persistence,
+/// chart artifact merge into message parts, and expected DB state.
+#[tokio::test]
+async fn test_render_tool_persists_tool_call_and_chart_artifact() {


[MEDIUM] No test for concurrent or out-of-order tool call updates

Lack of concurrency/ordering tests for render tool call persistence

Suggestion: Add a test that simulates two overlapping render tool calls or out-of-order (requested/processing/completed) updates to ensure idempotency and correct DB state.

Risk: Race conditions or inconsistent tool_call statuses under concurrent agent execution may go undetected.

Confidence: 0.85

[From SubAgent: testing]

github-actions · 2026-05-07T17:27:22Z

            run_id: None,
            role: "system".to_string(),
            content_markdown: "Context is now reset".to_string(),
+            parts_json: None,


[LOW] Missing DB fallback after persist_clear_context_reset_to_pool

The diff adds parts_json: None to all MessageInsert struct instantiations across several files, which is consistent. However, the struct likely already had a parts_json field. There is no evidence that this is wrong, but the diff does not show the struct definition update. If the field was newly added elsewhere, this is fine; if it is an older field, the diff simply ensures consistency.

Suggestion: Verify that MessageInsert already defines parts_json (possibly Option<serde_json::Value>). If it doesn't, define it before merging.

Risk: Minor compilation error if the struct doesn't include this field, but the existing CI typecheck should catch it.

Confidence: 0.85

[From SubAgent: general]

github-actions · 2026-05-07T17:27:22Z

        "INSERT INTO messages (id, thread_id, run_id, role, content_markdown,
-                message_type, status, metadata_json, attachments_json, created_at)
-         VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, strftime('%Y-%m-%dT%H:%M:%fZ', 'now'))",
+                parts_json, message_type, status, metadata_json, attachments_json, created_at)


[LOW] INSERT parameter count mismatch in comment vs actual bind arguments

The SQL INSERT statement now accepts 11 columns/values after adding parts_json, but the diff shows the old pattern. Ensure the new count is correct and verify the VALUES list is aligned.

Suggestion: No runtime bug; just verify that the VALUES clause actually has 11 placeholders in the final file. The diff suggests the code is correct, but a quick manual check is good hygiene.

Risk: Minimal — if the VALUES list were accidentally left with 10 placeholders, it would be a compile/test blocker; this seems cosmetic.

Confidence: 0.90

[From SubAgent: general]

github-actions · 2026-05-07T17:27:22Z

+  children,
+  className,
+}: WorkbenchPreviewOverlayProps) {
+  if (!open) return null;


[LOW] WorkbenchPreviewOverlay renders DOM nodes even when closed (conditional return vs CSS)

The overlay correctly avoids rendering any DOM when open is false, minimizing memory footprint.

Suggestion: None; this is the optimal approach.

Risk: None.

Confidence: 0.85

[From SubAgent: performance]

jorben added 3 commits May 6, 2026 20:32

Merge branch 'master' into feat/thread-chart-artifact-rendering

41b0f09

github-actions Bot reviewed May 6, 2026

View reviewed changes

jorben added 2 commits May 7, 2026 08:17

style(tests): 🎨 format assert line for readability

c2a2355

github-actions Bot reviewed May 7, 2026

View reviewed changes

jorben added 11 commits May 7, 2026 10:01

feat(chart): ✨ Adapt Vega-Lite theme to application theme

dcd787d

feat(runtime-thread-surface): ✨ add render_chart to default collapsed…

a7186d7

… tools

feat(repo): ✨ add source field to chart artifact merge

3e9ab8b

refactor(workbench-shell): ♻️ reorganize thread-chart-artifact-card U…

e631cd4

…I and remove unused import

feat(ui): ✨ replace dialog with custom preview overlay and use CodeBlock

2567766

github-actions Bot reviewed May 7, 2026

View reviewed changes

		@@ -0,0 +1,178 @@
		import { Component, useEffect, useRef, useState } from "react";

Conversation

jorben commented May 6, 2026

Summary

Changes

Backend (Rust)

Frontend (TypeScript/React)

Dependencies

Motivation

Testing

Breaking Changes

Checklist

Uh oh!

github-actions Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Code Review Summary

Overall Assessment

Major Findings by Severity

Actionable Suggestions

Potential Risks

Test Suggestions

File-Level Coverage Notes

Inline Downgraded Items (processed but not inline)

Coverage Status

Runtime/Budget

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot commented May 6, 2026 •

edited

Loading