diff --git a/src/studio/StagedEditSlot.tsx b/src/studio/StagedEditSlot.tsx index 889b0789..ffaa669f 100644 --- a/src/studio/StagedEditSlot.tsx +++ b/src/studio/StagedEditSlot.tsx @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2026 Andrii Shylenko and kernelCAD contributors -import { useCallback } from 'react'; +import { useCallback, useState } from 'react'; import { Check, X } from 'lucide-react'; import { useShellStore, shellStore } from './store/useShellStore'; import { useWorkbench } from './context/WorkbenchContext'; @@ -10,10 +10,9 @@ import type { StagedEdit } from './store/shellStore'; // populated, renders the intent, a minimal line-by-line diff, and // approve/reject buttons. When empty, renders the auto-apply placeholder. // -// Approve writes stagedEdit.toCode through workbench.setCode, which -// flows through the normal recompute pipeline (AST-edit primacy -// preserved — the proposed text IS the AST-edited result; we just -// hand the canonical script to the existing setter). +// Approve writes stagedEdit.toCode through workbench.setCode only if the +// editor still matches the staged baseline. That keeps generated edits from +// overwriting intervening human changes. function computeLineDiff(from: string, to: string): Array<{ kind: 'context' | 'add' | 'del'; text: string }> { // Trivial line diff: walk both, mark non-matching lines as add/del. @@ -87,15 +86,28 @@ function PlaceholderBody() { export function StagedEditSlot() { const { stagedEdit } = useShellStore(); - const { setCode } = useWorkbench(); + const { code, setCode } = useWorkbench(); + const [staleWarning, setStaleWarning] = useState<{ editId: string; message: string } | null>(null); + const visibleStaleWarning = + stagedEdit != null && staleWarning?.editId === stagedEdit.id + ? staleWarning.message + : null; const handleApprove = useCallback(() => { if (stagedEdit == null) return; + if (code !== stagedEdit.fromCode) { + setStaleWarning({ + editId: stagedEdit.id, + message: 'The editor changed since this edit was staged. Review the current code before applying this proposal.', + }); + return; + } setCode(stagedEdit.toCode); shellStore.clearStagedEdit(); - }, [stagedEdit, setCode]); + }, [code, stagedEdit, setCode]); const handleReject = useCallback(() => { + setStaleWarning(null); shellStore.clearStagedEdit(); }, []); @@ -121,6 +133,14 @@ export function StagedEditSlot() { {stagedEdit.source.kind} · {stagedEdit.source.label} )} + {visibleStaleWarning != null && ( +
+ {visibleStaleWarning} +
+ )}
)} - {phase.state === 'done' && !reviewing && ( + {phase.state === 'done' && !reviewing && resolution?.action === 'staged' && (
- ✓ applied — {phase.artifact.title} + ✓ staged for review — {phase.artifact.title} +
+ )} + {phase.state === 'done' && !reviewing && resolution?.action === 'discarded' && ( +
+ discarded — {phase.artifact.title}
)} {phase.state === 'error' && ( diff --git a/src/studio/__tests__/StagedEditSlot.test.tsx b/src/studio/__tests__/StagedEditSlot.test.tsx index 566cf989..3303b8a6 100644 --- a/src/studio/__tests__/StagedEditSlot.test.tsx +++ b/src/studio/__tests__/StagedEditSlot.test.tsx @@ -7,14 +7,16 @@ import { StagedEditSlot } from '../StagedEditSlot'; import { shellStore } from '../store/shellStore'; const setCodeMock = vi.fn(); +let workbenchCode = ''; vi.mock('../context/WorkbenchContext', () => ({ - useWorkbench: () => ({ setCode: setCodeMock }), + useWorkbench: () => ({ code: workbenchCode, setCode: setCodeMock }), })); beforeEach(() => { shellStore.reset(); setCodeMock.mockReset(); + workbenchCode = ''; }); afterEach(() => { @@ -45,10 +47,12 @@ describe('StagedEditSlot', () => { it('Approve calls workbench.setCode(toCode) and clears the slot', () => { const toCode = 'const a = box(10);\nreturn a;'; + const fromCode = 'return box(10);'; + workbenchCode = fromCode; shellStore.proposeStagedEdit({ id: 'e2', intent: 'demo', - fromCode: 'return box(10);', + fromCode, toCode, }); const { getByTestId } = render(); @@ -58,6 +62,23 @@ describe('StagedEditSlot', () => { expect(shellStore.getSnapshot().stagedEdit).toBeNull(); }); + it('Approve blocks stale staged edits when the editor changed since proposal', () => { + shellStore.proposeStagedEdit({ + id: 'e-stale', + intent: 'demo', + fromCode: 'return box(10);', + toCode: 'return box(20);', + }); + workbenchCode = 'return box(15);'; + + const { getByTestId } = render(); + fireEvent.click(getByTestId('staged-edit-approve')); + + expect(setCodeMock).not.toHaveBeenCalled(); + expect(shellStore.getSnapshot().stagedEdit?.id).toBe('e-stale'); + expect(getByTestId('staged-edit-stale-warning').textContent).toContain('changed since this edit was staged'); + }); + it('Reject leaves the script unchanged and clears the slot', () => { shellStore.proposeStagedEdit({ id: 'e3', diff --git a/src/studio/__tests__/StudioGenerate.test.tsx b/src/studio/__tests__/StudioGenerate.test.tsx index e2bc54c3..d6de032d 100644 --- a/src/studio/__tests__/StudioGenerate.test.tsx +++ b/src/studio/__tests__/StudioGenerate.test.tsx @@ -12,6 +12,7 @@ const mockGeneration = vi.hoisted(() => ({ })); const mockCode = vi.hoisted(() => ({ + code: '', setCode: vi.fn(), })); @@ -36,12 +37,17 @@ const mockShell = vi.hoisted(() => ({ state: 'drafted' | 'running'; } | null, setAgentRepairWorkflow: vi.fn(), + proposeStagedEdit: vi.fn(), })); vi.mock('../agentAvailability', () => ({ inAppAgentEnabled: () => true, })); +vi.mock('@monaco-editor/react', () => ({ + DiffEditor: () =>
, +})); + vi.mock('../../funnel/hooks/useGeneration', () => ({ useGeneration: () => mockGeneration, })); @@ -69,6 +75,7 @@ vi.mock('../store/useShellStore', () => ({ }), shellStore: { setAgentRepairWorkflow: mockShell.setAgentRepairWorkflow, + proposeStagedEdit: mockShell.proposeStagedEdit, }, })); @@ -78,6 +85,7 @@ beforeEach(() => { mockGeneration.phase = { state: 'idle' }; mockGeneration.events = []; mockGeneration.submit.mockReset(); + mockCode.code = ''; mockCode.setCode.mockReset(); mockGeometry.executeGeometry.mockReset(); mockSelection.selectedFeatureId = null; @@ -85,6 +93,7 @@ beforeEach(() => { mockShell.agentDraftPromptVersion = 0; mockShell.agentRepairWorkflow = null; mockShell.setAgentRepairWorkflow.mockReset(); + mockShell.proposeStagedEdit.mockReset(); }); afterEach(() => cleanup()); @@ -242,4 +251,181 @@ describe('StudioGenerate', () => { 'Fix assembly.part.floating: output-horn floats Action: add a mate', ); }); + + it('stages a generated artifact instead of applying it directly', () => { + mockCode.code = 'const oldPart = box(10, 10, 10);\nreturn oldPart;'; + mockGeneration.phase = { + state: 'done', + generationId: 'gen-1', + anonId: 'anon-1', + artifact: { + title: 'Add mounting holes', + code: 'const newPart = box(10, 10, 10);\nreturn newPart;', + parameters: [], + suggestions: [], + }, + }; + + render(); + + fireEvent.click(screen.getByRole('button', { name: /stage edit/i })); + + expect(mockShell.proposeStagedEdit).toHaveBeenCalledWith(expect.objectContaining({ + id: 'agent:gen-1', + intent: 'Add mounting holes', + fromCode: 'const oldPart = box(10, 10, 10);\nreturn oldPart;', + toCode: 'const newPart = box(10, 10, 10);\nreturn newPart;', + source: { kind: 'agent', label: 'Studio Generate' }, + })); + expect(mockCode.setCode).not.toHaveBeenCalled(); + expect(mockGeometry.executeGeometry).not.toHaveBeenCalled(); + }); + + it('preserves an empty submit baseline when staging after editor code changes', () => { + const { rerender } = render(); + const prompt = screen.getByLabelText('Generate prompt'); + + fireEvent.change(prompt, { target: { value: 'make a cube' } }); + fireEvent.submit(prompt.closest('form')!); + + mockCode.code = 'return box(99);'; + mockGeneration.phase = { + state: 'done', + generationId: 'gen-empty', + anonId: 'anon-empty', + artifact: { + title: 'Make a cube', + code: 'return box(10);', + parameters: [], + suggestions: [], + }, + }; + rerender(); + + fireEvent.click(screen.getByRole('button', { name: /stage edit/i })); + + expect(mockShell.proposeStagedEdit).toHaveBeenCalledWith(expect.objectContaining({ + id: 'agent:gen-empty', + fromCode: '', + toCode: 'return box(10);', + })); + }); + + it('does not show staged status after discarding a generated artifact', () => { + mockGeneration.phase = { + state: 'done', + generationId: 'gen-discard', + anonId: 'anon-discard', + artifact: { + title: 'Throwaway proposal', + code: 'return box(20);', + parameters: [], + suggestions: [], + }, + }; + + render(); + + fireEvent.click(screen.getByRole('button', { name: /discard/i })); + + expect(mockShell.proposeStagedEdit).not.toHaveBeenCalled(); + expect(screen.queryByText(/staged for review/i)).toBeNull(); + expect(screen.getByText(/discarded — Throwaway proposal/i)).toBeTruthy(); + }); + + it('preserves prompt, target, and repair workflow context on staged generated edits', () => { + mockCode.code = 'return box(10);'; + mockSelection.selectedFeatureId = 'output-horn'; + mockShell.agentDraftPrompt = 'Fix assembly.part.floating: output-horn floats Action: add a mate'; + mockShell.agentDraftPromptVersion = 1; + mockShell.agentRepairWorkflow = { + cardId: 'diagnostic:assembly.part.floating:output-horn:0', + code: 'assembly.part.floating', + promptText: 'Fix assembly.part.floating: output-horn floats Action: add a mate', + targetId: 'output-horn', + promptSource: 'fallback', + validityFingerprint: 'before', + state: 'running', + }; + mockGeneration.phase = { + state: 'done', + generationId: 'gen-2', + anonId: 'anon-2', + artifact: { + title: 'Repair output-horn', + code: 'return box(20);', + parameters: [], + suggestions: [], + }, + }; + + render(); + + fireEvent.click(screen.getByRole('button', { name: /stage edit/i })); + + expect(mockShell.proposeStagedEdit).toHaveBeenCalledWith(expect.objectContaining({ + context: { + promptText: 'Fix assembly.part.floating: output-horn floats Action: add a mate', + selectedFeatureId: 'output-horn', + repairWorkflow: mockShell.agentRepairWorkflow, + generationId: 'gen-2', + }, + })); + }); + + it('uses submit-time prompt, target, and workflow context when staging later', () => { + mockCode.code = 'return box(10);'; + mockSelection.selectedFeatureId = 'output-horn'; + const draftedWorkflow = { + cardId: 'diagnostic:assembly.part.floating:output-horn:0', + code: 'assembly.part.floating', + promptText: 'add a mate', + targetId: 'output-horn', + promptSource: 'fallback' as const, + validityFingerprint: 'before', + state: 'drafted' as const, + }; + mockShell.agentRepairWorkflow = draftedWorkflow; + + const { rerender } = render(); + const prompt = screen.getByLabelText('Generate prompt'); + fireEvent.change(prompt, { target: { value: 'add a mate' } }); + fireEvent.submit(prompt.closest('form')!); + + fireEvent.change(prompt, { target: { value: 'wrong later prompt' } }); + mockSelection.selectedFeatureId = 'hinge-pin'; + mockShell.agentRepairWorkflow = { + ...draftedWorkflow, + promptText: 'wrong later prompt', + targetId: 'hinge-pin', + state: 'running', + }; + mockGeneration.phase = { + state: 'done', + generationId: 'gen-context', + anonId: 'anon-context', + artifact: { + title: 'Repair original target', + code: 'return box(20);', + parameters: [], + suggestions: [], + }, + }; + rerender(); + + fireEvent.click(screen.getByRole('button', { name: /stage edit/i })); + + expect(mockShell.proposeStagedEdit).toHaveBeenCalledWith(expect.objectContaining({ + fromCode: 'return box(10);', + context: { + promptText: 'add a mate', + selectedFeatureId: 'output-horn', + repairWorkflow: { + ...draftedWorkflow, + state: 'running', + }, + generationId: 'gen-context', + }, + })); + }); }); diff --git a/src/studio/store/shellStore.ts b/src/studio/store/shellStore.ts index 2257ded0..ad239c98 100644 --- a/src/studio/store/shellStore.ts +++ b/src/studio/store/shellStore.ts @@ -22,6 +22,13 @@ export interface StagedEdit { readonly intent: string; readonly fromCode: string; readonly toCode: string; + /** Agent context captured when the edit was proposed. */ + readonly context?: { + readonly promptText: string; + readonly selectedFeatureId: SelectedFeatureId; + readonly repairWorkflow: AgentRepairWorkflow | null; + readonly generationId?: string; + }; /** Optional snapshot of validateAssembly predicted output post-edit. */ readonly expectedDiagnostics?: ReadonlyArray<{ code: string; severity: string; message: string }>; /** Where the proposal came from. */