oficina / docs Public

Ledger · worktree-tighten-chrome

Human

merge and deploy?

Human

<task-notification> <task-id>a3af3ffd64deaeb3f</task-id> <tool-use-id>toolu_01ExXDofJng15fX3dQWjoNEM</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-docs/7925695a-b185-431d-9d20-42b4dfbeb800/tasks/a3af3ffd64deaeb3f.output</output-file> <status>completed</status> <summary>Agent "Fable whole-branch review" finished</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>Review complete. Sanity check: `go build ./...` and `go test ./internal/docs/` both pass on the branch (122s); I re-read the ask (review only, no edits) and modified nothing. What follows is the report to relay. ## Whole-branch review: `worktree-tighten-chrome` (20515c2) vs `origin/main` ### BLOCKER **1. The repo's own keyboard-path gate is red on this branch, and was not run.** `fixtures/browser-drive.mjs:534` starts checks 7 and 8 by focusing `form[action="/signout"] button`. This branch moves that button inside the shell's closed `&lt;details rst-dropdown rst-shell-account&gt;` (`topbar.html`, `show.html:63`). Content of a closed `&lt;details&gt;` is not rendered, so `.focus()` is a no-op in Chromium and Firefox; the drive then starts from wherever focus already was — the editor, per line 503. Worse, even if the start were fixed to the account summary, the DOM path is now summary → toolbar (1 stop) → PDF → Markdown → editor = **4 presses**, and check 8 (`browser-drive.mjs:555`) asserts `reachedAfter &lt;= 3`. So the only mechanism this repo has for the `role="toolbar"` contract (AGENTS.md's own "invisible in a unit test" case) fails on this branch under every starting point. CI stays green because the drive is not in `10-gate` — precisely the "passed" vs "could not see" trap AGENTS.md records. The commit message claims Chromium measurements but never mentions the drive. Fix: retarget the start (e.g. `[rst-shell-account] &gt; summary`), raise the bound with a re-derived comment (line 512-514's "two presses suffice" is now false), run `scripts/browser-drive.sh` with the sandbox off, and say so in the discussion. ### SHOULD-FIX **2. `show.html:106-118` — `[docs-shell-title]`'s "truncation over wrapping" is not what the CSS does.** `[rst-shell-bar]` is `flex-wrap: wrap` (tokens.css:1346). Flex line-breaking uses each item's hypothetical main size — for `flex-basis: auto` on nowrap text, its full width — and `min-inline-size: 0` / shrink only act after the line is chosen. So a title that does not fit beside the brand drops to its own row, the account/menu control to the row after, and ellipsis appears only once the title alone exceeds the bar. Estimate at 320px (14px/650 brand "Documents" ≈ 68px, Menu summary ≈ 66px, three 16px gaps, 32px bar padding): any title over roughly 19 characters at 12.5px gives a two-row bar, over ~30 a three-row one. Estimated from tokens, not measured — measure it. Fix is `flex: 1 1 0` (basis 0 never forces a wrap; the item grows into free space and shrinks with ellipsis). Secondary: when it does truncate, the full title is readable nowhere on the page (not focusable, no tooltip, no other rendering) — an SC 1.4.10 loss-of-information question the old wrapping `&lt;h1&gt;` did not have. **3. `show.html:523-529, 559-565` — the auto-margin reasoning is inverted.** Flexbox hands each line's positive free space to auto margins before `justify-content` (css-flexbox-1 §9.5 step 16, per line). On its own wrapped line, `[docs-doc-actions]` with `margin-inline-start: auto` sits at the trailing end (flush *right*), not "flush left under the toolbar"; and `space-between` with a single item on a line is `flex-start` — flush left — and cannot "strand them apart", because both links are inside one flex item. The rendered result is fine; the recorded reason is backwards in a codebase that holds comments to be measured. **4. `show.html:54` — the brand link is a 21px-tall target, and the branch deleted the rule that had fixed exactly that.** The old `[docs-back]` set `min-block-size: 24px` with a measured comment on why derived sizes fail. Its replacement `[rst-shell-brand]` gets no padding (tokens.css:1347); body is 14px × 1.5 → 21px. It passes SC 2.5.8 only via the spacing exception (≥16px gap horizontally; 8px+1px+8px vertically to the toolbar), not via size, and AGENTS.md says "24×24 targets". Removed without mention. Add a min-block-size for the brand on the signed-in pages, or propose it upstream per spec §10. **5. `show.html:162-176` — arithmetic verified, character counts not.** 151/96/352 check out: 16px root, `--rst-sp-4` = 16px, 1in = 96px, and the `[rst-page]` override wins by source order at equal (0,1,0) specificity. But "352px ≈ 44 characters" uses 8px/char while "96px ≈ seven characters" uses 13.7px/char; 12pt Georgia averages 8–9px/char, so 96px is 11–12 characters. "Seven" is scaled from the old "five at 64px" (itself inconsistent with "40 at 320px" pre-branch) and the paragraph still says "measured". The 256→288 toolbar claim is correct against the old and new padding; I could not re-measure "both overflow" (no browser in the sandbox). **6. `render.go:39-43`, `signin.html:30-45` — the "must hide the account in CSS" reasoning is partly wrong.** tokens.css:1354 `[rst-shell-account]:has(&gt; [rst-dropdown-menu]:empty) { display: none }` already hides the account menu on any page that defines no account block, which signin does not. What signin actually has to hide is the default "Home" brand and, under 800px, the Menu disclosure (which would open an empty tail). The rule and its specificity note are correct; the stated reason is not. **7. Stale references the move left behind:** AGENTS.md:30 ("linked from both list.html and show.html's page headers"), render.go:174 (docs.signout.label "page headers"), render.go's `docs.export.*` comment ("show.html's page header"), handlers.go:68 ("list.html's header also carries"), handlers.go:273 ("[docs-back]"), internal/export/pdf.go:277, fixtures/toolbar-roving.mjs:15. ### NITS **8. `list_test.go:214-218` and the commit message:** "the route is exactly as easy to lose ... falls back to the layout's default" — the default *is* `&lt;a rst-shell-brand href="/"&gt;` (topbar.html), so an undefined block keeps the route and loses only the label. The test's first assertion therefore passes against the default too; only the label comparison is load-bearing. Fine as a test, wrong as a comment. **9. No `&lt;h1&gt;`:** not a WCAG 2.2 AA failure — no SC requires one, the span is not visually a heading so 1.3.1 does not bite, `&lt;title&gt;` carries it, the page is behind `RequireSession` so SEO is moot, and the old page already mixed the header h1 with the document's own h1s from `Store.HTML`. One sentence in the comment, no change. **10. `list.html:55`:** brand links to `/` from `/` — a self-link, standard pattern, and useful from `/?q=`. No action. ### Verified and fine CSS scoping "by delivery" is genuinely true (`render.go` clones base before `ParseFS` of each page; partials carry no `[rst-page]` rule). Nothing else on show depended on `[rst-page]`'s padding or max-width: `[docs-save-status]` and `[docs-jumpto]` are `position: fixed`, `[docs-bar]`'s sticky is unaffected. The roving widget (root `[docs-toolbar]`, `editor-entry.js:238`) owns exactly the 14 formatting controls; the exports are outside it. Account dropdown is a native `&lt;details&gt;` and sign-out is a form POST — scriptless. Skip link is a sibling of the hidden bar (`topbar.html`) and still targets `#main`. No literal English entered a template; the shell's own strings are rastrillo keys present in v0.22.0 `locales/en.toml`. Attribute grammar and `docs-*` prefixes respected. **VERDICT: CHANGES REQUIRED** result: Whole-branch review of tighten-chrome (20515c2) — CHANGES REQUIRED: the branch breaks fixtures/browser-drive.mjs's keyboard-path checks (sign-out start target now inside a closed details; editor is 4 tab presses from the bar against a ≤3 bound) and was not driven; plus six should-fix items (title truncation claim false under flex-wrap, inverted auto-margin comment, 21px brand target, inconsistent character counts, wrong account-hiding rationale, stale header references).</result> <usage><subagent_tokens>151641</subagent_tokens><tool_uses>33</tool_uses><duration_ms>631011</duration_ms></usage> </task-notification>

Human

<task-notification> <task-id>bjygqej0v</task-id> <tool-use-id>toolu_01DRiXtnSM8JdpPGZnC8ehVG</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-docs/7925695a-b185-431d-9d20-42b4dfbeb800/tasks/bjygqej0v.output</output-file> <status>completed</status> <summary>Background command "Poll CI until it settles" completed (exit code 0)</summary> </task-notification>

Human

go

Human

Did you file an issue on https://amadan.net/carlos/platform/discussions?