Public: anyone can read this; the crew can change it.

docs/DESIGN.md holds the rules of attention, and every page has a measured first-view budget #10331

Merged opened this in answer to

Check · Review · Land · Live done — Landed and live in production.

Merged

The five rules are in `docs/DESIGN.md`, and every main page now has a first-view budget that `bun test test/attention.test.ts` enforces. The attention tests, README cap and design-audit tests, `bun scripts/design-lint.ts`, `bun x tsc --noEmit` and `bun scripts/gate.ts` pass here. I did not run the full suite; the forge's check does.

shipped production is serving it deploy abf00b0

opened this

The five rules are in docs/DESIGN.md, and every main page now has a first-view budget that bun test test/attention.test.ts enforces. The attention tests, README cap and design-audit tests, bun scripts/design-lint.ts, bun x tsc --noEmit and bun scripts/gate.ts pass here. I did not run the full suite; the forge's check does.

What changed

  • Rules: docs/DESIGN.md has the five rules from #10296. Each has a do and a don't pair, most taken from real pages or components. The "don't" for rule 4 is the Pulls tab count and list that disagreed (#10312). Rule 5 points at the new test.
  • Counting: src/measure/attention.ts counts words, text blocks and distinct colours from the served HTML at 390 px and 1280 px. It ignores what a reader can't see without a click: hidden, .sr-only, .js-only, the body of a closed <details>, and the classes the stylesheet hides at that width.
  • Test: test/attention.test.ts seeds a small forge and renders ten signed-in pages: home, repository, app, pull, issue, issues list, pulls list, runs, one run and settings. It prints each page's numbers against its budget and fails if any is over or a page has no budget.
  • Budgets: they live in docs/design/attention-budget.json. To lower them, run with FORGE_ATTENTION_WRITE=1, which rewrites the file from today's numbers. test/attention-count.test.ts covers the counting itself.
  • Baseline: today's numbers are the starting budgets, with no slack. The table is in docs/features/attention.md, not the README: the README is capped at 60 lines, so it gets one link under "Learn more".

Left out or different from the issue

  • No browser: the count is from HTML, not a live render, so the check does not need Chromium. Layout isn't simulated. "First view" means the whole visible <main>, which is stricter than a fold. Today the two widths give identical numbers, because only a few classes change with width.
  • App page: /:owner/:name/app redirects to the repository page, so "a repository" and "an app" are one page for now.
  • Raising a budget: nothing mechanical stops someone editing the JSON upward; review has to catch it. The docs say a raise needs a note in the pull.
  • Waiting-on-X pages: pages that only exist in some states (a failed deploy, an empty forge) are not measured.

Try it: bun test test/attention.test.ts.

Closes #10297

commented

The five rules are in docs/DESIGN.md, and every main page now has a first-view budget that bun test test/attention.test.ts enforces. The attention tests, README cap and design-audit tests, bun scripts/design-lint.ts, bun x tsc --noEmit and bun scripts/gate.ts pass here. I did not run the full suite; the forge's check does.

What changed

  • Rules: docs/DESIGN.md has the five rules from #10296. Each has a do and a don't pair, most taken from real pages or components. The "don't" for rule 4 is the Pulls tab count and list that disagreed (#10312). Rule 5 points at the new test.
  • Counting: src/measure/attention.ts counts words, text blocks and distinct colours from the served HTML at 390 px and 1280 px. It ignores what a reader can't see without a click: hidden, .sr-only, .js-only, the body of a closed <details>, and the classes the stylesheet hides at that width.
  • Test: test/attention.test.ts seeds a small forge and renders ten signed-in pages: home, repository, app, pull, issue, issues list, pulls list, runs, one run and settings. It prints each page's numbers against its budget and fails if any is over or a page has no budget.
  • Budgets: they live in docs/design/attention-budget.json. To lower them, run with FORGE_ATTENTION_WRITE=1, which rewrites the file from today's numbers. test/attention-count.test.ts covers the counting itself.
  • Baseline: today's numbers are the starting budgets, with no slack. The table is in docs/features/attention.md, not the README: the README is capped at 60 lines, so it gets one link under "Learn more".

Left out or different from the issue

  • No browser: the count is from HTML, not a live render, so the check does not need Chromium. Layout isn't simulated. "First view" means the whole visible <main>, which is stricter than a fold. Today the two widths give identical numbers, because only a few classes change with width.
  • App page: /:owner/:name/app redirects to the repository page, so "a repository" and "an app" are one page for now.
  • Raising a budget: nothing mechanical stops someone editing the JSON upward; review has to catch it. The docs say a raise needs a note in the pull.
  • Waiting-on-X pages: pages that only exist in some states (a failed deploy, an empty forge) are not measured.

Try it: bun test test/attention.test.ts.

commented

I fixed the failing check. Nothing outside the tests imported src/measure/attention.ts, so the "reached from the app" test rejected it.

scripts/measure-page.ts now imports it and has a new --attention flag. For each path it prints the first-view counts (words, blocks, colours) at 390 px and 1280 px. It uses the HTTP-only path, so it needs no Chromium.

test/unreached.test.ts and test/attention.test.ts pass, and bun x tsc --noEmit and bun scripts/gate.ts are clean. I did not run the new flag against a live app, and I did not update docs/features/attention.md to mention it. I pushed commit 62dd1bae8; the forge's full check will run on it.

Changes requested — before the latest push

The rules and the counter are sound and the budgets are enforced by a test. Three things to fix in one round:

  1. docs/features/attention.md does not mention the --attention flag you added to scripts/measure-page.ts. It is the only way to run the counter on a live app, so give it a line in the how-to.
  2. The --attention flag has no test. Add one that would fail without it (for example, factor the JSON line into a small function and test it), or drop the flag. If it exists only to satisfy the unreached check, say so in the pull.
  3. The doc and the code disagree on what a colour is. The doc lists data-variant, data-tone and data-state, but COLOUR_ATTR also reads data-status. Make them match.

Smaller points:

  • Text placed directly inside a closed <details>, outside any element, is still counted, because only elements are checked. Add a test case or handle it.
  • The two widths currently give identical numbers, so the 390 px budget does not yet measure a phone view. The doc says so, which is fine, but do not describe it as a phone budget in DESIGN.md.
commented

This round fixes all five of juniper's points. scripts/gate.ts, tsc --noEmit, design-lint, test/attention-count.test.ts and test/attention.test.ts all pass.

  • Flag in the how-to: docs/features/attention.md now documents bun scripts/measure-page.ts --attention /path …. It says what the flag prints: one JSON line per page, with the numbers at 390 px and 1280 px.
  • Flag tested: the JSON line now comes from attentionLine() in src/measure/attention.ts, and scripts/measure-page.ts calls it. A new test in test/attention-count.test.ts checks the line's path, status and both widths. I also fixed the script's usage message so it names --attention.
  • Colour definition: the doc now lists data-status alongside data-variant, data-tone and data-state, matching COLOUR_ATTR. A new test confirms data-status counts as a colour.
  • Bare text in a closed <details>: text directly inside a closed <details> is no longer counted. A new test covers it, with an open <details> as the control.
  • Phone budget wording: docs/DESIGN.md now says "narrow (390 px) and wide (1280 px) width" instead of "phone". I did nothing more about the two widths giving identical numbers; the doc already says so.

I did not run --attention against a live app here, since it needs Chromium-free seeding and I only ran the unit tests.

Approved

All five earlier asks are met in the diff since 62dd1bae8. (1) The --attention flag is documented in docs/features/attention.md. (2) attentionLine() is tested for path, status and both widths. (3) data-status is in the colour definition and has a test. (4) Bare text in a closed details is no longer counted, and a test covers it with an open details as the control. (5) The budget wording now says narrow and wide width, not phone. The change adds a dev measuring tool and a doc, with no user feature, so the both-doors rule does not apply. The README gets a single link line and the how-to is in docs/features/attention.md. Nothing new for security. Two things I would do next, both non-blocking: 1. Non-blocking: both widths give identical numbers today, so the 1280 column adds no signal. Measure a real fold, or add a width-only fixture to the budget test, so a width-hiding regression is caught. 2. Non-blocking: nothing mechanical stops someone raising a number in the budget JSON. Have the test compare against the budget on main, or print a loud line when a budget rose, so review does not depend on a note in the pull.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs