Skip to content

feat(altair): implement waffle-basic - #9952

Closed
github-actions[bot] wants to merge 2 commits into
mainfrom
implementation/waffle-basic/altair
Closed

feat(altair): implement waffle-basic#9952
github-actions[bot] wants to merge 2 commits into
mainfrom
implementation/waffle-basic/altair

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Implementation: waffle-basic - python/altair

Implements the python/altair version of waffle-basic.

File: plots/waffle-basic/implementations/python/altair.py

Parent Issue: #998


🤖 impl-generate workflow

github-actions Bot and others added 2 commits July 26, 2026 10:31
Regen from quality 83. Addressed:
- Title format (SC-04 failed → fixed): now uses the mandated
  `{spec-id} · {language} · {library} · anyplot.ai` format with the
  required language token, plus a data-driven subtitle for storytelling.
- Design Excellence (DE-01/DE-03): added a subtitle calling out the
  largest allocation, and squared up the canvas (10x10 grid reads
  naturally as a symmetric square format rather than landscape).
- Library Mastery (LM-02 failed → fixed): added a legend-bound
  `selection_point` that dims non-selected categories on click in the
  interactive HTML export — idiomatic altair, doesn't touch the default
  static PNG state.
- Canvas contract: switched to square orientation (2400x2400) with the
  canonical altair inner-view sizing (430x430 @ scale_factor=4.0) plus
  the mandatory pad-to-target block; previous figsize (1600x900 @
  scale_factor=3.0) was historical drift, off-target for the current
  hard-contract sizes.
- Kept: 10x10 grid structure, budget-allocation data scenario, Imprint
  categorical order, theme-adaptive chrome, legend-with-percentages.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor Author

🔧 AI Review Produced No Score — Auto-Retrying

The Claude Code Action ran but didn't write quality_score.txt. Auto-retrying review once...


🤖 impl-review

@github-actions github-actions Bot added the ai-review-failed AI review action failed or timed out label Jul 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor Author

❌ AI Review Failed (auto-retry exhausted)

The AI review action completed but did not produce valid output files. Auto-retry already tried once.

What happened:

  • The Claude Code Action ran
  • No quality_score.txt file was created

Manual rerun:

gh workflow run impl-review.yml -f pr_number=9952

🤖 impl-review

@github-actions github-actions Bot added ai-review-rescued Review re-dispatched once after ai-review-failed ai-review-failed AI review action failed or timed out and removed ai-review-failed AI review action failed or timed out labels Jul 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor Author

❌ AI Review Failed (auto-retry exhausted)

The AI review action completed but did not produce valid output files. Auto-retry already tried once.

What happened:

  • The Claude Code Action ran
  • No quality_score.txt file was created

Manual rerun:

gh workflow run impl-review.yml -f pr_number=9952

🤖 impl-review

MarkusNeusinger added a commit that referenced this pull request Aug 5, 2026
…review (#10179)

## The bug

`impl-review.yml` used quality score `0` as its sentinel for *"the AI
review produced no output"*. But `0` is also a score the review prompt
**mandates**: the Stage 1 auto-reject gates in
`prompts/workflow-prompts/ai-quality-review.md` require exactly `Score =
0, verdict = REJECTED` for **AR-08** (clipped element) and **AR-09**
(mandatory title not visible).

Prompt and workflow therefore contradicted each other, and the pipeline
was guaranteed to dead-end on exactly the plots it is designed to reject
hardest.

## The deadlock chain

1. Plot renders without a visible title → **AR-09** → reviewer returns
`Score: 0/100`, `Verdict: REJECTED` — **this is correct behaviour**
2. `Extract quality score` normalised it: `::warning::Invalid quality
score '0', defaulting to 0`
3. `Validate review output` fired on `score == '0'` → `::error::AI
Review did not produce valid output files` → `ai-review-failed` → `exit
1`
4. `exit 1` skipped **`Add verdict label and take action`** — which the
file itself documents as *"the pipeline's only hand-off point: every
downstream workflow (merge, repair) starts from a call made right here"*
5. So **`impl-repair` was never dispatched** and the missing title was
never fixed
6. `impl-review-retry.yml` rescued once → the re-review scored `0` again
(deterministically — the plot was unchanged) → `ai-review-failed`
re-applied
7. `ai-review-failed` + `ai-review-rescued` matches **no** watchdog case
(`watchdog-stuck-jobs.yml:116` only emits a `::warning::` and defers to
a human) → **PR stranded permanently**

## Evidence

Six open PRs sit in exactly that state, each with an AR-09 verdict
already in hand:

| PR | Library / spec | Reported score | Verdict | Gate |
|---|---|---|---|---|
| #10152 | seaborn windrose-basic | 0 | REJECTED | AR-09 |
| #10130 | muix streamgraph-basic | 0 | REJECTED | AR-09 |
| #10009 | matplotlib wireframe-3d-basic | 0 | REJECTED | AR-09 |
| #10003 | d3 ternary-basic | 0 | REJECTED | AR-09 |
| #9968 | plotnine treemap-basic | 0 | REJECTED | AR-09 |
| #9776 | matplotlib polar-basic | 0 | REJECTED | AR-09 |

This was never an infrastructure failure. In run
[31035492709](https://github.com/MarkusNeusinger/anyplot/actions/runs/31035492709)
the Claude action reported `"subtype": "success"`, `"is_error": false`,
15 turns, `permission_denials_count: 0` — and posted a complete review
ending in `### Score: 0/100` / `### Verdict: REJECTED`. 94 of the last
100 `impl-review` runs are green; the 6 failures are these gate-tripping
plots.

## The fix

Output presence becomes its own signal, decoupled from the score value:

- `Extract quality score` now emits **`has_output`** alongside `score`.
`0` is accepted as a valid score; only a non-numeric or out-of-range
value marks output as missing.
- The six gates that keyed off `score != '0'` / `score == '0'` now key
off `has_output`.
- A score of `0` therefore flows into the normal `ai-rejected` →
`impl-repair` path (threshold floor is 50, so `0 < 50` → rejected →
repair dispatched), and only genuinely absent output raises
`ai-review-failed`.

**Second, latent bug fixed in the same step:** the comment fallback read
`.comments[-1].body`, but on a retry the workflow's own
*"auto-retrying"* notice is posted **after** the review — so the
fallback searched the notice and found no score. It now selects the last
`claude[bot]` comment.

**Deliberately not changed:** `watchdog-stuck-jobs.yml`. With the root
cause fixed, "review produced no output twice in a row" (PRs
#9953/#9952/#9951, which have no `claude[bot]` comment at all) is a
genuine failure that *should* escalate to a human rather than loop
forever.

## Verification

GitHub Actions changes have no verification loop in this repo, so the
step's shell body was tested directly: a harness extracts the `Extract
quality score` `run:` block **verbatim from the YAML** and exercises it
with a stubbed `gh`.

```
--- the regression that caused the deadlock ---
PASS  file score 0 (AR-09 auto-reject)               score=0    has_output=true
PASS  comment fallback, score 0                      score=0    has_output=true
--- normal operation must be unchanged ---
PASS  file score 87 / 100 / 1 / trailing-newline 73  score=...  has_output=true
PASS  comment fallback, score 87                     score=87   has_output=true
--- genuine 'no output' must still be detected ---
PASS  no file, no review comment                     score=0    has_output=false
PASS  no file, comment without a score line          score=0    has_output=false
PASS  file with non-numeric garbage                  score=0    has_output=false
PASS  file with out-of-range score                   score=0    has_output=false
PASS  empty file                                     score=0    has_output=false

ALL CASES PASS
```

YAML validity re-checked after the edit (`yaml.safe_load`, 20 steps
parsed).

Residual risk: the `if:` expression rewrites and the `REPOSITORY` env
addition are only observable on a real pipeline run. Recovery path for
the six stranded PRs after merge: re-dispatch `impl-review.yml -f
pr_number=<n>`, which will now score them 0, label `ai-rejected`, and
hand them to `impl-repair` to fix the titles.

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@MarkusNeusinger

Copy link
Copy Markdown
Owner

Closing as superseded.

main already carries plots/waffle-basic/implementations/python/altair with quality_score: 83, which is at or above what this PR would land. impl-merge.yml has no regression guard against the score already on main, so resuming this PR would overwrite the better implementation with a worse one.

This PR stalled on the score-0 sentinel bug in impl-review.yml (a legitimate AR-08/AR-09 auto-reject was misreported as a crashed review, so impl-repair was never dispatched). That root cause is fixed in #10179, but the fix does not make this particular PR worth resuming — the coverage it would provide already exists at a higher score.

🤖 Closed during PR-queue cleanup.

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

Labels

ai-review-failed AI review action failed or timed out ai-review-rescued Review re-dispatched once after ai-review-failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant