Skip to content

Record time as well as money for every non-free run (#143) - #147

Open
jonfroehlich wants to merge 11 commits into
mainfrom
feat/cost-and-time-accounting-143
Open

Record time as well as money for every non-free run (#143)#147
jonfroehlich wants to merge 11 commits into
mainfrom
feat/cost-and-time-accounting-143

Conversation

@jonfroehlich

@jonfroehlich jonfroehlich commented Aug 19, 2026

Copy link
Copy Markdown
Member

Closes #143 (except the Tillicum back-fill — see Blocked below).

We recorded money for paid APIs and nothing else. This adds the other three quadrants:
time for paid legs, time for free GPU legs, and money+time for cluster compute — and fixes
the defect the comment on #143 identified, which was actively losing data.

The bug that was live

DEFAULT_USAGE_LOG derived from REPO_ROOT, which in a linked worktree is the worktree. A
leg run from a scratch worktree wrote its ledger there and lost it when the worktree was
removed. That is how the #139 claude-opus-5 leg spent $70.41 and left no row.

#119's guard cannot see this, and the reason matters: it proves a log path was accepted,
never that the file it wrote survives. The operator followed the rule and lost the record
anyway, so promoting the rule into CLAUDE.md would not have caught it either.

Both ledgers now resolve through git rev-parse --git-common-dir, which every worktree of a
repo shares, so they all append to one canonical file in the main checkout. An explicit
--usage-log into a worktree warns; each logged leg prints the absolute path and the
running total; and it falls back to the local checkout when git can't answer (a tarball, an HF
clone) — bookkeeping must never be why a run refuses to start.

What else changed

Time is measured. There was no perf_counter anywhere in the harness. score_model now
fills a caller-owned timing dict (model-load / inference / panos actually called) and main
stops the clock in the finally, so a leg that dies partway still reports what it spent.
panos_called counts attempts — a call that raised burnt wall-clock and, on a paid provider,
still billed.

Free legs are no longer invisible. report_usage returned early whenever a detector had no
usage dict — i.e. OWLv2, Grounding DINO, Qwen, Molmo, YOLO, the entire GPU half of the
roster. They now write the same row with paid: false. Two legs deliberately write nothing: a
fully cached re-score, and --models rampnet, which replays committed detections rather than
running a model.

#139's recovered $70.41 is in the ledger, as a row marked kind: "recovered" carrying the
billed total minus what the surviving rows already account for. Cost totals include it —
omitting it under-reported this benchmark's Claude spend by ~200x — while reconciliation never
counts it as a measurement.

A compute ledger. scripts/analysis/slurm_usage.pyanalysis_out/compute_log.jsonl,
priced from a new verified-only COMPUTE_PRICING in pricing.py. Back-filled from a real
sacct pull: 3,991 allocations, 2,684.4 GPU-hours on klone since 2026-07-02
, with the raw
dump committed so it re-derives without a cluster account.

Reconciliation. vertex_usage.py --reconcile compares the committed ledger against Cloud
Monitoring per model. This is the only check that catches a silent no-write. On the real #139
numbers, before the recovery row existed it reports claude-opus-5 UNDER with 11,940,249
input tokens unexplained — the two richmond smoke legs (48,744 tokens) are logged — and with
that row committed it reports ok (1 recovered). A recovered row is still never counted as a
measurement; it is totalled in its own column and subtracted before the verdict, so the one gap
this PR closes is not re-raised as an emergency on every run.

The rule is now repo-level, in CLAUDE.md beside replication and record-in-GitHub: both
units, both cost types, ledgers committed into the main checkout.

Finding: sacct -D is worth 4.35x

96% of our klone allocations end in PREEMPTED (3,780 of 3,991), so the default sacct view —
last incarnation only — discards nearly everything:

#51 YOLO baseline rows GPU-hours
sacct -D 3,857 2,046.9
without -D 27 470.5

It validates the one figure the repo already had. docs/tillicum.md records 496.5
GPU-hours as of 2026-07-30; summing this ledger over jobs ending from 07-24, the running
total crosses 496.5 at 2026-07-29T21:17 — the evening before that line was written.

Blocked

  • Tillicum is not back-filled. Duo-gated and the control master was down. One command once
    it's up; the gap is stated in docs/compute_cost.md next to the numbers, not left implicit.

Overlap

Touches the same section of docs/model_comparison.md as
data/claude-opus-5-nine-splits-139, in a different hunk (that branch narrates the incident;
this one documents the mechanism and the fix). Should merge clean.

Tests

44 new, 1,340 passing (1 skipped), still CPU-only and network-free. The reconciliation tests
use the real #139 numbers; the compute tests are checked against docs/tillicum.md's
independently-derived $4.20 / 4.67 GPU-h.

🤖 Generated with Claude Code (claude-opus-5[1m])

jonfroehlich and others added 5 commits August 19, 2026 06:16
…worktree (#143)

Three defects in the cost accounting, one of them actively losing data.

**Time was not measured at all.** No perf_counter anywhere in the comparison
harness; usage_log.jsonl carried a completion timestamp and no duration, so the
ledger could answer $/pano and never s/pano. The runtimes we have survive only as
prose in docs/model_comparison.md. score_model now fills in a caller-owned timing
dict -- model-load seconds, inference seconds, panos actually put through the
model -- and main stops the clock in the finally, so a leg that dies partway still
reports the time it spent. panos_called counts attempts, not successes: a call
that raised burnt wall-clock and, on a paid provider, still billed.

**Free legs left no record at all.** report_usage returned early when a detector
had no usage dict, which is every local GPU model -- OWLv2, Grounding DINO, Qwen,
Molmo, YOLO. Those are free in API terms and cost real GPU-hours, and they were
invisible to the one ledger we have. Every leg that spends something now writes a
row, with a `paid` flag and the host/GPU it ran on. Legs that spent nothing (a
fully cached re-score, a leg that never loaded) still write nothing, and the
`--models rampnet` arm is excluded outright: it replays committed detections, so
its row would be a zero appended to a committed file on nearly every run.

**The ledger could be written somewhere that does not outlive the run.**
DEFAULT_USAGE_LOG derived from REPO_ROOT, which in a linked worktree is the
worktree -- so a leg run from a scratch worktree wrote its ledger there and lost
it when the worktree was removed. That is how the #139 claude-opus-5 leg spent
$70.41 and left no row, recovered only because Cloud Monitoring still had it.
#119's guard cannot see this: it proves a log path was accepted, not that the file
survives. The default now resolves through `git rev-parse --git-common-dir`, which
every worktree shares, so they all append to one canonical ledger; an explicit
--usage-log pointing into a worktree warns; and each logged leg prints the
absolute path plus the running total, so a run that logged somewhere unexpected is
visible while someone is still watching rather than six weeks later when the
provider's telemetry has aged out.

Falls back to REPO_ROOT whenever git cannot answer (a tarball, an HF clone):
bookkeeping must never be the reason a run refuses to start.

9 new tests, 171 passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
)

The API half of an experiment's cost has been recorded per run since #119. The
compute half was not recorded anywhere -- Tillicum's $0.90/GPU-hour and klone's
GPU-hours lived only as prose in docs/tillicum.md, written by hand, per job, when
someone remembered. Unlike API tokens this half is partly back-fillable from
`sacct`, which is the reason to build it now rather than later: the retention
window is finite.

scripts/analysis/slurm_usage.py parses `sacct` (live on a login node, or a saved
dump via --from-file for a machine that cannot reach the cluster) into
analysis_out/compute_log.jsonl. Verified against the two numbers docs/tillicum.md
already carries, which were arrived at independently of this code: the data-prep
job's 4.67 GPU-hours and $4.20 come back exactly.

Three things it gets right that a hand tally does not:

- **`-D`.** Slurm shows only the LAST incarnation of a requeued job by default,
  and our klone runs live on the preemptable ckpt partition -- the paper's Stage 2
  run was 15 preemptions, 44.7 h of compute across 74.6 h of calendar. Rows are
  therefore keyed on (cluster, job id, start), not the job id, or all but the last
  incarnation's compute disappears.
- **GPU-hours = elapsed x N GPUs**, which is how Tillicum bills: an idle GPU in a
  2-GPU job costs exactly as much as a busy one. The generic `gres/gpu=N` and the
  typed `gres/gpu:a40=N` are the same GPUs reported twice, so the generic count
  wins rather than being added.
- **Idempotent.** Re-running appends only what is new; a job first seen while
  RUNNING is re-appended once terminal, and readers take the last row per key.

pricing.py gains COMPUTE_PRICING under the same verified-only discipline as the
token table: rate, as-of date, source, and Slurm's per-QoS UsageFactor. Tillicum's
`debug` entry carries the unresolved conflict with it -- Slurm bills that QoS at
factor 0 while hyakusage charged the smoke job $0.03 anyway -- so nobody can quote
a bare "free" from it without the source. An unpriced cluster returns None rather
than $0, because "we checked and it is free" (klone) and "we have no rate" are
different statements and only one of them is safe to put in a paper.

canonical_repo_root and the JSONL read/append helpers move to rampnet/ledger.py so
both ledgers share them -- otherwise the new script would have reintroduced the
worktree defect it exists to help measure.

14 new tests; full suite green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ld (#143)

The standing rule lived in docs/model_comparison.md, which reads as a convention
of that one harness, and said nothing about time or about non-free compute. It is
now a repo-level rule beside the replication and record-in-GitHub ones: both units,
both cost types, recorded at run time, ledgers committed into the main checkout.

docs/model_comparison.md's cost section is rewritten to match what the code now
does -- the per-leg timing fields, the free-leg rows and the two legs that
deliberately write nothing, the fourth (compute) ledger, and a subsection on the
worktree defect that is precise about why the #119 guard could not catch it: it
proves a log path was accepted, never that the file survives. Promoting the rule
would not have caught it either, and saying so is the point.

docs/tillicum.md now says hyakusage is a live view rather than a record, and
points at the ledger. docs/replication.md lists compute_log.jsonl with its gap
stated: the sacct back-fill has not been run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…#143)

The comment on #143 is right that nothing in the plan caught the failure that
actually happened. #119's guard proves a log path was ACCEPTED; it says nothing
about whether the file survived, which is how the #139 leg billed $70.41 and left
no row. Layers 1 and 3 measure the same spend two ways and until now nothing
compared them.

`vertex_usage.py --reconcile` totals the committed ledger per model over the same
window as the metric query and prints them side by side. On the real #139 numbers
it reports claude-opus-5 as MISSING with 11,988,993 input tokens unaccounted for,
and says what to do about it: the metric retains ~6 weeks, recovery is per-model
per-DAY, so per-split attribution is gone even on a successful pull.

Deliberately asymmetric: billed > ledger is spend with no record and is called
out; ledger > billed is odd but harmless. Tolerance 2%, because Cloud Monitoring's
daily rows are 24 h windows ending at the query's time-of-day rather than calendar
days, so a leg straddling the boundary moves either way. Free legs carry no token
keys and are skipped -- they have no bill to reconcile against, and counting them
as zero-token models would invent a MISSING verdict.

The comparison functions are pure and tested against the real incident's numbers;
nothing in the new tests touches the network or needs cloud credentials.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…worth 4.3x (#143)

3,991 job allocations since 2026-07-02, written from a real `sacct` pull while the
klone control master was up. The raw dump is committed alongside, so every number
in docs/compute_cost.md is re-derivable from a clean clone with no cluster account
-- the same reason usage_log.jsonl is committed while vertex_usage.py needs cloud
credentials.

**`sacct -D` is worth 4.35x on this workload, and that is the finding.** Slurm
reports only the last incarnation of a requeued job by default, and 96% of our
allocations end in PREEMPTED (3,780 of 3,991) because the work lives on ckpt. The
#51 YOLO baseline is 2,046.9 GPU-hours across 3,857 incarnations with -D, and
470.5 across 27 job ids without it. Any hand tally from a default sacct is short
by that factor and nothing in the output says so.

It validates the one number the repo already had rather than contradicting it.
docs/tillicum.md records 496.5 GPU-hours on the baseline as of 2026-07-30; summing
this ledger over jobs ENDING from 07-24, the running total crosses 496.5 at
2026-07-29T21:17 -- the evening before that figure was written. Reproduces to the
hour, and confirms the original query was duplicate-inclusive.

Two fixes found by running it for real:

- --save-raw did not create its parent directory, so the replication input it
  exists to write failed on a fresh path.
- Rows embedded the whole pricing entry, which at 3,991 rows was 1.7 MB of the
  2.6 MB file. They now carry the rate and its as-of date; the table with its
  caveats stays versioned in pricing.py.
- --cluster silently restamped rows whose Cluster column disagreed, which would
  price another cluster's jobs at the wrong rate and attribute their hours to the
  wrong machine. It now warns. (Nothing was mislabeled here: all 3,991 rows report
  klone, and the 456 H200 allocations are klone's own ckpt-g2 nodes.)

The ledger records every job on the account, not only RampNet's -- 28 sal-*/arch_*
allocations, 1.3% -- deliberately, so the artifact is a complete measurement and
attribution is the reader's call rather than a filter chosen once and baked in.

Gaps stated in the doc: Tillicum is NOT back-filled (Duo, master down), the window
starts where this account's retention does, and the paper's own runs were on
another user's account and are outside this query entirely.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The eight-split claude-opus-5 leg of 2026-08-18 spent real money and left no row,
so every cost table built from usage_log.jsonl under-reported the benchmark's
Claude spend by about 200x -- $0.34 against a true $70.44. Layer 3 recovered the
number the next day; until now it existed only as prose on the #139 branch.

The row is the billed total for that model-day MINUS what the two surviving
richmond smoke rows already account for: 11,940,249 in / 415,751 out / $70.0950.
Written that way the ledger sums to $70.4375 against a $70.44 bill, instead of
double counting the smoke legs. The subtraction, the billed totals it came from
and the recovery command are all in the row, so the arithmetic is checkable
without cloud access.

What made this worth more than an append is the direction the two requirements
pull. Cost totals must INCLUDE recovered spend or they are wrong by the whole
amount. Reconciliation must EXCLUDE it, because a recovered row was read off the
same bill it would be reconciled against -- counting it as "logged" makes the
bill agree with itself and reports ok for exactly the gap that check exists to
find. So ledger_totals_by_model now skips kind == "recovered" explicitly rather
than by the accident of which keys a row happens to carry, and ledger_totals
reports the recovered share separately so it never reads as an as-run number.

A recovered row carries no bundle and no elapsed_s. Recovery is per-model
per-day, so which of the eight splits spent what is permanently gone; that
limitation travels inside the row rather than in someone's memory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jonfroehlich

Copy link
Copy Markdown
Member Author

Deep review

Re-ran everything from the branch head (37b7faa) in a clean detached worktree on Windows. The mechanism is right and the artifacts reproduce. Three independent checks came back clean:

  • Full suite: 1335 passed, 1 skipped (the body says 1,334; the last commit added one).
  • analysis_out/compute_log.jsonl regenerates byte-for-byte from the committed sacct_klone_2026-08-19.txt once recorded_at is re-stamped (3,991 rows, LF only, zero duplicate (cluster, job id, start) keys). Every number in docs/compute_cost.md re-derives: 2,684.4 GPU-h; 3,780/3,991 PREEMPTED; YOLO 3,857 rows / 2,046.9 GPU-h vs 27 / 470.5 without -D = 4.35x; GPU types 2,194 / 1,285 / 456 / 23 / 6 / 1 by allocation; 28 sal-*/arch_* rows = 34.3 GPU-h = 1.28%; RampNet-only 2,650.0; the 496.5 GPU-h running total crosses at 2026-07-29T21:17:10. All exact.
  • canonical_repo_root traced from the main checkout root (git prints .git), a main subdirectory (../.git), a scratchpad worktree root and subdirectory, and a .claude/worktrees/ nested worktree (all print the absolute common dir): every one resolves to D:\Git\RampNet. A non-git temp dir, a nonexistent path, and an empty PATH all fall back to start without raising.
  • Merging this branch onto main, onto data/claude-opus-5-nine-splits-139 (claude-opus-5 on nine splits: the annapolis displacement does not generalise (#139) #146), and onto main + claude-opus-5 on nine splits: the annapolis displacement does not generalise (#139) #146 in sequence: all three clean, no conflict in docs/model_comparison.md.

So what follows is about the half of the design the code does not yet implement, and about text that went stale between commits.


1. Medium — the compute ledger's idempotency is half-implemented: superseded rows are re-appended but never de-duplicated by any reader

slurm_usage.py:38-40 and new_rows (line 173) promise: "A job first recorded while RUNNING is re-appended once it reaches a terminal state; readers take the last row per key." No reader does. summarize (189), print_by_name (206), main's "Ledger now holds" (322) and ledger.ledger_totals (106) sum every row.

Reproduced with the test fixture's own shapes: a job parsed RUNNING at 3,600 s, then COMPLETED at 18,000 s → new_rows correctly re-appends → summarize(existing + fresh) reports 6.0 GPU-h for a 5.0 GPU-h job. The committed ledger is unaffected today only because the 2026-08-19 pull happened to contain no RUNNING rows (one PENDING row, 38640313, at start=Unknown, 0 GPU-h, which will get a new key when it starts and so never collides). The first re-pull after any job is caught mid-run double counts it in every total the script prints.

Fix: a latest_rows(rows) in slurm_usage.py that keeps the last row per row_key, applied in summarize, print_by_name and before the "Ledger now holds" summary. Do not put the dedupe in ledger.ledger_totals keyed on row_key: usage_log.jsonl rows have no job_id, so they would all collapse to one key. Add a test that asserts 5.0.

2. Medium — once a recovery row exists, --reconcile reports the recovered gap as an unexplained emergency, forever

ledger_totals_by_model skips RECOVERED rows by design (vertex_usage.py:117), and the docstring's reason is right: a recovered row must not make the bill agree with itself. But the output does not distinguish "billed more than we measured, and nobody has looked" from "billed more than we measured, and the gap is already in the ledger as a recovery." Run against the committed ledger on this branch with the real #139 bill:

claude-opus-5   billed 11,988,993   logged 48,744   rows 2   UNDER — ledger short
1 model(s) billed more than the ledger records (11,940,249 input tokens unaccounted for).
A missing layer-1 row is an emergency with a deadline ...

That 11,940,249 is exactly the recovered row's input_tokens. The check the PR adds will say "emergency" about the one gap this PR closed, every time it is run inside the window. An operator will learn to ignore it, which defeats the check.

Fix: have ledger_totals_by_model also total input/output of RECOVERED rows per model under a separate key (recovered_input), keep them out of logged_input, and have reconcile compute unexplained = billed − logged − recovered. Verdict ok (N recovered) when the remainder is within tolerance, UNDER only on the unexplained part; print_reconciliation prints the recovered column and totals the unexplained gap. The existing asymmetry and the "never compare the bill to itself" rule both survive: measured rows still have to account for the bill minus what was explicitly written down as unmeasured.

3. Medium-low — the read side of the worktree fix was not done: .env is still loaded from the worktree

Both entry points still read the repo-root .env from the running file's checkout: compare.py:836 (load_dotenv(str(REPO_ROOT))) and vertex_usage.py:54 (load_dotenv(str(REPO))). #146's write-up already documents the consequence: "Run vertex_usage.py from a worktree and it exits with 'no project'". So the tool that recovers a lost spend still cannot find the project id from the place a leg is most likely to have been run, and a Gemini/Claude leg from a worktree finds no key unless the operator copies .env. The PR body says "Both ledgers now resolve through git rev-parse --git-common-dir"; the credentials do not.

Fix: load .env from canonical_repo_root() as well as REPO_ROOT (the loader's setdefault semantics make loading both harmless; worktree first so a deliberate local override still wins). One line in each file, one test using the same git worktree add fixture as test_the_ledger_resolves_to_the_main_checkout_from_a_worktree.

4. Low — REQUEUED is a finished incarnation but is classed as non-terminal

TERMINAL_STATES (line 72) omits REQUEUED. The committed dump has 59 REQUEUED rows, all with an End time and elapsed > 0, carrying 848.5 GPU-h (e.g. 37745358, 29,072 s, 1 GPU). They are counted correctly today because they were first seen in that state. The path that goes wrong: a job recorded while RUNNING, then requeued rather than preempted, is never re-appended, so its row keeps the understated elapsed. Fix: add REQUEUED to TERMINAL_STATES; the (job id, start) key already separates it from the next incarnation.

5. Low — the PR body is stale relative to its own last commit, and one claim in it is wrong

Fix: edit the body (gh pr edit 147 --body-file), moving the recovery row out of "Blocked" and correcting the verdict and the count.

6. Low — two doc lines aged between commits

  • docs/replication.md:20: "back-fill pending" — klone is back-filled on this branch (3,991 rows); only Tillicum is pending. Say which.
  • docs/compute_cost.md:52: rampnet_cosine_rung_135 "still in flight" — true of the 2026-08-19 pull, not of the ledger as a statement. Add "as of the 2026-08-19 pull", or the sentence will read as wrong the day the rung finishes (it has, on analysis/run-b-power-135).

7. Low — CLAUDE.md overstates the token pricing table

Line 66: rates carry "the date it was checked and the page it came from". The COMPUTE_PRICING entries do carry a source; the token PRICING rows carry as_of and a note, no source. Either add source to the token rows or say "date checked" only.

8. Low — one phrase not in the repo's register

rampnet/ledger.py:34: "the difference is load-bearing". Say what it is: "the difference matters for reconciliation".


Decisions for Jon, not fixes

  • The ledger now lands in whatever branch the main checkout has checked out. That is the point of the fix, but it has a consequence the doc does not state: the row is safe from deletion yet is written into an uncommitted working tree on whatever unrelated branch the main checkout happens to be on. The experiment's own PR has to carry that row, so the finishing step is "copy the new line(s) into the experiment branch and commit", not "commit the ledger". Worth one sentence in CLAUDE.md's "A ledger is not a commit" bullet.
  • The recovered row's est_cost_usd (70.095) is the token delta priced at list, not billed_usd − already_logged_usd (70.0975). The ledger sums to $70.4375 against a $70.44 bill. Immaterial, but the row's note says "the BILLED total minus what the surviving rows already account for" and that is true of the tokens, not the dollars. Your call whether to leave it.
  • Whether the report_usage totals line should count free-leg hours. ledger now: … 0.0 h will start climbing once GPU legs write rows; it mixes API wall-clock and GPU wall-clock in one number. Fine as a visibility line, not a cost figure.

What holds up

  • The bug is real and the fix is the right shape. --git-common-dir is the one thing every worktree shares; the fallback to start on any failure means bookkeeping cannot stop a run; the warning on an explicit --usage-log into a worktree fires only when the two roots differ.
  • Timing is measured where it has to be. The clock stops in main's finally, so a leg that raised after paying still records; panos_called increments in a finally inside the loop so continue and break are both counted; a fully cached leg and --models rampnet are the only silent cases and both are tested.
  • sacct -D is worth 4.35x, and the 496.5 cross-check is exact to the minute. The generic-over-typed GPU count is correct for this dump (3,965 rows report both forms, 26 report neither), and no (cluster, job id, start) key collides.
  • Recovered rows are excluded from reconciliation for a correct reason — the fix in finding 2 keeps that reason and changes only what the report says about a gap that is already written down.
  • The compute pricing is sourced (Tillicum page + provisioning email + sacctmgr, with the debug/hyakusage disagreement carried in the row's note rather than dropped).
  • Clean merge onto main, onto claude-opus-5 on nine splits: the annapolis displacement does not generalise (#139) #146, and onto both in sequence.

Fix list

  • [F1] Medium — slurm_usage.py: add latest_rows() (last row per row_key), use it in summarize, print_by_name and before the "Ledger now holds" summary; test that RUNNING→COMPLETED sums to 5.0 not 6.0. Keep ledger.ledger_totals as is (usage rows have no job_id). — files: scripts/analysis/slurm_usage.py, tests/test_slurm_usage.py
  • [F2] Medium — vertex_usage.py: total RECOVERED rows per model separately (recovered_input/recovered_output), keep them out of logged_*, compute unexplained = billed − logged − recovered, verdict ok (N recovered) when the remainder is within tolerance, and print the recovered column and the unexplained gap; update test_a_recovered_row_counts_toward_cost_but_never_toward_reconciliation and test_vertex_usage.py to assert the real committed-ledger outcome (UNDER 11,940,249 without the row; ok-with-recovery once it is written). Update the "Reconcile" paragraph in docs/model_comparison.md and the CLAUDE.md bullet to say the check distinguishes an unexplained gap from a recorded recovery. — files: scripts/analysis/vertex_usage.py, tests/test_vertex_usage.py, tests/test_model_comparison.py, docs/model_comparison.md, CLAUDE.md
  • [F3] Medium-low — load .env from canonical_repo_root() after REPO_ROOT in compare.py:836 and from ledger.canonical_repo_root(REPO) in vertex_usage.py:_load_dotenv; one test with the worktree fixture. Mention in docs/model_comparison.md "The ledger has to outlive the run" that credentials resolve the same way now. — files: scripts/model_comparison/compare.py, scripts/analysis/vertex_usage.py, tests/test_model_comparison.py, docs/model_comparison.md
  • [F4] Low — add REQUEUED to TERMINAL_STATES; assert is_terminal("REQUEUED") in the terminal-states test. — files: scripts/analysis/slurm_usage.py, tests/test_slurm_usage.py
  • [F5] Low — gh pr edit 147 --body-file: move the $70.41 row out of "Blocked" (it is in as kind: recovered, commit 37b7faa), change "MISSING with 11,988,993" to "UNDER with 11,940,249 (the two richmond smoke legs are logged)", and the test count to whatever the suite reports after F1–F4. — files: PR body only
  • [F6] Low — docs/replication.md:20 → "klone back-filled 2026-08-19 (3,991 rows); Tillicum pending"; docs/compute_cost.md:52 → "still in flight as of the 2026-08-19 pull". — files: docs/replication.md, docs/compute_cost.md
  • [F7] Low — CLAUDE.md:66: "the date it was checked and, for compute, the page it came from" (or add source to the token PRICING rows in pricing.py). — files: CLAUDE.md (or scripts/model_comparison/pricing.py)
  • [F8] Low — rampnet/ledger.py:34: replace "the difference is load-bearing" with "the difference matters for reconciliation". — files: rampnet/ledger.py

🤖 Generated with Claude Code (claude-fable-5-1)

…143)

A job first recorded while RUNNING is re-appended once it finishes, so the
ledger holds both rows and summing all of them bills it twice: a 5.0 GPU-hour
job read as 6.0. summarize() and print_by_name() now read through latest_rows(),
which keeps the last row per (cluster, job id, start). REQUEUED joins
TERMINAL_STATES — that incarnation has an End time and a fixed elapsed, and the
next one carries its own start. Regenerating analysis_out/compute_log.jsonl from
the committed sacct dump gives the same 3,991 rows, identical apart from the
recorded_at stamp.

Review findings F1 and F4.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
--reconcile called the whole shortfall an emergency even after the gap had been
found, priced and written into the ledger as a recovery, so #139's 11,940,249
opus tokens would have been reported as unaccounted for on every run inside the
retention window. A recovered row still never counts as logged -- it was read off
the same bill -- but it is now totalled in its own column and subtracted before
the verdict, so the gap it covers reads as "ok (1 recovered)" and only the
unexplained remainder gets UNDER.

Review finding F2.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jonfroehlich and others added 3 commits September 3, 2026 06:22
…ee (#143)

The write side of the worktree fix landed but the read side did not: compare.py
and vertex_usage.py still loaded .env from the running file's checkout, and .env
is git-ignored, so a scratch worktree carries none. A Gemini or Claude leg
launched there found no key, and vertex_usage.py exited with "no project" -- the
tool that recovers a lost spend failing in the situation that loses one. Both now
load the worktree's .env first and the main checkout's second; setdefault means a
deliberate local override still wins.

Review finding F3.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…#143)

replication.md said the compute back-fill was pending after klone had been
back-filled (3,991 rows, 2026-08-19); only Tillicum still is. compute_cost.md
called the cosine rung "still in flight" as a standing fact rather than as of
the pull it was read from. CLAUDE.md said every rate carries the page it came
from, which is true of the compute prices and not of the token prices, which
carry a date and a note. One docstring phrase reworded to plain language.

Review findings F6, F7 and F8.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
)

The row_kind docstring still said a recovered row is never fed back into
reconciliation. It is read there now -- just never as a measurement -- so the
rule is stated as that, with the separate column and the subtraction named. Also
reflows the paragraph the F8 wording change left mid-line.

Follow-on to review findings F2 and F8.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jonfroehlich

Copy link
Copy Markdown
Member Author

Review fixes

  • F1 — fixed in b190da0. latest_rows() keeps the last row per (cluster, job id, start), and summarize() and print_by_name() now read through it, so the "Ledger now holds" summary does too. Without it a job first recorded RUNNING and re-appended once finished was billed twice: 6.0 GPU-hours for a 5.0-hour job. ledger.ledger_totals is unchanged, as the review asked — usage_log.jsonl rows carry no job_id. Regenerating analysis_out/compute_log.jsonl from the committed sacct dump gives the same 3,991 rows, identical apart from the recorded_at stamp the script re-writes, so the committed ledger is untouched.
  • F2 — fixed in 749d345. ledger_totals_by_model totals RECOVERED rows under recovered_input/recovered_output/recovered_rows, still outside logged_*; reconcile verdicts on billed − logged − recovered and reports ok (N recovered) when the remainder is inside tolerance; print_reconciliation prints a recovered column and totals the unexplained gap. Against the committed ledger and the real Run claude-opus-5 (effort low) on the nine remaining splits: the only model that ever beat the top challenger has 1/10 coverage #139 bill this now reads ok (1 recovered) instead of raising the same 11,940,249-token emergency on every run. The rule that a recovered row never makes the bill agree with itself is unchanged — it is not counted as a measurement. docs/model_comparison.md and the CLAUDE.md bullet say so, and the rampnet/ledger.py docstring was brought in step in aa4a142.
  • F3 — fixed in baf0b49. New compare.load_dotenv_for_run() loads .env from the running checkout and then from canonical_repo_root(); compare.main and vertex_usage._load_dotenv both use it. setdefault semantics mean the worktree's own file is read first and still wins. Tested with the same git worktree add fixture, now factored into a _main_and_worktree helper, and noted in docs/model_comparison.md under "The ledger has to outlive the run".
  • F4 — fixed in b190da0. REQUEUED added to TERMINAL_STATES with is_terminal("REQUEUED") asserted. The 59 REQUEUED rows in the committed dump (848.5 GPU-hours) were already counted correctly; the path this closes is a job recorded while RUNNING and then requeued, which was never re-appended and kept its understated elapsed.
  • F5 — done: PR body edited. The recovered row moved out of Blocked, the reconciliation verdict corrected, and the test count updated. The verdict sentence states both cases, because F2 changed what the check says: UNDER with 11,940,249 before the recovery row existed (the two richmond smoke legs, 48,744 tokens, are logged), ok (1 recovered) with it committed.
  • F6 — fixed in fbba3b7. docs/replication.md now says klone back-filled 2026-08-19 (3,991 rows), Tillicum pending; docs/compute_cost.md dates the cosine rung's status to the 2026-08-19 pull.
  • F7 — fixed in fbba3b7. CLAUDE.md now says "the date it was checked and, for compute, the page it came from" — only COMPUTE_PRICING carries a source.
  • F8 — fixed in fbba3b7, with the surrounding paragraph reflowed in aa4a142.

Nothing was skipped; every finding held up when checked against the code. Full suite: 1,340 passed, 1 skipped (was 1,335/1), CPU-only and network-free.

🤖 Generated with Claude Code (claude-opus-5)

jonfroehlich added a commit that referenced this pull request Sep 4, 2026
…e bundle truncation travels with its numbers (#135)

Two review passes on this PR (2026-08-18 and 2026-09-03) left thirteen findings open,
and the gate commit 4171a6e added an artifact that contradicts the document it sits
beside. This is the fixes pass those reviews said would follow.

THE HEADLINE CLAIM WAS READ AGAINST THE WRONG BAR

The pre-registration says |delta| / s.e. >= 1.96 per pair. The Results section instead
compared the largest delta to #138's MDE of 0.0063, which is 2.80 x s.e. at 80% power --
1.43x looser -- and concluded "tied at every epoch". Those are different bars: an effect
below the MDE is not thereby non-significant. run_b_gate_135.json, added in 4171a6e,
already records the consequence: epoch 7's +0.0042 gives |z| = 1.45 to 2.64 over the
measured s.e. bracket, and the artifact flags it as clearing 1.96 at the favourable end.

The accurate statement is narrower and is now what the document says. The pre-registered
primary (epoch 8, +0.0030, |z| = 1.02 to 1.86) is a tie at both ends of the bracket --
which is the reading the decision rests on and which does not depend on which s.e. inside
it is chosen. Six of the other seven epochs are ties at both ends. Epoch 7 is undetermined,
not a tie, and resolving it needs the cosine arm's per-panorama detections, which are not
committed. The deviation from the pre-registration is stated where the numbers are, along
with the fact that the pre-registration's own "Exact commands" could not have run the test
as written.

THE DECISION IS UNCHANGED, AND WAS NEVER LOAD-BEARING ON THE TIE

Run B at n=1 cannot be told apart from a seed draw at any length. That argument holds
whether or not epoch 7 separates, which is why overstating the tie bought nothing.

THE NINE CITY BUNDLES ARE CUT AT 0.55, AND EVERY UNPAIRED ROW INHERITS IT

Measured: all nine hold 0.0% of their detections below 0.55 (minima 0.5501-0.5607) while
manual_gold reaches 0.0501 with 28.8% below. So "the #54 operating point of 0.30" is a
no-op on nine of ten splits, max-F1 there peaks on an already-truncated curve (max_f1 ==
f1 exactly on eight of nine cities and on POOLED cities), and the pooling gain is
understated. Measured against the one untruncated arm in committed data
(--reference rampnet_1pass, all ten splits to 0.05): POOLED-all MDE 0.0105 against
manual_gold's 0.0121, a 14% gain rather than 7%. That arm is single-pass and missing seam
detections, so 14% bounds the correction rather than being it; there is no clean
uniform-0.30 RampNet arm in the repo, which is now stated beside the number. The
conclusion is unchanged on either reading: pooling is not a lever.

The artifact now records reference_min_confidence and protocol_threshold_binds per split
and truncated_members per pooled row, so this is visible rather than inferable -- it sat
undetected through two reviews precisely because the only evidence was indirect.

benchmark_power_135.json regenerates with ZERO changed values: the additions are new keys
plus self_pair's max-F1, which is now null. Every number the documents quote still stands.

REST OF THE FIX LIST

- self_pair max-F1 was identically zero by construction (max-F1 re-picks its own
  threshold, so shifting the read-out point cannot move it). It read as a measured zero
  with zero uncertainty in all 24 rows; it is now null with a note. That block bounds F1.
- The headline table's rows 1 and 4 were F1 standard errors under a max-F1 header
  (0.0042/0.0117 and 0.0039/0.0109). Now max-F1: 0.0041/0.0114 and 0.0039/0.0108.
- Four-decimal drift against the artifact, all corrected: +0.1119 -> +0.1117,
  +0.1492 -> +0.1489, c = 123 -> 124, "9 of 356" -> "10 of 357", discordance ranges to
  2.50-4.26 / 2.50-6.66 / 6.48-6.66, MDE upper 0.0081 -> 0.0082, and the rung doc's
  0.961/3.979 -> 0.962/3.980 (the trap that file's own provenance note warns about).
- The recall table printed six of the nine pairs the script computes, and the three it
  dropped included the only "not resolvable" verdict. All nine now print, and the table's
  verdicts carry the conditional the max-F1 table already had: they rest on a discordance
  range taken from three cross-detector pairs, none of them epoch-vs-epoch.
- "Every derived number in this document is in benchmark_power_135.json" was false. Four
  classes of exception are now named, and test_committed_json_matches_the_doc_headline_
  numbers pins every headline value so the prose cannot drift again.
- dump_peaks_from_cache.py hardcoded a run_a_epoch_N label whatever --summary-csv it was
  given, so a second arm's dumps were either invisible to the reader or overwrote Run A's
  committed ones. Adds --label-prefix; --verify now fails loudly on a fingerprint the
  summary does not contain instead of silently checking nothing; exclude_border is read
  from the extractor rather than restated; the docstring no longer recommends the one
  output directory the same file's help text forbids.
- stage2_epoch_curve_84.md's status block still stated the superseded curve shape. It now
  carries the #135 amendment: the plateau is 2-6, not 2-8.
- The rung's pre-registration claimed nothing above Results had been edited; two commits
  had. Says what was edited and what was not.
- Provenance gaps stated rather than implicit: the 21 restarts and 35.06 h come from
  sacct -D with no dump committed (a clean clone can count 18 event files, 11 with steps);
  the 560.9 GPU-hours are pending #147's compute_log.jsonl; the LR verification block gets
  its one-line reproduce command; whether the rung's eval cache survived is unknown and is
  now said so.
- The reopening condition pointed at docs/seed_variance_51_135.md, which is not on this
  branch; it points at PR #155. #151's +0.115 F1 is marked as not re-derivable here. The
  "~0.01 measured but not attributable" threshold the decision's third reason turns on is
  marked as a working assumption rather than a measurement.
- Run A's epoch-7 max-F1 differs between two documents in this PR (0.9110 from
  summary.csv, 0.9107 post-#140). Both arms were scored under the pre-#140 matcher so the
  comparison is internally consistent; that is now said, in both places.
- Dead Scored.gt_pano removed. observed_and_se(paired=...) asserts shared panorama order,
  the way mcnemar already asserts its own. The test's (prediction_confidence(p) or -1e9)
  mapped a legitimate 0.0 confidence to -1e9; it checks for None.
- Deliberately NOT changed, with the reason recorded in the code: the single Generator
  threaded through every group, and the redundant res_a/res_b bootstraps. Either change
  shifts the draw stream and moves every standard error these documents quote, for no gain
  in correctness. The --splits caveat is documented instead.
- train.py's epoch-boundary checkpoint window is described in the ResumeSkipSampler
  docstring rather than fixed: latest_checkpoint.pth is written after validation, so a
  preemption there loses the whole pass (the committed events show it happening twice).

Suite: 1,373 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Record time as well as money for every non-free run: the harness times nothing, and the free-GPU half of the roster logs nothing at all

1 participant