diff --git a/event-binding.js b/event-binding.js index 397912a..50e4edd 100644 --- a/event-binding.js +++ b/event-binding.js @@ -263,10 +263,45 @@ export function onMessageEditedController(runtime, messageId, meta = null) { runtime.refreshPersistedRecallMessageUi?.(); } -export function onMessageSwipedController(runtime, messageId, meta = null) { +export async function onMessageSwipedController(runtime, messageId, meta = null) { runtime.invalidateRecallAfterHistoryMutation("已切换楼层 swipe"); - runtime.scheduleHistoryMutationRecheck("message-swiped", messageId, meta); + const parsedFloor = Number(messageId); + const fromFloor = Number.isFinite(parsedFloor) ? parsedFloor : undefined; + let result = { + success: false, + rollbackPerformed: false, + extractionTriggered: false, + requestedFloor: fromFloor ?? null, + effectiveFromFloor: null, + recoveryPath: "reroll-handler-unavailable", + affectedBatchCount: 0, + error: "swipe reroll handler unavailable", + }; + + if (typeof runtime.onReroll === "function") { + try { + result = await runtime.onReroll({ fromFloor, meta }); + } catch (error) { + runtime.console?.error?.("[ST-BME] swipe reroll failed:", error); + result = { + success: false, + rollbackPerformed: false, + extractionTriggered: false, + requestedFloor: fromFloor ?? null, + effectiveFromFloor: null, + recoveryPath: "reroll-threw", + affectedBatchCount: 0, + error: error?.message || String(error) || "swipe reroll failed", + }; + } + } else { + runtime.console?.warn?.( + "[ST-BME] MESSAGE_SWIPED missing onReroll; skip generic history recovery fallback.", + { messageId, meta }, + ); + } runtime.refreshPersistedRecallMessageUi?.(); + return result; } export async function onGenerationAfterCommandsController( diff --git a/index.js b/index.js index 34727f9..1077dce 100644 --- a/index.js +++ b/index.js @@ -7474,6 +7474,19 @@ async function rollbackGraphForReroll(targetFloor, context = getContext()) { resultCode: "reroll.rollback.applied", }), ); + if ( + Array.isArray(context?.chat) && + Number.isFinite(currentGraph.historyState?.lastProcessedAssistantFloor) && + currentGraph.historyState.lastProcessedAssistantFloor >= 0 + ) { + // Preserve the rolled-back prefix immediately so a failed follow-up + // re-extraction does not look like a generic "missing processed hashes" + // corruption on the next history integrity check. + updateProcessedHistorySnapshot( + context.chat, + currentGraph.historyState.lastProcessedAssistantFloor, + ); + } pruneProcessedMessageHashesFromFloor(currentGraph, effectiveFromFloor); currentGraph.lastProcessedSeq = currentGraph.historyState?.lastProcessedAssistantFloor ?? -1; @@ -7623,6 +7636,16 @@ async function recoverHistoryIfNeeded(trigger = "history-recovery") { trigger, }), ); + const recoveredLastProcessedFloor = Number.isFinite( + currentGraph?.historyState?.lastProcessedAssistantFloor, + ) + ? currentGraph.historyState.lastProcessedAssistantFloor + : -1; + if (recoveredLastProcessedFloor >= 0) { + // Recovery replay has rebuilt the graph state; restore processed hashes so + // the next hash recheck does not immediately trigger another replay loop. + updateProcessedHistorySnapshot(chat, recoveredLastProcessedFloor); + } saveGraphToChat({ reason: "history-recovery-complete" }); refreshPanelLiveState(); updateStageNotice( @@ -8180,10 +8203,11 @@ function onMessageEdited(messageId, meta = null) { return result; } -function onMessageSwiped(messageId, meta = null) { - const result = onMessageSwipedController( +async function onMessageSwiped(messageId, meta = null) { + const result = await onMessageSwipedController( { invalidateRecallAfterHistoryMutation, + onReroll, refreshPersistedRecallMessageUi: schedulePersistedRecallMessageUiRefresh, scheduleHistoryMutationRecheck, }, diff --git a/reroll-history-review-notes.md b/reroll-history-review-notes.md new file mode 100644 index 0000000..82ce6bb --- /dev/null +++ b/reroll-history-review-notes.md @@ -0,0 +1,228 @@ +# ST-BME Reroll / History Recovery Review Notes + +## 1. Background and user pain points + +This note summarizes the issues found during a real debugging session, the user expectations behind the fixes, and the exact code changes that were made. + +Primary user pain points: + +- A new chat sometimes showed recall activity, but extraction state stayed at `processed floor = -1`, which made it look like nothing was recorded. +- Manual extraction worked, but automatic behavior felt inconsistent. +- The worst issue: after each new floor, ST-BME could enter history recovery, roll back, and replay extraction again, making every turn slow. +- The user explicitly does not want reroll/swipe to degrade into rebuilding everything before the changed floor. + +The user's strongest expectations: + +- Normal extraction should stay incremental. +- Hidden old floors do not need to be re-read just for extraction context. +- If floor 16 is rerolled/swiped, only memories related to the affected suffix should be updated. +- Floors before that prefix should not be replayed again. +- `swipe/reroll` should not silently fall back to generic full history recovery. + +## 2. Important product/logic expectations from the user + +These were treated as design constraints during the fix: + +- Do not change the "hide old floors" strategy just to feed more extraction context. +- Do not make reroll depend on re-reading all previous floors. +- Do not allow reroll/swipe to degrade into full rebuild. +- If targeted reroll rollback cannot be done safely, fail closed and report failure instead of rebuilding the whole prefix. + +## 3. What the code already intended to do + +The repo already had two different recovery concepts. + +### A. Generic history mutation recovery + +Used for broad history mutations such as edit, delete, or hash mismatch. + +Relevant code: + +- `inspectHistoryMutation(...)` in `index.js` +- `recoverHistoryIfNeeded(...)` in `index.js` +- `detectHistoryMutation(...)` in `runtime-state.js` + +This path can do larger recovery or replay work because it is trying to revalidate processed history integrity. + +### B. Dedicated reroll rollback + +Used for targeted suffix rollback around a reroll/swipe boundary. + +Relevant code: + +- `rollbackGraphForReroll(...)` in `index.js` +- `onRerollController(...)` in `extraction-controller.js` + +This path is closer to the desired product behavior: + +- find a recovery point near the target floor +- rollback only affected journals and suffix state +- prune processed hashes from the affected floor onward +- re-extract only the affected suffix + +## 4. Issue 1 that was fixed: recovery loop caused by lost processed hashes + +### Symptom + +The panel showed a pattern like: + +- history recovery starts +- replay completes +- next turn immediately triggers another recovery +- reason mentions missing `processedMessageHashes` + +### Root cause + +After successful history recovery, dirty state was cleared, but processed message hashes were not restored. On the next integrity recheck, the system saw: + +- there is already processed progress +- but `processedMessageHashes` is empty or missing + +That was interpreted as another dirty history condition, so recovery started again. + +### Fix + +After a successful recovery replay, restore processed hashes from current chat state before saving the graph. + +Relevant change: + +- `index.js`, `recoverHistoryIfNeeded(...)` +- added call: `updateProcessedHistorySnapshot(chat, recoveredLastProcessedFloor)` + +Why this matters: + +- recovery replay now leaves history state internally consistent +- the next hash recheck does not immediately trigger another replay loop + +### Regression test added + +- `tests/p0-regressions.mjs` +- `testHistoryRecoverySuccessRestoresProcessedHashesAfterReplay()` + +## 5. Issue 2 that was fixed: host swipe event was routed into generic history recovery + +### Symptom + +Even though the repo already had dedicated reroll rollback code, the actual host `MESSAGE_SWIPED` event still went through generic history mutation recheck first. + +That meant a user swipe could enter the broader history recovery pipeline instead of the suffix-only reroll path. + +### Why this was wrong + +This conflicts with the desired behavior: + +- `swipe/reroll` is a targeted suffix change +- it should not be treated like a generic broad history mutation +- it should not have a path that silently escalates into full replay of earlier floors + +### Fix + +Changed event routing so that `MESSAGE_SWIPED` directly calls `onReroll(...)` instead of scheduling generic history mutation recheck. + +Relevant changes: + +- `event-binding.js` + - `onMessageSwipedController(...)` is now async + - it calls `runtime.onReroll({ fromFloor, meta })` + - it no longer schedules `scheduleHistoryMutationRecheck("message-swiped", ...)` +- `index.js` + - `onMessageSwiped(...)` is now async + - runtime wiring now passes `onReroll` + +### Resulting behavior + +For swipe/reroll: + +- route to dedicated suffix rollback +- if rollback succeeds, only affected suffix is re-extracted +- if rollback cannot be done safely, fail as reroll rollback failure +- do not silently drop into generic history recovery fallback + +### Regression test added + +- `tests/p0-regressions.mjs` +- `testSwipeRoutesToRerollWithoutHistoryRecoveryFallback()` + +This test specifically asserts: + +- `onReroll` is called +- `scheduleHistoryMutationRecheck` is not called + +## 6. Things intentionally not changed + +These were discussed and intentionally left alone: + +- The "hide old floors" behavior was not redesigned. +- No attempt was made to force extraction to re-read very old floors. +- No attempt was made to make reroll reconstruct the entire prior conversation. + +This matches the user's explicit preference: + +- previous floors should already be recorded +- reroll should only repair the affected suffix +- old hidden floors should not become the reason for broader replay behavior + +## 7. Current intended invariants after the fixes + +These are the important invariants another reviewer should validate: + +1. Normal extraction remains incremental. +2. Generic edit/delete/hash corruption can still use broader recovery if needed. +3. `swipe/reroll` must use dedicated suffix rollback logic. +4. `swipe/reroll` must not silently degrade into generic full history recovery. +5. Successful recovery replay must leave `processedMessageHashes` populated consistently with processed floor state. + +## 8. Files directly involved + +- `index.js` +- `event-binding.js` +- `extraction-controller.js` +- `runtime-state.js` +- `chat-history.js` +- `tests/p0-regressions.mjs` + +## 9. Concrete code locations worth reviewing + +Suggested review targets: + +- `index.js` + - `recoverHistoryIfNeeded(...)` + - `rollbackGraphForReroll(...)` + - `onMessageSwiped(...)` +- `event-binding.js` + - `onMessageSwipedController(...)` +- `extraction-controller.js` + - `onRerollController(...)` +- `runtime-state.js` + - `detectHistoryMutation(...)` + - `clearHistoryDirty(...)` +- `chat-history.js` + - processed hash pruning and extraction window helpers + +## 10. What was verified locally + +Executed successfully: + +- `node --check event-binding.js` +- `node --check index.js` +- `node --check tests/p0-regressions.mjs` +- `node tests/p0-regressions.mjs` + +## 11. What I want the reviewing AI to focus on + +Please review for these questions: + +- Is there any remaining code path where `MESSAGE_SWIPED` can still end up in `recoverHistoryIfNeeded(...)` instead of dedicated reroll rollback? +- Is there any remaining reroll path that can still escalate into prefix/full rebuild instead of suffix-only repair? +- After reroll rollback, are `historyState`, `processedMessageHashes`, and vector repair state all kept mutually consistent? +- Are there any race conditions between swipe-triggered reroll, auto extraction, and delayed hide application? +- Does the current failure mode truly fail closed, or is there still an implicit generic fallback somewhere else? + +## 12. Short conclusion + +The key product decision behind these fixes is: + +- generic history corruption and targeted reroll are not the same thing +- reroll/swipe should be handled as suffix repair, not broad recovery + +The current changes aim to enforce exactly that distinction. diff --git a/tests/p0-regressions.mjs b/tests/p0-regressions.mjs index 8df323a..b713efb 100644 --- a/tests/p0-regressions.mjs +++ b/tests/p0-regressions.mjs @@ -10,6 +10,7 @@ import { onChatChangedController, onGenerationAfterCommandsController, onGenerationStartedController, + onMessageSwipedController, registerCoreEventHooksController, } from "../event-binding.js"; import { @@ -204,10 +205,13 @@ function createBatchStageHarness() { const marker = "function notifyHistoryDirty(dirtyFrom, reason) {"; const start = source.indexOf("function shouldAdvanceProcessedHistory("); const end = source.indexOf(marker); - if (start < 0 || end < 0 || end <= start) { + const resolvedEnd = end >= 0 ? end : endFallback; + if (start < 0 || resolvedEnd < 0 || resolvedEnd <= start) { throw new Error("无法从 index.js 提取批次状态机定义"); } - const snippet = source.slice(start, end).replace(/^export\s+/gm, ""); + const snippet = source + .slice(start, resolvedEnd) + .replace(/^export\s+/gm, ""); const context = { console, result: null, @@ -256,10 +260,13 @@ function createGenerationRecallHarness(options = {}) { const end = source.indexOf( 'function onMessageReceived(messageId = null, type = "") {', ); - if (start < 0 || end < 0 || end <= start) { + const resolvedEnd = end >= 0 ? end : endFallback; + if (start < 0 || resolvedEnd < 0 || resolvedEnd <= start) { throw new Error("无法从 index.js 提取生成召回事务定义"); } - const snippet = source.slice(start, end).replace(/^export\s+/gm, ""); + const snippet = source + .slice(start, resolvedEnd) + .replace(/^export\s+/gm, ""); const context = { console, Date, @@ -484,11 +491,15 @@ function createGenerationRecallHarness(options = {}) { function createHistoryRecoveryHarness() { return fs.readFile(indexPath, "utf8").then((source) => { const start = source.indexOf("async function recoverHistoryIfNeeded("); + const endFallback = source.indexOf("async function runExtraction()"); const end = source.indexOf("/**\n * 提取管线:处理未提取的对话楼层"); - if (start < 0 || end < 0 || end <= start) { + const resolvedEnd = end >= 0 ? end : endFallback; + if (start < 0 || resolvedEnd < 0 || resolvedEnd <= start) { throw new Error("无法从 index.js 提取 history recovery 定义"); } - const snippet = source.slice(start, end).replace(/^export\s+/gm, ""); + const snippet = source + .slice(start, resolvedEnd) + .replace(/^export\s+/gm, ""); const context = { console, Date, @@ -623,6 +634,19 @@ function createHistoryRecoveryHarness() { } return 0; }, + updateProcessedHistorySnapshot(chat, lastProcessedAssistantFloor) { + context.updatedProcessedHistorySnapshot = { + chatLength: Array.isArray(chat) ? chat.length : 0, + lastProcessedAssistantFloor, + }; + context.currentGraph.historyState ||= {}; + context.currentGraph.historyState.lastProcessedAssistantFloor = + lastProcessedAssistantFloor; + context.currentGraph.historyState.processedMessageHashes = + lastProcessedAssistantFloor >= 0 + ? { [lastProcessedAssistantFloor]: `hash-${lastProcessedAssistantFloor}` } + : {}; + }, clearHistoryDirty(graph, result) { context.clearedHistoryDirty = result; graph.historyState ||= {}; @@ -769,6 +793,20 @@ function createRerollHarness() { async deleteBackendVectorHashesForRecovery(...args) { context.deletedHashesCalls.push(args); }, + updateProcessedHistorySnapshot(chat, lastProcessedAssistantFloor) { + context.updatedProcessedHistorySnapshot = { + chatLength: Array.isArray(chat) ? chat.length : 0, + lastProcessedAssistantFloor, + }; + context.currentGraph.historyState ||= {}; + context.currentGraph.historyState.lastProcessedAssistantFloor = + lastProcessedAssistantFloor; + context.currentGraph.historyState.processedMessageHashes = + lastProcessedAssistantFloor >= 0 + ? { [lastProcessedAssistantFloor]: `hash-${lastProcessedAssistantFloor}` } + : {}; + context.currentGraph.lastProcessedSeq = lastProcessedAssistantFloor; + }, pruneProcessedMessageHashesFromFloor(graph, fromFloor) { return pruneProcessedMessageHashesFromFloor(graph, fromFloor); }, @@ -2861,6 +2899,53 @@ async function testChatChangedDoesNotClearCoreEventBindings() { assert.equal(clearPendingAutoExtractionCalls, 1); } +async function testSwipeRoutesToRerollWithoutHistoryRecoveryFallback() { + const invalidationReasons = []; + const rerollCalls = []; + let historyRecheckCalls = 0; + let refreshCalls = 0; + + const result = await onMessageSwipedController( + { + invalidateRecallAfterHistoryMutation(reason) { + invalidationReasons.push(reason); + }, + async onReroll(payload) { + rerollCalls.push(payload); + return { + success: true, + rollbackPerformed: true, + extractionTriggered: true, + requestedFloor: payload.fromFloor, + effectiveFromFloor: payload.fromFloor, + recoveryPath: "reverse-journal", + affectedBatchCount: 1, + error: "", + }; + }, + scheduleHistoryMutationRecheck() { + historyRecheckCalls += 1; + }, + refreshPersistedRecallMessageUi() { + refreshCalls += 1; + }, + console: { + warn() {}, + error() {}, + }, + }, + 16, + { reason: "host-swipe" }, + ); + + assert.equal(invalidationReasons.length, 1); + assert.deepEqual(rerollCalls, [{ fromFloor: 16, meta: { reason: "host-swipe" } }]); + assert.equal(historyRecheckCalls, 0); + assert.equal(refreshCalls, 1); + assert.equal(result.success, true); + assert.equal(result.recoveryPath, "reverse-journal"); +} + async function testAutoExtractionDefersWhenGraphNotReady() { const deferredReasons = []; const statuses = []; @@ -3661,6 +3746,57 @@ async function testHistoryRecoveryFallbackFullRebuildCarriesResultCode() { ); } +async function testHistoryRecoverySuccessRestoresProcessedHashesAfterReplay() { + const harness = await createHistoryRecoveryHarness(); + harness.chat = [ + { is_user: true, mes: "u1" }, + { is_user: false, mes: "a1" }, + ]; + harness.currentGraph = { + historyState: { + lastProcessedAssistantFloor: 1, + processedMessageHashes: { 1: "old-hash-1" }, + historyDirtyFrom: 1, + lastMutationSource: "message-edited", + }, + vectorIndexState: { + collectionId: "col-1", + dirty: false, + dirtyReason: "", + pendingRepairFromFloor: null, + replayRequiredNodeIds: [], + lastWarning: "", + lastIntegrityIssue: null, + }, + batchJournal: [], + lastProcessedSeq: 1, + }; + harness.findJournalRecoveryPointImpl = () => ({ + path: "full-rebuild", + affectedBatchCount: 0, + }); + harness.replayExtractionFromHistoryImpl = async () => { + harness.currentGraph.historyState.lastProcessedAssistantFloor = 1; + harness.currentGraph.lastProcessedSeq = 1; + return 1; + }; + + const result = await harness.result.recoverFromHistoryMutation({ + trigger: "message-edited", + dirtyFrom: 1, + detection: { source: "manual-test", reason: "edited" }, + }); + + assert.equal(result, true); + assert.deepEqual(harness.updatedProcessedHistorySnapshot, { + chatLength: 2, + lastProcessedAssistantFloor: 1, + }); + assert.deepEqual(harness.currentGraph.historyState.processedMessageHashes, { + 1: "hash-1", + }); +} + async function testHistoryRecoveryFailureCarriesResultCode() { const harness = await createHistoryRecoveryHarness(); harness.chat = [ @@ -3782,6 +3918,72 @@ async function testRerollFallsBackToDirectExtractForUnprocessedFloor() { assert.equal(harness.saveGraphToChatCalls, 0); } +async function testRerollPreservesPrefixHashesWhenReextractDoesNotAdvance() { + const harness = await createRerollHarness(); + harness.chat = [ + { is_user: true, mes: "u1" }, + { is_user: false, mes: "a1" }, + { is_user: true, mes: "u2" }, + { is_user: false, mes: "a2" }, + { is_user: true, mes: "u3" }, + { is_user: false, mes: "a3" }, + ]; + harness.currentGraph = { + historyState: { + lastProcessedAssistantFloor: 5, + processedMessageHashes: { + 1: "hash-1", + 3: "hash-3", + 5: "hash-5", + }, + }, + vectorIndexState: { + collectionId: "col-1", + }, + batchJournal: [{ id: "journal-1" }], + lastProcessedSeq: 5, + }; + harness.postRollbackGraph = { + historyState: { + lastProcessedAssistantFloor: 1, + processedMessageHashes: { + 1: "old-hash-1", + 3: "stale-hash", + }, + }, + vectorIndexState: { + collectionId: "col-1", + }, + batchJournal: [], + lastProcessedSeq: 1, + }; + harness.findJournalRecoveryPointImpl = () => ({ + path: "reverse-journal", + affectedBatchCount: 1, + affectedJournals: [{ id: "journal-1" }], + }); + harness.buildReverseJournalRecoveryPlanImpl = () => ({ + backendDeleteHashes: [], + replayRequiredNodeIds: [], + pendingRepairFromFloor: 2, + legacyGapFallback: false, + dirtyReason: "history-recovery-replay", + }); + harness.manualExtractLevel = "error"; + + const result = await harness.result.onReroll({ fromFloor: 3 }); + + assert.equal(result.success, true); + assert.equal(result.extractionStatus, "error"); + assert.deepEqual(harness.updatedProcessedHistorySnapshot, { + chatLength: 6, + lastProcessedAssistantFloor: 1, + }); + assert.deepEqual(harness.currentGraph.historyState.processedMessageHashes, { + 1: "hash-1", + }); +} + async function testLlmDebugSnapshotRedactsSecretsBeforeStorage() { const originalFetch = globalThis.fetch; const previousSettings = JSON.parse( @@ -4015,6 +4217,7 @@ await testGenerationRecallSkippedStateDoesNotLoopToBeforeCombine(); await testGenerationRecallSentMessageClearsStaleTransactionForSameKey(); await testRegisterCoreEventHooksIsIdempotent(); await testChatChangedDoesNotClearCoreEventBindings(); +await testSwipeRoutesToRerollWithoutHistoryRecoveryFallback(); await testAutoExtractionDefersWhenGraphNotReady(); await testAutoExtractionDefersWhenAlreadyExtracting(); await testAutoExtractionDefersWhenHistoryRecoveryBusy(); @@ -4034,9 +4237,11 @@ await testRecallSubGraphAndDataLayerEntryPoints(); await testRerollUsesBatchBoundaryRollbackAndPersistsState(); await testHistoryRecoveryAbortClearsVectorRepairState(); await testHistoryRecoveryFallbackFullRebuildCarriesResultCode(); +await testHistoryRecoverySuccessRestoresProcessedHashesAfterReplay(); await testHistoryRecoveryFailureCarriesResultCode(); await testRerollRejectsMissingRecoveryPoint(); await testRerollFallsBackToDirectExtractForUnprocessedFloor(); +await testRerollPreservesPrefixHashesWhenReextractDoesNotAdvance(); await testLlmDebugSnapshotRedactsSecretsBeforeStorage(); await testEmbeddingUsesConfigTimeoutInsteadOfDefault(); await testLlmOutputRegexCleansResponseBeforeJsonParse();