fix(review-runs): page the run census, so a day's window isn't the last hour - #886
Conversation
…st hour Step 1's census call takes the endpoint's default 30-item page and never follows the rest. Runs come back newest-first, so on a busy repo the "past 24 hours" list is the most recent hour: 30 of 110 tend-mention runs today, spanning 68 minutes. The truncation is silent — 200 from the API, a successful --jq, and a census reported as complete — and it under-counts the recurrence evidence the gates run on. --paginate on both calls, per_page=100 to hold the page count down, and a total_count cross-check when a count lands on a page boundary.
tend-agent
left a comment
There was a problem hiding this comment.
The diff is correct — I reproduced both sides on this repo over the current 24h window: the old form returns 30 tend-mention runs, the new form returns 114, matching .total_count.
Two adjacent gaps in the same census, both outside this diff:
token-report.sh truncates the same window at 100. Step 2 runs token-report.sh 24, which fetches per workflow with gh run list --created ">=$SINCE" --status completed --limit 100. Measured over the same window: tend-mention returns 100 against a true 114. So after this PR, Step 1 reports 114 runs and Step 2's token totals cover 100 of them, with nothing in the output marking the shortfall — the same silent-truncation shape #838 is addressing in list-recent-runs.sh (raise the limit, warn when a workflow comes back exactly at it). Distinct enough from the census pagination to be a follow-up rather than folded in here.
Step 1's startswith("tend-") filter drops review-reviewers — 21 completed runs in the current window, none of them in the census. Step 2 documents the escape hatch ("Pass additional workflow prefixes to include non-tend-* workflows … Check the repo's running-tend skill for the list"); Step 1 hard-codes the single prefix, so the two steps disagree about what the fleet is on this repo. Pre-existing and out of scope for an atomic PR — worth naming because the premise here is that the census should cover the fleet.
|
Both deferred gaps are now tracked in #888, with the measurements re-taken on the current window: |
…s hit (#887) `token-report.sh` fetches each workflow's runs with `--limit 100`. `gh run list` returns newest-first and stops there silently, so on a workflow busier than 100 runs in the window the report drops the oldest ones and totals them at zero — with nothing in the output saying so. ## Measured on this repo Current 24 h window: ``` $ gh run list --workflow tend-mention --created ">=$SINCE" --status completed --json databaseId --limit 100 | jq length 100 $ gh run list --workflow tend-mention --created ">=$SINCE" --status completed --json databaseId --limit 400 | jq length 116 ``` 16 of 116 `tend-mention` runs (14%) are outside today's report, and their tokens are simply absent from the totals. `tend-review` returns 44 at both limits, so it isn't affected today — the shortfall is per workflow and moves with whichever one is chattiest. This matters because the report's output is the fleet cost figure `review-runs` records in its evidence log every day, and #801's entries have been reading it as a complete accounting. Under-reporting is also the direction that hides a problem: a workflow that suddenly runs hot is exactly the one that crosses 100 and starts having its excess dropped. ## Change Three things, all small: - **`--limit 1000`.** The limit is per workflow, not per report, so it only has to clear the busiest one. 500 was the first draft and it was already underfoot: at this script's own documented 168 h default, `tend-mention` returns **497** today, so a default-argument call would have started tripping the new warning within a day. Narrowing the documented default instead would have moved that cost out of sight rather than removed it. 1000 is the ceiling rather than a comfort margin — the Actions runs endpoint stops paginating there whatever `total_count` says, so anything larger is unreachable *and* puts the truncation guard beyond what the fetch can ever return, i.e. buys no runs and costs the warning. It is also the value that makes `-ge` trip exactly at the ceiling. - **Warn on an exact hit.** A count landing on the limit is the only symptom of truncation visible without re-querying `.total_count`, so the loop says so on stderr rather than trusting it. - **Warn on a failed fetch.** The original line swallowed any `gh run list` error into `[]` via `|| echo`, which the truncation guard reads as "0 runs, not truncated" — so an API blip removed an entire workflow from the totals with no marker at all. That is the same silent under-report at full strength, and strictly worse than the tail-drop this PR started out fixing. Branching on the exit status covers it. Warning rather than exiting, unlike the sibling in `list-recent-runs.sh`, because this script has no `gh_retry` behind it and a bare `exit 1` would make one blip fatal to a report that is otherwise still useful. Raising a limit alone would only move the cliff. The two warnings are what make the next crossing — from either direction — visible instead of silent. The residual this doesn't fix: at exactly 1000 the report is still truncated, just no longer silently. Getting the full set past the ceiling needs `.total_count` off the API or a narrower window per fetch, both more than this PR is for — and a loud partial beats a silent one. Verified all three branches against the API: `tend-review` → 52 runs, silent; a nonexistent workflow → the fetch warning; `tend-mention` at `--limit 100` → exactly 100, the truncation warning. The ceiling and the guard's reachability at the new constant, measured on this repo: ``` $ gh api ".../actions/workflows/250047576/runs?status=completed&per_page=100&page=10" --jq '.workflow_runs | length' 100 $ gh api ".../actions/workflows/250047576/runs?status=completed&per_page=100&page=11" --jq '.workflow_runs | length' 0 $ gh run list --workflow tend-mention --created ">=2000-01-01T00:00:00Z" --status completed --json databaseId --limit 2000 | jq length 1000 $ gh run list --workflow tend-mention --created ">=2000-01-01T00:00:00Z" --status completed --json databaseId --limit 1000 | jq length 1000 ``` `total_count` for that workflow is 3265, so the 1000 is the endpoint's ceiling and not the window running out. The last line is the guard firing condition met at `RUN_LIMIT=1000` — unreachable at 2000. Patched script runs clean end to end (`token-report.sh 2 "review-"`, exit 0, 48 runs, no warnings), and `shellcheck` is clean. ## Provenance and scope Found by the review on #886 — that PR fixes the same silent-truncation shape in `review-runs`' Step 1 census (30 of 110 runs, a 68-minute view of a 24-hour window), and the reviewer measured this adjacent case in Step 2 while checking it. Kept separate because it's a different file and a different fetcher. #838 is doing the equivalent work for `list-recent-runs.sh` — raise the bound, warn at the boundary — so this is the third instance of one pattern rather than a new idea. Not a dedup hit: different script, different call, no overlap in the diff. ## Gate assessment - **Evidence level: High.** Reproduced directly against the API, twice, at two limits. Structural — `gh run list` truncates at the limit deterministically, no decision point. - **Change type: targeted fix** — one constant and a four-line guard. Normal Gate 1 bar, cleared. - **Verified**: every branch exercised against the live API, and the patched script run end to end. - **Revised twice after review** on this PR. Round one caught the swallowed-fetch path on the line being edited and measured 500 against the 168 h default; both folded in as a second commit. Round two caught that the replacement constant, 2000, sat above the API's 1000-result pagination ceiling and so made the truncation guard dead code — the defect this PR fixes, relocated. Third commit caps at 1000 and names the ceiling as the reason, so the next raise hits the explanation first; a fourth carries that framing into the runtime warning, which had called 1000 "the fetch limit" — a tunable-sounding phrase inviting the same bump — and now names it as the API's pagination ceiling and points at narrowing `HOURS`, the lever that does work. --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
…889) Step 1 of `review-runs` enumerates the fleet with a hard-coded single prefix, while Step 2 documents the opposite — that non-`tend-*` workflows using the tend action are in scope and their prefixes come from the repo's `running-tend` skill. The two steps therefore disagree about what the fleet is, and Step 1 wins: a workflow outside the `tend-` prefix is never classified for duration, never near-timeout-checked, and never reaches Step 3's log analysis. Nothing in the output marks the omission. ## Measured on this repo Over a 24 h window (`SINCE=2026-08-06T08:42:07Z`), `review-reviewers` — which runs the tend composite action but is not named `tend-*`, and which tend's own `running-tend` overlay already lists as an extra prefix for Step 2 — has 21 completed runs that Step 1 never sees: ``` $ gh api repos/max-sixty/tend/actions/workflows --jq '.workflows[] | select(.name | startswith("tend-")) | .name' | grep -c '^review-reviewers$' 0 $ gh api "repos/max-sixty/tend/actions/workflows/250009605/runs?created=>=$SINCE&status=completed&per_page=1" --jq '.total_count' 21 ``` ## Change Replace the inline `startswith("tend-")` with a `PREFIXES` array defaulting to `("tend-")`, matched as an anchored alternation. Step 2's sentence now points at the same list rather than describing a parallel one, so a single repo-level source drives both steps. Behaviour is unchanged for an adopter with no extra prefixes — the default array reproduces the old filter exactly. Running the edited Step 1 block verbatim: ``` # as written, default PREFIXES=("tend-") tend-review 30, tend-mention 30, tend-notifications 26, tend-ci-fix 9, tend-triage 6, tend-nightly 1 # with tend's running-tend prefix list, PREFIXES=("tend-" "review-") tend-review 30, tend-mention 30, tend-notifications 26, review-reviewers 20, tend-ci-fix 9, tend-triage 6, tend-nightly 1 ``` The `30`s in that output are #886's separate bug (the unpaginated endpoint capping at a page), still live on `main` — this change doesn't address it and doesn't depend on it. ## Scope and conflict note This is part 2 of #888. Part 1 of that issue — `token-report.sh` capping the same window at `--limit 100` — is already fixed by #887, so nothing here touches that script. #886 edits the same two `gh api` lines in this block to add `--paginate`. The changes are independent in intent but overlap textually, so whichever lands second needs a trivial rebase; the two edits compose (a `PREFIXES`-driven filter on a paginated fetch). Not included: an explicit prefix-list line in tend's own `.claude/skills/running-tend/SKILL.md`. Its "Usage analysis" section already names `review-` as the extra prefix, so this repo's Step 1 resolves correctly today; adding a dedicated line there is a separate overlay concern. ## Gate assessment - **Structural.** The filter is fixed in the recipe text, so it excludes identically on every run. No decision point. - **Evidence: High.** Exclusion reproduced directly against the API, and the fix verified by executing the edited block verbatim at both prefix lists. - **Change type: targeted fix** — one code block plus one cross-reference sentence, no new sections. --- Refs #888 ## Follow-up commits `3d4fcda` — review on this PR found the same disagreement one paragraph later: the near-timeout instruction resolved a workflow's `timeout-minutes` by globbing `.github/workflows/tend-*.yaml`, which doesn't match the very workflow the widened census now admits. It now reads the workflow's own file. `f823494` reworded the `PREFIXES` comment from rationale into an instruction. --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
# Conflicts: # plugins/tend-ci-runner/skills/review-runs/SKILL.md
…t now-24h (#939) ## Problem `review-runs` Step 1 opened its window with `date -u -d '24 hours ago'`, which resolves when the *agent* runs the command — the run's start plus container boot and skill loading. The predecessor started at *its* own start time, earlier by whatever drift it saw, so the window opened strictly after the predecessor and dropped every run in the gap. The gap is never zero and never negative, so the census systematically under-counted rather than flaking. Step 2 clipped the same band independently, and by a wider margin, because `token-report.sh 24` measured 24 hours back from its own later invocation. Reproduced on this repo, independently of the reporter's measurement on `max-sixty/cargo-affected`. Yesterday's `review-runs` run started `2026-08-09T08:01:45Z`; today's ([31369350870](https://github.com/max-sixty/tend/actions/runs/31369350870)) started `08:16:33Z`, so its window opened no earlier than that — a band of at least 14m48s, and wider by however long boot and skill loading took. Eight runs sat in it: ``` 2026-08-09T08:03:08Z review-reviewers 31302588340 2026-08-09T08:12:32Z ci 31302963325 2026-08-09T08:12:32Z tend-review 31302963274 2026-08-09T08:13:08Z ci 31302989157 2026-08-09T08:13:08Z tend-review 31302989224 2026-08-09T08:14:21Z tend-mention 31303039387 2026-08-09T08:16:03Z tend-mention 31303111366 2026-08-09T08:16:11Z tend-mention 31303117289 ``` Six of those are full agent sessions. The loss is also biased toward the runs that matter most to the audit: what happens in the minutes right after `review-runs` starts is largely the work that run triggers — its own PR getting reviewed, a mention firing on that review — and that is exactly the band the next run cannot see. This is distinct from #886 and #888, which decide how much of the window is *visible*. A fully paginated census of a window that opens too late still misses these runs, and Step 2 — the fallback that recovered the truncated runs in #888 — has the same defect. ## Solution Anchor `SINCE` on the predecessor's `created_at` so consecutive windows tile, and derive Step 2's lookback from the same anchor instead of a literal `24`. Three details beyond the reported shape: - **`status=success`, not `status=completed`.** The API's `completed` includes `failure` and `cancelled`, so anchoring on it would permanently strand the band of a predecessor that died before its census. Reaching for the last *successful* run covers that band on the next pass. - **Clamp a stale or missing anchor at 49h.** A fresh repo has no predecessor, and after an outage a week-old anchor would pull in a week of runs. 49h lets the window absorb one skipped day without unbounded growth; Step 5 already dedups findings a widened window sees twice. - **Exclude `$GITHUB_RUN_ID`.** A re-run attempt of the current run can surface as completed, and anchoring on itself collapses the window to zero. The workflow id is derived from the run rather than the file name. ## Testing No test harness covers skill text, so both recipes were extracted verbatim from the edited file and executed here: - Anchored form, simulating today's `review-runs` run: `SINCE=2026-08-09T08:01:45Z` — the predecessor's start, exit 0. - Empty-predecessor branch: falls through to the `25 hours ago` default, exit 0. - Stale anchor (`2026-08-01`): clamps to the 49h floor. - `HOURS` derivation from that `SINCE`: `25`, exit 0. The `if [[ ... ]]; then ... fi` form is deliberate over `[[ ... ]] && SINCE=$FLOOR`: the latter exits 1 when the test is false, which the agent's Bash tool reports as a failed block. <details><summary>`review-reviewers` — already anchored, no change needed</summary> The report flagged `review-reviewers` as possibly sharing the defect, unmeasured. It doesn't: it gets its window from [`list-recent-runs.sh`](https://github.com/max-sixty/tend/blob/fd23cd06b8850c8db6777a57f547931b86575f5c/plugins/tend-ci-runner/scripts/list-recent-runs.sh), which anchors the completion window to the most recent *intended* cron tick rather than to `now`, precisely so scheduler drift can't shift the window relative to actual start time. #845 extends that anchoring to every-N-hours crons. </details> ## Review follow-ups Two changes landed after review, both on this branch. **The census is now on completion, not creation.** Anchoring alone made the *creation* windows tile, but Step 1's heading promised completed-since-the-predecessor and the query still filtered `created>=$SINCE`. A run created before the anchor and still in progress at the predecessor's census was dropped there by `status=completed` and dropped again here — censused by nobody, and that is precisely the long-running class Step 3 exists to hunt. Step 1 now over-fetches by `created` and filters on `updated_at`, the shape [`list-recent-runs.sh`](https://github.com/max-sixty/tend/blob/74328fc2898f386fd4569fcfdd392a88db92da80/plugins/tend-ci-runner/scripts/list-recent-runs.sh) already uses. The over-fetch floor is 24h — a whole run lifetime, not the 6h hosted-runner job cap. `created_at` starts at queue time, and a `cancel-in-progress: false` group can hold a run queued for hours before execution begins; the longest completed run on this repo in the last three days spans 819 minutes wall-clock ([31281456692](https://github.com/max-sixty/tend/actions/runs/31281456692)), which a 6h floor still misses. Diffing the two floors against the live API in one pass, 24h is a strict superset that recovers exactly that run (242 -> 243). Also confirmed GitHub's `created=` filter honors time-of-day rather than rounding to the date, which the sub-day anchor depends on. **Step 1's lead comment is trimmed** per CLAUDE.md's skill-authoring brevity rule; the argument lives here, not in a file loaded into every session. **The anchor is now persisted to a file.** Step 1 set `$SINCE` in one Bash tool call and Steps 2 and 4 read it in others, where shell state is gone. Neither failed loudly: `date -d ""` resolves to today's midnight rather than erroring, so Step 2's `HOURS` silently became hours-since-midnight — 9 rather than 25 on this cron, pricing a *narrower* band than the literal `24` it replaced. Step 4's `closedAt > "$SINCE"` compared against the empty string, which sorts below every timestamp, so that cross-check admitted every closed bot PR ever (62 here against 10 for the correct anchor) and had never been windowed at all — that half predates this PR. Step 1 now writes the clamped anchor to `/tmp/review-runs-since` and both readers `cat` it back, verified across a real call boundary. --- Closes #938 — automated triage --------- Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Step 1's run census pages at 30 and never follows the rest, so the "past 24 hours" list it produces is really "the most recent 30 runs per workflow". On this repo today that is 68 minutes of
tend-mention, reported as a day.Measured, not inferred
GET /actions/workflows/{id}/runsdefaults toper_page=30and orders newest-first. Running the recipe verbatim againsttend-mention(workflow250047576) for the current window:30 of 110, spanning 68 minutes of the 24-hour window. The 80 dropped runs are the oldest 23 hours — the truncation is systematic, not a sample.
Across every
tend-*/review-*workflow, comparing what the recipe returns against.total_count:tend-mention79→30,tend-review32→30,tend-notifications32→30tend-mention116→30,tend-review41→30Why it has gone unnoticed
The failure is silent in both directions.
gh apireturns 200, the--jqprojection succeeds, and the run reports a census with no indication a page was dropped. Recent evidence-log entries on #801 quote 182 and 200 runs — close to the true 183 and 219 — because those runs each improvised their ownper_page/pagination rather than following the recipe. That improvisation is the tell: the recipe as written has not actually been the thing producing the numbers, and a run that follows it literally analyses a little over half the fleet while recording a full-window all-clear.The consequence lands on gate evaluation rather than on a single decision. Occurrence counts feed Gate 1 thresholds directly, so a census that drops the oldest 23 hours of the busiest workflow systematically under-counts exactly the recurrence evidence the gates need — and does it in the direction of inaction.
Change
--paginateon both calls,per_page=100to keep the page count down, and a comment recording why. Both--jqfilters are per-element projections, so--paginateapplying them per page is a no-op here — worth stating, becausereview/SKILL.mdalready carries the opposite caveat for an aggregating filter (| last), and the next reader should not have to re-derive which case this is.The added sentence after the block asks for a
.total_countcross-check when a count lands on a page boundary. A count that exactly equals the page size is the signature of the bug, and it is the one symptom visible without re-querying.Verified against this repo: the paginated form returns 110
tend-mentionand 40tend-review, matching.total_countfor both.Gate assessment
.total_count. Not a sampled observation — the drop is arithmetic from the endpoint's documented default page size.list-recent-runs.sh, which is a different fetcher with its own--limit 50over a 3-hour window; left alone here to keep this atomic.