Review follow-up for #7569: change 4 of the live-snapshot perf PR replaced
one quadratic with another. The window-free backward walk is correct, but it
re-walks the raw reasoning transcript's whitespace once per interim event —
on a whitespace-heavy transcript that span is megabytes, and the replay went
~55x slower than master on the production shape the gate measured
(1MB / 20-interim journal: 0.035s master → 1.922s branch).
## Thinking Path
The gate's finding was that the cost is structural, not a tweak: the matcher
consults the *whole* accumulated transcript on every interim event, so any
raw-text rescan of it is O(interim_count x whitespace_span). The fix is the
data-structure change the review asked for — maintain the folded view and the
raw cut offsets incrementally, and match/strip from that indexed state
instead of rescanning the raw buffer.
## What Changed
- `api/streaming.py`: new `_CompactEchoIndex` — an append-only folded index
that records, per non-whitespace character, its raw index. `append` folds a
chunk once (cost proportional to the chunk, never the buffer),
`matches_tail`/`cut_to` compare only the candidate against the folded tail
(O(len(candidate)), no raw walk), `reset` drops the state after the raw
text is truncated. The old raw helpers (`_find_compact_echo_suffix_start`,
`_strip_compact_echo_suffix`) stay as the equivalence oracle.
- `api/routes.py::_run_journal_live_snapshot`: reasoning chunks feed the
index as they are appended; `reasoning_echo_tail_matches` and
`strip_reasoning_echo_tail` consult the index. The strip re-indexes the
truncated transcript so later probes match against it.
- `api/streaming.py` live path: the same swap for the live-stream echo strip.
The three live reasoning buffers (STREAM_REASONING_TEXT, the coalescing
buffer, the per-message segments) each carry a mirrored index, fed on every
append and re-indexed after a strip. The buffer flush resets the buffer's
index with the text it drops, so the index never describes text that is no
longer there.
- `docs/rfcs/webui-run-state-consistency-contract.md`: the echo-match clause
now names the indexed representation and the scaling regression that pins
it.
## Why It Matters
Removing one quadratic by introducing another in the same hot path is the
exact risk the gate flagged — the headline benchmark used a journal shape
where the window-free walk's cost does not dominate, so the aggregate number
still looked like a win. The index keeps the original 8.9x and adds the
scaling guarantee on the shape that was regressing.
## Verification
- Measured on the reproduced shape (1MB whitespace span, 20 interim events):
0.972s → 0.049s. Doubling both span and interim count scales ~2x per step
(x1 0.041s / x2 0.090s / x4 0.175s) — linear, not quadratic.
- Behaviour equivalence: 300 randomized journals (whitespace-heavy,
Unicode, chunked, flagged/unflagged interims) rebuilt with the indexed
implementation and with the raw walk swapped in — sha256 of the canonical
snapshot JSON matched on all 300.
- `tests/test_compact_echo_index.py` (new, 6 tests): oracle equivalence over
2000+ randomized cases, chunked-append equivalence, matches_tail/cut_to
agreement, reset-after-truncation, and two cost probes that fail the raw
walk.
- `tests/test_live_snapshot_echo_scan_scaling.py` (new, 4 tests): the
proportional-scaling regression the review asked for — verified to FAIL on
the pre-fix matcher (14.6x time for 4x rows, the quadratic signature) and
pass on the indexed one.
- Existing echo/journal suites unchanged and green: 19 items across
`test_compact_echo_suffix_linear_scan` / `test_live_snapshot_echo_dedup` /
`test_live_snapshot_single_parse`, plus 66 neighbouring items
(`test_issue1361_cancel_data_loss`, `test_issue2565_reasoning_accumulation`,
`test_issue2713_streaming_segment_flush`, `test_cancel_interrupt`,
`test_run_journal`).
## Contract Change
None. The matcher's observable behaviour is unchanged (same cut points, same
whitespace-insensitive semantics); only its cost profile changes. No config
knobs, no API surface.
## Risks
- Index drift: if a future writer mutates one of the reasoning buffers
without feeding (or resetting) its index, the matcher would silently miss
echoes. Mitigated by keeping the index feed adjacent to every existing
write site and by the reset-on-flush/reset-on-strip symmetry; the
equivalence test would catch a dropped feed as a missed echo.
- Memory: the index stores one char plus one int per non-whitespace
character. On a whitespace-heavy transcript this is bounded by the folded
length, which is what the transcript would cost folded anyway; the raw
text is still the larger object.
## Model Used
Hermes Agent (step-5-preview) with direct file edits and pytest verification.
Cost properties were measured with a purpose-built scaling probe (proportional
span x interim count), not a fixed-size fixture; the probe is kept as a
regression test rather than a script.
## Related
#7569 (this PR), #7310 (slow /api/session under load), #7534 (incremental
journal read cache — complementary), #7273/#7291 (metering write-side skip).
* fix(tests): make mtime_invalidation and glm_5_3 tests order-independent (#7100)
* fix: expose max reasoning for GPT-5.6 models
* fix(recovery): do not reattach cancelling runs
ACTIVE_RUNS tracks worker lifecycle, which is deliberately broader than
"a turn a browser may attach to". cancel_stream() keeps the row as
phase="cancelling" while the worker unwinds so a successor turn cannot
start on top of it, but the client has already reached a terminal state
for that stream: its run journal ends in a terminal event.
The recovery lookups treated every same-session ACTIVE_RUNS row as
attachable. An idle session holding a cancelling row therefore received a
recovered server_turn_started on every /api/session/stream subscription:
the client attached, replayed the terminal event, tore the renderer down,
resubscribed, and the server replayed the same frame again. The result is
an endless attach/replay loop that rebuilds the transcript repeatedly.
Separate the two meanings instead of narrowing one call site:
- api/config.py gains active_run_is_attachable() and
active_run_cancel_is_stale() as the shared predicates.
- active_stream_id_for_session() (browser recovery) returns attachable
rows only; _session_has_active_turn() (busy check) keeps counting a
fresh cancellation so a successor cannot overlap the unwinding worker.
- _live_active_stream_id() applies the same rule to the hidden-tab
status poller, on both the STREAMS and ACTIVE_RUNS paths.
- routes._cancelled_run_is_stale() now delegates to the shared predicate
rather than keeping a parallel copy of the staleness rule.
- A cancelling row past a bounded unwind window with no live STREAMS
channel is reclaimed from ACTIVE_RUNS and its stream owner released, so
a wedged worker cannot suppress background wakeups forever. Age alone
does not reclaim a row that still owns a live channel.
Staleness anchors on cancelled_at, falling back to started_at, so a
long-running turn cancelled moments ago is never treated as an orphan.
tests/test_cancelling_run_not_attachable.py covers both directions of
each rule. The six behavioral tests fail on the unpatched tree and pass
with the fix; two targeted mutations (forcing the attachability predicate
true, and disabling the staleness reaper) each turn the suite red.
* docs(rfc): document cancellation attach/admission split
Records the runtime contract introduced by the recovery fix in the WebUI
run-state consistency RFC, so the distinction is discoverable instead of
living only in code comments.
- Adds ACTIVE_RUNS to the State Layers table as the worker-lifecycle
registry, explicitly not the set of runs a browser may attach to.
- Adds invariant 9: lifecycle-busy is not client-attachable. Cancellation
splits the two meanings, recovery paths must exclude cancelling rows,
and admission checks must keep counting them.
- Documents the bounded cancellation-unwind window: reclamation needs
both age and the absence of a live STREAMS channel, and staleness is
anchored on the cancellation timestamp.
- Extends the review checklist with the admission-vs-attachment question
and the evidence required when changing a reclamation window.
* docs(changelog): note #7096 cancelling-run reattach, #7083 GPT-5.6 max reasoning, #7101 test order-independence
---------
Co-authored-by: webtecnica <webtecnica@gmail.com>
Co-authored-by: Abdulrahman Elkenany <boudy.elkenany123@gmail.com>
Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com>
Co-authored-by: n <a@n>
The RFC (docs/rfcs/session-sse-contract-v1.md) cited hardcoded absolute
line numbers like `api/routes.py:12345-12346` and `api/routes.py:16177`,
and tests/test_issue4812_session_sse_contract_rfc.py validated those exact
lines against source. Any PR that shifted routes.py lines (e.g. #5543,
#5534) broke the test — a chronic brittle-failure class (#5542).
Root fix: anchor by SYMBOL, never by line number.
RFC:
- Strip every `api/routes.py:<NNNN>` / `<NNNN>-<NNNN>` and
`api/streaming.py:<NNNN>` line-number suffix; keep the symbol name
(route string, function/constant names) + the file, e.g.
"_handle_session_events_stream() in api/routes.py".
- Update the inventory note to state names are the stable anchors and the
RFC deliberately avoids line numbers so a source-layout shift can't
invalidate the doc or its contract test.
Test:
- Rewrite the two line-anchor tests to assert (a) each cited symbol still
exists in api/routes.py and (b) the RFC names that symbol — dropping the
`api/routes.py:<line>` regex bounds-checks entirely. Invariant preserved:
"the RFC's cited symbols are real and named in the doc", NOT "the cited
line numbers are exact".
- Add test_rfc_uses_no_hardcoded_routes_line_numbers as a regression guard
that fails if any `*.py:<line>` anchor is reintroduced into the RFC.
Proven de-brittled: prepending 31 lines to api/routes.py (scratch, reverted)
leaves all 34 tests green.
Refs #5542, #5513
Codex re-gate finding: line 83 cited api/routes.py:15703-15762 for live SSE id:
emission, but that range is replay/helper code — the actual _sse_with_id() call
sites are 15918 (live /api/chat/stream), 15811 (runner-observe), 15721/15734
(journal replay). Corrected the bullet with the real, distinct anchors.
Also extended the test (test_rfc_run_journal_anchors_land_on_real_source) to
validate each named-symbol anchor (_parse_run_journal_event_id/_after_seq,
_runner_event_id, _replay_run_journal) + the live-emission anchor against the
actual source token, so no stale line number can silently pass.
Co-authored-by: rodboev <rodboev@users.noreply.github.com>
Codex Phase-1 gate found the RFC cited stale/wrong source anchors (docs-only, but
actively misleading to future implementers): /api/sessions/events is routed at
routes.py:12345-12346 (RFC said 12320-12321 = /api/chat/cancel);
_handle_session_events_stream at 16177 (RFC said 16142 = a gateway-sse comment);
replay reads after_event_id/after_seq QUERY PARAMS (RFC said Last-Event-ID header
— that's the *proposed* new-endpoint contract, not current). Corrected all six
routes.py anchors + the replay-mechanism description, verified each against master.
The accompanying test asserted the WRONG anchors and passed (codifying the error).
Rewrote test_rfc_cites_current_global_endpoint_source to parse every routes.py
anchor from the RFC and verify it lands on the real /api/sessions/events route +
_handle_session_events_stream definition — so it now catches drift instead of
locking in a stale number.
Co-authored-by: rodboev <rodboev@users.noreply.github.com>
Rebase PR #5162 (rename busy_input_mode -> default_message_mode; flip the
default from 'queue' to 'steer') onto current origin/master WITHOUT dropping
the shipped #5170 localStorage persistence mirror.
Rename the mirror machinery to the new setting name for consistency:
_BUSY_INPUT_MODES -> _DEFAULT_MESSAGE_MODES (values unchanged)
_normalizeBusyInputMode -> _normalizeDefaultMessageMode (fallback now 'steer')
_persistBusyInputMode -> _persistDefaultMessageMode
_readPersistedBusyInputMode -> _readPersistedDefaultMessageMode
window._busyInputMode -> window._defaultMessageMode (+ renamed exports)
localStorage: write the new 'hermes-default-message-mode' key; read it with a
fallback to the legacy 'hermes-busy-input-mode' key so an existing user's
persisted preference survives the rename.
Preserve #5170 behavior at every mirror site under the new names:
- boot success -> window._defaultMessageMode=_persistDefaultMessageMode(...)
- boot FAILURE -> window._defaultMessageMode=_readPersistedDefaultMessageMode()
(NOT a hardcoded 'steer' — a saved 'interrupt'/'queue' must still apply when
the server is unreachable; do not regress #5167/#5132)
- preferences autosave, settings-panel load, and _applySavedSettingsUi all
persist through _persistDefaultMessageMode(...)
Tests updated for the rename while keeping the persistence-behavior assertions
(test_1062, test_5145, test_5167); test_5167 gains explicit guards that the
load-failure path reads the persisted pref and never hardcodes a literal mode,
plus autosave/panel-load mirror-write coverage.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Direction confirmed by @nesquena (Discord) and the RFC is merged (#3862), so
flip Status Proposed -> Accepted. Also record the settled-path spike result on
integration point A: ~31 lines, no new data, compact previews via the existing
buildToolCard, and reload consistency for free from the rebuild cleanup; DOM
insertion order verified with a node harness. Doc-only.
Co-authored-by: Frank Song <franksong2702@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: nesquena-hermes <nesquena+hermes@gmail.com>
* docs(rfc): add Transparent Stream activity display mode RFC (#3820)
Proposes Transparent Stream as an opt-in, chronological activity display
mode alongside the default Compact Worklog (#3400/#3401). Captures the
display-mode split agreed in #3820: each tool call as a first-class
chronological event, interleaved with reasoning/progress, with compact
previews, consistent across live, settled, and reload/replay paths.
Documents the asymmetry in the existing `simplified_tool_calling` toggle
(live-only, no settled/reload branch) and the three concrete integration
points so the follow-up can be sliced safely. Doc-only; no behavior change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(rfc): refine Transparent Stream rollout scope
---------
Co-authored-by: Frank Song <franksong2702@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Harden interrupted recovery control filtering
* Redesign live-to-final assistant replies
* Fix live activity anchor test fixture
* Fix CI lint issues for live reply tests
* Strengthen live progress prompt contract
* Recover PR #3401 refresh on origin/master
* Repair live-to-final refresh regressions
* Fix live worklog refresh regressions
* Show live footer timer on initial stream start
* Restore live stream shell after reload
* Preserve per-frame live SSE replay cursors
* Preserve reasoning as Worklog Thinking cards
* Quiet Worklog Thinking card styling
* Align Worklog Thinking card styling
* Scope live Worklog Thinking cards by segment
* Suppress exact duplicate settled Thinking
* Close#3401 merge review test gaps
* fix(#3401): resolve 4 deep-review regressions (inline-think, reconnect-dup, neon skin, busy-gate worklog)
Deep review (Codex diff-vs-master + live-browser drive) of the live-to-final refactor
surfaced 4 regressions vs master that the rewritten suite no longer guarded:
1. Inline <think>…</think>answer reasoning vanished — _assistantReasoningPayloadText
used $-anchored regexes so a leading think block + visible answer extracted nothing
and the Thinking card never rendered. Removed the 3 $ anchors to match the
(non-anchored) display stripper. Live: inline-think thinking-only turn now renders.
2. (CORE) reconnect/reload duplicated the live reply — _rememberRunJournalCursor advanced
a closure-local seq but never wrote INFLIGHT[activeSid].lastRunJournalSeq, so a reload
replayed the journal from after_seq=0 over restored lastAssistantText. Now mirrors the
cursor onto INFLIGHT + schedules a throttled persist.
3. Neon skin silently broke — PR deleted the :root[data-skin="neon"] CSS but left Neon in
the picker. Restored the neon CSS block from master.
4. Settled tool-worklog rebuild gated purely on !S.busy — dropped every prior settled
turn's worklog when renderMessages re-ran during an active stream (switch-back to an
in-progress session). Restored master's !S.busy || (S.toolCalls && S.toolCalls.length).
Live: busy re-render now preserves tool cards (4→4, was 4→0).
Live-verified all 4 + confirmed #3709/#3592 invariants still hold (1 thinking card, none
below footer; distinct siblings preserved). + tests/test_issue3401_deep_review_fixes.py (7).
* test(#3401): realign 3 stale source-shape assertions to the deep-review fixes
Fix commit changed two source literals that existing stage tests scanned for:
- test_live_activity_timeline.py (x2): split anchor 'if(!S.busy){' → the restored
'if(!S.busy || (S.toolCalls&&S.toolCalls.length)){' guard (fix 4).
- test_run_journal_frontend_static.py: 'after_seq=0' not in source — fix 2's comment
contained that literal; rephrased the comment to 'the zero floor (after_seq of 0)'.
Intent of all three assertions unchanged; only the matched string updated. No code
behavior change.
* docs(changelog): v0.51.294 — Release JJ (stage-3401, #3401 live-to-final redesign)
---------
Co-authored-by: Frank Song <franksong2702@gmail.com>
Co-authored-by: Nathan-Hermes <nesquena-hermes@users.noreply.github.com>
Co-authored-by: nesquena-hermes <[email protected]>
* Refine live-to-final long-running session RFC
* Mark RFC accepted, decouple from live PR status, normalize terminal state names
Three follow-up adjustments to the refined live-to-final RFC:
- Status: Proposed -> Accepted, since the doc is now referenced as the parent
contract for follow-up slices; live implementation status stays in #3400.
- Keep volatile PR/merge state out of the RFC body. The Public Inventory and
Delivery map now state that their classification/vehicle columns record
durable scope, and that #3400 is authoritative for open/merged/superseded
status. Dropped the point-in-time "has shipped through release" / "remains an
active PR" assertions that would drift as PRs land.
- Normalize terminal-state naming: use the backticked snake_case identifiers
(`cancelled`, `compression_exhausted`, `tool_limit_reached`, `no_response`,
`interrupted`, `error`) consistently in prose, and add a note that these name
product states, not a wire/enum or persisted schema contract (consistent with
Scope, which does not own a backend schema change).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Add artifact handoff scope to live-to-final RFC
* Add live-to-final lifecycle flowchart to RFC
---------
Co-authored-by: Frank Song <franksong2702@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Merging the RFC as the agreed product contract for long-running-session assistant replies. Thank you @franksong2702! 🙏
It's docs-only (no code), well-structured, and gives the project a shared vocabulary for the follow-up implementation slices — in particular the honest terminal-state set (completed / cancelled / interrupted / compression-exhausted / tool-limit-reached / no-response / error, specific-wins-over-generic) and the live → settled → recovery/replay lifecycle. Nathan blessed merging it as the north-star contract.
Opt-in HTTP runner-client boundary for the runner-local runtime adapter
(RFC hermes-run-adapter-contract / #1925, Slice 4c/4d). Default-OFF:
without HERMES_WEBUI_RUNNER_BASE_URL the factory preserves the bounded
'not configured' path; when set, WebUI acts only as a JSON HTTP client +
SSE bridge for start/observe/status/controls without owning runner maps.
New api/runner_client.py + additive _runner_* helpers in api/routes.py;
no change to the live _run_agent_streaming legacy path.
Co-authored-by: AJV20 <AJV20@users.noreply.github.com>
nesquena-hermes