oficina / docs Public

<task-notification> <task-id>af85bd42b61858e34</task-id> <tool-use-id>toolu_014t6bCpLXTPVNDnn7sLTvJc</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-docs/4a30c557-129a-4e57-8aa3-0c7518c36cf1/tasks/af85bd42b61858e34.output</output-file> <status>completed</status> <summary>Agent "Branch 3: sans-serif default" 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>Status: complete. Gate exit 0, tree clean, report at `.superpowers/sans-report.md`. Commits (branch `worktree-sans`, on f4bc155): - `13c8189` Serif becomes a mark of its own — `font_serif` in MARKS, editor schema, `.docs-font-serif` CSS, new `fixtures/fonts.mjs` vocabulary + Node test; new fixture `fonts.bin` (styled.bin left byte-identical, per your direction). - `cb1a73e` Sans is what a document is set in — `fonts.Default`, exporter reads it plus a `font_serif` case, stylesheet default, `DEFAULT_FONT`, option order, browser drive. Tests: 7 watched failing then passing (Rust HTML golden, class-styled, schema-mark, geometry test's two flip-stage failures, fonts.test.mjs 4 red, new toolbar-default test both halves, new PDF `/BaseFont` test both halves). Full failure output is in the report. Pagination: measured before and after with a throwaway probe — 4 pages and 2 pages either way, thresholds `&gt;=3`/`&gt;=2` untouched, nothing loosened. Concerns: Markdown still cannot express font at all (pre-existing, unchanged); `show.html`'s reflow character counts were measured in Georgia and I annotated rather than re-measured; the two font option orderings (Go and JS) are pinned only at the default's position; `json_names_every_font_mark` passes for a weak reason (unknown marks pass through `mark_names`) and its comment says so.</result> <usage><subagent_tokens>224269</subagent_tokens><tool_uses>122</tool_uses><duration_ms>1229869</duration_ms></usage> </task-notification>

<task-notification> <task-id>af5cbead2b2f4f3ff</task-id> <tool-use-id>toolu_013DGYHQAT757Dz1w1TMC6Rg</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-docs/4a30c557-129a-4e57-8aa3-0c7518c36cf1/tasks/af5cbead2b2f4f3ff.output</output-file> <status>completed</status> <summary>Agent "Fable review of sans-serif branch" 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>All verification is done. The worktree is clean (mutation reverted). Compiling the review. ## Verdict: approve with changes One small genuine defect worth fixing before merge; everything else is sound. The branch does what it says, the five-place vocabulary agrees, backward compatibility is pinned by tests rather than asserted, and the mid-task direction about the fixture was honoured. I ran the suites myself rather than trusting the report: `go test ./internal/export ./internal/docs ./internal/fonts` all pass (docs takes ~2 min), `node --test fonts.test.mjs` 8/8, `cargo test` in core/ all green. ### Findings, most severe first **1. Genuine defect (minor): the toolbar's multi-mark priority is the reverse of what screen and PDF actually do.** `fixtures/fonts.mjs` line ~57: `FONT_MARKS = ['font_serif', 'font_sans', 'font_mono']`, and `fontForMarks` returns the first hit — so serif outranks sans outranks mono. But the other two surfaces resolve the opposite way. `core/src/fragment.rs`'s `text_html` pushes opens in MARKS order (verified in source: opens in table order, closes reversed), so MARKS order is outer-to-inner and the *innermost* span's CSS rule wins — mono beats sans beats serif. `internal/export/pdf.go`'s `styleFor` switch tests `font_mono` first, then `font_sans`, then `font_serif` — the same mono &gt; sans &gt; serif. So for a run carrying two font marks — impossible through the editor (`excludes`), possible through `POST /d/{sqid}/updates`, which is exactly the caveat the code itself keeps citing — screen and PDF agree with each other and the toolbar disagrees with both. Concrete failure: a hand-crafted update marks a run `font_mono` + `font_sans`. Screen and PDF draw it in mono; the font control reads "Sans serif". That is precisely the silent mislabel this branch's own comments call "the failure worth naming". Worse, `fixtures/fonts.test.mjs`'s last test *pins the wrong answer*: `fontForMarks(['font_mono','font_sans'])` is asserted to be `'sans'`, so the disagreement is now doctrine. And the comment above `FONT_MARKS`'s iteration claims it is "the same choice internal/export/pdf.go's sizeOrder makes" — it is the opposite choice. Smallest fix: reorder `FONT_MARKS` to `['font_mono', 'font_sans', 'font_serif']` (or iterate it reversed in `fontForMarks`), flip the pinned literal in the dual-mark test to `'mono'`, and correct the comment. Nothing else uses the order — `setExclusive` clears the whole group regardless. Mitigation: it self-heals (any font selection clears the group), and only wire-crafted documents can reach it. That is why this is "approve with changes", not rework. **2. The eighth can't-fail assertion.** `fixtures/fonts.mjs`, `fontsAreCovered`, final guard: `if (fontMark(DEFAULT_FONT) !== null)`. `fontMark` returns `null` exactly when its argument equals `DEFAULT_FONT` and is in `FONTS`, and the preceding guard already threw if `DEFAULT_FONT` is not in `FONTS`. So this line compares a value to its own derivation and can never throw, for any value of `DEFAULT_FONT` — the exact shape the repo has found seven times. It guards nothing; delete it or accept it as documentation. (The dual-mark test's first assertion, `fontForMarks(both) === fontForMarks(reversed)`, is also self-referential, but its second assertion pins a literal, so the test as a whole can fail.) **3. A comment justifies the right decision with a backwards fact.** `fixtures/editor-entry.js` (~line 58) and `fixtures/fonts.mjs` (`FONT_MARKS` comment) both say the default's mark must exist because "a document written before the default moved carries the old default's mark". For this move that is false: the old default was serif, which by definition carried *no* mark — no pre-move document contains `font_serif`. What pre-move documents carry is `font_sans` — the *new* default's mark — which is actually the stronger argument for the same conclusion. Wording fix only; the code is right. ### Judged and found sound - **Screen/PDF default agreement (your point 1).** The two statements are genuinely independent: a CSS literal at `internal/docs/views/show.html:239` versus `fonts.Default` reached through `styleFor` via `export.FontCSSName()`. I mutation-tested it — flipped the stylesheet's `[docs-text]` family back to serif, and `TestPageGeometryMatchesTheStylesheet` failed with "default body face: the stylesheet names \"Docs Serif\", internal/export lays it out in \"Docs Sans\"", then restored the file (worktree clean). Line 237 is the only bare `[docs-text] {` rule, so the regex reads the right one. `TestPDFDrawsTheDefaultFaceAndTheSerifMark` closes the last gap by reading `/BaseFont` off actual output, and `CSSName()` derives from the same `Family` that selects the embedded font program, so the name checked is the metrics used. - **Five places agree**: MARKS (`core/src/fragment.rs`), schema (`fixtures/editor-entry.js`), stylesheet (three `.docs-font-*` rules plus the default), `pdf.go`, toolbar (`fonts.mjs` + `export.go`'s option list). The catalog key `docs.editor.toolbar.font.serif` pre-exists at `internal/docs/render.go:206`. Each pair is held by a test that reads real output, not a shared constant. - **Backward compatibility is pinned, not asserted.** `git diff f4bc155..worktree-sans -- fixtures/vectors/fragment/styled.bin` is empty — byte-identical, direction honoured; `fonts.bin` is the new vector. The `font_sans` MARKS row, CSS rule, and pdf case are untouched in the diff, and the untouched styled.bin goldens pass. Unmarked-is-sans is pinned on all three surfaces: the fixture's unmarked run must render with no span (Rust test), the geometry default row (CSS), `/BaseFont` LiberationSans (PDF), and the toolbar default test (control). The editor inherits `[docs-text]`'s family — `[docs-editor]` declares no font-family of its own (verified). - **Pagination (your point 4).** Credible. No threshold in the diff moved; the only pagination-adjacent edits are the sample-text mark swap and a new test. Sans being wider can only hold or *raise* page counts, so 4≥3 and 2≥2 are no closer to their thresholds than before — the 2-page test was already at exactly 2 under serif. Both pass now under my own run. - **`json_names_every_font_mark` (the flagged weak test).** Keep it. It could not have failed for the missing-MARKS-row defect (unknown marks pass through `mark_names`), but it does pin the JSON wire shape against literals and would fail on serialization drift; its own comment states the weakness and points at the HTML sibling as the real guard. That is an honest test, not a fake one. - **Reflow character counts (your point 6a)**: comment-only arithmetic; the annotation states the error direction (sans wider, counts generous) and the breakpoint conclusion survives. Fine as is. - **Orderings pinned only at position 0 (your point 6b)**: correct amount. The rendered order comes solely from `export.go`; JS `FONTS` order affects nothing functional except the default-first load guard, and values rather than indices cross the `&lt;select&gt;` boundary. A serif/mono swap in one list would be invisible to users and harmless. (They happen to match element-wise anyway.) - The `TestEveryClassTheRendererEmitsIsStyled` pass also proves the committed wasm contains the new mark — the class scrape renders `fonts.bin` through it. Key files: `/home/paulca/amadan.net/oficina/docs/.claude/worktrees/live-sync/fixtures/fonts.mjs` (findings 1, 2), `/home/paulca/amadan.net/oficina/docs/.claude/worktrees/live-sync/fixtures/fonts.test.mjs` (pinned wrong literal), `/home/paulca/amadan.net/oficina/docs/.claude/worktrees/live-sync/fixtures/editor-entry.js` (finding 3), `/home/paulca/amadan.net/oficina/docs/.claude/worktrees/live-sync/internal/export/pdf.go` and `/home/paulca/amadan.net/oficina/docs/.claude/worktrees/live-sync/internal/docs/export_test.go` (verified sound). The branch is otherwise sound, and the test discipline — literals not derivations, watched failures in both directions, the new vector instead of a regenerated one — is exactly what this repo's history calls for. Fix finding 1 (three-line change plus one test literal), optionally 2 and 3, and merge.</result> <usage><subagent_tokens>80082</subagent_tokens><tool_uses>18</tool_uses><duration_ms>415847</duration_ms></usage> </task-notification>