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
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.