Skip to content

feat: Agent streaming, animation fixes, and recursion guard#25

Merged
Ker102 merged 42 commits intomainfrom
feature/agent-streaming-animation-fixes
Mar 21, 2026
Merged

feat: Agent streaming, animation fixes, and recursion guard#25
Ker102 merged 42 commits intomainfrom
feature/agent-streaming-animation-fixes

Conversation

@Ker102
Copy link
Copy Markdown
Owner

@Ker102 Ker102 commented Mar 21, 2026

Summary

This PR contains agent infrastructure improvements, animation system fixes, and UI polish across ~41 commits.

🔧 Agent Infrastructure

  • Recursion guard: Executor prompt now caps step recovery attempts to a maximum of 3 (was unlimited — caused 684s loop on Test 16 MoMask)
  • Skip unavailable services: Agent now explicitly told to skip external AI services it doesn't have tools for, rather than attempting via execute_code
  • Dedup middleware: Cache key uses toolName:stableHash(args) to prevent identical re-calls
  • Agent stop button: Users can now abort running agent sessions + mode-switch unmount fix

🎬 Animation System

  • Blender 5.0+ channelbag API: Replaced all removed action.fcurves references in animation-guide.md with ActionChannelbag.fcurves via anim_utils.action_get_channelbag_for_slot()
  • Interpolation guidance: Clarified both pt.easing = 'EASE_IN_OUT' and handle_left_type = 'AUTO_CLAMPED' are valid (verified against official Blender API docs)
  • Material animations: Updated to channelbag for node_tree fcurves

🖥️ UI/UX

  • Thinking indicator: Shows "Thinking..." state while agent processes
  • Tool call logging: Simplified — single line per completed tool (success/fail), no spam
  • Agent streaming refactor: Reverted to agent.invoke() (ReactAgent doesn't support .stream())
  • Duplicate prompt fix: Fixed UI bug where prompt appeared twice on new sessions

🧹 Cleanup

  • Removed legacy agent middleware code
  • CRAG judge switched to faster model
  • Various memory leak and state cleanup fixes

Testing

  • Test 15 (bouncing ball animation): ✅ 64s, 0 errors
  • Test 16 (MoMask): Now gracefully skips unavailable MoMask service instead of recursion loop
  • Blender 5.0 animation API verified against official docs

Summary by CodeRabbit

  • New Features

    • Image attachments in chat and multimodal agent input
    • Real-time agent activity panel with per-tool status and a Stop button
    • Scene-state auto-injection for context-aware responses
    • Graded, concurrent retrieval (CRAG) for richer search results
  • Improvements

    • Stronger deduplication, retry/fallbacks, and error recovery during execution
    • Tighter follow-up generation and streaming event handling
    • Conversation workflow modes (autopilot vs studio) controls
  • Documentation

    • Extensive new Blender tool guides, canonical architecture, and roadmap
    • Research‑first content workflow enforced via new settings file

Ker102 added 30 commits March 19, 2026 17:13
Root cause: lastCalls Map was keyed by tool name only, so calls to
the same tool with different args (e.g. create_material for Wood then
Silver) overwrote each other's cache entries. True duplicates like
create_material(Ceramic) x2 slipped through.

Changes:
- Cache key changed from toolName to toolName:stableHash(args)
- Added stable key sorting so {a:1,b:2} == {b:2,a:1}
- Added 'No Duplicate Calls' rule #8 to system prompt
- Added dedup hint to executor prompt
- Autopilot: AbortController ref + Stop Processing button replaces Send when active
- Studio: onStop prop on StepSessionDrawer with red Stop button in running indicator
- studio-layout: handleStopStep aborts controller and marks step as failed
- Mode-switch fix: StudioLayout now uses CSS display:none instead of conditional
  render, preventing running agents from being killed on Autopilot switch
…leakage

- Add workflowMode column to Conversation model (default: autopilot)
- ensureConversation now filters by workflowMode when reusing conversations
- page.tsx only loads autopilot conversations for the project chat history
- Migration: add_workflow_mode_to_conversations applied to Neon
Two new general-purpose 3D engineering skills documents for the Blender agent:

- spatial-positioning-guide.md: rotation-aware placement, bounding-box math,
  world-space coordinate calculation, alignment patterns, real-world dimensions
- object-assembly-guide.md: connection point formulas, 5 archetypes
  (lamp/bench/table/bookshelf/vehicle), parent-child assembly, verification

Research backed by NotebookLM deep research with citation-verified sources.
Two changes to address agent generating duplicate tool calls:

1. blender-agent-system.md: Strengthened 'No Duplicate Calls' rule from
   a single generic line to explicit CRITICAL guidance with specific tool
   examples (create_material, assign_material, add_modifier, move_to_collection)

2. route.ts: Session logging now filters out _skipped_duplicate tool calls
   by building a set of skipped tool_call_ids from tool response messages.
   Only actually-executed calls are counted, giving accurate metrics.

Root cause: LLM re-issued identical calls across ReAct turns. Dedup
middleware caught them (confirmed via stableHash testing) but the log
still counted all 25 AI-requested calls instead of the ~15 real ones.
…set)

Professional test prompts targeting real-world use cases:
- Test 11: creative spatial reasoning (room layout, furniture placement,
  wall-relative positioning, warm atmospheric lighting, material variety)
- Test 12: technical game dev (exact dimensions, emissive materials,
  symmetric placement, origin-centered for tiling, GLB export)
- Added attachments to chatRequestSchema for multimodal image input
- Build image_url content array when attachments present, passed to LangChain agent
- Created createStreamingMiddleware emitting agent:tool_call events (started/completed/failed)
- Added AgentToolCall and AgentToolResult types to types.ts
- Created agent-activity.tsx component with hammer icon + tool name display
- Integrated activity panel into project-chat.tsx above input form
- Auto-activate agent panel on first tool_call event
- architectural-completeness-guide.md: component completeness principles,
  window glass pane patterns, boolean best practices, surface categories
- scene-composition-guide.md: furniture placement rules, camera framing,
  lighting balance, spatial organization
- material-realism-guide.md: functional surface differentiation, EEVEE
  glass/IOR setup, PBR realism rules, material organization

Research-backed via NotebookLM with 27 web sources covering Blender
architectural modeling, PBR workflows, and interior lighting.
All documents Blender 5.x compatible.
Complex scenes like Test 12 (dungeon wall + torches + emissive + export)
require more than 25 tool calls, hitting the default recursion limit.
- Clear agentEvents, agentActive, monitoringLogs, monitoringSummary
  when sending a new message for fresh streaming state
- Old failed plan blocks show faded with 'Previous run' label
- Detailed error content (steps, logs, etc.) hidden for old messages
  to prevent cluttering chat with stale error info
Teaches the agent to:
- Never substitute raw primitives for complex objects
- Match object style to scene context (medieval, modern, etc.)
- Use execute_code for multi-part decorative geometry
- Includes procedural patterns for torches, chairs, etc.
- Quality checklist for visual conviction
Removes torch/chair specific Python code, adds generic
multi-component assembly pattern and few-shot decomposition hints.
Prevents RAG from surfacing scene-specific code at wrong times.
- Test 13: Rigify biped rigging with weight painting
- Test 14: UniRig AI auto-rigging for quadrupeds
- Test 15: Keyframe bouncing ball animation with easing
- Test 16: MoMask AI text-to-motion walking animation
Consolidated from 3d-pipeline-strategy.md, addon-integration-roadmap.md,
and current session notes. This is now the single source of truth for
planned features. Future plans should NOT be put in gemini.md.
…sh prep and weight cleanup

- New rigging-guide.md: research-backed guide covering mesh prep, per-bone alignment, Rigify workflow, post-rigging weight cleanup, common pitfalls
- auto_rigify.py: added prepare_mesh_for_rigging(), post_rigging_weight_cleanup(), alignment warning
- future-plans.md: marked rigging as functional (quality WIP), linked new guides
- animation-guide.md: placeholder to be rewritten with proper research
- Rewrote animation-guide.md with NotebookLM research citations
- Covers Blender 5.0 breaking changes, keyframe performance hierarchy
- F-curve access/find API, modifier creation, interpolation patterns
- 12 Disney animation principles with volume-preserving code
- Added .gemini/settings.json workspace rule: research-first for docs
- Vertex group architecture (weights in vertex group elements, not groups)
- Python weight assignment: group.add() and bmesh deform layer (high-perf)
- Post-binding cleanup pipeline: limit 4, smooth 3x, normalize, clean
- Game engine export rules: Unity/Unreal max 4 bones per vertex
- Smoothing techniques: Smooth tool (Selected Pose Bones subset), Blur brush
- Heat weighting troubleshooting: scale-up, merge, decimate, data transfer
- Replace bare similaritySearch(limit:3) with dual-source CRAG in chat route
- Search both blender-scripts AND tool-guides with LLM relevance grading
- topK=5 per source, minSimilarity=0.35, 15s timeout safety
- Guides first (planning context), then scripts (code examples)
- Remove triggered_by from 6 skill guides (rigging, animation, weight-painting,
  aesthetic-quality, architectural-completeness, scene-composition)
- Skill guides now flow through CRAG semantic search, not TOOL_GUIDE_MAP
- Remove unused similaritySearch import from chat route
CRAG's 10+ LLM grading calls caused stream timeouts (Controller already closed).
Now uses direct similaritySearch(topK=5) across both blender-scripts and
tool-guides sources in parallel — completes in ~1s vs 15s+.
- Rewrote gradeDocuments() to batch all docs into a single prompt
- Gets JSON array response grading all docs at once
- maxOutputTokens: 1024 for batch, doc summaries capped at 800 chars
- Falls back to similarity-based heuristic if batch call fails
- Re-enabled CRAG in chat route with similarity-search fallback
- If CRAG times out/fails, bare similaritySearch still provides context
- Added userStoppedRef to distinguish user Stop from timeout abort
- Stop handler sets flag and lets catch/finally cleanup properly
- User stop: replaces empty assistant placeholder with inline
  'Stopped by user' message, clears error and isRetryable
- Timeout abort: still shows error banner with retry option
- Removed direct setIsSending(false) from stop handler to prevent
  race condition that left stale empty assistant messages
…preview

Document grading is a simple classification task — using Pro (slow, expensive)
was overkill. Flash Lite is ~100x cheaper and much faster for this use case.
- blender-scripts: topK 5 -> 4
- tool-guides: topK 5 -> 3 (only 16 docs total, 5 was too many)
- Added per-doc grade logging to verify CRAG filtering behavior
Ker102 added 11 commits March 21, 2026 01:52
- Switch agent.invoke() to agent.stream() with streamMode=values for real-time events
- Emit agent:reasoning SSE events with LLM thinking text deltas
- Add inline reasoning bubble (Agent thinking) in project-chat.tsx
- Add TOOL_LABELS map to step-session-drawer.tsx for friendly tool names
- Improve follow-up message generation with scene-aware prompts
- Add AgentReasoning type to orchestration types
- Reset agentReasoning state on new message send
- agent.stream() caused 'not a function' error since createAgent from langchain returns ReactAgent
- Reasoning now extracted post-execution from AI messages without tool calls
- Client handler: replace reasoning text per event (not append)
- Render reasoning bubble whenever text exists (not only when agentActive)
- Remove agentReasoning state, SSE handler, and reasoning text bubble
- Replace with clean Thinking spinner that shows during agent activity
- AgentActivity component handles friendly tool call names
- Remove server-side agent:reasoning emission (no longer needed)
- Export executeMcpCommand from agents.ts for external use
- Pre-call get_scene_info before agent.invoke() in route.ts, prepending
  scene state to the prompt so the agent knows existing objects
- Add AGENT BEHAVIORAL RULES section to rigging-guide.md:
  - Always delete default objects first
  - Never pose the rig after creation (leave in T-pose)
  - Never add materials unless explicitly asked
  - Prefer Rigify workflow over custom armature scripts
  - Scene cleanup comes before geometry creation

Fixes Test 13 issues: agent not deleting default cube, posing IK
hands unprompted, applying blue material without being asked.
Replace duplicated 40-line code block (identical to rigging-guide.md §3)
with a concise summary + cross-reference. Saves ~200 tokens per retrieval.
…indicator

Bug 1: Guard useEffect sync with isSending to prevent router.refresh()
from overwriting local messages state during streaming.

Bug 2: Emit agent:planning_start SSE event before agent.invoke() so the
UI sets agentActive=true and renders the Thinking indicator + AgentActivity.
…ith Thinking

Step-session-drawer.tsx:
- Filter first user message when it matches YOUR PROMPT header
- Replace 'Generating...' with 'Thinking...' using accent color
- Add AgentActivity component for tool call display (wired-ready)

Workflow-timeline.tsx:
- Add optional agentEvents property to WorkflowTimelineStep

Project-chat.tsx:
- Guard useEffect sync with isSending to prevent router.refresh() race

Route.ts:
- Emit agent:planning_start SSE event before agent.invoke()
- Handle agent:tool_call SSE events to show commands one-by-one as they execute
- appendCommandResult helper with started/completed pairing
- appendAgentEvent helper for AgentActivity component
- Prevent complete event from overwriting live-streamed results
- Add agentEvents to WorkflowTimelineStep type
…ailure

- Skip 'started' events, only add commandResults on completed/failed
- Clean one-liner labels: 'Tool call: <name>' / 'Execution of <name> failed'
- Failed entries get red-tinted background
- Header renamed to 'Tool Calls' from 'Executed Commands'
- Move UniRig/MoMask/MeshAnything to P3 autopilot section
- Update RunPod status: Hunyuan Paint/Part deployed, others not
- Near-term rigging via addon integration, not neural models
- Document prerequisite order: agent+RAG → addons → neural wiring
…nt recovery retries

- animation-guide.md: Replace removed action.fcurves with channelbag API
  (action_get_channelbag_for_slot) across all sections
- animation-guide.md: Add handle_type alternative for interpolation control
- executor.ts: Cap step recovery attempts to maximum 3 (was unlimited)
- executor.ts: Add instruction to skip unavailable external AI services
  rather than looping with execute_code
@github-actions github-actions bot added documentation Improvements or additions to documentation frontend backend scripts database labels Mar 21, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 21, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Adds research-first governance, a CRAG retrieval flow with scene-state injection and multimodal inputs, agent streaming events and UI for tool-call activity, dedup/streaming middleware and exported MCP command, many Blender tool guides/scripts, schema/workflowMode support, diagnostics scripts, and architecture/docs additions.

Changes

Cohort / File(s) Summary
Governance / Config
\.gemini/settings.json
New research-first document creation rule set enforcing NotebookLM (preferred) or equivalent research tooling, per-topic notebooks, citation-backed queries, single-document research→write→commit workflow, and Blender 5.x API verification for specified markdown/Python paths.
Chat API / Retrieval (CRAG)
app/api/ai/chat/route.ts, lib/ai/crag.ts
Replaced single similaritySearch RAG with concurrent CRAG retrieval (blender-scripts + tool-guides), batch document grading, 15s timeout, graded merging, graded-fallback path to ungraded similaritySearch, proactive MCP scene-state injection, multimodal agent input when attachments present, and agent planning event emission.
Agent Middleware & Dedup
lib/ai/agents.ts, lib/orchestration/types.ts, lib/orchestration/prompts/blender-agent-system.md
Exported executeMcpCommand, switched dedup cache to toolName+stableHash(args), added createStreamingMiddleware emitting agent:tool_call events (started/completed/failed), and added AgentToolCall/AgentToolResult/AgentReasoning event types plus idempotency guidance in system prompt.
Agent UI / Streaming Controls
components/projects/agent-activity.tsx, components/projects/project-chat.tsx, components/projects/step-session-drawer.tsx, components/projects/studio-layout.tsx, components/projects/workflow-timeline.tsx
New AgentActivity component; project chat now tracks AbortController, user-stop, clears agent/monitoring state on send, streams agent events, shows "Thinking…" and stop button, dims previous runs; step drawer/studio layout handle agent events, append command results/agentEvents, and expose onStop hook; workflow step type extended with agentEvents.
Executor / Orchestration
lib/orchestration/executor.ts
Agent prompt updated to enforce up-to-3 recovery attempts, avoid re-calling same tool with identical args, and treat unavailable external services as unavailable (no replication via execute_code).
Blender Scripts
data/blender-scripts/auto_rigify.py
Added prepare_mesh_for_rigging() (subdivide/remove doubles/fix normals) and post_rigging_weight_cleanup() (limit influences, smooth, normalize, cleanup); docstring warning on align_metarig_to_mesh.
Tool Guides (many docs)
data/tool-guides/*.md
Added nine comprehensive Blender/tool guides (aesthetic-quality, animation, architectural-completeness, material-realism, object-assembly, rigging, scene-composition, spatial-positioning, weight-painting) covering workflows, Blender 5.x notes, code patterns, and checklists.
Docs & Roadmap
docs/*.md, gemini.md
Added canonical docs/architecture.md, docs/future-plans.md, docs/rag-scaling-plan.md, extended docs/test-prompts.md with Tests 11–16, marked architecture-notes.md legacy, and updated gemini.md progress log.
Database Schema
prisma/schema.prisma
Added Conversation.workflowMode: String @default("autopilot") @db.VarChar(20) to support workflow mode filtering.
Diagnostics & Scripts
scripts/check-tool-args.ts, scripts/ingestion/rag-diagnostic.ts, scripts/rag-diagnostic.ts, scripts/test-tool-mutation.ts
New scripts for inspecting executed command args, RAG ingestion/embedding/search diagnostics, and a LangChain tool mutation inspector.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ChatAPI as Chat API Route
    participant CRAG
    participant VectorStore
    participant LLM
    participant MCP
    participant Agent

    Client->>ChatAPI: POST /chat (attachments?, workflowMode)
    ChatAPI->>MCP: executeMcpCommand("get_scene_info")
    MCP-->>ChatAPI: scene JSON (if available)

    ChatAPI->>CRAG: retrieve(query, ["blender-scripts","tool-guides"])
    CRAG->>VectorStore: similaritySearch (concurrent per-collection)
    VectorStore-->>CRAG: results (subject to 15s timeout)
    CRAG->>LLM: gradeDocuments(query, merged results)
    LLM-->>CRAG: graded array (batch)
    CRAG-->>ChatAPI: graded docs

    ChatAPI->>ChatAPI: build prompt (prepend scene-state + graded context)
    ChatAPI->>Agent: agent.invoke(prompt, multimodal attachments?)
    Agent->>Agent: emit agent:planning_start
    Agent->>MCP: tool calls...
    Agent-->>ChatAPI: stream events (agent:tool_call / results / reasoning)
    ChatAPI-->>Client: stream aggregated events and responses
Loading
sequenceDiagram
    participant User
    participant ProjectChat
    participant ChatAPI
    participant StudioLayout
    participant MCP

    User->>ProjectChat: handleSend(message)
    ProjectChat->>ProjectChat: clear agent state, create AbortController
    ProjectChat->>ChatAPI: fetch /chat (stream)
    ChatAPI->>ProjectChat: emit agent events (tool_call started/completed/failed)
    ProjectChat->>ProjectChat: append agentEvents, set agentActive=true
    ProjectChat-->>User: render AgentActivity, show stop button

    User->>ProjectChat: Click "Stop Processing"
    ProjectChat->>ProjectChat: abortController.abort(), mark userStopped
    ProjectChat-->>User: replace assistant placeholder with "⏹ Stopped by user"
    ProjectChat->>StudioLayout: append final commandResults / update step statuses
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

desktop

Poem

🐰
I nibbled through CRAG's tidy rows,
Found guides and scripts where knowledge grows,
Tools chatter, events hop by—so spry;
I pressed the stop, watched answers sigh,
Hooray for docs that help us fly! 🎉

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description provides good coverage of key changes across agent infrastructure, animation, UI/UX, and cleanup, but the template sections (Type of Change, Related Issues, Testing methodology) are not filled out according to the specified structure. Complete the PR description template by adding: Type of Change checkboxes, Related Issues section, Motivation/Context, and formalized Testing methodology (unit/integration/end-to-end test details).
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes three main changes: agent streaming, animation fixes, and a recursion guard, which are the core objectives of this PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/agent-streaming-animation-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 40

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
prisma/schema.prisma (1)

121-133: 🧹 Nitpick | 🔵 Trivial

Add an index for the new workflowMode query path.

Given filtering by mode and sorting by lastMessageAt, adding a composite index will improve conversation lookup latency at scale.

Suggested schema addition
 model Conversation {
   id            String   `@id` `@default`(dbgenerated("gen_random_uuid()")) `@db.Uuid`
   projectId     String   `@db.Uuid`
   sessionId     String?  `@db.VarChar`(255)
   workflowMode  String   `@default`("autopilot") `@db.VarChar`(20)
   createdAt     DateTime `@default`(now())
   lastMessageAt DateTime `@default`(now())
@@
   @@map("conversations")
+  @@index([projectId, workflowMode, lastMessageAt])
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@prisma/schema.prisma` around lines 121 - 133, The Conversation model is
missing a composite index for queries that filter by workflowMode and sort by
lastMessageAt; add a composite index on the Conversation model referencing the
workflowMode and lastMessageAt fields (e.g., @@index([workflowMode,
lastMessageAt], name: "idx_conversation_mode_lastMessageAt")) to speed up those
lookups and then generate/apply a migration so the DB gets the new index; target
the Conversation model and the fields workflowMode and lastMessageAt when making
the change.
components/projects/project-chat.tsx (2)

420-425: ⚠️ Potential issue | 🟠 Major

Gate free-tier sends before the optimistic UI mutation.

The return on Line 424 happens after Lines 400-417 have already appended temp messages and cleared the composer/attachments. On an unconfigured free-tier install, the user sees an unsent phantom turn and loses the selected images even though no request was sent.

Suggested fix
-    setIsSending(true)
-    setError(null)
-    setInput("")
-    // Clear agent streaming state for fresh run
-    setAgentEvents([])
-    setAgentActive(false)
-    setMonitoringLogs([])
-    setMonitoringSummary(null)
-    setMessages((prev) => [
-      ...prev,
-      {
-        id: tempUserId,
-        role: "user",
-        content: trimmed,
-        createdAt: now,
-        attachments: draftAttachments,
-      },
-      {
-        id: tempAssistantId,
-        role: "assistant",
-        content: "",
-        createdAt: now,
-        mcpCommands: [],
-      },
-    ])
-    setAttachments([])
-
-    try {
-      if (subscriptionTier === "free" && !localReady) {
+    if (subscriptionTier === "free" && !localReady) {
         setError(
           "The free tier requires a local LLM. Configure one in Settings → Local LLM Configuration before prompting."
         )
         return
-      }
+    }
+
+    setIsSending(true)
+    setError(null)
+    setInput("")
+    // Clear agent streaming state for fresh run
+    setAgentEvents([])
+    setAgentActive(false)
+    setMonitoringLogs([])
+    setMonitoringSummary(null)
+    setMessages((prev) => [
+      ...prev,
+      {
+        id: tempUserId,
+        role: "user",
+        content: trimmed,
+        createdAt: now,
+        attachments: draftAttachments,
+      },
+      {
+        id: tempAssistantId,
+        role: "assistant",
+        content: "",
+        createdAt: now,
+        mcpCommands: [],
+      },
+    ])
+    setAttachments([])
+
+    try {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/projects/project-chat.tsx` around lines 420 - 425, The
free-tier/local-LLM gate runs after the optimistic UI mutation, causing a
phantom unsent message and lost composer/attachments; move the check of
subscriptionTier === "free" && !localReady to run before any optimistic updates
(before appending temp messages and before clearing composer/attachments) or, if
you prefer to keep ordering, undo the optimistic changes on early return by
removing the temp message and restoring the composer and attachments via the
same state setters used to mutate them. Ensure you reference and modify the code
paths that append temp messages and clear the composer/attachments so the gate
(using subscriptionTier, localReady, and setError) prevents any UI mutations or
properly reverts them when blocking the send.

216-232: ⚠️ Potential issue | 🟠 Major

Don't clear a locally initialized first conversation once streaming stops.

If the stream has already emitted init, conversationId is local-only until router.refresh() supplies initialConversation. When isSending flips false, the reset on Lines 228-231 wipes the thread whenever history.length === 0, so a stopped/failed first conversation disappears.

Suggested fix
-    if (!initialConversation?.id && conversationId && history.length === 0) {
+    if (
+      !initialConversation?.id &&
+      conversationId &&
+      history.length === 0 &&
+      messages.length === 0
+    ) {
       setConversationId(null)
       setMessages([])
     }
-  }, [initialConversation, conversationId, history, isSending])
+  }, [initialConversation, conversationId, history, isSending, messages.length])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/projects/project-chat.tsx` around lines 216 - 232, The clearing
branch wipes a locally-created first conversation when streaming stops; add a
boolean flag (e.g., conversationInitializedLocallyRef or
conversationInitializedLocally state) that you set true when you assign a new
conversationId locally (the code path that setsConversationId during stream
init), then change the final reset condition (the block that currently checks
!initialConversation?.id && conversationId && history.length === 0) to only
clear when the conversation was NOT initialized locally (i.e., require
!conversationInitializedLocally); reference setConversationId,
initialConversation, conversationId, history and isSending when making the
change so local-only conversations survive when isSending flips false.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/api/ai/chat/route.ts`:
- Around line 270-276: The attachments schema in route.ts currently allows
unbounded base64 data via data: z.string(); update the zod schema for
attachments (the attachments array object) to enforce a maximum size by
validating either the provided size field (e.g., size: z.number().max(...)) or
by validating the base64 string length (e.g., z.string().refine(b64 =>
calcBytesFromBase64(b64) <= MAX_BYTES, "attachment too large")), and also
tighten the type validation using a regex like /^image\/(jpeg|png|gif|webp)$/.
Apply these checks to the object inside attachments so oversized images are
rejected before processing.
- Around line 314-318: The find-first query inside the startNew branch
(prisma.conversation.findFirst) incorrectly filters by workflowMode: mode which
excludes existing rows that have workflowMode = NULL; modify that query to
include conversations where workflowMode equals mode OR workflowMode is null
(e.g., add an OR clause for workflowMode null) so existing conversations without
a workflowMode value are matched; reference the startNew variable and the mode
value when updating the WHERE clause to preserve the intended behavior and avoid
creating duplicate conversations.

In `@components/projects/agent-activity.tsx`:
- Around line 131-161: The header and recent list only consider events with
status === "completed", so failed tool runs are hidden; update the logic that
computes completedCount and recentCompleted (and the similar block around the
other recent-list code) to include events where status === "completed" OR status
=== "failed" (i.e., treat "failed" as a terminal state), and ensure any UI that
derives from toolEvents (variables toolEvents, completedCount, recentCompleted)
counts and slices these terminal events so failures surface in the header and
recent list.
- Around line 115-129: The current activeTool logic uses a Set which cannot
track multiple concurrent starts of the same tool; replace the Set with a
Map<string, number> (or an object) that counts active instances per tool:
iterate toolEvents and for each event do counts.set(toolName,
(counts.get(toolName)||0) + 1) on "started" and decrement counts (and delete
when zero) on non-"started"/"completed" events; then scan toolEvents backwards
and return the first event where event.status === "started" and
counts.get(event.toolName) > 0. Update the useMemo block (activeTool,
toolEvents) to use this counting Map so parallel/coalesced same-tool calls are
handled correctly.

In `@components/projects/project-chat.tsx`:
- Around line 729-732: The renderer is producing blank rows because
agent:tool_call events are appended to agentEvents but not handled in the render
switch; update the code that renders agentEvents (the component handling
evt.type branches) to add a branch for "agent:tool_call" that either skips
rendering (ignore/return null) or renders a proper tool-call row (e.g., show
tool name/payload), or alternatively stop pushing tool_call events into
agentEvents by adding a filter when calling setAgentEvents in the code that
handles agentEvent; reference setAgentEvents, agentEvents, and agentEvent to
locate where to add the conditional and implement the new branch or filter.
- Around line 395-399: The reset block clears agent/log state but forgets to
clear the run-scoped active workflow, causing the previous workflow to remain
visible; update the same reset sequence (where setAgentEvents, setAgentActive,
setMonitoringLogs, setMonitoringSummary are called) to also clear the active
workflow by invoking the state setter for activeWorkflow (e.g.,
setActiveWorkflow(null) or equivalent) so the UI won’t render the prior workflow
between runs.
- Around line 135-136: The component currently only clears abortControllerRef
later, leaving in-flight streams running on unmount; add a cleanup that aborts
any active controller and prevents further state updates by calling
abortControllerRef.current?.abort() and setting userStoppedRef.current = true
(and also clearing any associated timers) in a useEffect cleanup or the existing
effect's return handler so any ongoing fetch/stream created by
abortControllerRef and timers are cancelled when the component unmounts.
- Around line 757-766: The current isUserStop branch unconditionally replaces
the assistant message (msg.id === tempAssistantId) with "⏹ Stopped by user",
which discards any already-streamed deltas; change the update in setMessages so
that for the message with id tempAssistantId you preserve existing content if
present (e.g., if msg.content is non-empty keep msg.content and append a stop
marker like " ⏹ Stopped by user", otherwise set content to "⏹ Stopped by user"),
leaving other fields intact; update the handler around isUserStop,
tempAssistantId, and setMessages to perform this conditional-preserve logic.

In `@components/projects/step-session-drawer.tsx`:
- Around line 9-44: Extract the shared TOOL_LABELS mapping and getToolLabel
function into a new shared module (e.g., lib/ui/tool-labels) and replace the
inline definitions in components (including
components/projects/step-session-drawer.tsx and agent-activity.tsx) with imports
from that module; specifically move the constant TOOL_LABELS and the
getToolLabel(toolName: string) function to the new module, export them, and
update both files to import { TOOL_LABELS, getToolLabel } (or just getToolLabel)
so the mapping is centralized and duplication is removed.

In `@components/projects/studio-layout.tsx`:
- Around line 329-364: The default switch case block handling eventType is
indented four spaces too deep; fix by aligning the entire default: { ... } block
with the other switch cases (remove the extra indentation) so the internal logic
that uses eventType, appendMonitoringLog, updateStep, appendCommandResult, and
appendAgentEvent sits at the same indentation level as other case blocks for
consistent readability.
- Around line 303-319: The executeStep callback reads workflowSteps when
deciding whether to apply batch commandResults and currently closes over a stale
snapshot; fix by reading the live steps via a ref or performing the check inside
a state updater rather than relying on the captured workflowSteps. Concretely:
create a ref (e.g. workflowStepsRef) that you keep in sync whenever
workflowSteps changes, then inside executeStep (the block that computes
hasLiveResults and calls updateStep) use workflowStepsRef.current to locate
currentStep and compute hasLiveResults before applying patch; alternatively move
the hasLiveResults check into the updater passed to updateStep (or use the
updater form of setState) so it runs against the latest state and prevents batch
commandSuggestions from overwriting live-streamed results appended by
appendCommandResult.

In `@components/projects/workflow-timeline.tsx`:
- Around line 72-73: The agentEvents prop currently typed as agentEvents?:
Array<{ type: string; [key: string]: unknown }> is too loose; replace it with a
discriminated union of specific event variants (e.g., ToolCallEvent,
ToolResultEvent, MessageEvent) keyed by the type field and use that union as the
type for agentEvents in the component (workflow-timeline.tsx) so consumers
(renderers/handlers) can safely narrow on event.type and access typed payload
fields; update any uses of agentEvents, plus helpers like renderers or switch
statements, to switch on the discriminant and handle each variant explicitly.

In `@data/blender-scripts/auto_rigify.py`:
- Around line 33-70: The rigify_pipeline function never invokes the mandatory
prep/cleanup helpers; call prepare_mesh_for_rigging(mesh, subdivisions,
merge_threshold) at the start of rigify_pipeline before any binding/weighting
steps, and invoke a post-bind cleanup helper (e.g.,
cleanup_mesh_after_rigging(mesh) — implement it if missing) immediately after
binding and weight transfers to restore normals, apply transforms, and remove
temporary data; ensure the pipeline passes through the same mesh variable and
forwards configurable args (subdivisions, merge_threshold) so the high-level
entrypoint performs the advertised topology prep and post-bind cleanup.

In `@data/tool-guides/aesthetic-quality-guide.md`:
- Around line 17-122: The markdown has heading-spacing violations (MD022); add a
single blank line before each top-level and sub-heading shown (e.g., "##
ANTI-MINIMALISM RULES", "### Never Substitute Primitives for Complex Objects",
"### Minimum Geometry Budget Per Object", "### When to Use `execute_code`
Instead of Basic Tools", "## STYLISTIC COHERENCE", "### Match Object Style to
Scene Context", "### Contextual Material Selection", "## MULTI-COMPONENT
ASSEMBLY PATTERN (via `execute_code`)", "### Generic Template", "### Key
Techniques for Realistic Shapes", "### Few-Shot: What \"Multi-Part\" Means in
Practice") by inserting a blank line above each heading (and ensure there is a
blank line after the previous paragraph) so the file conforms to MD022 and
linting passes.
- Around line 1-7: This guide's frontmatter lacks the triggered_by field so it
won't be bound to tools; add a triggered_by metadata entry (e.g., triggered_by:
["execute_code"]) to the YAML frontmatter so lib/ai/agents.ts can load it into
TOOL_GUIDE_MAP and thus append it to the execute_code tool pipeline; update the
top-of-file frontmatter in data/tool-guides/aesthetic-quality-guide.md to
include that triggered_by key with the appropriate tool name.

In `@data/tool-guides/animation-guide.md`:
- Around line 1-7: Add a triggered_by frontmatter field to this markdown so the
guide is included in TOOL_GUIDE_MAP by lib/ai/agents.ts; update the frontmatter
in Animation & Keyframing Guide (the top YAML block with
title/category/blender_version) to include triggered_by: ["execute_code"] (or
the appropriate trigger name) so the guide binds to execute_code and will be
pulled into TOOL_GUIDE_MAP by the code that checks frontmatter.triggered_by.
- Around line 17-233: The markdown violates lint rules
(MD022/MD031/MD040/MD058/MD009): ensure every heading (e.g., "## CRITICAL:
Blender 5.0 Breaking Changes", "## 1. Performance: NEVER Use bpy.ops for
Keyframes in Loops", etc.) has a blank line before and after it, surround all
fenced code blocks (the ``` blocks) with blank lines and add explicit language
tags (e.g., ```python) where missing, remove all trailing spaces at line ends,
and make sure lists and tables have proper blank-line separation so the file no
longer triggers MD022/MD031/MD040/MD058/MD009 during linting.
- Around line 107-125: The example incorrectly sets pt.easing while also using
pt.interpolation = 'BEZIER'; remove the pt.easing assignment for BEZIER
keyframes and instead demonstrate controlling timing via handles
(pt.handle_left_type, pt.handle_right_type and handle positions) because easing
only applies to easing-based interpolation modes (e.g., SINE, QUAD, BOUNCE);
update the sections that set pt.interpolation = 'BEZIER' (and the specific-frame
example) to omit pt.easing and add a short note to use handle types/positions
for slow-in/out control on fcu.keyframe_points.

In `@data/tool-guides/architectural-completeness-guide.md`:
- Around line 27-40: Add a language specifier to the fenced code block (use
```python) and remove the trailing space at the end of the line containing the
second create_cube example to satisfy MD009; update the block that includes the
create_cube("Wall", ...) examples so it starts with ```python and ensure the
wrong-wall line has no trailing whitespace after the closing parenthesis.

In `@data/tool-guides/object-assembly-guide.md`:
- Around line 34-53: Update the Markdown formula code blocks to include a
language specifier (e.g., python or text) so the linter stops flagging them;
locate the blocks containing top_z, seat_z, arm_x/arm_z, and end_x/end_z and
wrap each fenced block with the appropriate opening triple-backtick followed by
the language token (for example: ```python) and close with ``` to match each
formula block.

In `@data/tool-guides/scene-composition-guide.md`:
- Around line 22-24: Fix spacing and fenced-code language issues causing
markdownlint failures: add a single blank line before and after each heading and
before/after fenced code blocks and tables (apply to the sections around
"Against-Wall Items" and the other ranges called out), and update the unnamed
object-name fenced block to use a language tag (e.g., replace ``` with ```text
for the object list block such as the one containing Room_Floor / Room_Wall_Back
/ Camera_Main). Ensure all fenced blocks have a blank line above and below and
that headings are separated by blank lines to satisfy MD022/MD031/MD058/MD040.

In `@data/tool-guides/spatial-positioning-guide.md`:
- Around line 24-174: The markdown is failing lint because several headings,
fences, and tables lack required blank lines and code blocks lack language tags;
fix by adding a blank line before and after each top-level and sub-heading
(e.g., "Origin-Aware Height Calculation", "ROTATION-AWARE POSITIONING",
"World-Space Bounding Box Calculation", "Effective Height After X-Axis
Rotation", etc.), ensure every code fence has a language tag (add ```python for
the get_world_bbox function and the mathutils/Matrix examples and ```python or
```text for small snippets), and add blank lines around every table so they are
separated from surrounding text (all the tables under primitives, Relative vs.
Absolute Positioning, Real-World Dimension Reference, Street Furniture,
Architecture, Human Reference). Also ensure fenced inline formulas use proper
fences with language tags (e.g., the single-line formulas can be in ```text).
These edits will satisfy MD022/MD031/MD040/MD058.
- Around line 87-98: The bottom_shift formula is incorrect: when rotating the
panel about its center the bottom-edge vertical displacement is bottom_shift =
(H/2)*(1 - cos(theta)) - (D/2)*sin(theta), not just (D/2)*sin(theta); update the
computation of bottom_shift (and its numeric approximation) in the example that
uses H, D, theta and adjust the explanatory comment to show the corrected small
net lift (~7e-4 m for H=0.8, D=0.1, θ=15°).

In `@data/tool-guides/weight-painting-guide.md`:
- Around line 1-7: The frontmatter for this guide is missing the required
triggered_by key so lib/ai/agents.ts will not map it to the execute_code
binding; add a triggered_by field to the guide's YAML frontmatter (e.g.,
triggered_by: ["execute_code"] or the appropriate trigger used in
lib/ai/agents.ts) so the guide is discoverable and attached to the agent
mapping; ensure the exact key name "triggered_by" matches what lib/ai/agents.ts
expects and include the correct trigger string(s) used by the agent mapping.
- Around line 17-192: The markdown fails markdownlint rules
MD022/MD031/MD040—add blank lines before and after each heading and before/after
fenced code blocks (e.g., the "CRITICAL: Weight Data Architecture" header and
all numbered section headers like "## 1. Assigning Weights via Python"), and add
language identifiers to every fenced code block (use python for all three blocks
and mark the table/code blocks accordingly). Ensure every fenced block has a
blank line above and below and that list blocks and blockquotes are separated
from surrounding headings with blank lines so the file becomes lint-clean.

In `@docs/architecture.md`:
- Around line 55-66: The docs have markdownlint spacing violations
(MD022/MD031/MD058) around headings, tables, and fenced blocks—fix by
normalizing blank lines: ensure there is exactly one blank line before and after
each heading (e.g., the "**Key files:**" heading), one blank line before and
after the pipes table that follows it, and one blank line before and after any
fenced code block or markdown fence; also remove unnecessary blank lines inside
fences and eliminate trailing spaces in fenced blocks and table rows so the
sections referenced (the Key files table and other nearby headings/fences)
comply with MD022/MD031/MD058.

In `@docs/future-plans.md`:
- Line 34: The fenced code block that lists features (the block containing
"Text-to-3D, Image-to-3D, Multiview-to-3D") lacks a language specifier and
triggers the markdown linter; update the opening fence from ``` to include a
language identifier (e.g., use ```text) so the block is treated as plain text,
save the change, and re-run the linter to confirm the warning is resolved.

In `@docs/test-prompts.md`:
- Around line 309-324: Update the pass/fail criteria for Test 14 (unirig) and
Test 16 so they tolerate executor-marked unavailable external AI services: in
the test descriptions that reference UniRig and the GLB export, add an
alternative path that treats a service-unavailable response from the executor as
a skipped/soft-fail condition rather than a hard failure, and ensure logs must
record the UniRig API call only if the service was available; preserve checks
for mesh, material, armature embedding, GLB export path, and the "< 180 seconds"
timing target but make timing and UniRig-specific verifications conditional on
the service being available (reference the test IDs "Test 14: AI Auto-Rigging
with UniRig (Neural Skeleton)" and any similar block for Test 16).

In `@gemini.md`:
- Around line 9-31: The status note in gemini.md is contradictory: it currently
states "TypeScript compilation verified clean" while also noting a TS error for
the missing ExecutionResult type in route.ts; update the wording to clarify
scope (e.g., "TypeScript compilation verified clean for recently touched files;
repo contains a known legacy error: missing ExecutionResult import in route.ts")
and explicitly reference the known issue by name ("ExecutionResult" in route.ts)
so readers aren't misled.

In `@lib/ai/agents.ts`:
- Around line 964-981: The onStreamEvent call currently emits a
"agent:tool_call" with status "started" before awaiting handler and returns
early when the result.content contains "_skipped_duplicate", leaving no terminal
event; update the post-handler branch where you detect "_skipped_duplicate"
(same scope that uses handler, result, content, and onStreamEvent) to emit a
terminal stream event (e.g., onStreamEvent({... type: "agent:tool_call",
toolName, status: "completed", timestamp: new Date().toISOString(), result: /*
minimal metadata or content marker */ })) before returning the result so every
"started" has a matching terminal event; ensure the emitted event includes
toolName and timestamp and preserves the early return behavior.
- Around line 972-999: The middleware currently treats any non-thrown return
from handler as success, but executeMcpCommand and similar functions return
error payloads (e.g. objects like { error: ... } or JSON strings) instead of
throwing; update the post-handler logic in the block that handles result
(variable result from handler) to detect these error payloads and emit a failed
tool_call instead of completed. Specifically, after awaiting handler(request)
(in the same scope that currently checks for "_skipped_duplicate"), check: if
result is an object and has an "error" property (e.g., typeof result ===
"object" && "error" in result) OR if result is a string (or result.content
string) that parses to JSON containing an "error" key or contains '"error"'
text, then call onStreamEvent with type: "agent:tool_call", toolName, status:
"failed", timestamp: new Date().toISOString() and return result; otherwise
proceed to emit the "completed" event as before. Ensure the skipped-duplicate
check remains in place and that you reference handler, result,
executeMcpCommand, onStreamEvent and tool_call when making the changes.
- Around line 886-889: stableHash currently only sorts top-level keys and
collapses distinct nested objects into identical cache keys; update stableHash
to perform a deep/recursive key sort before stringification (or call a utility
like stableStringify) so nested objects and arrays are fully normalized;
implement or reuse a deepSortObject helper that walks the value returned to JSON
(handling plain objects, arrays, primitives, and preserving types) and returns a
new object with all object keys sorted, then JSON.stringify that result in
stableHash to produce correct, distinct cache keys for nested differing args.

In `@lib/ai/crag.ts`:
- Around line 47-48: The batch grader must emit an explicit index/id per item
and the runtime must validate the parsed JSON array before trusting parsed[i];
update the prompt generation to require an "index" or "id" field for each object
and then, in the parsing/processing code that currently references parsed[i] and
maps to docs (the parsing block around parsed[i] and the similar block at lines
77-85), verify each element is an object containing a known id/index and that
"grade" matches the allowed enum values
("relevant"|"partially_relevant"|"not_relevant"); if validation fails (missing
id, wrong type, unexpected grade, duplicate/missing indices or out-of-range
index), reject or fallback (e.g., log error and skip/make conservative default)
rather than directly mapping by position so the resulting mapping to docs uses
the validated id/index field to preserve order and correctness.

In `@lib/orchestration/executor.ts`:
- Around line 146-148: The prompt text in executor.ts currently forbids only
“identical arguments”; update that string to match the system prompt by
forbidding “identical or equivalent parameters” instead, e.g., replace the
clause "`Do NOT call the same tool with identical arguments more than once`"
with wording that reads "`Do NOT call the same tool with identical or equivalent
parameters more than once`" and ensure the surrounding guidance (the subsequent
sentences referencing checking previous tool results) remains intact so callers
check prior tool outputs before making a new call.

In `@prisma/schema.prisma`:
- Line 125: The schema adds a new field workflowMode (String
`@default`("autopilot")) but no corresponding migration was committed; run Prisma
migrations to add the column to the DB and commit the migration files. Execute
npx prisma migrate dev (or prisma migrate deploy in CI) to generate a migration
that adds the workflowMode column with the default, verify the migration applies
to existing environments (and seed/alter existing rows if needed), and commit
the generated files in prisma/migrations so future deployments and queries
referencing workflowMode succeed.

In `@scripts/check-tool-args.ts`:
- Around line 9-35: The script may leave the Prisma client connected if an error
is thrown before the current prisma.$disconnect() at the end of main(); wrap the
body of main() in a try/finally (or ensure prisma is disconnected in a finally
block) so that prisma.$disconnect() is always called even on errors—modify the
main function to call prisma.$disconnect() from a finally block (or call it
after awaiting main() in a top-level try/finally) and keep the existing
main().catch(console.error) behavior for logging.

In `@scripts/rag-diagnostic.ts`:
- Around line 1-95: The diagnostic logic in main(), including calls to
getDocumentCount, embedText, and similaritySearch, is duplicated between
scripts/rag-diagnostic.ts and scripts/ingestion/rag-diagnostic.ts; extract the
shared flow into a new reusable module (e.g., a runRagDiagnostic function
exported from lib/scripts/ragDiagnostic.js/ts) that encapsulates steps 1–3
(document counts, embedding generation, similarity search) and logging, then
have both scripts import and call that single runner (passing any config like
query or thresholds). Update the original main() to be a thin wrapper that
imports runRagDiagnostic and handles process.exit or error propagation so
behavior remains identical while removing duplicated code.
- Around line 85-93: The catch block that handles similarity search errors
currently logs the error but lets execution continue to the final "Diagnostic
Complete" and process.exit(0); change the catch in scripts/rag-diagnostic.ts so
that after logging (the err.message and err.stack) it either rethrows the error
or calls process.exit(1) to return a non-zero status; update the catch around
the similarity search (the try/catch that produces "Similarity search failed")
to use process.exit(1) (or throw err) so failures do not report success.

In `@scripts/test-tool-mutation.ts`:
- Around line 14-18: The test currently only logs before/after and then
unconditionally exits success, so failures are never reported; update the logic
around the testTool mutation (the testTool.description assignment and the
subsequent checks) to verify the mutation actually took effect: after assigning
testTool.description = testTool.description + "...GUIDE..." check if
testTool.description.includes("GUIDE") and if not write an error (console.error)
and call process.exit(1), otherwise print success and exit 0; ensure you remove
the unconditional success exit and use the include check to determine the
process exit code.
- Around line 20-25: The logs use JSON.stringify on property descriptors (desc
and protoDesc) which drops get/set accessors; import Node's util and replace
JSON.stringify(desc) / JSON.stringify(protoDesc) with util.inspect(desc,
{showHidden: true, depth: null}) (and same for protoDesc) so the getter/setter
functions are preserved in the output; update the top of the file to require or
import util if not present and use util.inspect where the PROPERTY_DESCRIPTOR
and PROTO_DESCRIPTOR are logged.

---

Outside diff comments:
In `@components/projects/project-chat.tsx`:
- Around line 420-425: The free-tier/local-LLM gate runs after the optimistic UI
mutation, causing a phantom unsent message and lost composer/attachments; move
the check of subscriptionTier === "free" && !localReady to run before any
optimistic updates (before appending temp messages and before clearing
composer/attachments) or, if you prefer to keep ordering, undo the optimistic
changes on early return by removing the temp message and restoring the composer
and attachments via the same state setters used to mutate them. Ensure you
reference and modify the code paths that append temp messages and clear the
composer/attachments so the gate (using subscriptionTier, localReady, and
setError) prevents any UI mutations or properly reverts them when blocking the
send.
- Around line 216-232: The clearing branch wipes a locally-created first
conversation when streaming stops; add a boolean flag (e.g.,
conversationInitializedLocallyRef or conversationInitializedLocally state) that
you set true when you assign a new conversationId locally (the code path that
setsConversationId during stream init), then change the final reset condition
(the block that currently checks !initialConversation?.id && conversationId &&
history.length === 0) to only clear when the conversation was NOT initialized
locally (i.e., require !conversationInitializedLocally); reference
setConversationId, initialConversation, conversationId, history and isSending
when making the change so local-only conversations survive when isSending flips
false.

In `@prisma/schema.prisma`:
- Around line 121-133: The Conversation model is missing a composite index for
queries that filter by workflowMode and sort by lastMessageAt; add a composite
index on the Conversation model referencing the workflowMode and lastMessageAt
fields (e.g., @@index([workflowMode, lastMessageAt], name:
"idx_conversation_mode_lastMessageAt")) to speed up those lookups and then
generate/apply a migration so the DB gets the new index; target the Conversation
model and the fields workflowMode and lastMessageAt when making the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 20bb88c1-4f4e-4fa3-b40c-306870795457

📥 Commits

Reviewing files that changed from the base of the PR and between 92afd67 and fe9a4e9.

⛔ Files ignored due to path filters (1)
  • server.log is excluded by !**/*.log
📒 Files selected for processing (34)
  • .gemini/settings.json
  • app/api/ai/chat/route.ts
  • app/dashboard/projects/[id]/page.tsx
  • components/projects/agent-activity.tsx
  • components/projects/project-chat.tsx
  • components/projects/step-session-drawer.tsx
  • components/projects/studio-layout.tsx
  • components/projects/workflow-timeline.tsx
  • data/blender-scripts/auto_rigify.py
  • data/tool-guides/aesthetic-quality-guide.md
  • data/tool-guides/animation-guide.md
  • data/tool-guides/architectural-completeness-guide.md
  • data/tool-guides/material-realism-guide.md
  • data/tool-guides/object-assembly-guide.md
  • data/tool-guides/rigging-guide.md
  • data/tool-guides/scene-composition-guide.md
  • data/tool-guides/spatial-positioning-guide.md
  • data/tool-guides/weight-painting-guide.md
  • docs/architecture-notes.md
  • docs/architecture.md
  • docs/future-plans.md
  • docs/rag-scaling-plan.md
  • docs/test-prompts.md
  • gemini.md
  • lib/ai/agents.ts
  • lib/ai/crag.ts
  • lib/orchestration/executor.ts
  • lib/orchestration/prompts/blender-agent-system.md
  • lib/orchestration/types.ts
  • prisma/schema.prisma
  • scripts/check-tool-args.ts
  • scripts/ingestion/rag-diagnostic.ts
  • scripts/rag-diagnostic.ts
  • scripts/test-tool-mutation.ts

Comment on lines +270 to +276
attachments: z.array(z.object({
id: z.string(),
name: z.string(),
type: z.string(),
size: z.number().optional(),
data: z.string(), // base64 encoded image data
})).optional(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Validate attachment size limits.

The attachments schema accepts data: z.string() for base64 image data without size validation. Large images could cause memory issues or exceed request body limits.

🛡️ Suggested validation
   attachments: z.array(z.object({
     id: z.string(),
     name: z.string(),
     type: z.string(),
     size: z.number().optional(),
-    data: z.string(), // base64 encoded image data
+    data: z.string().max(10_000_000), // ~7.5MB decoded, prevents oversized uploads
   })).optional(),

Also consider validating the type field:

type: z.string().regex(/^image\/(jpeg|png|gif|webp)$/),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
attachments: z.array(z.object({
id: z.string(),
name: z.string(),
type: z.string(),
size: z.number().optional(),
data: z.string(), // base64 encoded image data
})).optional(),
attachments: z.array(z.object({
id: z.string(),
name: z.string(),
type: z.string(),
size: z.number().optional(),
data: z.string().max(10_000_000), // ~7.5MB decoded, prevents oversized uploads
})).optional(),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/ai/chat/route.ts` around lines 270 - 276, The attachments schema in
route.ts currently allows unbounded base64 data via data: z.string(); update the
zod schema for attachments (the attachments array object) to enforce a maximum
size by validating either the provided size field (e.g., size:
z.number().max(...)) or by validating the base64 string length (e.g.,
z.string().refine(b64 => calcBytesFromBase64(b64) <= MAX_BYTES, "attachment too
large")), and also tighten the type validation using a regex like
/^image\/(jpeg|png|gif|webp)$/. Apply these checks to the object inside
attachments so oversized images are rejected before processing.

Comment on lines 314 to 318
if (!startNew) {
const existing = await prisma.conversation.findFirst({
where: {
workflowMode: mode,
project: {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's a migration that backfills workflowMode
echo "=== Checking for workflowMode migration ==="
fd -e sql -e ts . prisma/migrations 2>/dev/null | head -20

echo ""
echo "=== Searching for backfill logic ==="
rg -n "workflowMode.*autopilot" --type sql prisma/ 2>/dev/null || echo "No SQL backfill found"
rg -n "UPDATE.*Conversation.*workflowMode" prisma/ 2>/dev/null || echo "No UPDATE statement found"

echo ""
echo "=== Check schema for workflowMode ==="
rg -A2 -B2 "workflowMode" prisma/schema.prisma

Repository: Ker102/ModelForge

Length of output: 450


🏁 Script executed:

#!/bin/bash

# Search more thoroughly for migrations
echo "=== List all files in prisma directory ==="
find prisma -type f 2>/dev/null | head -30

echo ""
echo "=== Search for any mention of workflowMode in migrations ==="
find prisma/migrations -type f -exec grep -l "workflowMode" {} \; 2>/dev/null || echo "No workflowMode mentions in migrations"

echo ""
echo "=== Check git history for schema changes ==="
git log --oneline -n 20 -- prisma/schema.prisma 2>/dev/null || echo "Git not available"

echo ""
echo "=== Read the actual code at lines 314-318 ==="
sed -n '310,325p' app/api/ai/chat/route.ts

Repository: Ker102/ModelForge

Length of output: 729


🏁 Script executed:

#!/bin/bash

# Find the ensureConversation function
echo "=== Search for ensureConversation function ==="
rg -n "ensureConversation" app/api/ai/chat/route.ts -A 20 | head -40

echo ""
echo "=== Check if there are other conversation queries ==="
rg -n "prisma.conversation" app/api/ai/chat/route.ts | head -20

echo ""
echo "=== Look for any conversation creation logic ==="
rg -n "prisma.conversation.create" app/api/ai/chat/route.ts -A 5

Repository: Ker102/ModelForge

Length of output: 1694


Critical: workflowMode filter at line 317 excludes existing conversations with NULL values.

The query filtering by workflowMode: mode will not match existing Conversation rows that have NULL for this field (added without backfill during deployment). This causes:

  • Existing conversations become invisible to the lookup at line 315
  • Users lose access to conversation history
  • Each chat creates a new conversation instead of continuing the existing one

Note the inconsistency: the query at line 295 (by conversationId) has no workflowMode filter, but the fallback query at line 315 does—revealing this is a bug.

Fix: Either update the query to include NULL values:

where: {
-  workflowMode: mode,
+  OR: [
+    { workflowMode: mode },
+    { workflowMode: null },
+  ],

Or ensure the migration backfills existing rows with the default value before this code is deployed.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!startNew) {
const existing = await prisma.conversation.findFirst({
where: {
workflowMode: mode,
project: {
if (!startNew) {
const existing = await prisma.conversation.findFirst({
where: {
OR: [
{ workflowMode: mode },
{ workflowMode: null },
],
project: {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/ai/chat/route.ts` around lines 314 - 318, The find-first query inside
the startNew branch (prisma.conversation.findFirst) incorrectly filters by
workflowMode: mode which excludes existing rows that have workflowMode = NULL;
modify that query to include conversations where workflowMode equals mode OR
workflowMode is null (e.g., add an OR clause for workflowMode null) so existing
conversations without a workflowMode value are matched; reference the startNew
variable and the mode value when updating the WHERE clause to preserve the
intended behavior and avoid creating duplicate conversations.

Comment on lines +115 to +129
// Get the currently running tool (last "started" without a matching "completed")
const activeTool = useMemo(() => {
const started = new Set<string>()
for (const e of toolEvents) {
if (e.status === "started") started.add(e.toolName)
else started.delete(e.toolName)
}
// Return the last one still running
for (let i = toolEvents.length - 1; i >= 0; i--) {
if (started.has(toolEvents[i].toolName) && toolEvents[i].status === "started") {
return toolEvents[i]
}
}
return null
}, [toolEvents])
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

A Set cannot represent two active calls of the same tool.

Sequence started → started → completed for the same toolName clears the entry entirely even though one invocation is still running. The middleware explicitly supports parallel/coalesced same-tool calls, so this needs a counter or a per-call identifier rather than a Set.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/projects/agent-activity.tsx` around lines 115 - 129, The current
activeTool logic uses a Set which cannot track multiple concurrent starts of the
same tool; replace the Set with a Map<string, number> (or an object) that counts
active instances per tool: iterate toolEvents and for each event do
counts.set(toolName, (counts.get(toolName)||0) + 1) on "started" and decrement
counts (and delete when zero) on non-"started"/"completed" events; then scan
toolEvents backwards and return the first event where event.status === "started"
and counts.get(event.toolName) > 0. Update the useMemo block (activeTool,
toolEvents) to use this counting Map so parallel/coalesced same-tool calls are
handled correctly.

Comment on lines +131 to +161
// Count completed tools
const completedCount = useMemo(
() => toolEvents.filter((e) => e.status === "completed").length,
[toolEvents]
)

if (!isActive && toolEvents.length === 0) return null

// Show last few completed tools + active tool
const recentCompleted = toolEvents
.filter((e) => e.status === "completed")
.slice(-3)

return (
<div
className="rounded-xl border px-4 py-3 mb-3 transition-all duration-300"
style={{
backgroundColor: "var(--forge-glass)",
borderColor: "hsl(var(--forge-border))",
backdropFilter: "blur(12px)",
boxShadow: "var(--forge-shadow)",
}}
>
{/* Header */}
<div className="flex items-center gap-2 mb-2">
{isActive && <PulsingDot />}
<span
className="text-xs font-semibold uppercase tracking-wider"
style={{ color: "hsl(var(--forge-text-subtle))" }}
>
{isActive ? "Agent working" : `Done — ${completedCount} tools used`}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Failed tools never surface in this panel.

Both the header and the recent list only count status === "completed". If a tool fails, the panel can render Done — 0 tools used and show no trace of the failure. Include failed terminal events in the summary/list so the new "failed" stream state is actually visible to users.

Also applies to: 195-225

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/projects/agent-activity.tsx` around lines 131 - 161, The header
and recent list only consider events with status === "completed", so failed tool
runs are hidden; update the logic that computes completedCount and
recentCompleted (and the similar block around the other recent-list code) to
include events where status === "completed" OR status === "failed" (i.e., treat
"failed" as a terminal state), and ensure any UI that derives from toolEvents
(variables toolEvents, completedCount, recentCompleted) counts and slices these
terminal events so failures surface in the header and recent list.

Comment on lines +135 to +136
const abortControllerRef = useRef<AbortController | null>(null)
const userStoppedRef = useRef(false)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Abort the in-flight stream on unmount.

Only clearing the controller in Line 792 leaves the unmount path uncovered. If the user navigates away mid-stream, the request and stall timer keep running until they fail naturally, and the async path still schedules state updates after unmount.

Suggested fix
   const abortControllerRef = useRef<AbortController | null>(null)
   const userStoppedRef = useRef(false)
+
+  useEffect(() => {
+    return () => {
+      abortControllerRef.current?.abort()
+      abortControllerRef.current = null
+      userStoppedRef.current = false
+    }
+  }, [])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/projects/project-chat.tsx` around lines 135 - 136, The component
currently only clears abortControllerRef later, leaving in-flight streams
running on unmount; add a cleanup that aborts any active controller and prevents
further state updates by calling abortControllerRef.current?.abort() and setting
userStoppedRef.current = true (and also clearing any associated timers) in a
useEffect cleanup or the existing effect's return handler so any ongoing
fetch/stream created by abortControllerRef and timers are cancelled when the
component unmounts.

Comment on lines +9 to +35
async function main() {
// Find the most recent assistant messages with tool call metadata
const msgs = await prisma.message.findMany({
where: { role: "assistant" },
orderBy: { createdAt: "desc" },
take: 5,
select: { id: true, createdAt: true, metadata: true },
})

for (const m of msgs) {
console.log("\n=== Message", m.id, "at", m.createdAt, "===")
const meta = m.metadata as Record<string, unknown> | null
if (meta?.executedCommands && Array.isArray(meta.executedCommands)) {
for (const cmd of meta.executedCommands) {
const c = cmd as { tool: string; arguments?: Record<string, unknown>; description?: string }
console.log(` 🔧 ${c.tool}`)
console.log(` Args: ${JSON.stringify(c.arguments ?? {}, null, 4)}`)
}
} else {
console.log(" (no executedCommands)")
}
}

await prisma.$disconnect()
}

main().catch(console.error)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Ensure Prisma always disconnects on failure paths.

If an error occurs before Line 32, the client may remain open because disconnect is not in a finally block.

Suggested fix
 async function main() {
-  // Find the most recent assistant messages with tool call metadata
-  const msgs = await prisma.message.findMany({
-    where: { role: "assistant" },
-    orderBy: { createdAt: "desc" },
-    take: 5,
-    select: { id: true, createdAt: true, metadata: true },
-  })
-
-  for (const m of msgs) {
-    console.log("\n=== Message", m.id, "at", m.createdAt, "===")
-    const meta = m.metadata as Record<string, unknown> | null
-    if (meta?.executedCommands && Array.isArray(meta.executedCommands)) {
-      for (const cmd of meta.executedCommands) {
-        const c = cmd as { tool: string; arguments?: Record<string, unknown>; description?: string }
-        console.log(`  🔧 ${c.tool}`)
-        console.log(`     Args: ${JSON.stringify(c.arguments ?? {}, null, 4)}`)
-      }
-    } else {
-      console.log("  (no executedCommands)")
-    }
-  }
-
-  await prisma.$disconnect()
+  try {
+    const msgs = await prisma.message.findMany({
+      where: { role: "assistant" },
+      orderBy: { createdAt: "desc" },
+      take: 5,
+      select: { id: true, createdAt: true, metadata: true },
+    })
+
+    for (const m of msgs) {
+      console.log("\n=== Message", m.id, "at", m.createdAt, "===")
+      const meta = m.metadata as Record<string, unknown> | null
+      if (meta?.executedCommands && Array.isArray(meta.executedCommands)) {
+        for (const cmd of meta.executedCommands) {
+          const c = cmd as { tool: string; arguments?: Record<string, unknown>; description?: string }
+          console.log(`  🔧 ${c.tool}`)
+          console.log(`     Args: ${JSON.stringify(c.arguments ?? {}, null, 4)}`)
+        }
+      } else {
+        console.log("  (no executedCommands)")
+      }
+    }
+  } finally {
+    await prisma.$disconnect()
+  }
 }
 
-main().catch(console.error)
+main().catch((error) => {
+  console.error(error)
+  process.exitCode = 1
+})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/check-tool-args.ts` around lines 9 - 35, The script may leave the
Prisma client connected if an error is thrown before the current
prisma.$disconnect() at the end of main(); wrap the body of main() in a
try/finally (or ensure prisma is disconnected in a finally block) so that
prisma.$disconnect() is always called even on errors—modify the main function to
call prisma.$disconnect() from a finally block (or call it after awaiting main()
in a top-level try/finally) and keep the existing main().catch(console.error)
behavior for logging.

Comment on lines +1 to +95
/**
* RAG Pipeline Diagnostic
* Tests vectorstore connectivity, document counts, and similarity search.
*
* Run from project root: npx tsx scripts/rag-diagnostic.ts
*/
import dotenv from "dotenv"
import path from "path"
import { fileURLToPath } from "url"

const __dirname = path.dirname(fileURLToPath(import.meta.url))
const projectRoot = path.resolve(__dirname, "..")

dotenv.config({ path: path.join(projectRoot, ".env") })

// Use relative paths from scripts/ to lib/
const { getDocumentCount, similaritySearch } = await import("../lib/ai/vectorstore.js")

async function main() {
console.log("=== RAG Pipeline Diagnostic ===\n")

// 1. Check document counts
console.log("1. Document Counts:")
try {
const total = await getDocumentCount()
const scripts = await getDocumentCount("blender-scripts")
const guides = await getDocumentCount("tool-guides")
console.log(` Total: ${total}`)
console.log(` blender-scripts: ${scripts}`)
console.log(` tool-guides: ${guides}`)

if (total === 0) {
console.error("\n❌ VECTORSTORE IS EMPTY! No documents ingested.")
console.error(" Run: npx tsx scripts/ingestion/reingest-blender-scripts.ts --force")
process.exit(1)
}
} catch (e: unknown) {
const err = e as Error
console.error(` ❌ Failed to query document counts: ${err.message}`)
console.error(` Stack: ${err.stack}`)
process.exit(1)
}

// 2. Test embedding generation
console.log("\n2. Embedding Generation:")
try {
const { embedText } = await import("../lib/ai/embeddings.js")
const start = Date.now()
const embedding = await embedText("Add a camera to the scene")
const elapsed = Date.now() - start
console.log(` ✅ Generated ${embedding.length}-dim embedding in ${elapsed}ms`)
} catch (e: unknown) {
const err = e as Error
console.error(` ❌ Embedding generation failed: ${err.message}`)
process.exit(1)
}

// 3. Test similarity search
console.log("\n3. Similarity Search (query: 'Add a camera to the scene'):")
try {
const results = await similaritySearch("Add a camera to the scene", { limit: 5 })
console.log(` Found ${results.length} results:`)
for (const r of results) {
console.log(` - [${r.source}] sim=${r.similarity.toFixed(3)} | ${r.content.slice(0, 100)}...`)
}

if (results.length === 0) {
console.error("\n❌ SEARCH RETURNED 0 RESULTS.")
console.error(" Possible causes:")
console.error(" - Embedding model mismatch (ingested with different model)")
console.error(" - minSimilarity threshold too high (currently 0.5)")
console.error(" - Documents not relevant to query")

// Try with lower threshold
console.log("\n Retrying with minSimilarity=0.0:")
const allResults = await similaritySearch("Add a camera to the scene", {
limit: 5,
minSimilarity: 0.0,
})
console.log(` Found ${allResults.length} results with threshold=0.0:`)
for (const r of allResults) {
console.log(` - [${r.source}] sim=${r.similarity.toFixed(3)} | ${r.content.slice(0, 100)}...`)
}
}
} catch (e: unknown) {
const err = e as Error
console.error(` ❌ Similarity search failed: ${err.message}`)
console.error(` Stack: ${err.stack}`)
}

console.log("\n=== Diagnostic Complete ===")
process.exit(0)
}

main()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consolidate duplicated RAG diagnostic logic into a shared module.

This script and scripts/ingestion/rag-diagnostic.ts now carry near-identical flow and output logic; extracting a shared runner will reduce divergence risk.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/rag-diagnostic.ts` around lines 1 - 95, The diagnostic logic in
main(), including calls to getDocumentCount, embedText, and similaritySearch, is
duplicated between scripts/rag-diagnostic.ts and
scripts/ingestion/rag-diagnostic.ts; extract the shared flow into a new reusable
module (e.g., a runRagDiagnostic function exported from
lib/scripts/ragDiagnostic.js/ts) that encapsulates steps 1–3 (document counts,
embedding generation, similarity search) and logging, then have both scripts
import and call that single runner (passing any config like query or
thresholds). Update the original main() to be a thin wrapper that imports
runRagDiagnostic and handles process.exit or error propagation so behavior
remains identical while removing duplicated code.

Comment on lines +85 to +93
} catch (e: unknown) {
const err = e as Error
console.error(` ❌ Similarity search failed: ${err.message}`)
console.error(` Stack: ${err.stack}`)
}

console.log("\n=== Diagnostic Complete ===")
process.exit(0)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not return success when similarity search fails.

A caught similarity-search exception still ends with exit code 0, which makes the diagnostic report success on failure.

Suggested fix
   } catch (e: unknown) {
     const err = e as Error
     console.error(`   ❌ Similarity search failed: ${err.message}`)
     console.error(`   Stack: ${err.stack}`)
+    process.exit(1)
   }
 
   console.log("\n=== Diagnostic Complete ===")
   process.exit(0)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch (e: unknown) {
const err = e as Error
console.error(` ❌ Similarity search failed: ${err.message}`)
console.error(` Stack: ${err.stack}`)
}
console.log("\n=== Diagnostic Complete ===")
process.exit(0)
}
} catch (e: unknown) {
const err = e as Error
console.error(` ❌ Similarity search failed: ${err.message}`)
console.error(` Stack: ${err.stack}`)
process.exit(1)
}
console.log("\n=== Diagnostic Complete ===")
process.exit(0)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/rag-diagnostic.ts` around lines 85 - 93, The catch block that handles
similarity search errors currently logs the error but lets execution continue to
the final "Diagnostic Complete" and process.exit(0); change the catch in
scripts/rag-diagnostic.ts so that after logging (the err.message and err.stack)
it either rethrows the error or calls process.exit(1) to return a non-zero
status; update the catch around the similarity search (the try/catch that
produces "Similarity search failed") to use process.exit(1) (or throw err) so
failures do not report success.

Comment on lines +14 to +18
console.log("BEFORE:", JSON.stringify(testTool.description))
testTool.description = testTool.description + "\n\n--- GUIDE ---\nCamera must be 8m away"
console.log("AFTER:", JSON.stringify(testTool.description))
console.log("MUTATION_WORKED:", testTool.description.includes("GUIDE"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

This “test” always reports success, even on failure.

Line 27 unconditionally exits with success, and Lines 14–18 only print state. If mutation regresses, automation still passes.

Proposed fix
 console.log("BEFORE:", JSON.stringify(testTool.description))
 testTool.description = testTool.description + "\n\n--- GUIDE ---\nCamera must be 8m away"
 console.log("AFTER:", JSON.stringify(testTool.description))
-console.log("MUTATION_WORKED:", testTool.description.includes("GUIDE"))
+const mutationWorked = testTool.description.includes("GUIDE")
+console.log("MUTATION_WORKED:", mutationWorked)
+if (!mutationWorked) {
+  console.error("Description mutation did not persist")
+  process.exit(1)
+}
@@
-process.exit(0)
+process.exit(0)

Also applies to: 27-27

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/test-tool-mutation.ts` around lines 14 - 18, The test currently only
logs before/after and then unconditionally exits success, so failures are never
reported; update the logic around the testTool mutation (the
testTool.description assignment and the subsequent checks) to verify the
mutation actually took effect: after assigning testTool.description =
testTool.description + "...GUIDE..." check if
testTool.description.includes("GUIDE") and if not write an error (console.error)
and call process.exit(1), otherwise print success and exit 0; ensure you remove
the unconditional success exit and use the include check to determine the
process exit code.

Comment on lines +20 to +25
const desc = Object.getOwnPropertyDescriptor(testTool, "description")
console.log("PROPERTY_DESCRIPTOR:", JSON.stringify(desc))

// If direct assignment fails, check prototype
const protoDesc = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(testTool), "description")
console.log("PROTO_DESCRIPTOR:", JSON.stringify(protoDesc))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

find . -name "test-tool-mutation.ts" -type f

Repository: Ker102/ModelForge

Length of output: 92


🏁 Script executed:

cat -n scripts/test-tool-mutation.ts | head -30

Repository: Ker102/ModelForge

Length of output: 1187


🏁 Script executed:

node << 'EOF'
// Verify JSON.stringify behavior with property descriptors
const obj = {};
Object.defineProperty(obj, 'prop', {
  get() { return 'value'; },
  set(v) { },
  enumerable: true,
  configurable: true
});

const desc = Object.getOwnPropertyDescriptor(obj, 'prop');
console.log("Full descriptor object:", desc);
console.log("\nJSON.stringify output:", JSON.stringify(desc));

// Also check inspect behavior
const { inspect } = require('util');
console.log("\nutil.inspect output:", inspect(desc, { depth: null }));
EOF

Repository: Ker102/ModelForge

Length of output: 356


JSON.stringify omits get/set accessors from property descriptors.

On lines 21 and 25, using JSON.stringify() to log property descriptors silently drops function-valued get/set accessors, making it impossible to diagnose whether the property is writable via getter/setter. Use util.inspect() instead to preserve these details:

Proposed fix
+import { inspect } from "node:util"
@@
 const desc = Object.getOwnPropertyDescriptor(testTool, "description")
-console.log("PROPERTY_DESCRIPTOR:", JSON.stringify(desc))
+console.log("PROPERTY_DESCRIPTOR:", inspect(desc, { depth: null }))
@@
 const protoDesc = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(testTool), "description")
-console.log("PROTO_DESCRIPTOR:", JSON.stringify(protoDesc))
+console.log("PROTO_DESCRIPTOR:", inspect(protoDesc, { depth: null }))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const desc = Object.getOwnPropertyDescriptor(testTool, "description")
console.log("PROPERTY_DESCRIPTOR:", JSON.stringify(desc))
// If direct assignment fails, check prototype
const protoDesc = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(testTool), "description")
console.log("PROTO_DESCRIPTOR:", JSON.stringify(protoDesc))
import { inspect } from "node:util"
const desc = Object.getOwnPropertyDescriptor(testTool, "description")
console.log("PROPERTY_DESCRIPTOR:", inspect(desc, { depth: null }))
// If direct assignment fails, check prototype
const protoDesc = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(testTool), "description")
console.log("PROTO_DESCRIPTOR:", inspect(protoDesc, { depth: null }))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/test-tool-mutation.ts` around lines 20 - 25, The logs use
JSON.stringify on property descriptors (desc and protoDesc) which drops get/set
accessors; import Node's util and replace JSON.stringify(desc) /
JSON.stringify(protoDesc) with util.inspect(desc, {showHidden: true, depth:
null}) (and same for protoDesc) so the getter/setter functions are preserved in
the output; update the top of the file to require or import util if not present
and use util.inspect where the PROPERTY_DESCRIPTOR and PROTO_DESCRIPTOR are
logged.

Critical:
- Fix stableHash() to deep-sort nested object keys in dedup middleware
  (old version collapsed different nested args into same cache key)

Major:
- Add triggered_by field to weight-painting-guide.md for tool binding
- Update test criteria for Test 14/16 to handle unavailable AI services
- Fix bottom_shift formula in spatial-positioning-guide.md (was ~18x off)

Minor:
- Add blank lines before tables in architecture.md (MD058)
- Clarify TS health scope in gemini.md (touched files vs legacy error)
@coderabbitai coderabbitai bot added the desktop label Mar 21, 2026
@Ker102 Ker102 merged commit 73e05c8 into main Mar 21, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant