From dbe0269f60905b5ae3315fa2071c15b1dde42954 Mon Sep 17 00:00:00 2001 From: abose Date: Thu, 19 Feb 2026 18:24:14 +0530 Subject: [PATCH] refactor: purify AISnapshotStore as data-only layer Move _undoApplied (UI button state) to AIChatPanel. Eliminate _initialSnapshotCreated (redundant with getSnapshotCount() > 0) and _lastSnapshotAfter (redundant with _snapshots[last]). Reorder createInitialSnapshot before recordFileBeforeEdit so back-fill populates _snapshots[0] directly. Add Phoenix MCP usage hints to CLAUDE.md. --- CLAUDE.md | 12 ++++++++ src/core-ai/AIChatPanel.js | 21 ++++++++----- src/core-ai/AISnapshotStore.js | 44 ++-------------------------- src/core-ai/editApplyVerification.md | 23 ++++++++------- 4 files changed, 41 insertions(+), 59 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cf905ef06..224ead6eb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,3 +12,15 @@ - Always use curly braces for `if`/`else`/`for`/`while`. - No trailing whitespace. - Use `const` and `let` instead of `var`. + +## Phoenix MCP (Desktop App Testing) + +Use `exec_js` to run JS in the Phoenix browser runtime. jQuery `$()` is global. `brackets.test.*` exposes internal modules (DocumentManager, CommandManager, ProjectManager, FileSystem, EditorManager). Always `return` a value from `exec_js` to see results. Prefer reusing an already-running Phoenix instance (`get_phoenix_status`) over launching a new one. + +**Open AI sidebar tab:** `document.querySelectorAll('span').forEach(s => { if (s.textContent.trim() === 'AI' && s.childNodes.length === 1) s.parentElement.click(); });` + +**Send AI chat message:** `$('.ai-chat-textarea').val('prompt'); $('.ai-chat-textarea').trigger('input'); $('.ai-send-btn').click();` + +**Click AI chat buttons:** `$('.ai-edit-restore-btn:contains("Undo")').click();` + +**Check logs:** `get_browser_console_logs` with `filter` regex (e.g. `"AI UI"`, `"error"`) and `tail` — includes both browser console and Node.js (PhNode) logs. Use `get_terminal_logs` for Electron process output (only available if Phoenix was launched via `start_phoenix`). diff --git a/src/core-ai/AIChatPanel.js b/src/core-ai/AIChatPanel.js index c555f517a..af54050c2 100644 --- a/src/core-ai/AIChatPanel.js +++ b/src/core-ai/AIChatPanel.js @@ -42,6 +42,7 @@ define(function (require, exports, module) { const _previousContentMap = {}; // filePath → previous content before edit, for undo support let _currentEdits = []; // edits in current response, for summary card let _firstEditInResponse = true; // tracks first edit per response for initial PUC + let _undoApplied = false; // whether undo/restore has been clicked on any card // --- AI event trace logging (compact, non-flooding) --- let _traceTextChunks = 0; let _traceToolStreamCounts = {}; // toolId → count @@ -292,6 +293,7 @@ define(function (require, exports, module) { _hasReceivedContent = false; _isStreaming = false; _firstEditInResponse = true; + _undoApplied = false; SnapshotStore.reset(); Object.keys(_previousContentMap).forEach(function (key) { delete _previousContentMap[key]; @@ -493,15 +495,16 @@ define(function (require, exports, module) { linesRemoved: oldLines }); - // Capture pre-edit content into pending snapshot and back-fill + // Capture pre-edit content for snapshot tracking const previousContent = _previousContentMap[edit.file]; const isNewFile = (edit.oldText === null && (previousContent === undefined || previousContent === "")); - SnapshotStore.recordFileBeforeEdit(edit.file, previousContent, isNewFile); - // On first edit per response, insert initial PUC if needed + // On first edit per response, insert initial PUC if needed. + // Create initial snapshot *before* recordFileBeforeEdit so it pushes + // an empty {} that recordFileBeforeEdit will back-fill directly. if (_firstEditInResponse) { _firstEditInResponse = false; - if (!SnapshotStore.isInitialSnapshotCreated()) { + if (SnapshotStore.getSnapshotCount() === 0) { const initialIndex = SnapshotStore.createInitialSnapshot(); // Insert initial restore point PUC before the current tool indicator const $puc = $( @@ -525,6 +528,9 @@ define(function (require, exports, module) { } } + // Record pre-edit content into pending snapshot and back-fill + SnapshotStore.recordFileBeforeEdit(edit.file, previousContent, isNewFile); + // Find the oldest Edit/Write tool indicator for this file that doesn't // already have edit actions. This is more robust than matching by toolId // because the SDK with includePartialMessages may re-emit tool_use blocks @@ -601,6 +607,7 @@ define(function (require, exports, module) { function _appendEditSummary() { // Finalize snapshot and get the after-snapshot index const afterIndex = SnapshotStore.finalizeResponse(); + _undoApplied = false; // Aggregate per-file stats const fileStats = {}; @@ -630,7 +637,7 @@ define(function (require, exports, module) { .attr("title", "Restore files to this point"); // Determine button label: "Undo" if not undone, else "Restore to this point" - const isUndo = !SnapshotStore.isUndoApplied(); + const isUndo = !_undoApplied; const label = isUndo ? "Undo" : "Restore to this point"; const title = isUndo ? "Undo changes from this response" : "Restore files to this point"; @@ -687,7 +694,7 @@ define(function (require, exports, module) { $msgs.find(".ai-restore-highlighted").removeClass("ai-restore-highlighted"); // Once any "Restore to this point" is clicked, undo is no longer applicable - SnapshotStore.setUndoApplied(true); + _undoApplied = true; // Reset all buttons to "Restore to this point" $msgs.find('.ai-edit-restore-btn').each(function () { @@ -718,7 +725,7 @@ define(function (require, exports, module) { */ function _onUndoClick(afterIndex) { const $msgs = _$msgs(); - SnapshotStore.setUndoApplied(true); + _undoApplied = true; const targetIndex = afterIndex - 1; // Reset all buttons to "Restore to this point" diff --git a/src/core-ai/AISnapshotStore.js b/src/core-ai/AISnapshotStore.js index d5eb7c200..efe70b9d2 100644 --- a/src/core-ai/AISnapshotStore.js +++ b/src/core-ai/AISnapshotStore.js @@ -33,10 +33,7 @@ define(function (require, exports, module) { // --- Private state --- const _contentStore = {}; // hash → content string (content-addressable dedup) let _snapshots = []; // flat: _snapshots[i] = { filePath: hash|null } - let _lastSnapshotAfter = {}; // cumulative state after last completed response let _pendingBeforeSnap = {}; // built during current response: filePath → hash|null - let _initialSnapshotCreated = false; // has the initial (pre-AI) snapshot been pushed? - let _undoApplied = false; // --- Path utility --- @@ -226,10 +223,6 @@ define(function (require, exports, module) { snap[filePath] = hash; } }); - // Also back-fill _lastSnapshotAfter - if (_lastSnapshotAfter[filePath] === undefined) { - _lastSnapshotAfter[filePath] = hash; - } } } @@ -239,19 +232,10 @@ define(function (require, exports, module) { * @return {number} the snapshot index (always 0) */ function createInitialSnapshot() { - const snap = Object.assign({}, _lastSnapshotAfter); - _snapshots.push(snap); - _initialSnapshotCreated = true; + _snapshots.push({}); return 0; } - /** - * @return {boolean} whether the initial snapshot has been created this session - */ - function isInitialSnapshotCreated() { - return _initialSnapshotCreated; - } - /** * Finalize snapshot state when a response completes. * Builds an "after" snapshot from current document content for edited files, @@ -261,8 +245,8 @@ define(function (require, exports, module) { function finalizeResponse() { let afterIndex = -1; if (Object.keys(_pendingBeforeSnap).length > 0) { - // Build "after" snapshot = current _lastSnapshotAfter + current content of edited files - const afterSnap = Object.assign({}, _lastSnapshotAfter); + // Build "after" snapshot = last snapshot + current content of edited files + const afterSnap = Object.assign({}, _snapshots[_snapshots.length - 1]); Object.keys(_pendingBeforeSnap).forEach(function (fp) { const vfsPath = realToVfsPath(fp); const openDoc = DocumentManager.getOpenDocumentForPath(vfsPath); @@ -271,11 +255,9 @@ define(function (require, exports, module) { } }); _snapshots.push(afterSnap); - _lastSnapshotAfter = afterSnap; afterIndex = _snapshots.length - 1; } _pendingBeforeSnap = {}; - _undoApplied = false; return afterIndex; } @@ -294,20 +276,6 @@ define(function (require, exports, module) { }); } - /** - * @return {boolean} whether undo has been applied (latest summary clicked) - */ - function isUndoApplied() { - return _undoApplied; - } - - /** - * @param {boolean} val - */ - function setUndoApplied(val) { - _undoApplied = val; - } - /** * @return {number} number of snapshots */ @@ -321,10 +289,7 @@ define(function (require, exports, module) { function reset() { Object.keys(_contentStore).forEach(function (k) { delete _contentStore[k]; }); _snapshots = []; - _lastSnapshotAfter = {}; _pendingBeforeSnap = {}; - _initialSnapshotCreated = false; - _undoApplied = false; } exports.realToVfsPath = realToVfsPath; @@ -332,11 +297,8 @@ define(function (require, exports, module) { exports.storeContent = storeContent; exports.recordFileBeforeEdit = recordFileBeforeEdit; exports.createInitialSnapshot = createInitialSnapshot; - exports.isInitialSnapshotCreated = isInitialSnapshotCreated; exports.finalizeResponse = finalizeResponse; exports.restoreToSnapshot = restoreToSnapshot; - exports.isUndoApplied = isUndoApplied; - exports.setUndoApplied = setUndoApplied; exports.getSnapshotCount = getSnapshotCount; exports.reset = reset; }); diff --git a/src/core-ai/editApplyVerification.md b/src/core-ai/editApplyVerification.md index f8891953b..e30c5e3c2 100644 --- a/src/core-ai/editApplyVerification.md +++ b/src/core-ai/editApplyVerification.md @@ -22,11 +22,12 @@ _snapshots[2] = after R2 edits ## State Variables -- `_snapshots[]`: flat array of `{ filePath: hash|null }` snapshots -- `_lastSnapshotAfter`: cumulative state after last completed response -- `_pendingBeforeSnap`: per-file pre-edit tracking during current response -- `_initialSnapshotCreated`: whether snapshot 0 has been pushed -- `_undoApplied`: whether undo/restore has been clicked on any card +### AISnapshotStore (pure data layer) +- `_snapshots[]`: flat array of `{ filePath: hash|null }` snapshots. `getSnapshotCount() > 0` replaces the old `_initialSnapshotCreated` flag. +- `_pendingBeforeSnap`: per-file pre-edit tracking during current response (dedup guard for first-edit-per-file + file list for `finalizeResponse`) + +### AIChatPanel (UI state) +- `_undoApplied`: whether undo/restore has been clicked on any card (UI control for button labels) ## DOM Layout Example @@ -49,18 +50,18 @@ _snapshots[2] = after R2 edits ### AISnapshotStore - `recordFileBeforeEdit(filePath, previousContent, isNewFile)`: tracks pre-edit state, back-fills all existing snapshots -- `createInitialSnapshot()`: pushes snapshot 0 from `_lastSnapshotAfter`, returns index 0 -- `isInitialSnapshotCreated()`: returns whether snapshot 0 exists -- `finalizeResponse()`: builds after-snapshot from current doc content, pushes it, resets `_undoApplied`, returns index (or -1) +- `createInitialSnapshot()`: pushes empty `{}` as snapshot 0, returns index 0. Must be called *before* `recordFileBeforeEdit` so the back-fill populates it. +- `getSnapshotCount()`: returns `_snapshots.length` (replaces `isInitialSnapshotCreated()`) +- `finalizeResponse()`: builds after-snapshot from `_snapshots[last]` + current doc content, pushes it, returns index (or -1) - `restoreToSnapshot(index, callback)`: applies `_snapshots[index]` to files, calls `callback(errorCount)` -- `isUndoApplied()` / `setUndoApplied(val)`: getter/setter for undo state - `reset()`: clears all state for new session ### AIChatPanel - `_$msgs()`: live DOM query helper — returns `$(".ai-chat-messages")` to avoid stale cached `$messages` reference (see Implementation Notes) -- `_onToolEdit()`: on first edit per response, inserts initial PUC if not yet created. Diff toggle only (no per-edit undo). -- `_appendEditSummary()`: calls `finalizeResponse()`, creates summary card with "Undo" or "Restore to this point" button +- `_undoApplied`: local module state — reset to `false` in `_appendEditSummary()` (after `finalizeResponse()`) and `_newSession()`; set to `true` in `_onRestoreClick()` and `_onUndoClick()` +- `_onToolEdit()`: on first edit per response, creates initial snapshot (if none) *then* records pre-edit state. Inserts initial PUC. Diff toggle only (no per-edit undo). +- `_appendEditSummary()`: calls `finalizeResponse()`, resets `_undoApplied`, creates summary card with "Undo" or "Restore to this point" button - `_onUndoClick(afterIndex)`: sets `_undoApplied`, resets all buttons to "Restore to this point", restores to `afterIndex - 1`, highlights target element as "Restored", scrolls to it - `_onRestoreClick(snapshotIndex)`: sets `_undoApplied`, resets all buttons to "Restore to this point", restores to the given snapshot, marks clicked element as "Restored" - `_setStreaming(streaming)`: disables/enables all restore buttons during AI streaming