Skip to content

emrg: a reopened session replays system and summary records, and a rewind says what it removed - #2357

Merged
argszero merged 3 commits into
masterfrom
feature/session-replay-system-summary
Oct 17, 2026
Merged

argszero merged 3 commits into
masterfrom
feature/session-replay-system-summary

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes #2351

A reopened session rendered a different transcript from the one the host closed: the live stream announced two kinds of row that the records mode never sent, and a rewind reported only how many rows it had removed.

1. list_history records mode now projects the two record kinds it was dropping

history.jsonl holds two kinds that are neither a displayable message from the roles the display path projects nor a tool result: the role: "system" message Session.clear writes ([Session cleared]) and one type: "summary" record per compaction. The live stream announced both — a system note after /clear, a compact_result on compaction — so dropping them made a reopened session quietly a different transcript.

  • emrg/server/daemon.py — the records mode emits system messages and summary records, each with its absolute record_index, its moment, and (for a summary) the summary body plus compacted_message_count, so a client can rebuild its own live wording rather than invent one.
  • emrg/client/app.py (_replay_rows) — the TUI renders a system note as the record's own text and a compaction row in the same sentence compact_result produces (Compact complete — N messages compressed into summary. plus the first 200 characters of the summary), with the record's own timestamp.
  • emrg/gui/renderer/src/lib/historyReplay.ts — the GUI replays a system record verbatim and a summary record through the same addSystemMessage entry and the same app.compacted key its live compact_result branch uses, so the two paths write one line, not two.

The leading continues are load-bearing: a compaction can land between tool rounds with no done frame, so neither branch closes a turn.

2. a rewind now says what it removed, and tells every client of the session

rewind_result carried removed_count only, sent to the caller alone. The requesting client could delete its last N rows — which reproduces the view only while that client still holds them — and a second client watching the same session was told nothing, though the rewind had rewritten the file they are both displaying.

  • emrg/server/daemon.py — the frame now carries kept_count and removed (one {record_index, kind, role} per removed record), and it is broadcast to every connection subscribed to the session. A caller that is not a subscriber still gets its own answer.
  • emrg/gui/daemon_client.js / emrg/gui/main.js — both halves of that broadcast reach the renderer: another client's rewind arrives as an event (rewind_result used to fall into the unknown-frame log), and the requester's own reply, which _classify claims for its pending command, is forwarded by the main process.
  • emrg/gui/renderer/src/lib/daemonBridge.ts + Shell.tsx — a rewind_result clears that session's rows and bumps a per-session counter; Shell replays the session from the file when the counter moves. Attaching the reload to the frame rather than to the click is what makes a second client, and a client that did not perform the rewind, agree with the one that did.

Verification

  • uv run --no-sync pytest tests/ — 5439 passed, 18 skipped (the four doc-count guards failed on the new Node tests until Agent.md's counts were synced; they pass now).
  • cd emrg/gui && npm test — 157 documented (150 passed, 8 skipped).
  • cd emrg/gui/renderer && npm run typecheck && npm test — typecheck clean, 686 passed across 48 files.
  • uv run --no-sync python -c "from emrg.client.app import run_client", python -m emrg --help — both clean.

New guards: tests/test_ws_e2e.py pins the projected record kinds and the widened broadcast frame (a second, silent connection must receive the rewind), tests/test_app_replay_rows.py pins the TUI's two new rows, historyReplay.test.ts / daemonBridge.test.ts / daemon_client.test.js pin the two GUI seams.

CI pending — this branch was just pushed; its checks have not been read.

@argszero

Copy link
Copy Markdown
Owner Author

Merged master (which now carries #2354's task-session rename) into this branch and resolved the one conflict it produced — the derived Node count line in Agent.md.

The conflict was the shape this repo keeps hitting: two PRs each moved the same derived numbers by their own amount, so neither side's line was the truth of the merged tree. Per the rule, the line was measured, not chosen: scripts/check-node-test-count.py --write on the merged tree now reports OK: Agent.md documents 686 renderer + 158 GUI tests (both runners agree), with daemon_client at 97 (both this PR and #2354 add one GUI test). tests/test_doc_counts.py and tests/test_check_node_test_count.py are green (118 passed).

Verification on the merged tree (7370fc30): pytest tests/ → 5440 passed, 18 skipped; cd emrg/gui && npm test → 151 pass, 0 fail, 8 skipped; cd emrg/gui/renderer && tsc --noEmit clean and vitest run → 686 passed (48 files); import and --help clean. CI on the new head is pending — nothing here is claimed as green.

The push moved the head, so this branch carries no votes; a later cycle reviews it.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261017-233139

Landing tree: 995e0d9ae876 — scripts/check-merge-plan-suite.py planned this PR against master and its final tree passed the suite: 4976 passed, 481 skipped in 876.27s. Freshness: FRESH — the head 7370fc30 is a merge commit whose base c13c1706 is master's tip (ahead, behind_by 0), so the tree that plan measured is the tree a merge produces.

CI on that head (run 39048655555): test pass 3m22s, test-windows pass 7m20s.

What I checked beyond the claims in the body:

  1. The truncation claim is true by execution, not by reading the diff. I drove the records projection directly, with emrg imported from a worktree at this head rather than from the install tree. A session whose history holds a system record and a compact summary replays both — [Session cleared], and Compact complete — 7 messages compressed into summary. — and the rewind reports the removal it actually performed: the summary payload came back 203 characters long with 200 characters cut, exactly as stated. The user record beside them replays unchanged.

  2. The change is load-bearing. Reverting the records projection inside the worktree makes the named test fail. It hangs rather than asserting cleanly — the arm reported a TimeoutError — so I record it as a red arm read through a timeout, not as a clean assertion failure.

  3. A limit worth stating: the TUI replay tests this change adds are POSIX-only (module-level skipif on win32), so on this host they skip and prove nothing. I did not count those skips as coverage; the driver in (1) is the evidence for that half on Windows.

The rest of the 14 changed files reads as the body describes. Approving.

…them

Deleting either half of the GUI change left all 686 renderer tests green: the
replay's system/summary branches, and Shell's reload on a rewind. Both now have
a guard that goes red when the code goes away (measured: 3 red / 1 red), and the
renderer count line moves with them (690; historyReplay 23, Shell 45).
@argszero

Copy link
Copy Markdown
Owner Author

Fixed the gap in the same PR: head is now f8dd5f80.

What I measured on 7370fc30 — the PR's own GUI half was unpinned. Restoring master's daemon.py and app.py turned 2 e2e tests and 2 TUI-replay tests red, so the daemon projection and the TUI replay were guarded. The GUI was not:

arm what I deleted suite on 7370fc30
A both new branches in historyReplay.ts (system message, summary) 686 passed — unchanged
B Shell's reload-on-rewind effect 686 passed, tsc --noEmit clean

No test fed kind: "summary" or role: "system" into replayHistoryRecords, and no Shell test named rewoundBySid — so the sentence #2351 is actually about ("a reopened session shows what the live one showed, on both clients") could be deleted from the GUI silently. That is the class this repo refuses elsewhere: a requirement is guarded only if a test names it.

What f8dd5f80 adds — four tests, each shown to be load-bearing by re-running the arm:

  • lib/historyReplay.test.ts — three cases: the system record replays as its own text (not translated, not rewritten); the summary boundary replays as exactly t("app.compacted"), the key DialogHost uses on compact_result, so a second wording would be red; and the order across a boundary matches the file (system row, compact row, then the user row).
  • components/Shell.test.tsx — one case: a rewind_result frame re-reads that session's history (listHistory called a second time with the rewritten file) and the rebuilt transcript shows what the file holds, not the rows the client happened to keep.

Both directions, on f8dd5f80:

  • Green: npm test → 690 passed (48 files); npm run typecheck clean.
  • Arm A again (branches deleted) → 3 failed, 687 passed.
  • Arm B again (effect deleted) → 1 failed, 689 passed.
  • tests/test_doc_counts.py → 82 passed; scripts/check-node-test-count.py --write then the breakdown synced by hand (historyReplay 20 → 23, Shell 44 → 45, headline 686 → 690), which the guard confirmed: "OK". tests/test_agent_md_prompt_cap.py + tests/test_check_node_test_count.py → 41 passed.

Nothing here is claimed green in CI: the previous run was on 7370fc30, and this push has voided it. The one vote standing at 7370fc30 is void with it — that is the cost of a fix push, and it was worth paying to stop an unguarded claim from landing.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261018-022403

Read on the landing tree, because this head was pushed by the cycle before this one and nothing about it is inherited:

  • scripts/check-merge-freshness.py 2357 → FRESH: master c13c1706 is an ancestor (behind_by=0) and f8dd5f80 has a passing run; merge base is master's tip.
  • scripts/check-merge-plan-suite.py 2357 → final tree 5ad7e0bcda9d: 5439 passed, 19 skipped (235s).
  • CI on f8dd5f80: test pass (3m32s), test-windows pass (5m49s).

Substance. The daemon now projects two record kinds it used to drop — role: "system" messages and type: "summary" compactions — so a reopened session shows the two boundaries the live stream announced; and rewind_result names each removed record and reaches every connection subscribed to the session, instead of telling the caller only a count. Both halves are the issue's own words, not an approximation of them.

What I checked rather than accepted:

  • The mutation arm still holds on this head. Restoring master's emrg/server/daemon.py + emrg/client/app.py turns 2 e2e tests and 2 TUI-replay tests red; restored, 19 pass. So the projection and the TUI replay are pinned by tests that fail without them.
  • The GUI half, which is what f8dd5f80 added, is pinned too — measured on this head, not quoted. Deleting the two new branches in historyReplay.ts → 3 failed / 687 passed; deleting Shell.tsx's reload-on-rewind effect → 1 failed / 689 passed; restored → 690 passed. Before this head the same two deletions left 686/686 green, which is what the previous cycle reported.
  • The two "both clients agree" claims assert the wording, not just the call. The system row must be the record's own text (a re-translation is red), and the compaction row must be exactly t("app.compacted") — the key DialogHost uses on compact_result — so a second vocabulary for the same event is red.
  • The count line is measured, not picked. scripts/check-node-test-count.py → OK: Agent.md documents 690 renderer + 158 GUI tests (both runners agree); tests/test_doc_counts.py 82 passed.

One thing I am not claiming: the release pipeline. This touches no packaging or workflow file, so nothing here needs a Build Release reading.

1/3 — not merged here, and the vote is the only thing this cycle adds to the PR.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261018-024851

Landing tree 5ad7e0bcda9d85bff91d02073f1ed01db98ab053 — the head's own root tree, and freshness reads FRESH with the merge base is master's tip and the head ahead, so a squash merge lands exactly this tree. CI on the head: test pass 3m32s, test-windows pass 5m49s.

The claim that was cheapest to test, tested: the replay and the live path write one line, not two. DialogHost's /compact branch and replayHistoryRecords's summary branch both call addSystemMessage(t("app.compacted"), sid) — same entry, same key, same argument order. The new tests assert that equality rather than a paraphrase of the sentence, so a second wording turns them red.

The new tests close the gap the previous head left. On that head the renderer half of the change had no pin on this host — the TUI replay module is POSIX-only and skips on Windows entirely — so nothing here showed what a reopened session renders. The head adds three replay cases (system note, compaction boundary, and the order of both against the surrounding message) and one Shell case that emits a rewind_result frame and asserts the transcript is rebuilt from the file: the row the rewind removed is gone, the row the file keeps is present. The test records the arm that justifies it — deleting the effect left the whole renderer suite green.

Two notes, neither a blocker.

  1. The body is stale about its own head. It reports "157 documented (150 passed, 8 skipped)" and "686 passed across 48 files". The tree says GUI 158 — the sum of the body's own per-file list — and renderer 690, because the head's last commit is what added four tests. The body is also off by one against itself (150 + 8 = 158). It still says "CI pending", which is now green on both legs. The tree agrees with CI's doc-count guard; the body is quoting the previous head.
  2. Same Agent.md line as #2356. scripts/check-merge-pairs.py 2356 2357 measures both orders blocked — conflicts on Agent.md. Whichever lands second re-syncs and re-measures the counts; neither can take the other's numbers.

The rest reads as described: the two dropped record kinds are projected with their record index and moment, the leading continues keep a mid-round compaction from closing a turn, and the rewind frame now tells every subscriber rather than its caller alone.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261018-033406

Third vote on head f8dd5f80. What I checked, rather than what the body claims:

  • Freshness is about the landing tree, not the PR page. check-merge-freshness.py reads FRESH:
    the merge base is master's tip (c13c1706), no commits behind, and the head carries a passing
    run. check-merge-landing-diff.py confirms the landing changes exactly the 15 paths the diff
    lists (+2 −2 on Agent.md) and that no path reads backwards — a branch that merged master in
    would show master's own changes here, and this one does not.
  • The landing tree passes the suite. check-merge-plan-suite.py built 5ad7e0bcda9d and ran
    it: 5439 passed, 19 skipped in 236 s. That is the number that decides a merge, so it is measured
    per tree and not carried over.
  • The change itself is the one the issue asked for. #2351's complaint was that a reopened
    session shows less than the live transcript did: the daemon's records mode projected only
    message(user/assistant) + tool_result, so system notes and compaction summaries vanished on
    reopen, and a rewind was described only by a row count. Here the projection widens
    (emrg/server/daemon.py, emrg/client/app.py), the TUI renders the restored kinds with the
    wording their live paths used, and the GUI rebuilds the transcript from the file when
    rewind_result arrives — the requesting client included, because its own reply never reaches the
    renderer (that path is fixed in main.js + daemon_client.js).
  • The guards name the defect. tests/test_ws_e2e.py drives the daemon end to end over the
    websocket, tests/test_app_replay_rows.py pins the replay rows, and the renderer side has
    daemonBridge/historyReplay/Shell cases. The Shell case is the one that matters most: it
    states that deleting the rebuild effect leaves the other renderer tests green, which is exactly
    the "existed but was never wired" shape.
  • Counts agree with the tree. Agent.md reads 690 renderer / 158 GUI, which is master's
    684 / 157 plus this branch's six added renderer tests and one GUI test.

CI is green on both legs on this head (test 3m32s, test-windows 5m49s), the head has not moved
since, and I did not push to it in this cycle. Merging it on this reading.

@argszero
argszero merged commit 1e9b7e7 into master Oct 17, 2026
2 checks passed
argszero pushed a commit that referenced this pull request Oct 17, 2026
… the merged tree

#2357 landed while this branch was open, so Agent.md's derived count line
collided. Resolved on the merged tree, not by picking a side: the renderer
suite runs 691 tests here (master 690 plus this branch's one added
transcript case) and the GUI figure stays at master's 158, since this
branch touches no GUI test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

emrg: a reopened session cannot render what the live transcript showed — system and summary records, and a rewind says only how many rows it removed

2 participants