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).
219 lines
7.7 KiB
Python
219 lines
7.7 KiB
Python
"""``_CompactEchoIndex`` must agree with the raw backward walk, cheaply.
|
|
|
|
The index is the echo-match authority for both the journal rebuild
|
|
(``api/routes.py::_run_journal_live_snapshot``) and the live stream
|
|
(``api/streaming.py``). It replaces a per-interim raw backward walk that
|
|
rescanned the whole transcript's whitespace — quadratic on whitespace-heavy
|
|
text (#7569). Two contracts must hold for the swap to be safe:
|
|
|
|
1. **Equivalence** — incremental appends must reach the same cut point as a
|
|
raw walk over the concatenated text, for every shape including chunked
|
|
appends, whitespace-only chunks, and exotic Unicode whitespace.
|
|
2. **Cost** — the index must never rescan the buffer it has already folded:
|
|
a probe after a wide whitespace span must not walk that span.
|
|
"""
|
|
import random
|
|
import re
|
|
import string
|
|
|
|
import pytest
|
|
|
|
|
|
def _compact(value):
|
|
return re.sub(r'\s+', '', str(value or ''))
|
|
|
|
|
|
def _raw_reference(value, suffix):
|
|
"""Deliberately naive oracle: probe every cut index, fold the tail."""
|
|
raw = str(value or '')
|
|
candidate = _compact(suffix)
|
|
if not raw or not candidate:
|
|
return raw, False
|
|
for idx in range(len(raw) + 1):
|
|
if _compact(raw[idx:]) == candidate:
|
|
return raw[:idx].rstrip(), True
|
|
return raw, False
|
|
|
|
|
|
def _cases():
|
|
cases = [
|
|
# Exact echo at the end.
|
|
("bla bla bla une conclusion", "une conclusion"),
|
|
# Echo differing only by whitespace shape.
|
|
("bla bla une conclusion", "une conclusion"),
|
|
("bla bla\nune\tconclusion", "une conclusion"),
|
|
# No echo.
|
|
("texte quelconque sans rapport", "une conclusion"),
|
|
# Partial echo must NOT match.
|
|
("bla une conclu", "une conclusion"),
|
|
# The buffer is exactly the echo.
|
|
("une conclusion", "une conclusion"),
|
|
# Empty / None inputs.
|
|
("", "une conclusion"),
|
|
("bla bla", ""),
|
|
("", ""),
|
|
(None, "x"),
|
|
("x", None),
|
|
# Whitespace around the cut point.
|
|
("bla bla une conclusion ", "une conclusion"),
|
|
("bla bla \n une conclusion", "une conclusion"),
|
|
# Unicode, accents, emoji, exotic whitespace.
|
|
("préambule ✅ conclusion émise", "conclusion émise"),
|
|
("texte 🟢 Réponse", "🟢 Réponse"),
|
|
("bla\u00a0bla une conclusion", "une\u00a0conclusion"),
|
|
("bla\u2028une conclusion", "une conclusion"),
|
|
# Whitespace-only buffer.
|
|
(" ", "une conclusion"),
|
|
# Repetitions: the leftmost cut point is contractual.
|
|
("abc abc abc", "abc"),
|
|
("abc abc abc", "abc abc"),
|
|
("aaaa", "aa"),
|
|
]
|
|
rnd = random.Random(20260920)
|
|
alphabet = string.ascii_letters + " \n\t" + "éà✅\u00a0"
|
|
for _ in range(2000):
|
|
base = "".join(rnd.choice(alphabet) for _ in range(rnd.randint(0, 120)))
|
|
if rnd.random() < 0.5 and len(base) > 4:
|
|
k = rnd.randint(1, max(1, len(base) // 2))
|
|
suffix = base[-k:]
|
|
else:
|
|
suffix = "".join(rnd.choice(alphabet) for _ in range(rnd.randint(0, 30)))
|
|
cases.append((base, suffix))
|
|
return cases
|
|
|
|
|
|
def test_index_matches_raw_reference_oracle():
|
|
"""Incremental index == naive raw probe, for one-shot and chunked appends."""
|
|
import api.streaming as streaming
|
|
|
|
divergences = []
|
|
for value, suffix in _cases():
|
|
expected = _raw_reference(value, suffix)
|
|
index = streaming._CompactEchoIndex()
|
|
index.append(value)
|
|
actual = index.cut_to(suffix)
|
|
if actual is None:
|
|
got = (str(value or ''), False)
|
|
else:
|
|
got = (str(value or '')[:actual].rstrip(), True)
|
|
if got != expected:
|
|
divergences.append((value, suffix, expected, got))
|
|
|
|
assert not divergences[:5], (
|
|
f"{len(divergences)} divergence(s) from the reference implementation; "
|
|
f"first: {divergences[:1]}"
|
|
)
|
|
|
|
|
|
def test_chunked_appends_reach_the_same_cut_point():
|
|
"""Appending in pieces must equal appending the whole string."""
|
|
import api.streaming as streaming
|
|
|
|
rnd = random.Random(99)
|
|
for _ in range(500):
|
|
parts = [
|
|
"".join(
|
|
rnd.choice(string.ascii_letters + " \n\té")
|
|
for _ in range(rnd.randint(1, 20))
|
|
)
|
|
for _ in range(rnd.randint(1, 8))
|
|
]
|
|
value = "".join(parts)
|
|
if rnd.random() < 0.6 and len(value) > 4:
|
|
suffix = value[-rnd.randint(1, len(value) // 2):]
|
|
else:
|
|
suffix = "".join(rnd.choice("abc \t") for _ in range(rnd.randint(0, 12)))
|
|
|
|
chunked = streaming._CompactEchoIndex()
|
|
for part in parts:
|
|
chunked.append(part)
|
|
whole = streaming._CompactEchoIndex()
|
|
whole.append(value)
|
|
|
|
assert chunked.cut_to(suffix) == whole.cut_to(suffix), (
|
|
f"chunked append diverged for value={value!r} suffix={suffix!r}"
|
|
)
|
|
|
|
|
|
def test_matches_tail_and_cut_to_agree():
|
|
"""The boolean probe and the cut probe must never disagree."""
|
|
import api.streaming as streaming
|
|
|
|
for value, suffix in _cases():
|
|
index = streaming._CompactEchoIndex()
|
|
index.append(value)
|
|
assert index.matches_tail(suffix) == (index.cut_to(suffix) is not None), (
|
|
f"matches_tail/cut_to disagree for value={value!r} suffix={suffix!r}"
|
|
)
|
|
|
|
|
|
def test_reset_clears_state_after_truncation():
|
|
"""After reset + re-append the index must describe only the new text."""
|
|
import api.streaming as streaming
|
|
|
|
index = streaming._CompactEchoIndex()
|
|
index.append("old reasoning tail with a wide span " + " " * 5000)
|
|
index.reset()
|
|
index.append("fresh start only")
|
|
|
|
assert index.compact_length == len("freshstartonly")
|
|
assert index.cut_to("start only") == len("fresh ")
|
|
assert index.cut_to("old reasoning") is None
|
|
|
|
|
|
def test_index_probe_does_not_rescan_a_wide_whitespace_span(monkeypatch):
|
|
"""The probe must not walk raw characters it already folded.
|
|
|
|
Pins the cost property the raw backward walk violated: after a 1 MB
|
|
whitespace span is appended, a probe must cost O(candidate), not
|
|
O(span). Counting ``str.isspace``-adjacent raw access is impractical, so
|
|
this counts how many characters ``append`` has to fold: exactly the
|
|
appended length, once — and the probe itself must not fold anything.
|
|
"""
|
|
import api.streaming as streaming
|
|
|
|
fold_calls = {'n': 0}
|
|
original = streaming._compact_for_echo_compare
|
|
|
|
def counting(value):
|
|
fold_calls['n'] += 1
|
|
return original(value)
|
|
|
|
monkeypatch.setattr(streaming, '_compact_for_echo_compare', counting)
|
|
|
|
index = streaming._CompactEchoIndex()
|
|
index.append("alpha" + " " * 100_000)
|
|
fold_calls['n'] = 0
|
|
|
|
# 20 probes of a short candidate: the raw walk would fold/walk the span
|
|
# each time; the index folds only the candidate.
|
|
for i in range(20):
|
|
assert index.matches_tail(f"progress {i}") is False
|
|
|
|
assert fold_calls['n'] <= 20, (
|
|
f"echo probe folded {fold_calls['n']} times — it is rescanning the "
|
|
"raw transcript instead of using the indexed tail"
|
|
)
|
|
|
|
|
|
@pytest.mark.timeout(60)
|
|
def test_index_stays_cheap_on_a_wide_span_with_many_probes():
|
|
"""Wide whitespace span + many probes must not cost seconds of CPU."""
|
|
import time
|
|
|
|
import api.streaming as streaming
|
|
|
|
index = streaming._CompactEchoIndex()
|
|
index.append("alpha" + " " * 1_000_000)
|
|
probes = [f"progress {i}" for i in range(200)]
|
|
|
|
start = time.perf_counter()
|
|
for probe in probes:
|
|
index.matches_tail(probe)
|
|
index.cut_to(probe)
|
|
elapsed = time.perf_counter() - start
|
|
|
|
# The raw backward walk needs ~0.05s per probe here (2M+ character walks
|
|
# total). A generous budget still fails it by an order of magnitude.
|
|
assert elapsed < 0.5, f"200 probes on a 1MB span took {elapsed:.3f}s"
|