diff --git a/docs/harness-limits.md b/docs/harness-limits.md index 64658ca7..3084e10c 100644 --- a/docs/harness-limits.md +++ b/docs/harness-limits.md @@ -164,11 +164,11 @@ see Group boundaries). | Topic | Behavior | |-------|----------| -| Header (default) | `N tools called` / `1 tool called`, default-**collapsed**. Right-aligned count chips shown only when >0: success **✓ N** (TEAL), failed **✗ N** (EMBER — danger only), pending **… N** (WARM). Status marks paint from embedded faces (`✓`/`✗` via DejaVu Sans Symbols; `…` via Noto heading) — **no tofu**. The **📋** (copy) control lives on this header row | -| Two-level expand | Level 1: one one-liner per tool — colored status glyph **✓/✗/…** is the *single* status channel, plus a preview: when the row has level-2 detail, `brief` (≤64 chars) if non-empty else the tool **`name`**. When there is **no** L2, host status fallbacks (`name · running…` / `name · ok` / `name · failed`) paint as **`name`** (glyph is the status); a real one-liner brief (e.g. `ERROR str_replace path: …`) still shows. Level 2: that tool's inline `detail` — a bounded, redacted **preview** (phase 3 #353) built server-side from flattened+redacted tool output: `exec` command/exit/stdout+stderr head/tail, `read_file`/`write_file`/`str_replace`/`list_dir` path/size/entries/brief preview, `http_*` URL/status/bounded body. Successful `str_replace` appends a redacted `-old_string` / `+new_string` audit block (4 KiB UTF-8 per side, `… (truncated)` when clipped). L2 windows **each side** independently (`STR_REPLACE_SIDE_HEAD_LINES` = 20 / `STR_REPLACE_SIDE_TAIL_LINES` = 6) so both sentinels and the start of new stay in the **payload**; generic 40/10 is not used on that shape. A content line that equals a header is escaped with a leading space. Paint **strips** those sentinels: old body on an **EMBER surface** (`ember_text` ink), new body on a **TEAL surface** (`teal_text` ink). Empty new is a visible TEAL strip (one mono line of fill). Other tools stay one `teal_text` block. Short single-line results carry **no** detail → a static label (no blank, duplicate-of-L1 expander). Clicking a row toggles; second click collapses; per-item isolation | +| Header (default) | `N tools called` / `1 tool called`, default-**collapsed**. A group that contains `str_replace` is **pinned open while its Busy turn is active** (even while that item is still running) — see Open state. Right-aligned count chips shown only when >0: success **✓ N** (TEAL), failed **✗ N** (EMBER — danger only), pending **… N** (WARM). Status marks paint from embedded faces (`✓`/`✗` via DejaVu Sans Symbols; `…` via Noto heading) — **no tofu**. The **📋** (copy) control lives on this header row | +| Two-level expand | Level 1: one one-liner per tool — colored status glyph **✓/✗/…** is the *single* status channel, plus a preview: when the row has level-2 detail, `brief` (≤64 chars) if non-empty else the tool **`name`**. When there is **no** L2, host status fallbacks (`name · running…` / `name · ok` / `name · failed`) paint as **`name`** (glyph is the status); a real one-liner brief (e.g. `ERROR str_replace path: …`) still shows. Level 2: that tool's inline `detail` — a bounded, redacted **preview** (phase 3 #353) built server-side from flattened+redacted tool output: `exec` command/exit/stdout+stderr head/tail, `read_file`/`write_file`/`str_replace`/`list_dir` path/size/entries/brief preview, `http_*` URL/status/bounded body. Successful `str_replace` appends a redacted `-old_string` / `+new_string` audit block (4 KiB UTF-8 per side, `… (truncated)` when clipped). L2 windows **each side** independently (`STR_REPLACE_SIDE_HEAD_LINES` = 20 / `STR_REPLACE_SIDE_TAIL_LINES` = 6) so both sentinels and the start of new stay in the **payload**; generic 40/10 is not used on that shape. A content line that equals a header is escaped with a leading space. Paint **strips** those sentinels: old body uses fill token **`ember_border`** (`ember_text` ink, `ember_muted` rim), new body uses **`teal_border`** (`teal_text` ink, `teal_muted` rim) so the bands read against `teal_bg`; inner `textLayout`s are fill-through. Empty new is a visible TEAL strip (one mono line of fill). Other tools stay one `teal_text` block. Short single-line results carry **no** detail → a static label (no blank, duplicate-of-L1 expander). Clicking a row toggles and a second click collapses **unless** the Busy-turn pin is holding that expander open (Open state); per-item isolation | | Detail vs scroll | Level-2 detail paints **inline** inside the one outer transcript scroller — there is **no** nested `scrollArea`. Command/output previews use the embedded **Vera Sans Mono** face (`exec`/filesystem/`http_*`, and any **multi-line** detail so MCP/custom-tool output also reads as a block); prose/single-line detail stays the body face. Long detail is bounded per-tool by the server preview cap (`TOOL_RUN_PREVIEW_MAX_CHARS` = 100k) with the **real** head **40** / tail **10** lines + `… (M more lines)`, and the whole group is bounded by the encode budget + hard clamp below, so the transcript wheel is not trapped | -| Painter | `native/harness/src/ui/toolrun.zig` → `paintToolRun`; payload decode in `native/harness/src/rich/toolrun.zig` (fail-open → raw body text) | -| Open state | Two module-level `std.AutoHashMap(dvui.Id, void)` open-branch maps (per message id / per item id) survive repaint / `update_last`; cleared on reload / Clear / truncate → collapsed-by-default | +| Painter | `native/harness/src/ui/toolrun.zig` → `paintToolRun`; payload decode in `native/harness/src/rich/toolrun.zig` (fail-open → raw body text). `str_replace` sides are split **at decode** (slot-cached, O(dirty)); committed side `textLayout`s use `cache_layout`; side bodies are mono `addText` (not mixed-face). L1 expander is extra `+2`; no-detail static label is `+9` (never the same persist slot) | +| Open state | Two module-level `std.AutoHashMap(dvui.Id, void)` open-branch maps (per message id / per item id) survive repaint / `update_last`; cleared on reload / Clear / truncate → collapsed-by-default. A group that contains `str_replace` has L0 **pinned open while its Busy turn is active** (even while that item is still running); `str_replace` L2 pins once detail exists. Both **collapse when the turn completes** (operator may re-open). Other tools stay default-collapsed L2 | | Group boundaries | Grouping keys off the **last painted ring row** via the host's `lastRingRowIsToolRun` flag (the host is the only ring writer): a tool event grows the open card **iff** the last ring row is a tool-run; a **thinking row last**, a real (non-empty trimmed) assistant segment, a user send, or an error/turn-end opens a NEW card at `1`; empty **and whitespace-only** assistant (or a blank `text_delta`) is **not** a boundary. Counts **paint live** — each tool event opens/grows the kind-6 card immediately (`1 tool called` → `2…`) via `update_last`, never withheld until a boundary (removed commit-once). A group still rolls to a new card at `TOOL_RUN_ITEMS_MAX`, and the rolled (full) card is never grown | | Group bound | A group stores at most **200** items (`TOOL_RUN_ITEMS_MAX`); a longer streak rolls a new `tool_run` group (counts stay exact across groups). The whole group encodes into **one** message, so the host enforces a group **encode budget** (`TOOL_RUN_GROUP_DETAIL_ENC_MAX` ≈ 229 KiB of encoded `detail`) that clips/omits previews plus an encode-time hard clamp to `TOOL_RUN_MSG_HARD_MAX` (`262 144`) — a multi-item streak of large previews can **never** overflow the ring/cloud per-msg cap (it clips an explicit `…` or falls back to the L1 static label, never a silent mid-payload truncation) | | Session | One `tool_run` message per group round-trips local + cloud and repaints collapsed on restore; **not** folded into the model prompt (display-only). Caveat: prior tool summaries no longer reach the model on a **continue after a mid-tool cancel** — the model sees only persisted assistant prose and may re-run or infer tools. That is the documented product rule (kept for the cancel/Copy-fed transcript). A **confirmed successful `change_dir`** still lands in the session `cwd` even when the turn later cancels / times out / hard-errors — the mid-tool-cancel re-run rule is unchanged, but the next turn boots where the model actually worked instead of against a silently-reset cwd. | diff --git a/native/harness/src/rich/toolrun.zig b/native/harness/src/rich/toolrun.zig index dd293319..9668b756 100644 --- a/native/harness/src/rich/toolrun.zig +++ b/native/harness/src/rich/toolrun.zig @@ -23,12 +23,23 @@ pub const Status = enum(u8) { fail = 2, }; +/// L2 preview sides for a successful `str_replace` audit block. +/// Slices point into `detail` (no allocation). +pub const StrReplaceSides = struct { + status: []const u8, + old: []const u8, + new: []const u8, +}; + pub const Item = struct { id: u32, status: Status, name: []const u8, brief: []const u8, detail: []const u8, + /// Set at decode when `name == "str_replace"` and the detail has the L2 + /// sentinel shape. Slices point into `detail` (no extra alloc). + str_replace_sides: ?StrReplaceSides = null, }; /// L1 item label. @@ -53,14 +64,6 @@ fn isHostStatusFallback(brief: []const u8, name: []const u8) bool { std.mem.eql(u8, rest, " · failed"); } -/// L2 preview sides for a successful `str_replace` audit block. -/// Slices point into `detail` (no allocation). -pub const StrReplaceSides = struct { - status: []const u8, - old: []const u8, - new: []const u8, -}; - const STR_REPLACE_OLD_SENTINEL = "-old_string"; const STR_REPLACE_NEW_SENTINEL = "+new_string"; @@ -94,6 +97,19 @@ pub fn splitStrReplaceDetail(detail: []const u8) ?StrReplaceSides { return null; } +/// L0 expander pin: open while the Busy turn is active if this group contains +/// a `str_replace` item (detail optional — so the row is visible while running). +pub fn toolRunL0PinnedOpen(is_active_turn: bool, has_str_replace: bool, operator_open: bool) bool { + return operator_open or (is_active_turn and has_str_replace); +} + +/// L2 expander pin: open while the Busy turn is active for a `str_replace` +/// that already has detail (bands need the payload). Operator-open wins too. +pub fn strReplaceL2PinnedOpen(is_active_turn: bool, name: []const u8, has_detail: bool, operator_open: bool) bool { + if (operator_open) return true; + return is_active_turn and has_detail and std.mem.eql(u8, name, "str_replace"); +} + pub const ToolRun = struct { ok: u32 = 0, fail: u32 = 0, @@ -198,12 +214,14 @@ pub fn decode(alloc: std.mem.Allocator, text: []const u8) ?Decoded { const name = unescape(alloc, name_s) catch continue; const brief = unescape(alloc, brief_s) catch continue; const detail = unescape(alloc, detail_s) catch continue; + const sides = if (std.mem.eql(u8, name, "str_replace")) splitStrReplaceDetail(detail) else null; items.append(alloc, .{ .id = id, .status = status, .name = name, .brief = brief, .detail = detail, + .str_replace_sides = sides, }) catch return null; } // Recount statuses from the kept items so the UI header can never disagree @@ -333,7 +351,8 @@ test "ui id packing is unique for a full group and across rows" { // reviewer-requested cheap guard that MAX_ITEMS items never alias. const item_stride: usize = 1024; const row_step: usize = 1000003; - const slot_max: usize = 4; + // Highest extra in `ui/toolrun.zig` paintToolRun: +9 static L1 label. + const slot_max: usize = 9; // Within one row: every (item, slot) pair maps to a distinct widget id. var seen = std.AutoHashMap(u64, void).init(std.testing.allocator); @@ -471,3 +490,45 @@ test "splitStrReplaceDetail empty old with both sentinels is empty body not null try std.testing.expectEqualStrings("", sides.old); try std.testing.expectEqualStrings("new", sides.new); } + +test "decode str_replace encoded line stores sides; exec stays null" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + const text = + "toolrun\t1\t1/0/1\n" ++ + "1\tok\tstr_replace\tstr_replace lib/foo.ts: ok replacements=1 bytes=12\t" ++ + "str_replace lib/foo.ts: ok replacements=1 bytes=12\\n-old_string\\nold line\\n+new_string\\nnew line\n" ++ + "2\trunning\texec\texec · running…\t"; + var d = decode(a, text) orelse return error.ExpectedDecode; + defer d.deinit(); + try std.testing.expectEqual(@as(usize, 2), d.run.items.len); + + const sr = d.run.items[0]; + try std.testing.expectEqualStrings("str_replace", sr.name); + const sides = sr.str_replace_sides orelse return error.ExpectedSides; + try std.testing.expectEqualStrings("str_replace lib/foo.ts: ok replacements=1 bytes=12", sides.status); + try std.testing.expectEqualStrings("old line", sides.old); + try std.testing.expectEqualStrings("new line", sides.new); + + try std.testing.expectEqualStrings("exec", d.run.items[1].name); + try std.testing.expect(d.run.items[1].str_replace_sides == null); +} + +test "toolRunL0PinnedOpen pins on name during active turn even without detail" { + try std.testing.expect(toolRunL0PinnedOpen(true, true, false)); + try std.testing.expect(!toolRunL0PinnedOpen(true, false, false)); + try std.testing.expect(toolRunL0PinnedOpen(true, false, true)); + try std.testing.expect(!toolRunL0PinnedOpen(false, true, false)); + try std.testing.expect(toolRunL0PinnedOpen(false, true, true)); +} + +test "strReplaceL2PinnedOpen requires active turn + str_replace + detail" { + try std.testing.expect(strReplaceL2PinnedOpen(true, "str_replace", true, false)); + try std.testing.expect(!strReplaceL2PinnedOpen(true, "str_replace", false, false)); + try std.testing.expect(!strReplaceL2PinnedOpen(true, "exec", true, false)); + try std.testing.expect(!strReplaceL2PinnedOpen(false, "str_replace", true, false)); + try std.testing.expect(strReplaceL2PinnedOpen(false, "str_replace", true, true)); + try std.testing.expect(strReplaceL2PinnedOpen(false, "exec", true, true)); +} diff --git a/native/harness/src/ui/toolrun.zig b/native/harness/src/ui/toolrun.zig index f6e1f547..b150d6d5 100644 --- a/native/harness/src/ui/toolrun.zig +++ b/native/harness/src/ui/toolrun.zig @@ -11,6 +11,7 @@ const mixed_text = @import("../rich/mixed_text.zig"); const state = @import("state.zig"); const chrome = @import("chrome.zig"); const metrics = @import("metrics.zig"); +const bridge = @import("../bridge.zig"); /// True for tool names whose level-2 detail is a command/output block (exec /// stdout/stderr, filesystem results, http bodies). Those previews paint in the @@ -85,10 +86,34 @@ pub fn paintToolRun( const l1_raw: usize = id_base + 7; const l1_key: dvui.Id = @enumFromInt(l1_raw); - var l1_expanded = state.toolrun_open_l1.contains(l1_key); if (total == 0) return false; + // Turn-membership, not thinking-specific: `thinking_collapse_state.isActiveTurnFull` + // answers "does physical ring slot `s` fall inside the current Busy turn's + // written range [active_turn_start_slot, ring_head)?" — a general predicate the + // thinking rows also use. We reuse it to pin a tool-run group open while its + // turn is active. The helper lives in the thinking_collapse module for history + // reasons; it is NOT a thinking-only concept. Do not rename/relocate it as a + // layering cleanup — the name is Plan #715-locked. + const is_active = if (slot) |s| + state.thinking_collapse_state.isActiveTurnFull(s, bridge.messageHead(), bridge.RING_CAP) + else + false; + var has_str_replace = false; + for (runv.items) |it| { + if (std.mem.eql(u8, it.name, "str_replace")) { + has_str_replace = true; + break; + } + } + const pin_l0 = rich_toolrun.toolRunL0PinnedOpen(is_active, has_str_replace, false); + var l1_expanded = rich_toolrun.toolRunL0PinnedOpen( + is_active, + has_str_replace, + state.toolrun_open_l1.contains(l1_key), + ); + // ── Level 0 header: expander label + right-aligned colored count chips ── var header_label: [40]u8 = undefined; const label = @@ -115,7 +140,14 @@ pub fn paintToolRun( .gravity_y = 0.5, .id_extra = id_base + 2, }); - if (open) state.toolrun_open_l1.put(l1_key, {}) catch {} else _ = state.toolrun_open_l1.remove(l1_key); + if (pin_l0) { + // Pinned for an in-flight str_replace; click cannot collapse. + l1_expanded = true; + } else if (open) { + state.toolrun_open_l1.put(l1_key, {}) catch {}; + } else { + _ = state.toolrun_open_l1.remove(l1_key); + } // Trailing pack (right): 📋 then ✓N / ✗N / …N — one gravity box so // padding/baseline match (operator: glyphs were tiny + clipboard pad fat @@ -168,19 +200,27 @@ pub fn paintToolRun( const l2_key: dvui.Id = @enumFromInt(l1_raw *% 31 + it.id); // `it.id` is 1-based per group with up to MAX_ITEMS items. Each item // owns a 1024-wide namespace (`it_id *% 1024`) under this message's - // id_base, holding up to 5 widget slots, so every (item, widget) + // id_base, holding slots `+0..+9`, so every (item, widget) // pair is unique within the row even for a full 200-item group; // 1000003 > 200·1024 keeps distinct rows disjoint. Matches the // rich/paint.zig `msg_index *% …` discipline (id = src + id_extra, // not a parent chain). const it_id: usize = it.id; // Widget slots inside the item's 1024-wide namespace: - // +0 item box · +1 status glyph · +2 expander / static label + // +0 item box · +1 status glyph · +2 expander (has-detail) // +3 detail box · +4 status (str_replace) / detail body // +5 old box · +6 old tl · +7 new box · +8 new tl (str_replace sides) + // +9 static L1 label (no-detail) — distinct from +2 so a running + // row that later gains detail does not reuse expander persist const item_base: usize = id_base + it_id *% 1024; const has_detail = it.detail.len > 0; - var l2_expanded = state.toolrun_open_l2.contains(l2_key); + const pin_l2 = rich_toolrun.strReplaceL2PinnedOpen(is_active, it.name, has_detail, false); + var l2_expanded = rich_toolrun.strReplaceL2PinnedOpen( + is_active, + it.name, + has_detail, + state.toolrun_open_l2.contains(l2_key), + ); { var item_head = dvui.box(src, .{ .dir = .horizontal }, .{ @@ -229,13 +269,19 @@ pub fn paintToolRun( .expand = .horizontal, .gravity_y = 0.5, }); - if (open) state.toolrun_open_l2.put(l2_key, {}) catch {} else _ = state.toolrun_open_l2.remove(l2_key); + if (pin_l2) { + l2_expanded = true; + } else if (open) { + state.toolrun_open_l2.put(l2_key, {}) catch {}; + } else { + _ = state.toolrun_open_l2.remove(l2_key); + } } else { // No level-2 detail — static label, not a blank expander. // `itemLabel` already chose brief (real error/path one-liner) // or `name` (host status fallback). Glyph is the status channel. var tl = dvui.textLayout(src, .{}, .{ - .id_extra = item_base + 2, // expander slot — mutually exclusive + .id_extra = item_base + 9, // not +2 — expander persist is distinct .expand = .horizontal, .color_text = palette.teal_text, .gravity_y = 0.5, @@ -255,16 +301,15 @@ pub fn paintToolRun( }); defer detail.deinit(); - const sides = if (std.mem.eql(u8, it.name, "str_replace")) - rich_toolrun.splitStrReplaceDetail(it.detail) - else - null; + const sides = it.str_replace_sides; + const sides_stable = it.status != .running; if (sides) |s| { { - var st = dvui.textLayout(src, .{}, .{ + var st = dvui.textLayout(src, .{ .cache_layout = sides_stable }, .{ .id_extra = item_base + 4, .expand = .horizontal, + .background = false, .color_text = palette.teal_text, }); mixed_text.addTextMixed(st, s.status, .theme(.body), .{ @@ -281,20 +326,22 @@ pub fn paintToolRun( .id_extra = item_base + 5, .expand = .horizontal, .background = true, - .color_fill = palette.ember_surface, + .color_fill = palette.ember_border, + .color_border = palette.ember_muted, + .border = .{ .x = 1, .y = 1, .w = 1, .h = 1 }, .color_text = palette.ember_text, .padding = .all(2), .min_size_content = .{ .w = 0, .h = band_h }, }); defer old_box.deinit(); - var otl = dvui.textLayout(src, .{}, .{ + var otl = dvui.textLayout(src, .{ .cache_layout = sides_stable }, .{ .id_extra = item_base + 6, .expand = .horizontal, + .background = false, .color_text = palette.ember_text, + .font = mono, }); - mixed_text.addTextMixed(otl, s.old, mono, .{ - .color_text = palette.ember_text, - }); + otl.addText(s.old, .{}); otl.deinit(); } @@ -303,24 +350,26 @@ pub fn paintToolRun( .id_extra = item_base + 7, .expand = .horizontal, .background = true, - .color_fill = palette.teal_surface, + .color_fill = palette.teal_border, + .color_border = palette.teal_muted, + .border = .{ .x = 1, .y = 1, .w = 1, .h = 1 }, .color_text = palette.teal_text, .padding = .all(2), .min_size_content = .{ .w = 0, .h = band_h }, }); defer new_box.deinit(); - var ntl = dvui.textLayout(src, .{}, .{ + var ntl = dvui.textLayout(src, .{ .cache_layout = sides_stable }, .{ .id_extra = item_base + 8, .expand = .horizontal, + .background = false, .color_text = palette.teal_text, + .font = mono, }); - mixed_text.addTextMixed(ntl, s.new, mono, .{ - .color_text = palette.teal_text, - }); + ntl.addText(s.new, .{}); ntl.deinit(); } } else { - var tl = dvui.textLayout(src, .{}, .{ + var tl = dvui.textLayout(src, .{ .cache_layout = sides_stable }, .{ .id_extra = item_base + 4, .expand = .horizontal, .color_text = palette.teal_text,