fix(undo): preserve partial /redo progress, bound _states, public redo_count bump, clamp /redo

[Kyzcreig]

Summary

Four follow-up fixes for the real issues Greptile flagged on merged PR #49
(feat: reversible half-turn /undo + /redo across CLI, gateway, TUI). All four
were valid; none were addressed before that PR merged.

1. redo() discarded earlier progress on a multi-op /redo (count/state desync)

In a /redo N (N≥2), the loop pops ops LIFO. If an earlier op restored rows
(real DB work — redo_stack grew) and a later op hit the transcript-rewrite
path (restore_ids → 0, e.g. after /compress or /retry), the old code
cleared the whole stack and returned reactivated_count: 0. Consequences:

• the CLI sees reactivated <= 0, prints "Nothing to redo", and skips its
  history reload → on-screen transcript desyncs from the DB, which did change;
• redo_count is never bumped even though an op committed.

Now: stop at the rewrite, keep and report the earlier ops' progress
(reactivated_count > 0 + an explanatory message), drop only the dead remainder,
and bump redo_count only when real work committed. A single-op partial restore
still fails loud (unchanged).

2. Reaching into the private SessionDB._execute_write

redo() bumped redo_count via db._execute_write(...). Added a public
SessionDB.bump_redo_count(session_id) helper and call that instead.

3. Unbounded _states module-global (gateway memory leak)

_states: Dict[str, UndoRedoState] was keyed by session_id and never evicted,
growing without limit in a long-running gateway. Replaced with an LRU
OrderedDict capped at _STATE_CAP = 2048. Eviction merely drops an in-memory
redo branch — identical to the existing "redo doesn't survive a restart" contract
that redo() already handles gracefully.

4. /redo had no non-positive-count guard

/undo clamps a non-positive count to 1; /redo 0 / /redo -1 fell through to a
misleading "nothing to redo". Added the same clamp for parity.

Tests

8 new cases + full undo/redo suite green:

tests/test_undo_redo_stack.py ............... (incl. partial-progress, LRU, helper-usage)
tests/cli/test_undo_redo_half_turn.py ...... (incl. /redo clamp parametrization)
+ gateway/tui undo+redo suites
49 passed

Behavior contracts, not snapshots: partial-progress preservation, _states LRU
eviction (recency-refresh on access), helper-not-private-access (AST/source check),
and clamp parity.

[github-actions]

🔎 Lint report: fix/undo-redo-49 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11214 on HEAD, 11212 on base (🆕 +2)

🆕 New issues (1):

Rule	Count
invalid-argument-type	1

First entries

tests/cli/test_undo_redo_half_turn.py:140: [invalid-argument-type] invalid-argument-type: Argument to function `HermesCLI.process_command` is incorrect: Expected `HermesCLI`, found `SimpleNamespace`

✅ Fixed issues: none

Unchanged: 5856 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

[Kyzcreig]

CI note (not a blocker): the test (5) shard is RED on this PR, but it is a
pre-existing, unrelated infra timeout — not caused by this change.

• The shard reports 4269 tests passed, 0 failed. The single red is
  tests/run_agent/test_run_agent.py hitting the 140.00s per-file timeout cap
  (a 6,619-line / 378-test file — the heaviest in the slice), reaching only ~10%
  before being killed: "1 file where no tests ran (timeout before collection)".
• This PR's diff does not touch tests/run_agent/ or run_agent.
• It reproduces identically on a second, independent PR off the same base
  (the sibling Greptile-fix branch) — same file, same 140.00s cap — confirming
  it's environmental shard-load/timeout, not either change.

All deterministic floor checks are green: sast, secret_scan, ruff enforcement, ruff + ty diff, e2e, check-attribution, osv-scanner,
supply-chain, and test shards 1–4 + 6.

[greptile-apps]

Greptile Summary

This PR addresses four follow-up fixes for the undo/redo feature merged in PR #49: preserving partial /redo progress when a later op hits a transcript-rewrite boundary, exposing a public SessionDB.bump_redo_count() helper to avoid reaching into the private _execute_write, bounding the module-global _states dict with an LRU OrderedDict capped at 2048 entries to prevent gateway memory growth, and clamping non-positive /redo N counts to 1 for parity with /undo.

• hermes_undo.redo() now tracks transcript_changed mid-loop, breaks early, clears only the remaining dead ops, and falls through to commit + report partial progress instead of discarding all earlier work and returning reactivated_count: 0.
• hermes_state.SessionDB gains bump_redo_count(session_id) as a clean public boundary; hermes_undo no longer bypasses encapsulation.
• _states is replaced with a bounded OrderedDict whose get_state() accessor refreshes recency on hit and evicts the LRU entry on insertion overflow.

Confidence Score: 4/5

Safe to merge; all four targeted bugs are correctly fixed and covered by tests. The only gap is that the partial-success warning message produced by hermes_undo.redo() is not surfaced to the user in redo_last.

The core logic change in hermes_undo.redo() is correct and well-tested. The LRU eviction, public helper, and count clamp are all clean. The one loose end is that cli.py's redo_last prints a success banner without forwarding the new partial-success warning message when result["message"] is set alongside a positive reactivated_count — users who hit this path won't know some ops were dropped.

cli.py — redo_last does not display result.get("message") on the success path

Important Files Changed

Filename	Overview
hermes_undo.py	Core redo logic correctly preserves partial-op progress on transcript-rewrite path, LRU OrderedDict cap is properly implemented, and private _execute_write access is replaced with the new public helper.
hermes_state.py	Adds public bump_redo_count() helper; clean delegation to _execute_write with no logic changes to the surrounding code.
cli.py	Adds non-positive count clamp for /redo (parity with /undo), but redo_last does not surface the new partial-success message that hermes_undo.redo() now returns.
tests/test_undo_redo_stack.py	New tests cover partial-progress preservation, LRU eviction with recency refresh, public-helper contract via AST/source inspection, all scenarios well-parameterised and cleanly isolated.
tests/cli/test_undo_redo_half_turn.py	Adds parametrised tests for /redo count clamping (0, -3) and non-numeric rejection, consistent with the /undo contract.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["/redo N called"] --> B["k = min(N, len(undo_stack))"]
    B --> C{k == 0?}
    C -->|Yes| D["Return: nothing to redo"]
    C -->|No| E["Loop k times: pop op from undo_stack"]
    E --> F["db.restore_ids(op.rewound_ids)"]
    F --> G{reactivated == 0\nAND op had ids?}
    G -->|Yes: transcript rewritten| H["transcript_changed = True\nbreak loop"]
    G -->|No| I{reactivated != len\nop.rewound_ids?}
    I -->|Yes: partial| J["raise RuntimeError\n(fail loud)"]
    I -->|No: full restore| K["reactivated_total += reactivated\nops_redone += 1\nredo_stack.append(op)"]
    K --> E
    H --> L{reactivated_total > 0?}
    L -->|No work done| M["undo_stack.clear()\nredo_stack.clear()\nReturn: reactivated=0 + message"]
    L -->|Earlier ops committed| N["undo_stack.clear()\nfall through"]
    N --> O["db.bump_redo_count(session_id)"]
    K --> O
    O --> P["Return: reactivated_total\n+ optional partial message"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["/redo N called"] --> B["k = min(N, len(undo_stack))"]
    B --> C{k == 0?}
    C -->|Yes| D["Return: nothing to redo"]
    C -->|No| E["Loop k times: pop op from undo_stack"]
    E --> F["db.restore_ids(op.rewound_ids)"]
    F --> G{reactivated == 0\nAND op had ids?}
    G -->|Yes: transcript rewritten| H["transcript_changed = True\nbreak loop"]
    G -->|No| I{reactivated != len\nop.rewound_ids?}
    I -->|Yes: partial| J["raise RuntimeError\n(fail loud)"]
    I -->|No: full restore| K["reactivated_total += reactivated\nops_redone += 1\nredo_stack.append(op)"]
    K --> E
    H --> L{reactivated_total > 0?}
    L -->|No work done| M["undo_stack.clear()\nredo_stack.clear()\nReturn: reactivated=0 + message"]
    L -->|Earlier ops committed| N["undo_stack.clear()\nfall through"]
    N --> O["db.bump_redo_count(session_id)"]
    K --> O
    O --> P["Return: reactivated_total\n+ optional partial message"]

Loading

Comments Outside Diff (1)

1. cli.py, line 6209-6210 (link)

   Partial-redo warning message is silently dropped

   hermes_undo.redo() now sets result["message"] on a partial-success path (e.g. "redid 1 operation(s); the rest can't be redone (transcript changed since undo)"), but redo_last only reads result.get('message') when reactivated <= 0. When reactivated > 0 the message is ignored entirely — a user who runs /redo 3 and gets only 1 op restored sees (^_^)b Redid 3 undo operation(s) (X message(s) restored). with no indication the other two ops were silently dropped. Consider printing result.get("message") after the success line when it is present.

_{Reviews (1): Last reviewed commit: "fix(undo): preserve partial /redo progre..." | Re-trigger Greptile}