feat(llm-reach): the checkpoint summaries publish the stage-local running usage — per attempted batch, exception-safe, the delta semantics (#603) - #641
Merged
Conversation
gadievron
requested review from
dgeyshis,
shahar-davidson and
sounil
as code owners
September 17, 2026 10:00
| incomplete=max( | ||
| 0, len(units) - (len(adopted) + persisted)), | ||
| ) | ||
| except OSError: |
…ning usage — per attempted batch, exception-safe, the delta semantics (#603) The two writes per pass (start + end) passed usage=None (a silent omission) and nothing updated the summary per batch — the stage-local running cost was invisible to any summary-reading consumer mid-stage (the terminal step report retained it: live visibility, not terminal accounting). The fix publishes at the pass start (the true zero baseline — the prior-usage injection now precedes the write, so the first snapshot is internally consistent), after EVERY attempted batch — including the failed/dropped/unrecovered-split paths that continue past the record persistence (the billed-but-empty cases whose spend the live summary exists to show), and at termination. The publication is exception-safe (except OSError, best-effort refreshes: the #599 counters own the pass-start/final failure signals). THE DELTA SEMANTICS: the global tracker is cumulative (the prior phases' spend included), so a naive totals read would double-count with the step report and the earlier phases' summaries. The published figure is current-minus-baseline (a get_totals snapshot at function entry) — the step-context semantics exactly, making the terminal summary equal the step report by construction. The #216 markers (cost_incomplete/unpriced_models) and the #605 accounting- error counter survive the publication (run-cumulative, the accepted trade in step_report.py:147-163). serves: I-llr, I-out. de-balances-checked: the #599 write-failure contract preserved (the sequence [pass-start, per-batch x N, final]; the failure tests retarget the final to phase=='done'); the adopted spend enters the tracker ONCE (the injection now before the first publication). Refs #603
…and the injection ordering driven, the count policy stated truthfully (#603) The combined 2d+DU seat's findings, all dispositioned: - THE DELTA PINNED (the high): the exact sequence [0, 10, 10] asserted (the baseline excluded, the FakeAdapter's 10/10 response) — a regression to naive cumulative totals ([110, 120, 120]) now fails. - SITE #3 DRIVEN (the high): a dropped batch's PRE-CONTINUE publication pinned (3 writes for 1 dropped batch; the write carries the 10/10 spend — a bare-continue regression yields 2). - THE INJECTION ORDERING DRIVEN (the medium): a two-pass receipt — the first pass seeds the record, the second's pass-start snapshot carries the adopted spend (a write-then-inject regression reads 0). - the count policy stated truthfully: the per-batch refreshes are DELIBERATELY UNCOUNTED (the #599 counters cover the pass-start/final writes — the persistent case; a per-batch counter would drown the loud signal); the comments claimed the counters carry it — false. - the 'equal to the step report by construction' comment corrected (co-located snapshots over spend-free windows, not a shared boundary); the test-4 docstring corrected (the None path is reachable via a raising get_totals, not tracker=None — the global fallback); the #599 module docstring updated (the write sequence is no longer 'exactly two'); the unused _TotalsTracker + noqa import removed. - the CHANGELOG entry added (the repo's stated convention). 6/6 on the file + the #599/#532/#294 sibling suites green (62 passed); ruff clean. RED at base: 2 failed + 2 passed (receipt red-receipt-638641.log).
…he stale comment corrected, the OSError wording true (#603) The panel + deep-refute findings, all dispositioned: - the orphan #614 CHANGELOG heading (cross-contamination from the sibling PR's insertion) deleted — this PR carries ONLY the #603 entry. - the 'Exception-safe throughout' wording corrected (the publications catch OSError — the write_json failure class; a broader exception escapes). - the final-write stderr comment corrected ('its last per-batch state', not 'pass-start' — post-#603 the per-batch refreshes precede it). 6/6 on the file + the sibling suites; ruff clean.
gadievron
force-pushed
the
fix/issue603-llr-summary-usage
branch
from
September 17, 2026 12:56
a07f83b to
0cda98a
Compare
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 free
to 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.
Fixes #603.
What was wrong
The LLM-reachability checkpoint summaries omitted usage and received no batch-progress refresh — two writes per pass, at start and end, both passing
usage=None(a silent omission). The tracker was already wired into the stage (the omission was caller-side). The stage-local running cost was invisible to any summary-reading consumer mid-stage: in the referenced run, the llr stage spent ~$8 over 41 minutes while any summary-derived cost sum held the app-context step's $0.03. The terminal step report retained the tracked cost — the loss was live visibility, not terminal accounting.The fix
Publication at three points: the pass start (the prior-usage injection now precedes the write, so the first snapshot is internally consistent with
completed=len(adopted)and carries the adopted spend), after every attempted batch — including the failed/dropped/unrecovered-split paths thatcontinuepast the record persistence (the billed-but-empty cases whose spend the live summary exists to show), and at termination. Publication failures (OSError) never mask the pass.The delta semantics (the hunt's correction): the global tracker is cumulative — prior phases' spend included — so a naive totals read would double-count with the step report and the earlier phases' summaries. The published figure is current minus baseline (a
get_totals()snapshot at function entry) — the step-context semantics. The terminal summary equals the step report's delta while the windows between the two snapshots (step_context's start vs this entry) stay spend-free — co-located snapshots, not one shared boundary (a future LLM call between them would need a pin; none exists today). The #216 markers (cost_incomplete/unpriced_models) and the #605 accounting-error counter survive the publication (run-cumulative, the accepted trade instep_report.py).The count policy, stated truthfully: the per-batch refreshes are deliberately uncounted on failure — the #599 counters cover the pass-start/final writes (the persistent-failure case); a per-batch counter would fire N times for one disk-full episode and drown the loud signal.
Evidence
pytest -q tests/test_issue603_llr_summary_usage.py→6 passed) — the exact delta pin[0, 10, 10](the baseline's 110/55 excluded; the FakeAdapter's 10/10 response — a regression to naive cumulative totals[110, 120, 120]fails); the 4-write count for 2 batches; the site-build(deps): bump minimatch from 10.2.2 to 10.2.4 in /libs/openant-core/parsers/javascript #3 receipt — a dropped batch's PRE-CONTINUE publication pinned (3 writes; the write carries the 10/10 spend; a bare-continue regression yields 2); the injection-ordering receipt — a two-pass drive (the first pass seeds the record; the second's pass-start snapshot carries the adopted spend — a write-then-inject regression reads 0); the poisoned-publication pass-completes; the no-get_totals shape. The full suite locally:pytest -q→2 failed, 4208 passed, 34 skipped— the 2 aretest_llm_sdk_contract_floorSDK-pin host drift, identical on master; CI is the authoritative suite. Ruff clean; Semgrep 0 findings.[pass-start, per-batch × N, final]; the failure tests key onphase == "done"(call-count drift cannot retarget the injection); the both-fail counter still counts 2 from the loud sites.The de-balancing check (the intent trace)
serves: I-llr, I-out;de-balances-checked: the #599 counters increment at exactly the same two sites as before (the pass-start/final writes); the resume machinery's reads are per-unit records, never the summary; the phase state machine unchanged (the per-batch writes only refreshcompleted/incompletemid-loop — the Go resume prompt now shows accurate progress instead of the pass-start count).Notes for the reviewer
usagekey is write-only inside the product (the GoSummarystruct has no field; the Python resume reads per-unit records) — the audience is an operator tailing the file or external tooling, the same family parity as the analyzer/verifier/enhancer summaries.llm_reach_checkpoints→ the completedllm-reachabilitystep or it will double-count on resume (the issue's own caveat, carried).