Record usage-gate pauses as typed skip rows in metrics/runs.jsonl, published hourly - #163
Conversation
A usage-gate pause (exit 10) left NO runs.jsonl row — the runners logged one line and exited 0 — so the rain-org-health metrics page, which draws runs from that file on main, rendered a paused stretch as a dead cron (#160). The pause path now mirrors the preflight-abort just below it: an empty trace through `pr-review-report run-metrics`, so no second place knows what a runs.jsonl line looks like. The row carries the typed contract a consumer keys on — `"skipped": "usage-gate"` plus `"skipReason"` holding the gate's own PAUSE line verbatim — beside the standard fields (runId, role, model, exitCode 10 = the gate's own, as the preflight row records preflight's 12) and a typed `outcome: "skipped"`, since 10 over an empty trace would otherwise classify `error`. Both fields are ABSENT, not null, on every other row. A config REFUSAL (exit 2) is NOT a skip: it stays a loud abort and writes no row. Visibility during the pause is the point: the hourly refresh-human-queue cron — data-only, never usage-gated, already committing straight to main — now stages metrics/runs.jsonl beside the snapshot and publishes when either moved, so a skip row reaches origin/main within ~an hour of its gated tick even when nothing else runs for days. The history rollup stays gated on the SNAPSHOT having changed, so a metrics-only publish appends no {ts, counts} line. No back-fill: the ~9 log-only paused ticks before this stay log-only; rows start at deploy. Closes #160 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WalkthroughUsage-gate pauses now produce skipped metrics rows with exit code 10 and empty traces. Configuration refusals remain unrecorded with exit code 2. The hourly refresh cron publishes metrics-only changes. ChangesUsage-gate metrics flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Runner
participant usage-gate
participant pr-review-report
participant metrics/runs.jsonl
participant refresh-human-queue
Runner->>usage-gate: Evaluate usage gate
usage-gate-->>Runner: PAUSE with exit code 10
Runner->>pr-review-report: Record skipped run and empty trace
pr-review-report->>metrics/runs.jsonl: Append skip row
refresh-human-queue->>metrics/runs.jsonl: Detect metrics change
refresh-human-queue->>refresh-human-queue: Commit and push metrics ledger
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@refresh-human-queue.sh`:
- Around line 120-127: Update refresh-human-queue.sh lines 120-127 to use git
diff --quiet HEAD -- for both snapshot_changed and metrics_changed probes, so
staged and unstaged changes are detected. In
pr-review-report-rs/tests/refresh_human_queue.rs lines 496-541, add a regression
case that stages the appended metrics/runs.jsonl skip row with git add before
the tick, then verifies the tick commits and pushes the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8ec950ff-0461-4890-bb5a-5a4252984d7f
📒 Files selected for processing (7)
README.mdcampaign-run.shpr-review-report-rs/src/main.rspr-review-report-rs/tests/refresh_human_queue.rspr-review-report-rs/tests/usage_gate_skip.rsrefresh-human-queue.shreview-run.sh
CodeRabbit: a tick that staged its files and then failed to commit (lock contention, full disk) leaves the change STAGED, and `git diff --quiet -- <p>` reads staged-only content as unchanged — every later tick would then skip the publish, and during a pause the skip rows would sit staged until unrelated queue churn rescued them. Both probes now diff against HEAD. Regression test drives the failure shape (row staged before the tick) and failed on the pre-fix code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Reviewed eb480fe: ready — typed skip rows (outcome:skipped, absent-not-null fields, refusal writes nothing) emitted through the one row-shape source, refresh-human-queue becomes the metrics committer awake mid-pause with either-file publish and snapshot-gated history, CodeRabbit's real index-vs-HEAD Major fixed TDD-style, four mutants killed, base-discrimination verified. |
Closes #160
A usage-gate pause (exit 10) left NO row in
metrics/runs.jsonl— the runners logged one line and exited 0 — so the rain-org-health metrics page, which fetches that file raw from main, drew the current nine-plus-tick pause as a dead stretch indistinguishable from a broken cron. Human report, verbatim: "roh metrics page is not showing runs that rate limited on usage, so we just have long dead stretches in the chart."The row (the contract a renderer builds against)
The pause path mirrors the preflight-abort just below it — an empty trace through
pr-review-report run-metrics, so no second place knows what a runs.jsonl line looks like. One real emitted row (dev build at this head):{"trace":"/tmp/empty-trace-160.jsonl","stage":"final","toolCalls":0,"startupToolCalls":0,"startupPct":0.0,"wakeupCalls":0,"firstMutationIndex":null,"bootMs":null,"ttlMs":null,"startupMs":null,"durationMs":0,"numTurns":0,"tokensIn":0,"tokensOut":0,"cacheRead":0,"cacheCreation":0,"costUsd":0.0,"rateLimits":{},"unreadableFiles":[],"commandsNotFound":[],"missingTools":[],"infraDown":false,"infraReason":"","infraRootCause":"","runId":"20260731T130001Z","role":"producer","model":"claude-fable-5","exitCode":10,"outcome":"skipped","skipped":"usage-gate","skipReason":"PAUSE: 91% of the weekly budget used (endpoint) — at/over the 90% ceiling"}(In production
traceis$RUNDIR/<runId>.jsonl— the empty trace the runner creates for the row, same relationship every other row has with its trace.)"skipped": "usage-gate"+"skipReason": "<the gate's own line, verbatim>"— the typed discriminant a consumer keys on. Both fields are ABSENT (not null) on every other row, so pre-usage-gate pauses leave no runs.jsonl row — paused stretches read as a dead cron everywhere downstream #160 records read unchanged.exitCodeis the GATE's 10, the same way the preflight-abort row records preflight's 12; the runner itself still exits 0 (a pause is not a failure).outcomeis a typed"skipped"— without it, exit 10 over an empty trace classifieserror, which is exactly the dead-cron rendering this exists to correct.claprequiresboth ways), so no runner edit can half-supply the pair.Visibility: how a skip row reaches main within the hour
metrics/runs.jsonlhad NO committer: the runners append and never push ("committed periodically" meant hand-runchore(metrics):commits, last on 2026-07-28). During a pause that is fatal — the paused runs are the only writers, and nothing commits.Chosen mechanism: the hourly
refresh-human-queuecron now stagesmetrics/runs.jsonlbeside the snapshot it already commits straight to main, and publishes when EITHER file moved. Justification:git addby one tracked file is the smallest possible delta.{ts, counts}history rollup stays gated on the SNAPSHOT having changed — a metrics-only publish appends no history line, or an idle queue would grow one identical rollup per skip row.Side effect at deploy, deliberate: the first hourly tick also publishes the ordinary run rows accrued locally since the last hand commit. That is the file's normal content finally getting its committer — NOT a back-fill of skip rows. The ~9 log-only paused ticks stay log-only; skip rows start at deploy.
QA
skip_row_tests::{a_skip_row_carries_the_gate_and_its_reason_verbatim, an_unskipped_row_omits_both_skip_fields_entirely, a_skip_classifies_skipped_where_the_bare_exit_code_would_read_error, a_refusal_exit_is_never_skipped_without_the_flag}(contract),run_metrics_trace(CLI: skip form parses; either flag alone is refused),tests/usage_gate_skip.rs::{a_paused_producer_tick_writes_one_skip_row_and_exits_zero, a_paused_vetter_tick_writes_one_skip_row_and_exits_zero, a_refused_producer_tick_aborts_loudly_and_writes_no_row, a_refused_vetter_tick_aborts_loudly_and_writes_no_row}(real scripts as processes, onlyusage-gatestubbed —run-metricsis the REAL binary),refresh_human_queue::{a_metrics_only_append_publishes_without_a_history_line, a_staged_but_uncommitted_append_is_still_published}(real git remote; the second is the CodeRabbit round — probes now diff against HEAD so a change left staged by a failed commit still publishes, and the test failed on the pre-fix code). Verified on base (main @ 9785128, new test files copied onto an untouched base clone):a_paused_producer_tick…anda_paused_vetter_tick…FAIL (base writes no row) anda_metrics_only_append…FAILS (base commits nothing, origin unmoved); the unit/CLI tests cannot compile on base (the flags don't exist — the contract is new surface). The two refusal tests pass on base by design: they pin the boundary that must NOT move, and are the named killers of the swap mutant below.final_recordskip-insert dropped (let _ = skip;) →a_skip_row_carries_the_gate_and_its_reason_verbatim+a_paused_producer_tick_writes_one_skip_row_and_exits_zeroFAILED; (2)classify_outcomeskip-precedence dropped (let _ = skipped;) →a_skip_classifies_skipped_where_the_bare_exit_code_would_read_errorFAILED; (3) skip/refusal SWAPPED incampaign-run.sh(row on exit 2, bare abort on 10) →a_refused_producer_tick_aborts_loudly_and_writes_no_row+a_paused_producer_tick_writes_one_skip_row_and_exits_zeroFAILED; (4) fields inserted asnullwhen unskipped →an_unskipped_row_omits_both_skip_fields_entirelyFAILED. Each reverted; full suite green at head.skipReasonfixture is the gate's real ceiling-pause line fromusage_gate_decide(em-dash and all), asserted byte-identical through the runner's$_ugcapture; expected JSON asserted as hand-written literals, never re-derived throughfinal_record; the preflight-abort path is the modeled row shape, per the runners' own stated principle.run-metrics, (2) both runners' exit-10 paths emit, (3) a skip row visible on main within ~1h during a long pause, (4) refusal ≠ skip, (5) no back-fill. Covered: 1 (contract + unit/CLI tests), 2 (both runners, role producer/vetter, behavioral tests), 3 (hourly refresh-human-queue stages the file, publish-on-either-change, behavioral test on a real remote), 4 (exit 2 aborts loudly, no row, tested on both runners), 5 (this diff touches no data file — rows start at deploy).🤖 Generated with Claude Code