From 952b645ffb9d8d6d57781fb04bb5f992b4ed3e01 Mon Sep 17 00:00:00 2001 From: Andrii Shylenko <14119286+w1ne@users.noreply.github.com> Date: Thu, 9 Jul 2026 17:45:19 +0200 Subject: [PATCH] fix(studio): harden repair workflow states Signed-off-by: Andrii Shylenko <14119286+w1ne@users.noreply.github.com> --- src/studio/StagedEditSlot.tsx | 9 +++ src/studio/StudioGenerate.tsx | 31 ++++++--- src/studio/__tests__/StagedEditSlot.test.tsx | 34 ++++++++++ src/studio/__tests__/StudioGenerate.test.tsx | 65 +++++++++++++++++++ .../__tests__/tabs/ValidityTab.test.tsx | 34 ++++++++++ .../__tests__/useRecomputeResult.test.tsx | 38 +++++++++++ src/studio/hooks/useRecomputeResult.ts | 13 +++- src/studio/store/shellStore.ts | 1 + src/studio/tabs/ValidityTab.tsx | 36 ++++++++-- 9 files changed, 245 insertions(+), 16 deletions(-) diff --git a/src/studio/StagedEditSlot.tsx b/src/studio/StagedEditSlot.tsx index 73e9faa9..27b94246 100644 --- a/src/studio/StagedEditSlot.tsx +++ b/src/studio/StagedEditSlot.tsx @@ -235,6 +235,15 @@ export function StagedEditSlot() { const handleReject = useCallback(() => { if (stagedEdit != null) { shellStore.recordStagedEditOutcome(stagedEdit, 'rejected'); + const workflow = stagedEdit.context?.repairWorkflow; + const currentWorkflow = shellStore.getSnapshot().agentRepairWorkflow; + if ( + workflow?.state === 'running' && + currentWorkflow?.state === 'running' && + currentWorkflow.cardId === workflow.cardId + ) { + shellStore.setAgentRepairWorkflow({ ...currentWorkflow, state: 'drafted' }); + } } setStaleWarning(null); shellStore.clearStagedEdit(); diff --git a/src/studio/StudioGenerate.tsx b/src/studio/StudioGenerate.tsx index 5ab5dc10..50d329c2 100644 --- a/src/studio/StudioGenerate.tsx +++ b/src/studio/StudioGenerate.tsx @@ -45,7 +45,7 @@ const StudioGenerateInner: React.FC = () => { const { code } = useCode(); const currentCode = code ?? ''; const { selectedFeatureId } = useFeatureSelection(); - const { agentDraftPrompt, agentDraftPromptVersion, agentRepairWorkflow } = useShellStore(); + const { agentDraftPrompt, agentDraftPromptVersion, agentRepairWorkflow, stagedEdit } = useShellStore(); const [editedPrompt, setEditedPrompt] = useState(''); const [acknowledgedDraftVersion, setAcknowledgedDraftVersion] = useState(-1); // The generationId we've already staged/rejected — gates the review panel @@ -102,13 +102,19 @@ const StudioGenerateInner: React.FC = () => { e.preventDefault(); const trimmed = prompt.trim(); if (!trimmed || busy) return; - const agentPrompt = selectedFeatureId === null ? trimmed : `Edit selected target "${selectedFeatureId}": ${trimmed}`; - let repairWorkflowForRun = agentRepairWorkflow; - if ( + const matchesDraftedRepair = agentRepairWorkflow != null && agentRepairWorkflow.state === 'drafted' && - agentRepairWorkflow.promptText === trimmed && - agentRepairWorkflow.targetId === selectedFeatureId + agentRepairWorkflow.promptText === trimmed; + const runTargetId = + matchesDraftedRepair && agentRepairWorkflow.targetId === null + ? null + : selectedFeatureId; + const agentPrompt = runTargetId === null ? trimmed : `Edit selected target "${runTargetId}": ${trimmed}`; + let repairWorkflowForRun = agentRepairWorkflow; + if ( + matchesDraftedRepair && + agentRepairWorkflow.targetId === runTargetId ) { repairWorkflowForRun = { ...agentRepairWorkflow, state: 'running' }; shellStore.setAgentRepairWorkflow(repairWorkflowForRun); @@ -116,7 +122,7 @@ const StudioGenerateInner: React.FC = () => { runAgent(agentPrompt, { fromCode: currentCode, promptText: trimmed, - selectedFeatureId, + selectedFeatureId: runTargetId, repairWorkflow: repairWorkflowForRun, }); }; @@ -154,6 +160,7 @@ const StudioGenerateInner: React.FC = () => { const stageGeneratedEdit = () => { if (phase.state !== 'done') return; + if (stagedEdit != null) return; const snapshot = reviewSnapshot ?? { fromCode: currentCode, promptText: prompt.trim(), @@ -177,6 +184,9 @@ const StudioGenerateInner: React.FC = () => { }; const reject = () => { if (phase.state !== 'done') return; + if (agentRepairWorkflow?.state === 'running') { + shellStore.setAgentRepairWorkflow({ ...agentRepairWorkflow, state: 'drafted' }); + } setResolution({ generationId: phase.generationId, action: 'discarded' }); }; @@ -229,7 +239,7 @@ const StudioGenerateInner: React.FC = () => { )} {/* Review gate: diff + verified badge + accept/reject. Never auto-applies. */} - {reviewing && phase.state === 'done' && ( + {reviewing && phase.state === 'done' && stagedEdit == null && (
@@ -274,6 +284,11 @@ const StudioGenerateInner: React.FC = () => {
)} + {reviewing && phase.state === 'done' && stagedEdit != null && ( +
+ Review the current staged edit before staging another proposal. +
+ )} {phase.state === 'done' && !reviewing && resolution?.action === 'staged' && (
diff --git a/src/studio/__tests__/StagedEditSlot.test.tsx b/src/studio/__tests__/StagedEditSlot.test.tsx index 7095ec87..eff7828d 100644 --- a/src/studio/__tests__/StagedEditSlot.test.tsx +++ b/src/studio/__tests__/StagedEditSlot.test.tsx @@ -103,6 +103,40 @@ describe('StagedEditSlot', () => { expect(getByTestId('applied-edit-history').textContent).toContain('demo'); }); + it('Reject rolls a running staged repair workflow back to drafted', () => { + const repairWorkflow = { + cardId: 'diagnostic:assembly.part.floating:output-horn:0', + code: 'assembly.part.floating', + promptText: 'Fix output horn', + targetId: 'output-horn', + promptSource: 'fallback' as const, + validityFingerprint: 'before', + state: 'running' as const, + }; + shellStore.setAgentRepairWorkflow(repairWorkflow); + shellStore.proposeStagedEdit({ + id: 'e-reject-workflow', + intent: 'Reject repair', + fromCode: 'return box(10);', + toCode: 'return box(20);', + context: { + promptText: 'Fix output horn', + selectedFeatureId: 'output-horn', + repairWorkflow, + generationId: 'gen-reject', + }, + }); + + const { getByTestId } = render(); + fireEvent.click(getByTestId('staged-edit-reject')); + + expect(shellStore.getSnapshot().agentRepairWorkflow).toEqual({ + ...repairWorkflow, + state: 'drafted', + }); + expect(getByTestId('applied-edit-history').textContent).toContain('Rejected'); + }); + it('source label renders when provided', () => { shellStore.proposeStagedEdit({ id: 'e4', diff --git a/src/studio/__tests__/StudioGenerate.test.tsx b/src/studio/__tests__/StudioGenerate.test.tsx index d6de032d..ebddd2c2 100644 --- a/src/studio/__tests__/StudioGenerate.test.tsx +++ b/src/studio/__tests__/StudioGenerate.test.tsx @@ -36,6 +36,7 @@ const mockShell = vi.hoisted(() => ({ validityFingerprint: string; state: 'drafted' | 'running'; } | null, + stagedEdit: null as { id: string } | null, setAgentRepairWorkflow: vi.fn(), proposeStagedEdit: vi.fn(), })); @@ -72,6 +73,7 @@ vi.mock('../store/useShellStore', () => ({ agentDraftPrompt: mockShell.agentDraftPrompt, agentDraftPromptVersion: mockShell.agentDraftPromptVersion, agentRepairWorkflow: mockShell.agentRepairWorkflow, + stagedEdit: mockShell.stagedEdit, }), shellStore: { setAgentRepairWorkflow: mockShell.setAgentRepairWorkflow, @@ -92,6 +94,7 @@ beforeEach(() => { mockShell.agentDraftPrompt = null; mockShell.agentDraftPromptVersion = 0; mockShell.agentRepairWorkflow = null; + mockShell.stagedEdit = null; mockShell.setAgentRepairWorkflow.mockReset(); mockShell.proposeStagedEdit.mockReset(); }); @@ -182,6 +185,33 @@ describe('StudioGenerate', () => { ); }); + it('submits a drafted whole-model repair without stale selected-feature prefix', () => { + mockSelection.selectedFeatureId = 'hinge-pin'; + mockShell.agentDraftPrompt = 'Repair deterministic mechanism failures.'; + mockShell.agentDraftPromptVersion = 1; + mockShell.agentRepairWorkflow = { + cardId: 'mechanism:mechanism.disconnect:0', + code: 'mechanism.disconnect', + promptText: 'Repair deterministic mechanism failures.', + targetId: null, + promptSource: 'review', + validityFingerprint: 'before', + state: 'drafted', + }; + render(); + + const prompt = screen.getByLabelText('Generate prompt'); + fireEvent.submit(prompt.closest('form')!); + + expect(mockShell.setAgentRepairWorkflow).toHaveBeenCalledWith({ + ...mockShell.agentRepairWorkflow, + state: 'running', + }); + expect(mockGeneration.submit).toHaveBeenCalledWith( + 'Repair deterministic mechanism failures.', + ); + }); + it('rolls a running repair workflow back to drafted when generation errors', () => { mockGeneration.phase = { state: 'error', @@ -312,6 +342,15 @@ describe('StudioGenerate', () => { }); it('does not show staged status after discarding a generated artifact', () => { + mockShell.agentRepairWorkflow = { + cardId: 'diagnostic:assembly.part.floating:output-horn:0', + code: 'assembly.part.floating', + promptText: 'Fix output horn', + targetId: 'output-horn', + promptSource: 'fallback', + validityFingerprint: 'before', + state: 'running', + }; mockGeneration.phase = { state: 'done', generationId: 'gen-discard', @@ -329,10 +368,36 @@ describe('StudioGenerate', () => { fireEvent.click(screen.getByRole('button', { name: /discard/i })); expect(mockShell.proposeStagedEdit).not.toHaveBeenCalled(); + expect(mockShell.setAgentRepairWorkflow).toHaveBeenCalledWith({ + ...mockShell.agentRepairWorkflow, + state: 'drafted', + }); expect(screen.queryByText(/staged for review/i)).toBeNull(); expect(screen.getByText(/discarded — Throwaway proposal/i)).toBeTruthy(); }); + it('does not overwrite an existing staged edit with a new generated proposal', () => { + mockShell.stagedEdit = { id: 'existing-edit' }; + mockCode.code = 'return box(10);'; + mockGeneration.phase = { + state: 'done', + generationId: 'gen-overwrite', + anonId: 'anon-overwrite', + artifact: { + title: 'New proposal', + code: 'return box(20);', + parameters: [], + suggestions: [], + }, + }; + + render(); + + expect(screen.queryByRole('button', { name: /stage edit/i })).toBeNull(); + expect(screen.getByText(/review the current staged edit before staging another/i)).toBeTruthy(); + expect(mockShell.proposeStagedEdit).not.toHaveBeenCalled(); + }); + it('preserves prompt, target, and repair workflow context on staged generated edits', () => { mockCode.code = 'return box(10);'; mockSelection.selectedFeatureId = 'output-horn'; diff --git a/src/studio/__tests__/tabs/ValidityTab.test.tsx b/src/studio/__tests__/tabs/ValidityTab.test.tsx index e13b1b57..d1893820 100644 --- a/src/studio/__tests__/tabs/ValidityTab.test.tsx +++ b/src/studio/__tests__/tabs/ValidityTab.test.tsx @@ -414,6 +414,40 @@ describe('ValidityTab', () => { expect(card.textContent).toContain('Still failing'); }); + it('marks a pair repair still failing when the validator reverses part order', () => { + const first: ValidatorDiagnostic = { + code: 'assembly.interference.overlap', + severity: 'error', + message: 'bracket collides with cover', + hint: 'move the cover clear of the bracket', + partA: 'bracket', + partB: 'cover', + }; + const reversed: ValidatorDiagnostic = { + ...first, + message: 'cover still collides with bracket', + partA: 'cover', + partB: 'bracket', + }; + mockUseRecomputeResult.mockReturnValue( + withValidity(makeValidity('error', [first], 2, 0)), + ); + const { rerender } = render(); + + fireEvent.click(screen.getByRole('button', { name: 'Use prompt' })); + const workflow = shellStore.getSnapshot().agentRepairWorkflow; + shellStore.setAgentRepairWorkflow(workflow == null ? null : { ...workflow, state: 'running' }); + mockUseRecomputeResult.mockReturnValue( + withValidity(makeValidity('error', [reversed], 2, 0)), + ); + rerender(); + + const card = screen.getByTestId('validity-suggestion-card'); + expect(card.getAttribute('data-workflow-state')).toBe('still-failing'); + expect(card.textContent).toContain('Still failing'); + expect(screen.queryByTestId('validity-suggestion-workflow-summary')).toBeNull(); + }); + it('does not mark drafted cards rechecked before the repair has been submitted', () => { const diag: ValidatorDiagnostic = { code: 'assembly.part.floating', diff --git a/src/studio/__tests__/useRecomputeResult.test.tsx b/src/studio/__tests__/useRecomputeResult.test.tsx index c83ead5d..527f7223 100644 --- a/src/studio/__tests__/useRecomputeResult.test.tsx +++ b/src/studio/__tests__/useRecomputeResult.test.tsx @@ -304,4 +304,42 @@ describe('useRecomputeResult — Slice 1.2 data wiring', () => { recheckedScriptFingerprint: fingerprintStudioScript(approvedCode), }); }); + + it('does not recheck an approved edit with stale review output when only code changed', async () => { + const oldCode = 'const part = box(10);\nreturn part;'; + const approvedCode = 'const part = box(20);\nreturn part;'; + shellStore.recordStagedEditOutcome( + { + id: 'approved-stale-review', + intent: 'approved stale review', + fromCode: oldCode, + toCode: approvedCode, + }, + 'approved', + ); + const oldReview: ScriptReviewSummary = { ok: true, diagnostics: [] }; + workbenchValue.code = oldCode; + workbenchValue.scriptReview = oldReview; + + const { useRecomputeResult } = await import('../hooks/useRecomputeResult'); + const { rerender } = renderHook(() => useRecomputeResult()); + + workbenchValue.code = approvedCode; + workbenchValue.scriptReview = oldReview; + rerender(); + + expect(shellStore.getSnapshot().appliedEditHistory[0]).toMatchObject({ + editId: 'approved-stale-review', + recheckStatus: 'pending-recheck', + }); + + workbenchValue.scriptReview = { ok: true, diagnostics: [] }; + rerender(); + + expect(shellStore.getSnapshot().appliedEditHistory[0]).toMatchObject({ + editId: 'approved-stale-review', + recheckStatus: 'rechecked-solved', + recheckedScriptFingerprint: fingerprintStudioScript(approvedCode), + }); + }); }); diff --git a/src/studio/hooks/useRecomputeResult.ts b/src/studio/hooks/useRecomputeResult.ts index 46dabd96..f3105195 100644 --- a/src/studio/hooks/useRecomputeResult.ts +++ b/src/studio/hooks/useRecomputeResult.ts @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2026 Andrii Shylenko and kernelCAD contributors -import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo, useRef } from 'react'; import { useWorkbench } from '../context/WorkbenchContext'; import { reviewToValidity, reviewToMechanismBanner } from '../adapters/reviewToValidity'; import { serializedParamsToTable } from '../adapters/serializedParamsToTable'; @@ -31,6 +31,7 @@ import type { StudioRecomputeResult, StudioRepairEvidence } from '../types'; */ export function useRecomputeResult(): StudioRecomputeResult { const workbench = useWorkbench(); + const lastPublishedReviewRef = useRef(UNPUBLISHED_REVIEW); const validity = useMemo( () => reviewToValidity(workbench.scriptReview ?? null), @@ -88,8 +89,12 @@ export function useRecomputeResult(): StudioRecomputeResult { // Publish validity into the shell store so BottomDrawer + // ValidityDeltaHeader see the delta (current ↔ previous). useEffect(() => { - shellStore.publishValidity(validity, { scriptFingerprint }); - }, [scriptFingerprint, validity]); + const reviewChanged = lastPublishedReviewRef.current !== workbench.scriptReview; + lastPublishedReviewRef.current = workbench.scriptReview ?? null; + shellStore.publishValidity(validity, { + scriptFingerprint: reviewChanged ? scriptFingerprint : undefined, + }); + }, [scriptFingerprint, validity, workbench.scriptReview]); const updateParam = (workbench as { updateParam?: StudioRecomputeResult['updateParam'] }).updateParam; const setGeometryTransformOverride = @@ -140,6 +145,8 @@ export function useRecomputeResult(): StudioRecomputeResult { ); } +const UNPUBLISHED_REVIEW = Symbol('kernelcad.unpublishedReview'); + function normalizePrompt(value: string | undefined): string | null { if (value == null) return null; const trimmed = value.trim(); diff --git a/src/studio/store/shellStore.ts b/src/studio/store/shellStore.ts index 3c2cebd0..c6a5f929 100644 --- a/src/studio/store/shellStore.ts +++ b/src/studio/store/shellStore.ts @@ -72,6 +72,7 @@ export interface AgentRepairWorkflow { readonly code: string; readonly promptText: string; readonly targetId: SelectedFeatureId; + readonly targetIds?: readonly string[]; readonly promptSource: 'review' | 'fallback'; readonly validityFingerprint: string; readonly state: AgentRepairWorkflowState; diff --git a/src/studio/tabs/ValidityTab.tsx b/src/studio/tabs/ValidityTab.tsx index f38b8a69..c25d8efa 100644 --- a/src/studio/tabs/ValidityTab.tsx +++ b/src/studio/tabs/ValidityTab.tsx @@ -158,6 +158,7 @@ function SuggestionCard({ code: card.code, promptText: card.promptText, targetId: card.targetId, + targetIds: card.targetIds, promptSource: card.promptSource, validityFingerprint, state: 'drafted', @@ -330,7 +331,9 @@ function resolveWorkflowView(input: { function cardMatchesWorkflow(card: ValiditySuggestionCard, workflow: AgentRepairWorkflow): boolean { if (card.id === workflow.cardId) return true; - return card.code === workflow.code && card.targetId === workflow.targetId; + if (card.code !== workflow.code) return false; + if (sameTargetSet(card.targetIds, workflow.targetIds ?? [])) return true; + return card.targetId === workflow.targetId; } function workflowFailureStillPresent( @@ -347,10 +350,33 @@ function workflowFailureStillPresent( if (mechanismBanner?.entries.some((entry) => entry.code === workflow.code) === true) { return true; } - return diagnostics.some((diagnostic) => ( - diagnostic.code === workflow.code && - diagnosticTargetId(diagnostic) === workflow.targetId - )); + return diagnostics.some((diagnostic) => { + if (diagnostic.code !== workflow.code) return false; + if (sameTargetSet(diagnosticTargetIds(diagnostic), workflow.targetIds ?? [])) return true; + return diagnosticTargetId(diagnostic) === workflow.targetId; + }); +} + +function sameTargetSet(a: readonly string[], b: readonly string[]): boolean { + if (a.length === 0 || b.length === 0 || a.length !== b.length) return false; + const left = [...a].sort(); + const right = [...b].sort(); + return left.every((value, index) => value === right[index]); +} + +function diagnosticTargetIds(diagnostic: ValidatorDiagnostic): string[] { + return uniqueNonEmpty([diagnostic.partName, diagnostic.partA, diagnostic.partB]); +} + +function uniqueNonEmpty(values: ReadonlyArray): string[] { + const seen = new Set(); + const ids: string[] = []; + for (const value of values) { + if (!value || seen.has(value)) continue; + seen.add(value); + ids.push(value); + } + return ids; } function fingerprintValidity(input: {