From d79fb958e0df6f2e4514fe11fd488576817ba758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20GS=20Pereira?= Date: Fri, 24 Apr 2026 16:00:25 -0300 Subject: [PATCH] sync(ai): mirror openplc-web fix/improve-ai-feature-2 Paired PR for Autonomy-Logic/openplc-web#375. Mirrors shared-surface changes from the web repo: - Per-hunk Accept/Reject UI with brand colors (Monaco overlay) - Orphan cleanup on Undo: snapshot/restore tabs, editors, flows, libraries, files - Data-type tools: update_datatype, delete_datatype + fix create_datatype field propagation - Variable sync fix: use variable name instead of undefined id - Per-POU pending diff state lifted to AI slice (survives tab switches) - Disable inline suggestions toggle + settings popover - Hardcoded model selection (sonnet for chat, haiku for inline) - Agentic loop iteration cap raised from 8 to 16 - Workspace resize handle moved out of nested panel Co-Authored-By: Claude Opus 4.7 (1M context) --- .../editor/monaco/ai-diff-review.ts | 133 +++++++----- .../[workspace]/editor/monaco/index.tsx | 201 +++++++++--------- src/frontend/screens/workspace-screen.tsx | 18 +- src/frontend/store/__tests__/ai-slice.test.ts | 125 +++++++++-- .../store/__tests__/project-slice.test.ts | 12 +- src/frontend/store/slices/ai/index.ts | 2 +- src/frontend/store/slices/ai/slice.ts | 58 ++++- src/frontend/store/slices/ai/types.ts | 44 +++- src/frontend/store/slices/project/slice.ts | 20 +- src/middleware/shared/ports/types.ts | 2 + 10 files changed, 409 insertions(+), 206 deletions(-) diff --git a/src/frontend/components/_features/[workspace]/editor/monaco/ai-diff-review.ts b/src/frontend/components/_features/[workspace]/editor/monaco/ai-diff-review.ts index 7663103c6..2669920d2 100644 --- a/src/frontend/components/_features/[workspace]/editor/monaco/ai-diff-review.ts +++ b/src/frontend/components/_features/[workspace]/editor/monaco/ai-diff-review.ts @@ -2,25 +2,22 @@ import * as monaco from 'monaco-editor' import type { DiffHunk } from '../../../../../utils/ai-diff-review' +// Brand colors (must match src/backend/shared/styles/globals.css :root vars) +const BRAND_DEFAULT = '#0464fb' +const BRAND_MEDIUM_DARK = '#023c97' +// Subtle 10%-alpha tint of the brand color — used for the outlined Reject button's hover. +// The solid BRAND_LIGHT (#b4d0fe) was too loud against the blue text/border. +const REJECT_HOVER_BG = 'rgba(4, 100, 251, 0.1)' + /** Render all diff review UI for the given hunks. Returns a cleanup function. */ export function renderDiffReview( editor: monaco.editor.IStandaloneCodeEditor, hunks: DiffHunk[], - onKeep: (hunkId: string) => void, - onUndo: (hunkId: string) => void, + onAccept: (hunkId: string) => void, + onReject: (hunkId: string) => void, ): () => void { const viewZoneIds: string[] = [] - // Clean up any stale buttons from previous renders - const editorDom = editor.getDomNode() - if (editorDom) { - editorDom.querySelectorAll('.ai-hunk-buttons').forEach((el) => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ;(el as any)._scrollDisposable?.dispose() - el.remove() - }) - } - // 1. Line decorations (green backgrounds for added/modified lines) const decoOptions: monaco.editor.IModelDeltaDecoration[] = [] for (const hunk of hunks) { @@ -70,11 +67,29 @@ export function renderDiffReview( } }) - // 3. Action buttons per hunk ("Keep" / "Undo") + // 3. Action buttons per hunk ("Accept" / "Reject") — rendered as a right-aligned DOM + // overlay anchored to each hunk's start line. Monaco content widgets can't be + // anchored to the viewport edge (they track a code column), so we use a manual + // overlay with an onDidScrollChange handler to keep buttons in sync with scroll. + // Adjacent hunks get a vertical offset if their natural positions would collide. const buttonContainers: HTMLDivElement[] = [] + const scrollDisposables: monaco.IDisposable[] = [] + const editorDom = editor.getDomNode() + const BUTTON_HEIGHT = 18 // approx height of one row of buttons (10px font + 2×padding + border) + const MIN_GAP = 4 + + // Clean up any stale button containers from previous renders (defensive — the cleanup + // closure below should handle this, but guard in case the caller forgets to invoke it). if (editorDom) { - for (const hunk of hunks) { + editorDom.querySelectorAll('.ai-hunk-buttons').forEach((el) => el.remove()) + } + + if (editorDom) { + // Sort by startLine so collision offsetting reads prior bottom positions correctly. + const sortedHunks = [...hunks].sort((a, b) => a.startLine - b.startLine) + + for (const hunk of sortedHunks) { const container = document.createElement('div') container.className = 'ai-hunk-buttons' container.style.cssText = ` @@ -82,64 +97,75 @@ export function renderDiffReview( display: inline-flex; flex-direction: row; gap: 4px; ` - const keepBtn = document.createElement('button') - keepBtn.textContent = 'Keep' - keepBtn.style.cssText = ` - cursor: pointer; border: none; background: rgba(34,197,94,0.2); - color: #4ade80; border-radius: 3px; padding: 1px 8px; + const acceptBtn = document.createElement('button') + acceptBtn.textContent = 'Accept' + acceptBtn.style.cssText = ` + cursor: pointer; border: none; background: ${BRAND_DEFAULT}; + color: #ffffff; border-radius: 3px; padding: 1px 8px; font-size: 10px; font-weight: 500; font-family: inherit; transition: background 0.15s; line-height: 16px; ` - keepBtn.onmouseenter = () => { - keepBtn.style.background = 'rgba(34,197,94,0.35)' + acceptBtn.onmouseenter = () => { + acceptBtn.style.background = BRAND_MEDIUM_DARK } - keepBtn.onmouseleave = () => { - keepBtn.style.background = 'rgba(34,197,94,0.2)' + acceptBtn.onmouseleave = () => { + acceptBtn.style.background = BRAND_DEFAULT } - keepBtn.addEventListener('click', (e) => { + acceptBtn.addEventListener('click', (e) => { e.stopPropagation() - onKeep(hunk.id) + onAccept(hunk.id) }) - const undoBtn = document.createElement('button') - undoBtn.textContent = 'Undo' - undoBtn.style.cssText = ` - cursor: pointer; border: none; background: rgba(239,68,68,0.2); - color: #f87171; border-radius: 3px; padding: 1px 8px; + const rejectBtn = document.createElement('button') + rejectBtn.textContent = 'Reject' + rejectBtn.style.cssText = ` + cursor: pointer; background: transparent; color: ${BRAND_DEFAULT}; + border: 1px solid ${BRAND_DEFAULT}; border-radius: 3px; padding: 0 7px; font-size: 10px; font-weight: 500; font-family: inherit; transition: background 0.15s; line-height: 16px; ` - undoBtn.onmouseenter = () => { - undoBtn.style.background = 'rgba(239,68,68,0.35)' + rejectBtn.onmouseenter = () => { + rejectBtn.style.background = REJECT_HOVER_BG } - undoBtn.onmouseleave = () => { - undoBtn.style.background = 'rgba(239,68,68,0.2)' + rejectBtn.onmouseleave = () => { + rejectBtn.style.background = 'transparent' } - undoBtn.addEventListener('click', (e) => { + rejectBtn.addEventListener('click', (e) => { e.stopPropagation() - onUndo(hunk.id) + onReject(hunk.id) }) - container.appendChild(keepBtn) - container.appendChild(undoBtn) - - // Position based on the line's top coordinate - const topPx = Math.max(0, editor.getTopForLineNumber(hunk.startLine) - editor.getScrollTop()) - container.style.top = `${topPx}px` + container.appendChild(acceptBtn) + container.appendChild(rejectBtn) editorDom.appendChild(container) buttonContainers.push(container) + } - // Update position on scroll - const scrollDisposable = editor.onDidScrollChange(() => { - const newTop = Math.max(0, editor.getTopForLineNumber(hunk.startLine) - editor.getScrollTop()) - container.style.top = `${newTop}px` + const layoutButtons = () => { + const scrollTop = editor.getScrollTop() + const editorHeight = editor.getLayoutInfo().height + let lastBottom = -Infinity + sortedHunks.forEach((hunk, idx) => { + const container = buttonContainers[idx] + const natural = editor.getTopForLineNumber(hunk.startLine) - scrollTop + + // Hide buttons whose anchor line is outside the visible editor area — otherwise + // hunks that scroll off the top would clamp to top:0 and pile up at the viewport edge. + if (natural < 0 || natural > editorHeight - BUTTON_HEIGHT) { + container.style.display = 'none' + return + } + + container.style.display = 'inline-flex' + const top = Math.max(natural, lastBottom + MIN_GAP) + container.style.top = `${top}px` + lastBottom = top + BUTTON_HEIGHT }) - - // Store disposable for cleanup - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ;(container as any)._scrollDisposable = scrollDisposable } + + layoutButtons() + scrollDisposables.push(editor.onDidScrollChange(() => layoutButtons())) } // Cleanup function — removes everything @@ -150,10 +176,7 @@ export function renderDiffReview( accessor.removeZone(id) } }) - for (const container of buttonContainers) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ;(container as any)._scrollDisposable?.dispose() - container.remove() - } + for (const d of scrollDisposables) d.dispose() + for (const container of buttonContainers) container.remove() } } diff --git a/src/frontend/components/_features/[workspace]/editor/monaco/index.tsx b/src/frontend/components/_features/[workspace]/editor/monaco/index.tsx index 74f4f2b60..57a68d301 100644 --- a/src/frontend/components/_features/[workspace]/editor/monaco/index.tsx +++ b/src/frontend/components/_features/[workspace]/editor/monaco/index.tsx @@ -10,7 +10,7 @@ import { useAI, useCapabilities, useProject } from '../../../../../../middleware import { useDebugBoolValuesMap, useDebugNonBoolValuesMap } from '../../../../../hooks/use-debug-value' import { executeSaveActiveFile, executeSaveProject } from '../../../../../services/save-actions' import { openPLCStoreBase, useOpenPLCStore } from '../../../../../store' -import { applyAcceptedHunks, computeHunks, type DiffHunk } from '../../../../../utils/ai-diff-review' +import { applyAcceptedHunks, computeHunks } from '../../../../../utils/ai-diff-review' import { getExtensionFromLanguage, getFolderFromPouType } from '../../../../../utils/PLC/pou-file-extensions' import { parseHybridPouFromString, parseTextualPouFromString } from '../../../../../utils/PLC/pou-text-parser' import { Modal, ModalContent, ModalTitle } from '../../../../_molecules/modal' @@ -161,6 +161,8 @@ const MonacoEditor = (props: monacoEditorProps): ReturnType(null) - // AI diff review state — per-hunk inline review with keep/undo buttons - const [diffReview, setDiffReview] = useState<{ - active: boolean - oldBody: string - newBody: string - hunks: DiffHunk[] - acceptedHunks: Set - } | null>(null) + /** + * Bumped every time @monaco-editor/react re-mounts the underlying editor + * (happens on tab switch in web, because ``). + * Used as a render-effect dep so the diff-review UI re-attaches to the fresh + * editor instance. Without this, the render effect runs before the new + * editor's onMount fires and silently no-ops on a disposed editor instance. + */ + const [editorInstanceId, setEditorInstanceId] = useState(0) const [templatesInjected, setTemplatesInjected] = useState>(new Set()) @@ -206,84 +208,87 @@ const MonacoEditor = (props: monacoEditorProps): ReturnType + // remounts on tab switch. The new editor's onMount fires AFTER this effect first runs + // with the new `name`, so the effect would otherwise see a stale/disposed editor ref. + // The counter bumps inside handleEditorDidMount, triggering this effect to re-run + // once the new editor is actually ready. useEffect(() => { const editor = editorRef.current if (!editor) return - if (!diffReview?.active || diffReview.hunks.length === 0) return () => {} - // Get only pending (unresolved) hunks - const pendingHunks = diffReview.hunks.filter((h) => diffReview.acceptedHunks.has(h.id)) + const entry = pendingDiffs[name] + if (!entry || entry.hunks.length === 0) return () => {} + + const pendingSet = new Set(entry.acceptedHunks) + const pendingHunks = entry.hunks.filter((h) => pendingSet.has(h.id)) if (pendingHunks.length === 0) { - // All hunks resolved — exit diff review - setDiffReview(null) + clearPendingDiff(name) return () => {} } const handleKeepHunk = (hunkId: string) => { - // "Keep" = accept this hunk (new code stays), remove from pending - setDiffReview((prev) => { - if (!prev) return prev - const newAccepted = new Set(prev.acceptedHunks) - newAccepted.delete(hunkId) // Remove from pending set = resolved as kept - const remaining = prev.hunks.filter((h) => newAccepted.has(h.id)) - if (remaining.length === 0) return null // All resolved - return { ...prev, acceptedHunks: newAccepted } - }) + const state = openPLCStoreBase.getState() + const current = state.ai.pendingDiffs[name] + if (!current) return + const nextAccepted = current.acceptedHunks.filter((id) => id !== hunkId) + if (nextAccepted.length === 0) { + clearPendingDiff(name) + return + } + updatePendingDiffAcceptedHunks(name, nextAccepted) } const handleUndoHunk = (hunkId: string) => { - // "Undo" = reject this hunk, revert those lines to old version - setDiffReview((prev) => { - if (!prev) return prev - const newAccepted = new Set(prev.acceptedHunks) - newAccepted.delete(hunkId) - - // Rebuild body: accepted hunks keep new code, this rejected hunk keeps old code - const keptIds = new Set() - for (const h of prev.hunks) { - if (h.id === hunkId) continue // This one is undone - if (!newAccepted.has(h.id)) { - // Already resolved as kept - keptIds.add(h.id) - } else { - // Still pending — treat as kept for now (new code) - keptIds.add(h.id) - } - } - - const newBody = applyAcceptedHunks(prev.oldBody, prev.newBody, prev.hunks, keptIds) - - // Update editor model with rebuilt body - const model = editor.getModel() - if (model) { - isSyncingModelRef.current = true - const fullRange = model.getFullModelRange() - editor.executeEdits('ai-diff-undo-hunk', [{ range: fullRange, text: newBody }]) - isSyncingModelRef.current = false - } - setLocalText(newBody) - - // Update store - const state = openPLCStoreBase.getState() - state.projectActions.updatePou({ name, content: { language, value: newBody } }) - - // Recompute hunks for remaining pending changes - const remainingHunks = prev.hunks.filter((h) => newAccepted.has(h.id)) - if (remainingHunks.length === 0) return null + const state = openPLCStoreBase.getState() + const current = state.ai.pendingDiffs[name] + if (!current) return + + // Rebuild body: every hunk except this one is treated as "kept" (new code), + // the rejected one reverts to the old text. + const keptIds = new Set(current.hunks.filter((h) => h.id !== hunkId).map((h) => h.id)) + const newBody = applyAcceptedHunks(current.oldBody, current.newBody, current.hunks, keptIds) + + // Update editor model with rebuilt body + const model = editor.getModel() + if (model) { + isSyncingModelRef.current = true + const fullRange = model.getFullModelRange() + editor.executeEdits('ai-diff-undo-hunk', [{ range: fullRange, text: newBody }]) + isSyncingModelRef.current = false + } + setLocalText(newBody) - // Recompute line positions for remaining hunks - const freshHunks = computeHunks(prev.oldBody, newBody) - const freshAccepted = new Set(freshHunks.map((h) => h.id)) + // Propagate to project slice + state.projectActions.updatePou({ name, content: { language, value: newBody } }) - if (freshHunks.length === 0) return null - return { ...prev, newBody, hunks: freshHunks, acceptedHunks: freshAccepted } + // Recompute hunks for remaining pending changes, against the new body. + const freshHunks = computeHunks(current.oldBody, newBody) + if (freshHunks.length === 0) { + clearPendingDiff(name) + return + } + updatePendingDiff(name, { + newBody, + hunks: freshHunks, + acceptedHunks: freshHunks.map((h) => h.id), }) } const cleanup = renderDiffReview(editor, pendingHunks, handleKeepHunk, handleUndoHunk) return cleanup - }, [diffReview, name, language]) + }, [ + pendingDiffs, + name, + language, + editorInstanceId, + clearPendingDiff, + updatePendingDiff, + updatePendingDiffAcceptedHunks, + ]) useEffect(() => { if (editorRef.current && searchQuery) { @@ -827,6 +832,7 @@ const MonacoEditor = (props: monacoEditorProps): ReturnType registration.dispose() - }, [name, language, aiState.isEnabled, aiState.hasConsented, capabilities.hasAIAssistant, aiPort]) + }, [ + name, + language, + aiState.isEnabled, + aiState.hasConsented, + aiState.preferences.inlineCompletionsEnabled, + capabilities.hasAIAssistant, + aiPort, + ]) // ----------------------------------------------------------------------- // Theme management @@ -865,6 +879,10 @@ const MonacoEditor = (props: monacoEditorProps): ReturnType id + 1) if (!editorInstance || !monacoInstance) return @@ -1032,29 +1050,16 @@ const MonacoEditor = (props: monacoEditorProps): ReturnType { - const { - pouName: targetPou, - body, - oldBody, - } = (e as CustomEvent<{ pouName: string; body: string; oldBody?: string }>).detail + const { pouName: targetPou, body } = (e as CustomEvent<{ pouName: string; body: string; oldBody?: string }>) + .detail if (targetPou !== name) return - // If no old body provided (backward compat), apply directly - if (oldBody === undefined) { - const model = editorInstance.getModel() - if (model && model.getValue() !== body) { - isSyncingModelRef.current = true - const fullRange = model.getFullModelRange() - editorInstance.executeEdits('ai-tool-update', [{ range: fullRange, text: body }]) - isSyncingModelRef.current = false - } - setLocalText(body) - return - } - - // Update the editor model with the new body first (so decorations render on actual lines) const model = editorInstance.getModel() if (model && model.getValue() !== body) { isSyncingModelRef.current = true @@ -1063,29 +1068,21 @@ const MonacoEditor = (props: monacoEditorProps): ReturnType h.id)) - setDiffReview({ active: true, oldBody, newBody: body, hunks, acceptedHunks: acceptedIds }) } window.addEventListener('ai-pou-updated', handlePouUpdated) - // Listen for global accept/reject from chat panel + // Listen for global accept/reject from chat panel. These fire on the chat's + // Keep/Undo All buttons and clear per-POU entries; the chat panel itself + // also calls clearAllPendingDiffs() to cover POUs that aren't currently active. const handleAcceptAllHunks = (e: Event) => { const { pouName: targetPou } = (e as CustomEvent<{ pouName: string }>).detail - if (targetPou !== name) return - setDiffReview(null) + clearPendingDiff(targetPou) } window.addEventListener('ai-accept-all-hunks', handleAcceptAllHunks) const handleRejectAllHunks = (e: Event) => { const { pouName: targetPou } = (e as CustomEvent<{ pouName: string }>).detail - if (targetPou !== name) return - setDiffReview(null) + clearPendingDiff(targetPou) } window.addEventListener('ai-reject-all-hunks', handleRejectAllHunks) diff --git a/src/frontend/screens/workspace-screen.tsx b/src/frontend/screens/workspace-screen.tsx index 5466fad52..50943487d 100644 --- a/src/frontend/screens/workspace-screen.tsx +++ b/src/frontend/screens/workspace-screen.tsx @@ -397,18 +397,12 @@ const WorkspaceScreen = () => { ) : ( )} - - - + +
{ expect(ai.isEnabled).toBe(false) expect(ai.isLoading).toBe(false) expect(ai.hasConsented).toBe(false) - expect(ai.model).toBe('haiku') expect(ai.creditsUsed).toBe(0) expect(ai.creditsTotal).toBe(500) expect(ai.tier).toBe('free') @@ -49,6 +48,21 @@ describe('createAISlice', () => { expect(ai.isAgenticLoopRunning).toBe(false) expect(ai.isChatOpen).toBe(false) expect(ai.error).toBeNull() + expect(ai.preferences).toEqual({ inlineCompletionsEnabled: true }) + expect(ai.pendingDiffs).toEqual({}) + }) + }) + + describe('setPreference', () => { + it('toggles inlineCompletionsEnabled off', () => { + store.getState().aiActions.setPreference('inlineCompletionsEnabled', false) + expect(store.getState().ai.preferences.inlineCompletionsEnabled).toBe(false) + }) + + it('toggles inlineCompletionsEnabled back on', () => { + store.getState().aiActions.setPreference('inlineCompletionsEnabled', false) + store.getState().aiActions.setPreference('inlineCompletionsEnabled', true) + expect(store.getState().ai.preferences.inlineCompletionsEnabled).toBe(true) }) }) @@ -95,19 +109,6 @@ describe('createAISlice', () => { }) }) - describe('setAIModel', () => { - it('changes the model to sonnet', () => { - store.getState().aiActions.setAIModel('sonnet') - expect(store.getState().ai.model).toBe('sonnet') - }) - - it('changes the model back to haiku', () => { - store.getState().aiActions.setAIModel('sonnet') - store.getState().aiActions.setAIModel('haiku') - expect(store.getState().ai.model).toBe('haiku') - }) - }) - describe('setCredits', () => { it('sets both used and total credits', () => { store.getState().aiActions.setCredits(42, 1000) @@ -327,6 +328,82 @@ describe('createAISlice', () => { expect(store.getState().ai.isChatOpen).toBe(false) }) }) + + // --------------------------------------------------------------------------- + // Pending diff review (per-POU) + // --------------------------------------------------------------------------- + + describe('pendingDiffs', () => { + const sampleEntry = { + oldBody: 'OLD', + newBody: 'NEW', + hunks: [{ id: 'h1', type: 'added' as const, startLine: 1, endLine: 1, newLines: ['NEW'], oldLines: [] }], + acceptedHunks: ['h1'], + } + + it('defaults to empty object', () => { + expect(store.getState().ai.pendingDiffs).toEqual({}) + }) + + it('setPendingDiff writes an entry keyed by POU name', () => { + store.getState().aiActions.setPendingDiff('main', sampleEntry) + expect(store.getState().ai.pendingDiffs.main).toEqual(sampleEntry) + }) + + it('supports multiple POUs simultaneously', () => { + store.getState().aiActions.setPendingDiff('main', sampleEntry) + store.getState().aiActions.setPendingDiff('helper', { ...sampleEntry, oldBody: 'OLD2' }) + expect(Object.keys(store.getState().ai.pendingDiffs)).toEqual(['main', 'helper']) + }) + + it('updatePendingDiffAcceptedHunks updates only the acceptedHunks of an existing entry', () => { + store.getState().aiActions.setPendingDiff('main', sampleEntry) + store.getState().aiActions.updatePendingDiffAcceptedHunks('main', []) + expect(store.getState().ai.pendingDiffs.main.acceptedHunks).toEqual([]) + expect(store.getState().ai.pendingDiffs.main.newBody).toBe('NEW') + }) + + it('updatePendingDiffAcceptedHunks is a no-op for missing POU', () => { + store.getState().aiActions.updatePendingDiffAcceptedHunks('nope', ['x']) + expect(store.getState().ai.pendingDiffs.nope).toBeUndefined() + }) + + it('updatePendingDiff replaces newBody + hunks + acceptedHunks atomically', () => { + store.getState().aiActions.setPendingDiff('main', sampleEntry) + const freshHunks = [ + { id: 'h2', type: 'modified' as const, startLine: 2, endLine: 2, newLines: ['X'], oldLines: ['Y'] }, + ] + store + .getState() + .aiActions.updatePendingDiff('main', { newBody: 'NEWER', hunks: freshHunks, acceptedHunks: ['h2'] }) + const entry = store.getState().ai.pendingDiffs.main + expect(entry.newBody).toBe('NEWER') + expect(entry.hunks).toEqual(freshHunks) + expect(entry.acceptedHunks).toEqual(['h2']) + // Old body preserved + expect(entry.oldBody).toBe('OLD') + }) + + it('updatePendingDiff is a no-op for missing POU', () => { + store.getState().aiActions.updatePendingDiff('nope', { newBody: 'x', hunks: [], acceptedHunks: [] }) + expect(store.getState().ai.pendingDiffs.nope).toBeUndefined() + }) + + it('clearPendingDiff removes a specific POU entry', () => { + store.getState().aiActions.setPendingDiff('main', sampleEntry) + store.getState().aiActions.setPendingDiff('helper', sampleEntry) + store.getState().aiActions.clearPendingDiff('main') + expect(store.getState().ai.pendingDiffs.main).toBeUndefined() + expect(store.getState().ai.pendingDiffs.helper).toBeDefined() + }) + + it('clearAllPendingDiffs resets to empty', () => { + store.getState().aiActions.setPendingDiff('main', sampleEntry) + store.getState().aiActions.setPendingDiff('helper', sampleEntry) + store.getState().aiActions.clearAllPendingDiffs() + expect(store.getState().ai.pendingDiffs).toEqual({}) + }) + }) }) // --------------------------------------------------------------------------- @@ -342,16 +419,26 @@ describe('createAISliceFactory', () => { }) it('overrides isEnabled and hasConsented from config', () => { - const store = createStore()(createAISliceFactory({ isFeatureEnabled: true, hasUserConsented: true })) + const store = createStore()( + createAISliceFactory({ isFeatureEnabled: true, hasUserConsented: true, inlineCompletionsEnabled: true }), + ) const { ai } = store.getState() expect(ai.isEnabled).toBe(true) expect(ai.hasConsented).toBe(true) }) + it('hydrates inlineCompletionsEnabled preference from config', () => { + const store = createStore()( + createAISliceFactory({ isFeatureEnabled: true, hasUserConsented: true, inlineCompletionsEnabled: false }), + ) + expect(store.getState().ai.preferences.inlineCompletionsEnabled).toBe(false) + }) + it('preserves other default values when config is provided', () => { - const store = createStore()(createAISliceFactory({ isFeatureEnabled: true, hasUserConsented: false })) + const store = createStore()( + createAISliceFactory({ isFeatureEnabled: true, hasUserConsented: false, inlineCompletionsEnabled: true }), + ) const { ai } = store.getState() - expect(ai.model).toBe('haiku') expect(ai.creditsUsed).toBe(0) expect(ai.creditsTotal).toBe(500) expect(ai.tier).toBe('free') @@ -364,7 +451,9 @@ describe('createAISliceFactory', () => { }) it('creates functional actions when config is provided', () => { - const store = createStore()(createAISliceFactory({ isFeatureEnabled: true, hasUserConsented: true })) + const store = createStore()( + createAISliceFactory({ isFeatureEnabled: true, hasUserConsented: true, inlineCompletionsEnabled: true }), + ) store.getState().aiActions.setAIEnabled(false) expect(store.getState().ai.isEnabled).toBe(false) }) diff --git a/src/frontend/store/__tests__/project-slice.test.ts b/src/frontend/store/__tests__/project-slice.test.ts index cace90ae4..0a2efb237 100644 --- a/src/frontend/store/__tests__/project-slice.test.ts +++ b/src/frontend/store/__tests__/project-slice.test.ts @@ -782,13 +782,13 @@ describe('createProjectSlice', () => { expect(store.getState().project.data.pous[0].interface?.variables).toHaveLength(1) }) - it('returns ok even when variables array not available', () => { + it('returns fail when variables array not available', () => { const result = store.getState().projectActions.deleteVariable({ scope: 'local', associatedPou: 'Missing', variableId: 'x', }) - expect(result.ok).toBe(true) + expect(result.ok).toBe(false) }) }) @@ -2173,14 +2173,14 @@ describe('createProjectSlice', () => { // ------------------------------------------------------------------------- describe('defensive guards for missing configs', () => { - it('deleteVariable by rowId/variableId when variable not found', () => { + it('deleteVariable by rowId/variableId when variable not found returns fail', () => { seedPou(store, makePou('MyProg', 'program')) const result = store.getState().projectActions.deleteVariable({ scope: 'local', associatedPou: 'MyProg', rowId: 99, }) - expect(result.ok).toBe(true) + expect(result.ok).toBe(false) }) it('rearrangeStructureVariables when item at rowId does not exist', () => { @@ -2552,13 +2552,13 @@ describe('createProjectSlice', () => { expect(store.getState().project.data.pous[0].interface?.variables[0].class).toBe('input') }) - it('deleteVariable for global scope when variableName not found skips external check', () => { + it('deleteVariable for global scope when variableName not found returns fail', () => { store.getState().projectActions.createVariable({ scope: 'global', data: makeVariable('exists', 'global') }) const result = store.getState().projectActions.deleteVariable({ scope: 'global', variableName: 'doesNotExist', }) - expect(result.ok).toBe(true) + expect(result.ok).toBe(false) // The existing variable should still be there expect(store.getState().project.data.configurations.resource.globalVariables).toHaveLength(1) }) diff --git a/src/frontend/store/slices/ai/index.ts b/src/frontend/store/slices/ai/index.ts index 6e6257698..e3095fd36 100644 --- a/src/frontend/store/slices/ai/index.ts +++ b/src/frontend/store/slices/ai/index.ts @@ -1,3 +1,3 @@ export { createAISlice, createAISliceFactory } from './slice' -export type { AIActions, AIModel, AISlice, AIState, ChatMessage, ChatMessageRole } from './types' +export type { AIActions, AIPreferences, AISlice, AIState, ChatMessage, ChatMessageRole, DiffReviewEntry } from './types' export { MAX_CONVERSATION_MESSAGES } from './types' diff --git a/src/frontend/store/slices/ai/slice.ts b/src/frontend/store/slices/ai/slice.ts index 1c7e2f828..a21ba843f 100644 --- a/src/frontend/store/slices/ai/slice.ts +++ b/src/frontend/store/slices/ai/slice.ts @@ -9,7 +9,9 @@ const DEFAULT_AI_STATE: AISlice['ai'] = { isEnabled: false, isLoading: false, hasConsented: false, - model: 'haiku', + preferences: { + inlineCompletionsEnabled: true, + }, creditsUsed: 0, creditsTotal: 500, tier: 'free', @@ -19,10 +21,17 @@ const DEFAULT_AI_STATE: AISlice['ai'] = { isAgenticLoopRunning: false, isChatOpen: false, error: null, + pendingDiffs: {}, } export function createAISliceFactory(config?: AIFeatureConfig): StateCreator { - const overrides = config ? { isEnabled: config.isFeatureEnabled, hasConsented: config.hasUserConsented } : {} + const overrides = config + ? { + isEnabled: config.isFeatureEnabled, + hasConsented: config.hasUserConsented, + preferences: { inlineCompletionsEnabled: config.inlineCompletionsEnabled }, + } + : {} return (setState) => ({ ai: { ...DEFAULT_AI_STATE, ...overrides }, @@ -48,10 +57,10 @@ export function createAISliceFactory(config?: AIFeatureConfig): StateCreator { + setPreference: (key, value) => { setState( produce(({ ai }: AISlice) => { - ai.model = model + ai.preferences[key] = value }), ) }, @@ -150,6 +159,47 @@ export function createAISliceFactory(config?: AIFeatureConfig): StateCreator { + setState( + produce(({ ai }: AISlice) => { + ai.pendingDiffs[pouName] = entry + }), + ) + }, + updatePendingDiffAcceptedHunks: (pouName, acceptedHunks) => { + setState( + produce(({ ai }: AISlice) => { + const existing = ai.pendingDiffs[pouName] + if (!existing) return + existing.acceptedHunks = acceptedHunks + }), + ) + }, + updatePendingDiff: (pouName, { newBody, hunks, acceptedHunks }) => { + setState( + produce(({ ai }: AISlice) => { + const existing = ai.pendingDiffs[pouName] + if (!existing) return + existing.newBody = newBody + existing.hunks = hunks + existing.acceptedHunks = acceptedHunks + }), + ) + }, + clearPendingDiff: (pouName) => { + setState( + produce(({ ai }: AISlice) => { + delete ai.pendingDiffs[pouName] + }), + ) + }, + clearAllPendingDiffs: () => { + setState( + produce(({ ai }: AISlice) => { + ai.pendingDiffs = {} + }), + ) + }, }, }) } diff --git a/src/frontend/store/slices/ai/types.ts b/src/frontend/store/slices/ai/types.ts index e330db688..ebd284cfa 100644 --- a/src/frontend/store/slices/ai/types.ts +++ b/src/frontend/store/slices/ai/types.ts @@ -1,14 +1,43 @@ import type { ChatMessage, ChatMessageRole } from '../../../../middleware/shared/ports/types' +import type { DiffHunk } from '../../../utils/ai-diff-review' export type { ChatMessage, ChatMessageRole } // --------------------------------------------------------------------------- -// AI model and message types +// AI message types // --------------------------------------------------------------------------- -export type AIModel = 'haiku' | 'sonnet' export type AIAction = 'complete' | 'chat' +// --------------------------------------------------------------------------- +// User preferences (persisted to localStorage) +// --------------------------------------------------------------------------- + +export type AIPreferences = { + inlineCompletionsEnabled: boolean +} + +// --------------------------------------------------------------------------- +// Per-POU pending diff review state +// --------------------------------------------------------------------------- + +/** + * Snapshot of a POU's diff review session. One entry exists per POU with + * unresolved hunks — stored by POU name so the user can switch tabs freely + * and still return to pending reviews. + * + * `acceptedHunks` holds the ids of hunks that are still pending (the name is + * historical — see handleKeepHunk/handleUndoHunk in monaco/index.tsx, which + * pre-dates this lift-to-store). Accepting a hunk removes its id; rejecting + * rebuilds the body and recomputes the remaining hunks. + */ +export type DiffReviewEntry = { + oldBody: string + newBody: string + hunks: DiffHunk[] + acceptedHunks: string[] +} + // --------------------------------------------------------------------------- // AI state // --------------------------------------------------------------------------- @@ -18,7 +47,7 @@ export type AIState = { isEnabled: boolean isLoading: boolean hasConsented: boolean - model: AIModel + preferences: AIPreferences creditsUsed: number creditsTotal: number tier: 'free' | 'pro' @@ -28,6 +57,8 @@ export type AIState = { isAgenticLoopRunning: boolean isChatOpen: boolean error: string | null + /** Pending diff review entries, keyed by POU name. */ + pendingDiffs: Record } } @@ -39,7 +70,7 @@ export type AIActions = { setAIEnabled: (enabled: boolean) => void setAILoading: (loading: boolean) => void setAIConsented: (consented: boolean) => void - setAIModel: (model: AIModel) => void + setPreference: (key: K, value: AIPreferences[K]) => void setCredits: (used: number, total: number) => void setTier: (tier: 'free' | 'pro') => void setCurrentPeriodEnd: (date: string | null) => void @@ -52,6 +83,11 @@ export type AIActions = { clearConversation: () => void toggleChat: () => void setChatOpen: (open: boolean) => void + setPendingDiff: (pouName: string, entry: DiffReviewEntry) => void + updatePendingDiffAcceptedHunks: (pouName: string, acceptedHunks: string[]) => void + updatePendingDiff: (pouName: string, update: { newBody: string; hunks: DiffHunk[]; acceptedHunks: string[] }) => void + clearPendingDiff: (pouName: string) => void + clearAllPendingDiffs: () => void } // --------------------------------------------------------------------------- diff --git a/src/frontend/store/slices/project/slice.ts b/src/frontend/store/slices/project/slice.ts index e5381dbed..11375a439 100644 --- a/src/frontend/store/slices/project/slice.ts +++ b/src/frontend/store/slices/project/slice.ts @@ -495,24 +495,36 @@ const createProjectSlice: StateCreator = (se } } + let response: ProjectResponse = { ok: true } setState( produce((slice: ProjectSlice) => { const variables = scope === 'local' && associatedPou ? slice.project.data.pous.find((p) => p.name === associatedPou)?.interface?.variables : slice.project.data.configurations.resource.globalVariables - if (!variables) return + if (!variables) { + response = fail('Variable container not found') + return + } if (variableName) { const idx = variables.findIndex((v) => v.name.toLowerCase() === variableName.toLowerCase()) - if (idx !== -1) variables.splice(idx, 1) + if (idx === -1) { + response = fail(`Variable "${variableName}" not found`, 'Variable not found') + return + } + variables.splice(idx, 1) return } const found = getVariableBasedOnRowIdOrVariableId(variables, rowId, variableId) - if (found) variables.splice(found.index, 1) + if (!found) { + response = fail('Variable not found') + return + } + variables.splice(found.index, 1) }), ) - return ok() + return response }, rearrangeVariables: ({ scope, associatedPou, rowId, variableId, newIndex }) => { setState( diff --git a/src/middleware/shared/ports/types.ts b/src/middleware/shared/ports/types.ts index 7e7d5b7d3..11d931b64 100644 --- a/src/middleware/shared/ports/types.ts +++ b/src/middleware/shared/ports/types.ts @@ -714,6 +714,8 @@ export interface AIFeatureConfig { isFeatureEnabled: boolean /** Whether the user has previously consented to AI usage */ hasUserConsented: boolean + /** User preference: whether inline ghost-text completions are active in editors */ + inlineCompletionsEnabled: boolean } // ---------------------------------------------------------------------------