From 63a1fb204646fb40f752a714688135bd04bf39f2 Mon Sep 17 00:00:00 2001 From: Yunfei He Date: Sat, 30 May 2026 01:51:49 +0800 Subject: [PATCH] fix(runtime): draw box borders per-edge without the min-size guard (Ink parity, G05+G15) (#37) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(runtime): draw box borders per-edge without the min-size guard (Ink parity, G05+G15) Removed blanket `w<2||h<2` return from drawBorder; replaced it with a `w<1||h<1` degenerate guard. Vertical sides now start at `offsetY = top ? 1 : 0` and run for `Math.max(0, h - topRows - bottomRows)`, matching Ink render-border.ts:133. Fixes: (G05) a 1-cell-tall box with only side rails rendered nothing; (G15) with borderTop=false the left/right rails were shifted one row down. Updated 4 existing snapshots that encoded the old buggy behavior and added 3 new tests that verified red before green. Co-Authored-By: Claude Opus 4.8 * chore(parity): ledger — G05+G15 pr-open, reconcile G03 merged, log snapshot decision Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .agents/docs/parity-ledger.md | 7 +- .../integration/components/borders.test.tsx | 78 ++++++++++++++++++- packages/runtime/src/paint/paint.ts | 22 +++++- 3 files changed, 96 insertions(+), 11 deletions(-) diff --git a/.agents/docs/parity-ledger.md b/.agents/docs/parity-ledger.md index d5c0dac..124bc62 100644 --- a/.agents/docs/parity-ledger.md +++ b/.agents/docs/parity-ledger.md @@ -24,6 +24,7 @@ Non-obvious calls made while fixing gaps, recorded for review in the final repor - _"Box backgroundColor survives border rendering"_: unchanged, still passes (the inner fill keeps the bg). - Added _"Box backgroundColor does not bleed onto border glyphs (Ink parity)"_ as the failing-first reproduction. - **2026-05-30 — G02 (useAnimation throttle):** to actually fix the gap by default (not just when a caller passes maxFps), `maxFps` now defaults to **30** (Ink: `options.maxFps ?? 30`) and a single `renderThrottleMs = ceil(1000/maxFps)` feeds BOTH the commit scheduler and `createAnimationScheduler`, mirroring Ink's one-value architecture. Behavior change: the default commit-throttle shifts 32ms→34ms (no test depended on 32ms; explicit `maxFps` already drove commit cadence on main). Test rewrite: the pre-existing `"delta accounts for throttled ticks"` test was weak/self-contradictory (asserted only `delta>0`, passed trivially against the bug) — replaced with `"delta accumulates across coalesced ticks…"` (maxFps:5, 200ms window) plus a default-path test using a count-based assertion (≈31 rendered ticks unfixed vs <15 fixed) so it deterministically discriminates. Both verified red on unfixed. +- **2026-05-30 — G05+G15 (border geometry):** rewrote `paint.ts` drawBorder to draw each edge independently (mirroring Ink render-border.ts): removed the blanket `w<2||h<2` early-return (→ `w<1||h<1`) and changed the vertical-side loop to start at `offsetY = top?1:0` with run length `max(0, h - visibleTop - visibleBottom)`. Per conflict policy, 4 existing snapshot tests that encoded the buggy hide-top/bottom output were updated to Ink-correct values (rails on the content row, not shifted/missing) — codex traced each against Ink render-border.ts and confirmed they match (not just code-blessed). Normal (top+bottom) boxes are byte-identical to before. - **2026-05-29 — G06 REFUTED (false positive from the audit):** the audit claimed ``'s fn gets a hardcoded index `0` "instead of the childNode index". Re-verification against Ink `output.ts:230-239` shows Ink's index is the **line index** (transformers apply per output line: `transformer(line, index)`), not a child index — the audit misread it. vue-tui **already** applies per-line line indices for multi-line (block) transforms via the yoga-carrier path: the existing tests `transform with multiple lines` → `[0: hello world]\n[1: goodbye world]` and transform-yoga `[0: hello]\n[1: world]` pass on unmodified code. `paint.ts:314`'s `transform(innerText, 0)` is only the inline ``-inside-`` path, whose content is a single logical line where `0` matches Ink (all inline tests assert `[0: …]`). No observable gap; not fixed. ## Confirmed gaps @@ -34,9 +35,9 @@ Non-obvious calls made while fixing gaps, recorded for review in the final repor | --- | ------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | ------- | ---------------------------------- | --- | | G01 | static-newline-spacer | Static keeps every already-written item permanently mounted instead of unmounting it | P1 | merged | `fix/parity-static-unmount` | #34 | | G02 | app-exit-instances-animation-sr | useAnimation does not coalesce ticks within the render-throttle window — delta does not 'account for throttled renders' | P1 | merged | `fix/parity-useanimation-throttle` | #35 | -| G03 | render-lifecycle-reconciler | Live screen-reader render path is missing; commit() always paints the visual grid | P1 | pr-open | `fix/parity-sr-render` | #36 | +| G03 | render-lifecycle-reconciler | Live screen-reader render path is missing; commit() always paints the visual grid | P1 | merged | `fix/parity-sr-render` | #36 | | G04 | box-layout-border | Border edges incorrectly inherit the Box backgroundColor | P2 | merged | `fix/parity-border-bg` | #30 | -| G05 | box-layout-border | Borders skipped when content area is 1 cell tall or wide (w<2 / h<2 guard) | P2 | todo | — | — | +| G05 | box-layout-border | Borders skipped when content area is 1 cell tall or wide (w<2 / h<2 guard) | P2 | pr-open | `fix/parity-border-1cell` | #37 | | G06 | text-wrap-transform | Nested / transform fn receives hardcoded index 0 instead of childNode index | P2 | refuted | — | — | | G07 | input-keypress-kitty-paste | Kitty-protocol Ctrl+C triggers app exit in vue-tui but only suppresses the handler in Ink | P2 | todo | — | — | | G08 | focus | useFocus does not react to changes in the id prop | P2 | merged | `fix/parity-usefocus-id` | #31 | @@ -46,7 +47,7 @@ Non-obvious calls made while fixing gaps, recorded for review in the final repor | G12 | render-lifecycle-reconciler | Renderer frame width/rows lack terminal-size fallback (only ?? defaults) | P2 | merged | `fix/parity-renderer-size` | #33 | | G13 | box-layout-border | Custom border style objects (BoxStyle) not supported | P3 | todo | — | — | | G14 | app-exit-instances-animation-sr | No per-stdout instance reuse/guard — two concurrent renderers can compete for the same stdout | P3 | todo | — | — | -| G15 | box-layout-border | Vertical border sides not shifted up when borderTop=false (Ink offsetY) — left/right rails mispositioned | P2 | todo | — | — | +| G15 | box-layout-border | Vertical border sides not shifted up when borderTop=false (Ink offsetY) — left/right rails mispositioned | P2 | pr-open | `fix/parity-border-1cell` | #37 | | G16 | box-layout-border | Per-edge borderDimColor=false cannot override general borderDimColor (`\|\| dimAll` vs Ink's `??`) | P3 | todo | — | — | | G17 | render-lifecycle-reconciler | Screen-reader live-path edges: still grid-painted (Ink linearizes, skipStaticElements:false) + empty SR frame gets a trailing newline (Ink writes wrapped output directly) | P3 | todo | — | — | diff --git a/packages/runtime-tests/integration/components/borders.test.tsx b/packages/runtime-tests/integration/components/borders.test.tsx index 5e8a4b1..a82e963 100644 --- a/packages/runtime-tests/integration/components/borders.test.tsx +++ b/packages/runtime-tests/integration/components/borders.test.tsx @@ -676,8 +676,9 @@ test("render border edge changes after update when borderStyle is unchanged", as showTop.value = false; await nextTick(); + // G15 fix: with borderTop=false, side rails now start at row 0 (Ink parity) expect(lastFrame()).toMatchInlineSnapshot(` - " Content + "│Content│ ╰───────╯" `); @@ -704,9 +705,10 @@ test("hide top border", async ({ expect }) => { )), { columns: 100 }, ); + // G15 fix: side rails now start at row 0 when borderTop=false (Ink parity) expect(lastFrame()).toMatchInlineSnapshot(` "Above - Content + │Content│ ╰───────╯ Below" `); @@ -726,10 +728,11 @@ test("hide bottom border", async ({ expect }) => { )), { columns: 100 }, ); + // G15 fix: side rails now span the full content height when borderBottom=false (Ink parity) expect(lastFrame()).toMatchInlineSnapshot(` "Above ╭───────╮ - Content + │Content│ Below" `); }); @@ -748,9 +751,11 @@ test("hide top and bottom borders", async ({ expect }) => { )), { columns: 100 }, ); + // G05+G15 fix: side rails now render even at h=1 when both top and bottom are + // hidden — the per-edge geometry draws them at the single content row (Ink parity) expect(lastFrame()).toMatchInlineSnapshot(` "Above - Content + │Content│ Below" `); }); @@ -1199,6 +1204,71 @@ test("foreground, background and dim combine correctly", async ({ expect }) => { expect(frame).toContain("[2m"); }); +// G05 — height-1 box with side-only borders renders rails (Ink parity) +// A box that is exactly 1 cell tall (borderTop/Bottom=false) must still render +// the left and right rails on the single content row, producing │X│ output. +// Previously the blanket `w < 2 || h < 2` guard aborted the entire drawBorder. +test("G05: height-1 box with side-only borders renders rails", async ({ expect }) => { + const { lastFrame } = await render( + defineComponent(() => () => ( + // height=1 → only 1 row; no top/bottom borders, only left+right rails + + X + + )), + { columns: 20 }, + ); + const frame = stripAnsi(lastFrame()!); + // The single row must carry the left AND right rails + expect(frame).toContain("│X│"); +}); + +// G05 — width-1 box with top/bottom-only borders renders the edge glyphs (Ink parity) +// A box that is exactly 1 cell wide (after removing left/right borders) must still +// render the top and bottom horizontal edges, each a single glyph. +test("G05: width-1 box with top/bottom-only borders renders edge glyphs", async ({ expect }) => { + const { lastFrame } = await render( + defineComponent(() => () => ( + // width=1 content area; no left/right borders; only top+bottom + + X + + )), + { columns: 20 }, + ); + const frame = stripAnsi(lastFrame()!); + const lines = frame.split("\n"); + // Top edge must be present (─) + expect(lines[0]).toContain("─"); + // Bottom edge must be present (─) + expect(lines[lines.length - 1]).toContain("─"); +}); + +// G15 — vertical side rails are not shifted down when borderTop=false (Ink parity) +// With borderTop=false, the vertical sides must start at row 0 (the content row), +// not row 1 (which is how the buggy i=1 loop positioned them). +test("G15: side rails appear on the first content row when borderTop=false", async ({ expect }) => { + const { lastFrame } = await render( + defineComponent(() => () => ( + + Content + + )), + { columns: 20 }, + ); + const frame = stripAnsi(lastFrame()!); + const lines = frame.split("\n"); + // First line is the content row (no top border); it must carry both rails. + // The "single" style uses │ (U+2502 BOX DRAWINGS LIGHT VERTICAL), not ASCII |. + expect(lines[0]).toMatch(/^│.*│$/); +}); + // borderDimColor should not dim styled child Text touching left edge test("borderDimColor does not dim styled child Text touching left edge", async ({ expect }) => { const { lastFrame } = await render( diff --git a/packages/runtime/src/paint/paint.ts b/packages/runtime/src/paint/paint.ts index 3c896e0..fb75141 100644 --- a/packages/runtime/src/paint/paint.ts +++ b/packages/runtime/src/paint/paint.ts @@ -335,7 +335,13 @@ function drawBorder( if (!style) return; const chars = (cliBoxes as unknown as Record)[style]; if (!chars) return; - if (w < 2 || h < 2) return; + // No blanket min-size guard here — each edge is drawn independently when it is + // visible and its run length is ≥ 1. This matches Ink's render-border.ts which + // has no such guard and draws every edge on its own, so a 1-cell-tall box with + // only side rails still renders │X│ (G05), and a 1-cell-wide box with only + // top/bottom still renders the top/bottom glyph. Guard individual repeat() + // counts with Math.max(0, …) so a degenerate dimension doesn't throw. + if (w < 1 || h < 1) return; const top = props["borderTop"] !== false; const bottom = props["borderBottom"] !== false; @@ -375,9 +381,17 @@ function drawBorder( const raw = bl + chars.bottom.repeat(fill) + br; output.write(x, y + h - 1, [colorizeEdge(safeSliceEnd(raw, w), "bottom")], transformers); } - for (let i = 1; i < h - 1; i++) { - if (left) output.write(x, y + i, [colorizeEdge(chars.left, "left")], transformers); - if (right) output.write(x + w - 1, y + i, [colorizeEdge(chars.right, "right")], transformers); + + // Ink parity (render-border.ts:133): vertical sides start at y + offsetY where + // offsetY = showTopBorder ? 1 : 0. Without this, the loop starting at i=1 + // always skips row 0, shifting rails one row down when borderTop=false (G15). + // The run length equals h minus the visible top/bottom rows, clamped ≥ 0. + const offsetY = top ? 1 : 0; + const verticalRun = Math.max(0, h - (top ? 1 : 0) - (bottom ? 1 : 0)); + for (let i = 0; i < verticalRun; i++) { + if (left) output.write(x, y + offsetY + i, [colorizeEdge(chars.left, "left")], transformers); + if (right) + output.write(x + w - 1, y + offsetY + i, [colorizeEdge(chars.right, "right")], transformers); } }