Skip to content

plan: harness tofu fix — str_replace L2 sides + thinking preview (#718) #732

Description

@btipling

Plan: harness tofu fix — str_replace L2 sides + thinking preview

Source: #718 — harness: tofu (.notdef) in str_replace L2 sides and composer textEntry
Refs: #358 (tool-run status marks prior tofu work), #594 (fence/inline mixed prior tofu work)
Status: IMPLEMENTED

Review notes (2025 — plan-review of the DRAFT)

Reviewed by plan-review on the DRAFT; verdict HANDOFF-READY with one Major + Minors now fixed in-body:

  1. Major (Living docs): the plan missed updating docs/harness-limits.md:171 — the Tool-run Painter row currently states "side bodies are mono addText (not mixed-face)". That exact sentence is the behavior Phase 1 changes, so it must be edited alongside lines 225/235 or the doc self-contradicts the Missing-glyphs row after this PR lands. Added as Living docs item.
  2. Minor (Living docs, line 225 rewrite): the proposed replacement for the Missing glyphs row dropped the existing "That is not mojibake; Copy still yields UTF-8 source" sentence — an existing user-facing promise. Preserved it and merged the "now covered" note instead of replacing the row wholesale.
  3. Minor (Tests): test 1.1 Create GitHub repo invincible #3 (thinking preview uses addTextMixed) is asserted via "same seam pattern", but the seam section only declared StrReplaceTextPainter for toolrun.zig. Locked a matching ThinkingPreviewTextPainter constant in thinking.zig so the assertion actually has something to pin.
  4. Minor (Tests): toolrun.test.zig is a new host test file (does not exist on main; verified native/harness/src). Noted it must be wired into build.zig test-rich like paint_diff.test.zig, not just created.
  5. Nit (toolrun.zig:321): const mono = palette.fontMono(); is declared once (line 321) inside the if (sides) |s| block and is in scope for both the old and new bands — there is no per-block re-assignment. The plan's "verify precise local var name per block" caveat is unnecessary; confirmed single mono binding.

Baseline line numbers, color passthrough, and seam pattern all verified against live code on main (diff-hunks below, unchanged by review).


Summary

Two tofu surfaces fixed (addTextaddTextMixed); the textEntry gap is documented honestly (requires dvui enhancement, not in this plan).


Current baseline (verified on main)

Surface What it does File:Line
str_replace L2 old band otl.addText(s.old, .{}) on palette.fontMono() (Vera Sans Mono) toolrun.zig:344
str_replace L2 new band ntl.addText(s.new, .{}) on palette.fontMono() (Vera Sans Mono) toolrun.zig:368
str_replace L2 detail mixed_text.addTextMixed(tl, it.detail, detail_font, ...) — already mixed ✅ toolrun.zig:385
Thinking collapsed preview tl.addText(preview, .{}) on .font = .theme(.body) thinking.zig:133
Composer textEntry dvui.textEntry(...) — no font override, dvui uses theme body (Noto) internally ui.zig:681
Queue-row textEntry dvui.textEntry(...) — same single-face limitation queue_band.zig:179
Empty-state placeholder Three tl.addText(...) static English strings ui.zig:275-288
Expander titles dvui.expander(src, label, ...) — tool names like "str_replace" toolrun.zig:138,267
paintDiffText mixed_text.addTextMixed(tl, line, .theme(.mono), ...) — already mixed ✅ paint_diff.zig:100
diffTextPainter constant pub const diffTextPainter: DiffTextPainter = .mixed — test seam ✅ paint_diff.zig:9

Root cause: dvui has no automatic per-glyph fallback. addText paints every codepoint on a single font face. addTextMixed (from mixed_text.zig) splits text runs at codepoint boundaries by face (Noto → body, DejaVu → symbols, OpenMoji → emoji). str_replace L2 sides and thinking preview use bare addText instead of addTextMixed.

TextEntry limitation: dvui's TextEntry widget uses a single Font for its internal TextLayout. There is no per-glyph face-switching hook in the dvui TextEntry API. Fixing composer/queue-row tofu requires a dvui enhancement — not in scope for this plan.

Color passthrough (verified): dvui TextLayoutWidget.addTextEx merges per-run options over the widget's own options (self.data().options.override(opts)). addTextMixed's paintSlice only sets .font when PaintOpts.color_text is null, so calling it with {} opts preserves the textLayout's existing .color_text (ember_text / teal_text / teal_muted) — no color override is needed, matching the Skill row / detail paths. Emoji still ink via palette.emoji_ink (teal) regardless, consistent with existing diff/transcript paint.


Design

Phase 1: str_replace L2 old/new bands

File: native/harness/src/ui/toolrun.zig

The mixed_text import is already present (used for it.detail at line 385). The fix is a drop-in call swap:

// Before (line 344):
otl.addText(s.old, .{});

// After:
mixed_text.addTextMixed(otl, s.old, mono, .{});
// Before (line 368):
ntl.addText(s.new, .{});

// After:
mixed_text.addTextMixed(ntl, s.new, mono, .{});

mono (const mono = palette.fontMono();) is declared once at toolrun.zig:321 inside the if (sides) |s| block and is in scope for both bands (used by mono.lineHeight() and both textLayouts) — no per-block re-assignment. The PaintOpts default {} passes through the textLayout's .color_text (already palette.ember_text / palette.teal_text) per the verified color passthrough above.

Phase 2: Thinking collapsed preview

File: native/harness/src/ui/thinking.zig

// Before (line 133):
tl.addText(preview, .{});

// After:
mixed_text.addTextMixed(tl, preview, .theme(.body), .{});

The mixed_text import needs to be added to thinking.zig (currently not imported; confirmed its import list at thinking.zig:1-10).

Phase 3: Composer + queue-row textEntry — document only

No code change. dvui's TextEntry uses a single font for all glyphs. The gap is already partially documented in docs/harness-limits.md:235:

Composer | Canvas textEntry uses theme body (Noto Sans); emoji/symbol while typing follows the same face rules when painted in the transcript after send

This plan updates the row to be explicit about tofu while typing:

Composer + queue-row textEntry use a single font face (Noto Sans) because dvui's TextEntry widget has no per-glyph face-switching hook. Symbols and emoji that Noto Sans does not cover will show as missing-glyph placeholders while typing. After send, user bubbles and transcript text use addTextMixed and paint correctly. Fixing textEntry requires a dvui enhancement.

Audit results (no changes needed)

Surface File:Line Verdict
Empty-state placeholder text ui.zig:275-288 Static English strings — no emoji/symbols
Expander titles toolrun.zig:138,267 Tool names like "str_replace" — never contain emoji/symbols
Status marks (✓/✗/…) toolrun.zig:252 Already use pinned symbol/heading faces (documented in harness-limits.md:168)
paintDiffText paint_diff.zig:100 Already uses addTextMixed
str_replace detail toolrun.zig:385 Already uses addTextMixed

Tests

Zig test matrix — new file native/harness/src/toolrun.test.zig

This is a new host test file (no toolrun.test.zig exists on main). It must be wired into build.zig test-rich as its own addTest step (mirroring the paint_diff.test.zig registration around build.zig:690-700, including the dvui_testing + zmd imports) — not merely created on disk.

# Test What it verifies
1 str_replace old band uses addTextMixed Same seam pattern as diffTextPainter constant in paint_diff.zig — a revert must flip a test constant
2 str_replace new band uses addTextMixed Same
3 thinking preview uses addTextMixed Same seam pattern, pinned via ThinkingPreviewTextPainter below

Test seams (matches paint_diff.zig:8-9 pattern)

Following the existing pattern in paint_diff.zig:

// In toolrun.zig, export a test-only constant:
pub const StrReplaceTextPainter = enum { mixed, plain };
pub const strReplaceTextPainter: StrReplaceTextPainter = .mixed;
// In thinking.zig, export a matching test-only constant so test #3 has a pin:
pub const ThinkingPreviewTextPainter = enum { mixed, plain };
pub const thinkingPreviewTextPainter: ThinkingPreviewTextPainter = .mixed;

Each test asserts .mixed; a revert to addText must flip the corresponding constant and the test fails.


Living docs

docs/harness-limits.md

  1. Line 171 (Tool-run Painter row): this row currently reads "…side bodies are mono addText (not mixed-face)" — the exact behavior Phase 1 changes. Update it to reflect the switch:

    str_replace sides are split at decode (slot-cached, O(dirty)); committed side textLayouts use cache_layout; side bodies are mono addTextMixed (face-aware — DejaVu symbols + OpenMoji emoji, body face otherwise). L1 expander is extra +2; no-detail static label is +9 (never the same persist slot) |

    (Keep the rest of the Painter row intact.)

  2. Line 225 (Missing glyphs row): extend the row rather than replace it — preserve the existing "That is not mojibake; Copy still yields UTF-8 source when the browser allows clipboard write" promise:

    Scripts outside these faces (notably full CJK) may still show a missing-glyph placeholder. That is not mojibake; Copy still yields UTF-8 source when the browser allows clipboard write. Note: str_replace L2 old/new bands and thinking collapsed previews do use addTextMixed, so symbols/emoji there paint from DejaVu + OpenMoji (not tofu); composer + queue-row textEntry tofu is a known dvui limitation (see Composer row).

  3. Line 235 (Composer row): update to be explicit about the gap:

    Composer + queue-row textEntry use Noto Sans only — dvui's TextEntry widget has no per-glyph face-switching hook. Symbols/emoji that Noto does not cover tofu while typing/editing. After send, transcript text uses addTextMixed and paints correctly. Fixing this requires a dvui enhancement; no DOM input workaround is planned.


Caps

N/A — no new caps, no existing caps changed. This is a paint-face change only; no schema, no protocol, no config caps.


Layer checklist

Layer Change Rationale
Wasm (dvui) 2 call swaps in toolrun.zig, 1 in thinking.zig, test constants (both files) The only paint surfaces that need fixing
Vercel backend None No agent-tool, API, or DB change
Sandbox daemon None No daemon change
Client/Wasm bridge None No protocol change
DOM host None No DOM involvement
Docs docs/harness-limits.md Three row updates (171, 225, 235)

Cloud ops: N/A — Wasm-only, no Production mutate.
Build: build-harness required (Zig changes).
Phase issue rule: Single PR — all changes are same-layer (Wasm-only), no blocking dependency, ships together.


Non-goals

  • Full CJK face pack (out of scope)
  • Color emoji (monochrome OpenMoji teal is intentional)
  • Dual DOM composer / dual chat
  • Changing Copy / wire UTF-8 (source is already intact)
  • dvui TextEntry per-glyph enhancement (separate dvui issue, tracked in docs)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions