feat(reviews): ui-reviewer agent + visual gate for UI PRs (bookshelf-g3y8) #624

Merged

zombor merged 2 commits from bd-bookshelf-g3y8 into main 2026-06-19 00:38:35 +00:00

Conversation 2 Commits 2 Files changed 3 +153

zombor commented 2026-06-19 00:22:45 +00:00

Owner

Copy link

Adds a ui-reviewer agent that Reads a rendered screenshot and judges spacing/alignment/typography/token consistency + canonical-component reuse, reporting per review-standard. Documents the UI-PR convention in review-standard.md (screenshot gate + dispatch ui-reviewer alongside code+security on templates/static-css PRs). Registers the agent in CLAUDE.md.

Closes the recurring gap where diff-review + unit tests pass but the rendered UI is visually off or re-invents canonical components (#603 bespoke modal classes, #622 hand-rolled email modals).

Files: .claude/agents/ui-reviewer.md (new), .claude/rules/review-standard.md, CLAUDE.md. Docs/config only — no Go change.

Closes bead bookshelf-g3y8 on merge.

Adds a `ui-reviewer` agent that Reads a rendered screenshot and judges spacing/alignment/typography/token consistency + canonical-component reuse, reporting per review-standard. Documents the UI-PR convention in review-standard.md (screenshot gate + dispatch ui-reviewer alongside code+security on templates/static-css PRs). Registers the agent in CLAUDE.md. Closes the recurring gap where diff-review + unit tests pass but the rendered UI is visually off or re-invents canonical components (#603 bespoke modal classes, #622 hand-rolled email modals). Files: .claude/agents/ui-reviewer.md (new), .claude/rules/review-standard.md, CLAUDE.md. Docs/config only — no Go change. Closes bead bookshelf-g3y8 on merge.

zombor added 1 commit 2026-06-19 00:22:45 +00:00

feat(reviews): ui-reviewer agent + visual gate for template/CSS PRs (bookshelf-g3y8) ... 9a00f4a066

Adds a ui-reviewer agent (.claude/agents/ui-reviewer.md) that Reads a rendered
screenshot and judges spacing/alignment/typography/token consistency and
canonical-component reuse, reporting per review-standard format. Documents the
UI-PR convention in review-standard.md: PRs touching templates/ or static/css/
post a rendered screenshot and the orchestrator dispatches ui-reviewer alongside
code+security review. Closes the recurring gap where diff-review + unit tests
pass but the rendered UI is visually off or re-invents canonical components
(#603, #622).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

zombor commented 2026-06-19 00:26:49 +00:00

Author

Owner

Copy link

Security Review — bookshelf-g3y8 / PR #624

DOCS/CONFIG-ONLY PR: adds .claude/agents/ui-reviewer.md, a UI-review section in .claude/rules/review-standard.md, and a one-line CLAUDE.md agents-list entry. No application code in this diff.

Bash tool grant — The agent is granted Bash. The only documented use is: read the Forgejo API token via an awk one-liner (identical to every other agent in the project), curl -sk -L -H "Authorization: token $TOKEN" to download a screenshot from the project's own Forgejo instance (git.zombor.net) into /tmp/uirev.png, then pass that file to Read. The token is used solely as an Authorization header; it is never echoed, logged, or posted anywhere. No destructive commands (no git reset, no rm -rf, no migrate-down, no secret-dumping). The grep in Step 3 targets changed template/CSS files only.

Secret/credential handling — No instruction to surface credentials in PR comments or logs. Token stays in a shell variable scoped to the curl invocation. Pattern is identical to the existing code-reviewer/merge-supervisor agents.

Existing security controls — review-standard.md adds a third parallel agent alongside (not instead of) code+security reviews. All existing orchestrator reaction rules (blocker/major/minor thresholds, auto-fix on MAJOR, CI-green gate) are preserved verbatim. No rule is weakened or bypassed.

No application code — Zero Go, SQL, HTML templates, CSS, or CI workflow changes in the diff. Attack surface is nil.

REVIEW VERDICT: 0 blocker, 0 major, 0 minor

**Security Review — bookshelf-g3y8 / PR #624** DOCS/CONFIG-ONLY PR: adds `.claude/agents/ui-reviewer.md`, a UI-review section in `.claude/rules/review-standard.md`, and a one-line CLAUDE.md agents-list entry. No application code in this diff. **Bash tool grant** — The agent is granted `Bash`. The only documented use is: read the Forgejo API token via an awk one-liner (identical to every other agent in the project), `curl -sk -L -H "Authorization: token $TOKEN"` to download a screenshot from the project's own Forgejo instance (`git.zombor.net`) into `/tmp/uirev.png`, then pass that file to `Read`. The token is used solely as an Authorization header; it is never echoed, logged, or posted anywhere. No destructive commands (no `git reset`, no `rm -rf`, no `migrate-down`, no secret-dumping). The `grep` in Step 3 targets changed template/CSS files only. **Secret/credential handling** — No instruction to surface credentials in PR comments or logs. Token stays in a shell variable scoped to the curl invocation. Pattern is identical to the existing code-reviewer/merge-supervisor agents. **Existing security controls** — `review-standard.md` adds a *third* parallel agent `alongside` (not instead of) code+security reviews. All existing orchestrator reaction rules (blocker/major/minor thresholds, auto-fix on MAJOR, CI-green gate) are preserved verbatim. No rule is weakened or bypassed. **No application code** — Zero Go, SQL, HTML templates, CSS, or CI workflow changes in the diff. Attack surface is nil. REVIEW VERDICT: 0 blocker, 0 major, 0 minor

zombor commented 2026-06-19 00:27:08 +00:00

Author

Owner

Copy link

CODE REVIEW: APPROVED (1 minor)

Phase 0: DEMO Verification

This is a docs/config-only PR with no DEMO block — confirmed correct for this PR type. Proceeding directly to content review.

Phase 1: Front-matter format consistency

ui-reviewer.md front-matter:

name: ui-reviewer
description: Visual UI review of a RENDERED screenshot...
model: sonnet
tools:
  - Read
  - Glob
  - Grep
  - Bash

Compared against code-reviewer.md (name/description/model/tools) and merge-supervisor.md (name/description/model/tools): structure is identical. model: sonnet is consistent with the project dispatch convention (dispatch-model-sonnet.md). Tools listed (Read/Glob/Grep/Bash) match the agent's described process — Read for viewing PNGs, Bash for the curl download, Grep for source-level checks. No Write/Edit tools included, which is correct per the "NEVER write or edit code" anti-rubber-stamp rule.

Phase 2: Content, references, and factual accuracy

Referenced files/classes verified to exist:

• templates/partials/modal_shell.html — exists (find confirmed)
• .modal-overlay, .modal-dialog, .modal-header, .modal-body, .modal-footer — all defined in static/css/main.css
• .metadata-field, .metadata-field-input — defined in static/css/main.css
• .btn, .btn-primary, .btn-small, .btn-ghost — defined in static/css/main.css
• --space-* tokens (--space-1 through …) — defined in static/css/main.css
• --fg-muted, --border CSS variables — defined in static/css/main.css

One factual inaccuracy found:

[MINOR] .claude/agents/ui-reviewer.md:55 — .btn-secondary listed as a canonical class but does not exist
ui-reviewer.md line 55 lists Buttons MUST use .btn/.btn-primary/.btn-secondary/.btn-small/.btn-ghost. Grepping static/css/main.css and all templates shows no .btn-secondary definition — the actual classes are .btn (base), .btn-primary (alias for .btn), .btn-small/.btn-sm, .btn-ghost, .btn-icon, .btn-danger-ghost. A ui-reviewer following this list might flag uses of the real classes as non-canonical or incorrectly accept a truly invented .btn-secondary. Fix: remove .btn-secondary from the list (or replace with .btn-sm as the alias for .btn-small).

Process consistency with review-standard.md:

The agent's output format uses [BLOCKER]/[MAJOR]/[MINOR] + file:line + REVIEW VERDICT: <B> blocker, <M> major, <m> minor — exactly matching the required format in review-standard.md. Severity guidance in the new section (bespoke component = MAJOR; inline style= = MAJOR; unreachable control = BLOCKER; polish nits = MINOR) does not contradict the existing severity definitions — it is a narrowing/specialization for the UI domain, not a redefinition.

review-standard.md UI review section:

The new section correctly references all four memories: post-ui-screenshots-in-pr, ui-reuse-canonical-components, ui-interactive-needs-browser-test, and orchestrator-eyeball-ui-screenshots. The statement that the orchestrator dispatches the ui-reviewer agent alongside (not instead of) code+security reviews is consistent with the existing "three parallel agents" convention. The section does not override or contradict any existing orchestrator reaction rules.

CLAUDE.md agents list:

The new entry reads: ui-reviewer — visual review of a rendered screenshot (spacing, alignment, canonical-component reuse) for template/CSS PRs; dispatch alongside code+security review (see .claude/rules/review-standard.md → UI review). This is accurate, concise, and consistent with the existing two-agent list entries in style.

---

REVIEW VERDICT: 0 blocker, 0 major, 1 minor

## CODE REVIEW: APPROVED (1 minor) ### Phase 0: DEMO Verification This is a docs/config-only PR with no DEMO block — confirmed correct for this PR type. Proceeding directly to content review. ### Phase 1: Front-matter format consistency `ui-reviewer.md` front-matter: ```yaml name: ui-reviewer description: Visual UI review of a RENDERED screenshot... model: sonnet tools: - Read - Glob - Grep - Bash ``` Compared against `code-reviewer.md` (name/description/model/tools) and `merge-supervisor.md` (name/description/model/tools): structure is identical. `model: sonnet` is consistent with the project dispatch convention (`dispatch-model-sonnet.md`). Tools listed (Read/Glob/Grep/Bash) match the agent's described process — Read for viewing PNGs, Bash for the `curl` download, Grep for source-level checks. No `Write`/`Edit` tools included, which is correct per the "NEVER write or edit code" anti-rubber-stamp rule. ### Phase 2: Content, references, and factual accuracy **Referenced files/classes verified to exist:** - `templates/partials/modal_shell.html` — exists (`find` confirmed) - `.modal-overlay`, `.modal-dialog`, `.modal-header`, `.modal-body`, `.modal-footer` — all defined in `static/css/main.css` - `.metadata-field`, `.metadata-field-input` — defined in `static/css/main.css` - `.btn`, `.btn-primary`, `.btn-small`, `.btn-ghost` — defined in `static/css/main.css` - `--space-*` tokens (`--space-1` through …) — defined in `static/css/main.css` - `--fg-muted`, `--border` CSS variables — defined in `static/css/main.css` **One factual inaccuracy found:** [MINOR] `.claude/agents/ui-reviewer.md:55` — `.btn-secondary` listed as a canonical class but does not exist `ui-reviewer.md` line 55 lists `Buttons MUST use .btn/.btn-primary/.btn-secondary/.btn-small/.btn-ghost`. Grepping `static/css/main.css` and all templates shows no `.btn-secondary` definition — the actual classes are `.btn` (base), `.btn-primary` (alias for `.btn`), `.btn-small`/`.btn-sm`, `.btn-ghost`, `.btn-icon`, `.btn-danger-ghost`. A ui-reviewer following this list might flag uses of the real classes as non-canonical or incorrectly accept a truly invented `.btn-secondary`. Fix: remove `.btn-secondary` from the list (or replace with `.btn-sm` as the alias for `.btn-small`). **Process consistency with review-standard.md:** The agent's output format uses `[BLOCKER]`/`[MAJOR]`/`[MINOR]` + `file:line` + `REVIEW VERDICT: <B> blocker, <M> major, <m> minor` — exactly matching the required format in `review-standard.md`. Severity guidance in the new section (bespoke component = MAJOR; inline style= = MAJOR; unreachable control = BLOCKER; polish nits = MINOR) does not contradict the existing severity definitions — it is a narrowing/specialization for the UI domain, not a redefinition. **review-standard.md UI review section:** The new section correctly references all four memories: `post-ui-screenshots-in-pr`, `ui-reuse-canonical-components`, `ui-interactive-needs-browser-test`, and `orchestrator-eyeball-ui-screenshots`. The statement that the orchestrator dispatches the `ui-reviewer` agent *alongside* (not instead of) code+security reviews is consistent with the existing "three parallel agents" convention. The section does not override or contradict any existing orchestrator reaction rules. **CLAUDE.md agents list:** The new entry reads: `ui-reviewer — visual review of a rendered screenshot (spacing, alignment, canonical-component reuse) for template/CSS PRs; dispatch alongside code+security review (see .claude/rules/review-standard.md → UI review)`. This is accurate, concise, and consistent with the existing two-agent list entries in style. --- REVIEW VERDICT: 0 blocker, 0 major, 1 minor

zombor added 1 commit 2026-06-19 00:28:07 +00:00

fix(ui-reviewer): don't hardcode wrong .btn class list; grep main.css for real set (bookshelf-g3y8) ... 22e2e910d8

Code review caught .btn-secondary listed as canonical but it doesn't exist
(codebase has .btn-ghost + BEM .btn--secondary mix). Replace the fragile
enumeration with an instruction to grep static/css/main.css for the actual
.btn* classes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

zombor force-pushed bd-bookshelf-g3y8 from 22e2e910d8 to 5d432c438d 2026-06-19 00:32:57 +00:00 Compare

zombor merged commit 520f88904a into main 2026-06-19 00:38:35 +00:00

zombor referenced this pull request from a commit 2026-06-19 00:38:36 +00:00

Merge pull request 'feat(reviews): ui-reviewer agent + visual gate for UI PRs (bookshelf-g3y8)' (#624) from bd-bookshelf-g3y8 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!624

No description provided.