ST-Bionic-Memory-Ecology/reroll-history-review-notes.md

# 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.