Skip to content

harness: str_replace L2 bands invisible, default-closed, and stall the frame when opened #714

Description

@btipling

Summary

#692 (plan #691, source #687) did not deliver the requested str_replace L2 UX.

Operator-visible failures on current main:

  1. Backgrounds are not EMBER / TEAL. Old/new sides do not read as colored bands. The only obvious delta is ink: old uses ember_text, new stays teal_text. That is not what was asked.
  2. The tool call is closed. Successful str_replace L2 starts collapsed (▸ str_replace · ok · path · 1 replacement · …). The operator cannot see the diff unless they expand it.
  3. Expanding it stalls the harness. Opening L2 does expensive per-frame work (shape + addTextMixed on both sides, no cache_layout). The canvas becomes unusable while the expander is open.

Close the str_replace L2 expander when the turn ends. While the turn is in flight, it should be open.

Observed (live main, after #692)

Screenshot / live paint: L0 ▼ 1 tool called can be open; the item expander is still (collapsed). The L1 line is the brief (str_replace · ok · docs/harness-limits.md · 1 replacement · 5226…). No bands.

What #687 / #691 asked What shipped
Old band on a visible EMBER background dvui.box .color_fill = palette.ember_surface (#1a100c) on teal_bg (#050a0c) — near-black on near-black. Not recognizably EMBER.
New band on a visible TEAL background .color_fill = palette.teal_surface (#0a1215) — same problem vs teal_bg.
Side ink as a secondary channel Old ember_text / new teal_text — this is the only thing the operator can actually see.
Expandable L2 audit Default collapsed. l2_expanded = state.toolrun_open_l2.contains(l2_key) starts false (native/harness/src/ui/toolrun.zig ~183, 227–232).
Frame budget (docs/harness-limits.md Frame budget): no GPA / heavy CPU on dvui_update Open L2: every frame splitStrReplaceDetail + two textLayouts + mixed_text.addTextMixed on each side with no cache_layout. Body markdown uses cache_layout for committed rows; tool-run L2 does not.

ember_surface / teal_surface were locked in #691 as “distinct from teal_bg.” They are not, in paint. Thinking rows actually show a band because they fill warm_bg. Chips that need a visible TEAL fill use teal_border, not teal_surface.

Desired UX

  1. Bands must read as EMBER and TEAL fills — not an ink-only change, not two near-black slabs. Palette tokens only (no freehand hex, no ember_accent on the old band — accent stays the ✗ glyph). Pick fills that are actually visible against teal_bg (the previous *_surface lock failed this). Status line stays uncolored chrome.
  2. str_replace L2 is open while the turn is in flight so the operator can watch old → new without clicking. Same idea as thinking: active-turn pin (thinking.zig / thinking_collapse.zig). L0 must be open too while that pin holds, otherwise the item is hidden under a collapsed N tools called.
  3. When the turn completes, close the str_replace L2 expander (default collapsed, like committed thinking). Operator can re-open. Other tools stay default-collapsed L2; do not pin-open exec / read_file stdout.
  4. Open state must not stall the frame. Parse / split once (slot + revision, same O(dirty) discipline as toolrunCacheSlot). Committed side textLayouts use cache_layout = true. Do not GPA-alloc or re-shape both sides every frame. addTextMixed on ASCII/mono source is wasted face-switching — plain addText on fontMono() is enough for the bodies.

Empty new_string still shows a visible empty TEAL strip. Sentinels stay on the payload, stripped from paint. No per-line +/-. No emit / preview / cap change.

Likely seam

native/harness/src/ui/toolrun.zig paintToolRun L2 (has_detail and l2_expanded, ~250–341):

  • Fill tokens ~284 / ~306 (ember_surface / teal_surface) — invisible bands.
  • textLayout for old/new has no .cache_layout (contrast rich/paint_text.zig which defaults true on committed rows).
  • mixed_text.addTextMixed every frame on s.old / s.new.
  • Expand state is only the open-set hashmap; there is no active-turn pin for str_replace (thinking already has this policy).

Splitter itself (splitStrReplaceDetail in rich/toolrun.zig) is a no-alloc slice scan — cheap. The stall is paint, not parse.

Non-goals

  • Re-doing emit / buildStrReplacePreview / 4 KiB-per-side / 20/6 windows
  • Per-line +/- prefixes
  • Pin-opening other tools’ L2
  • Dual DOM chat
  • New caps
  • Freehand hex / ember_accent as a band fill

Acceptance

  • Expanded str_replace L2 shows two visibly colored bands (EMBER old, TEAL new) against the transcript — not ink-only, not near-black-on-near-black
  • While the turn is in flight, str_replace L2 (and its L0 group) is open without a click
  • When the turn completes, that str_replace L2 closes; operator can re-open
  • Open L2 does not stall /harness (committed sides cache_layout; no per-frame GPA; no per-frame mixed-face walk of the bodies)
  • Empty new still a visible TEAL strip; other tools unchanged; no protocol bump
  • docs/harness-limits.md Tool-run row matches (open-during-turn, visible fills, cached paint)

Related

  • Failed ship: #692 / plan #691
  • Product ask: #687 · parent #368
  • Analog (open while Busy, collapse when committed): native/harness/src/ui/thinking.zig + thinking_collapse.zig
  • Frame budget: docs/harness-limits.md · Frame budget; #404 slot-keyed cache_layout on body rows
  • Paint: native/harness/src/ui/toolrun.zig · split: native/harness/src/rich/toolrun.zig

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingharnessHarness / agent UIui

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions