Skip to content

Commit 652d979

Browse files
server: the live stream's cursor describes what it read, not the file (#120)
`frames()` polls the trace's size, rebuilds a snapshot when it changes, and stores that snapshot's `size` as "everything up to here has been sent": last_size = snapshot.size if snapshot.size else size `build_snapshot` took that number from `path.stat()` *after* reading the events, and the two can disagree in the direction that matters. The read stops at the last newline, and -- this being a live view -- the run is appending while it happens, so bytes can land between the read and the stat. The cursor then claims events the snapshot never saw, the next poll sees an unchanged file size and rebuilds nothing, and if the writer has stopped the file never changes size again. A finished run streams as a running one for as long as the page is open. Demonstrated against the old code rather than argued: with one append landing in that window, the cursor came back equal to the file's final size while `done` was still False -- the exact pair that leaves `frames()` with no reason to rebuild and nothing left to learn from. `TailRecorder.read_tail()` now returns the events *and* the byte count they came from; `read_events()` keeps its signature over it, since that is what every other caller uses. `build_snapshot` uses the count and reads only the mtime from the file's current state, which is safe to be too new: it makes a run look more recently active, delaying an idle verdict by one poll rather than stopping the stream. Nine tests, including the ordinary quiet-and-complete case -- where the cursor is the file size and the stream must still settle -- and the half-written final line, which is excluded from the count because bytes after the last newline cannot parse into an event yet. This is the `build_snapshot` read-then-stat race from the 2026-08 sweep. It was recorded as suspected and never demonstrated; it is demonstrated now. Verified: 194 tests across the live, server, trace and replay files, ruff clean, figure refreshed to 2,189. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5295050 commit 652d979

4 files changed

Lines changed: 251 additions & 9 deletions

File tree

‎docs/deep-dive.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ A stable system is not one that claims to have no edges — it is one whose edge
254254
- **`.env` and `grapharc.toml` follow the same discovery rule: the working directory, and nowhere else.** Neither searches parent directories — a run must not be governed by a file you did not know about, and must not be *billed* to one either. **This is a behaviour change:** the credential loader used to walk up to `/`, so a `.env` in an ancestor directory (a `$HOME` one on a shared box, a client project one above a demo checkout) was picked up silently. If you relied on that, move the file into the directory you run from, `export` the variable, or pass `env_file=` to name it explicitly. A real environment variable still beats any file.
255255
- **`grapharc run` has no budget unless you give it one.** Set any of `--max-tokens`, `--max-iterations`, `--max-seconds`, or `--max-concurrency`; without them each dimension is unlimited and the gate admits a topology of any worst-case cost.
256256

257-
**Verified this pass:** `pytest` → green, 2,180 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.7` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift.
257+
**Verified this pass:** `pytest` → green, 2,189 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.7` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift.
258258

259259
[ROADMAP.md](../ROADMAP.md) tracks what is built and what is not, item by item.
260260

‎grapharc/observe/trace.py‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -271,15 +271,32 @@ def record(self, event: TraceEvent) -> None:
271271
raise RuntimeError("TailRecorder is read-only")
272272

273273
def read_events(self, run_id: str | None = None) -> list[TraceEvent]:
274+
return self.read_tail(run_id)[0]
275+
276+
def read_tail(self, run_id: str | None = None) -> tuple[list[TraceEvent], int]:
277+
"""The events, and **how many bytes they came from**.
278+
279+
The byte count is not a detail a caller can re-derive with `stat()`
280+
afterwards, which is why it is returned here. Two things make the
281+
file's size at any later moment a different number: this read stops at
282+
the last newline, so a half-written final line is excluded; and another
283+
process may append between the read and the stat.
284+
285+
A caller that uses a later `st_size` as its "everything up to here is
286+
rendered" cursor therefore claims to have consumed bytes it never saw,
287+
and will skip them for as long as the file stays that size — forever,
288+
if the run has finished. `server.live` is that caller.
289+
"""
274290
try:
275291
raw = self.path.read_bytes()
276292
except OSError:
277-
return []
293+
return [], 0
278294
cut = raw.rfind(b"\n")
279295
if cut < 0:
280-
return []
296+
return [], 0
297+
consumed = cut + 1
281298
events = []
282-
for line in raw[: cut + 1].splitlines():
299+
for line in raw[:consumed].splitlines():
283300
if not line.strip():
284301
continue
285302
try:
@@ -288,7 +305,7 @@ def read_events(self, run_id: str | None = None) -> list[TraceEvent]:
288305
continue
289306
if run_id is None or event.run_id == run_id:
290307
events.append(event)
291-
return events
308+
return events, consumed
292309

293310

294311
def load_events(

‎grapharc/server/live.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,14 +294,25 @@ def build_snapshot(root: Path, rel: str, run_id: str | None) -> LiveSnapshot:
294294
"""
295295
path = resolve_trace(root, rel)
296296
recorder = TailRecorder(path)
297-
events = recorder.read_events()
297+
# `size` is the stream's "everything up to here is rendered" cursor, so it
298+
# must describe *what was read*, not what the file is now. Those differ two
299+
# ways: this read stops at the last newline, and the writer may append
300+
# between the read and any later `stat()`. Taking it from `st_size`
301+
# therefore claimed bytes the snapshot never saw, and `frames()` skips a
302+
# file whose size has not changed since the last cursor — so those events
303+
# were never sent, and never would be once the run stopped writing. A
304+
# finished run then streamed as one still running, forever.
305+
events, consumed = recorder.read_tail()
298306
if not events:
299307
return LiveSnapshot(trace=rel)
308+
size = consumed
300309
try:
301-
stat = path.stat()
302-
size, quiet_for = stat.st_size, time() - stat.st_mtime
310+
# Only the mtime comes from the file's current state. Erring *new* here
311+
# is harmless: it makes the run look more recently active, which delays
312+
# an idle verdict by one poll rather than stopping the stream.
313+
quiet_for = time() - path.stat().st_mtime
303314
except OSError:
304-
size, quiet_for = 0, float("inf")
315+
quiet_for = float("inf")
305316
return compose_snapshot(
306317
rel,
307318
recorder,

‎tests/test_live_stream_cursor.py‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
"""`LiveSnapshot.size` is a cursor, and it has to describe what was read.
2+
3+
`frames()` in `server.live` polls the trace file's size, rebuilds a snapshot
4+
when it changes, and then stores the snapshot's own `size` as "everything up to
5+
here has been sent":
6+
7+
last_size = snapshot.size if snapshot.size else size
8+
9+
So a `size` larger than the bytes the snapshot actually covers is not a
10+
cosmetic error. It claims events that were never sent, and the next poll sees
11+
an unchanged file size and rebuilds nothing. If the writer has stopped — the
12+
run finished — the file never changes size again, the stream never rebuilds,
13+
and a finished run is streamed as a running one for as long as the page is
14+
open.
15+
16+
`build_snapshot` used to take that number from `path.stat()` *after* reading
17+
the events, which can exceed the bytes read two different ways:
18+
19+
1. `TailRecorder` cuts at the last newline, so a half-written final line is
20+
excluded from the events but counted by `st_size`.
21+
2. The run is appending concurrently — that is the whole premise of a live
22+
view — so bytes can land between the read and the stat.
23+
24+
Only (2) loses whole events, because the bytes (1) leaves out cannot yet parse
25+
into one. Both are fixed by the same change: the count comes from the read.
26+
"""
27+
28+
from __future__ import annotations
29+
30+
import json
31+
from pathlib import Path
32+
33+
from grapharc.observe.trace import TailRecorder
34+
from grapharc.server.live import build_snapshot
35+
36+
37+
def _event(
38+
run_id: str = "r1", phase: str = "start", step: int = 1, *, terminal: bool = False
39+
) -> str:
40+
body: dict = {
41+
"ts": "2026-01-01T00:00:00+00:00",
42+
"run_id": run_id,
43+
"graph": "g",
44+
"node": "n",
45+
"phase": phase,
46+
"step": step,
47+
}
48+
if terminal:
49+
# What `compose_snapshot` actually reads to decide a run is over; a bare
50+
# `stop` phase on a node is not it.
51+
body["state_delta"] = {"termination_reason": "completed"}
52+
return json.dumps(body)
53+
54+
55+
def _trace(tmp_path: Path, *lines: str, tail: str = "") -> Path:
56+
path = tmp_path / "trace.jsonl"
57+
path.write_text("".join(line + "\n" for line in lines) + tail, encoding="utf-8")
58+
return path
59+
60+
61+
# -- what the recorder reports ----------------------------------------------
62+
63+
64+
def test_the_recorder_reports_the_bytes_its_events_came_from(tmp_path):
65+
path = _trace(tmp_path, _event(), _event(step=2))
66+
events, consumed = TailRecorder(path).read_tail()
67+
68+
assert len(events) == 2
69+
assert consumed == path.stat().st_size
70+
71+
72+
def test_a_half_written_final_line_is_excluded_from_the_count(tmp_path):
73+
"""The bytes after the last newline cannot parse into an event, so counting
74+
them would claim an event that does not exist yet."""
75+
path = _trace(tmp_path, _event(), _event(step=2), tail='{"ts": "2026-01-0')
76+
events, consumed = TailRecorder(path).read_tail()
77+
78+
assert len(events) == 2
79+
assert consumed < path.stat().st_size
80+
# And the count is exactly the complete prefix.
81+
assert path.read_bytes()[:consumed].endswith(b"\n")
82+
83+
84+
def test_a_file_with_no_complete_line_reads_as_nothing(tmp_path):
85+
path = _trace(tmp_path, tail='{"ts": "2026-01-0')
86+
87+
assert TailRecorder(path).read_tail() == ([], 0)
88+
89+
90+
def test_read_events_still_returns_just_the_events(tmp_path):
91+
"""The old signature is what every other caller uses."""
92+
path = _trace(tmp_path, _event(), _event(step=2))
93+
94+
assert TailRecorder(path).read_events() == TailRecorder(path).read_tail()[0]
95+
96+
97+
# -- the cursor the stream stores --------------------------------------------
98+
99+
100+
def test_the_snapshot_size_never_exceeds_what_it_read(tmp_path, monkeypatch):
101+
"""The wedge, demonstrated.
102+
103+
A writer appending between the read and the stat is the ordinary case for a
104+
live view, not an exotic one; it is made deterministic here by appending
105+
from inside the read. With `size` taken from a later `stat()`, the snapshot
106+
reported 2 events and a cursor past the third, and `frames()` would store
107+
that cursor and skip the third event for as long as the file stayed that
108+
size.
109+
"""
110+
path = _trace(tmp_path, _event(), _event(step=2))
111+
real_read_tail = TailRecorder.read_tail
112+
113+
def read_then_append(self, run_id=None):
114+
result = real_read_tail(self, run_id)
115+
# The writer lands in the window.
116+
with open(self.path, "a", encoding="utf-8") as handle:
117+
handle.write(_event(phase="stop", step=3) + "\n")
118+
return result
119+
120+
monkeypatch.setattr(TailRecorder, "read_tail", read_then_append)
121+
snapshot = build_snapshot(tmp_path, "trace.jsonl", None)
122+
123+
assert len(snapshot.run_ids) == 1
124+
# The cursor describes the two events that were read, not the three bytes'
125+
# worth now on disk.
126+
assert snapshot.size < path.stat().st_size, (
127+
"size claims bytes the snapshot never read; frames() will skip them"
128+
)
129+
130+
131+
def test_the_skipped_event_is_picked_up_on_the_next_read(tmp_path, monkeypatch):
132+
"""The point of the fix: the stream self-corrects.
133+
134+
Because the cursor stopped short, the file's size now differs from it, so
135+
the next poll rebuilds and the event arrives. That is the difference
136+
between a one-poll delay and a stream that never recovers.
137+
"""
138+
path = _trace(tmp_path, _event(), _event(step=2))
139+
real_read_tail = TailRecorder.read_tail
140+
appended = {"done": False}
141+
142+
def read_then_append_once(self, run_id=None):
143+
result = real_read_tail(self, run_id)
144+
if not appended["done"]:
145+
appended["done"] = True
146+
with open(self.path, "a", encoding="utf-8") as handle:
147+
handle.write(_event(phase="stop", step=3) + "\n")
148+
return result
149+
150+
monkeypatch.setattr(TailRecorder, "read_tail", read_then_append_once)
151+
first = build_snapshot(tmp_path, "trace.jsonl", None)
152+
153+
monkeypatch.setattr(TailRecorder, "read_tail", real_read_tail)
154+
second = build_snapshot(tmp_path, "trace.jsonl", None)
155+
156+
assert first.size < path.stat().st_size # cursor short, so the poll refires
157+
assert second.size == path.stat().st_size
158+
assert second.size > first.size
159+
160+
161+
def test_a_quiet_complete_trace_reports_the_whole_file(tmp_path):
162+
"""The ordinary case must not regress: nothing is being appended, the last
163+
line is complete, so the cursor is the file size and the stream settles."""
164+
path = _trace(tmp_path, _event(), _event(step=2), _event(phase="stop", step=3))
165+
snapshot = build_snapshot(tmp_path, "trace.jsonl", None)
166+
167+
assert snapshot.size == path.stat().st_size
168+
169+
170+
def test_an_empty_or_missing_trace_is_still_a_waiting_snapshot(tmp_path):
171+
"""The URL is posted before the run starts writing."""
172+
(tmp_path / "trace.jsonl").write_text("", encoding="utf-8")
173+
174+
assert build_snapshot(tmp_path, "trace.jsonl", None).size == 0
175+
176+
177+
def test_a_finished_run_stops_reporting_itself_as_running(tmp_path, monkeypatch):
178+
"""The symptom a viewer actually sees, end to end.
179+
180+
Reproduced against the old code: an append landing in the window left the
181+
cursor equal to the file's final size while `done` was still False, so
182+
`frames()` had no reason to rebuild and never learned the run had stopped.
183+
The page showed a finished run as running for as long as it stayed open.
184+
185+
With the cursor taken from the read, it falls short of the file, the next
186+
poll rebuilds, and that rebuild sees the `stop` event.
187+
"""
188+
path = _trace(tmp_path, _event(), _event(step=2))
189+
real_read_tail = TailRecorder.read_tail
190+
fired = {"n": 0}
191+
192+
def read_then_append_once(self, run_id=None):
193+
result = real_read_tail(self, run_id)
194+
fired["n"] += 1
195+
if fired["n"] == 1:
196+
with open(self.path, "a", encoding="utf-8") as handle:
197+
handle.write(_event(phase="stop", step=3, terminal=True) + "\n")
198+
return result
199+
200+
monkeypatch.setattr(TailRecorder, "read_tail", read_then_append_once)
201+
first = build_snapshot(tmp_path, "trace.jsonl", None)
202+
final_size = path.stat().st_size
203+
204+
# The old failure was exactly this pair: cursor == final size, done False.
205+
assert not (first.size == final_size and not first.done), (
206+
"cursor matches the finished file while done is False: frames() will "
207+
"never rebuild and the run streams as running forever"
208+
)
209+
210+
monkeypatch.setattr(TailRecorder, "read_tail", real_read_tail)
211+
second = build_snapshot(tmp_path, "trace.jsonl", None)
212+
213+
assert second.done is True
214+
assert second.size == final_size

0 commit comments

Comments
 (0)