Uh oh!
There was an error while loading. Please reload this page.
fix(fleet): replace bounded event-log reads with uncapped streaming - #489
Merged
Merged
Conversation
The Runs/History/run-detail screens read a run's JSONL event log through three separate bounded windows (a 512 KiB tail, a 512 KiB head recovery read, and an 8 MiB full-log cap). A long-lived or resumed run outgrows these: a 9.72 MB / 20,361-line log lost its current step and token/cost totals, and a resumed run's second workflow_started event fell outside the head window, misreporting topology. summary.py::stream_event_log is a single streamed reader bounded only by the longest line, replacing all three windows. It supports a keep_types prefilter so the Runs screen's ~2s poll can skip uninteresting lines via regex before JSON-parsing. history.py now delegates to the same reader and takes the latest root workflow_started's timestamp for a resumed run's duration fallback.
Applies PR #489 review findings on top of the uncapped event-log streaming change: - `_scan_agent_details` (run-detail screen) now resets `open_steps`, `gated`, and `started_at_by_name` at every root `workflow_started`, matching `_scan_events`'s existing resume-boundary reset. Previously a generation killed mid-step (an open step or unresolved gate with no closing event) would survive across a resume forever, so the Runs screen correctly reported "running, nothing open" while run-detail simultaneously reported a dead step as still "at-gate" with a stale, ever-growing elapsed clock. - `derive_step_detail`'s consumption loop is extracted into `_scan_step_events`, returning its five locals in one tuple assigned atomically inside the caller's `try`/`except OSError`. Previously a mid-stream `OSError` left `status`/`output`/`activity` holding whatever a partial scan had accumulated, which was then returned as if it were authoritative (e.g. a completed step with real output rendering as "running, no output" forever). The same pass also resets a step whose `status` was still "running" across a resume boundary to "pending", and clears `prompt` alongside `output`/`activity` on a restart. Also applies several review recommendations: - `derive_run_detail` now prefilters its scan with `keep_types=_SUMMARY_EVENT_TYPES` (a verified superset of everything `_scan_agent_details` branches on), cutting its cost roughly 5x. - Promotes `history.py`'s `_finite_float` NaN/Infinity guard into `summary.py` and reuses it for token/cost accumulation in both `_scan_events` and `_scan_agent_details`, so a `NaN`/`Infinity` value from a corrupted log entry can no longer crash the Runs/run-detail poll loops. - Corrects several docstrings left describing the deleted bounded tail/head/full-log readers or overstating scan frequency/cost (`RunDetail`, `stream_event_log`'s `keep_types` Args/Raises, `derive_step_detail`'s Args/Returns, `step_detail.py`, `dag.py`, and history.py's resume-boundary paragraph). - Adds regression tests: run-detail generation reset, mid-stream `OSError` no longer surfacing a partial scan, a nested sub-workflow start not being mistaken for a resume boundary (both `summary.py` and `history.py`), a `_scan_agent_details` prefilter-equivalence test, a NaN/Infinity token/cost regression test, and a strengthened totals-accumulate-across-generations assertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces three separate bounded event-log read windows (512 KiB tail, 512 KiB head recovery, 8 MiB full-log cap) used by the Fleet Manager's Runs/History/run-detail screens with a single uncapped streamed reader (
summary.py::stream_event_log), bounded only by the longest single line.A long-lived or resumed run could outgrow these caps: a real 9.72 MB / 20,361-line log lost its current step and token/cost totals, and a resumed run's second
workflow_startedevent fell outside the head-recovery window, misreporting workflow topology.stream_event_logsupports an optionalkeep_typesprefilter so the Runs screen's ~2s poll can skip uninteresting lines via a cheap regex check before JSON-parsing (measured 12.5 ms vs 65 ms unfiltered on the 9.72 MB log)._scan_eventsis generation-aware: a resumed run's second rootworkflow_startedresets status/gate/open-steps and overwrites topology, but token/cost totals accumulate across generations.history.pynow delegates its line-reading to the shared streamed reader and derivesstarted_atfrom the latest rootworkflow_started, so a resumed run's duration fallback measures the current attempt.Closes#485