From a06d22e52a44c5ed1aa647d0710c4fba39672713 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 29 Sep 2026 22:49:12 +0000 Subject: [PATCH] fix(review): pass Review Army checklists by path, run research alongside dispatch, always probe the design detector; state review-log invocation and statuses in the caller fixture - review-army-perf-n-plus-one: the parent copied full checklists into agent prompts and ran web research before dispatch (290 s on a 12-line diff); 212 s now. - review-design-lite: 5 of 6 captured trials reported the detector absent without probing; the probe is mandatory and its first line is reported, and the contract credits only fake-engine rule ids the checklist never names. - review-exploratory-small-cli: the fixture never gave review-log's direct invocation or status vocabulary; the model ran it through bun and wrote status "blocked". The prompt states both and the validator rejects out-of-vocabulary review statuses. Each case passed a focused paid run after repair. --- TODOS.md | 8 ++- review/SKILL.md | 2 +- review/SKILL.md.tmpl | 2 +- review/design-checklist.md | 2 +- review/sections/review-army.md | 16 +++--- scripts/resolvers/design-checklist.ts | 2 +- scripts/resolvers/review-army.ts | 16 +++--- ship/sections/review-army.md | 16 +++--- test/fixtures/golden/factory-ship-SKILL.md | 16 +++--- ...ew-design-lite-reports-ci-36633323521.json | 35 +++++++++++++ test/helpers/fake-impeccable.ts | 11 ++++ test/helpers/qa-callers-fixture.ts | 9 +++- test/qa-exploratory-callers.test.ts | 23 ++++++++ test/review-finalization-budget.test.ts | 52 +++++++++++++------ test/skill-e2e-review.test.ts | 7 +-- 15 files changed, 155 insertions(+), 62 deletions(-) create mode 100644 test/fixtures/review-design-lite-reports-ci-36633323521.json diff --git a/TODOS.md b/TODOS.md index 8f5959eac..7d20a689b 100644 --- a/TODOS.md +++ b/TODOS.md @@ -10,8 +10,7 @@ budgets. The CI image stays on 2.1.251 until those cases get faster. Effort M. - **Recurring reds to repair, not rerun** — `plan-design-review-plan-mode` (one ~250 s thinking block before its single write; times out at 300 s on - 2.1.251 in every recent run), `review-army-perf-n-plus-one` (290-300 s on a - 12-line diff; web search plus a conditional red-team pass), the HOLD SCOPE + 2.1.251 in every recent run) and the HOLD SCOPE routing case when its next brief happens not to name the mode (see the handoff item below). Effort M each. - **`/plan-ceo-review` skips its Step 0E mode handoff** — in 4 of 4 asked-mode @@ -20,6 +19,11 @@ decisions: …` chat. The routing case still passes on other posture text; a wording change moving the handoff ahead of the question log did not change the behavior in two paid runs, so it was not shipped. Effort M. +- **`/ship` design-lite probe wording** — `scripts/resolvers/design.ts` still + says "Probe for a design detector the user installed", the wording that let + `/review` skip its probe in 5 of 6 captured trials before this release made it + mandatory. The ship union ratio is at 1.3966 of 1.397, so the same sentence + needs a trim elsewhere first. Effort S. - **Let pass-rate history decide the rest** — every census on this branch had a different handful of single-trial reds. Once `eval:pass-rates` has 10 weekly trials per case, apply the CASE_QUARANTINE entry rule instead of chasing one diff --git a/review/SKILL.md b/review/SKILL.md index 01757a13f..d43064512 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -693,7 +693,7 @@ _aside_exec "Search the web for {framework} {version} {pattern} current best pra ``` Without Aside `READY`, use WebSearch if available; with neither, disclose the gap -and use existing knowledge. +and use existing knowledge. Don't wait on research: run it alongside independent work, such as specialist dispatch. ### Shared-code opportunities (core pass) diff --git a/review/SKILL.md.tmpl b/review/SKILL.md.tmpl index 95b5423b2..9aae36eb3 100644 --- a/review/SKILL.md.tmpl +++ b/review/SKILL.md.tmpl @@ -173,7 +173,7 @@ _aside_exec "Search the web for {framework} {version} {pattern} current best pra ``` Without Aside `READY`, use WebSearch if available; with neither, disclose the gap -and use existing knowledge. +and use existing knowledge. Don't wait on research: run it alongside independent work, such as specialist dispatch. ### Shared-code opportunities (core pass) diff --git a/review/design-checklist.md b/review/design-checklist.md index fab8a7637..8c78dfff4 100644 --- a/review/design-checklist.md +++ b/review/design-checklist.md @@ -15,7 +15,7 @@ source <(~/.claude/skills/gstack/bin/gstack-diff-scope 2>/dev/null) If `SCOPE_FRONTEND=false`, skip the entire design review silently. -**0. Mechanical pass first.** Probe for a design detector the user installed (this pass never offers to install one; the design skills ask, once) and, on `IMPECCABLE_READY`, scan the changed frontend files before reading them yourself: +**0. Mechanical pass first.** Always run the probe below for a design detector the user installed. It searches the environment and install caches, which no file listing shows, so never assume or report a detector absent without its output; state its first line in the design review. This pass never offers to install one (the design skills ask, once). On `IMPECCABLE_READY`, scan the changed frontend files before reading them yourself: ```bash bun --no-env-file run ~/.claude/skills/gstack/bin/gstack-design-detect.ts probe --host claude diff --git a/review/sections/review-army.md b/review/sections/review-army.md index 137133909..2cc0465ec 100644 --- a/review/sections/review-army.md +++ b/review/sections/review-army.md @@ -78,7 +78,7 @@ so they run in parallel. Each subagent has fresh context — no prior review bia Construct the prompt for each specialist. The prompt includes: -1. The specialist's checklist content (you already read the file above) +1. The specialist's checklist path from the selection above (the subagent reads it; never paste its content) 2. Stack context: "This is a {STACK} project." 3. Past learnings for this domain (if any exist): @@ -90,7 +90,7 @@ If learnings are found, include them: "Past learnings for this domain: {learning 4. Instructions: -"You are a specialist code reviewer. Read the checklist below, then run +"You are a specialist code reviewer. Read the checklist at {checklist path}, then run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"` to get the full diff. Apply the checklist against the diff. For each finding, output a JSON object on its own line: @@ -109,10 +109,7 @@ If no findings: output `NO FINDINGS` and nothing else. Do not output anything else — no preamble, no summary, no commentary. Stack context: {STACK} -Past learnings: {learnings or 'none'} - -CHECKLIST: -{checklist content}" +Past learnings: {learnings or 'none'}" **Subagent configuration:** - Use `subagent_type: "general-purpose"` @@ -181,6 +178,7 @@ Only specialist findings enter this header and `quality_score`; core findings do Use the merged NON-advisory specialist findings for both counts and score: `quality_score = max(0, 10 - (critical_count * 2 + informational_count * 0.5))` Cap at 10 and retain for the review-log entry in Step 5.8. These are not final unresolved-defect totals. +Print only this block: the stage 6 activity object and `test_stub` bodies are log and Fix-First data. Validated `"advisory": true` findings from any source are excluded from score, header, unresolved-defect totals and clean-status blockers. Show them separately; they remain ASK-only, never auto-applied. Real defects follow normal Fix-First. @@ -239,13 +237,13 @@ completion. Advice never permits edits while readers are active or replaces a re If activated, dispatch one more subagent via the Agent tool (pass `run_in_background: false` — foreground; subagents default to background since Claude Code v2.1.198). The Red Team subagent receives: -1. The red-team checklist from `~/.claude/skills/gstack/review/specialists/red-team.md` -2. The merged specialist findings from Step 4.6 (so it knows what was already caught) +1. The red-team checklist path `~/.claude/skills/gstack/review/specialists/red-team.md` (it reads the file) +2. The merged specialist findings from Step 4.6, one line each (so it knows what was already caught) 3. The git diff command Prompt: "You are a red team reviewer. The code has already been reviewed by N specialists who found the following issues: {merged findings summary}. Your job is to find what they -MISSED. Read the checklist, run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`, and look for gaps. +MISSED. Read the checklist at {red-team checklist path}, run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`, and look for gaps. Output findings as JSON objects (same schema as the specialists). Focus on cross-cutting concerns, integration boundary issues, and failure modes that specialist checklists don't cover." diff --git a/scripts/resolvers/design-checklist.ts b/scripts/resolvers/design-checklist.ts index 6c5509d25..9268704f8 100644 --- a/scripts/resolvers/design-checklist.ts +++ b/scripts/resolvers/design-checklist.ts @@ -68,7 +68,7 @@ source <(~/.claude/skills/gstack/bin/gstack-diff-scope 2>/dev/null) If \`SCOPE_FRONTEND=false\`, skip the entire design review silently. -**0. Mechanical pass first.** Probe for a design detector the user installed (this pass never offers to install one; the design skills ask, once) and, on \`${SENTINEL.READY}\`, scan the changed frontend files before reading them yourself: +**0. Mechanical pass first.** Always run the probe below for a design detector the user installed. It searches the environment and install caches, which no file listing shows, so never assume or report a detector absent without its output; state its first line in the design review. This pass never offers to install one (the design skills ask, once). On \`${SENTINEL.READY}\`, scan the changed frontend files before reading them yourself: \`\`\`bash bun --no-env-file run ~/.claude/skills/gstack/bin/gstack-design-detect.ts probe --host claude diff --git a/scripts/resolvers/review-army.ts b/scripts/resolvers/review-army.ts index 707c3ddd9..3af34dcb8 100644 --- a/scripts/resolvers/review-army.ts +++ b/scripts/resolvers/review-army.ts @@ -95,7 +95,7 @@ so they run in parallel. Each subagent has fresh context — no prior review bia Construct the prompt for each specialist. The prompt includes: -1. The specialist's checklist content (you already read the file above) +1. The specialist's checklist path from the selection above (the subagent reads it; never paste its content) 2. Stack context: "This is a {STACK} project." 3. Past learnings for this domain (if any exist): @@ -107,7 +107,7 @@ If learnings are found, include them: "Past learnings for this domain: {learning 4. Instructions: -"You are a specialist code reviewer. Read the checklist below, then run +"You are a specialist code reviewer. Read the checklist at {checklist path}, then run \`DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"\` to get the full diff. Apply the checklist against the diff. For each finding, output a JSON object on its own line: @@ -126,10 +126,7 @@ If no findings: output \`NO FINDINGS\` and nothing else. Do not output anything else — no preamble, no summary, no commentary. Stack context: {STACK} -Past learnings: {learnings or 'none'} - -CHECKLIST: -{checklist content}" +Past learnings: {learnings or 'none'}" **Subagent configuration:** - Use \`subagent_type: "general-purpose"\` @@ -203,6 +200,7 @@ Only specialist findings enter this header and \`quality_score\`; core findings Use the merged NON-advisory specialist findings for both counts and score: \`quality_score = max(0, 10 - (critical_count * 2 + informational_count * 0.5))\` Cap at 10 and retain for ${persistRef}. These are not final unresolved-defect totals. +Print only this block: the stage 6 activity object and \`test_stub\` bodies are log and Fix-First data. Validated \`"advisory": true\` findings from any source are excluded from score, header, unresolved-defect totals and clean-status blockers. Show them separately; they remain ASK-only, never auto-applied. Real defects follow normal Fix-First. @@ -264,13 +262,13 @@ function generateRedTeam(ctx: TemplateContext): string { If activated, dispatch one more subagent via the Agent tool (pass \`run_in_background: false\` — foreground; subagents default to background since ${CC_BACKGROUND_DEFAULT_SINCE}). The Red Team subagent receives: -1. The red-team checklist from \`${ctx.paths.skillRoot}/review/specialists/red-team.md\` -2. The merged specialist findings from Step ${stepMerge} (so it knows what was already caught) +1. The red-team checklist path \`${ctx.paths.skillRoot}/review/specialists/red-team.md\` (it reads the file) +2. The merged specialist findings from Step ${stepMerge}, one line each (so it knows what was already caught) 3. The git diff command Prompt: "You are a red team reviewer. The code has already been reviewed by N specialists who found the following issues: {merged findings summary}. Your job is to find what they -MISSED. Read the checklist, run \`DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"\`, and look for gaps. +MISSED. Read the checklist at {red-team checklist path}, run \`DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"\`, and look for gaps. Output findings as JSON objects (same schema as the specialists). Focus on cross-cutting concerns, integration boundary issues, and failure modes that specialist checklists don't cover." diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 03c7f2627..4e29932c0 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -303,7 +303,7 @@ so they run in parallel. Each subagent has fresh context — no prior review bia Construct the prompt for each specialist. The prompt includes: -1. The specialist's checklist content (you already read the file above) +1. The specialist's checklist path from the selection above (the subagent reads it; never paste its content) 2. Stack context: "This is a {STACK} project." 3. Past learnings for this domain (if any exist): @@ -315,7 +315,7 @@ If learnings are found, include them: "Past learnings for this domain: {learning 4. Instructions: -"You are a specialist code reviewer. Read the checklist below, then run +"You are a specialist code reviewer. Read the checklist at {checklist path}, then run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"` to get the full diff. Apply the checklist against the diff. For each finding, output a JSON object on its own line: @@ -334,10 +334,7 @@ If no findings: output `NO FINDINGS` and nothing else. Do not output anything else — no preamble, no summary, no commentary. Stack context: {STACK} -Past learnings: {learnings or 'none'} - -CHECKLIST: -{checklist content}" +Past learnings: {learnings or 'none'}" **Subagent configuration:** - Use `subagent_type: "general-purpose"` @@ -406,6 +403,7 @@ Only specialist findings enter this header and `quality_score`; core findings do Use the merged NON-advisory specialist findings for both counts and score: `quality_score = max(0, 10 - (critical_count * 2 + informational_count * 0.5))` Cap at 10 and retain for the review-log persist. These are not final unresolved-defect totals. +Print only this block: the stage 6 activity object and `test_stub` bodies are log and Fix-First data. Validated `"advisory": true` findings from any source are excluded from score, header, unresolved-defect totals and clean-status blockers. Show them separately; they remain ASK-only, never auto-applied. Real defects follow normal Fix-First. @@ -464,13 +462,13 @@ completion. Advice never permits edits while readers are active or replaces a re If activated, dispatch one more subagent via the Agent tool (pass `run_in_background: false` — foreground; subagents default to background since Claude Code v2.1.198). The Red Team subagent receives: -1. The red-team checklist from `~/.claude/skills/gstack/review/specialists/red-team.md` -2. The merged specialist findings from Step 9.2 (so it knows what was already caught) +1. The red-team checklist path `~/.claude/skills/gstack/review/specialists/red-team.md` (it reads the file) +2. The merged specialist findings from Step 9.2, one line each (so it knows what was already caught) 3. The git diff command Prompt: "You are a red team reviewer. The code has already been reviewed by N specialists who found the following issues: {merged findings summary}. Your job is to find what they -MISSED. Read the checklist, run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`, and look for gaps. +MISSED. Read the checklist at {red-team checklist path}, run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`, and look for gaps. Output findings as JSON objects (same schema as the specialists). Focus on cross-cutting concerns, integration boundary issues, and failure modes that specialist checklists don't cover." diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index 1fb0bb370..e66d6ccda 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -2163,7 +2163,7 @@ so they run in parallel. Each subagent has fresh context — no prior review bia Construct the prompt for each specialist. The prompt includes: -1. The specialist's checklist content (you already read the file above) +1. The specialist's checklist path from the selection above (the subagent reads it; never paste its content) 2. Stack context: "This is a {STACK} project." 3. Past learnings for this domain (if any exist): @@ -2175,7 +2175,7 @@ If learnings are found, include them: "Past learnings for this domain: {learning 4. Instructions: -"You are a specialist code reviewer. Read the checklist below, then run +"You are a specialist code reviewer. Read the checklist at {checklist path}, then run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"` to get the full diff. Apply the checklist against the diff. For each finding, output a JSON object on its own line: @@ -2194,10 +2194,7 @@ If no findings: output `NO FINDINGS` and nothing else. Do not output anything else — no preamble, no summary, no commentary. Stack context: {STACK} -Past learnings: {learnings or 'none'} - -CHECKLIST: -{checklist content}" +Past learnings: {learnings or 'none'}" **Subagent configuration:** - Use `subagent_type: "general-purpose"` @@ -2266,6 +2263,7 @@ Only specialist findings enter this header and `quality_score`; core findings do Use the merged NON-advisory specialist findings for both counts and score: `quality_score = max(0, 10 - (critical_count * 2 + informational_count * 0.5))` Cap at 10 and retain for the review-log persist. These are not final unresolved-defect totals. +Print only this block: the stage 6 activity object and `test_stub` bodies are log and Fix-First data. Validated `"advisory": true` findings from any source are excluded from score, header, unresolved-defect totals and clean-status blockers. Show them separately; they remain ASK-only, never auto-applied. Real defects follow normal Fix-First. @@ -2324,13 +2322,13 @@ completion. Advice never permits edits while readers are active or replaces a re If activated, dispatch one more subagent via the Agent tool (pass `run_in_background: false` — foreground; subagents default to background since Claude Code v2.1.198). The Red Team subagent receives: -1. The red-team checklist from `$GSTACK_ROOT/review/specialists/red-team.md` -2. The merged specialist findings from Step 9.2 (so it knows what was already caught) +1. The red-team checklist path `$GSTACK_ROOT/review/specialists/red-team.md` (it reads the file) +2. The merged specialist findings from Step 9.2, one line each (so it knows what was already caught) 3. The git diff command Prompt: "You are a red team reviewer. The code has already been reviewed by N specialists who found the following issues: {merged findings summary}. Your job is to find what they -MISSED. Read the checklist, run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`, and look for gaps. +MISSED. Read the checklist at {red-team checklist path}, run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"`, and look for gaps. Output findings as JSON objects (same schema as the specialists). Focus on cross-cutting concerns, integration boundary issues, and failure modes that specialist checklists don't cover." diff --git a/test/fixtures/review-design-lite-reports-ci-36633323521.json b/test/fixtures/review-design-lite-reports-ci-36633323521.json new file mode 100644 index 000000000..db116ab94 --- /dev/null +++ b/test/fixtures/review-design-lite-reports-ci-36633323521.json @@ -0,0 +1,35 @@ +{ + "provenance": "review-output.md bodies written by the review-design-lite Write tool call in Periodic Evals native captures (native-captures-ci--1-eval-slices-N). scanRan records whether the transcript ran gstack-design-detect.ts scan. Only the 36633323521 t1 capture probed and scanned; the others never ran the probe yet the legacy contract accepted t3 and 36629958451 t1 because they mention 'detector' or the checklist-named [ai-color-palette]. The local focused run t2 (aba80c8 plus the first checklist repair) inferred absence from a file listing and never probed.", + "reports": [ + { + "run": "36633323521", + "trial": "t1", + "scanRan": true, + "report": "# /review — feature/add-landing-page vs main\n\nBase branch: `main` (no `origin` remote; git-native fallback). Diff: `landing.html` (+41), `styles.css` (+86/−1).\nScope: `SCOPE_FRONTEND=true` → design checklist applied. No `DESIGN.md` → universal principles.\nDetector: `IMPECCABLE_READY`, exit 2, 6 hits (low-contrast ×3, skipped-heading, ai-color-palette, marketing-buzzword). Detector `file` field names `test/fixtures/review-eval-design-slop.html`, which does not exist in this repo — hits are credited only where they match code I read in `landing.html`/`styles.css`. No ignored rules in config.\nFixes are listed by tier but **not applied** — this run was requested as a written report.\n\n---\n\n## Pre-Landing Review: 4 issues (0 critical, 4 informational)\n\nPass 1 (SQL, races, LLM trust boundary, shell injection, enum completeness): nothing applicable — static HTML/CSS, no data or execution paths.\n\n**AUTO-FIX:**\n- [INFORMATIONAL] (confidence: 9/10) `styles.css:1`, `landing.html:10,17` — Eval annotations (`/* Planted design anti-patterns for E2E eval — 7 issues */`, ``) ship with the page.\n Fix: delete the fixture comments before landing.\n- [INFORMATIONAL] (confidence: 8/10) `landing.html:14` — `` has no `type`, no handler and no destination; `Terms of Service` is a dead placeholder link. Both CTAs are inert.\n Recommended fix: make the primary CTA an `` styled as a button (or give the button a `type` + handler); point Terms at the real URL.\n\n---\n\n## Design Review: 18 issues (4 auto-fixable, 11 need input, 3 possible)\n\n**AUTO-FIXED:** (none applied — see note above; these are the AUTO-FIX-tier items)\n- D1 [HIGH] (confidence: 10/10) `styles.css:56` — `button { outline: none; }` with no replacement focus indicator; keyboard users lose the focus ring on the only CTA. → Remove `outline: none`; add `button:focus-visible { outline: 2px solid currentColor; outline-offset: 2px; }`.\n- D2 [HIGH] (confidence: 10/10) `styles.css:72-73` — `!important` ×2 in `.override` (`color: red !important; margin-left: 10px !important;`). Nothing competes with `.override`'s specificity; the escape hatch is unneeded. → Delete both `!important`s.\n- D3 [HIGH] [tiny-text] (confidence: 10/10) `styles.css:7` — `body { font-size: 14px; }` — base body text under 16px (and this diff *removes* the previous `body { font-size: 16px; }`). → `font-size: 16px` (or `1rem`).\n- D4 [HIGH] [tiny-text] (confidence: 9/10) `styles.css:67` — `.small-link { font-size: 11px; }` — 11px link text is below any readable body floor. → Bump to ≥ 14px for a legal footer link, 16px preferred.\n\n**NEEDS INPUT:**\n- D5 [HIGH] Blacklisted font (confidence: 10/10) `styles.css:6` — `font-family: 'Papyrus', sans-serif;` — Papyrus is on the blacklist, and the fallback is a bare generic.\n Recommended fix: pick a real typeface with a proper stack (e.g. a self-hosted or system-available serif/sans, then generic fallback). Avoid the overused-default list too (Inter, Roboto, Poppins…).\n- D6 [MEDIUM] [ai-color-palette] (confidence: 9/10) `styles.css:14` — `linear-gradient(135deg, #6366f1, #8b5cf6)` is the canonical indigo→violet AI gradient. The whole palette follows it: button `#6366f1` (:57), icon circle `#ede9fe` (:49), footer `#1e1b4b` (:80).\n Recommended fix: one solid brand colour the palette owns for the hero and CTA; drop the gradient.\n- D7 [MEDIUM] Generic hero copy (confidence: 10/10) `landing.html:12-13` — \"Welcome to Our Platform\" / \"Your all-in-one solution for everything you need\" — two of the checklist's literal grep strings, plus `Our Platform` (:7).\n Recommended fix: say what the product does and for whom in the h1; make the subhead a concrete claim.\n- D8 [MEDIUM] [marketing-buzzword] (confidence: 9/10) `landing.html:32`, `landing.html:37` — \"streamline your workflow effortlessly\" (streamline + effortless) and \"Unlock the power of our platform today\" (unlock + a listed generic-copy phrase). Feature descriptions at :23/:28 (\"will change your life\", \"sets us apart from the competition\") are placeholder filler.\n Recommended fix: replace with specific outcomes/numbers per feature.\n- D9 [MEDIUM] \"Get Started\" as the only CTA (confidence: 9/10) `landing.html:14` — the page has exactly one button and its label is \"Get Started\".\n Recommended fix: name the outcome the click buys (\"Start a free project\", \"See pricing\", …).\n- D10 [MEDIUM] Centered everything (confidence: 9/10) `styles.css:15,21,26,35,41,77` — `text-align: center` on `.hero`, `.hero h1`, `.hero p`, `.features`, `.feature-card`, `.footer` — 6 of 6 text containers (100%, threshold 60%). The h1/p rules are also redundant with the parent.\n Recommended fix: left-align body copy and feature descriptions; center at most the hero.\n- D11 [HIGH] Heading hierarchy skips a level (confidence: 10/10) `landing.html:12` → `landing.html:21,26,31` — `

` followed directly by three `

`s with no `

`.\n Recommended fix: change the feature titles to `

` (or add a section `

` above the grid and keep h3s).\n- D12 [MEDIUM] Missing hover/focus states (confidence: 9/10) `styles.css:55-63`, `styles.css:66-69` — no `:hover`, `:focus` or `:focus-visible` rule anywhere in the file for `button` or `.small-link`; combined with D1 the button has zero interaction feedback.\n Recommended fix: add `:hover` (colour shift) and `:focus-visible` (ring) for both.\n- D13 [MEDIUM] Grid has no responsive breakpoint (confidence: 9/10) `styles.css:32` — `grid-template-columns: repeat(3, 1fr)` with `padding: 60px 40px`, `gap: 24px`, and `.feature-card { padding: 32px }` and no `@media`. At 360px the content column is ~0px wide; cards collapse on phones.\n Recommended fix: `grid-template-columns: repeat(auto-fit, minmax(16rem, 1fr))` or a single column under a breakpoint.\n- D14 [MEDIUM] Missing `max-width` on text containers (confidence: 8/10) `styles.css:25-28`, `styles.css:39-44` — `.hero p` and `.feature-card p` have no `max-width`; no `max-width` appears anywhere in the diff, so hero copy runs full-bleed on wide screens.\n Recommended fix: `max-width: 60ch; margin-inline: auto;` on the hero paragraph and a content wrapper.\n- D15 [MEDIUM] Unicode glyphs as icons (confidence: 8/10) `landing.html:20,25,30` — `★` ★, `⚡` ⚡, `⚙` ⚙ used as decorative icons inside the coloured circles; rendering varies by platform (⚡ renders as colour emoji on most systems) and they carry no semantics.\n Recommended fix: inline SVG icons with `aria-hidden=\"true\"`, or drop the icon slot entirely.\n\n**POSSIBLE (verify visually):**\n- D16 [LOW] 3-column feature grid (confidence: 9/10 on pattern, LOW-tier by checklist) `landing.html:18-34` + `styles.css:30-44` — exactly three symmetric children, each icon-in-circle + heading + 2-line paragraph. This is the checklist's \"THE most recognizable AI layout\". Verify with /design-review; consider a different rhythm (one real screenshot + copy, or an asymmetric list).\n- D17 [LOW] Icons in coloured circles as decoration `styles.css:46-54` — `.icon-circle` is `border-radius: 50%` + `background: #ede9fe` around a glyph. SaaS-template look; verify visually.\n- D18 [LOW] Touch target under 44px `styles.css:66-69` — `.small-link` at 11px + `4px 8px` padding ≈ 27px tall; `button` at 14px + `12px 24px` ≈ 41px. Both under 44px; verify with computed sizes.\n\nNot flagged (below threshold): border-radius uniformity — `24px` on 2 of 3 radiused elements (67%, threshold 80%); default-themed `::selection`/`accent-color`/scrollbars — worth doing once a real palette exists (D6), not a standalone finding here.\n\n---\n\n## Summary\n\n| Area | Count | AUTO-FIX tier | Needs input | Possible |\n|---|---|---|---|---|\n| Code review | 5 | 2 | 3 | 0 |\n| Design review | 18 | 4 | 11 | 3 |\n\nAll 7 planted anti-patterns in the fixture comments were caught (Papyrus, 14px body, `outline: none`, `!important`, purple gradient, generic hero copy, 3-column grid), plus 16 unannounced ones — the most consequential being the near-invisible footer link (UA link colour on dark indigo), the missing responsive breakpoint on the grid, and the total absence of hover/focus states.\n\nNo fixes were applied to `styles.css` or `landing.html`. Never commit/push from /review — that's /ship's job.\n" + }, + { + "run": "36629958451", + "trial": "t1", + "scanRan": false, + "report": "# /review — feature/add-landing-page vs main\n\nBase branch: `main` (no `origin` remote; git-native fallback). Diff: `landing.html` (+41), `styles.css` (+86/−1).\nGreptile step skipped (no PR). No `DESIGN.md` → universal design principles. No `TODOS.md`.\nNote: the source is annotated \"Planted design anti-patterns for E2E eval\" — findings were **written, not applied**. Every AUTO-FIX row below is mechanical and can be applied on request.\n\n---\n\n## Pre-Landing Review: 8 issues (0 critical, 8 informational)\n\nPass 1 (SQL / Race / LLM trust / Shell / Enum): not applicable — static HTML+CSS only.\n\n**AUTO-FIXED (eligible — not applied):**\n- [INFORMATIONAL] (confidence: 9/10) styles.css:5-7 — Diff **deletes** the pre-existing `body { font-size: 16px; }` and replaces it with `14px`; this is a regression on a shared stylesheet, not just a new rule. → Restore `font-size: 16px` (or `1rem`).\n- [INFORMATIONAL] (confidence: 9/10) styles.css:1-3,12,30,45,56,70 — `/* Planted design anti-patterns for E2E eval — 7 issues */` and `/* Issue N: ... */` comments, plus `` in landing.html:10,17, ship to users via view-source. → Remove eval scaffolding comments before landing.\n- [INFORMATIONAL] (confidence: 8/10) landing.html:14 — `