Fix empty preview when parametric turns finish without SCAD (#181)#191
Fix empty preview when parametric turns finish without SCAD (#181)#191adoodevv wants to merge 1 commit into
Conversation
|
@adoodevv is attempting to deploy a commit to the Adam Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds explicit error surfacing when a parametric turn finishes without a
Confidence Score: 3/5Merging as-is ships the error-surfacing UI correctly, but the SCAD salvage feature described in the PR is broken and will silently fail for every case where the model embeds code in text instead of calling the tool. The core defensive UI (red banner, destructive toast, empty-preview message, auto-continue on rejected answer_user) all work. The problem is that the 'salvage from text' path — the other half of the stated fix for #181 — is dead code: the guard condition that enables the recovery branch also guarantees the recovery data is absent. Users who hit the exact scenario the salvage path was built for (model writes SCAD to prose instead of calling the tool) will always see the failure toast even when perfectly valid code was produced. src/components/chat/ChatSession.tsx — the salvage logic in onFinish needs the guard restructured so extractScadFromAssistantParts is checked before (not inside) the shouldReportMissingParametricBuild gate. Important Files Changed
Reviews (1): Last reviewed commit: "Fix empty preview when parametric turns ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const recoveredCode = extractScadFromAssistantParts(message.parts); | ||
| if (recoveredCode) { | ||
| void (async () => { | ||
| try { | ||
| await previewScadColoredViaToolWorker(recoveredCode); | ||
| onViewArtifact( | ||
| { | ||
| title: 'Recovered model', | ||
| version: 'v1', | ||
| code: recoveredCode, | ||
| }, | ||
| message.id, | ||
| ); | ||
| toast({ | ||
| title: 'Model code recovered', | ||
| description: | ||
| 'OpenSCAD was found in the reply text and loaded into the preview.', | ||
| }); | ||
| } catch (error) { | ||
| console.warn('Recovered OpenSCAD failed to compile:', error); | ||
| toast({ | ||
| title: 'Found code but it did not compile', | ||
| description: | ||
| 'OpenSCAD was in the reply but could not be rendered. Try retrying the prompt.', | ||
| variant: 'destructive', | ||
| }); | ||
| } | ||
| })(); | ||
| } else { | ||
| toast({ | ||
| title: 'No model was generated', | ||
| description: | ||
| 'Adam replied without OpenSCAD code. Try again or retry with a different model.', | ||
| variant: 'destructive', | ||
| }); | ||
| } |
There was a problem hiding this comment.
shouldReportMissingParametricBuild returns false when canSalvageParametricBuildFromText is true (line 292 in parametricParts.ts). Both canSalvageParametricBuildFromText and extractScadFromAssistantParts call the same underlying extractScadFromText(getAssistantSalvageText(...)), so whenever the guard lets us inside the if (shouldReportMissingParametricBuild(...)) block, recoveredCode is unconditionally undefined. The entire if (recoveredCode) branch — previewScadColoredViaToolWorker, onViewArtifact, and the "Model code recovered" toast — can never execute. The whole salvage-from-text feature described in the PR is dead on arrival; every case lands in the destructive-toast else.
| const fencePatterns = [ | ||
| /```(?:openscad|scad|open-scad)?\s*\n([\s\S]*?)```/gi, | ||
| /```\s*\n([\s\S]*?)```/gi, | ||
| ]; |
There was a problem hiding this comment.
(?:openscad|scad|open-scad)? is optional, so pattern 1 already matches plain ``` blocks (zero-width match → \s* backtracks off the newline → matches). Pattern 2 will never find a block that pattern 1 missed; it just re-scans all the same fences a second time.
| const fencePatterns = [ | |
| /```(?:openscad|scad|open-scad)?\s*\n([\s\S]*?)```/gi, | |
| /```\s*\n([\s\S]*?)```/gi, | |
| ]; | |
| const fencePatterns = [ | |
| /```(?:openscad|scad|open-scad)?\s*\n([\s\S]*?)```/gi, | |
| ]; |
| if ( | ||
| options?.leafRole === 'user' && | ||
| !hasParametricBuildAttempt(parts) && | ||
| !extractScadFromAssistantParts(parts) | ||
| ) { |
There was a problem hiding this comment.
!hasParametricBuildAttempt(parts) is always true here — the enclosing if (!hasBuild) already confirmed no tool-build_parametric_model part exists, and hasParametricBuildAttempt performs the identical check. The condition adds cognitive noise without any guard value.
| if ( | |
| options?.leafRole === 'user' && | |
| !hasParametricBuildAttempt(parts) && | |
| !extractScadFromAssistantParts(parts) | |
| ) { | |
| if ( | |
| options?.leafRole === 'user' && | |
| !extractScadFromAssistantParts(parts) | |
| ) { |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
Summary
build_parametric_modelcall and show explicit errors in chat, toasts, and the preview pane instead of a silent empty viewer.answer_usercalls before a successful build and auto-continue the agent loop so the model can retry.toolChoicevalues for Anthropic).Fixes #181
Test plan
node --test shared/parametricParts.test.ts shared/parametricAgentLoop.test.ts(16 tests pass)npm run typecheckSummary by cubic
Fixes empty preview in parametric chats when the model ends a turn without calling
build_parametric_model. Adds clear errors, auto-continue, and SCAD recovery to keep users unblocked. Fixes #181.Bug Fixes
answer_userbefore a successfulbuild_parametric_model; persist an error and keep the turn running.New Features
answer_userviashouldAutoContinueParametricBuild.build_parametric_modeland log turns that finish without a build.Written for commit 7da254c. Summary will update on new commits.