Skip to content

perf(review-reviewers): cut cadence to 3-hourly, and anchor the window to the cron period - #845

Open
tend-agent wants to merge 6 commits into
mainfrom
daily/review-runs-30989943084
Open

perf(review-reviewers): cut cadence to 3-hourly, and anchor the window to the cron period#845
tend-agent wants to merge 6 commits into
mainfrom
daily/review-runs-30989943084

Conversation

@tend-agent

@tend-agent tend-agent commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Anthropic quota exhaustion has now hard-failed the fleet twice in three days. The Aug 3 diagnosis on #808 pre-registered the trigger — "I've left this to a maintainer decision rather than opening a cadence PR — it's the first occurrence and the plan-vs-cadence tradeoff is yours. If it recurs it's worth landing the cron cut" — and the review-runs tracking entry carries the same condition. It recurred, an order of magnitude larger. This lands option 1 from that comment.

What happened in this window

review-reviewers failed 15 consecutive runs × 5 matrix legs = 75 job failures across 14.5 hours (09:41Z Aug 4 → 23:10Z Aug 4), recovering on the first tick after the quota reset. Every leg dies ~4 s into the claude step with the same signature, read from the session-log artifacts:

{"type":"rate_limit_event","rate_limit_info":{"status":"rejected","resetsAt":1785888000,
 "rateLimitType":"seven_day","overageStatus":"rejected","overageDisabledReason":"org_level_disabled"}}
{"type":"result","is_error":true,"api_error_status":429,"num_turns":1,
 "result":"You've hit your weekly limit · resets 12am (UTC)"}

resetsAt: 1785888000 is 2026-08-05T00:00:00Z; the first green run is 30962483562 at 00:11Z. This is the seven-day quota, not the rolling session quota that caused the Aug 3 incident — the same capacity wall, hit on a longer accounting period, which is the harder one to ride out.

Why cadence is the lever

Normalised over the 8 runs that actually consumed tokens (the other 15 were free failures):

Workflow Runs w/ tokens Cost (list) Per run
review-reviewers 8 $83.88 $10.49
tend-review 21 $30.53 $1.45
tend-mention 18 $29.55 $1.64
tend-triage 6 $15.05 $2.51
tend-nightly 1 $4.07 $4.07

At its designed rate — 24 runs/day — review-reviewers alone projects to ~$252/day, against $79 for the entire rest of the fleet. Per-run cost is also climbing at flat cadence: $5.31 (Aug 3) → $7.86 (Aug 4) → $10.49 (Aug 5). Both drivers push the same way, and cadence is the one that can be changed deterministically.

Correction (2026-08-08), in reply to "why does doing it less frequently save costs?": cadence does not cut cost proportionally, and an earlier comment on this PR projecting $71/day was wrong to assume a flat per-run cost while the window triples. Per-leg cost measured across the matrix is **$1.12 fixed + $0.12 x runs surveyed**, so 65% of a quiet run is per-tick startup and only that part is saved. The cut is **$213 -> ~$120/day (~40%)**, not the 3x the tick count implies. The session-count argument is unaffected: 120 -> 40 Claude sessions/day is the full 3x, and that is what saturates the seven-day quota. Full derivation.

47 */3 * * * takes it from 120 to 40 Claude sessions/day. Outcome-acceptance signal — was a bot PR merged, was a comment answered, did a human push back — evolves over hours to days, so little analytical value is lost at 3-hourly. The workflows that lose when the quota is gone are the user-facing ones: on Aug 3 the same exhaustion dropped the nightly sweep entirely plus two PR reviews and two mention-handles.

The window-anchoring change is a prerequisite, not a drive-by

list-recent-runs.sh only anchors its completion window when the cron matches ^([0-9]+) \* \* \* \*$. Any other shape falls through to a now-anchored 1-hour window. Verified against the actual regex:

'47 * * * *'   -> hourly branch (minute=47)
'47 */3 * * *' -> FALLBACK: now-anchored 1h window

So cutting the cron alone would have left every run analysing one hour out of every three — a silent two-thirds coverage hole, reported as an all-clear. This change generalises the detection to MM */N * * * and derives the window from the cron period.

N is only accepted when it divides 24. Cron's */N restarts at hour 0 each day, so a step like */5 fires at 0,5,10,15,20 and then wraps after 4 h — no constant period, and a floor computed from one would under-reach across midnight. Those shapes keep falling through to the now-anchored window, same as before.

Verification

Hourly behaviour is unchanged. cron_hour_step is 1 for the hourly form, so period is 3600 and every expression reduces to the original arithmetic. Checked exhaustively rather than by inspection — old vs new tick computation over 24 hours × 3 minute offsets × 4 cron-minute values, plus month/quarter boundaries:

PASS: hourly arithmetic identical across 288 timestamps x 4 minute values + boundaries

Consecutive 3-hourly ticks tile exactly — no gap, no overlap, including the midnight wrap:

47 */3 * * *  @ 2026-08-05T00:10Z -> window [08-04T18:47 .. 08-04T21:47]
47 */3 * * *  @ 2026-08-05T01:00Z -> window [08-04T21:47 .. 08-05T00:47]
47 */3 * * *  @ 2026-08-05T03:50Z -> window [08-05T00:47 .. 08-05T03:47]
47 */3 * * *  @ 2026-08-05T06:50Z -> window [08-05T03:47 .. 08-05T06:47]
47 */3 * * *  @ 2026-08-05T09:50Z -> window [08-05T06:47 .. 08-05T09:47]

Non-conforming shapes still fall back: 47 */5 * * *, 17 6 * * *, */15 * * * * → now-anchored 1 h.

shellcheck clean; bash -n clean; the workflow YAML parses with the new cron.

The 6 h dropped-tick reach-back cap is deliberately left as a literal. At hourly that is unchanged; at the 3-hourly step it is two full periods, so a single dropped tick is still recoverable — and leaving it untouched keeps this diff off the lines #838 rewrites.

Overlap with #838

#838 also edits list-recent-runs.sh, in the dropped-tick recovery block (anchor on the last successful run; raise the fetch limit). Different concern, and the two are complementary — that one widens the window after an outage, this one sets the window's period. The edits are textually adjacent, so whichever lands second wants a small rebase; neither depends on the other.

Gate assessment

  • Evidence level: Critical/High — 75 job failures in one window, 13 in the Aug 3 window, both traced to the same capacity wall in session logs.
  • Occurrences: 2 distinct exhaustion events in 3 days (2026-08-03, 2026-08-04/05), plus a pre-registered "act on recurrence" from the prior run.
  • Structural: yes. No decision point — sustained demand above the quota produces the same failure every time, and the failure is invisible in the sense that starved runs are marked failure having silently not done their work.
  • Change type: targeted fix (config + the script change it requires).

What this does not address

The failures themselves are annotated only as claude -p exited non-zero (exit=1) with empty stderr — the 429 text sits in the stream-json the action already has, and is discarded on the non-zero-exit path. That is #818, open since Aug 3, and it is why diagnosing this needed a second round of manual session-log archaeology. Outage tracker #831 now carries 78 rows and no cause. Cadence reduces how often the wall is hit; #818 makes it legible when it is.

…w to the cron period

Anthropic quota exhaustion has now hard-failed the fleet twice in three
days. review-reviewers runs hourly across a 5-repo matrix (120 Claude
sessions/day) and takes ~70-79% of fleet spend, which keeps the
subscription quota saturated; event-driven workflows that land mid-window
fail at startup having done nothing.

Cutting to `47 */3 * * *` takes it to 40 sessions/day. Outcome-acceptance
signal evolves over hours to days, so little analytical value is lost.

list-recent-runs.sh only anchored its completion window for a bare hourly
cron (`MM * * * *`); any other shape fell back to a now-anchored 1h window.
Cutting the cron alone would therefore have left each run seeing one hour
out of every three — a silent two-thirds coverage hole. Generalize the
detection to `MM */N * * *` where N divides 24, and derive the window from
the cron period. For the hourly form `cron_hour_step` is 1 and every
expression reduces to the original arithmetic.

@tend-agent tend-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cadence cut is well-evidenced and the window generalisation holds up — I re-derived the tiling independently (exact tiling at 3-hourly including the midnight wrap, byte-identical arithmetic on the hourly path across 24h × 3 minute offsets × 4 cron-minute values, and */5 / MM H * * * / */15 * * * * still falling through). Three gaps, one of which is the PR's own failure mode surviving in a second place.

The "past hour" framing survives in the skill prompt. The PR's thesis is that cutting the cron without generalising the window leaves a silent two-thirds coverage hole. review-reviewers/SKILL.md updates the frontmatter description but keeps two hourly assumptions in the body that the run list no longer matches:

  • The opening instruction — "Analyze tend's CI behavior on the target repo over the past hour" (immediately under # Review Reviewers). This is the agent's own framing for the whole analysis, so it now contradicts the 3-hour run list it is handed in Step 1.
  • The Step 2 survey-subagent prompt — "tend-notifications: check for recent bot comments/issue-close events in the past hour". Unlike the other three mappings in that list, this one has no run-scoped anchor (no headBranch, no triggering comment) — it reconstructs outcomes purely from the time window. A subagent told "past hour" while the run list spans three will under-survey tend-notifications outcomes by exactly the two-thirds this PR exists to prevent.

Both want to be period-agnostic ("the analysis window", "the window covered by the run list") rather than restamped to "past 3 hours" — otherwise the next cadence change reopens the same hole.

The script's header docstring wasn't carried along with the detection comment. The comment at the detection site is generalised, but the module header above set -euo pipefail still documents the old contract: "whose updatedAt falls within a 1-hour completion window", "a simple hourly cron (MM * * * *)", "tile exactly: [intended-1h, intended], then [intended, intended+1h]", and "For non-schedule events or non-hourly crons, falls back to a now-anchored 1h window" — the last of which now reads as excluding the very shape the PR adds. # Fetches runs started in the past 3 hours is stale too: CREATED_SINCE is COMPLETED_AFTER - 7200, so at the 3-hourly step it reaches five hours back. That header is what a reader hits first, so it's the one that misleads.

I've pushed a commit fixing all three (the third is the inline suggestion below) — it's doc-only and doesn't touch the lines #838 rewrites, so the rebase note in the PR body still stands.

One thing I checked and am not flagging: the widened CREATED_SINCE span makes the per-workflow --limit 50 truncate silently if it ever binds, but current density on this repo is 4–10 runs per workflow per 5h, so there's an order of magnitude of headroom.

Comment thread plugins/tend-ci-runner/scripts/list-recent-runs.sh Outdated
…er and the skill prompt

The cron detection was generalised to MM */N * * *, but three places still
described a one-hour window:

- list-recent-runs.sh's module header, which still documented hourly-only
  detection, a 1h window, and a 3h created lookback.
- review-reviewers/SKILL.md's opening instruction, which framed the whole
  analysis as 'the past hour'.
- The Step 2 survey-subagent prompt's tend-notifications mapping, which has
  no run-scoped anchor and so reconstructs outcomes from the window alone —
  told 'past hour' against a 3h run list, it would under-survey by exactly
  the two-thirds this change exists to prevent.

Also corrects the 6h reach-back cap comment: 21600/3600 is six periods at
hourly, not one.
…e cadence

The skill still told the agent to open its PRs on `hourly/review-<run-id>`.
The workflow no longer runs hourly, so every branch it creates would have
carried a wrong cadence in its name.

Prefixing with the workflow name instead of the cadence keeps it correct
across future cadence changes — the same period-agnostic treatment the
opening instruction and the Step 2 survey prompt just got.
@tend-agent

Copy link
Copy Markdown
Collaborator Author

All three points are in 8631cab. Re-grepping the two files afterwards turned up a fourth hourly assumption the review missed, now fixed in 3a1d13a: Step 5 told the agent to open its PRs on hourly/review-$GITHUB_RUN_ID, so every PR this workflow files would have carried a cadence in its branch name that stopped being true with this change. Prefixed with the workflow name instead of the cadence, which is the same period-agnostic treatment the opening instruction and the Step 2 survey prompt got — restamping it to 3-hourly/ would just defer the problem to the next cadence change.

Nothing filters on that prefix (single occurrence across the repo), so it was cosmetic rather than a coverage hole. The commit is doc-only and still clear of the lines #838 rewrites.

@tend-agent tend-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One finding on the incremental plus a smaller note; both land on the same missing piece. Green CI, and self-authored so this is a COMMENT rather than an approval.

Secondary — workflow_dispatch now under-covers by the same two-thirds this PR closes. A dispatch event carries no .schedule, so it takes the fallback branch and gets a now-anchored 1 h window. Under 47 * * * * that happened to equal one period, so manual and scheduled runs saw the same span; at 47 */3 * * * a dispatched run analyzes one hour of a three-hour period, and Step 1's "if empty, record the run as all-clear" means that under-coverage can be written down as a clean window. Dispatch is the natural way to re-run after a dropped tick or a quota outage, which is when coverage matters most. I don't think the dispatch path should guess a period — but surfacing the window (below) at least makes the short span legible in the log instead of silent.

Both points reduce to: the window is computed and never published. list-recent-runs.sh emits only the per-run JSON array; COMPLETED_AFTER is used by the final jq filter and discarded. One line after the if/else that sets it, before the fetch loop, would publish it without disturbing the stdout contract:

echo "Completion window: >= $(date -u -d "@$COMPLETED_AFTER" +%Y-%m-%dT%H:%M:%SZ)" >&2

Step 1 then tells the agent to carry that value forward, and Step 2 substitutes it for <window-start> and for the new line-235 wording. Happy to push it if you want — it touches lines outside this diff, so I've left it as an offer.

Nothing else new: the four hourly assumptions are correctly retired, 21600 / 3600 reads six now, and the hourly/ branch prefix has no other references in the repo (review-runs keeps daily/, which is still true of its cron).

Comment thread plugins/tend-ci-runner/skills/review-reviewers/SKILL.md Outdated
…ilter on it

The window's floor was computed and then discarded — used only by the final
jq filter inside the script. Two consumers needed it and neither could get it:

- Step 2's corruption scan interpolates a literal `<window-start>` into five
  --jq filters with no stated source.
- The tend-notifications outcome mapping has no run-scoped anchor, so it
  reconstructs outcomes from the window alone. Telling it to "use the window's
  actual span" was unresolvable from inside that prompt.

Reconstructing the floor from the run list's own min/max updatedAt isn't
equivalent: that's the span of runs that happened to fire, it collapses toward
a point when one run lands in the period, and it's empty exactly when the list
is empty and the all-clear gets recorded.

Printing it also makes the fallback branch legible. A workflow_dispatch carries
no .schedule, so it takes the now-anchored 1h window; under the old hourly cron
that equalled one period, but at 47 */3 * * * a dispatched run covers one hour
of three. Dispatch is how you re-run after a dropped tick or a quota outage,
which is when coverage matters most. The dispatch path still doesn't guess a
period — the short span is now visible rather than silent.

stderr, so stdout stays the run-list JSON. Verified end-to-end against
max-sixty/tend: window line on stderr, stdout parses as a 15-element array.
@tend-agent

Copy link
Copy Markdown
Collaborator Author

Took the offer — b9523c6 publishes the window, which closes both the workflow_dispatch under-coverage note and the inline finding on Step 2.

list-recent-runs.sh now prints Completion window: >= <timestamp> to stderr, placed after the if/else that sets COMPLETED_AFTER so the cron and fallback branches both emit. Step 1 tells the agent to note it and flags that a dispatched run covers an hour rather than a period; Step 2's preamble binds it once for the whole prompt, which resolves the tend-notifications mapping and the corruption scan's five <window-start> filters in one place. The dispatch path still doesn't guess a period — the short span is legible instead of silent, which was the proposal.

The <window-start> half was the more serious of the two: a literal placeholder with no stated source was being interpolated into commands the survey subagent runs, independent of any cadence change.

Verification

Ran it rather than inspecting it:

$ TARGET_REPO=max-sixty/tend ./plugins/tend-ci-runner/scripts/list-recent-runs.sh "tend-" "review-"
stderr: Completion window: >= 2026-08-05T08:16:35Z
stdout: valid JSON array, 15 runs

That run took the fallback branch (this session is not a schedule event), which is the path the note is about. shellcheck and bash -n clean. No call site captures the script's stderr into stdout — the only invocations are the Step 1 snippet and the usage line in tend's running-tend overlay — so the run-list contract is unchanged.

The module header now documents the stderr line alongside the window contract.

@tend-agent tend-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both open findings from the last pass are closed on the incremental: the survey prompt now names the window's source and tells the subagent to substitute it, which covers all five <window-start> interpolations in the corruption scan (they're inside the same prompt block), and Step 1 now flags the workflow_dispatch under-coverage rather than leaving it silent. I checked the published floor's format against the --jq string comparisons it feeds — +%Y-%m-%dT%H:%M:%SZ sorts lexically against GitHub's created_at/submitted_at, so > does what it looks like it does — and the echo sits after the if/else, so both branches emit.

One nit inline on the new width claim.

Comment thread plugins/tend-ci-runner/skills/review-reviewers/SKILL.md Outdated
…tick recovery

The default floor is one cron period back, but dropped-tick recovery pulls it
to the previous actual run's intended tick, capped at intended - 21600 — so a
scheduled run can publish a window up to 6h wide, two periods at 3-hourly.
Step 1's description is what the agent reasons from when judging how much
ground the window covers, so it shouldn't state the default as the maximum.
max-sixty pushed a commit that referenced this pull request Aug 7, 2026
…he whole matrix (#877)

`review-reviewers` lost **8 h 15 m** of coverage today across all five
target repos, and the cause was a two-line gate in its own workflow
rather than anything in a skill.

## What happened

A critical GitHub Actions incident ([status
page](https://stspg.io/rcz3fcm83sff), created 2026-08-06T15:22:49Z,
still `investigating`) made runner acquisition unreliable. Four
consecutive `review-reviewers` ticks then failed identically —
`init-tracking` sat unscheduled for ~15 minutes, GitHub marked it
`cancelled`, and because the matrix job has a hard `needs:
init-tracking`, all five legs were `skipped` without ever trying for a
runner:

| Run | Created | `init-tracking` | matrix |
|---|---|---|---|
|
[31122877870](https://github.com/max-sixty/tend/actions/runs/31122877870)
| 17:21:15Z | `cancelled` | `skipped` ×5 |
|
[31125649607](https://github.com/max-sixty/tend/actions/runs/31125649607)
| 18:17:43Z | `cancelled` | `skipped` ×5 |
|
[31126978811](https://github.com/max-sixty/tend/actions/runs/31126978811)
| 19:34:17Z | `cancelled` | `skipped` ×5 |
|
[31128259981](https://github.com/max-sixty/tend/actions/runs/31128259981)
| 21:22:19Z | `cancelled` | `skipped` ×5 |

That's 20 leg-analyses discarded. The legs were not themselves
unschedulable: in
[31119653960](https://github.com/max-sixty/tend/actions/runs/31119653960)
(16:23Z) `init-tracking` did succeed, and matrix legs went on to acquire
runners and run — `numbagg/numbagg` completed successfully at 16:34Z
during the same incident.

## Why the gate isn't worth its cost

`init-tracking` exists to win a find-or-create race that can only bite
on the **first cron tick of a month** — before that tick, no tracking
issue exists for the new month and N legs would each try to create one.
The `review-reviewers` skill still carries its own find-or-create
fallback, sorting lowest-numbered-first precisely so a lost race
degrades to a duplicate rather than a crash. This change makes that path
load-bearing on a first-tick failure rather than a pure safety net, so
the skill's wording is updated in the same commit to say so.

So the gate trades *up to N-1 duplicate tracking issues (four, at
today's five legs), once a month* against *the entire analysis window
for every target repo, every time the job can't get a runner*. This
change keeps `needs:` — so `init-tracking` still runs first and still
wins the race on every normal tick — and only relaxes the failure
propagation.

`!cancelled()` rather than `always()` is deliberate: a user cancelling
the run should still stop the matrix.

## Gate assessment

- **Evidence level: High** — 4 occurrences in a single window, all with
byte-identical job shape.
- **Structural, not stochastic.** A hard `needs:` gate always
propagates; there is no decision point for the agent to get right.
Confirmed against the last 14 `review-reviewers` failures: the
`init=cancelled → matrix=skipped` shape appears **only** on today's
four, while every 2026-08-05 failure ran `init=success` with the matrix
reaching the agent. The gate is the amplifier; the incident is only the
trigger.
- **Change type: removal of a gate** — 2 functional lines. Under the
magnitude gate a removal clears at 1 occurrence; this has 4.
- **Not an incident workaround.** The skill's own rule is to record
transient upstream incidents rather than commit code around them, and
this run recorded the rest of the incident's damage without acting on
it. This one is different in kind: it removes a permanent single point
of failure that will amplify the *next* infrastructure hiccup the same
way, and it stays correct after the incident resolves.
- **Dedup**: no open or closed tend issue or PR mentions
`init-tracking`. #845 (cadence) and #858 (per-leg branch names) touch
this workflow but not this gate.

Evidence log: https://gist.github.com/192514ea2c36586f9b7f842a482d62ab

Found while analysing `PRQL/prql` in run
[31132205674](https://github.com/max-sixty/tend/actions/runs/31132205674)
— the run that had to absorb all four lost windows.

---------

Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
max-sixty pushed a commit that referenced this pull request Aug 7, 2026
)

## Problem

`review-reviewers` matrixes over five target repos
([`review-reviewers.yaml`](https://github.com/max-sixty/tend/blob/f3e309a475af03147ce6290104c85baafe6ad9f5/.github/workflows/review-reviewers.yaml)),
and all five legs of a tick share one `$GITHUB_RUN_ID`. Step 5 of the
skill hands every leg the same branch recipe:

> **PR** (default): Branch `hourly/review-$GITHUB_RUN_ID`, fix, commit,
push, …

So the moment two legs of the same tick both find something worth a PR,
they contend for one ref. The loser's `git push` is rejected as a
non-fast-forward and it has to recover mid-session; the ordering that
hurts more is the other one, where the second leg pushes *before* the
first opens its PR and the two unrelated concerns land on a single
branch — the atomic-PR rule broken by the branch name, not by any
judgement the agent made.

This just happened on run 31058673934: the `max-sixty/cargo-affected`
leg's push to `hourly/review-31058673934` was rejected against a sibling
leg's commit, which by then was #856. It recovered by suffixing the
target name (#857), and an earlier tick shows the same improvisation
baked into `hourly/review-30962483562-numbagg` — legs have been working
around the recipe rather than following it.

The run ID is non-unique along a second axis too: the same section caps
a leg at two PRs (`**Limit to at most 2 PRs per run**`), and a leg that
acts on two findings computes one name for both — same collision, one
job instead of two.

## Fix

Put both discriminators in the recipe:
`hourly/review-$GITHUB_RUN_ID-<target-repo-name>-<topic-slug>`. The
target name is unique across the matrix by construction, the topic slug
separates a single leg's two PRs, and the branch stays greppable back to
its run.

Scoped to this skill. `review-runs` carries the same
`$GITHUB_RUN_ID`-only shape, but `tend-review-runs.yaml` has no matrix,
so a run there is a single job and the name can't collide — no change
needed and none made.

**Overlaps #845 on this line.** That PR rewrites the same recipe to
rename the prefix (`hourly/` → `review-reviewers/`) as part of its
cadence change, and keeps `$GITHUB_RUN_ID` as the whole discriminator —
so it ships the collision under a new name. The two are orthogonal in
substance and conflict only textually: whichever lands second wants
`<prefix>/review-$GITHUB_RUN_ID-<target-repo-name>-<topic-slug>`, taking
the prefix from #845 and the suffixes from here.

## Gate assessment

- **Evidence level**: High — the collision is deterministic given two
acting legs, and there are two independent traces of it (this run's
rejected push, and a prior tick's hand-suffixed branch). The
one-leg-two-PRs axis is structural rather than traced.
- **Structural, not stochastic**: no decision point. Every leg is told
to compute the same name from the same variable; replayed ten times it
collides ten times.
- **Change type**: targeted fix — one line of the recipe.
- **Passes both gates.**

Evidence log: https://gist.github.com/dca23a6e6a0d8cae2665944ba31676fb

---------

Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
max-sixty pushed a commit that referenced this pull request Aug 7, 2026
…nst tend before filing upstream (#891)

## Problem

Two dedup blocks were blind in two different ways, and the cited
duplicate needed both fixed.

**State filter.** `review-runs` Step 5 and `review-reviewers` Step 4
both deduped against PRs with `gh pr list --state open`. A merged PR is
never returned by that query, so a finding whose fix already landed
reads as undeduped and gets filed again. `running-in-ci`'s PR-creation
dedup recheck already gets this right ("with `--state all` so closed and
merged siblings show up"); these two recipes contradicted it.

**Repo scope.** `review-runs` is a generated workflow
([`generator/src/tend/config.py:24`](https://github.com/max-sixty/tend/blob/f65f49f/generator/src/tend/config.py#L24)
lists it in the enabled set), so it runs in each adopter's checkout and
an unqualified `gh pr list` returns *the adopter's* PRs. Step 6 routes
bundled-skill defects upstream to tend, but neither Step 5 nor any of
`running-in-ci`'s dedup recipes — all local-repo — told the agent to
dedup in the target repo before filing there. `--state all` alone does
not close this: the adopter's PR list never contained the upstream fix
at any state.

`review-reviewers` is unaffected by the second half. It runs in
`max-sixty/tend` and files onto tend, so its unqualified `gh pr list`
already resolves to the right repo; only the state filter was wrong
there.

This bites hardest on tend specifically, because of the pinning model:
adopters call `max-sixty/tend/<harness>@X.Y.Z`, so a merged skill fix
stays dormant on their repos until the next release tags. The bug keeps
reproducing after the fix merges — which is exactly the window in which
the analysis legs are looking at it, and exactly when the dedup queries
are blind to the fix.

## What happened

`max-sixty/cargo-affected`'s `tend-review-runs` run
[31160677649](https://github.com/max-sixty/cargo-affected/actions/runs/31160677649)
(08:11:33Z → 08:21:41Z) hit the `| last` evidence-log mis-selection: it
appended ~12 KB of run evidence into the nightly's unrelated comment on
target [#73](max-sixty/cargo-affected#73),
noticed on its post-verify read, restored comment `5188771252`, and
re-appended to the real log `5150650688`. Good recovery. It then filed
[#883](#883) upstream, whose
"Proposed fix" is a `## Run ` heading predicate on the comment selector.

[#875](#875) merged that exact fix
at 07:34:40Z — 46 minutes before the issue was filed — as `test("^## Run
[0-9]")` on the same selector, in the same file. #883 is a duplicate of
a merged PR.

The run made three dedup queries before filing (`gh issue list --state
all --search "tracking issue comment append"`, a broader `gh issue list
--state all` title regex, and a final `gh issue list --state open`
recheck). All three were `gh issue list`, which never returns PRs — and
all three ran against `max-sixty/cargo-affected`. Even had it run Step
5's PR line verbatim, it would not have returned #875, for both reasons:
the state filter excluded merged PRs, and the query's repo was the
adopter's, not tend's.

## The fix

- Both skills: `gh pr list --state open` → `--state all`, projecting
`state,mergedAt` so a merged hit is legible.
- `review-runs` only: add the cross-repo pair (`gh pr list`/`gh issue
list --repo max-sixty/tend --state all`) so a finding heading upstream
under Step 6 is deduped against tend first.
- `review-runs` only: the pinning note is scoped to the upstream repo,
since in that skill the reader is the adopter and the local `gh pr list`
above it has nothing to do with pinned refs. `review-reviewers` keeps
the original wording, where tend is the reader and "on adopters" is the
correct direction.

Both added commands were run against this repo to confirm they parse and
return the expected shape.

## Gate assessment

- **Evidence level**: High. **Occurrences: 1** direct, verified end to
end (session log, both dedup query sets, #875's merge time and diff,
#883's body).
- **Structural, not stochastic.** `gh pr list --state open`
deterministically cannot return a merged PR, and a query scoped to the
adopter's repo deterministically cannot return a tend PR; replayed ten
times it misses #875 ten times. There is no decision point.
- **Change type**: targeted fix — query lines plus one sentence naming
the pinning consequence. It brings both recipes into line with a rule
the same plugin already states in `running-in-ci`, rather than
introducing new policy.
- **Why act at one occurrence**: the six-PR batch merged at 07:34:40Z
([#875](#875),
[#834](#834),
[#868](#868),
[#818](#818),
[#877](#877),
[#858](#858)) is all unreleased,
so six distinct bugs remain observable on every adopter until the next
release. The first analysis leg after that batch produced the first
duplicate. The exposure is six-wide and standing, not one-off.

## Not covered by

No open PR touches either dedup block.
[#845](https://github.com/max-sixty/tend/pull/845)/[#850](https://github.com/max-sixty/tend/pull/850)/[#838](https://github.com/max-sixty/tend/pull/838)
are the run-window cluster;
[#849](#849),
[#856](#856),
[#857](#857),
[#864](#864),
[#869](#869),
[#876](#876),
[#809](#809),
[#837](#837),
[#821](#821),
[#836](#836) are elsewhere.

#883 stays open as a maintainer call — it is redundant with #875 but the
option-2 half of its body (finish the gist migration for `review-runs`)
is not.

Evidence: https://gist.github.com/dca23a6e6a0d8cae2665944ba31676fb

---------

Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
# Conflicts:
#	plugins/tend-ci-runner/skills/review-reviewers/SKILL.md
@tend-agent

Copy link
Copy Markdown
Collaborator Author

Adding a measurement this PR has not had until now: the 24 h window 2026-08-07T08:02Z → 2026-08-08T08:02Z is the first clean one in the review-runs evidence log — 257 completed runs, zero failures, and all 24 hourly review-reviewers ticks fired, succeeded, and were metered. Every earlier window this PR could have been decided on was suppressed by an outage: Aug 3 session quota (13 runs), Aug 5 weekly quota (75 jobs), Aug 6 rate-limit blackout (36 runs), Aug 7 Actions incident (13 jobs). Aug 4 had no failures but only 20 ticks landed.

So this is the steady-state cost of 47 * * * * across the 5-repo matrix, not a number diluted by zero-token runs:

this window
review-reviewers runs 24 (24 success)
review-reviewers cost $212.79 across 2,967 turns
per run $8.87
fleet total $325.44
share 65% — highest of the six consecutive windows this has been recorded in (70 / 59 / — / 59 / 55 / 65)

At that rate the hourly cadence alone is ~$6,400/month at API list prices. This PR's 47 */3 * * * takes 24 ticks/day to 8, so on the same $8.87/run it lands near $71/day.

On what the cadence buys: all six PRs the workflow produced in the window came from two of the 24 runs — 31160773238 (#882, #884, #885) and 31164742947 (#891, #892, #893), which are also the window's two most expensive runs at $15.28 and $18.36. The other 22 appended to their per-target evidence gists and produced no PR or issue, which is the intended below-threshold path — the gist for max-sixty/tend has a ## Run heading for nearly every hourly tick in the window. Cutting to 3-hourly reduces the evidence sampling rate proportionally; it does not appear to cost findings at the rate they currently clear the gates.

Recording here rather than opening a competing PR — this is the existing lever, open since 2026-08-05, and it is MERGEABLE (nightly merged main into it at 06:30Z today).

@max-sixty

Copy link
Copy Markdown
Owner

why does doing it less frequently save costs?

@tend-agent

Copy link
Copy Markdown
Collaborator Author

Because most of what a run costs is paid at startup, per tick, and has nothing to do with how much window it analyses. Cutting ticks removes that fixed part; it does not remove the analysis work, which just gets batched into fewer, bigger runs.

That also means the projection in the PR body is wrong, and your question is the reason I checked. It said "on the same $8.87/run it lands near $71/day" — that assumes per-run cost stays flat while the window triples, which is exactly the thing that can't be true. Measured properly the cut is ~$213 → ~$120/day, about 40%, not the 67% implied by 24 ticks → 8.

The 5-repo matrix is a natural experiment for the split: every leg pays the same startup (skill load, bot-login resolution, fetching the target's running-tend, gist read + append, tracking-issue lookup) but surveys wildly different numbers of runs. If cost tracked the window, the dormant repos would be near-free. They aren't — in the 08:02Z tick, numbagg/numbagg (5 runs surveyed) cost $1.68 while max-sixty/tend (6 runs, but the busiest repo overall) cost $1.71.

Regressing cost on runs-surveyed across 10 legs (two ticks, 08:02Z and 09:59Z):

cost per leg ≈ $1.12 fixed + $0.12 × runs surveyed — so at typical density ~65% of an hourly run is fixed overhead.

hourly (47 * * * *) 3-hourly (47 */3 * * *)
runs surveyed per leg 5.1 15.3
cost per leg $1.73 $2.94
cost per tick (5 legs) $8.62 $14.70
ticks/day 24 8
per day $207 $118

The model lands on $207/day for the hourly cadence against the $212.79 actually measured in the clean window, which is the main reason I trust it.

So the mechanism is just: 24 ticks/day pay the $1.12 floor 24 times ($134/day of pure startup across the matrix); 8 ticks pay it 8 times (~$45). That ~$90/day is the saving. The $0.12/run survey work is unchanged — the same runs get analysed either way, three hours at a time instead of one.

Worth being explicit that this weakens the cost argument for the PR by a third, though I think it still clears: ~$90/day is real, and the quota-exhaustion case — 120 Claude sessions/day saturating the seven-day limit and hard-failing the user-facing workflows — rests on session count, which does drop by the full 3×, independent of dollars.

Method, data, and caveats

Per-leg costs come from each matrix job's own token-usage.json inside the claude-session-logs-* artifacts (the token report aggregates the 5 legs into one number, which hides exactly this). Legs mapped to target repos by grepping the session JSONL; runs-surveyed counted with gh run list over each leg's actual completion window, filtered to the tend-*/review-* workflows list-recent-runs.sh feeds the survey.

Run 31247520175 (window 07:02–08:02Z):

repo runs surveyed cost turns
max-sixty/cargo-affected 3 $1.29 17
numbagg/numbagg 5 $1.68 22
max-sixty/tend 6 $1.71 23
PRQL/prql 6 $2.37 31
max-sixty/worktrunk 14 $2.75 27

Run 31251792896 (window 08:59–09:59Z):

repo runs surveyed cost
numbagg/numbagg 2 $1.18
PRQL/prql 2 $1.34
max-sixty/worktrunk 3 $1.62
max-sixty/cargo-affected 4 $1.91
max-sixty/tend 6 $1.40

Fit on all 10 points: fixed $1.12, variable $0.119/run, R² 0.70. Dropping worktrunk-at-14 (the high-leverage point) gives fixed $1.06, variable $0.135/run — the conclusion doesn't rest on it. The turns column corroborates the floor independently: even the 3-run leg needs 17 turns.

Caveats, both pushing the estimate around by less than the headline gap:

  • The fit is on quiet runs. None of these 10 legs escalated to session logs or opened a PR. The window's two expensive runs ($15.28, $18.36) were findings-driven, and findings track how much bot activity happened, not how many ticks looked at it — so that escalation cost largely persists at 3-hourly. That pushes the real saving slightly below 40%, maybe to ~37%.
  • Some of what I'm charging as variable is really per-window. The repo-wide sweep in Step 2 takes no run ID, so it costs the same whether the window holds 5 runs or 15. To the extent that's inside the $0.12/run slope, batching wins a little more than the linear model credits. That pushes the other way.

n=10 across two ticks, so treat $1.12 ± a bit rather than a precise constant. The structural claim — a large per-tick floor, roughly two thirds of a quiet run — is what the two independent fits agree on, and it's the part the argument needs.

@tend-agent

Copy link
Copy Markdown
Collaborator Author

The quota wall came back last night, ~14 h after the comment above called the preceding 24 h the first clean window this PR could be decided on. That window did not hold a day.

This one was the rolling session quota, not the seven-day one: every agent invocation from 22:24:22Z to the 00:00Z reset died on the synthetic You've hit your session limit · resets 12am (UTC), a 1 h 36 min blackout. Smaller than the Aug 4 weekly outage by an order of magnitude, and the fourth exhaustion in six days.

What it actually cost, all of it verified from the run list and the session-log artifacts:

review-reviewers ticks lost 2 — runs 31282882062 (22:57) and 31285087877 (23:56), 10 legs, every one dead ~60 s in
analysis coverage lost 2 h × 5 target repos, with no recovery path other than the next tick widening its own window
tend-review affected 1 — 31281456692 on #904. No review lost: it had already posted its APPROVE at 22:22:42Z and died in the post-approval CI poll
outage rows on #905 11 (1 + 10 comments), which is #809's flood, still open

The attribution is the part worth adding here. Metering the five hours preceding the first refusal (17:24Z–22:24Z — the window the rolling quota was actually accumulated over):

runs cost
review-reviewers 5 $45.10
everything else (tend-review, tend-mention, tend-notifications) 33 $39.19

53.5% of the window's spend from 13% of its runs — and that is against an unusually busy fleet half, three PRs (#902/#903/#904) generating 33 metered runs in five hours. On a quiet evening the share would be higher, not lower.

The asymmetry that makes this a cadence argument rather than a cost argument: the workflow that drove the wall was also the only permanent casualty of it. tend-review recovered — its approval was already on the PR — and tend-mention and tend-triage had nothing queued. The two hours of review-reviewers coverage across five repos are simply gone. Cutting ticks lowers both the odds of hitting the wall and the amount of work stranded behind it when it is hit.

Recorded in the evidence gist; no new PR, since this one already proposes the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-runs Findings from review-runs skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants