fix(ink-compat): optimize translation and layout hot paths#227
Merged
RtlZeroMemory merged 4 commits intomainfrom Feb 27, 2026
Merged
fix(ink-compat): optimize translation and layout hot paths#227RtlZeroMemory merged 4 commits intomainfrom
RtlZeroMemory merged 4 commits intomainfrom
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughCentralizes generation-aware layout state, augments reconciler node/container marker APIs and mutation helpers, exposes translation metadata and test hooks, refactors render pipeline with percent-resolution, style/SGR caching and clipping optimizations, and adds extensive tests and performance benchmarks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Translation
participant PercentResolver
participant Renderer
participant LayoutState
Client->>Translation: translateDynamicTreeWithMetadata(root)
activate Translation
Translation-->>Client: { vnode, meta }
deactivate Translation
Client->>PercentResolver: resolvePercentMarkers(vnode)
activate PercentResolver
alt parent layouts present
PercentResolver-->>Client: vnode (percent-resolved)
else missing parent layouts
PercentResolver-->>Client: vnode (marks for second pass)
end
deactivate PercentResolver
Client->>Renderer: renderFrame(vnode)
activate Renderer
Renderer->>LayoutState: readCurrentLayout(node)
LayoutState-->>Renderer: layout | undefined
Renderer->>Renderer: normalize styles, compute clip, draw cells
Renderer->>LayoutState: writeCurrentLayout(node, newLayout, gen)
LayoutState-->>Renderer: confirmed gen
Renderer-->>Client: { ansi, grid, shape }
deactivate Renderer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f73007c87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/ink-compat/src/runtime/render.ts (1)
2707-2831: Please attach renderer verification evidence for this runtime/layout refactor.Given the breadth of render/layout hot-path changes, please include full-suite and frame-audit evidence with this PR.
#!/bin/bash set -euo pipefail node scripts/run-tests.mjs REZI_FRAME_AUDIT=1 node scripts/run-tests.mjs node scripts/frame-audit-report.mjsBased on learnings: For rendering/layout/theme regressions, validate with live PTY and collect
REZI_FRAME_AUDITevidence; and runnode scripts/run-tests.mjsafter renderer/layout changes.Also applies to: 3368-3368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ink-compat/src/runtime/render.ts` around lines 2707 - 2831, The reviewer asks for renderer verification artifacts for the render/layout refactor: run the full test suite and the frame-audit run, capture and attach the outputs so we can verify no regressions in renderFrame hot-path; specifically run node scripts/run-tests.mjs, then REZI_FRAME_AUDIT=1 node scripts/run-tests.mjs and node scripts/frame-audit-report.mjs, collect the resulting logs/reports (console output, frame-audit JSON/HTML) and attach them to the PR, and mention evidence for the renderFrame changes (include links to artifacts and note any failures tied to renderer.render, coerceRootViewportHeight, resolvePercentMarkers or assignHostLayouts invocations).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ink-compat/src/__tests__/integration/basic.test.ts`:
- Line 616: Reformat the runtimeRender callsite to match project formatting:
adjust the call to runtimeRender(React.createElement(App, { parentWidth: 20 }),
{ stdin, stdout, stderr }) so it conforms to the repo's Prettier/formatter rules
(e.g., proper spacing and argument line breaks); locate the invocation using the
runtimeRender call and the React.createElement(App, { parentWidth: 20 })
expression and apply the same formatting style used elsewhere in tests to clear
the CI formatter drift.
- Around line 1265-1291: The test "nested non-overlapping clips do not leak
text" asserts immediately after mounting so it can pass before any frame is
emitted; update the test to wait for output to flush before asserting (e.g.,
await a data event on stdout or await a short microtask) by using the existing
stdout stream: use something like await new Promise(resolve =>
stdout.once("data", () => resolve())); then compute latest =
stripTerminalEscapes(latestFrameFromWrites(writes)) and perform the assert; keep
references to the test name, writes, stdout, latestFrameFromWrites, and
stripTerminalEscapes when locating the code to change.
In `@packages/ink-compat/src/__tests__/perf/bottleneck-profile.test.ts`:
- Around line 115-133: textStylesEqual_FIXED currently ensures both objects have
the same set of keys but then only compares a fixed subset of known fields, so
extra unknown properties with differing values will be ignored; update
textStylesEqual_FIXED to, after confirming keys lengths match, iterate over
every key in keysA and verify values are equal for each key (use rgbEqual when
the key is "fg" or "bg", otherwise use strict equality or a deep-equality helper
for nested objects) instead of only checking the hardcoded properties, and keep
the existing final checks for known boolean fields if desired (or remove them in
favor of the generic key-based comparisons) so all properties declared on the
TextStyleMap index signature are validated.
In `@packages/ink-compat/src/runtime/render.ts`:
- Around line 4-10: Replace the non-existent Rgb import with Rgb24 (import type
{ Rgb24 } from "@rezi-ui/core"), update the CellStyle interface to use Rgb24 for
fg/bg instead of Rgb, and change the color normalization in the renderer (the
logic that currently reads TextStyle.fg/.bg as objects with .r/.g/.b) to treat
fg/bg as Rgb24 numeric values: detect the sentinel 0 as “unset”, and when
present convert a non-zero Rgb24 number to RGB channels via bit masks/shifts
(extract r = (val >> 16) & 0xff, g = (val >> 8) & 0xff, b = val & 0xff) so
downstream code receives the expected channel structure while preserving the 0
unset semantics.
In `@packages/ink-compat/src/translation/propsToVNode.ts`:
- Around line 1230-1251: The OSC-handling branch in propsToVNode uses output ===
null to decide whether to include the OSC slice, which can return the original
unsanitized input with ESC ] ... BEL/ST sequences; update the logic around
findOscEndIndex/oscEnd so that whenever an OSC is found you (a) ensure output is
initialized if needed and push only the preceding non-OSC slice (use
input.slice(0, index) or input.slice(runStart, index)) but do NOT push the OSC
slice itself, (b) advance index = oscEnd and runStart = index and continue; also
ensure any later return path does not return the original input when output is
null but instead returns the sanitized output (or an initialized output array)
so OSC sequences are never leaked; use the existing symbols findOscEndIndex,
oscEnd, index, runStart, input and output to locate and change the code
accordingly.
---
Nitpick comments:
In `@packages/ink-compat/src/runtime/render.ts`:
- Around line 2707-2831: The reviewer asks for renderer verification artifacts
for the render/layout refactor: run the full test suite and the frame-audit run,
capture and attach the outputs so we can verify no regressions in renderFrame
hot-path; specifically run node scripts/run-tests.mjs, then REZI_FRAME_AUDIT=1
node scripts/run-tests.mjs and node scripts/frame-audit-report.mjs, collect the
resulting logs/reports (console output, frame-audit JSON/HTML) and attach them
to the PR, and mention evidence for the renderFrame changes (include links to
artifacts and note any failures tied to renderer.render,
coerceRootViewportHeight, resolvePercentMarkers or assignHostLayouts
invocations).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/ink-compat/src/__tests__/integration/basic.test.tspackages/ink-compat/src/__tests__/perf/bottleneck-profile.test.tspackages/ink-compat/src/__tests__/reconciler/types.test.tspackages/ink-compat/src/__tests__/runtime/newApis.test.tspackages/ink-compat/src/__tests__/translation/propsToVNode.test.tspackages/ink-compat/src/reconciler/hostConfig.tspackages/ink-compat/src/reconciler/types.tspackages/ink-compat/src/runtime/ResizeObserver.tspackages/ink-compat/src/runtime/bridge.tspackages/ink-compat/src/runtime/domHelpers.tspackages/ink-compat/src/runtime/getBoundingBox.tspackages/ink-compat/src/runtime/layoutState.tspackages/ink-compat/src/runtime/measureElement.tspackages/ink-compat/src/runtime/render.tspackages/ink-compat/src/translation/propsToVNode.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/ink-compat/src/runtime/render.ts (1)
1225-1236:⚠️ Potential issue | 🔴 CriticalPreserve
Rgb24sentinel semantics for0in normalization.At Line 1225 and Line 1233,
fg/bg === 0currently pass through and are treated as explicit black, which can emit color SGR later. In ink-compat this value is sentinel “unset/default” and must be suppressed.Proposed fix
- if (isRgb24(style.fg)) { + if (isRgb24(style.fg) && style.fg !== 0) { const fg = rgb(clampByte(rgbR(style.fg)), clampByte(rgbG(style.fg)), clampByte(rgbB(style.fg))); // Rezi carries DEFAULT_BASE_STYLE through every text draw op. Ink treats // terminal defaults as implicit, so suppress those default color channels. if (fg !== CORE_DEFAULT_FG) { normalized.fg = fg; } } - if (isRgb24(style.bg)) { + if (isRgb24(style.bg) && style.bg !== 0) { const bg = rgb(clampByte(rgbR(style.bg)), clampByte(rgbG(style.bg)), clampByte(rgbB(style.bg))); if (bg !== CORE_DEFAULT_BG) { normalized.bg = bg; } }Based on learnings: In packages/ink-compat,
Rgb24value0is an intentional sentinel meaning “unset/terminal default,” and color SGR should be skipped for0.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ink-compat/src/runtime/render.ts` around lines 1225 - 1236, The normalization currently treats Rgb24 value 0 as explicit black; update the logic in render normalization so that when style.fg or style.bg is an Rgb24 whose raw value equals 0 it is treated as the sentinel "unset/default" and not emitted: inside the Rgb24 handling (the block that uses isRgb24(style.fg) / isRgb24(style.bg), rgb(...), clampByte, rgbR/rgbG/rgbB) add a check for the raw Rgb24 value === 0 and skip assigning normalized.fg / normalized.bg in that case (same suppression behavior used for CORE_DEFAULT_FG / CORE_DEFAULT_BG), ensuring color SGRs are not emitted for the sentinel 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/ink-compat/src/runtime/render.ts`:
- Around line 1225-1236: The normalization currently treats Rgb24 value 0 as
explicit black; update the logic in render normalization so that when style.fg
or style.bg is an Rgb24 whose raw value equals 0 it is treated as the sentinel
"unset/default" and not emitted: inside the Rgb24 handling (the block that uses
isRgb24(style.fg) / isRgb24(style.bg), rgb(...), clampByte, rgbR/rgbG/rgbB) add
a check for the raw Rgb24 value === 0 and skip assigning normalized.fg /
normalized.bg in that case (same suppression behavior used for CORE_DEFAULT_FG /
CORE_DEFAULT_BG), ensuring color SGRs are not emitted for the sentinel 0.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/ink-compat/src/__tests__/integration/basic.test.tspackages/ink-compat/src/__tests__/perf/bottleneck-profile.test.tspackages/ink-compat/src/__tests__/translation/propsToVNode.test.tspackages/ink-compat/src/runtime/render.tspackages/ink-compat/src/translation/propsToVNode.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ink-compat/src/tests/translation/propsToVNode.test.ts
RtlZeroMemory
added a commit
that referenced
this pull request
Feb 27, 2026
RtlZeroMemory
added a commit
that referenced
this pull request
Feb 28, 2026
#230) * Fix starship rendering regressions and add PTY debug runbook * Fix starship template theme to use packed rgb colors * Address PR feedback and fix lint/native diff telemetry audit * Fix native vendor integrity check for uninitialized submodules * Fix CI color-style tests and native payload short-read audit * fix(ink-compat): optimize translation and layout hot paths * fix(ink-compat): handle packed rgb and stabilize review regressions * chore(lint): resolve biome violations in ci files * fix(core): satisfy strict index-signature access in hashTextProps * docs(changelog): add missing merged PR entries through #227 * feat(bench): add ink-compat benchmark harness * feat(ink-compat): add bench phase hook * fix(ink-compat): match Ink soft-wrap whitespace * bench: improve determinism and resize handling * feat(bench): add validity doc and reporting * perf(ink-compat): coalesce renders and reduce churn * chore(bench): add active cpuprofile hotspot evidence * perf(ink-compat): reduce runtime renderer overhead * perf(ink-compat): reduce dashboard-grid tail latency * refactor(core): reduce hot-path allocation churn * chore(ink-compat): safety checkpoint before perf refactors * fix(layout): guard forced dimension cache keys * perf(ink-compat): apply claude optimization batch * chore(bench): safety checkpoint before long-run harness refactor * fix(ink-compat): harden ansi transform rendering and bench wiring * docs(ink-compat): overhaul migration and discovery docs * fix(pr): address review feedback on harness and ink-compat * docs(ink-compat): align debug links and parity verification note * fix(lint): resolve ci biome diagnostics
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rgb24packed colors0) color values when translating Ink propsScope
packages/ink-compat/**files are changedValidation
npx biome check packages/ink-compat/src/runtime/render.ts packages/ink-compat/src/translation/propsToVNode.ts packages/ink-compat/src/__tests__/integration/basic.test.ts packages/ink-compat/src/__tests__/perf/bottleneck-profile.test.ts packages/ink-compat/src/__tests__/translation/propsToVNode.test.tsnpm run buildnpx tsx --test packages/ink-compat/src/__tests__/integration/basic.test.ts packages/ink-compat/src/__tests__/perf/bottleneck-profile.test.ts packages/ink-compat/src/__tests__/translation/propsToVNode.test.tsnode scripts/run-tests.mjsSummary by CodeRabbit
New Features
Tests
Performance