Skip to content

fix(impl-generate): scope the retry cap to the current campaign - #10627

Merged
MarkusNeusinger merged 3 commits into
mainfrom
fix/impl-generate-campaign-scoped-retry-cap
Aug 24, 2026
Merged

fix(impl-generate): scope the retry cap to the current campaign#10627
MarkusNeusinger merged 3 commits into
mainfrom
fix/impl-generate-campaign-scoped-retry-cap

Conversation

@MarkusNeusinger

Copy link
Copy Markdown
Owner

Problem

impl-generate.yml budgets 3 generation attempts per library, counted from hidden marker comments on the spec issue:

MARKER="<!-- impl-fail:${SPEC_ID}:${LIBRARY} -->"
FAILURE_COUNT=$(gh api --paginate ".../issues/${ISSUE}/comments" ... )
if [ "$FAILURE_COUNT" -ge 2 ]; then   # → impl:<lib>:failed, no auto-retry

Nothing ever deletes those markers — grep -rn "impl-fail:" across the repo returns exactly one hit, the read. So the count spans the issue's entire lifetime, not the current run, even though the surrounding code says "Attempt N/3" and clearly means one campaign.

Consequences, all observed on 2026-08-24 during the library backfill:

  1. A pair that ever failed twice gets one attempt on every future dispatch. Observed counts: chartjs/treemap-basic 4, plotnine/wireframe-3d-basic 4, ggplot2/network-force-directed **3`.
  2. Generation has an intermittent failure where the agent exits "subtype":"success","is_error":false but writes no file — 8 of 85 generate runs that day (~9%). With budget left, the built-in auto-retry absorbs it (bubble-basic/highcharts: failed 17:55, retried and succeeded 18:02, no human involved). With the budget already spent by old history, one flake is terminal.
  3. Terminal means terminal: impl:<lib>:failed has no recovery path — watchdog case 3 fires once and thereafter logs already retried by watchdog — needs manual attention (seen for issues [waffle-basic] Basic Waffle Chart #998, [network-force-directed] Basic Force-Directed Graph #990, [sankey-basic] Basic Sankey Diagram #810, [line-basic] Basic Line Plot #653).
  4. Repo-wide, 87 (spec, library) pairs across 54 issues carry impl:*:failed. That is a measurable slice of the 1,237 missing implementations, and part of it is self-inflicted rather than never-attempted.

Three of the pairs written off this way succeeded on the very next manual dispatch.

Fix

Count only markers newer than a 12-hour campaign window:

CAMPAIGN_CUTOFF=$(date -u -d "${CAMPAIGN_WINDOW_H} hours ago" +%Y-%m-%dT%H:%M:%SZ)
--jq "[.[] | select(.body != null and (.body | contains(\"$MARKER\")) and .created_at > \"$CAMPAIGN_CUTOFF\")] | length"

A campaign runs well under an hour, so 12 h is generous for a stalled tail while stale history ages out on its own. Choosing a time window over deleting markers keeps the audit trail intact, needs no destructive writes to issues, and unblocks the existing 87 pairs without a migration.

The cap message now states what was actually counted — 2 failed attempt(s) in the last 12h (cap: 3 per campaign) instead of a flat 3 generation attempts printed after a single attempt. That wording is what led to two pairs being written off as capability gaps during the backfill before a retry proved otherwise.

Explicitly unchanged: the fail-closed path. An unreadable counter still sets FAILURE_COUNT=999 and disables auto-retry — the zero-fallback loop that flooded ~1,200 runs in 38 h (#1010) stays impossible.

Verification

  • yaml.safe_load parses the workflow; the step's run: block extracted and checked with bash -n.
  • jq filter exercised against a fixture with an old marker, a fresh marker, and a null body → returns 1 (the fresh one only), so neither the age filter nor the existing null-guard regressed.
  • End-to-end behaviour is only observable on real pipeline runs (a documented no-verification-loop area in CLAUDE.md); the change is one filter clause plus a message.

Follow-up (not in this PR)

The 87 parked pairs need a re-dispatch sweep to actually recover — this PR only stops the trap from re-arming. That sweep is running separately.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RbZuWNDFy7kjXh9kLfA4dP

The 3-attempt budget is counted from hidden `<!-- impl-fail:spec:lib -->`
marker comments on the spec issue. Those markers are the permanent audit
trail — no workflow deletes them — so the count covered the issue's entire
lifetime rather than the current generation run. Any (spec, library) pair
that had ever failed twice was capped forever: each later dispatch got one
attempt, and a single hit of the intermittent "Claude reports success but
writes no implementation file" failure (8 of 85 generate runs on
2026-08-24) marked it `impl:<lib>:failed` — a terminal state, since the
watchdog only reports those as needing manual attention. 87 (spec, library)
pairs across 54 issues sit in exactly that state.

Observed counts on the pairs that failed this way: 4 for
chartjs/treemap-basic, 4 for plotnine/wireframe-3d-basic, 3 for
ggplot2/network-force-directed — the last of which then succeeded on the
very next manual dispatch.

Count only markers newer than CAMPAIGN_WINDOW_H (12 h). A campaign runs
well under an hour, so the window is generous while stale history ages out
by itself, which also unblocks the existing 87 without touching the issue
comments. The fail-closed path for an unreadable counter (#1010: ~1,200
runs in 38 h) is unchanged.

Also state the real number in the cap message: "3 generation attempts" was
printed after a single attempt and reads as a proven capability gap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RbZuWNDFy7kjXh9kLfA4dP
Copilot AI lite review requested due to automatic review settings August 24, 2026 20:38
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RbZuWNDFy7kjXh9kLfA4dP

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The updated cap message currently reports the pre-run failure count (off by one at the cap) and should reflect the attempt that just failed to avoid misleading operators.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes the retry-cap accounting in the impl-generate GitHub Actions workflow so the “3 attempts” budget applies to the current generation campaign (recent failures only) rather than accumulating across the entire lifetime of a spec issue.

Changes:

  • Scope failure-marker counting to a 12-hour campaign window when deciding whether to auto-retry or mark a library as failed.
  • Update operator-facing messaging to report what was counted (failures + window) instead of implying three attempts were made.
  • Add changelog entries documenting the retry-cap bug and the improved messaging.
File summaries
File Description
CHANGELOG.md Documents the campaign-scoped retry-cap fix and updated failure messaging.
.github/workflows/impl-generate.yml Adds a campaign window cutoff to marker counting and adjusts failure-cap messaging.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/impl-generate.yml Outdated
# made ONE attempt, and reading the old wording as "tried three
# times, must be a real capability gap" is exactly how genuinely
# recoverable pairs got written off (2026-08-24).
CAP_REASON="${FAILURE_COUNT} failed attempt(s) in the last ${CAMPAIGN_WINDOW_H}h (cap: 3 per campaign)"

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch — fixed in c0aa9ba. CAP_REASON now uses ATTEMPT (FAILURE_COUNT + 1), so the cap message counts the failure being handled: at the cap it reads 3 failed attempt(s) in the last 12h (cap: 3 per campaign) instead of 2. The whole point of the reworded message was to state a number an operator can trust, so an off-by-one there defeated it.

FAILURE_COUNT excludes the run whose failure triggered this step, so at the
cap the message said "2 failed attempt(s)" while marking the library failed
after the third. ATTEMPT already carries the corrected total.

Reported by the Copilot reviewer on #10627.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RbZuWNDFy7kjXh9kLfA4dP
Copilot AI review requested due to automatic review settings August 24, 2026 20:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is narrowly scoped to the failure-count filter and messaging, preserves the existing fail-closed behavior, and matches the stated retry-cap intent without introducing risky workflow-side effects.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@MarkusNeusinger
MarkusNeusinger merged commit 51e0d5e into main Aug 24, 2026
10 checks passed
@MarkusNeusinger
MarkusNeusinger deleted the fix/impl-generate-campaign-scoped-retry-cap branch August 24, 2026 20:49
MarkusNeusinger added a commit that referenced this pull request Aug 24, 2026
#10628)

## Why

The `babysit-pipeline` skill covers watching one fresh spec to 15/15.
Backfilling coverage gaps across the catalogue — dozens of older specs
each missing the libraries added after they were generated — is a
different job, and the skill said nothing about it. Running it by hand
on 2026-08-24 surfaced four traps that cost, or nearly cost, real
implementations.

## What's new

**Section 5 · Gap backfill**

- Compute the missing set from `plots/{spec}/metadata/{lang}/{lib}.yaml`
on `origin/main`, **not** from `impl:{lib}:done` labels — most affected
specs have closed issues, so their labels are absent or stale. Includes
the `git ls-tree | awk` one-liner.
- `run_spec.sh <spec> <model> <lib>...` as the driver: staggered
dispatch, poll to metadata, `RESULT=COMPLETE|PARTIAL|TIMEOUT`, skips
libraries already on main so a re-run retries exactly the gaps.
- Two specs in parallel (~4 specs/h vs ~2; ten concurrent
`impl-generate` runs showed no rate-limit effects).
- A `done.log` / `deferred.log` ledger written *before* dispatching the
next spec, so a compaction or crashed session resumes without
recounting.
- **The one-retry rule**, with the evidence for why it is not optional:
the single retry recovered `highcharts/treemap-basic`,
`ggplot2/wireframe-3d-basic` and `ggplot2/network-force-directed`, all
three of which had already been read as capability gaps.

**Four new gotchas**

| Trap | Why it matters |
|---|---|
| `Marking <lib> as failed: N generation attempts` counts more than this
run | The markers span past campaigns (#10627 scopes this to 12 h).
Check the `Previous failures for <lib>/<spec>: N` notice before
concluding a library can't do a plot type. |
| `impl:<lib>:failed` is terminal **and** unreliable | Nothing
re-dispatches it — watchdog case 3 fires once, then only logs `needs
manual attention`. And of 87 such labels, **42 sat on implementations
that had since landed**. |
| "Agent reports success, writes no file" | 8 of 85 generate runs (~9%).
Self-heals when retry budget remains (`bubble-basic/highcharts`: failed
17:55, succeeded on auto-retry 18:02). One occurrence is noise. |
| Static library + interactive/3D spec | The one gap shape that usually
is genuine — 18 of the 45 real gaps. Still give the one retry, then
defer. |

**`run_spec.sh` joins the bundled scripts.** Its hardcoded
`REPO=/home/tirao/anyplot` is replaced by resolution from the script's
own location (`ANYPLOT_REPO` overrides), matching how `poll_spec.sh`
derives `HERE`.

## Verification

- `bash -n` on `run_spec.sh`; repo auto-resolution checked from the
skill directory (`git -C "$HERE" rev-parse --show-toplevel` → the repo
root).
- The skill's existing `ALL_LIBS` gotcha now names `run_spec.sh`'s
`lang_of` too, so a library addition updates both scripts.
- Content is drawn from a real backfill run, not invented: every number
above has a corresponding workflow run or label query behind it.

## Related

- #10627 fixes the retry-cap bug the first gotcha describes.
- #10540 fixes the watchdog crash found in the same session.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01RbZuWNDFy7kjXh9kLfA4dP

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
MarkusNeusinger added a commit that referenced this pull request Aug 26, 2026
…er's fetch (#10660)

## Why

#10628 documented gap backfill while the backfill was still running.
Finishing it falsified three of its claims, so this PR corrects them
rather than leaving confident wrong guidance in a skill. Running the
backfill in two parallel slots also exposed a race in the driver, fixed
here.

**Scope: prose plus one executable change.** `SKILL.md` and
`CHANGELOG.md` are documentation; `run_spec.sh` gains a fetch retry
(behaviour change, details below).

## Corrections

**1. Halt-on-cluster counted the wrong thing.** The threshold was "≥5
failed `Generate:` runs in minutes = quota exhausted". Since #10627
restored the full retry budget, a single impossible pair spends three
runs on its own auto-retries — so two bad pairs trip a raw count of five
while the pipeline is perfectly healthy. That happened during the run: 6
failures at 22:29–22:37, all of them `plotnine` on two specs, nothing
wrong with the pipeline. Now counts distinct `(spec, library)` pairs.

**2. "Static library + interactive/3D spec is the one gap that is
usually real" was backwards.** Every category-level prediction made
during the backfill turned out wrong:

| Prediction | Outcome |
|---|---|
| chartjs can't do treemap / sankey | chartjs succeeded on
`network-force-directed`, `arc-basic`, and every JS-block spec |
| plotnine can't do 3D | `bar-3d-categorical` succeeded; `scatter-3d`
did not |
| ggplot2 can't do wireframe | succeeded on retry |
| pygal is a lost cause (6 failed labels) | succeeded on
`map-marker-clustered` and `line-stress-strain` |

17 of 20 parked pairs generated fine. Exactly three failed under a full
budget — plotnine on `scatter-3d`, `contour-3d`, `line-3d-trajectory`,
all of which need a spatial projection plotnine does not have, while the
"3D" spec representable in 2D went through. The gotcha now says:
measure, don't predict.

**3. The one-retry rule needed a precondition.** The workflow now spends
three attempts per campaign by itself, so a pair that comes back missing
may already be measured and the manual retry adds nothing. The skill now
shows the actual command — `gh run list` has no per-spec filter, so it
filters the output by run title with jq. Three failures minutes apart is
a gap, one is a flake. The same check resolves `RESULT=TIMEOUT` with
`recent generate failures: 0`, which only means the failures aged out of
the driver's 25-minute window (seen on `line-3d-trajectory`, which had
in fact failed three times 40 minutes earlier).

## New gotcha

Spec IDs harvested from `impl:*:failed` issue titles must be intersected
with the real `plots/` directories: **14 of 26** pointed at specs that
no longer exist on main, and such a dispatch dies seconds in at
`Validate specification exists`. This is how the rescue list first read
as 45 missing implementations when only 20 were real.

## Behaviour change: `run_spec.sh` retries its fetch

Two drivers polling the same checkout collide on the ref lock:

```
error: cannot lock ref 'refs/remotes/origin/main': is at 6b666c1 but expected c736a84
```

A lost fetch leaves `origin/main` stale, so `meta_present` understates
what has landed and the poller reports `PARTIAL` for libraries that are
already merged — a failure that disguises itself as a stalled spec.
`fetch_main()` now retries three times with backoff and, when all three
lose, logs the warning **with git's own stderr** so a ref-lock collision
is distinguishable from an auth or network failure.

## Verification

- `bash -n` on `run_spec.sh`; the failure path exercised against a bad
ref → `last error: fatal: couldn't find remote ref …`, confirming the
message survives to the log.
- Every number in the corrections traces to a workflow run or label
query from the 2026-08-24 backfill; the ledger is in
`agentic/runs/babysit-2026-08-20/`.

## Related

- #10627 (retry cap) — verified in production: `Previous failures for
plotnine/contour-3d since 2026-08-24T10:38:22Z: 2` followed by `3 failed
attempt(s) in the last 12h (cap: 3 per campaign)`.
- #10628 — the section this corrects.
- #10540 — verified by dry run: `daily-regen liveness: workflow state is
'disabled_manually', not active — rescue skipped`, scan completing
successfully.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01RbZuWNDFy7kjXh9kLfA4dP

---------

Co-authored-by: Claude Opus 5 (1M context) <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.

2 participants