amadan

oficina / docs Public

← Discussions

Impeccable pass: PRODUCT.md, and five design defects on the three real screens closed

On branch worktree-impeccable-init

claude operated by paul@keymail.dev session 48ea084f · claude-code/2.1.252 · DOCS-T1

Opened by the Claude Code session Paul asked to "initialise, run and respond to any issues raised by /impeccable:impeccable", run overnight and unattended.

Two commits, pushed:

  • 3d25741 — PRODUCT.md. init normally interviews first; Paul's "do this overnight" is what the skill's own autonomy rule treats as permission to infer from the repo instead. The file says so at the top and marks anything that is a reading rather than a citation as \[inferred — confirm]. Two such marks. It records no visual world: init does not write DESIGN.md.
  • ffebf47 — five defects, all found by running the app and measuring rendered pages at 1440px and at the 320px reflow width, not by reading templates.

The five

  1. A document had no way back to the list. GET /d/{sqid} rendered sign-out and nothing else, so a bookmark or a mailed link stranded a person there. Adds \[docs-back], reusing docs.list.title rather than inventing a key that could drift from the screen it points at. No new user-visible string.
  2. A list row was 7% clickable — 990x61 row, 268x16 target, inert thumbnail. Also cleared 24x24 only via SC 2.5.8's Spacing exception, thin under "no exemptions". Stretched-link overlay plus a pointer-only hover tint. Costs the timestamp's text selection.
  3. Sign-in was an unlaid-out form — a 950px email field with its button flush against it, zero gap. Now a 32rem measure shared with its callouts.
  4. The mobile header spent 175px of a 720px viewport before the first document. Now 133px.
  5. No favicon; /favicon.ico 404'd. Hand-drawn mark in the shell's documented "head" block.

Two mistakes worth reading

Both were caught by mechanisms, and both now have one.

  • The icon's first version put -- inside an XML comment, which XML forbids. It served a flawless 200 with the right Content-Type and byte count and no browser could parse it. Exactly the shape AGENTS.md warns about: a check that cannot tell "no" from "I can't see it". TestIconIsWellFormedXML now decodes every token.
  • It was first written to internal/docs/static/, which is the editor build's output. The gate rebuilds into that directory, fails on any dirt, then git-cleans it — so the file reddened CI and CI then deleted it. Assets now live in brand/, and TestExactlyOneEditorBundleIsEmbedded asserts static/ holds nothing else, so it fails in go test rather than in CI.

Verification

  • .amadan/ci.d/10-gate exits 0 locally.
  • No horizontal overflow, no console errors, on five surfaces at both widths.
  • Design detector: one finding, verified false positive — a blockquote's 3px border-inline-start inside \[docs-text], which is prose typography in document content, not a card accent.

Not done, and why

  • Highest-capability-model review is outstanding. AGENTS.md requires a whole-branch read by Fable before this squash-merges. It has not had one; this session is Opus. Please do not merge on the strength of the green gate alone.
  • The canvas tray does not bleed. \[docs-canvas] is inset to the shell's 992px column, so the grey field has hard edges inside a white page rather than surrounding the paper. Every real editor bleeds it. Left alone deliberately: breaking out of rastrillo's column shell is a structural change to a settled choice, and it is Paul's call, not a defect to fix in passing.
  • \[docs-thumb] is still an empty bordered box on every row. Reserved space per its own comment, awaiting the layout engine — but six identical empty boxes currently read as failed images. Worth a decision when the thumbnails land.
  • The detector's blockquote false positive will fire on every future edit of show.html. /impeccable hooks ignore-value would silence it; not run, because configuring the project's hooks was not asked for.

claude operated by paul@keymail.dev session 48ea084f · claude-code/2.1.252 · DOCS-T1

Remote CI is green: amadan ci status oficina/docs -branch worktree-impeccable-init reports ffebf47 ... CI passed, exit 0.

That is two of the three merge gates from AGENTS.md. The third is outstanding and this session cannot supply it: the branch still needs its whole-branch read by the highest available capability model (Fable), and Paul's approval on the two judgement calls left open above — the canvas tray bleed, and what \[docs-thumb] should look like until the layout engine fills it.

Read it as one diff rather than commit by commit. The parts most worth an adversarial eye:

  • the stretched-link overlay in list.html, specifically whether losing selection on the timestamp is an acceptable price, and whether \[docs-row] belongs on rastrillo's \[rst-row] element at all rather than on a wrapper of ours;
  • reusing docs.list.title as the back link's label — it is the choice that avoids new copy, but it does couple two screens through one key;
  • brand/ as the home for hand-authored assets, and whether that name is the one this repo wants long-term.

claude driven by paul@keymail.dev session 48ea084f · claude-code/2.1.252 · DOCS-T1

Fable review returned DO NOT MERGE. Two blockers, both correct, both now fixed in c3b40ed.

Reviewed by claude-fable-5 per AGENTS.md's highest-available-capability rule, read as one diff. I verified both blockers independently before acting rather than taking the review at face value; both held up.

B1 — the whole-row-click fix was a no-op, and its measurement was a false record.

rastrillo v0.22.0's own tokens.css, which this app serves live via ui.TokensCSS(), already ships all of it: [rst-row] has position: relative (:500), [rst-row]:hover has the --rst-surface-2 tint (:515), and [rst-row-main] > a::after has content:"" ; inset: 0 (:558), under rastrillo's own comment — "The name link's overlay makes the whole row clickable without nesting one anchor inside another." Our markup already matched that selector. The row was already fully clickable, already hover-tinted, already past 24x24 without leaning on SC 2.5.8's Spacing exception.

The "7% clickable" figure came from getBoundingClientRect on the anchor, which reports an element's own box and cannot see an absolutely positioned pseudo-element. Worse, the hit test that later "confirmed" the fix was hitting rastrillo's overlay — it would have passed identically before the change. The check that was never run is the one against the pre-change state. That is precisely the failure shape AGENTS.md names, and it had produced a fix for a defect that did not exist and very nearly written it into the permanent record.

Removed the rules, the attributes, the @media (hover: hover) gate (which could not have worked anyway — rastrillo's :hover is un-gated and still applies on touch), and the test. Left a comment in list.html recording why, so the next reader measuring 268x16 does not reach for the same non-fix. Re-verified after removal: all six probe points across the 990x61 row open the document; hover still resolves to --rst-surface-2.

B2 — PRODUCT.md asserted a security property this app does not have. It said, present tense, that content is encrypted in SQLite under a DEK and that a plaintext byte-grep test enforces it. Store.Text's own doc comment says sealing is out of scope for the plan that built it, and no such test exists in the repo. Rewritten as the spec commitment it is, with an explicit instruction not to describe an instance as encrypting content at rest.

Also taken: S2 (the icon test checked template bytes, so the head-block wiring could vanish while it stayed green — now asserts the rendered page), S3 ("passkeys are ruled out" narrowed to instance-local; §8.1.1's suite path keeps them), and three factual nits.

So one of the five "defects" was never a defect. The remaining four — the back link, the sign-in measure, the mobile header, the favicon — Fable read as sound, and they are unchanged and re-verified after the removal.

Gate exits 0. A confirmation pass is with Fable now; I will not merge until it returns MERGE.

claude driven by paul@keymail.dev session 48ea084f · claude-code/2.1.252 · DOCS-T1

Merged and deployed.

Fable returned MERGE on the re-review, confirming both blockers were fixed at the root rather than reworded, that nothing was left orphaned by the removal, and that the four remaining fixes are intact. It re-ran build, vet, gofmt and the full suite itself.

Merged: squashed to main as cafb696. Recorded with amadan ledger landed cafb696 -target main -via squash. Main's CI passed.

One thing worth recording about the merge: main moved 8 commits while this branch was open — the live-updates/SSE work landed, and it touches show.html and render.go, the two files this branch also changed. The squash auto-merged with no conflicts, but a clean textual merge is not evidence of a correct one, so it was re-verified rather than assumed: the gate exits 0 on the merged tree (and now runs a new live-update step), and in a browser the back link, the favicon on all three pages, the 133px mobile header, the still-clickable row and typing into the ProseMirror editor — whose mount the live-sync work rewrote — all behave, with no console errors or horizontal overflow at either width.

Deployed to https://docs.bds.oncarlos.com — shipped cafb696, promoted to edge, instance cycled.

Verified by asking the process itself, not the platform's bookkeeping:

  • /api/version returns cafb696
  • /static/icon.svg returns 200 image/svg+xml — that path 404s on every previous build, so it is independent proof the new binary is the one serving
  • the live sign-in page renders the fix: 512px measure, field and button on one line with a real gap, no overflow or console errors at 1440px or 320px

Two deploy notes for the next person.

First, the restart timing. carlos restart issued immediately after promote did nothing — the instance stayed pinned to the previous release through two checks. A second restart, after the channel had actually moved to cafb696, is what cycled it. So the rule is not just "restart after deploy": it is confirm the channel shows your version, then restart, and confirm again from the app's own logs (carlos logs), which name the version the process reports at boot.

Second, and worth fixing separately: a6fa843 is running as version=dev. The live-sync release was built without -X …rastrillo.BuildVersion=$(git rev-parse --short HEAD), so while it was serving, /api/version — the only probe that asks the process rather than the router — answered dev and distinguished nothing. It is superseded now, but the ship line that produced it will do the same again.

Closing this thread; the branch is landed.