feat(mcp): critique designs through reviewer personas and keep approved lessons
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Marcos Paulo
@@ -1,180 +1,110 @@
|
||||
---
|
||||
name: papercraft-critique
|
||||
description: Pre-emit critique gate for work built in PaperCraft. Five panels (designer, critic, brand, accessibility, copy) score a screen 0-10 against PaperCraft's own objective evidence (lint_design, verify_code, get_screenshot, get_design_spec, get_variables), the weighted composite must reach 8.0, and a failed gate ships the best round instead of the last one. Use when an agent is about to hand over a PaperCraft screen and wants a tool-backed review instead of a self-praise.
|
||||
description: Critique finished PaperCraft work the way a design team does - against the person's objective, through independent reviewers (the person's vision, UX heuristics, flow across screens, QA states, visual, accessibility, copy, an end user from the audience), each finding rated 0-4 and backed by tool evidence, then fixed, reported and, when the person approves, kept as a lesson. Use when finish_working_on_nodes offers a critique, when asked to review or critique a design, or before handing over a screen or flow.
|
||||
---
|
||||
|
||||
# papercraft-critique
|
||||
|
||||
A gate, not a compliment. Before a screen leaves the app it goes through five panels. Each panel is a rubric pass over the same artifact, and each panel may only cite what a PaperCraft tool actually returned. The panels produce five scores, the scores produce one composite, and the composite decides whether the work ships. Mechanics live in the `papercraft` skill, taste in `papercraft-design`; this skill is the gate.
|
||||
A critique judges a design against what it is for, not against taste. The tool `critique_design`
|
||||
hands you the evidence and the reviewers; this skill is how to run them. Mechanics live in the
|
||||
`papercraft` skill, taste in `papercraft-design`. Per-reviewer checklists:
|
||||
[personas.md](references/personas.md).
|
||||
|
||||
Reference: [panels.md](references/panels.md) (what each panel inspects, which tool backs it, its 0–10 bands).
|
||||
## 1. When
|
||||
|
||||
## 1. When the gate runs
|
||||
* `finish_working_on_nodes` returns `next`: ask the person (`ask_user`) whether to critique now.
|
||||
* The person asks "review this", "is it ready?", "critique it".
|
||||
* Before handing over a whole screen or flow. Not for one component, an early draft or a copy tweak.
|
||||
|
||||
Run it when the resolver (section 2) says on, or when the request is a review of finished work:
|
||||
## 2. Objective first
|
||||
|
||||
* a whole screen is about to be handed over: landing page, dashboard, sign-in, settings, an onboarding flow, a whiteboard plan;
|
||||
* a redesign of an existing screen, after the edits and a clean `lint_design`;
|
||||
* the user asked "does this look right?", "review this", "critique it", "is it ready to ship?".
|
||||
Critique without an objective is opinion. Before judging, have:
|
||||
|
||||
Do not run it for: a single component or icon, an exploratory draft before a direction is chosen (the `papercraft-design` section 0 step), or a one-line copy tweak. The gate costs a lint and a screenshot per round; spend it on work that is finished.
|
||||
* **Objective**: what the work must achieve ("a returning customer reorders in two taps").
|
||||
* **Audience**: who uses it, in what context (phone, in a hurry, first visit, low vision).
|
||||
* **Top tasks**: the two to five things people come to do.
|
||||
|
||||
If the gate is on for a run, it must produce a status. Never skip it silently: report `Degraded` with the reason.
|
||||
Take them from the brief, the spec, the person's notes (`get_feedback`) and references. Missing?
|
||||
Ask with `ask_user` and pass them to `critique_design {objective, audience}`.
|
||||
|
||||
## 2. Enable resolver
|
||||
## 3. The reviewers
|
||||
|
||||
Four levels, highest priority first. Take the first level that gives an explicit answer; "unset" falls through to the next one. A higher level saying *off* is final — a lower level cannot turn the gate back on.
|
||||
|
||||
| Order | Level | PaperCraft equivalent | How to read it |
|
||||
|---|---|---|---|
|
||||
| 1 | Per-task policy | the instruction the agent was given for this run | "run the critique gate before you finish", "review this screen" |
|
||||
| 2 | Per-project setting | a toggle stored in the project's own metadata | **not implemented today** — PaperCraft has no project-level flag. Treat this level as unset and fall through to the environment flag or the per-task instruction |
|
||||
| 3 | Environment flag | `PAPERCRAFT_CRITIQUE_ENABLED` | `1` / `true` / `always` = on, `0` / `false` / `never` = off, unset = fall through |
|
||||
| 4 | Rollout phase | the release phase of the skill itself | off by default: the gate ships disabled until a run or a project turns it on |
|
||||
|
||||
So today the gate turns on in one of two ways: the user asks for it in the run's instructions, or `PAPERCRAFT_CRITIQUE_ENABLED=1` is in the environment. A future project setting — stored with the project, not with the user — would sit between those two and win over the environment, so a project could opt in permanently without an env var.
|
||||
|
||||
## 3. Order of operations
|
||||
|
||||
Objective checks first, subjective panels second. A panel never scores above evidence that contradicts it.
|
||||
|
||||
One round is: **evidence → repair → judge → score**.
|
||||
|
||||
1. **Snapshot.** Note the `pageId` and the artboard `nodeId` under review and keep them for the whole gate. A page switch between rounds invalidates the evidence.
|
||||
2. **Evidence.** `lint_design` (`nodeId`, `pageId`, no `fix` yet) for the raw findings — the payload counts `errors`, `warnings` and `info`, groups them in `byRule`, and lists each issue with its `rule`, `severity`, `node`, `message` and `fix`; `get_screenshot` of the same node (`scale: 2` to read small text); `get_variables` for the token set and the active mode; `get_design_spec` (`nodeId`, `target`) when the design is headed for code. If there is code to check, `verify_code` (`nodeId`, `target`, `code`).
|
||||
3. **Repair the hard evidence.** Fix every `lint_design` error and warning and re-run the lint until it is clean (`lint_design {changedOnly: true}` between fixes, one full pass at the end; see `papercraft/references/incremental-checks.md`); fix every `verify_code` violation until `passed` is true. A panel cannot be scored on a design that still carries a lint error.
|
||||
4. **Judge the five panels** on the evidence from steps 2–3 plus the screenshot. Every finding cites the tool result or the screenshot region behind it.
|
||||
5. **Score.** Five scores 0–10, then the composite (section 5). Composite ≥ 8.0 → status `Shipped`, stop, `finish_working_on_nodes`.
|
||||
6. **Otherwise fix and go again.** Take the findings in composite-impact order, fix them, run round N+1. Maximum 3 rounds.
|
||||
7. **Ship the best round** if round 3 is still below the threshold (section 6).
|
||||
|
||||
The panels are rubric passes by the agent that holds the tools: one agent, five passes, five different rubrics. A runtime that can spawn subagents may run the panels in parallel, but the evidence vetoes, the composite and the round counter belong to the coordinator.
|
||||
|
||||
## 4. Hard evidence vetoes a panel
|
||||
|
||||
`lint_design` and `verify_code` are not suggestions. Each finding belongs to one panel, and that panel cannot score above the band its evidence forces.
|
||||
|
||||
| Evidence | Rule | Owner panel | Cap |
|
||||
|---|---|---|---|
|
||||
| `lint_design` error | `contrast` | Accessibility (the Critic reads it too) | 3.9 |
|
||||
| `lint_design` warning | `text-size` (under 12px) | Accessibility | 6.9 |
|
||||
| `lint_design` warning | `spacing-grid` (off the 4px grid) | Designer | 6.9 |
|
||||
| `lint_design` warning | `touch-target` (under 44px) | Accessibility | 6.9 |
|
||||
| `lint_design` warning | `layer-name` (default names) | Designer | 6.9 |
|
||||
| `lint_design` warning | `hard-coded-color` | Brand | 6.9 |
|
||||
| `lint_design` info | `type-scale` | Designer | none — must be listed |
|
||||
| `verify_code` error | `missing-text` | Copy | 3.9 |
|
||||
| `verify_code` error | `hardcoded-color` | Brand | 3.9 |
|
||||
| `verify_code` error | `forbidden-absolute` | Designer | 3.9 |
|
||||
| `verify_code` error | `missing-layout` | Designer | 3.9 |
|
||||
| `verify_code` warning | `missing-component` | Brand | 6.9 |
|
||||
| `verify_code` warning | `spacing-drift` | Designer | 6.9 |
|
||||
|
||||
An error caps the owning panel at 3.9 (Blocker band), a warning at 6.9 (Weak band), an `info` item caps nothing but must appear in the panel's notes. `hard-coded-color` arrives in both flavours: a warning when the color equals a variable that exists, an info when it is a literal used several times — the warning caps Brand, the info does not. The cap lifts as soon as the finding is gone; the next round re-scores the panel freely.
|
||||
|
||||
A finding is evidence for its owner and a hint for the others: `contrast` also informs the Critic, `spacing-grid` also informs the Brand (was it the token or the layout?), but only the owner is capped.
|
||||
|
||||
## 5. The panels and the composite
|
||||
|
||||
| Panel | Judges | Weight |
|
||||
| Persona | Asks | Method |
|
||||
|---|---|---|
|
||||
| Designer | layout, composition, hierarchy, spacing rhythm, type scale in use | **0 in v1** — it still scores and its findings still drive the next round |
|
||||
| Critic | does it meet the brief; contrast, weight, readability | 0.4 |
|
||||
| Brand | token compliance, on-brand color, voice | 0.2 |
|
||||
| Accessibility | WCAG, focus rings, semantic structure, alt text | 0.2 |
|
||||
| Copy | voice, tone, terseness, error-message quality | 0.2 |
|
||||
| `vision` | Does it achieve the objective, as the person imagined it? | Objective, element, effect, why (Connor and Irizarry) |
|
||||
| `ux` | Can people use it without help? | Nielsen's 10 heuristics |
|
||||
| `flow` | Do the screens make one coherent journey per task? | Task walkthrough across screens, 4 questions per step |
|
||||
| `qa` | What breaks with real data and states? | State and edge-case matrix, pass/fail/n.a. |
|
||||
| `visual` | Does the eye land right, does it hold together? | Hierarchy, balance, contrast, gestalt, rhythm, tokens |
|
||||
| `accessibility` | Can everyone use it? | W3C POUR applied to a design |
|
||||
| `copy` | Is every word clear and specific? | Plain language, labels, errors, empty states |
|
||||
| `end-user` | Where would this user get stuck or give up? | Persona walkthrough of the top task |
|
||||
|
||||
```
|
||||
composite = critic x 0.4 + brand x 0.2 + accessibility x 0.2 + copy x 0.2
|
||||
```
|
||||
`vision` always runs: the person's intent is the yardstick for the others. Pick the rest with
|
||||
`personas` (default: all). The person's own words outrank any reviewer.
|
||||
|
||||
The weights sum to 1.0, so the composite is also 0–10. The Designer carries weight 0 in v1 on purpose: it is the panel most likely to argue with itself, so its score is reported and its findings are acted on, but it does not move the composite.
|
||||
## 4. Independence
|
||||
|
||||
Each panel scores 0–10 in half-point steps. The composite is the weighted sum, rounded to two decimals.
|
||||
Several reviewers find far more than one, but only when they work apart (Nielsen: one
|
||||
evaluator finds about a third of the problems, three to five most of them). Reviewers who
|
||||
discuss or debate converge and amplify each other's bias.
|
||||
|
||||
| Composite | Verdict |
|
||||
* **With subagents**: one subagent per persona, given only its persona brief, the objective and
|
||||
the evidence. No reviewer sees another's findings before rating.
|
||||
* **Alone**: one pass per persona, in order, writing that persona's findings before reading the
|
||||
next brief. Do not revise earlier personas' findings during later passes.
|
||||
* **You coordinate**: you merge and judge; you never let reviewers argue a finding out.
|
||||
|
||||
## 5. Evidence
|
||||
|
||||
From `critique_design`: lint (fix every error and warning first), the `flow` graph (arrows between
|
||||
screens, `deadEnds`, `unreached`), the lessons. Then `get_screenshot` each artboard at scale 1
|
||||
(2 for small text) in every variable mode, `get_bounds` for overflow, `get_feedback` for the
|
||||
person's notes. Every finding cites one of these: a tool result or a screenshot region. No
|
||||
evidence, no finding.
|
||||
|
||||
## 6. A finding
|
||||
|
||||
`persona`, `heuristic or check`, `element` (layer name), `seen` (the observation), `effect` (on
|
||||
the objective), `evidence`, `severity`, `suggestion`. Keep the observation and the suggestion
|
||||
apart: critique is analysis, not brainstorming. Each persona also names at least one thing that
|
||||
works and should stay.
|
||||
|
||||
Severity (Nielsen), from frequency, impact and persistence:
|
||||
|
||||
| | |
|
||||
|---|---|
|
||||
| ≥ 8.0 | Ship |
|
||||
| < 8.0 | Not shippable as is: fix and re-run, or ship the best round after round 3 |
|
||||
| 0 | not a problem |
|
||||
| 1 | cosmetic: fix if time allows |
|
||||
| 2 | minor: low priority |
|
||||
| 3 | major: fix before handing over |
|
||||
| 4 | catastrophe: blocks the task, fix now |
|
||||
|
||||
The band legend is the same for every panel:
|
||||
Every broken lesson (`lessons`) is a finding of at least 3.
|
||||
|
||||
| Band | Range | Means |
|
||||
|---|---|---|
|
||||
| Blocker | 0.0–2.9 | Broken: the panel's own evidence is violated. Cannot ship |
|
||||
| Poor | 3.0–4.9 | Works against the brief or the brand; ships visible damage |
|
||||
| Weak | 5.0–6.9 | Passable but noticeably off; expect a second round |
|
||||
| Acceptable | 7.0–7.9 | No blocker, but a reviewer would ask for one change |
|
||||
| Good | 8.0–9.4 | Ships as is |
|
||||
| Excellent | 9.5–10.0 | Reference quality for this project |
|
||||
## 7. Merge, fix, report
|
||||
|
||||
What each panel inspects, which tool backs it and where its scores land: [panels.md](references/panels.md).
|
||||
1. **Merge** into one list: join duplicates, count the personas that found each (two or more is
|
||||
confirmed), severity is the mean of their independent ratings.
|
||||
2. **Fix** every 3 and 4 with the smallest edit (`update_styles`, `set_text_content`), then check
|
||||
that finding again with the same evidence. The work is ready when no 3 or 4 is open and lint is
|
||||
clean. Three fix rounds at most; then hand over with the open findings stated.
|
||||
3. **Report** to the person: what works, then findings by severity, each with what was seen, the
|
||||
evidence, the suggestion and whether it was fixed. Phrase open ones as questions about the
|
||||
objective ("How does a first-time user find Reorder from here?"), not verdicts.
|
||||
|
||||
## 6. Rounds and the fallback
|
||||
## 8. Lessons
|
||||
|
||||
* At most **3 rounds** per gate. The counter is per gate, not per panel.
|
||||
* A round ends when the composite is computed.
|
||||
* Rounds 2 and 3 exist to fix what the panels found. Fix in composite-impact order: the panel with the largest `weight x (10 - score)` first.
|
||||
* **`fallbackPolicy: ship_best`.** If the third round is still below 8.0, the status is `Below threshold` and what ships is the best round, not the last one. Best = highest composite; a tie goes to the earlier round (fewer changes). A later round edited the canvas, so put the canvas back into the best round's state before handing over — undo back to it, or re-apply that round's fix list in order.
|
||||
* Ship the best round's artifact and report the failing score. `Below threshold` is a status, not a licence to hide the number.
|
||||
Ask the person with `ask_user` which findings to remember, one question per finding: for every
|
||||
project, for this project, or not. Save only what they approve, with
|
||||
`design_lessons {add: {rule, why, scope}}`: the rule is one line written as an instruction
|
||||
("Screens keep their device height; the main column scrolls"), `why` is what went wrong. Never
|
||||
save a lesson the person did not approve, and never save praise. Lessons come back in
|
||||
`get_basic_info` and in the next `critique_design`; `design_lessons {remove: "L3"}` drops one.
|
||||
|
||||
## 7. Status vocabulary
|
||||
## Sources
|
||||
|
||||
Every gate ends with exactly one status.
|
||||
|
||||
| Status | When | What ships |
|
||||
|---|---|---|
|
||||
| `Shipped` | composite ≥ 8.0 | the reviewed artifact, unchanged |
|
||||
| `Below threshold` | still < 8.0 after round 3 | the best-scoring round's artifact (`ship_best`), with the score reported |
|
||||
| `Timed out` | the gate ran out of its budget (rounds or minutes) before a verdict | the best round so far, with the timeout stated |
|
||||
| `Interrupted` | the user or the session stopped the gate mid-run | the last complete round's artifact |
|
||||
| `Degraded` | a panel could not be judged properly (section 8) | the artifact, but the verdict is provisional and the reason is named |
|
||||
|
||||
`Degraded` outranks `Shipped`: a degraded panel is capped at 6.9, so a degraded run reports `Degraded`, not `Shipped`, even when the composite clears 8.0.
|
||||
|
||||
## 8. Degraded reasons
|
||||
|
||||
A panel that cannot be judged on real evidence is not scored as if it were fine. It is capped at 6.9, the composite is computed with the cap, and the run reports `Degraded` with the reason and the panel it hit.
|
||||
|
||||
| Reason | Cause | Handling |
|
||||
|---|---|---|
|
||||
| `malformed_block` | a tool returned something the gate could not parse (lint payload in an unexpected shape, a truncated screenshot answer) | retry the tool once; if it still fails, judge the panel on the evidence you do have and name the missing part |
|
||||
| `oversize_block` | the evidence was too big to read whole (a screenshot scaled down past legibility, a tree deeper than the context, a lint payload whose `truncated` flag is set) | narrow the scope: screenshot or lint a subtree, split the screen into regions and judge region by region |
|
||||
| `adapter_unsupported` | the requested target or format is not supported here (a `verify_code` target the project does not build) | drop that check; the panel falls back to the screenshot plus the design rules |
|
||||
| `protocol_version_mismatch` | the MCP server exposes a different tool set than the gate expects (a tool renamed, an argument changed) | stop using the broken tool, finish what you can, report `Degraded`; do not guess the schema |
|
||||
| `missing_artifact` | the thing under review is gone (artboard deleted, page switched, stale node id) | re-snapshot with `get_basic_info` and restart the gate; if the artifact is truly gone, report `Degraded` |
|
||||
|
||||
Degradation is a report, not an excuse: name the reason, the panel and the evidence you lost.
|
||||
|
||||
## 9. What the gate reports
|
||||
|
||||
End the gate with the status, the numbers and the findings, in that order:
|
||||
|
||||
```
|
||||
Critique gate - Sign in (page p1, artboard 3fa9c1:7)
|
||||
Round 2 of 3 - composite 8.44 / threshold 8.00 - status Shipped
|
||||
Designer 7.5 (weight 0) | Critic 8.5 | Brand 8.5 | Accessibility 8.5 | Copy 8.2
|
||||
Composite = 8.5x0.4 + 8.5x0.2 + 8.5x0.2 + 8.2x0.2 = 8.44
|
||||
```
|
||||
|
||||
Then, per finding: the panel, what was seen, the evidence behind it (tool and finding, or the region of the screenshot), the fix, and whether it was fixed in this round or is still open.
|
||||
|
||||
Machine-readable form, when the caller wants JSON:
|
||||
|
||||
```json
|
||||
{
|
||||
"status": "Shipped",
|
||||
"threshold": 8.0,
|
||||
"composite": 8.44,
|
||||
"round": 2,
|
||||
"rounds": [{ "round": 1, "composite": 7.35 }, { "round": 2, "composite": 8.44 }],
|
||||
"scores": { "designer": 7.5, "critic": 8.5, "brand": 8.5, "accessibility": 8.5, "copy": 8.2 },
|
||||
"weights": { "designer": 0.0, "critic": 0.4, "brand": 0.2, "accessibility": 0.2, "copy": 0.2 },
|
||||
"degraded": [],
|
||||
"findings": [{ "panel": "critic", "evidence": "get_screenshot: headline at 16px over a photo", "severity": "warning", "state": "fixed" }]
|
||||
}
|
||||
```
|
||||
|
||||
## 10. Attribution
|
||||
|
||||
The five-panel critique contract — the roles, the weights, the composite, the 8.0 threshold, the three rounds and the `ship_best` fallback — is inspired by OpenDesign's Critique Theater (https://github.com/nexu-io/open-design, Apache-2.0). The contract is re-implemented here in PaperCraft's own words and against PaperCraft's own tools; no text was copied.
|
||||
Adam Connor and Aaron Irizarry, *Discussing Design* (objective, element, effect, rationale);
|
||||
Nielsen Norman Group: 10 usability heuristics, severity ratings, cognitive walkthroughs, design
|
||||
critiques, visual design principles; W3C WAI accessibility principles (POUR); work on panels of
|
||||
independent LLM judges (debate amplifies bias, a coordinating judge resists it).
|
||||
|
||||
@@ -1,182 +0,0 @@
|
||||
# Panel rubrics
|
||||
|
||||
Five panels, one artifact, one set of evidence. A panel is a pass with a fixed rubric: what it inspects, which tool output backs it, what it does **not** judge, and the band each finding lands in. The panels share the band legend in [../SKILL.md](../SKILL.md) (Blocker 0–2.9, Poor 3–4.9, Weak 5–6.9, Acceptable 7–7.9, Good 8–9.4, Excellent 9.5–10).
|
||||
|
||||
## How to run one panel
|
||||
|
||||
1. Read the evidence you were handed: the lint payload, the screenshot, the design spec, the variables, the code result.
|
||||
2. Write findings. Every finding cites its evidence — the lint rule and node, the region of the screenshot, the spec's MUST rule, the `verify_code` violation.
|
||||
3. Score the panel 0–10. The score is the panel's verdict on the whole screen, not an average of its findings: two serious defects on one region cost more than two trivial ones on two regions, but a screen with three medium defects is not "Good" either.
|
||||
4. Apply the caps. Hard evidence caps the owning panel (SKILL.md section 4); degraded evidence caps it at 6.9 (SKILL.md section 8).
|
||||
|
||||
Evidence a panel may not use: taste, guesses about a user's preferences, or a claim about something no tool showed. When a panel wants to judge what no tool exposes — whether an image is meaningful, whether a flow makes sense — it raises the question as an open item instead of scoring it as a defect.
|
||||
|
||||
Tool arguments used below are the current names. Note that `lint_design`'s `scale` is the list of allowed font sizes, not a zoom; the zoom argument of `get_screenshot` is `scale: 1 | 2`.
|
||||
|
||||
---
|
||||
|
||||
## Designer — weight 0 in v1
|
||||
|
||||
**Judges:** layout and composition, alignment, spacing rhythm, hierarchy, the type scale in use, how things are grouped, screen states, overflow, density.
|
||||
|
||||
**Evidence it uses**
|
||||
|
||||
| Tool | How |
|
||||
|---|---|
|
||||
| `get_screenshot` (`nodeId`, `scale: 2`) | the primary evidence: alignment, edges, rhythm, hierarchy, clipping, empty areas |
|
||||
| `lint_design` (`nodeId`, `pageId`) | `spacing-grid`, `type-scale`, `layer-name` findings |
|
||||
| `get_design_spec` (`nodeId`, `target`) | flow and sizing (`fixed`, `hug`, `fill`), grouping, where the spacing rules were broken |
|
||||
| `get_tree_summary` (`nodeId`, `depth`) | structure depth, unnamed frames, accidental nesting |
|
||||
|
||||
**Does not judge:** contrast and WCAG (Accessibility), token and palette choices (Brand), wording (Copy).
|
||||
|
||||
**Bands**
|
||||
|
||||
| Band | What the Designer saw |
|
||||
|---|---|
|
||||
| 0.0–2.9 | Broken layout: frames overlap, content sits outside the artboard, text is clipped, the screen reads as a pile |
|
||||
| 3.0–4.9 | Poor composition: everything on the default grid, equal weight everywhere, no grouping, ragged edges |
|
||||
| 5.0–6.9 | Weak: spacing off the 4px grid in places, sizes off the type scale, mixed grouping (space *and* containers everywhere), a state missing |
|
||||
| 7.0–7.9 | Acceptable: coherent, but one region is crowded or one rhythm is broken |
|
||||
| 8.0–9.4 | Good: clear hierarchy, aligned to a few strong edges, consistent rhythm, states present |
|
||||
| 9.5–10.0 | Excellent: deliberate rhythm, one memorable idea, whitespace used as a material |
|
||||
|
||||
**Fixes that move the score:** snap spacing to the 4/8 scale (`lint_design` with `fix: true`), pull one size step per level, give each group either a container or space — not both, align to one left edge, add the missing empty/loading/error state.
|
||||
|
||||
---
|
||||
|
||||
## Critic — weight 0.4
|
||||
|
||||
**Judges:** whether the screen meets the brief (the request this run is executing), whether visual weight matches the priority, readability as a human experiences it, and whether the real cases are covered.
|
||||
|
||||
**Evidence it uses**
|
||||
|
||||
| Tool | How |
|
||||
|---|---|
|
||||
| the brief | the user's request and the `papercraft-design` section 0 direction, read against the screenshot |
|
||||
| `get_screenshot` (`nodeId`, `scale: 2`) | one primary action, reading order, what is above the fold, legibility by eye |
|
||||
| `lint_design` (`nodeId`, `pageId`) | `contrast`, `text-size`, `type-scale` findings as the objective part of readability |
|
||||
| `get_design_spec` (`nodeId`, `target`) | what the screen actually contains, against what was asked for |
|
||||
| `verify_code` (`nodeId`, `target`, `code`) | when the design is code-bound: whether the implementation kept the intent |
|
||||
|
||||
**Does not judge:** which palette (Brand), line-level wording (Copy), implementation details beyond fidelity to the design.
|
||||
|
||||
**Bands**
|
||||
|
||||
| Band | What the Critic saw |
|
||||
|---|---|
|
||||
| 0.0–2.9 | Misses the brief: wrong screen or wrong content, the primary action is absent or ambiguous |
|
||||
| 3.0–4.9 | Fights the brief: right elements, wrong order or emphasis, or hard to read (contrast errors, tiny type) |
|
||||
| 5.0–6.9 | Weak: it does the job, but the weight is flat and something important sits below the fold or is buried |
|
||||
| 7.0–7.9 | Acceptable: meets the brief; one area stays unclear or the priority is not obvious |
|
||||
| 8.0–9.4 | Good: the brief is met, the primary action is obvious, readability is solid |
|
||||
| 9.5–10.0 | Excellent: unmistakable hierarchy, the screen reads in one pass, the edge cases are covered |
|
||||
|
||||
**Fixes that move the score:** make one thing clearly primary and quiet the rest, move the important content up, cut a competing element, re-read the brief line by line and check each requirement against the screenshot.
|
||||
|
||||
---
|
||||
|
||||
## Brand — weight 0.2
|
||||
|
||||
**Judges:** token compliance, on-brand color, semantic color use, mode coverage, product voice consistency, component reuse.
|
||||
|
||||
**Evidence it uses**
|
||||
|
||||
| Tool | How |
|
||||
|---|---|
|
||||
| `get_variables` (`pageId`, `format`) | the variables, tokens, active `mode` and available `modes` — the reference palette |
|
||||
| `lint_design` (`nodeId`, `pageId`) | `hard-coded-color` findings |
|
||||
| `get_computed_styles` (`nodeIds`) / `find_nodes` (`filters`) | fills and text colors that are not following a variable |
|
||||
| `get_design_spec` (`nodeId`, `target`) | which tokens the design already binds |
|
||||
| `get_components` / `get_tree_summary` | components built and instanced versus copies |
|
||||
| `verify_code` (`nodeId`, `target`, `code`) | `hardcoded-color`, `missing-component` in the implementation |
|
||||
|
||||
**Does not judge:** composition (Designer), wording line by line (Copy), whether the layout serves the brief (Critic).
|
||||
|
||||
**Bands**
|
||||
|
||||
| Band | What the Brand saw |
|
||||
|---|---|
|
||||
| 0.0–2.9 | Off-system: hard-coded colors where tokens belong, a foreign palette, no modes |
|
||||
| 3.0–4.9 | Off-brand: technically on-token but the accent is used everywhere, or the screens read as different products |
|
||||
| 5.0–6.9 | Weak: one or two hard-coded colors, the accent covers much of the screen, components copied instead of instanced |
|
||||
| 7.0–7.9 | Acceptable: on-system, with one token or component inconsistency |
|
||||
| 8.0–9.4 | Good: every color is a token, the accent marks the primary action only, modes defined and checked, components reused |
|
||||
| 9.5–10.0 | Excellent: coherent across screens and modes, the palette carries meaning |
|
||||
|
||||
**Fixes that move the score:** `lint_design` with `fix: true` relinks hard-coded colors that match a variable, add the missing variable before using the color, cut accent back to the primary action, build the repeated part once and place instances, check the second mode with `set_variable_mode`.
|
||||
|
||||
---
|
||||
|
||||
## Accessibility — weight 0.2
|
||||
|
||||
**Judges:** WCAG contrast, the text-size floor, touch-target size, focus and hover states, semantic structure, alt text, and signalling that does not rely on color alone.
|
||||
|
||||
**Evidence it uses**
|
||||
|
||||
| Tool | How |
|
||||
|---|---|
|
||||
| `lint_design` (`nodeId`, `pageId`) | `contrast`, `text-size`, `touch-target` findings |
|
||||
| `get_screenshot` (`nodeId`, `scale: 2`) | whether a visible focus or hover state was designed at all, and how the error state reads |
|
||||
| `get_design_spec` (`nodeId`, `target`) | which text is a title, how the screen is grouped, what each control is |
|
||||
| `find_nodes` (`textValue`, `filters`) | image and icon nodes, unlabeled icon buttons |
|
||||
| `get_tree_summary` (`nodeId`, `depth`) | semantic structure: heading levels, groups, control order |
|
||||
|
||||
**Does not judge:** visual taste (Designer), palette taste (Brand), voice (Copy).
|
||||
|
||||
**Bands**
|
||||
|
||||
| Band | What Accessibility saw |
|
||||
|---|---|
|
||||
| 0.0–2.9 | Fails WCAG: contrast errors, text under 12px, targets under 44px, meaning carried by color alone |
|
||||
| 3.0–4.9 | Poor: borderline contrast, unlabeled controls, an error state that is red text and nothing else |
|
||||
| 5.0–6.9 | Weak: passes on the desktop artboard but fails on the mobile one, one image without alt text |
|
||||
| 7.0–7.9 | Acceptable: compliant baseline, one gap (no visible focus state, one icon-only button) |
|
||||
| 8.0–9.4 | Good: WCAG clean, targets pass, semantics and labels present, focus and hover states designed |
|
||||
| 9.5–10.0 | Excellent: every state specified, sensible keyboard order, no meaning by color alone |
|
||||
|
||||
**Fixes that move the score:** fix the color, not the check, until `lint_design` reports no contrast error, name interactive layers `Button…` / `Tab /…` so the target rule can see them and grow them to 44px, pair every error color with an icon or a word, design the focus and hover variants as components, give every image an alt text.
|
||||
|
||||
PaperCraft has no focus-ring or alt-text checker: judge those from the design itself (a `Button / Focused` variant, alt text in the spec) and report what is missing as a gap, not as a tool finding.
|
||||
|
||||
---
|
||||
|
||||
## Copy — weight 0.2
|
||||
|
||||
**Judges:** voice and tone against the product's voice, terseness, labels and microcopy, error-message quality, empty states, placeholders versus labels, the absence of filler.
|
||||
|
||||
**Evidence it uses**
|
||||
|
||||
| Tool | How |
|
||||
|---|---|
|
||||
| `get_design_spec` (`nodeId`, `target`) | the authoritative copy: every text node's content, in order |
|
||||
| `get_screenshot` (`nodeId`, `scale: 2`) | how the copy reads in context: line breaks, truncation, all-caps abuse |
|
||||
| `find_nodes` (`textValue`) | repeated strings, leftover placeholders, "Lorem ipsum", inconsistent labels for the same action |
|
||||
| `verify_code` (`nodeId`, `target`, `code`) | `missing-text` — copy that was lost between the design and the code |
|
||||
|
||||
**Does not judge:** typography and layout (Designer), color (Brand), whether the screen meets the brief (Critic).
|
||||
|
||||
**Bands**
|
||||
|
||||
| Band | What Copy saw |
|
||||
|---|---|
|
||||
| 0.0–2.9 | Wrong: placeholder text, lorem ipsum, or copy that contradicts the brief |
|
||||
| 3.0–4.9 | Poor: filler adjectives, two voices on one screen, errors that blame the user |
|
||||
| 5.0–6.9 | Weak: wordy, mixed formality, vague labels ("Submit" where "Save changes" is meant) |
|
||||
| 7.0–7.9 | Acceptable: clear but plain; one error message says only "Error" |
|
||||
| 8.0–9.4 | Good: terse, one voice, every action labeled with a verb, errors say what happened and what to do next |
|
||||
| 9.5–10.0 | Excellent: reads like a product that shipped, not like a mockup |
|
||||
|
||||
**Fixes that move the score:** cut every word that does not inform, give each action a verb label ("Save changes", not "OK"), write errors as *what happened + what to do next*, fill empty states with an explanation and the next action, replace placeholder copy with real content, keep one formality level per screen.
|
||||
|
||||
PaperCraft has no spell-checker or copy linter: the copy panel reads the spec and the screenshot, and it flags what a human reviewer would flag — it does not invent grammar rules.
|
||||
|
||||
---
|
||||
|
||||
## Scoring discipline
|
||||
|
||||
* Half-point steps only: 7.5, 8.0, 8.5. Never 8.37.
|
||||
* The score is a verdict on the screen, not an average of findings.
|
||||
* A panel that finds nothing wrong and cites no evidence scores 7.0–7.9, not 10. A 10 means the screen is a reference for the next one.
|
||||
* Caps bind: hard evidence (SKILL.md section 4) and degraded evidence (section 8) are applied after the verdict, never negotiated with.
|
||||
* No panel may raise another panel's score. If the Critic likes the hierarchy, that stays a Designer finding.
|
||||
@@ -0,0 +1,110 @@
|
||||
# Reviewer checklists
|
||||
|
||||
Each reviewer works from its own list, alone, and cites evidence for every finding. Mark each
|
||||
item pass, fail or n/a; a state or screen that was never designed is an open question, not a pass.
|
||||
|
||||
## vision: the person's vision
|
||||
|
||||
For each key element on each screen, four questions (Connor and Irizarry):
|
||||
|
||||
1. **Objective**: which part of the objective is this element for?
|
||||
2. **Element**: which elements serve it?
|
||||
3. **Effect**: do they achieve it, for this audience?
|
||||
4. **Why** or why not?
|
||||
|
||||
Also: everything the brief, references and feedback notes asked for is there; nothing contradicts
|
||||
an open or resolved note (`get_feedback`); the tone matches the references. Unknown product facts
|
||||
are questions for the person, never guesses.
|
||||
|
||||
## ux: usability heuristics (Nielsen)
|
||||
|
||||
1. Visibility of system status: progress, saving, selection shown at once.
|
||||
2. Match with the real world: the users' words and order, not the system's.
|
||||
3. User control and freedom: a marked exit, undo, cancel.
|
||||
4. Consistency and standards: one word and one look per thing, platform conventions.
|
||||
5. Error prevention: risky actions confirmed, impossible choices removed.
|
||||
6. Recognition rather than recall: options visible, no remembering across screens.
|
||||
7. Flexibility and efficiency: shortcuts for experts that novices need not see.
|
||||
8. Aesthetic and minimalist design: nothing that competes with what matters.
|
||||
9. Recognize, diagnose, recover from errors: plain language, the problem, the fix.
|
||||
10. Help and documentation: needed rarely, and then where the task is.
|
||||
|
||||
## flow: coherent journeys
|
||||
|
||||
Walk each top task across screens (the `flow` graph when arrows connect screens, else the screen
|
||||
order and names; ask the person when the intended order is unclear). At each step, the four
|
||||
walkthrough questions: will they try to do this, will they notice the control, will they connect
|
||||
it to their goal, will they see progress afterwards?
|
||||
|
||||
* Every task has an entry point (including first run and empty state) and a clear end
|
||||
(success screen or state).
|
||||
* Every screen has a way forward and a way back or out; no dead ends (`deadEnds`), no screen
|
||||
nothing leads to (`unreached`) unless it is an entry.
|
||||
* Each action that can fail has its error state; each destructive one a confirmation and undo.
|
||||
* Navigation, back and primary actions sit in the same place with the same labels on every screen.
|
||||
* What the user entered or chose shows on the following screens (the cart count, the name typed).
|
||||
* Steps a user must remember between screens are shown instead (recognition over recall).
|
||||
* The number of steps fits the objective ("two taps" means two).
|
||||
|
||||
## qa: states and edge cases
|
||||
|
||||
Per screen and component, pass, fail or n/a:
|
||||
|
||||
* States: default, hover, focus, pressed, disabled, selected, loading, empty, error, success.
|
||||
* Content: the longest realistic text and names, minimum content, zero, one and many items,
|
||||
missing images and avatars, large numbers, special characters.
|
||||
* Modes and sizes: every variable mode (screenshot each); the narrow and wide screen sizes when the
|
||||
design has them.
|
||||
* Layout: nothing overflows or clips (`get_bounds`, lint `overflow`), screens keep their device
|
||||
height and scroll inside, fixed bars stay fixed.
|
||||
* Consistency: repeated pieces are instances of the same component (lint `not-an-instance`).
|
||||
|
||||
## visual
|
||||
|
||||
* One focal point per screen; the primary action is the most prominent control.
|
||||
* Scale and hierarchy: size, weight and colour follow importance.
|
||||
* Balance and alignment: shared edges, a steady grid.
|
||||
* Gestalt proximity: related items closer together than unrelated ones.
|
||||
* Rhythm: spacing from the 4px scale, the same gaps for the same relationships.
|
||||
* Type: sizes on the scale, at most three levels per screen.
|
||||
* Tokens: every colour a variable, consistent radii, components reused.
|
||||
|
||||
## accessibility (W3C POUR, for a design)
|
||||
|
||||
**Perceivable**
|
||||
* Text contrast 4.5:1, 3:1 for large text (18px, or 14px bold) and for control borders and icons.
|
||||
* Colour is never the only signal (errors also have an icon or text; links are underlined or bold).
|
||||
* Text is real text, not pictures of text; icons that act have a label or tooltip.
|
||||
* The layout holds at 320px wide and with text at 200% (a narrow artboard shows it).
|
||||
|
||||
**Operable**
|
||||
* Touch targets 44px (24px minimum with spacing).
|
||||
* A visible focus state is designed for every interactive component.
|
||||
* Headings, a page title and a visible current location (active nav item).
|
||||
* Destructive actions confirmed or undoable; every gesture has a button alternative.
|
||||
* Nothing flashes; motion can be reduced.
|
||||
|
||||
**Understandable**
|
||||
* Fields keep a visible label (a placeholder alone disappears).
|
||||
* Errors say what went wrong and how to fix it, next to the field.
|
||||
* Navigation and labels are the same across screens.
|
||||
* Plain language; abbreviations explained.
|
||||
|
||||
**Robust**
|
||||
* Controls are named for their role (layer names, code links to real components), so the code
|
||||
hand-off keeps them buttons and inputs, not divs.
|
||||
|
||||
## copy
|
||||
|
||||
* The users' words, not internal jargon; verbs on buttons ("Save changes", not "OK").
|
||||
* No placeholder or lorem text, no "Item 1".
|
||||
* Errors: what happened and what to do. Empty states: what this is and the first action.
|
||||
* Neutral wording about people whose gender is unknown (their name, or they).
|
||||
* Consistent terms: one word per concept across all screens.
|
||||
|
||||
## end-user: a walkthrough as the audience
|
||||
|
||||
Take the `audience` (else ask). Write as that person, in the first person, for each top task:
|
||||
what I see first, what I think it does, what I try, where I hesitate, and the first point where I
|
||||
would give up. One persona per run; launch another critique with a different `audience` for a
|
||||
second user type.
|
||||
@@ -0,0 +1,42 @@
|
||||
# Design critique and lessons
|
||||
|
||||
## Problem
|
||||
When an agent finishes a design it says it is done; nobody reviews the work unless the person asks,
|
||||
and the `papercraft-critique` skill only reaches agents that load skills (not the in-app chat). The
|
||||
same mistakes come back in the next design (a page that grows instead of scrolling, gendered copy,
|
||||
clipped labels) because nothing the person corrected is remembered.
|
||||
|
||||
## Goal
|
||||
After a design is finished the person is offered a critique; the corrections they approve are
|
||||
remembered and shown to every agent before its next design, so the same mistake is not repeated.
|
||||
|
||||
## Non-Goals
|
||||
- Scoring by a model inside the app: the agent does the judging, the tool gathers the evidence.
|
||||
- Saving anything the person did not approve.
|
||||
- A settings screen for lessons (the tool lists and removes them).
|
||||
- Syncing lessons between machines.
|
||||
|
||||
## User Stories
|
||||
- As a designer, I want to be offered a critique when an agent finishes, so that I don't have to remember to ask.
|
||||
- As a designer, I want to choose which critique findings become lessons, so that only what I agree with sticks.
|
||||
- As a designer, I want agents to follow my lessons in later designs, so that I don't correct the same thing twice.
|
||||
- As a designer, I want lessons for one product to stay in that product, so that a brand rule does not leak elsewhere.
|
||||
- As a designer, I want to remove a lesson, so that a rule I no longer want stops applying.
|
||||
|
||||
## Acceptance Criteria
|
||||
- [ ] Given edits since the last critique, when an agent calls `finish_working_on_nodes`, then the result tells it to offer the person a critique (`ask_user`), and it does not after a critique with no edits since.
|
||||
- [ ] Given a page or artboard, when `critique_design` runs, then it returns the objective (or that it is missing and must be asked), the chosen reviewers (vision, ux, flow, qa, visual, accessibility, copy, end-user) with their methods, the finding format and 0-4 severity scale, the lint, the flow between screens and the lessons that apply.
|
||||
- [ ] Given screens joined by arrows, when `critique_design` runs, then the flow lists the links, the dead ends (reached, leading nowhere) and the screens nothing leads to or from.
|
||||
- [ ] Given several reviewers, the steps have them review independently (one subagent each, or one pass each) and a coordinator merge, never debate.
|
||||
- [ ] Given the person approved a finding, when `design_lessons` adds it, then it is stored in the project store and listed by `get_basic_info` and `critique_design` from then on, in every session.
|
||||
- [ ] Given a lesson saved with scope `project`, when another project is open, then it is not listed.
|
||||
- [ ] Given a lesson id, when `design_lessons` removes it, then it is no longer listed.
|
||||
- [ ] The tool descriptions say a lesson is saved only after the person approved it.
|
||||
|
||||
## Constraints
|
||||
- Stack: Rust, MCP tools in `src/mcp`; lessons in a JSON file at the project store root, written atomically.
|
||||
- Compat: missing or unreadable lessons file means no lessons, never an error for other tools.
|
||||
- Security: lessons are plain text the person approved; no secrets.
|
||||
|
||||
## Open Questions
|
||||
- None.
|
||||
@@ -0,0 +1,224 @@
|
||||
//! A critique offered when a design is finished, and the lessons the person keeps from it
|
||||
//! (`specs/design-critique/spec.md`). `critique_design` gathers the evidence the agent judges
|
||||
//! by; `design_lessons` stores what the person approved, in the project store, so every later
|
||||
//! session sees it in `get_basic_info` and in the next critique.
|
||||
|
||||
use crate::io::atomic;
|
||||
use crate::project::Store;
|
||||
use crate::model::{Document, NodeId, NodeKind};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use serde_json::{Value, json};
|
||||
use std::path::PathBuf;
|
||||
|
||||
const FILE: &str = ".design-lessons.json";
|
||||
/// Lessons are rules read before every design: short enough to scan.
|
||||
const MAX_RULE: usize = 160;
|
||||
/// Lint issues listed in a critique; the counts cover the rest.
|
||||
pub const MAX_ISSUES: usize = 20;
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct Lesson {
|
||||
pub id: String,
|
||||
pub rule: String,
|
||||
#[serde(default, skip_serializing_if = "String::is_empty")]
|
||||
pub why: String,
|
||||
/// The project (file id) it belongs to; none for every project.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub project: Option<String>,
|
||||
}
|
||||
|
||||
fn path(store: &Store) -> PathBuf {
|
||||
store.root.join(FILE)
|
||||
}
|
||||
|
||||
/// Every stored lesson; none when the file is missing or unreadable.
|
||||
pub fn load(store: &Store) -> Vec<Lesson> {
|
||||
crate::vfs::read(path(store)).ok().and_then(|b| serde_json::from_slice(&b).ok()).unwrap_or_default()
|
||||
}
|
||||
|
||||
/// The lessons that apply in `project` (its own and the global ones).
|
||||
pub fn applying(store: &Store, project: Option<&str>) -> Vec<Lesson> {
|
||||
load(store).into_iter().filter(|l| l.project.is_none() || l.project.as_deref() == project).collect()
|
||||
}
|
||||
|
||||
fn save(store: &Store, lessons: &[Lesson]) -> Result<(), String> {
|
||||
let json = serde_json::to_vec_pretty(lessons).map_err(|e| e.to_string())?;
|
||||
atomic::write(&path(store), &json).map_err(|e| format!("could not save lessons: {e}"))
|
||||
}
|
||||
|
||||
/// Store a lesson the person approved and return it.
|
||||
pub fn add(store: &Store, rule: &str, why: &str, project: Option<String>) -> Result<Lesson, String> {
|
||||
let rule = rule.trim();
|
||||
if rule.is_empty() {
|
||||
return Err("a lesson needs a rule".into());
|
||||
}
|
||||
if rule.chars().count() > MAX_RULE {
|
||||
return Err(format!("keep the rule under {MAX_RULE} characters; put the reason in `why`"));
|
||||
}
|
||||
let mut all = load(store);
|
||||
if let Some(same) = all.iter().find(|l| l.rule.eq_ignore_ascii_case(rule) && l.project == project) {
|
||||
return Ok(same.clone());
|
||||
}
|
||||
let next = all.iter().filter_map(|l| l.id.strip_prefix('L')?.parse::<u32>().ok()).max().unwrap_or(0) + 1;
|
||||
let lesson = Lesson { id: format!("L{next}"), rule: rule.into(), why: why.trim().into(), project };
|
||||
all.push(lesson.clone());
|
||||
save(store, &all)?;
|
||||
Ok(lesson)
|
||||
}
|
||||
|
||||
/// Remove a lesson; false when no lesson has that id.
|
||||
pub fn remove(store: &Store, id: &str) -> Result<bool, String> {
|
||||
let mut all = load(store);
|
||||
let before = all.len();
|
||||
all.retain(|l| l.id != id);
|
||||
if all.len() == before {
|
||||
return Ok(false);
|
||||
}
|
||||
save(store, &all)?;
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
/// A reviewer's point of view: who they are, what they ask, and how they look.
|
||||
pub struct Persona {
|
||||
pub id: &'static str,
|
||||
pub role: &'static str,
|
||||
pub asks: &'static str,
|
||||
pub method: &'static str,
|
||||
}
|
||||
|
||||
/// The reviewers, the person's vision first: a critique judges the design against what it is
|
||||
/// for. Sources: Connor and Irizarry's objective/element/effect/rationale critique, Nielsen's
|
||||
/// heuristics, severity scale and cognitive walkthrough (NN/g), W3C's POUR principles.
|
||||
pub const PERSONAS: &[Persona] = &[
|
||||
Persona {
|
||||
id: "vision",
|
||||
role: "The person's vision (product owner)",
|
||||
asks: "Does each screen achieve the objective, for the audience, as the brief, references and feedback notes describe?",
|
||||
method: "For each key element: the objective it serves, the element, its effect on that objective, and why. Read the person's notes (get_feedback) and references first; flag anything the brief asked for that is missing.",
|
||||
},
|
||||
Persona {
|
||||
id: "ux",
|
||||
role: "UX designer",
|
||||
asks: "Can people understand and use it without help?",
|
||||
method: "Nielsen's 10 heuristics: system status, real-world language, control and freedom (undo, exit), consistency, error prevention, recognition over recall, efficiency, minimalism, error recovery, help. Cite the heuristic for each finding.",
|
||||
},
|
||||
Persona {
|
||||
id: "flow",
|
||||
role: "Flow reviewer",
|
||||
asks: "Do the screens make one coherent journey for each top task?",
|
||||
method: "Walk each top task across screens (the `flow` graph, else screen order and names): a clear entry, a way forward and a way back or out on every screen, no dead ends or orphan screens, the same navigation and labels in the same place throughout, what is entered on one screen shown on the next, a confirmation, success and error screen for each action, a first-run or empty entry. At each step ask: will they try this, notice the control, connect it to their goal, see progress?",
|
||||
},
|
||||
Persona {
|
||||
id: "qa",
|
||||
role: "QA tester",
|
||||
asks: "What breaks with real data and real states?",
|
||||
method: "A matrix per screen, each item pass, fail or n/a: empty, loading, error, disabled, focus, long text and names, minimum content, zero, one and many items, missing images, every variable mode, the narrow and wide screen sizes. Overflow and clipping from get_bounds or lint. A state with no design is an open question, not a pass.",
|
||||
},
|
||||
Persona {
|
||||
id: "visual",
|
||||
role: "Visual designer",
|
||||
asks: "Does the eye go to the right thing first, and does it hold together?",
|
||||
method: "Scale and hierarchy (one focal point per screen), balance, contrast between roles, gestalt proximity and alignment, a steady spacing rhythm on the 4px grid, the type scale, every colour a variable, consistent radii and components.",
|
||||
},
|
||||
Persona {
|
||||
id: "accessibility",
|
||||
role: "Accessibility reviewer",
|
||||
asks: "Can people with low vision, colour blindness, motor or cognitive limits and keyboard or screen-reader users use it?",
|
||||
method: "W3C POUR for a design. Perceivable: text contrast 4.5:1 (3:1 for large text and for control borders and icons), colour never the only signal, real text, the layout holds at 320px wide and 200% text. Operable: targets 44px (24px minimum), a focus state designed for every control, headings and a visible current location, undo or confirm on destructive actions, an alternative to every gesture. Understandable: fields keep a visible label (not only a placeholder), errors say how to fix, the same navigation and labels across screens. Robust: controls named for their role in layers and code links.",
|
||||
},
|
||||
Persona {
|
||||
id: "copy",
|
||||
role: "Content designer",
|
||||
asks: "Is every word clear, specific and kind?",
|
||||
method: "Plain language in the users' words, specific buttons (verbs, not OK), no placeholder or lorem text, errors that name the problem and the fix, empty states that say what to do next, neutral wording about people whose gender is unknown.",
|
||||
},
|
||||
Persona {
|
||||
id: "end-user",
|
||||
role: "A user from the audience",
|
||||
asks: "As this user, doing my top task, where do I get stuck or give up?",
|
||||
method: "Take the audience given (else ask): their goal, context and constraints (first visit, in a hurry, on a phone, low vision). Walk the top task screen by screen: what you see, what you think, where you hesitate, the first point you would give up.",
|
||||
},
|
||||
];
|
||||
|
||||
/// Severity of a finding (Nielsen): rate on frequency, impact and persistence.
|
||||
pub const SEVERITY: &[&str] = &[
|
||||
"0 not a problem",
|
||||
"1 cosmetic: fix if time allows",
|
||||
"2 minor: low priority",
|
||||
"3 major: fix before handing over",
|
||||
"4 catastrophe: blocks the task, fix now",
|
||||
];
|
||||
|
||||
pub const FINDING: &str = "persona, heuristic or check, element (layer name), what was seen, its effect on the objective, evidence (the tool result or screenshot region), severity 0-4, suggestion (kept apart from the observation)";
|
||||
|
||||
/// What the agent does with a critique's evidence, in order.
|
||||
pub const STEPS: &[&str] = &[
|
||||
"Objective: without `objective` (what this must achieve, for whom, the top tasks) ask the person first; a critique without one is opinion.",
|
||||
"Evidence: fix every lint error and warning above and lint again; get_screenshot each artboard at scale 1 (2 for small text) in every variable mode; read the person's notes (get_feedback) and the `flow` graph.",
|
||||
"Review each persona independently: with subagents, one per persona given only its brief and the evidence; alone, one pass per persona, writing its findings before starting the next. Never let reviewers see or debate each other's findings before rating.",
|
||||
"Each persona lists findings in the finding format and at least one thing that works. Check every lesson; a broken lesson is a finding.",
|
||||
"Merge: one list, duplicates joined, how many personas found each (two or more is confirmed), severity the mean of their independent ratings.",
|
||||
"Fix every severity 3 and 4 finding with the smallest edit and check it again; the work is ready when none is open and lint is clean.",
|
||||
"Report to the person: what works, then findings by severity (observation, evidence, suggestion, fixed or open). Phrase open ones as questions about the objective.",
|
||||
"Ask the person with ask_user which findings to remember (one question per finding: every project, this project, or not). Save only the approved ones with design_lessons {add}: a one-line rule and its reason.",
|
||||
];
|
||||
|
||||
/// The screens of a page and the arrows drawn between them: the flow a critique walks.
|
||||
pub fn flow(doc: &Document) -> Value {
|
||||
let mut screens = crate::editor::screens::declared(doc);
|
||||
if screens.is_empty() {
|
||||
screens = doc
|
||||
.root
|
||||
.iter()
|
||||
.copied()
|
||||
.filter(|id| doc.get(*id).is_some_and(|n| n.is_frame() && crate::editor::screens::device_of(n.rect.w).is_some()))
|
||||
.collect();
|
||||
}
|
||||
let screen_of = |id: NodeId| std::iter::once(id).chain(doc.ancestors(id)).find(|a| screens.contains(a));
|
||||
let mut links: Vec<(NodeId, NodeId)> = doc
|
||||
.nodes
|
||||
.values()
|
||||
.filter_map(|n| match &n.kind {
|
||||
NodeKind::Connector { start: Some(s), end: Some(e), .. } => Some((screen_of(s.node)?, screen_of(e.node)?)),
|
||||
_ => None,
|
||||
})
|
||||
.filter(|(a, b)| a != b)
|
||||
.collect();
|
||||
links.sort_by_key(|(a, b)| (a.0, b.0));
|
||||
links.dedup();
|
||||
let name = |id: &NodeId| doc.get(*id).map_or(String::new(), |n| n.name.clone());
|
||||
let mut v = json!({
|
||||
"screens": screens.iter().map(name).collect::<Vec<_>>(),
|
||||
"links": links.iter().map(|(a, b)| [name(a), name(b)]).collect::<Vec<_>>(),
|
||||
});
|
||||
if links.is_empty() {
|
||||
v["note"] = json!("no arrows between screens: walk the flow by screen order and names, and ask the person for the intended flow");
|
||||
} else {
|
||||
let from = |id: &NodeId| links.iter().any(|(a, _)| a == id);
|
||||
let to = |id: &NodeId| links.iter().any(|(_, b)| b == id);
|
||||
v["deadEnds"] = json!(screens.iter().filter(|s| to(s) && !from(s)).map(name).collect::<Vec<_>>());
|
||||
v["unreached"] = json!(screens.iter().filter(|s| !to(s) && !from(s)).map(name).collect::<Vec<_>>());
|
||||
}
|
||||
v
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn lessons_are_kept_per_project_and_removed_by_id() {
|
||||
let store = Store { root: std::env::temp_dir().join(format!("papercraft-lessons-{:016x}", fastrand::u64(..))) };
|
||||
std::fs::create_dir_all(&store.root).unwrap();
|
||||
let all = add(&store, "Screens scroll; they never grow", "", None).unwrap();
|
||||
let brand = add(&store, "Headlines use the accent colour", "brand book", Some("p1".into())).unwrap();
|
||||
assert_eq!(add(&store, "screens scroll; they never grow", "", None).unwrap().id, all.id, "a repeat is not stored twice");
|
||||
assert_eq!(applying(&store, Some("p1")).len(), 2);
|
||||
assert_eq!(applying(&store, Some("p2")).len(), 1, "a project's lesson stays in it");
|
||||
assert!(remove(&store, &brand.id).unwrap());
|
||||
assert!(!remove(&store, &brand.id).unwrap());
|
||||
assert_eq!(load(&store).len(), 1);
|
||||
assert!(add(&store, &"x".repeat(MAX_RULE + 1), "", None).is_err());
|
||||
std::fs::remove_dir_all(&store.root).unwrap();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user