fix(runtime): clip horizontally before applying transforms (Ink parity, G63) (#63)
Ink's output.ts maps sliceAnsi(line, from, to) (the horizontal clip) over the
lines BEFORE the lines.entries() loop that runs transformer(line, index), and it
never re-clips the transformer's output. vue-tui's paint write() did the reverse
(commit 2c99431 deliberately moved clip AFTER transform), so a width-sensitive
transform inside an overflowX:"hidden" box received the FULL line and had its
result sliced — corrupting gradients (wrong char count) and OSC-8 hyperlinks
(closing sequence sliced off).
Reorder so the per-line horizontal clip runs first, then the transformers apply
to the already-clipped span. The post-vertical-clip line index passed to each
transformer is unchanged, so transform index/nesting behavior (G21/G32/G52/G58)
is untouched. The pre-existing overflow test that asserted the old re-clip order
(transform-returns-wide-char dropped at the boundary) is rewritten to Ink's
clip-then-transform output (the widened glyph overflows), verified against the
pinned Ink reference (7.0.4 / 40b3a75) via renderToString.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Yunfei He
@@ -16,6 +16,7 @@
|
||||
| sweep-6 (2026-05-30) | `40b3a75` | re-audit (G39 merged) → 9 cand → 8 confirmed: **3 MEDIUM (G44 Static layout-style dropped, G45 programmatic focus-while-disabled, G46 non-empty SR-frame trailing newline) + 5 LOW (G47-G51)** | recorded |
|
||||
| sweep-7 (2026-05-30) | `40b3a75` | re-audit (G44/G45/G46 merged) → 6 confirmed: **1 MEDIUM (G52 Transform index shift from comment-node siblings) + 5 LOW (G53-G57)** | recorded |
|
||||
| sweep-8 (2026-05-30) | `40b3a75` | re-audit (G52 merged) → 5 confirmed (3 refuted): **2 MEDIUM (G58 standalone-Transform bare-text dropped, G59 SR frame routes through interactive clearTerminal/static path) + 3 LOW (G60-G62)** | recorded |
|
||||
| sweep-9 (2026-05-30) | `40b3a75` | re-audit (G58/G59 merged) → 8 cand → 4 confirmed (4 refuted): **2 MEDIUM (G63 transform-then-clip order, G64 Static forces terminal width vs Ink content-size) + 2 LOW (G65-G66)** | recorded |
|
||||
|
||||
Refuted (NOT gaps): ~~`exit()` second-wins — vue-tui is already guarded~~ **REVERSED by sweep-4: it IS last-wins vs Ink first-wins → now tracked as G33**; kitty key-release printable-text suppression — Ink behaves the same.
|
||||
|
||||
@@ -36,6 +37,8 @@ Non-obvious calls made while fixing gaps, recorded for review in the final repor
|
||||
- **2026-05-30 — G33 (exit() first-wins) — REVERSES sweep-1 refutation + test rewrite:** sweep-1 wrongly refuted exit()-second-wins ('already guarded'); sweep-4 + a red test confirmed vue-tui was LAST-wins. Added an `exitInitiated || teardownStarted` guard (Ink's `isUnmounted || isUnmounting` first-wins). Per conflict policy, 3 existing exit tests that ASSERTED the last-wins bug (resolving 'second', with comments noting Ink does first-wins) were rewritten to assert first-wins ('first') — they had documented the divergence; now aligned to Ink.
|
||||
- **2026-05-30 — G23 (SR <Transform>) — spec corrected via empirical Ink check:** the sweep-2 finding claimed Ink applies the Transform's fn to its squashed SR children. Building Ink from source and tracing squash-text-nodes.ts showed Ink only applies `internal_transform` of CHILD nodes, never the top-level node handed to squashTextNodes — so a `<Transform>` directly under a `<Box>` outputs its children CONCATENATED with no transform. Only the `\n`→`""` join was a real bug. Fixed to match Ink (concat, no top-level transform); applying it would have DIVERGED. (Per align-with-Ink; codex independently confirmed.)
|
||||
- **2026-05-29 — G06 REFUTED (false positive from the audit):** the audit claimed `<Transform>`'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 `<Transform>`-inside-`<Text>` path, whose content is a single logical line where `0` matches Ink (all inline tests assert `[0: …]`). No observable gap; not fixed.
|
||||
- **2026-05-30 — G63 left-edge wide-glyph origin → ALIGNED TO INK (x = clip.x1) + 2 test rewrites — FLAG FOR MAINTAINER:** the G63 follow-up codex review found one must-fix in `paint.ts` write(). When a wide glyph (e.g. `中`) straddles the LEFT clip edge it is dropped whole; vue-tui then advanced the write origin by the dropped glyph's EXTRA column (`lineX += droppedWidth`), emitting a leading space (`" x"`). Ink instead flatly sets the per-line write origin to the clipped left edge (`output.ts:210-212` `if (x < clip.x1) x = clip.x1`) and writes the kept content from there with NO leading offset. Verified empirically against the pinned Ink build (40b3a75) via `renderToString` on the SAME tree (`<Box width=4 overflow=hidden><Box marginLeft=-1 flexShrink=0>…</Box></Box>`): plain `中x` → `"x"`; `<Transform>=>"z"` → `"z"`; `<Transform>=>[l]` → `"[x]"` (vue-tui previously emitted `" x"`/`" z"`/`" [x]"`). Fix: replaced the `droppedWidth` origin advance with `if (lineX < clipH.x1) lineX = clipH.x1`; the G63 clip-before-transform order is unchanged, right-edge clipping and the no-clip path are untouched. Per conflict policy, 2 wide-char tests that LOCKED the old leading-space padding were rewritten to Ink's clipped-origin output: `overflow.test.tsx` _"text after clipped left-edge wide char …"_ (`" x"`→`"x"`) and `grapheme-clip.test.tsx` _"left-edge wide grapheme straddle …"_ (`startsWith(" x")`→`=== "x"`). **FLAG:** if the maintainer prefers vue-tui's leading-space padding (keeps the dropped wide cell visually reserved) over Ink's flush clip origin, this is the single change to reverse (restore the `droppedWidth` advance + revert those 2 test assertions).
|
||||
- **2026-05-30 — G63 (clip-then-transform) — test rewrite per conflict policy:** `paint.ts` write() applied the line transformer to the FULL line then horizontally clipped (added by commit 2c99431 "reorder clip/transform pipeline … so transforms are clipped correctly", which deliberately put clip AFTER transform to stop Transform-widened text escaping the clip). That reverses Ink's `output.ts` order — Ink maps `sliceAnsi(line, from, to)` (horizontal clip) over `lines` BEFORE the `lines.entries()` loop that runs `transformer(line, index)`, and never re-clips the transformer's output. Verified empirically with `renderToString` against the pinned Ink build (7.0.4 / 40b3a75): `<Box width=5 overflowX=hidden><Box width=16 flexShrink=0><Transform l=>'['+l+']'>hello world</Transform></Box></Box>` → `"[hello]"` (clip "hello" first, then bracket), not `"[hell"` (bracket then slice). Reordered the two blocks in `paint.ts`; the post-vertical-clip line `index` passed to the transformer is unchanged (still the `lines.entries()` counter), so the G21/G32/G52/G58 transform-index/nesting tests are untouched. The pre-existing overflow test `"transform returning wide char in clipped box is clipped"` asserted the OLD (commit-2c99431) order — that the transform's wide-`中` output is re-clipped. Ink renders `"abc中"` there (transform output overflows, never re-clipped), confirmed against the reference. Per conflict policy the test was renamed to `"… overflows (Ink clip-then-transform)"` and now asserts `"abc中"`. No-clip transform paths and plain (no-transform) overflow clipping are unchanged.
|
||||
|
||||
## Confirmed gaps
|
||||
|
||||
@@ -105,6 +108,10 @@ Non-obvious calls made while fixing gaps, recorded for review in the final repor
|
||||
| G60 | box-layout-border | String-typed dimension/position values (bare numeric string e.g. width="50") treated as POINT not PERCENT; Ink applies all string dims as percent — sweep-8 LOW | P3 | todo | — | — |
|
||||
| G61 | stdout-stderr-stdin-size-cursor | Bracketed-paste disable write during stdin dispose() lacks Ink's destroyed/writableEnded (canWriteToStdout) guard — sweep-8 LOW | P3 | todo | — | — |
|
||||
| G62 | render-lifecycle-reconciler | resolveExit()/teardown() writable-stream checks omit the writableLength fallback + stdout.writable flag that Ink's getWritableStreamState uses — sweep-8 LOW | P3 | todo | — | — |
|
||||
| G63 | text-wrap-transform | Transform applied to the FULL line THEN horizontally clipped; Ink clips THEN transforms (output.ts), so a width-sensitive transform (gradient / OSC-8 hyperlink) inside an overflowX:hidden box gets the wrong char span (MEDIUM, sweep-9) | P1 | pr-open | `fix/parity-clip-then-transform` | #63 |
|
||||
| G64 | static-newline-spacer | `<Static>` isolated paint forces terminal width; Ink content-sizes the static box (position:absolute, auto width) so flex-fill children (Spacer/flexGrow/justify/align/percent) collapse to content — refines G44's over-correction (MEDIUM, sweep-9) | P1 | todo | — | — |
|
||||
| G65 | app-exit-instances-animation-sr | SR unchanged-frame skip compares WRAPPED output, not Ink's UNWRAPPED linearization — diverges on resize with identical content — sweep-9 LOW | P3 | todo | — | — |
|
||||
| G66 | app-exit-instances-animation-sr | app.waitUntilRenderFlush() lacks Ink's unmount/unmounting short-circuit to awaitExit() — sweep-9 LOW | P3 | todo | — | — |
|
||||
|
||||
## Gap details
|
||||
|
||||
|
||||
@@ -26,7 +26,7 @@ describe("grapheme-aware clipping (issue #21)", () => {
|
||||
expect(frame).not.toContain("👨");
|
||||
});
|
||||
|
||||
test("left-edge wide grapheme straddle positions following text correctly", async () => {
|
||||
test("left-edge wide grapheme straddle starts following text at clip origin (Ink parity)", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box width={4} height={1} overflow="hidden">
|
||||
@@ -38,7 +38,13 @@ describe("grapheme-aware clipping (issue #21)", () => {
|
||||
{ columns: 100 },
|
||||
);
|
||||
const frame = stripAnsi(lastFrame({ trimLines: true })!);
|
||||
expect(frame.startsWith(" x")).toBe(true);
|
||||
// "中" (width 2) straddles the left clip edge → dropped whole. Ink output.ts:210-212
|
||||
// sets the write origin to the clipped left edge, so the kept "x" starts AT that
|
||||
// origin with NO leading space. Verified against the built Ink reference
|
||||
// (/tmp/ink-40b3a75 renderToString of this exact tree → "x"). Previously this
|
||||
// locked vue-tui's leading-space padding (" x"); rewritten to Ink per the G63
|
||||
// decisions-log entry in .agents/docs/parity-ledger.md.
|
||||
expect(frame).toBe("x");
|
||||
});
|
||||
|
||||
// absolute-non-edge class (R2-000045): an absolutely-positioned ZWJ emoji is
|
||||
|
||||
@@ -657,7 +657,14 @@ describe("absolute overlay wide glyph clipping", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("transform returning wide char in clipped box is clipped", async () => {
|
||||
// G63: Ink clips THEN transforms, and never re-clips the transformer's output.
|
||||
// The source char "x" (width 1) sits at left=3, fully inside the width-4 clip, so
|
||||
// it survives the horizontal clip; the transform then replaces it with the wide
|
||||
// "中" (width 2), which is emitted UNCLIPPED past the boundary. Ink reference
|
||||
// (v7.0.4) renders exactly "abc中" here. Previously vue-tui clipped AFTER the
|
||||
// transform (commit 2c99431), dropping the "中" — that was the non-Ink order G63
|
||||
// reverses, so this test now asserts Ink's clip-then-transform output.
|
||||
test("transform returning wide char in clipped box overflows (Ink clip-then-transform)", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box width={4} height={1} overflow="hidden">
|
||||
@@ -672,12 +679,12 @@ describe("absolute overlay wide glyph clipping", () => {
|
||||
{ columns: 100 },
|
||||
);
|
||||
const frame = lastFrame({ trimLines: true })!;
|
||||
expect(stripAnsi(frame)).toBe("abc");
|
||||
expect(stripAnsi(frame)).toBe("abc中");
|
||||
});
|
||||
});
|
||||
|
||||
describe("left-edge wide glyph clipping", () => {
|
||||
test("text after clipped left-edge wide char is correctly positioned", async () => {
|
||||
test("text after clipped left-edge wide char starts at clip origin (Ink parity)", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box width={4} height={1} overflow="hidden">
|
||||
@@ -690,9 +697,53 @@ describe("left-edge wide glyph clipping", () => {
|
||||
);
|
||||
const frame = lastFrame({ trimLines: true })!;
|
||||
const stripped = stripAnsi(frame);
|
||||
// "中" (width 2) starts at col -1, straddling the left edge → clipped entirely
|
||||
// "x" should start at col 1 (not col 0)
|
||||
expect(stripped.startsWith(" x")).toBe(true);
|
||||
// "中" (width 2) starts at col -1, straddling the left edge → clipped whole.
|
||||
// Ink output.ts:210-212 sets the write origin to the clipped left edge, so the
|
||||
// kept "x" starts AT that origin with NO leading offset. Verified against the
|
||||
// built Ink reference (/tmp/ink-40b3a75 renderToString of this exact tree → "x").
|
||||
// (Was vue-tui-specific " x" leading-space padding; rewritten to Ink. See the
|
||||
// G63 decisions-log entry in .agents/docs/parity-ledger.md.)
|
||||
expect(stripped).toBe("x");
|
||||
});
|
||||
|
||||
// G63 MUST-FIX: after a LEFT horizontal clip, the per-line write origin is set
|
||||
// to the clipped left edge (Ink output.ts:210-212 `x = clip.x1`), THEN the
|
||||
// transform runs and writes from that origin. So a wide glyph straddling the
|
||||
// left edge is clipped whole and the kept content starts AT the clip origin —
|
||||
// no leading offset. Verified against the built Ink reference (/tmp/ink-40b3a75,
|
||||
// renderToString of the SAME component): transform=>"z" → "z", [l] → "[x]".
|
||||
test("transform after clipped left-edge wide char starts at clip origin (Ink parity)", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box width={4} height={1} overflow="hidden">
|
||||
<Box marginLeft={-1} flexShrink={0}>
|
||||
<Transform transform={() => "z"}>
|
||||
<Text>中x</Text>
|
||||
</Transform>
|
||||
</Box>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 100 },
|
||||
);
|
||||
const frame = lastFrame({ trimLines: true })!;
|
||||
expect(stripAnsi(frame)).toBe("z");
|
||||
});
|
||||
|
||||
test("width-sensitive transform after clipped left-edge wide char (Ink parity)", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box width={4} height={1} overflow="hidden">
|
||||
<Box marginLeft={-1} flexShrink={0}>
|
||||
<Transform transform={(l: string) => `[${l}]`}>
|
||||
<Text>中x</Text>
|
||||
</Transform>
|
||||
</Box>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 100 },
|
||||
);
|
||||
const frame = lastFrame({ trimLines: true })!;
|
||||
expect(stripAnsi(frame)).toBe("[x]");
|
||||
});
|
||||
|
||||
test("wide chars clipped on both edges simultaneously", async () => {
|
||||
@@ -710,3 +761,54 @@ describe("left-edge wide glyph clipping", () => {
|
||||
expect(lineWidth(frame)).toBeLessThanOrEqual(2);
|
||||
});
|
||||
});
|
||||
|
||||
// G63: Ink clips a line horizontally FIRST, THEN applies the line transformer to
|
||||
// the already-clipped span (output.ts: the `clipHorizontally` sliceAnsi map runs
|
||||
// before the `lines.entries()` loop that calls `transformer(line, index)`). So a
|
||||
// width-sensitive transform inside an overflowX:"hidden" box must receive the
|
||||
// CLIPPED substring, not the full line. The buggy order (transform-then-clip)
|
||||
// feeds the transformer the full line and slices its output, which corrupts
|
||||
// gradients (wrong char count) and OSC-8 hyperlinks (closing sequence sliced off).
|
||||
describe("G63 clip-then-transform order (Ink parity)", () => {
|
||||
test("transform inside overflowX hidden receives the clipped span", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box width={5} overflowX="hidden">
|
||||
<Box width={16} flexShrink={0}>
|
||||
<Transform transform={(l: string) => `[${l}]`}>
|
||||
<Text>hello world</Text>
|
||||
</Transform>
|
||||
</Box>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 20 },
|
||||
);
|
||||
// Ink reference (v7.0.4, columns=20): the line "hello world" is clipped to the
|
||||
// 5-col content area → "hello", THEN the transform brackets it → "[hello]"
|
||||
// (verified by running renderToString against the pinned Ink build).
|
||||
// The buggy transform-then-clip order brackets the full line "[hello world]"
|
||||
// then slices to 5 cols → "[hell".
|
||||
expect(lastFrame({ trimLines: true })).toBe("[hello]");
|
||||
});
|
||||
|
||||
test("transform inside overflowX hidden — left clip feeds the right span", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box width={5} overflowX="hidden">
|
||||
<Box width={16} marginLeft={-6} flexShrink={0}>
|
||||
<Transform transform={(l: string) => `[${l}]`}>
|
||||
<Text>hello world</Text>
|
||||
</Transform>
|
||||
</Box>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 20 },
|
||||
);
|
||||
// The inner box is shifted left by 6 cols, so the visible window over
|
||||
// "hello world" is columns 6..10 → "world". Ink clips to "world" first, then
|
||||
// brackets → "[world]". The buggy order would bracket "[hello world]" then
|
||||
// slice columns 6..10 of THAT → "world" (no brackets), so the brackets reveal
|
||||
// which span the transform actually saw.
|
||||
expect(lastFrame({ trimLines: true })).toBe("[world]");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -180,12 +180,14 @@ class Output {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Apply transforms BEFORE horizontal clipping
|
||||
for (const transformer of transformers) {
|
||||
line = transformer(line, index);
|
||||
}
|
||||
|
||||
// Horizontal clip (per-line, after transform)
|
||||
// Horizontal clip BEFORE applying transforms, matching Ink's
|
||||
// output.ts: the `clipHorizontally` sliceAnsi map runs first, THEN the
|
||||
// `lines.entries()` loop calls `transformer(line, index)` on the
|
||||
// already-clipped span. Width-sensitive transforms (gradients spread
|
||||
// across the visible columns, OSC-8 hyperlinks whose closing sequence
|
||||
// must not be sliced off) depend on receiving exactly the clipped
|
||||
// substring — applying them to the full line and slicing the result
|
||||
// corrupts the gradient stops and can truncate the link terminator.
|
||||
let lineX = x;
|
||||
if (clipH) {
|
||||
const lineWidth = this.caches.getStringWidth(line);
|
||||
@@ -196,20 +198,25 @@ class Output {
|
||||
}
|
||||
const from = lineX < clipH.x1 ? clipH.x1 - lineX : 0;
|
||||
const to = lineX + lineWidth > clipH.x2 ? clipH.x2 - lineX : lineWidth;
|
||||
if (from > 0) {
|
||||
// Advance lineX by however many columns slice-ansi actually drops
|
||||
// from the left. slice-ansi@9 is grapheme-aware: a wide grapheme
|
||||
// straddling the clip edge is dropped whole, so the retained
|
||||
// content starts at `lineX + droppedWidth` (which may exceed
|
||||
// `from`). Measuring the kept-prefix width would under-count here
|
||||
// and misplace the following text.
|
||||
const droppedWidth = lineWidth - this.caches.getStringWidth(sliceAnsi(line, from));
|
||||
lineX = lineX + droppedWidth;
|
||||
}
|
||||
// After a LEFT clip the write origin is the clipped left edge — matching
|
||||
// Ink output.ts:210-212 `if (x < clip.x1) x = clip.x1`. slice-ansi@9 is
|
||||
// grapheme-aware, so a wide glyph straddling the clip edge is dropped
|
||||
// whole and the kept content begins at this origin with NO leading
|
||||
// offset. (We deliberately do NOT advance the origin by the dropped
|
||||
// glyph's extra column — that produced a vue-tui-specific leading space
|
||||
// that Ink never emits; see G63 decisions-log in parity-ledger.md.)
|
||||
if (lineX < clipH.x1) lineX = clipH.x1;
|
||||
const maxWidth = clipH.x2 - lineX;
|
||||
line = safeSliceEnd(sliceAnsi(line, from, to), maxWidth);
|
||||
}
|
||||
|
||||
// Apply transforms to the (now horizontally clipped) line. `index` is the
|
||||
// post-vertical-clip line index — unchanged from the loop counter, matching
|
||||
// Ink's `transformer(line, index)` where index is the entry index.
|
||||
for (const transformer of transformers) {
|
||||
line = transformer(line, index);
|
||||
}
|
||||
|
||||
const characters = this.caches.getStyledChars(line);
|
||||
let offsetX = lineX;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user