| 1 | # Visual Review Rubric |
| 2 | |
| 3 | > Per-page visual self-check rubric for slide SVGs. Read by the subagents spawned during the `visual-review` stage. Companion to the [`visual-review` stage](../workflows/stages/visual-review.md) and the [`visual_review.py`](../scripts/visual_review.py) renderer. |
| 4 | |
| 5 | ## §0 Prerequisites |
| 6 | |
| 7 | This rubric **does not repeat** what `svg_quality_checker.py` already covers. Required upstream order: |
| 8 | |
| 9 | ``` |
| 10 | Executor finishes page → svg_quality_checker.py passes → visual_review.py renders PNG → this rubric runs |
| 11 | ``` |
| 12 | |
| 13 | If the static checker has not been run or has failed, the subagent must abort with status `prereq_failed` and not start the rubric. Topics already enforced by the static checker (do **not** re-check here): |
| 14 | |
| 15 | - font-size anchor drift (more than `2px` from every declared role anchor) |
| 16 | - id uniqueness, XML well-formed |
| 17 | - canvas/structural typography validation and informational spec-lock anchor comparison (contextual colors/fonts are allowed) |
| 18 | - animation_config compliance |
| 19 | |
| 20 | ## §0.1 Subagent inputs |
| 21 | |
| 22 | Each review subagent processes a **batch** of pages (see §6.1 for batch sizing). The inputs are: |
| 23 | |
| 24 | 1. **Page batch** — a list of `(svg_path, png_path, page_role)` tuples, one per assigned page. `svg_path` resolves under `<project>/svg_output/<page>.svg`, `png_path` under `<project>/.preview/<page>.png`. `page_role` is one of `cover` / `chapter` / `tldr` / `content` / `data` / `closing` / `breathing`, parsed from `design_spec.md §IX` by the orchestrator — subagents do **not** guess. |
| 25 | 2. **Path to this rubric file** |
| 26 | 3. **`<project>/design_spec.md`** (read-only) — §IX outline is the source of truth for "what should this page deliver" |
| 27 | 4. **`<project>/spec_lock.md`** (read-only) — brand-locked values |
| 28 | 5. **Style Review Focus excerpt** (conditional, read-only) — supplied by the orchestrator only when `<project>/templates/design_spec.md` contains an active Style segment; it retains the source path and exact §VII wording |
| 29 | 6. **`<project>/.review/`** (writable) — where backups and findings JSON go |
| 30 | |
| 31 | The subagent reads inputs 2–5 **once** at the start of its turn (input 5 may be absent), then iterates over the page batch sequentially (one page at a time): apply the rubric → apply any Style supplement → write `<project>/.review/<page>.json` → move on. This is the core token-saving move — fixed context is read N/K times instead of N times. |
| 32 | |
| 33 | Style Review Focus is supplemental acceptance context, not a second rubric. It cannot create new Hard rules, weaken §§1–3, or authorize content, identity, or structural edits. Record a clearly unmet focus item as `rule: "STYLE"`: apply a fix only when the existing rubric already permits that atomic edit; otherwise add a `needs_human_items` entry with a concise suggested fix. |
| 34 | |
| 35 | ## §1 Hard rules (fix every hit) |
| 36 | |
| 37 | | # | Category | Trigger | Permitted fix | |
| 38 | |---|----------|---------|---------------| |
| 39 | | H1 | Out-of-bounds | element bbox falls outside `0,0,1280,720` | shrink or reposition into canvas | |
| 40 | | H2 | Text overflow | text bbox extends past its visual container | reduce font-size or line-break | |
| 41 | | H3 | Text overlap | two `<text>` elements' bboxes intersect (tspans within one text excluded) | reposition or resize | |
| 42 | | H4 | Readability | contrast < 4.5 (small text) / < 3.0 (font-size ≥ 24px); OR text directly atop a complex image with no scrim | if **neither** the foreground nor the background color is a brand token: position-only escape — add a `<rect>` scrim under the text, or raise the offending text's font-size to ≥ 24px so the 3.0 threshold applies. If **either** color is a brand token: do not edit the SVG → goto §1.1 escalation. | |
| 43 | | ~~H5~~ | Font-ramp drift | *covered by `svg_quality_checker.py` — see §0 prerequisites* | n/a (do not re-check) | |
| 44 | | H6 | Element collision | rect/circle/path bboxes overlap with z-order violating semantics | open spacing | |
| 45 | | H7 | Anchored element displaced | page number / header / footer covered, missing, or out of canvas | restore to anchor position | |
| 46 | | H8 | Image rendering broken | `<image>` empty / broken-image / severe distortion | fix `href`; for `adaptive`, choose `meet` or a safer crop; a new complete-display requirement returns to §VIII `Crop Policy` and lock projection | |
| 47 | | H9 | Missing key element | element required by `design_spec §IX` outline is absent from rendered slide | recreate from spec | |
| 48 | |
| 49 | Detection order (run sequentially, do not parallelize within a single subagent): |
| 50 | |
| 51 | ``` |
| 52 | H1 → H2 → H7 (structure) |
| 53 | H3 → H6 (collisions) |
| 54 | H4 (readability) |
| 55 | H8 → H9 (content) |
| 56 | ``` |
| 57 | |
| 58 | ### §1.1 Brand-token contrast escalation |
| 59 | |
| 60 | If H4 fires and the foreground or background color is a **brand token** (defined in `spec_lock.md`) — i.e., the violation will repeat on every page using that token — do **not** touch the SVG. Brand decisions are §3 Don't-Touch; even position-only escapes (scrim insertion, font-size escalation) shift the page's visual weight in ways that should be a brand-level decision, not a per-page subagent decision. Instead: |
| 61 | |
| 62 | 1. Record the finding in the page JSON under `needs_human_items` with `rule: "H4"`, the offending element selector, and `suggested_fix_summary` describing the brand-level options (e.g., "raise body-text token from `#6E7681` to `#8B949E` deck-wide" or "introduce a scrim style in the brand"). |
| 63 | 2. Append the finding to `<project>/.review/brand_review.json` (append-only log; one entry per distinct token+context pair). The orchestrator aggregates and surfaces this to the main agent at the end of the run so the user can make one cross-deck decision instead of N per-page ones. |
| 64 | 3. The page's `status` is `needs_human` if H4 is the only Hard hit on the page; if other (non-brand) Hard hits were fixed, the page still finishes as `fixed` and the brand-token H4 entry sits in `needs_human_items` alongside. |
| 65 | |
| 66 | The aggregated brand review is the responsibility of the orchestrator at the end of the run, not the per-page subagent. |
| 67 | |
| 68 | ## §2 Soft rules (act only when clearly bad) |
| 69 | |
| 70 | Subagents must apply the **明显** ("clearly bad") threshold — when in doubt, leave it. Better to under-fix than to oscillate. |
| 71 | |
| 72 | | # | Category | Trigger | Fix direction | |
| 73 | |---|----------|---------|---------------| |
| 74 | | S1 | Vertical rhythm tight | Within the **same logical text block**, consecutive baselines have gap < 1.05× larger font-size | open to 1.15–1.3× | |
| 75 | | S2 | Vertical rhythm hollow | Within one logical block, > 150 px non-decorative whitespace; `breathing` pages exempt | tighten | |
| 76 | | S3 | Visual centroid off | hero/title block centroid offset from canvas center exceeds threshold by `page_role`: `cover` > 35%, `chapter` > 25%, `tldr`/`closing`/`breathing` > 25%, `content`/`data` > 20% | shift toward intended anchor | |
| 77 | | S4 | Alignment drift | same-column elements differ in `x` by > 4 px (or same-row baselines by > 4 px) **and** are semantically meant to be on the same grid line | snap to grid | |
| 78 | | S5 | Grid non-uniform | N-card row: neighbor `x`-spacing differs by > 5% of the average | re-distribute | |
| 79 | | S6 | CJK letter-spacing | CJK characters with `letter-spacing / font-size > 5%` | reduce to ≤ 2% | |
| 80 | | S7 | Accent overload | > 2 accent colors across ≥ 3 distinct elements | collapse to 1 primary + 1 secondary | |
| 81 | | S8 | Emphasis mismatch | most visually prominent element ≠ the element `design_spec §IX` declares as the page's primary | rescale to match intent | |
| 82 | | S9 | Image-text relationship | caption > 60 px from its image; text on busy image without scrim; image clearly purposeless | tighten / add scrim / remove | |
| 83 | | S10 | Breathing violation | only when `page_role = breathing`: ≥3 rounded card grid | replace with naked text / single hero | |
| 84 | |
| 85 | ## §3 Don't-touch |
| 86 | |
| 87 | Hard boundary, equal weight to §1. |
| 88 | |
| 89 | - **Brand decisions** — color tokens, font families, geometry style (decided by `spec_lock.md` / brand directory) |
| 90 | - **Layout restructure** — do not change column counts, replace chart types, add/remove sections |
| 91 | - **Content** — do not add or remove copy; only adjust position, font-size (within the mapped role's anchor `±2px`), spacing, letter-spacing, alignment, scrim |
| 92 | - **Other files** — never edit `design_spec.md` / `spec_lock.md` / `animations.json` / `image_prompts.json` / `images/` / other pages' SVGs |
| 93 | - **Atomicity** — one edit per fix, no bulk multi-element replacements |
| 94 | |
| 95 | If a "violation" requires reinterpreting `design_spec.md` to fix → mark `needs_human` with a one-line `suggested_fix_summary`. |
| 96 | |
| 97 | ## §4 Iteration protocol |
| 98 | |
| 99 | ### §4.0 Iteration 0 — PNG sanity check |
| 100 | |
| 101 | Run before applying any rule: |
| 102 | |
| 103 | - PNG file exists and is non-zero bytes |
| 104 | - PNG dimensions = 1280 × 720 |
| 105 | - PNG is **not** all-background (a histogram check: count of background-color pixels < 99% of total) — guards against blank/white-out renders only, **does not** filter sparse dark layouts |
| 106 | |
| 107 | Any check fails → status = `render_failed`, abort without scanning rules. |
| 108 | |
| 109 | ### §4.1 Iteration loop |
| 110 | |
| 111 | The full loop is defined here but the **default budget is 1 iteration**. Multi-iteration runs require an explicit opt-in in the orchestrator prompt and roughly double render cost per added iteration. |
| 112 | |
| 113 | ``` |
| 114 | iteration 1: scan all Hard + Soft → fix → (re-render only if budget ≥ 2) |
| 115 | iteration 2 (opt-in): re-verify changed elements + scan for new Hard hits → fix → re-render |
| 116 | iteration 3 (opt-in): report only, no further fix |
| 117 | ``` |
| 118 | |
| 119 | Per-iteration fix caps: |
| 120 | |
| 121 | - **Hard rules**: no per-round cap — every Hard hit must be addressed in the iteration it was found in |
| 122 | - **Soft rules**: ≤ 2 fixes per iteration; remaining Soft hits go to `untouched_concerns` |
| 123 | |
| 124 | ### §4.2 Termination conditions |
| 125 | |
| 126 | - **Rollback trigger**: any iteration's fix introduces a **new Hard hit** that did not exist before → immediately `cp` the backup back over the SVG, status = `needs_human`, finding records "rolled back fix X — created Hard Y" |
| 127 | - **Soft thrash trigger** (iteration budget ≥ 2 only): iteration 2's fix introduces a **new Soft hit** that did not exist before → stop, status = `needs_human` with note "fixes are competing" |
| 128 | - **Clean exit**: iteration ends with zero Hard hits and ≤ 1 Soft hit remaining → status = `ok` if no fixes were applied, `fixed` if any were applied |
| 129 | |
| 130 | ### §4.3 Backup discipline |
| 131 | |
| 132 | Before the **first** `Edit` on a page in any iteration `N`, the subagent must: |
| 133 | |
| 134 | ```bash |
| 135 | cp <project>/svg_output/<page>.svg <project>/.review/backup/<page>.iter<N>.svg |
| 136 | ``` |
| 137 | |
| 138 | The backup path is recorded in every finding's `backup_path` field. Backups are the rollback anchor for §4.2. |
| 139 | |
| 140 | ## §5 Output schema |
| 141 | |
| 142 | Each subagent writes exactly one file to `<project>/.review/<page>.json`: |
| 143 | |
| 144 | ```json |
| 145 | { |
| 146 | "page": "02_three_steps.svg", |
| 147 | "page_role": "content", |
| 148 | "status": "ok" | "fixed" | "needs_human" | "render_failed" | "prereq_failed", |
| 149 | "iterations_run": 1, |
| 150 | "screenshot_paths": [ |
| 151 | ".preview/02_three_steps.png", |
| 152 | ".preview/02_three_steps.iter1.png" |
| 153 | ], |
| 154 | "findings": [ |
| 155 | { |
| 156 | "iter": 1, |
| 157 | "rule": "S6", |
| 158 | "severity": "soft", |
| 159 | "evidence": "letter-spacing=10 on font-size=84, ratio=11.9% > 5%", |
| 160 | "fix_applied": { |
| 161 | "element": "#hero-statement text[font-size='84']", |
| 162 | "before": "letter-spacing=\"10\"", |
| 163 | "after": "letter-spacing=\"2\"" |
| 164 | }, |
| 165 | "verified_in_iter": 2, |
| 166 | "backup_path": ".review/backup/02_three_steps.iter1.svg" |
| 167 | } |
| 168 | ], |
| 169 | "untouched_concerns": [ |
| 170 | { |
| 171 | "rule": "S1", |
| 172 | "evidence": "...", |
| 173 | "reason": "soft-cap reached" | "ambiguous_design_intent" |
| 174 | } |
| 175 | ], |
| 176 | "needs_human_items": [ |
| 177 | { |
| 178 | "rule": "H9", |
| 179 | "suggested_fix_summary": "Hero subtitle declared in spec §IX.4 missing; add a <text> at (80,496) per design language" |
| 180 | } |
| 181 | ], |
| 182 | "design_intent_check": { |
| 183 | "spec_says": "TL;DR — emphasize 意图 as the core abstraction", |
| 184 | "render_delivers": true, |
| 185 | "note": "..." |
| 186 | } |
| 187 | } |
| 188 | ``` |
| 189 | |
| 190 | `needs_human_items` must include a `suggested_fix_summary` for every entry — never bare problem descriptions. |
| 191 | |
| 192 | ## §6 Dispatch & messaging contract |
| 193 | |
| 194 | This rubric is consumed by subagents spawned via the `visual-review` stage. Mandatory dispatch invariants: |
| 195 | |
| 196 | ### §6.1 Orchestrator → subagent (batched dispatch) |
| 197 | |
| 198 | The orchestrator partitions the N pages into `ceil(N/K)` batches of ≤ K pages each (default **K = 5**; configurable per run via the orchestrator prompt) and spawns one subagent per batch. |
| 199 | |
| 200 | - Spawn all batch subagents in **one assistant message** (parallel `Agent` calls). Sequential dispatch breaks pipelining. |
| 201 | - Each subagent prompt is **self-contained** — no prior conversation context. Inline the absolute paths for §0.1 inputs 1–5 explicitly, plus the full `(svg_path, png_path, page_role)` list for that batch. Do not assume the subagent knows the project root. |
| 202 | - `subagent_type: general-purpose`. Tool restrictions: Read, Edit, Bash (for `cp` backups), Write (for JSON output). MCP playwright is **not** required by subagents — orchestrator pre-renders PNGs. |
| 203 | - `name` / `team_name` parameters may be unavailable from nested teammate context. Dispatch must remain functional with anonymous subagents — do not require named addressing. |
| 204 | |
| 205 | **Why batched, not per-page**: the rubric (~2.5K tokens), `design_spec.md` (~4–5K), and `spec_lock.md` (~1K) are identical inputs across all pages and do **not** share a prompt cache between sibling subagents. A 20-page deck with per-page dispatch re-reads ~150K tokens of fixed documents; batched dispatch with K=5 cuts that by ~75% while staying inside default parallel-subagent limits (~10). Batches also bound failure blast radius — one crashed subagent loses K pages, not the entire run. |
| 206 | |
| 207 | **Batch size guidance**: |
| 208 | - `K = 5` (default) — balanced; safe for decks up to ~50 pages |
| 209 | - `K = 3` — high-fidelity / small decks (≤ 12 pages); slightly higher parallelism |
| 210 | - `K = 10` — token-sensitive / large decks (50+ pages); fewer subagents, larger blast radius per failure |
| 211 | |
| 212 | Larger K is **not** always better: subagent context fills with prior pages' SVG / PNG / findings as the batch progresses, and beyond ~10 pages context auto-compression starts dropping early findings. Keep K such that `K × (avg_svg_size + image_token_cost + report_size)` stays well under the subagent context budget. |
| 213 | |
| 214 | ### §6.2 Subagent → orchestrator |
| 215 | |
| 216 | - Subagent's **final action before going idle** must be `SendMessage(to=<lead>)` listing one JSON path per processed page (e.g., `<project>/.review/<page>.json`) and a ≤150-word text summary covering all pages in the batch. Going idle without messaging — or messaging with a partial batch — is a protocol violation. |
| 217 | - If the subagent aborts mid-batch (rule §4.2 rollback, tool error, etc.), it must still send the batch report covering both completed and aborted pages, with the aborted pages marked `needs_human` or `render_failed` as appropriate. |
| 218 | |
| 219 | ### §6.3 Orchestrator → main agent |
| 220 | |
| 221 | - Orchestrator's **final action before going idle** must be `SendMessage(to=<lead>)` containing: |
| 222 | - the aggregate Markdown table (page × status × hard_hits × soft_hits × fixes_applied × needs_human_reason) |
| 223 | - one ≤150-word "plumbing verdict" paragraph |
| 224 | - path to `brand_review.json` if any §1.1 aggregations occurred |
| 225 | |
| 226 | ### §6.4 Concurrency |
| 227 | |
| 228 | - Pre-rendering is serialized by `visual_review.py`'s file lock at `<project>/.preview/.render.lock`. Subagents must **not** call the renderer concurrently. Re-renders during iteration loop go through the same lock. |
| 229 | |
| 230 | ## §7 Renderer expectations *(script contract)* |
| 231 | |
| 232 | `visual_review.py <project> [pages...]` must guarantee: |
| 233 | |
| 234 | - Output PNG matches what the user would see in the live-preview browser (inlined `<use data-icon>`, resolved `<image href>`) |
| 235 | - Output dimensions = 1280 × 720 |
| 236 | - File-lock serialization at `<project>/.preview/.render.lock` |
| 237 | - Clean exit codes: |
| 238 | - `0` — all requested pages rendered |
| 239 | - `2` — live-preview server not running for this project (subagent should not retry; surfaces to the orchestrator) |
| 240 | - `3` — rendering backend (playwright + chromium) missing or unable to launch (config error, surface to user) |
| 241 | - `4` — page-level render failure (specific failures listed in stderr; partial output is acceptable) |
| 242 | |
| 243 | The renderer never edits SVGs and never reads any rule from this rubric — it is a pure render-and-validate tool. |
| 244 |