fix(altair,review): PAD-only normalization + AR-09 edge-clipping auto-reject#7528
Merged
Merged
Conversation
…-reject Two coordinated fixes prompted by the sn-curve-basic run after #7517 shipped: the canvas gate hit 17/17 PASS at exact 3200×1800, but altair's PIL center-crop fallback silently chopped the title at the top edge and the first digit of every y-axis tick label off the left edge. The AI review scored that 84/100 — high enough to merge on the attempt-2 ≥80 threshold — and a visibly broken chart shipped to the gallery. altair.md: - Shrink inner-view defaults from (800, 450) / (600, 600) to (620, 320) / (500, 460) so vl-convert's external padding for title + legend + axes still fits within 3200×1800 / 2400×2400. - Replace the crop-or-pad normalization with PAD-only. If vl-convert somehow still produces an oversized output, raise SystemExit instead of cropping — the impl-repair loop will pick that up. Cropping is exactly what destroyed the title pixels last time. ai-quality-review.md + quality-evaluator.md + quality-criteria.md: - Add AR-09 EDGE_CLIPPING as a hard auto-reject (Score=0, REJECTED). Fires when title, axis tick labels, axis title, legend, or annotation pixels are clipped at the canvas border — a visually broken chart that escapes the merge cascade is the catalog's most embarrassing failure mode. The post-render canvas-size gate enforces dimensions but cannot see what is at those edges; AR-09 is the reviewer's contract. - Explicit false-positive guard so the rule does not over-trigger on decorative borders, tooltips, or text that overflows its axis but stays within the canvas (that's still a VQ-05 deduction). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the generation/review prompts to prevent visually broken charts from shipping despite passing the hard canvas-dimension gate, by (a) changing Altair normalization to PAD-only with fail-fast on oversize renders and (b) introducing a new AR-09 auto-reject for edge-clipped text.
Changes:
- Add AR-09 (EDGE_CLIPPING) as a hard auto-reject in the workflow reviewer prompt and the offline evaluator prompt.
- Update Altair prompt defaults (inner view dims) and switch PNG normalization to PAD-only, raising
SystemExiton overshoot instead of cropping. - Extend the quality criteria catalogue to include AR-09 and update the Stage 1 check order.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| prompts/workflow-prompts/ai-quality-review.md | Adds AR-09 auto-reject guidance (edge clipping) alongside AR-08. |
| prompts/quality-evaluator.md | Adds AR-09 to the offline evaluator’s AI auto-reject list and trigger/exceptions text. |
| prompts/quality-criteria.md | Adds AR-09 to the Stage 1 auto-reject catalogue and updates check order. |
| prompts/library/altair.md | Shrinks recommended inner view dims and replaces crop-or-pad normalization with PAD-only + fail-fast on oversize outputs. |
Comment on lines
+121
to
+130
| Inspect both renders for any text, axis tick label, axis title, plot title, legend, or annotation that **touches or extends past the canvas border**. This is the single most embarrassing failure mode for the catalog — a chart that visibly chops off content publishes broken into the gallery. | ||
|
|
||
| Trigger AR-09 if you see ANY of: | ||
| - **Title cropped at the top edge** — top of letters cut, descenders missing, or title not fully visible above the plot area. | ||
| - **Y-axis tick labels missing their leftmost digit/character** because they touch the left canvas edge (e.g. "500" rendered as "00", "1,000" as ",000"). | ||
| - **X-axis labels cut at the bottom edge** — "Number of Cycles to Failure" missing or partially below the visible area. | ||
| - **Legend entries hidden behind / merged into the canvas edge.** | ||
| - **Any annotation, label, or category text whose bounding box is partially outside the saved PNG.** | ||
|
|
||
| This is distinct from VQ-05's softer "no overflow" check: AR-09 is for clipping AT THE CANVAS EDGE (pixels chopped), not for elements that simply overflow their axis. The post-render canvas-size gate enforces dimensions, but it cannot see WHAT is at those edges — that's your job. |
Comment on lines
25
to
29
| Before scoring, check for: | ||
| - **AR-06: NOT_FEASIBLE** — Library cannot implement the spec | ||
| - **AR-08: FAKE_FUNCTIONALITY** — Static library simulates interactive features | ||
| - **AR-09: EDGE_CLIPPING** — Title, axis label, legend, or other text is clipped at the canvas border | ||
|
|
| | AR-09 | EDGE_CLIPPING | Title / axis label / legend clipped at canvas border | AI decision (visual) | | ||
|
|
||
| **Check order:** AR-01 → AR-02 → AR-03 → AR-04 → AR-05 → AR-06 → AR-07 → AR-08 | ||
| **Check order:** AR-01 → AR-02 → AR-03 → AR-04 → AR-05 → AR-06 → AR-07 → AR-08 → AR-09 |
|
|
||
| **Check order:** AR-01 → AR-02 → AR-03 → AR-04 → AR-05 → AR-06 → AR-07 → AR-08 | ||
| **Check order:** AR-01 → AR-02 → AR-03 → AR-04 → AR-05 → AR-06 → AR-07 → AR-08 → AR-09 | ||
|
|
Four sensible Copilot comments on PR #7528: 1. ai-quality-review.md AR-09 wording was internally inconsistent: opened with "touches or extends past the canvas border", closed with "AR-09 is for *clipped* pixels, not proximity". Tighten the definition to require evidence of *missing pixels*. Touching the border, tight margins, or being rendered up against the edge with all pixels visible is now explicitly NOT AR-09. 2. quality-evaluator.md had a dedicated Step 0 for AR-08 but no corresponding step for AR-09. Add Step 0a (AR-08) / Step 0b (AR-09) with the strict trigger list and the not-AR-09 false-positive guard. 3. quality-criteria.md Overview diagram still said "(8 quick checks)" and "On fail: Score=0, no retry". Update count to 9 and clarify that workflow-handled AR-01..AR-05+AR-07 reject without retry while AI-handled AR-06, AR-08, AR-09 set score=0 and enter the review cascade. 4. quality-criteria.md was missing a dedicated AR-09 section (AR-08 has one). Add it with triggers, exceptions, and the explicit "pixels missing, not just proximity" rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #7517. The canvas gate worked perfectly on yesterday's
sn-curve-basicregen (17/17 PASS at exact 3200×1800), but altair's PIL center-crop fallback silently chopped the title at the top edge and the first digit of every y-axis tick label off the left edge. AI review scored that 84/100, which cleared the attempt-2 ≥80 threshold, and a visibly broken altair chart shipped to the gallery.Two coordinated fixes:
SystemExitinstead of cropping if vl-convert still overshoots — repair will pick that up).Changes
prompts/library/altair.md(800, 450)→(620, 320)(leaves ~500 px / 580 px of room for title + legend + axes inside 3200×1800)(600, 600)→(500, 460)(similarly for 2400×2400)Image.new(...) + paste(...). Ifvl-convertstill produces an oversized PNG, raiseSystemExitwith the actual dims — impl-repair triggers and the next attempt shrinks further.prompts/workflow-prompts/ai-quality-review.mdprompts/quality-evaluator.md+prompts/quality-criteria.mdWhy now
The canvas-size hard gate (#7517) enforces dimensions but cannot see what is at those edges. The combination of "exact 3200×1800 PNG" + "title pixels chopped off" is the worst possible outcome because it gives a green check while shipping a broken chart. AR-09 closes that gap.
Test plan
SystemExiton overshoot.bulk-generate.yml -f specification_id=sn-curve-basic(where altair's title-clip was reproducible) and confirm:canvas_gate_report.pyoutput over the next few daily-regen runs — altair should now consistently hit target without padding ugliness.🤖 Generated with Claude Code