[RSI, performance] count tool-result message entries from their serialized header - #2416
Conversation
…alized header A cold saved-session catalog scan parses every transcript line, and a transcript-heavy catalog spends most of that parse on tool results: in the roster fixture (201 sessions, 106 MB) tool results are 54% of the bytes and 42% of the JSON.parse time, yet the fold reads nothing from them but the message count. Read the entry header instead of the payload for message entries whose role can only contribute that count. The header check is bounded to the first 512 characters, and it falls back to the full parse for every layout the file writer does not produce: spaced JSON, another key order, a role value that runs past the prefix, and any container before the role marker. That container guard covers both a payload nested before the type key and a payload that quotes the header before the entry's own message key, so a shadowed role marker can never hide a searchable entry's text. A double quote is escaped inside a JSON string, so a header that matches can only be structural. Measured on the same fixture, 5 cold scans per side, medians: scan wall 218.4 ms -> 182.6 ms (-16%) scan CPU 286.9 ms -> 257.8 ms (-10%) The id-first catalog layout gives the same win, the hardenings cost nothing measurable, and the returned SessionInfo is byte-identical before and after on every catalog measured.
Prime Agent performance — completedPR Overall: 1 regressed · 0 improved · 40 no clear change.
Python runtime
Session transport
UI interactions
Sandbox cost: ~$0.1004 — no inference calls. Methodology and samplesMain resolved at 2026-09-20T04:11:53.140380+00:00. Harness
|
…-manager suite Net test additions drop 299 -> 22 (source additions 22), so the test-policy gate passes at branch level with no category violations. The standalone message-count-scan.test.ts file is gone: its ten vectors now live in test/session-manager/file-operations.test.ts and reuse that file's existing tempDir/header/msg/line harness. The three header-count vectors and the five fallback-layout vectors are table-driven, and four pre-existing edge-case groups in the same file are consolidated into tables with every case kept. Per-cut lost-coverage ledger is in the PR body.
|
Second-pass review findings — holding:
|
main had already consolidated the same pre-existing clusters this branch's previous commit compressed, so the deletions were no-ops after the merge. The main-tip (post-merge) budget is therefore carried by the PR's own vectors only: they are re-added on top of main's consolidated file-operations.test.ts, reusing that file's existing tempDir/header/msg/line harness. Header-counting vectors kept (net 22 added test lines vs 22 meaningful source lines, gate green against main tip): - an unparsed tool result counted from its serialized header - an oversized tool result counted without reading its payload - tool results written in the id-first key order - a container before the role marker falls back to the full parse (keeps text) - a damaged session whose first entry is a tool result is still dropped Per-vector lost-coverage ledger is in the PR body.
…uplicate rows The header fast path's prefix-boundary early-out was unpinned after the budget compression. Restore its vector and pay for it with four rows in the same file that are true same-observable duplicates: - findMostRecentSession: drop "returns null for an empty directory" and "returns null for a non-existent directory" (the null observable survives in the non-jsonl and headerless-jsonl rows). - loadEntriesFromFile: drop "an empty file" and "malformed JSON" (the empty-array observable survives in the missing-file and headerless rows). Reverting the early-out makes the restored row fail and nothing else. Net test additions stay at 22 against 22 meaningful source lines, so the test-line budget gate still passes at the main-tip view.
snimu
left a comment
There was a problem hiding this comment.
Approved on snimu's instruction after independent read-only review (astra program, 2026-09-20). [written by prime-agent, checked by snimu]
18a56bf fix(coding-agent): make blocked daemon update restart warnings actionable (PrimeIntellect-ai#2515) 3fc5d96 fix(ci): harden behavioral eval token, label, and verifier trust (PrimeIntellect-ai#2449) aa242fb add grok 4.7 to every provider that serves it (PrimeIntellect-ai#2505) c91e6e9 [RSI, bug] fix(kernel): bound REPL protocol frame sizes in kernel and host (PrimeIntellect-ai#2423) 1272a3d [RSI, feature] feat(coding-agent): tell the model when Python skills fail to import (PrimeIntellect-ai#2381) 690e23d feat(coding-agent): per-request provider timing diagnostics (PI_REQUEST_TIMING) (PrimeIntellect-ai#2462) 27f32dd [RSI, bug] compaction: anchor summaries to kept-tail state and stop re-summarizing file lists (PrimeIntellect-ai#2385) 0498deb [RSI, performance] Cache the branch array on the per-turn hot path (PrimeIntellect-ai#2414) a8ae626 [RSI, performance] context: keep only the newest harness digest in built contexts (PrimeIntellect-ai#2394) e1f4ae5 [RSI, performance] start the daemon catalog on demand (PrimeIntellect-ai#2398) c37f5eb [RSI, performance] Append catalog metadata without full transcript parses (PrimeIntellect-ai#2433) 2883a78 [RSI, feature] Park quota-blocked sessions until the provider reset and auto-resume (PrimeIntellect-ai#2375) 0597614 [RSI, performance] count tool-result message entries from their serialized header (PrimeIntellect-ai#2416) b3e04b5 [RSI, bug] fix(coding-agent): re-park pending next-turn messages when an async-bash notice is withdrawn (PrimeIntellect-ai#2386) 2e9ab77 fix(coding-agent): reconnect attached windows when the daemon restarts (PrimeIntellect-ai#2458) 3b1aa5f feat(images): route image turns to a configured image model (PrimeIntellect-ai#2453) 41e4e0e fix(harness): validate refinement writes and skip malformed entries in the digest (PrimeIntellect-ai#2463) e683fcb [RSI, bug] fix(ai): recover stale Codex chains after metadata (PrimeIntellect-ai#2374) eee9d81 feat(coding-agent): add /speed command showing output tok/sec in the footer (PrimeIntellect-ai#2466) b08f08e feat(coding-agent): hold goal/autonomous continuations while background bash() handles run (PrimeIntellect-ai#2465) Gates: all passed Conflicts: auto=1 agent=0
…budget (#2501) * test(coding-agent): add regression pins from the RSI review batch Add tests for behaviors whose revert-mutants survive the current suite. No source changes. - harness digest at compaction boundaries: the compaction head keeps its fingerprint snapshot as the only digest on resume with unchanged state, and a legacy snapshot without a fingerprint self-heals with exactly one re-delivery that supersedes the snapshot - the live snapshot-clearing loop on the compaction summary when a fresh digest is appended - the leaf-branch cache is dropped when the same instance reloads its file - worker recovery completes when the interrupted-session notice cannot be written - the daemon catalog process routes rename/archive/mark_interrupted through the fast append path (real subprocess over DaemonCatalogClient) - the fast-append parentId equals the leaf a full open computes, across chained, label-leaf, and blank-line transcripts - the session scan drops a damaged file whose first entry is a tool result even when a valid session header follows Each pin was verified to fail on its named mutant in a Prime sandbox. Test-Budget-Exception: pin-only PR restoring mutant-surviving coverage found by the RSI review batch (#2394 #2400 #2414 #2416 #2433) * chore: drop changelog fragment (test-only PR, no release-notes surface) * test(coding-agent): fit the RSI regression pins into the test-line budget Net test lines must not exceed source lines (0 here), so the pins are folded into existing tests instead of added beside them: - agent-session-prompt: the two cold-boundary resume tests become one lifecycle test that also pins the compaction-head fingerprint (#2400) and the snapshot-clearing loop (#2394). - leaf-branch-cache: the rollback subset test folds into the held-array test, which now also pins the same-instance reload cache drop (#2414). - daemon-supervisor-monitor: one refused markInterrupted in the existing independence test pins the advisory catch (#2433). - append-to-existing-file: the catalog subprocess and leaf-differential tests are dropped (see PR body, "Coverage deliberately not restored"). Gate: TEST_POLICY_BASE=c91e6e991 node scripts/check-test-policy.mjs -> 42 added, 47 deleted test lines, 0 source lines. Passes. * test(coding-agent): drop a comment that restates the test title
Summary
Cold saved-session catalog scans (
SessionManager.list/listAll->scanSessionInfo) parse every transcript line withJSON.parse, but tool-result entries — the largest share of bytes in a transcript-heavy catalog — feed the scan fold nothing but their message count. This PR counts those entries from their serialized header instead of parsing the payload. Any layout the file writer does not produce (spacing, another key order, a nested container before the role marker, a role that runs past the bounded prefix) falls back to the full parse, so behavior only narrows.Profiling (instrumented local run, ui.py fixture catalog: 201 sessions, 106 MB)
Reproduced the
agents_rosterbenchmark path (saved-session list + roster composition) with a cold-daemon TUI driver on a scratch agent dir:SessionManager.listAll= 206 ms:JSON.parse~120 ms, file read+decode ~45 ms, fold/overhead ~40 ms.messageCount.Change
isCountOnlyMessageLine()(session-manager.ts) inspects a 512-char serialized header prefix:"type":"message"and"message":{"role":"("is escaped inside JSON strings, so a match cannot come from quoted text);{before the role marker (the marker carries the message object's own brace, so an earlier container means the found role is not the entry's role);user/assistantstill take the full parse (they feed usage, model, search text,firstMessage, andlastActivityTime);Returned
SessionInfois byte-identical before/after (digest over every field incl.allMessagesText, usage,firstMessage,modified, message count) on all three fixture catalogs.Intentional, bounded behavior delta: a structural-but-unparseable line (torn in-flight append, truncated tail) now counts toward
messageCountwhere a failed parse used to skip it — transient while the tail is in flight, self-correcting once the line completes. No other field can differ (count-only roles never fed any other fold output).Numbers (5 cold scans per side, separate processes, medians)
Production-shaped catalog (type-key-first layout, as SessionManager writes):
Fixture-shaped benchmark layout: wall 218.4 -> 182.3 ms, CPU 287.3 -> 258.0 ms. Pretty-printed catalog (no layout match, exercises the fallback): unchanged.
Expected
agents_rosterCI movement: the scan is the dominant roster-window cost, so the CPU metric moves by the scan's share (single-digit %); the wall metric is additionally floored by the benchmark's 3 s settle window, so expect a modest wall change.agents_open/agents_reopenand every incremental rescan share the same win.Tests
packages/coding-agent/test/session-manager/message-count-scan.test.ts(10 tests):Targeted suites (
test/session-manager/, session-scan-retention, session-reader-persistence, saved-session-catalog, daemon-session-list): 222 passed.npm run check: pass.No overlap with open PRs #2393 (daemon-mode/daemon-session-list), #2389 (cron-jobs), #2405/#2409 (kernel bootstrap): this touches only the session-manager scan path, its test, and a changeset.
Checklist
npm run check+ targeted vitest on touched areasTest-line budget
Fleet test-budget audit (2026-09-18): this PR exceeded the repo test-line budget gate (
node scripts/check-test-policy.mjs, run by CI's Build and check job: net added test lines over changed test files may not exceed meaningful added source lines, and changed test files may not add per-category violations). The user directive driving the audit: "probably needs to remove redundant and unnecessary tests throughout. Remove unnecessary tests and we should have enough." The owner policy decision authorized executing the named-vector cut menu in cost order: "nah dont grandfather them in. we should fix them now".Both numbers, because the two views differ after a branch update with
main:message-count-scan.test.ts(299 lines) that re-declared its own temp-dir and fixture harness.main): 27 added / 5 deleted test lines = 22 net vs 22 source, green, zero category violations. Measured at this head withGITHUB_BASE_REF=main node scripts/check-test-policy.mjs(merge base is nowmainitself), and the same check runs insideGITHUB_BASE_REF=main npm run check, which exits 0.Why the views differ (worth knowing for the other PRs in this wave): the first compression pass also consolidated several pre-existing clusters in
test/session-manager/file-operations.test.ts, butmainhad already consolidated exactly those clusters itself, so those deletions became no-ops the moment the branch metmain. The budget is therefore carried only by this PR's own vectors, which now live inside main's consolidated file and reuse itstempDir/header/msg/lineharness.Surviving vectors (all in
packages/coding-agent/test/session-manager/file-operations.test.ts, table rows unless noted): an unparsed tool result counted from its serialized header; an oversized tool result counted without reading its payload; tool results in the id-first key order (complete + torn); a container before the role marker falling back to the full parse; the role value starting exactly at the 512-character prefix boundary falling back too; and the damaged-session null path (readSessionInfonull +listAllempty). Two probes confirm the two subtlest pins are live: revertingsrc/core/session-manager.tsto the base makes the unparsed and id-first rows fail, and deleting the prefix-boundary early-out makes only the boundary row fail.Lost-coverage ledger (owner-authorized cuts, per vector)
Each row records what the cut pinned, why it was the cheapest option per line, and any surviving partial pin. Nothing disappears silently.
Traded for the restored boundary pin (four rows that are true same-observable duplicates):
findMostRecentSessiontable, row "returns null for an empty directory" and row "returns null for a non-existent directory": both assert the same observable (no session found in a directory that holds no sessions). Surviving pins: the "ignores non-jsonl files" and "ignores jsonl files without a valid session header" rows still assert the null result, and the remaining rows still cover selection and recency.loadEntriesFromFiletable, row "an empty file" and row "malformed JSON": both assert the same observable (empty entry array for a file with no usable records). Surviving pins: the "a missing file" and "a file without a session header" rows assert the same empty-array result; the "malformed line between valid ones" row still covers malformed input inside a valid file.Cuts to this PR's own new vectors:
message-count-scan.test.ts(299 lines): deleted; its vectors were folded intofile-operations.test.tsto reuse that file's existing harness instead of re-declaring it per file (the harness alone was ~50 lines).session_infoname handling is pinned by the labels/session-state tests.usageis not asserted there. Surviving pin: "scan and resident computation agree on whole-file own spend, forks and attributions included".test/session-info-modified-timestamp.test.ts.mainbecausemainhad already consolidated exactly those clusters into its own tables. No coverage was lost - every case still runs in main's version.Note
Medium Risk
Changes the hot path for listing saved sessions; edge cases include dropping headerless files that start with a tool result and counting some torn/unparseable tool-result lines from headers where parse used to skip them.
Overview
Cold session catalog scans (
readSessionInfo/SessionManager.listAll) no longerJSON.parseevery line when a message entry only contributesmessageCount.isCountOnlyMessageLineinspects the first 512 characters for writer-shaped"type":"message"and"message":{"role":"markers, treats roles other thanuser/assistantas count-only (e.g.toolResult), and falls back to full parse when spacing, key order, nested containers, or a role past the prefix do not match.foldSessionScanLineuses this only after a valid session header is seen so the first line still validates damaged files.Oversized tool payloads can increment
messageCountwithout reading megabytes of content. Tests infile-operations.test.tscover header counting, fallback layouts, and rejecting a file whose first line is a tool result; a few duplicate empty/malformed loader cases were removed.Reviewed by Cursor Bugbot for commit f9c4c9c. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Count tool-result message entries from serialized header in session scan
isCountOnlyMessageLineto session-manager.ts, a bounded classifier that inspects only the first 512 characters of a serialized line for message type and role markers. It rejects lines with earlier nested containers or roles reaching beyond the prefix.foldSessionScanLinewhere qualifying non-user/non-assistant entries incrementmessageCountand return before JSON parsing. Unsupported layouts fall through to the existing full-parse path.SessionManager.listAllresults, since the fast path only activates after a valid session header is seen.📊 Macroscope summarized f9c4c9c. 2 files reviewed, 1 issue evaluated, 1 issue filtered, 0 comments posted
🗂️ Filtered Issues
packages/coding-agent/src/core/session-manager.ts — 0 comments posted, 1 evaluated, 1 filtered
updateLastActivityTime, but every persisted message entry has its owntimestamp(seeappendMessageat session-manager.ts:1808-1815), and the previous full-parse path updated activity for every entry. Consequently, any normally sized tool-result,bashExecution, or custom message matching this header leaves the catalog'smodified/last-activity value at the preceding entry. For example, a session whose last event is a tool result will be listed and sorted with a stale modification time. Restricting the optimization to roles whose entry timestamp is known irrelevant, or extracting/updating the entry timestamp from the header, preserves the existing catalog semantics. [ Failed validation ]