amadan

oficina / docs Public

← Discussions

Triage: the 320px failure on main, two silent-corruption bugs, and the palette's checks folded in open

On branch worktree-discussions-triage

claude operated by paul@keymail.dev session 4831640d · claude-code/2.1.258 · DOCS-T1

Paul asked overnight for a triage-and-fix pass on everything open in Discussions. Seven were open (#7, #8, #9, #11, #14, #16, #20). This branch carries the four things that turned out to be fixable code; the rest is triage, below, and most of what remains is yours rather than anybody's to implement.

Branched off origin/main at 7507a33. Full gate green locally, browser drive 44/44, CI running.

The one live bug on main

The document list scrolled sideways at 320px — WCAG 2.2 SC 1.4.10, which AGENTS.md takes with no exemptions. Measured, not inferred: scrollWidth 356 against a 320 viewport, innermost offenders button at x=356, button at x=350, form[docs-search-form] at x=356.

Discussion #16 reported two such failures and shipped reflow-320 (f05c041), which never landed. Nine commits later that branch is stale, so I re-measured against today's main rather than merging it — and the re-measurement is the whole point:

  • The document page is now clean. scrollWidth 320, zero offenders. Something in tighten-chrome fixed the toolbar half independently. Merging reflow-320 would have re-landed a fix for a bug that no longer exists.
  • The list page was still broken, and that half of the branch still applied.

[docs-search-form] held its field at a fixed 11rem beside a button whose label does not wrap, under a comment asserting the group wrapping kept 320px safe. It never had — max-inline-size: 100% resolves against a form sized by its own content, so the form set a min-content floor of ~334px and simply stood wider than the 288px column. A desktop layout nicety traded for an accessibility failure by a comment that sounded like it had been checked.

The second offender was not a second bug. [docs-create-form] is written correctly and shrinks as it should; it overflowed because a wrapping flex container is at least as wide as its widest item's min-content, so the search form's floor widened the line the create form had to itself. Both went green on the one fix, which is the evidence that they were one bug rather than a guess that they were. It is in the comment so the next reader does not "fix" the create form too.

The check that was not looking

This failure reached main past a Fable whole-branch review and a 22-check browser drive, because nothing in the repo looked at the one width AGENTS.md commits to without exemption. fixtures/browser-drive.mjs now sets a real 320px viewport, walks every element on both pages, and names the innermost one whose box crosses the edge — "form[docs-search-form] ends at 356" rather than "something overflows". Confirmed red against the unfixed CSS before being kept.

Two more defects, same shape as the ones these threads keep finding

Search could not find a title typed in the other Unicode form (branch task 24 on #14). "café" has two encodings — U+00E9, and "e" plus combining acute — and normaliseForSearch compared them as bytes. A title typed on a machine that produces decomposed text (a macOS filesystem, several IMEs) was unfindable from one that produces precomposed text: an empty result for a title the person is looking straight at, with nothing on screen to say why. NFC is applied inside that one function so the title and the query go through the same code — normalising one side fixes one direction and looks fixed from whichever side you tested.

This is not the accent-folding decision. That one — whether cafe should match café — is yours, task 21, and is untouched; the new test asserts the non-folding directly so nobody later mistakes this for having settled it.

A literal newline in a text run could inject Markdown into an export (branch task 23 on #11). escapeMarkdown decided "the start of a line" by asking whether the index was 0, which is the start of the run. A run can contain a newline — POST /d/{sqid}/updates takes Yjs bytes that never met ProseMirror's schema, and a paste can carry one — so "intro\n# not a heading" exported as a real heading, "- " as a real bullet, "1." as a real ordered list. An exported document says something its author did not write, which is the one failure this module is not allowed to have.

The other half of task 23 — hard_break vanishing in all three renderers — already landed with the typography branch. Verified against origin/main rather than assumed.

The palette's keyboard contract is a gate now, not a memory

Branch task 22 on #14. Those seventeen checks existed and passed, in a scratch driver deliberately not committed because the harness was on another branch. Both have landed, so this is the fold — twenty checks, and the interesting part is that two of the seventeen could not fail:

  • "focus is no longer held in the field" passed with the focus-return call deleted. Closing the palette hides the input, so focus leaves it either way. It now focuses a named element, opens from there, and asserts Escape brings focus back to that exact element; broken, it reports "focus is on BODY".
  • "Up moves back to the first" passed with the arrow handler gutted, because the first option is selected both when Up walked back to it and when nothing moved. It is guarded on Down having actually moved, and says so when it did not.

Three checks are new, one of them a regression check for the stale-empty-message bug Fable's review of #14 found and fixed without a test.

Every query is derived from the titles actually on the list rather than typed in as "s" and "waterworks" — those worked against the six seeded documents and would have gone red the day somebody edited seeddata, reporting "the palette is broken" when it was not.

Five deliberate breaks, each confirmed to turn the intended checks red and no others:

| broken thing | checks that caught it | |---|---| | the Control-K binding | 12, cascading | | aria-activedescendant never set | 1, exactly | | arrow keys stop moving | 2 (was 1) | | the status not cleared on reopen | 1, quoting the stale text | | focus never returned on Escape | 1 (was 0) |

The first break exposed a fault in the harness rather than in the palette. The throw ended the run at 31 checks instead of 44, with the 320px checks never reached and simply absent from the output rather than reported as anything — a failing check that deletes other checks, which is this repo's own "cannot tell no from I can't see it" shape. Groups now run through group(), an aborted group is its own named failure, and the probes read through null-safe selectors.

One more worth recording, because it nearly cost the evidence: the first break run came back green at 44/44. The bundle build had failed silently — fixtures/node_modules was missing — so the drive ran the old bundle. Every break run since asks whether the bundle actually rebuilt before it believes a result.

Verification

  • .amadan/ci.d/10-gate green locally: Rust, wasm reproducibility, Yjs wire compatibility both directions, Go vet/test/gofmt/build.
  • scripts/browser-drive.sh 44/44, run against the real app in headless Chrome.
  • Each fix confirmed red first, and each new check confirmed red against deliberately broken code before being kept.

Chromium cannot start inside the agent sandbox, as browser-drive.sh's own header says, so every drive run here was outside it. That does not change task 13's status: whether the gate may run a step with the sandbox off is still a decision for whoever owns the runner.

Not done, and why

Two items on #14's task 24 need copy that has not been reviewed, so I have not written it. Both are real and both are one small change plus a catalog key:

  1. The 50-result cap truncates with no signal. A person with sixty matching documents sees fifty and is told nothing.
  2. An expired session makes the palette say "unavailable" rather than "sign in again" — an outage message for a routine event.

Every user-visible string here is a catalog key across twelve locales, and AGENTS.md is explicit that a hardcoded English string is a bug. Writing copy you have not seen and then having twelve catalogs translate it is worse than leaving the two items for a five-minute copy pass. They join the existing unreviewed copy — docs.editor.toolbar.*, docs.export.*, docs.list.search.*, docs.jumpto.* — which is one review, not five.

#20 (import) is untouched. It is blocked on two rulings that are yours, and its session has torn down. Relayed in my triage reply rather than decided here.

Triage of the other six threads

  • #7, #8, #9 — merged, done, no open tasks, nothing outstanding. Closing them.
  • #11, #14 — merged. What is left on them is your calls (copy, the exported title, accent folding, whether the drive joins CI) plus the two follow-up tasks fixed on this branch.
  • #16 — superseded by this branch, and its open question ("amadan branch merge or squash-and-fast-forward?") is still unanswered and still yours.
  • #20 — blocked on you; left alone.

For you

  1. The copy review, which now unblocks four threads at once and the two items above.
  2. #16's merge-method question — it stopped that branch landing, and a WCAG failure sat on main for nine commits as a result. Worth answering even though this branch routes around it.
  3. Accent folding (task 21) and the exported title (task 12), both still open, both reversible.
  4. Whether scripts/browser-drive.sh joins CI (task 13) — now guarding 44 checks including the only 320px coverage in the repo, which makes it worth more than when the question was asked.

Not merging until this has had its highest-capability-model review per AGENTS.md.

— claude~paul@keymail.dev, session "discussions triage overnight"

claude driven by paul@keymail.dev session 4831640d · claude-code/2.1.258 · DOCS-T1

Whole-branch review by Fable, and what it changed — 3363262

Done per AGENTS.md's highest-capability rule, read as one diff. It found one blocking defect and two instances of this repo's own recurring shape. All fixed; gate green, drive 44/44, CI running.

BLOCKING — my Markdown fix was incomplete, in the way its own commit message warned about

74320cb's title says "a newline in a run can no longer inject Markdown into an export". It could. I taught escapeMarkdown where a line begins and then asked i == lineStart — the first character on a line, not the first non-space. CommonMark allows a block marker up to three spaces of indentation and all of them interrupt a paragraph:

"intro\n # one space then hash"      -> a real heading
"intro\n   # three spaces then hash" -> a real heading
"intro\n - one space then bullet"    -> a real bullet
"intro\n 1. one space then list"     -> a real ordered list

And two characters were never escaped in any position:

"intro\n==="            -> a setext H1, which promotes the paragraph ABOVE it
"intro\n~~~\nswallowed" -> a code fence that interrupts the paragraph and
                           then eats every following line of the document

The fence is the worst of them. It does not restyle a line; it swallows the rest of the file.

The thing worth recording is not the bug, it is that I described it and then shipped it. That commit message names this repo's recurring failure — a check that reports success while measuring something adjacent to the question — and the fix underneath it was true of the cases I tested and false of the sentence I wrote. Five test cases all shaped "newline then marker at column zero" agreed with each other and none of them asked the question the commit claimed to have answered.

Fixed by tracking "anything but spaces seen on this line yet". Any run of spaces counts as still-at-line-start rather than counting to three — over-escaping renders correctly, which is the trade the function's own header already made. = and ~ are escaped at the start of a line only, so a = b is untouched.

One detail I got wrong in the other direction and Fable's report let me correct: I first expected \=\=\=. Only the first character needs escaping, because an escaped = means the line is no longer a setext underline. \=== is the whole fix, and three backslashes would have bought what one bought.

Recorded rather than fixed: four spaces is an indented code block

No escaping prevents it — the block is made by the whitespace, not by a character a backslash can go in front of. The text survives verbatim, which is the module's actual promise, but renders as code. Fixing it means rewriting the author's own leading whitespace, which is a different decision from escaping and not one to take in passing. TestMarkdownIndentedRunLinesBecomeCodeBlocks records it so it stays a decision rather than becoming somebody's discovery.

Two more checks that could not see their own failure

That makes four on this branch, which is worth stating plainly: this pass set out to fold in checks and ended up finding that a third of the interesting ones could not fail.

  • "a query matching nothing says so" tested that some text appeared in the status element. The palette writes two different strings there — data-empty when nothing matched, data-failed when the fetch threw — so the check passed just as well when the search was broken and the person was being told so. It compares against the element's own data-empty now. Confirmed red by making the empty case report the failure copy: it says "Search is unavailable right now."; the empty-result copy is "No documents match".
  • The 320px checks never asserted the viewport was 320. They measured against documentElement.clientWidth, which reads 320 only because rastrillo's vendored topbar emits a viewport meta — nothing in this app does. Lose it on a module upgrade and Chrome lays out at 980px, clientWidth reads 980, and both checks pass while measuring a desktop-width page. That is the check I added tonight to catch exactly this class, carrying the flaw it was written to prevent. It asserts the viewport now, and a mismatch says "this check measured nothing" rather than reporting a comfortable pass.

Also from the review

  • list.html's locale caveat had the threshold and the symptom wrong. Because the field gives way first and the button never shrinks, a wide label eats the field long before anything overflows: ~200px leaves ~80px of field, ~270px a sliver, and only past ~280px does anything cross the edge. So the first failure a wide catalog produces is a search box too narrow to read what you typed — a usability problem, not a 1.4.10 breach. Stated as the sequence it actually is. ([docs-create-form]'s existing comment states the same caveat as a bare "about 270px", which is the second half of that sequence.)
  • caretChecks is wrapped in group() too. The toolbar section is still inline and still aborts the run when it cannot select a word — named as a follow-up in the comment rather than moved, because relocating a hundred lines of working checks at the same time as fixing them is how a harness stops being trustworthy.
  • A correction to 55a4ac0's own message: it says "twenty checks". The palette group is 19. 19 + 2 reflow + 23 existing = 44, which is what the drive prints.

What the review checked and did not find

Recording this because "what a review covered" is worth as much as what it objected to:

  • The CSS fix is real, not moved — verified against the pinned rastrillo@v0.22.0 ui/tokens.css: [rst-input]'s width: 100% loses to flex-basis on the main axis, our two-attribute selector outranks its one, no !important in the file, and [rst-page]'s padding makes the 288px column arithmetic right. The reasoning that the create form was a casualty of the search form's min-content floor is correct.
  • NFC is genuinely orthogonal to accent folding. NFC never drops a combining mark, so cafe cannot match café. Its singleton decompositions never interact with the punctuation-stripping loop except on spaces, which are spaces on both sides. NFC is not NFKC, so ligatures and compatibility forms are untouched.
  • The multi-byte reasoning in the first fix did hold: range always yields rune starts, \n is one byte, so s[lineStart:i] never starts mid-rune. Checked with "é\n# after multibyte" and "日本\n1) after cjk".
  • "the exact title ranks first", the derived-query logic, the no-JS search checks, and the stale-message checks are all genuine — each asserts the thing its name claims.

Still yours, unchanged

Nothing in the review touches the four decisions waiting on you: the copy review, #16's merge-method question, accent folding, and whether the drive joins CI. The last one is worth a little more after tonight than it was: the drive now carries the only 320px coverage in the repo, and this round proved it can be wrong in a way only a deliberate break reveals.

— claude~paul@keymail.dev, session "discussions triage overnight"

claude driven by paul@keymail.dev session 4831640d · claude-code/2.1.259 · DOCS-T1

Merged to main and deployed. Paul approved with "merge and deploy".

  • worktree-discussions-triage → 1b6d392 on main
  • Live at https://docs.bds.oncarlos.com, confirmed running 1b6d392
  • The branch's own CI passed at 3363262; the squash passed at 1b6d392 before main moved; main's own run is in the queue behind other branches.

How it landed

Squash-and-fast-forward, which is the method #16 asked about and never got an answer to. Squashed onto origin/main on a temp branch (land-triage), pushed that, waited for its own CI to pass, then fast-forwarded main onto it. So main never carried an unverified commit, and the squash was verified as the thing that would actually land rather than as the branch that fed it.

Two checks before the push, both of which have been wrong here before:

  • origin/main was re-fetched immediately beforehand and was still 7507a33, so the fast-forward was real rather than assumed. Several sessions were merging tonight.
  • The squashed tree was diffed against the reviewed branch tip and is byte-identical. What Fable reviewed is what landed.

Deployed, and the platform lied twice on the way

Shipped 1b6d392 for linux-arm64, promoted to edge, confirmed the channel, restarted, verified. The runbook in my notes earned itself again:

The build. file on the binary — ARM aarch64, checked rather than declared, because --target labels an architecture rather than verifying one and an amd64 ELF once shipped happily as linux-arm64. And the BuildVersion stamp confirmed present in the binary, because an unstamped build makes /api/version answer dev and that probe then distinguishes nothing.

The first restart did nothing, and said nothing. The instance was asleep; the restart returned its usual success message; a curl then woke it and it served 7507a33. A second restart, after carlos channels already showed edge at the new version, is what actually cycled it — exactly the silent no-op already recorded from 2026-09-01.

carlos instances list still reports the old version right now. It says VERSION 7507a33, Asleep, while the running process answers 1b6d392. That column is the platform's bookkeeping about an adopted release, not a statement about the binary serving traffic. Anyone reading it as "the deploy did not land" would be wrong; anyone reading it as "the deploy did land" would also be wrong. It simply is not that claim.

What actually answered, which is the app and the platform agreeing in carlos logs:

23:02:12 [app]      rastrillo: serving ... version=7507a33   <- first restart, still old
23:02:12 [platform] instance woke (version 7507a33)
23:02:37 [app]      rastrillo: serving ... version=1b6d392   <- second restart
23:02:37 [platform] instance woke (version 1b6d392)

Plus GET /api/version → 1b6d392 on two separate requests, GET / → 303 to /signin, and the sign-in page rendering.

Also worth knowing

There is a stale .git/config.lock in the shared checkout, zero bytes, read-only, dating from 19:28. Any git operation that writes config fails against it — it broke a push -u and, worse, made git checkout -b fail after it had already rewritten the index and working tree, so a branch briefly looked reverted. Nothing was lost because everything was committed and pushed. Worked around with --no-track rather than deleting another session's lock, but the next session to hit it will not know why. Someone should clear it.

One of my own verification steps lied, in this branch's own genre. The stamp check was strings "$binary" | grep -qx "$SHA" under set -o pipefail — grep -q exits on first match, SIGPIPEs strings, and pipefail reports the pipeline as failed. So a stamp that was present read as absent. Counted into a variable instead. Related, and recorded by another session tonight: reading the gate through | tail gives you tail's exit code, not the gate's, which is why the authoritative signal here is CI's own run and not my local one.

What is still open, unchanged by the merge

The four decisions are still yours and none of them blocked shipping:

  1. The copy review — now the only thing standing between the repo and four closed threads, plus the two remaining halves of #14's task 24 (the 50-result cap needs a "more matched" signal, an expired session needs "sign in again").
  2. Accent folding (#14 task 21) — cafe still does not match café; one line, with a test asserting the current behaviour either way.
  3. The exported title (#11 task 12).
  4. Whether scripts/browser-drive.sh joins CI (#11 task 13) — worth more than when it was asked. It is at 44 checks and carries the only 320px coverage in the repo, and tonight it caught a live WCAG failure that a Fable review and a 22-check drive had both walked past.

And #20 (import) is still blocked on its own two rulings, untouched.

— claude~paul@keymail.dev, session "discussions triage overnight"