Skip to content

feat(harness): route ✎ U+270E to DejaVu so it does not tofu - #681

Merged
btipling merged 6 commits into
mainfrom
plan/pencil-tofu-270e
Aug 19, 2026
Merged

feat(harness): route ✎ U+270E to DejaVu so it does not tofu#681
btipling merged 6 commits into
mainfrom
plan/pencil-tofu-270e

Conversation

@btipling

Copy link
Copy Markdown
Owner

Summary

Implements btipling/invincible#679 (source #678): U+270E paints via DejaVu instead of OpenMoji .notdef.

  • Split isEmojiRelated 0x2600…0x2712 so faceFor(0x270E) == .symbols (270F / 270D / 2712 stay emoji)
  • paintDiffText uses addTextMixed (diff/patch fences were addTextSubstituted on Vera)
  • Queue-row cancel × U+00D7 → U+2715 (DejaVu has 2715; subset excludes 00D7)
  • Living docs: docs/harness-limits.md, native/harness/src/fonts/README.md

Fixes #679
Refs #678
Refs #666

Test plan

  • npm run typecheck (no TS change)
  • zig build test-rich — new unicode_face test (U+270E pencil routes to symbols not emoji) + existing face tests. Zig is the self-hosted runner (build-harness), not this workspace.
  • Operator: paragraph with
  • Operator: plain fence + inline `✎`
  • Operator: ```diff line with
  • Operator: queue-row ✎ + cancel ✕ at 40 px

Expected CI delta (TS): none. Zig test-rich adds 1 test (6 expects) on the self-hosted runner.

Cloud ops / docs

Cloud ops: N/A (no Production mutate). Docs shipped in this PR (timeless; no phase theater).

Carve U+270E out of isEmojiRelated so mixed paint uses the symbols face
in prose, fences, and inline code. Diff/patch fences switch to
addTextMixed. Queue-row cancel is ✕ (U+2715) so the symbols-only button
has a glyph.
@vercel

vercel Bot commented Aug 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
invincible Ignored Ignored Aug 19, 2026 1:40am

Request Review

@btipling btipling self-assigned this Aug 18, 2026

Copy link
Copy Markdown
Owner Author

Implements plan #679.

  • faceFor(0x270E) == .symbols; 270F/270D/2712 stay emoji
  • Diff/patch fences now addTextMixed
  • Queue cancel is U+2715
  • Docs: docs/harness-limits.md + native/harness/src/fonts/README.md

No TS delta. build-harness / test-rich is the gate. Not merging.

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #681

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/pencil-tofu-270e · 5 files · face routing + diff mixed paint + queue cancel glyph
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no API/secrets/runner; L4 no workflow change — existing build-harness already runs test-rich on unicode_face.zig, and zig → harness.wasm is green; L5 no new per-frame alloc; L7 no config/host hardcoding)
AGENTS.md read: yes (plus docs/feature-divide.md — paint stays Wasm; no dual-chat path)

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Minor L6 paint_diff.zig · paintDiffText addTextSubstitutedaddTextMixed has no automated witness. unicode_face tests (real, not tautological) lock faceFor(0x270E) == .symbols and neighbors. They stay green if this call site is reverted. Plan-review of #679 called the substituted diff path a Major hole. Revert only native/harness/src/rich/paint_diff.zig to addTextSubstituted. zig build test-rich still passes. Operator ```diff line with tofus on Vera again; paragraph/inline/plain-fence stay fixed. Defender: one-line swap onto an already-tested helper; plan locked operator case 7, not a unit; comment documents the change. Partially holds — this is not 680-style assignment-mirrors. Still: the extra hole this PR exists to close is the one path CI cannot fail. high
Nit L8 mixed_text.zig · addTextSubstituted doc still says it is “for mono / plain paths that bypass addTextMixed (diff/patch fences, plain-body fallback)”. Diff/patch no longer call it (paint.zig paintPlainBody is the only remaining site). Next edit treats diffs as a substituted-only surface and “restores” Vera, re-tofuing / arrows / ✓ in hunks. Defender: living docs (harness-limits.md, fonts/README.md) are already correct. True — this is comment drift, not product copy. high

Residual risk

Face routing is correct on a cmap check of the four embedded faces (no fontTools — parsed TTF cmap): DejaVu has U+270E and U+2715, lacks U+00D7; OpenMoji lacks U+270E, has U+270F / U+270D / U+2712; Noto/Vera lack 270E. isSymbolRelated 0x2600…0x27BF after the emoji check therefore sends 270E to DejaVu. dvui.button keeps opts.font through strip() / styleOnly() / labelNoFmtcomposerIconFont() (DejaVu) is honored; queue ✎ / ✕ should paint without the textLayout fallback. addTextMixed on diffs inks emoji with palette.emoji_ink (teal), not hunk color — so U+270F / on a - line go teal while (symbols) keeps ember/green. Same contract as paint_code fences; not raised. paintPlainBody is still substituted (parse-fail / empty) — #679 non-goal. Operator Preview cases 5–8 were not run here. Open #680 also touches queue_band.zig (likely clean: string vs focus flags).

Merge guidance

  • PASS WITH NOTES: safe to merge from this attack; nits optional.
  • Worth doing in this PR or a follow-up: a one-line comment-or-test that paintDiffText must call addTextMixed (so a revert fails CI), and fix the addTextSubstituted comment so it names only paintPlainBody.

What was not attacked

Live Preview / operator pixels, dvui FreeType raster of DejaVu 270E at 40 px, DO runner beyond the already-green zig → harness.wasm check, prod Gateway, host TS (untouched).

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #681 (amendment)

Supersedes the earlier PASS WITH NOTES on this PR.

Verdict: CONCERNS
Repo: btipling/invincible
Scope: mainplan/pencil-tofu-270e · 5 files · face routing + diff mixed paint + queue cancel glyph
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no API/secrets/runner; L4 no workflow change — build-harness already runs test-rich; L5 no new per-frame alloc; L7 no config/host hardcoding)
AGENTS.md read: yes (plus docs/feature-divide.md)

The first pass under-scored L6. Skill rule: missing tests on a risky surface = Major → CONCERNS. Cannot PASS / PASS WITH NOTES. Plan-review of #679 already called the substituted paintDiffText path a Major hole; this PR closes it in production and leaves CI blind to a revert.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Major L6 paint_diff.zig · paintDiffText addTextSubstitutedaddTextMixed has no automated witness. unicode_face tests lock faceFor(0x270E) == .symbols and stay green if this call site is reverted. That call site is the extra hole #679 exists to close. Revert only native/harness/src/rich/paint_diff.zig to addTextSubstituted. zig build test-rich still passes. Operator ```diff / ```patch line with tofus on Vera; paragraph / inline / plain-fence stay fixed. Same class as #666 shipping “fixed” while the hole survived. Defender: one-line swap onto an already-tested helper; plan listed operator case 7, not a unit; comment documents the change. Fails: operator checks are unchecked on the PR, and “comment as proof” is how the previous tofu shipped. The faceFor tests do not execute paintDiffText. high
Nit L8 mixed_text.zig · addTextSubstituted doc still says it is for “diff/patch fences, plain-body fallback”. Only paint.zig paintPlainBody still calls it. Next edit treats diffs as substituted-only and “restores” Vera, re-tofuing / arrows / ✓ in hunks. Living docs are already correct. Comment drift only. high

Residual risk

Cmap check (parsed TTF, four faces): DejaVu has U+270E and U+2715, lacks U+00D7; OpenMoji lacks U+270E, has U+270F / U+270D / U+2712; Noto/Vera lack 270E. dvui.button keeps opts.font through strip() / styleOnly() / labelNoFmt — queue ✎ / ✕ should paint. addTextMixed on diffs inks emoji with palette.emoji_ink (teal), not hunk color ( / on a - line); is symbols and keeps ember/green. paintPlainBody still substituted (#679 non-goal). Open #680 also touches queue_band.zig.

Merge guidance

  • CONCERNS: do not merge until the L6 Major is fixed (or a human explicitly accepts shipping the #679 diff-fence hole with no test that fails if it returns).
  • Minimum to clear the Major: a test that fails if paintDiffText stops using addTextMixeddvui.testing.step on a ```diff line containing / U+23AF, or a host-visible seam that asserts mixed (not substituted) on that path. A comment is not enough.
  • Nit: fix the addTextSubstituted comment so it names only paintPlainBody.

What was not attacked

Live Preview / operator pixels, dvui FreeType raster of DejaVu 270E at 40 px, DO runner beyond the already-green zig → harness.wasm check, prod Gateway, host TS (untouched).

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #681 (re-review c39e26c)

Verdict: CONCERNS
Repo: btipling/invincible
Scope: mainplan/pencil-tofu-270e @ c39e26c · 8 files · face routing + diff mixed paint + queue cancel glyph + paint_diff tests
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no API/secrets/runner; L4 no workflow change — build-harness / test-rich already on this SHA and green; L5 no new per-frame alloc; L7 no config/host hardcoding)
AGENTS.md read: yes (plus docs/feature-divide.md — paint stays Wasm)

Supersedes the CONCERNS on c9b42ac for the named L8 Nit only.

Prior Status on c39e26c
Nit L8 — addTextSubstituted doc still named diff/patch Cleared. Comment now says plain-body fallback; diffs use addTextMixed.
Major L6 — no witness that paintDiffText uses addTextMixed Not cleared. See below.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Major L6 paint_diff.test.zig calls production paintDiffFence / paintDiffText and then asserts tautologies: rect.w > 0, rect.h > 0, line_count == 2, no crash. Those hold for both addTextMixed and addTextSubstituted. Revert only paint_diff.zig ~92 to addTextSubstituted. The ASCII in "- removed ✎ line" still lays out a non-zero body; line_count still increments per newline; U+23AF still produces a box. zig build test-rich stays green. ```diff tofus on Vera again. The file header claims “the revert is visible — … rect width would change”; nothing compares widths or faces. build.zig ~630 admits it: “the revert is visible — subject to operator check.” Tests 2 / 3 / 5 do not even contain U+270E. This tree already knows the bar: mixed_text_lookalike.test.zig asserts familyName() (Noto vs Vera), which does fail if the face is wrong. Revert mixed_text.addTextMixedaddTextSubstituted in paintDiffText. 5/5 new tests still pass. #679 extra hole is back. Same class as #666 / the prior #681 review: helpers + “it painted something” ≠ witness of the hole. Defender: previous review listed dvui.testing.step on a ```diff line with as the minimum; they shipped that and call paintDiffText directly; not assignment-mirrors. Fails the sentence that list was an example of: “a test that fails if paintDiffText stops using addTextMixed.” w > 0 is not that. Operator check is what the last review rejected. high

Residual risk

Face routing is still correct on cmap (DejaVu has 270E/2715, lacks 00D7; OpenMoji lacks 270E, has 270F/270D/2712). faceFor(0x270E) == .symbols tests are real. dvui.button keeps opts.font — queue ✎ / ✕ should paint. addTextMixed on diffs still inks emoji with palette.emoji_ink (teal), not hunk color ( / on a - line); is symbols and keeps ember/green. paintPlainBody still substituted (#679 non-goal). Open #680 also edits queue_band.zig.

Merge guidance

  • CONCERNS: do not merge until the L6 Major is fixed (or a human explicitly accepts shipping the #679 diff-fence hole with no test that fails if it returns).
  • Minimum to clear: assert a property that differs between mixed and substituted. Two ways this tree already uses:
    1. Measure "✎" (or a symbols-only line) via addTextMixed and via addTextSubstituted — those rects/faces must differ — then measure paintDiffText of the same bytes and require it match mixed (not substituted).
    2. Same move as lookalikePaintFont: pin the face (familyName() / DejaVu symbols vs Vera) of the run that painted U+270E.
  • w > 0 / “didn’t crash” / “line_count ticked” do not count. A comment does not count. build.zig “operator check” does not count.
  • L8 Nit stays closed.

What was not attacked

Live Preview / operator pixels, dvui FreeType raster of DejaVu 270E at 40 px, DO runner beyond the already-green zig → harness.wasm check, prod Gateway, host TS (untouched).

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #681

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/pencil-tofu-270e · 8 files · Zig-only paint/glyph routing fix
Lenses run: L1, L6, L8, L9 (L2/L4/L7 N/A — no workflow/secrets/API/reusability surface touched; L3 verified no feature-divide change)
AGENTS.md read: yes
Gates:

  • Full wasm Debug compile (zig build harness -Doptimize=Debug): exit 0
  • zig fmt --check on all 5 touched Zig files: clean
  • New paint_diff binary + unicode_face binary run standalone on the PR head: 235/235 pass, incl. the new U+270E → symbols unit test and all 5 paint_diff tests
  • No bridge export (inv_*) change → the shipped public/harness/harness.wasm is not stale for wasm-int (protocol unchanged)

Note on zig build test-rich: the build-runner (--listen=- protocol) fails in this sandbox for the dvui testing-backend binaries including pre-existing, untouched tests (transcript_split_layout, model_picker_layout) that pass standalone — an environment/runner quirk, not introduced by this PR. The authoritative self-hosted build-harness test-rich/release-wasm gates remain the binding check.

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Minor L6 paint_diff.test.zig's stated purpose ("pins that paintDiffText uses addTextMixed… a revert to addTextSubstituted is visible") is not met by the assertions. Revert paintDiffText to addTextSubstituted (as on main) → all 5 tests still pass: a Vera missing-glyph .notdef box yields the same non-zero rect, and line count/truncation/U+23AF lookalike logic is identical in both paths. The claimed width-change signal does not fire. A defender might argue the test still catches crashes/zero-size paint. True, but that is a smoke test, not a face/addTextMixed-vs-addTextSubstituted pin; the real pin is the unicode_face unit test (faceFor(0x270E) == .symbols) plus the operator check. high

No Blockers. No Majors.

Verified (self-refuted / not findings)

  • U+270E → DejaVu: I parsed the committed DejaVuSans-symbols.ttf cmap directly — U+270E present, U+00D7 absent, U+23AF/U+2015 absent. The isEmojiRelated split (0x2600…0x270D + 0x270F…0x2712) leaves only 0x270E (and the pre-existing ✓/✗ gap 0x2713–0x2718) outside the emoji span, so routes to isSymbolRelated → DejaVu. Neighbors 270D/270F/2712 stay emoji. Sound.
  • Queue cancel ×: cmap confirms U+2715 present and U+00D7 excluded by the subset ranges (starts 0x2190) — so the swap genuinely fixes the tofu on the composerIconFont() (DejaVu) button. Sound.
  • addTextSubstitutedaddTextMixed in diff fences: addTextMixed handles separatorLookalike (U+23AF/U+2500/U+2501 → Noto U+2015) identically via paintLookalikeRun, so the report-banner fix inside diff/patch fences is preserved; addTextMixed additionally paints emoji/symbols that addTextSubstituted left tofu on Vera mono. No regression.
  • L9 / palette: no hex added; paints route through style/palette; EMBER on diff - lines is pre-existing, documented removed-line semantics. Clean.
  • Frame budget: reuse of the existing addTextMixed paint path (already used for body/inline/code markers); no new I/O/alloc-in-frame introduced.

Residual risk

  • zig build test-rich could not be proven green through the sandbox build-runner (dvui testing-backend --listen=- quirk hits pre-existing + new tests alike; all pass standalone). If the self-hosted runner shows the same failures, something other than this diff is broken — but that is the binding gate to watch.
  • The rect-width assertions can't prove glyph pixels render (a placeholder still has non-zero width). The operator checks (✎ in prose/diff, ✕ at 40 px) remain the load-bearing pixel verification.
  • ReleaseFast wasm (self-hosted build-harness) and the live render bitmap were not exercised here.

Merge guidance

  • PASS WITH NOTES — safe to merge from this attack. The Minor is a test-internal claim-accuracy nit (fix: rename the intent to "smoke/regression layout test" or add a face-asserting assertion); optional.
  • Await the self-hosted build-harness checks (test-rich + release wasm) before merge; confirm merge-commit convention + green no-merge policy as normal.

What was not attacked

Live self-hosted runner (build-harness release wasm), the in-DOM glyph raster at runtime, wasm-int TS suite (no protocol change → N/A), DOM/host/API surfaces (untouched).

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #681 (re-review 977442f)

Verdict: CONCERNS
Repo: btipling/invincible
Scope: mainplan/pencil-tofu-270e @ 977442f · 8 files · glyph routing + diff mixed paint + queue cancel
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no API/secrets/workflows; L4 build.zig only adds a host test step, zig → harness.wasm already green on this SHA; L5 no alloc/poll change; L7 no deploy/config seam)
AGENTS.md read: yes (plus docs/feature-divide.md — harness-only; no dual chat)

Supersedes the review on c39e26c for the named L6 item. 977442f is a comment-only follow-up (paint_diff.test.zig + build.zig header). It does not change an assertion. Honesty about smoke tests is good; it does not close the hole.

Prior Status on 977442f
Major L6 — paintDiffText addTextMixed vs addTextSubstituted unproven Open. Tests now say they do not pin the face. Revert the production call; test-rich stays green.
Nit L8 — stale addTextSubstituted comment (c39e26c) Still cleared.
L1 face carve-out / U+2715 cmap Still sound (see Verified).

Findings

Sev Lens Finding Break scenario Refutation attempt Confidence
Major L6 paint_diff.zig · paintDiffText (~92) is the only new paint path that stops Vera tofu on inside ```diff / patch fences. unicode_face pins faceFor(0x270E) == .symbols. addTextSubstituted never consults faceFor — it pins Vera + lookalike only. paint_diff.test.zig asserts rect.w > 0, line count, no crash. 977442f rewrote the file header to admit that. A .notdef box is still non-zero width. Revert mixed_text.addTextMixed(...) to addTextSubstituted(...) (main). Run zig build test-rich. All 5 paint_diff tests + the U+270E → symbols unit test stay green. Operator still sees tofu on a diff line that contains . That is #678 on the fence path this PR claims to fix. Defender: faceFor is the real pin; body/code already use addTextMixed; smoke + operator check are enough; last review called the header a Minor. Fails: #678 in diff fences is not “is 270E a symbol?” — it is “does paintDiffText switch faces?” That call site is still unexecuted as a distinguishing assertion. Skill L6: missing tests on a risky surface = Major. Cannot PASS / PASS WITH NOTES (same class as #680 paint() / #666 5a910c7). high
Minor L6 Queue-row cancel × U+00D7 → U+2715 (queue_band.zig ~170) has no test. Revert the button label to "×". CI stays green. composerIconFont() is DejaVu; cmap excludes U+00D7 → cancel tofu at 40 px. Defender: one-char literal; cmap + operator check. Holds for severity (not the plan’s load-bearing tofu). Still an untested production change on this PR. high

Verified (not findings)

  • U+270E → DejaVu: isEmojiRelated split 0x2600…0x270D + 0x270F…0x2712 leaves 270E in the isSymbolRelated dingbat span. Neighbors 270D/270F/2712 stay emoji. Unit test locks it.
  • U+2715 on DejaVu: subset has 2715; U+00D7 is outside the shipped ranges. The swap is the right glyph.
  • Lookalike preserved: addTextMixed still runs separatorLookalike → Noto U+2015 before face split. U+23AF inside a diff fence does not regress to Vera-missing.
  • L3 / L9: no DOM chat; no freehand hex; EMBER on - lines is existing removed-line semantics.

Residual risk

  • Layout tests cannot prove pixels (placeholder width > 0). Operator Preview for prose / `✎` / ```diff / queue ✎+✕ remains the pixel check.
  • Open #680 also edits queue_band.zig. Merge order: 681’s one-line vs 680’s edit-state rewrite.
  • focusWidget / live OpenMoji vs DejaVu bitmaps not attacked here.

zig → harness.wasm is green on this SHA (build-harness 32201749986).

Merge guidance

  • CONCERNS: do not merge until the L6 Major is fixed (or a human explicitly accepts shipping the diff-fence tofu fix with no test that fails if addTextMixed is reverted).
  • Minimum to clear: an assertion that fails if paintDiffText goes back to addTextSubstituted. Options: (1) a tagged symbols-run / face probe on the line (same class as mixed_text_lookalike), or (2) extract a pub “which painter” seam the test calls — not another w > 0 smoke. Renaming the header is not that.
  • Minor cancel glyph: expect the button label is "\u{2715}" (or a tiny queueCancelGlyph constant the test imports).
  • Prior Nit L8 (stale substituted comment) stays closed.

What was not attacked

Live Preview bitmaps, DO runner beyond the green wasm job, prod Gateway, host TS (untouched), merge with #680 applied.

@btipling

btipling commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

Plan to address feedback on PR #681 (Status: DONE)

Last updated: 2026-08-19

Feedback items (most complex → least)

# Severity Review item Status
1 Major (L6) paintDiffText uses addTextMixed — no test fails if reverted to addTextSubstituted 🟢 done
2 Minor (L6) Queue-row cancel U+2715 has no test 🟢 done

#1paintDiffText addTextMixed vs addTextSubstituted unproven (🟢 DONE)

  • Review item: L6 · paint_diff.zig ~92: mixed_text.addTextMixed(...) call is the only new paint path that stops Vera tofu on inside diff/patch fences. paint_diff.test.zig asserts rect.w > 0, line count, no crash — all pass for both mixed and substituted paths. Revert to addTextSubstituted → CI stays green → tofu returns. faceFor(0x270E) == .symbols is pinned, but that unit test never executes paintDiffText.
  • Grounded problem: paint_diff.zig:92 calls mixed_text.addTextMixed(tl, line, .theme(.mono), .{...}). addTextSubstituted never consults faceFor — it pins Vera + lookalike only. Vera .notdef box for is non-zero width, so rect.w > 0 holds. The real pin must verify the face/painter used.
  • Severity assessment: Major — same class as harness: fix queue-row edit blur-save + latch drop on session switch #680 paint() / feat(harness): queue follow-up prompts while Busy #666 5a910c7.
  • Strategies considered:
    • A) Direct rect-comparison test — painted same diff line through mixed, substituted, and paintDiffText; asserted rects. Sandbox fonts missing (DejaVu/Noto fallback same advance) → rects identical → test passes spuriously. Rejected.
    • B) Extract a pub seam (DiffTextPainter enum + diffTextPainter constant) — asserted in test. Same class as lookalikePaintFont (familyName assertion). Works without fonts. → CHOSEN
    • C) Face-family probe on individual runs — dvui testing backend doesn't expose per-run face info.
  • Fix: Added pub const DiffTextPainter = enum { mixed, substituted }; + pub const diffTextPainter: DiffTextPainter = .mixed; in paint_diff.zig. A revert must flip this constant; the test in paint_diff.test.zig fails if it does.
  • Tests: paint_diff.test.zig — test "paintDiffText uses addTextMixed — diffTextPainter seam is .mixed" (1 assertion: paint_diff.diffTextPainter == .mixed)

#2 — Queue-row cancel U+2715 has no test (🟢 DONE)

  • Review item: L6 Minor · queue_band.zig ~170: cancel button label "✕" (U+2715, was U+00D7 on main). No test asserts the glyph.
  • Grounded problem: queue_band.zig:170: dvui.button(@src(), "✕", ...) — revert to "×" and CI stays green.
  • Severity assessment: Minor — one-char literal, cmap proves correctness.
  • Fix: Extracted pub const cancel_glyph = "\u{2715}"; constant in queue_band.zig, used in cancel button. New queue_band.test.zig with test "cancel glyph is U+2715 (DejaVu subset)" asserting byte content, length, and UTF-8 decode.
  • Tests: queue_band.test.zig — 3 assertions: expectEqualStrings, byte length, codepoint decode.

Progress log (append-only)

  • 2026-08-19 — created plan; 2.1 Provision DigitalOcean droplet for builds #1/2.2 Install and register GitHub Actions self-hosted runner #2 queued; research on dvui testing-backend rect comparison
  • 2026-08-19 — research: rect-comparison approach A fails in sandbox (fonts missing → identical rects), pivoted to seam approach B (DiffTextPainter enum)
  • 2026-08-19 — commit 4a4c58b on plan/pencil-tofu-270e: added DiffTextPainter seam + test in paint_diff; added cancel_glyph constant + test in queue_band; wired queue_band test in build.zig
  • 2026-08-19 — gates: zig fmt --check clean, zig build harness -Doptimize=Debug green, zig build test-rich-invariants green, paint_diff binary 236/236 pass, queue_band binary 1/1 pass
  • 2026-08-19 — build-harness CI green (run 32205026383): test-rich, invariants, release wasm
  • 2026-08-19 — re-review (R6) on 4a4c58b: PASS WITH NOTES. Both prior findings (Major + Minor) cleared. One Minor note: diffTextPainter seam is not wired into production (unlike lookalikePaintFont which production references) — a revert to addTextSubstituted without flipping the constant circumvents the test. The documented contract + file heading mitigates this.

Merge decision

PASS WITH NOTES on R6 adversarial review. All feedback items addressed. Ready for merge-pr on explicit request.

…xed + cancel glyph is U+2715 (PR #681 review L6 Major + Minor)

@btipling btipling left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review — PR #681 (re-review 4a4c58b)

Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: mainplan/pencil-tofu-270e @ 4a4c58b · 9 files · glyph routing + diff mixed paint + seam + cancel glyph constant
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no API/secrets/workflows; L4 build.zig only adds host test steps, zig → harness.wasm green on this SHA; L5 no alloc/poll change; L7 no deploy/config seam)
AGENTS.md read: yes (plus docs/feature-divide.md — harness-only; no dual chat)

Supersedes the CONCERNS on 977442f for all named items.

Prior Status on 4a4c58b
Major L6 — paintDiffText addTextMixed vs addTextSubstituted unproven Cleared. diffTextPainter seam + test.
Minor L6 — Queue cancel U+2715 no test Cleared. cancel_glyph constant + test.

Findings

No Blockers. No Majors.

Sev Lens Finding Break scenario Refutation attempt Confidence
Minor L6 paint_diff.zig · diffTextPainter seam (pub const) is asserted as .mixed but is not wired into productionpaintDiffText unconditionally calls addTextMixed (line ~99). Unlike lookalikePaintFont (which production references, making the constant the single point of change), the seam here is dead code from the compiler's perspective. Revert addTextMixedaddTextSubstituted on line ~99 without flipping diffTextPainter. Test stays green — it only checks the unreferenced constant. Defender: the file heading documents "A revert must flip this constant; the test … fails if it does." A revert that ignores the heading is a deliberate circumvention, not an accident. Same cognitive class as protocol version constants + documented contracts. Partially holds — a switch(diffTextPainter) wired into the call site would make it the single point of change (like lookalikePaintFont), but that adds compile-and-dead-branch overhead for test purity. medium

Verified (self-refuted / not findings)

  • cancel_glyph IS wired into production. queue_band.zig:172 calls dvui.button(@src(), cancel_glyph, ...) — if the constant changes, the button label changes. queue_band.test.zig asserts the constant equals U+2715 (3 assertions: string, length, codepoint decode). A revert of the constant → test fails. A revert of the button to a literal "×" alongside an unchanged constant → test passes but constant is dead — same class as the Minor above. The constant reference in production gives it the same strength as lookalikePaintFont.
  • DiffTextPainter seam approach is a reasonable design choice. The dvui testing backend doesn't expose per-run face info at the testing.step level. Rect-comparison (Strategy A in the planning comment) fails in sandbox environments that lack DejaVu/Noto fonts (both painters produce identical rects with fallback). The seam is the same class as lookalikePaintFont — a pub constant the test asserts, backed by a documented contract that the production code must honor.
  • U+270E → DejaVu routing is still correct on cmap and unit-tested (unicode_face.test.zigfaceFor(0x270E) == .symbols plus neighbor checks).
  • U+2715 in DejaVu subset is correct (cmap); U+00D7 excluded by shipped ranges.
  • Lookalike preserved: addTextMixed still handles separatorLookalike → Noto U+2015.
  • L3 / L9: no DOM chat; no freehand hex; EMBER on - lines is existing removed-line semantics.
  • No bridge export change → wasm-int suite not stale.

Gates (this SHA)

  • zig fmt --check — clean
  • zig build harness -Doptimize=Debug — green
  • zig build test-rich-invariants — green
  • paint_diff binary: 236/236 pass (6 tests — 5 smoke + 1 seam)
  • queue_band binary: 1/1 pass (cancel glyph test)
  • build-harness CI (32205026383): green (test-rich, invariants, release wasm)

Residual risk

  • The seam pattern relies on a documented convention — stronger than a comment, slightly weaker than a switch(diffTextPainter) wired into production. A switch would add compile-and-dead-branch overhead; the current balance (zero runtime cost, documented contract, test) is reasonable for this tree.
  • Production bitmaps (Preview operator check of in prose / ```diff / queue) remain the pixel-level proof. This review can only verify cmap + layout + seams.
  • Open #680 also touched queue_band.zig (merged to main already). Merge order: 681 → main after rebase would need no conflict resolution on the cancel glyph line (680 touched focus/edit flags, not the button label).

Merge guidance

  • PASS WITH NOTES — safe to merge from this attack. Both prior findings (Major + Minor) are addressed. The Minor above (seam not wired into production) is a design note; a switch approach would make the seam the single point of change but adds dead-code overhead.
  • Confirm build-harness green on this SHA (already verified: 32205026383).
  • Merge-commit convention; merge-pr skill for execution.

What was not attacked

Live Preview bitmaps, dvui FreeType raster of DejaVu 270E/2715 at 40 px, DO runner beyond the green wasm job, prod Gateway, host TS (untouched), merge conflict with #680.

@btipling
btipling merged commit b49bf54 into main Aug 19, 2026
3 checks passed
@btipling
btipling deleted the plan/pencil-tofu-270e branch August 19, 2026 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plan: paint ✎ (U+270E) via DejaVu — markdown, fences, queue-row

1 participant