Skip to content

EPIC 6: packed style pipeline + Zireael vendor bump#217

Merged
RtlZeroMemory merged 10 commits intomainfrom
perf/ink-compat-optimized-renderer
Feb 26, 2026
Merged

EPIC 6: packed style pipeline + Zireael vendor bump#217
RtlZeroMemory merged 10 commits intomainfrom
perf/ink-compat-optimized-renderer

Conversation

@RtlZeroMemory
Copy link
Owner

@RtlZeroMemory RtlZeroMemory commented Feb 26, 2026

Summary

EPIC 6 is implemented in this branch with two outcomes:

  • Vendor Zireael is bumped to the latest origin/main from /home/k3nig/Zireael.
  • Style/color plumbing is migrated to packed numeric representation end-to-end in runtime hot paths.

A) Zireael Vendor Bump

  • Vendored trees updated from upstream into:
    • packages/native/vendor/zireael/include
    • packages/native/vendor/zireael/src
  • packages/native/vendor/VENDOR_COMMIT.txt now points to:
    • 52f3fd81a437e5a351dd026a75b2f9b01ca44a76
  • Native wrapper/build updated and validated against current vendored ABI.
  • Note: current upstream ABI still contains drawlist-version fields (requested_drawlist_version, negotiated_drawlist_version), so these were retained where mapped.

B) Packed Style Redesign

  • Rgb object style representation removed from exported API usage in the render pipeline.
  • Packed Rgb24 values are used through style APIs and renderer paths.
  • rgb(...) now returns packed numbers and all touched call sites were updated accordingly.
  • Style merge/resolution now works on primitive packed values and an attrs bitmask.
  • Drawlist path keeps wire encoding identical while removing versioned builder branches.
  • Legacy versioned builder source files removed/renamed to a single current builder/types flow.

Additional Cleanup

  • Remaining non-test widget color constants converted from {r,g,b} to packed values.
  • Ink-compat runtime and translation paths migrated off Rgb object channels.

Tests / Validation

Pass:

  • npm run typecheck
  • npm -w @rezi-ui/native run build:native
  • npm -w @rezi-ui/native run test:native:smoke
  • npm -w @rezi-ui/core run build
  • node --test packages/core/dist/drawlist/__tests__/builder.style-encoding.test.js
  • npm -w @rezi-ui/ink-compat run build
  • npx tsc -p packages/bench/tsconfig.json --pretty false

Known in this branch:

  • Full npm -w @rezi-ui/core run test currently reports broader existing failures outside this epic-focused validation set.

Perf Proof

Added counters and profiler coverage for packed-style merge behavior:

  • counters: style_merges_performed, style_objects_created, packRgb_calls
  • profiler script: packages/bench/src/profile-packed-style.ts

Observed run (REZI_PERF=1 REZI_PERF_DETAIL=1 npx tsx src/profile-packed-style.ts):

  • style_merges_performed: 192000
  • style_objects_created: 44800
  • packRgb_calls: 0
  • style object reuse: 76.67%
  • render avg: 6115µs
  • drawlist_build avg: 11µs

These numbers demonstrate removal of pack calls from hot style merge/render paths and reduced style object churn.

Summary by CodeRabbit

  • New Features
    • Layout & render profiling tools and a new Ink-compatible renderer for richer tracing.
  • Performance Improvements
    • New perf counters, faster style/color handling, and selective frame damage updates for quieter, safer redraws.
  • Bug Fixes
    • Improved percent-dimension handling and stronger commit/render diagnostics.
  • Refactor
    • Unified drawlist/builder surface and consolidated color model for consistent theming and color APIs.

RtlZeroMemory and others added 6 commits February 26, 2026 11:27
- Add concurrency group to ci.yml and codeql.yml to cancel in-progress
  runs on re-push (biggest win for runner contention)
- Extract lint/typecheck/codegen/portability/unicode into a dedicated
  `checks` job that gates the matrix — lint failures caught in ~2 min
  instead of after the full 15-min pipeline
- Dynamic matrix: 5 runners on PRs (Linux × Node 18/20/22, macOS × 22,
  Windows × 22), full 3×3 on push to main
- Remove redundant lint/typecheck/codegen/biome-install steps from each
  matrix cell and the bun job
- Remove duplicate docs job (already handled by docs.yml)

Net effect on PRs: 13 jobs → 9, ~44% fewer runners, fast-fail on
static checks, stale runs cancelled automatically.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 26, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a559378 and 8f0978b.

📒 Files selected for processing (22)
  • docs/backend/native.md
  • packages/bench/src/profile-packed-style.ts
  • packages/core/src/app/widgetRenderer.ts
  • packages/core/src/drawlist/builder.ts
  • packages/core/src/theme/defaultTheme.ts
  • packages/core/src/theme/extract.ts
  • packages/core/src/theme/heatmapPalettes.ts
  • packages/core/src/widgets/__tests__/colorScales.test.ts
  • packages/core/src/widgets/__tests__/graphicsWidgets.test.ts
  • packages/core/src/widgets/__tests__/overlays.typecheck.ts
  • packages/core/src/widgets/canvas.ts
  • packages/core/src/widgets/diffViewer.ts
  • packages/core/src/widgets/heatmap.ts
  • packages/core/src/widgets/logsConsole.ts
  • packages/core/src/widgets/style.ts
  • packages/core/src/widgets/toast.ts
  • packages/ink-compat/src/runtime/createInkRenderer.ts
  • packages/ink-compat/src/translation/colorMap.ts
  • packages/ink-compat/src/translation/propsToVNode.ts
  • packages/native/vendor/zireael/src/core/zr_engine_present.inc
  • packages/native/vendor/zireael/src/core/zr_framebuffer.c
  • scripts/guardrails.sh

📝 Walkthrough

Walkthrough

Consolidates drawlist builder variants into a single DrawlistBuilder/createDrawlistBuilder API, migrates color model from object RGB to packed 24-bit Rgb24 with rgb(...) helpers, removes multi-version frame-writer branching in renderers/backends, adds resource-backed drawlist support (strings/blobs) and native resource APIs, and updates many tests and tooling accordingly.

Changes

Cohort / File(s) Summary
Drawlist core & API
packages/core/src/drawlist/builder.ts, packages/core/src/drawlist/builderBase.ts, packages/core/src/drawlist/index.ts, packages/core/src/drawlist/types.ts
Introduce unified createDrawlistBuilder() and DrawlistBuilder type (adds buildInto, setCursor, hideCursor, setLink, drawCanvas, drawImage); remove legacy V1/V2/V3 exports and legacy EncodedStyleV1/DrawlistBuilderV* artifacts.
Builder consumers & render pipeline
packages/core/src/app/..., packages/core/src/renderer/..., packages/core/src/renderer/renderToDrawlist/**, packages/core/src/widgets/**
Update callers to use unified DrawlistBuilder; remove version guards (isV2/isV3), simplify cursor/link handling and frame-writer branching, and adapt render pipeline to always use builder.build()/buildInto().
Native vendor & engine resource model
packages/native/vendor/zireael/**, packages/native/vendor/VENDOR_COMMIT.txt
Pin drawlist protocol to v1, add resource subsystem (strings/blobs) and DEF/FREE resource opcodes, rename index→id fields, add resource-store APIs and preflight/execute integration, and introduce damage-rect copy APIs.
Color model & theme
packages/core/src/widgets/style.ts, packages/core/src/widgets/styleUtils.ts, packages/core/src/theme/**, packages/core/src/ui/**, many packages/core/src/widgets/**
Replace object RGB with packed 24-bit Rgb24; add rgb(), rgbR/G/B, rgbBlend; migrate theme tokens, defaults, constants, and many helpers to Rgb24 and numeric color literals.
Tests & test doubles
many packages/core/src/**/__tests__/*, packages/core/src/testing/renderer.ts, packages/core/src/drawlist/__tests__/*
Update tests to use createDrawlistBuilder() and numeric Rgb24 values; expand test builder doubles to implement new DrawlistBuilder surface (setCursor, hideCursor, setLink, drawCanvas, drawImage, buildInto).
Native API additions
packages/native/vendor/zireael/include/*.h, packages/native/vendor/zireael/src/core/*
Add zr_dl_resources_t/resource-store types and management functions, zr_dl_preflight_resources, and related public functions; update zr_drawlist header/structs to resource-oriented fields.
Backend / Node runtime
packages/node/src/backend/*.ts, packages/node/src/index.ts, packages/node/src/__tests__/*
Remove configurable drawlistVersion/frame-begin plumbing; hard-code backend protocol marker to v1; remove BackendBeginFrame/BackendFrameWriter exports and related frame-writer APIs; adjust tests.
Profiling & perf
packages/core/src/layout/engine/layoutEngine.ts, packages/core/src/perf/perf.ts, packages/bench/src/profile-packed-style.ts, packages/ink-compat/src/_bench_layout_profile.ts
Add layout instrumentation counters, PerfAggregator counters and perfCount API, and new bench/profile scripts leveraging counters.
Pipeline entrypoint & exports
packages/core/src/pipeline.ts, packages/core/src/index.ts, packages/jsx/src/index.ts
Add pipeline re-exports for commit/layout/render primitives; re-export ZR_DRAWLIST_VERSION; switch public style exports to Rgb24.
Examples & tooling
examples/gallery/src/scenes.ts, scripts/guardrails.sh
Migrate example theme overrides to rgb() calls and adjust guardrails to check new builder import locations.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App
  participant WidgetRenderer
  participant DrawlistBuilder
  participant Backend
  participant NativeEngine

  App->>WidgetRenderer: commit → render
  WidgetRenderer->>DrawlistBuilder: emit draw commands (drawText, addBlob, drawCanvas, setLink, setCursor)
  DrawlistBuilder->>DrawlistBuilder: manage resources (DEF_STRING/DEF_BLOB), build()/buildInto()
  DrawlistBuilder->>Backend: requestFrame(built.bytes) + resource defs
  Backend->>NativeEngine: preflight resources → execute drawlist
  NativeEngine-->>Backend: execution result (ok / error)
  Backend-->>App: frame ack / error
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Poem

🐇 I hopped through builders, one by one,
I packed the colors, now they're done.
Strings and blobs I safely store,
One drawlist hums from core to core.
A rabbit's cheer — compact and bright, 24-bit snug all night! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'EPIC 6: packed style pipeline + Zireael vendor bump' accurately describes the main changes: a major refactor consolidating packed style representation and a vendor bump for the Zireael native library.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch perf/ink-compat-optimized-renderer

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a19b132e14

ℹ️ 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".

…ized-renderer

# Conflicts:
#	packages/core/src/abi.ts
#	packages/core/src/app/__tests__/partialDrawlistEmission.test.ts
#	packages/core/src/app/createApp.ts
#	packages/core/src/app/rawRenderer.ts
#	packages/core/src/app/widgetRenderer.ts
#	packages/core/src/drawlist/__tests__/builder.build-into.test.ts
#	packages/core/src/drawlist/__tests__/builder.cursor.test.ts
#	packages/core/src/drawlist/__tests__/builder.round-trip.test.ts
#	packages/core/src/drawlist/__tests__/writers.gen.test.ts
#	packages/core/src/drawlist/builder.ts
#	packages/core/src/drawlist/builder_v1.ts
#	packages/core/src/drawlist/builder_v2.ts
#	packages/core/src/drawlist/index.ts
#	packages/core/src/drawlist/types.ts
#	packages/core/src/index.ts
#	packages/core/src/renderer/renderToDrawlist/widgets/renderCanvasWidgets.ts
#	packages/node/src/__tests__/config_guards.test.ts
#	packages/node/src/backend/nodeBackend.ts
#	packages/node/src/backend/nodeBackendInline.ts
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 20

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (7)
packages/core/src/widgets/field.ts (1)

11-24: ⚠️ Potential issue | 🟠 Major

Replace hardcoded error color with a semantic theme token.

At Line 24, rgb(255, 0, 0) is still a hardcoded RGB value (just in packed form), which breaks the design-system/theming rule. Please use the existing semantic error token from theme/design tokens for FIELD_ERROR_STYLE.fg and remove the new rgb import at Line 11 if no longer needed.

As per coding guidelines: “Never hardcode RGB color values; let the theme handle colors via tokens. Use semantic color tokens/design system props.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/widgets/field.ts` around lines 11 - 24, The
FIELD_ERROR_STYLE constant uses a hardcoded rgb(...) value for fg; replace that
value with the semantic error color token from the design/theme (e.g., the
theme's error/semantic token) by assigning the token to FIELD_ERROR_STYLE.fg,
and remove the now-unused rgb import at the top (import { rgb } from
"./style.js") if nothing else in this file uses it; update the Object.freeze
definition for FIELD_ERROR_STYLE accordingly so the file uses the theme token
instead of rgb(255,0,0).
packages/core/src/widgets/__tests__/basicWidgets.render.test.ts (1)

126-139: ⚠️ Potential issue | 🟠 Major

renderBytes and renderBytesV3 are now identical, but later tests still assert version-specific differences.

Line 126 and Line 138 both use createDrawlistBuilder(), so both helpers now produce the same protocol output. That conflicts with downstream assertions that expect v3-only fields and v1 degradation, and can cause invalid/failing tests.

Suggested direction (collapse to single current-protocol expectations)
-function renderBytesV3(
-  vnode: VNode,
-  viewport: Readonly<{ cols: number; rows: number }> = { cols: 64, rows: 20 },
-  opts: Readonly<{
-    focusedId?: string | null;
-    theme?: Theme;
-    focusAnnouncement?: string | null;
-  }> = {},
-): Uint8Array {
-  return renderBytesWithBuilder(vnode, () => createDrawlistBuilder(), viewport, opts);
-}
+// Unified protocol path: keep a single render helper.
-  test("link encodes hyperlink refs on v3 and degrades on v1", () => {
-    const v3 = renderBytesV3(ui.link("https://example.com", "Docs"), { cols: 40, rows: 4 });
-    const v1 = renderBytes(ui.link("https://example.com", "Docs"), { cols: 40, rows: 4 });
-    ...
-    assert.equal((v3Docs?.linkUriRef ?? 0) > 0, true);
-    assert.equal(v1Docs?.linkUriRef ?? 0, 0);
-  });
+  test("link encodes hyperlink refs", () => {
+    const bytes = renderBytes(ui.link("https://example.com", "Docs"), { cols: 40, rows: 4 });
+    const docs = parseDrawTextCommands(bytes).find((cmd) => cmd.text === "Docs");
+    assert.equal((docs?.linkUriRef ?? 0) > 0, true);
+  });

Based on learnings, public-facing protocol references should consistently use v1 after the reset, not legacy version splits.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/widgets/__tests__/basicWidgets.render.test.ts` around lines
126 - 139, renderBytes and renderBytesV3 currently call the same builder
(createDrawlistBuilder) so both produce identical protocol output; change
renderBytes to use the v1 drawlist builder so it emits the legacy/public
protocol (e.g., call the v1 factory such as createDrawlistBuilderV1 or the
builder with version:1 options) while leaving renderBytesV3 using
createDrawlistBuilder(), ensuring tests that assert v1 vs v3 differences can
pass.
packages/core/src/runtime/commit.ts (1)

108-151: ⚠️ Potential issue | 🔴 Critical

textStyleEqual misses underline fields, which can cause incorrect leaf reuse.

When only underline style/color changes, this equality can still return true, and the leaf fast path may skip required updates.

🐛 Proposed fix
 function textStyleEqual(
   a:
     | {
         bold?: boolean;
         dim?: boolean;
         italic?: boolean;
         underline?: boolean;
+        underlineStyle?: unknown;
+        underlineColor?: unknown;
         inverse?: boolean;
         strikethrough?: boolean;
         overline?: boolean;
         blink?: boolean;
         fg?: unknown;
         bg?: unknown;
       }
     | undefined,
   b:
     | {
         bold?: boolean;
         dim?: boolean;
         italic?: boolean;
         underline?: boolean;
+        underlineStyle?: unknown;
+        underlineColor?: unknown;
         inverse?: boolean;
         strikethrough?: boolean;
         overline?: boolean;
         blink?: boolean;
         fg?: unknown;
         bg?: unknown;
       }
     | undefined,
 ): boolean {
@@
     a.underline === b.underline &&
+    a.underlineStyle === b.underlineStyle &&
+    colorEqual(a.underlineColor, b.underlineColor) &&
     a.inverse === b.inverse &&
     a.strikethrough === b.strikethrough &&
     a.overline === b.overline &&
     a.blink === b.blink &&
     colorEqual(a.fg, b.fg) &&
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/runtime/commit.ts` around lines 108 - 151, The
textStyleEqual function currently omits the underline color field from its type
and equality checks, causing underline-only style changes to be treated as
equal; update the style shape in textStyleEqual to include underlineColor?:
unknown (alongside fg/bg) and add a corresponding comparison using
colorEqual(a.underlineColor, b.underlineColor) in the returned boolean
expression, referencing the existing textStyleEqual and colorEqual helpers so
underline color differences trigger updates.
packages/core/src/widgets/__tests__/styleUtils.test.ts (1)

37-41: ⚠️ Potential issue | 🟠 Major

Fix stale underlineColor expectation shape

At Line 37, the test now passes packed RGB input but still expects an { r, g, b } object. sanitizeTextStyle() now emits packed Rgb24, so this assertion will fail.

Suggested patch
   test("sanitizeTextStyle preserves underlineColor rgb", () => {
-    assert.deepEqual(sanitizeTextStyle({ underlineColor: ((1 << 16) | (2 << 8) | 3) }).underlineColor, {
-      r: 1,
-      g: 2,
-      b: 3,
-    });
+    assert.deepEqual(
+      sanitizeTextStyle({ underlineColor: ((1 << 16) | (2 << 8) | 3) }).underlineColor,
+      ((1 << 16) | (2 << 8) | 3),
+    );
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/widgets/__tests__/styleUtils.test.ts` around lines 37 - 41,
The test is asserting that sanitizeTextStyle returns an object {r,g,b} for
underlineColor but the implementation now returns a packed Rgb24 integer; update
the expectation in the assert.deepEqual call inside the styleUtils.test (the
line using sanitizeTextStyle({ underlineColor: ((1 << 16) | (2 << 8) | 3)
}).underlineColor) to compare against the packed numeric value ((1 << 16) | (2
<< 8) | 3) instead of an { r, g, b } object so the assertion matches the new
Rgb24 output.
packages/core/src/drawlist/__tests__/builder.round-trip.test.ts (1)

385-406: ⚠️ Potential issue | 🟡 Minor

Rename stale v2 test titles to match actual protocol assertions.

These test names still say v2, but Line 396 and Line 519 assert ZR_DRAWLIST_VERSION_V1. The mismatch makes failures harder to interpret.

✏️ Suggested title updates
-  test("v2 header uses version 2 and correct cmd byte/count totals", () => {
+  test("v1 header uses version 1 and correct cmd byte/count totals", () => {
@@
-  test("v2 setCursor readback preserves payload fields and reserved byte", () => {
+  test("v1 setCursor readback preserves payload fields and reserved byte", () => {
@@
-  test("v2 multiple cursor commands are emitted in-order", () => {
+  test("v1 multiple cursor commands are emitted in-order", () => {
@@
-  test("v2 mixed frame keeps aligned sections and expected total byte size", () => {
+  test("v1 mixed frame keeps aligned sections and expected total byte size", () => {
Based on learnings: In the Rezi project, public API references and documentation must consistently use the current public protocol version (v1) after the reset.

Also applies to: 426-506

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/drawlist/__tests__/builder.round-trip.test.ts` around lines
385 - 406, Test titles in builder.round-trip.test.ts incorrectly reference "v2"
while assertions (e.g., comparing h.version to ZR_DRAWLIST_VERSION_V1 in the
tests that call createDrawlistBuilder(), readHeader(), assert.equal(...
ZR_DRAWLIST_VERSION_V1)) verify the v1 protocol; update those test name strings
(for example the tests currently titled "v2 header uses version 2 and correct
cmd byte/count totals" and "v2 setCursor readback preserves payload fields and
reserved byte" and any other "v2" titled tests in the same block) to reflect v1
(or remove the version prefix) so the descriptions match the asserted
ZR_DRAWLIST_VERSION_V1 behavior.
packages/core/src/drawlist/__tests__/builder.cursor.test.ts (1)

2-10: ⚠️ Potential issue | 🟡 Minor

Update stale v2 wording in the test header comment.

The block comment still says “Correct v2 header version,” but this test validates ZR_DRAWLIST_VERSION_V1.

📝 Proposed fix
- *   - Correct v2 header version
+ *   - Correct current header version (v1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/drawlist/__tests__/builder.cursor.test.ts` around lines 2 -
10, The test header comment incorrectly states "Correct v2 header version" while
the test validates ZR_DRAWLIST_VERSION_V1; update the block comment in
builder.cursor.test.ts to refer to the v1 header version (e.g., "Correct v1
header version" or "Correct header version (v1)") and ensure any other v2
wording in that top comment is changed to match ZR_DRAWLIST_VERSION_V1 so the
comment accurately reflects the test's intent.
packages/core/src/renderer/__tests__/renderer.damage.test.ts (1)

486-526: ⚠️ Potential issue | 🟠 Major

Packed color migration currently weakens style assertions in this test harness.

colorKey() doesn’t handle numeric Rgb24, so distinct packed colors collapse to "?". This can hide real color regressions in framebuffer comparisons.

💡 Proposed fix
 function colorKey(value: unknown): string {
   if (value === undefined || value === null) return "";
   if (typeof value === "string") return `s:${value}`;
+  if (typeof value === "number" && Number.isFinite(value)) {
+    return `n:${String((value >>> 0) & 0xffffff)}`;
+  }
   if (typeof value === "object") {
     const rgb = value as { r?: unknown; g?: unknown; b?: unknown };
     if (
       typeof rgb.r === "number" &&
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/renderer/__tests__/renderer.damage.test.ts` around lines
486 - 526, The test harness collapses distinct packed colors because colorKey()
doesn't handle numeric Rgb24 values, causing different packed colors to become
"?" and masking regressions; update colorKey() to detect numeric packed RGB24
values (e.g., the same packed ints used in renderer.damage.test.ts and
represented by type Rgb24) and return a stable unique string (such as a hex
"#RRGGBB" or "rgb(r,g,b)") instead of "?", so framebuffer comparisons and style
assertions remain precise; modify the implementation used by the tests (where
colorKey is referenced) to convert the packed int into separate R, G, B
components and produce that deterministic representation.
🧹 Nitpick comments (14)
packages/ink-compat/src/_bench_layout_profile.ts (2)

44-55: Include frameCount in the dump output for complete snapshots.

frameCount is tracked and incremented, but it’s missing from dumpLayoutCounters, so profile dumps are slightly incomplete.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ink-compat/src/_bench_layout_profile.ts` around lines 44 - 55, The
layout profile dump is missing the tracked frameCount value; update the
dumpLayoutCounters output (where the array of lines is built using c.* values)
to include a line like `frameCount: ${c.frameCount}` so the snapshot includes
frameCount alongside layoutNodeCalls, measureNodeCalls, etc.; locate the array
construction in dumpLayoutCounters (references to c.layoutNodeCalls,
c.measureCacheHits, c.textMeasureTotalMs) and insert the frameCount entry in the
list.

13-39: Avoid duplicating the counter schema across initialization and reset.

The same field list is maintained in two places (layoutCounters init and resetLayoutCounters), which is easy to desync when adding/removing counters.

♻️ Proposed refactor
+type LayoutCounters = {
+  layoutNodeCalls: number;
+  measureNodeCalls: number;
+  measureCacheHits: number;
+  layoutCacheHits: number;
+  constraintPassCount: number;
+  textMeasureCalls: number;
+  textMeasureTotalChars: number;
+  textMeasureTotalMs: number;
+  stackLayoutCalls: number;
+  leafLayoutCalls: number;
+  frameCount: number;
+};
+
+const ZERO_LAYOUT_COUNTERS: Omit<LayoutCounters, "frameCount"> = {
+  layoutNodeCalls: 0,
+  measureNodeCalls: 0,
+  measureCacheHits: 0,
+  layoutCacheHits: 0,
+  constraintPassCount: 0,
+  textMeasureCalls: 0,
+  textMeasureTotalChars: 0,
+  textMeasureTotalMs: 0,
+  stackLayoutCalls: 0,
+  leafLayoutCalls: 0,
+};
+
-export const layoutCounters = {
-  layoutNodeCalls: 0,
-  measureNodeCalls: 0,
-  measureCacheHits: 0,
-  layoutCacheHits: 0,
-  constraintPassCount: 0,
-  textMeasureCalls: 0,
-  textMeasureTotalChars: 0,
-  textMeasureTotalMs: 0,
-  stackLayoutCalls: 0,
-  leafLayoutCalls: 0,
-  frameCount: 0,
-};
+export const layoutCounters: LayoutCounters = { ...ZERO_LAYOUT_COUNTERS, frameCount: 0 };

 export function resetLayoutCounters(): void {
-  layoutCounters.layoutNodeCalls = 0;
-  layoutCounters.measureNodeCalls = 0;
-  layoutCounters.measureCacheHits = 0;
-  layoutCounters.layoutCacheHits = 0;
-  layoutCounters.constraintPassCount = 0;
-  layoutCounters.textMeasureCalls = 0;
-  layoutCounters.textMeasureTotalChars = 0;
-  layoutCounters.textMeasureTotalMs = 0;
-  layoutCounters.stackLayoutCalls = 0;
-  layoutCounters.leafLayoutCalls = 0;
-  layoutCounters.frameCount += 1;
+  const nextFrame = layoutCounters.frameCount + 1;
+  Object.assign(layoutCounters, ZERO_LAYOUT_COUNTERS, { frameCount: nextFrame });
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ink-compat/src/_bench_layout_profile.ts` around lines 13 - 39, The
counter fields are duplicated between the layoutCounters object and
resetLayoutCounters, risking drift; create a single source of truth (e.g., an
initialLayoutCounters constant or a function getInitialLayoutCounters()) and
have resetLayoutCounters reuse it (for example via Object.assign(layoutCounters,
initialLayoutCounters) and then increment layoutCounters.frameCount) instead of
manually resetting each field; update references to layoutCounters and
resetLayoutCounters accordingly so adding/removing counters only requires
changing the initializer.
packages/core/src/renderer/__tests__/renderer.scrollbar.test.ts (1)

508-508: Packed RGB color representation is correct, but Object.freeze on a primitive is a no-op.

The packed 24-bit RGB value ((1 << 16) | (2 << 8) | 3) correctly encodes RGB(1,2,3) as 0x010203. However, Object.freeze() has no effect on primitive numbers in JavaScript—it returns the value unchanged. Consider removing the freeze for clarity:

🔧 Suggested simplification
-    const customFg = Object.freeze(((1 << 16) | (2 << 8) | 3));
+    const customFg = (1 << 16) | (2 << 8) | 3;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/renderer/__tests__/renderer.scrollbar.test.ts` at line 508,
The test declares const customFg = Object.freeze(((1 << 16) | (2 << 8) | 3)) but
Object.freeze is a no-op on primitives; remove the unnecessary Object.freeze
wrapper around the packed RGB expression so customFg is simply the numeric value
((1 << 16) | (2 << 8) | 3), leaving the packed 24-bit RGB representation intact
and clearer in the test.
packages/core/src/widgets/commandPalette.ts (1)

284-290: Replace hardcoded palette RGB constants with theme token keys.

This keeps command palette visuals aligned with theme overrides and avoids hardcoded color drift.

As per coding guidelines "Never hardcode RGB color values; let the theme handle colors via tokens. Use variant props on badges, tags, callouts, status indicators for semantic colors. Use dsVariant/dsTone on buttons and interactive widgets for consistent theming".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/widgets/commandPalette.ts` around lines 284 - 290,
PALETTE_COLORS currently hardcodes rgb(...) values; replace these with theme
token keys so the command palette follows theme overrides. Update the
PALETTE_COLORS export to map keys (background, border, selectedBg, highlight,
placeholder) to appropriate theme tokens or token-resolving helpers used in the
codebase (e.g., use tokens like token('palette.background') or
theme.get('palette.background')), and for interactive/semantic colors prefer
using component props (dsVariant/dsTone or variant on
badges/tags/callouts/status indicators) where those colors are applied; ensure
any consumer of PALETTE_COLORS expects token strings or resolved values
consistently.
packages/core/src/pipeline.ts (1)

34-35: Avoid exporting temporary profiling hooks from the shared pipeline entrypoint.

__layoutProfile is labeled temporary, but exporting it here makes downstream coupling likely. Prefer keeping it on an internal/unstable path to avoid accidental API lock-in.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/pipeline.ts` around lines 34 - 35, The temporary profiling
symbol __layoutProfile should not be exported from the shared pipeline
entrypoint; remove the export statement "export { __layoutProfile } from
'./layout/engine/layoutEngine.js'" from the pipeline entrypoint and instead
expose it only from an internal/unstable module (or behind a clearly marked
unstable flag/namespace) so downstream code doesn't accidentally depend on it;
update any consumers to import __layoutProfile from the new internal/unstable
path or the guarded export location.
packages/core/src/widgets/splitPane.ts (1)

373-373: Consider using a theme token for divider color.

The migration from { r: 80, g: 80, b: 80 } to rgb(80, 80, 80) correctly aligns with the packed Rgb24 representation. However, per coding guidelines, hardcoded RGB values should be avoided in favor of theme tokens for consistency and theming support.

If this is intentional infrastructure code that must remain constant regardless of theme, this is acceptable. Otherwise, consider exposing this as a theme token.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/widgets/splitPane.ts` at line 373, DIVIDER_COLOR is
currently a hardcoded rgb(80, 80, 80); replace it with a theme token so the
divider color follows theming: introduce or use an existing theme token (e.g.,
"divider" or "colors.divider") and have DIVIDER_COLOR reference that token (with
an explicit fallback to rgb(80,80,80) if the theme token is absent) so consumers
like the SplitPane component read from the theme instead of a fixed Rgb24 value;
update usages of DIVIDER_COLOR accordingly to consume the theme-aware value.
packages/core/src/renderer/styles.ts (1)

9-9: Prefer semantic theme color over hardcoded disabled gray

At Line 9, hardcoding rgb(128, 128, 128) bypasses theme presets and can drift from design-token contrast choices. Consider deriving disabled text color from theme tokens at call site (or by threading theme into this helper).

As per coding guidelines "Never hardcode RGB color values; let the theme handle colors via tokens."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/renderer/styles.ts` at line 9, Replace the hardcoded
DISABLED_FG constant with a theme-driven value: remove or convert const
DISABLED_FG = rgb(128, 128, 128) in styles.ts and instead derive the disabled
text color from the theme token at use-site or by changing DISABLED_FG into a
function that accepts the Theme (e.g., getDisabledFg(theme) or using
theme.colors.disabledText / theme.tokens.text.disabled). Update callers to pass
the theme or use the theme accessor so colors follow the design tokens rather
than a hardcoded RGB.
packages/core/src/app/createApp.ts (1)

206-219: Centralize drawlist version policy to avoid drift.

Validation is currently split between marker parsing ([1..5]) and startup enforcement (=== 1). Consider using a single constant/policy path so future version changes only need one edit.

Also applies to: 598-605

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/app/createApp.ts` around lines 206 - 219, Centralize the
drawlist version policy by introducing a single constant (e.g.,
ALLOWED_DRAWLIST_VERSIONS or DRAWLIST_VERSION_POLICY with min/max/current) and
use it in readBackendDrawlistVersionMarker (replace the hard-coded [1..5]
checks) and in the startup enforcement logic (the code that currently checks ===
1) so both parsing/validation and startup enforcement reference the same
constant; update readBackendDrawlistVersionMarker and the startup check to read
from that shared constant and adjust the invalidProps message to reference the
policy constant.
packages/core/src/drawlist/__tests__/builder_style_attrs.test.ts (1)

33-77: Deduplicate equivalent dim-attr tests after unified builder migration.

These cases now hit the same builder path and no longer validate distinct behavior. Consolidating them will reduce noise and keep the suite intent clear.

♻️ Suggested cleanup
-  test("drawText attrs include dim without shifting existing bits (repeat)", () => {
-    const b = createDrawlistBuilder();
-    b.drawText(0, 0, "dim", { dim: true });
-    const res = b.build();
-    assert.equal(res.ok, true);
-    if (!res.ok) return;
-
-    assert.equal(drawTextAttrs(res.bytes), 1 << 4);
-  });
+  // Removed duplicate case: identical coverage to previous drawText dim test.

-  test("v1 text-run attrs include dim without shifting existing bits", () => {
+  test("text-run attrs include dim without shifting existing bits", () => {
     const b = createDrawlistBuilder();
     const blobIndex = b.addTextRunBlob([
       { text: "dim", style: { dim: true } },
       { text: "base", style: { bold: true, italic: true, underline: true, inverse: true } },
     ]);
@@
-  test("v2 text-run attrs include dim without shifting existing bits", () => {
-    const b = createDrawlistBuilder();
-    const blobIndex = b.addTextRunBlob([
-      { text: "dim", style: { dim: true } },
-      { text: "base", style: { bold: true, italic: true, underline: true, inverse: true } },
-    ]);
-    assert.equal(blobIndex, 0);
-    if (blobIndex === null) return;
-
-    b.drawTextRun(0, 0, blobIndex);
-    const res = b.build();
-    assert.equal(res.ok, true);
-    if (!res.ok) return;
-
-    assert.equal(textRunAttrs(res.bytes, 0), 1 << 4);
-    assert.equal(textRunAttrs(res.bytes, 1), (1 << 0) | (1 << 1) | (1 << 2) | (1 << 3));
-  });
+  // Removed duplicate text-run case: same setup and assertions.

Based on learnings: In the Rezi project, public API references and documentation must consistently use the current public protocol version (v1) after the reset.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/drawlist/__tests__/builder_style_attrs.test.ts` around
lines 33 - 77, Consolidate duplicate dim-attribute tests: keep the drawText
attrs test for drawText (createDrawlistBuilder + drawText + drawTextAttrs) and
replace the two redundant text-run tests ("v1 text-run attrs include dim..." and
"v2 text-run attrs include dim...") with a single text-run test that adds a blob
via addTextRunBlob, calls drawTextRun, builds, and asserts textRunAttrs for
index 0 equals 1<<4 and for index 1 equals (1<<0)|(1<<1)|(1<<2)|(1<<3); remove
the extra v1/v2 variants so the suite validates the unified builder path using
createDrawlistBuilder, addTextRunBlob, drawTextRun, textRunAttrs, and
drawTextAttrs exactly once each.
packages/core/src/app/__tests__/rawRender.test.ts (1)

31-39: Keep buildInto() consistent with build() in the main stub.

Line 31-35 enforces single-build behavior, but Line 37-39 bypasses it. If the runtime uses buildInto(), this test double can mask duplicate-build regressions.

♻️ Suggested adjustment
-    buildInto() {
-      return { ok: true, bytes } as const;
-    },
+    buildInto() {
+      return this.build();
+    },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/app/__tests__/rawRender.test.ts` around lines 31 - 39, The
test stub's buildInto() bypasses the single-build guard present in build():
update buildInto() to mirror build() behavior by checking the same built flag,
returning the identical error shape ({ ok: false, error: { code:
"ZRDL_INTERNAL", detail: "built twice" } } as const) when built is already true,
and otherwise set built = true and return { ok: true, bytes } as const; keep the
same const type assertions and use the same bytes variable so both build() and
buildInto() enforce single-build semantics.
packages/core/src/theme/resolve.ts (1)

68-75: JSDoc examples show outdated RGB object format.

The code examples in the docstring still show { r: 230, g: 225, b: 207 } object format, but the function now returns Rgb24 (packed 24-bit integer). Consider updating the examples to reflect the new return type.

📝 Suggested documentation update
 * `@example`
 * ```typescript
 * const color = resolveColorToken(darkTheme, "fg.primary");
-* // { r: 230, g: 225, b: 207 }
+* // 0xe6e1cf (packed RGB24)
 *
 * const error = resolveColorToken(darkTheme, "error");
-* // { r: 240, g: 113, b: 120 }
+* // 0xf07178 (packed RGB24)
 * ```
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/theme/resolve.ts` around lines 68 - 75, Update the JSDoc
examples in the resolveColorToken docstring to show the new Rgb24 packed 24-bit
integer return format instead of the old { r,g,b } object; specifically change
the example outputs for resolveColorToken(darkTheme, "fg.primary") and
resolveColorToken(darkTheme, "error") to their hex-packed Rgb24 values (e.g.,
0xe6e1cf and 0xf07178) and mention "packed RGB24" so the example matches the
actual return type of resolveColorToken and any related types like Rgb24.
packages/core/src/drawlist/__tests__/builder.reset.test.ts (1)

103-104: Consider updating test names to reflect unified builder.

The test names still reference "v1" and "v2" (e.g., "v1 reset clears prior commands", "v2 reset drops cursor"), but these now use the unified createDrawlistBuilder(). Consider renaming to avoid confusion, e.g., "reset clears prior commands/strings/blobs" and "reset drops cursor and string state".

Also applies to: 134-135

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/drawlist/__tests__/builder.reset.test.ts` around lines 103
- 104, Tests still reference "v1"/"v2" but use the unified
createDrawlistBuilder(); rename the test titles (e.g., change "v1 reset clears
prior commands/strings/blobs" to "reset clears prior commands/strings/blobs" and
"v2 reset drops cursor" to "reset drops cursor and string state") so names
reflect the unified builder; update the test descriptions in the files that call
createDrawlistBuilder() (the tests around the occurrences currently labeled with
v1/v2) to remove version prefixes and use the suggested clearer wording.
packages/ink-compat/src/translation/propsToVNode.ts (1)

501-502: Hoist NATIVE_PERCENT_PROPS to module scope to avoid per-node allocations.

new Set([...]) is created on every translateBox() call.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ink-compat/src/translation/propsToVNode.ts` around lines 501 - 502,
NATIVE_PERCENT_PROPS is being instantiated inside translateBox() causing a new
Set allocation on every call; move the declaration const NATIVE_PERCENT_PROPS =
new Set(["width","height","flexBasis"]) to module scope (top of the file) so
translateBox() and any other functions reuse the single Set instance, leaving
all uses of NATIVE_PERCENT_PROPS unchanged.
packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts (1)

10-10: Rename legacy v6 test artifact to v1 terminology.

The test file naming still carries pre-reset version wording (builder_v6_*), which can mislead readers about the active public protocol version.

Based on learnings: In the Rezi project, public API references and documentation must consistently use the current public protocol version (v1) after the reset, and must not reference the old v6 iteration.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts` at line
10, Rename legacy v6 references to v1 in this test: change the file name from
builder_v6_resources.test.ts to builder_v1_resources.test.ts and update any
in-file artifacts that mention "v6" (including test descriptions, snapshot
names, constants, or import paths) to use "v1" instead; specifically update the
describe string and any identifiers or fixtures referencing "v6" so they read
"v1" (e.g., update "DrawlistBuilder resource inputs" contexts or any variables
like builder_v6_* to builder_v1_*), run tests to ensure imports/snapshots are
found and update snapshot filenames if needed.
🤖 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/bench/src/profile-packed-style.ts`:
- Around line 75-77: The three computed-key property accesses should use dot
notation for clarity and to satisfy linting: change
counters["style_merges_performed"] ?? 0 to counters.style_merges_performed ?? 0,
counters["style_objects_created"] ?? 0 to counters.style_objects_created ?? 0,
and counters["packRgb_calls"] ?? 0 to counters.packRgb_calls ?? 0; update the
assignments for merges, styleObjects, and packRgbCalls accordingly while
preserving the nullish-coalescing defaults.

In `@packages/core/src/animation/interpolate.ts`:
- Line 5: The import specifiers in interpolate.ts are not sorted per CI; reorder
the specifiers in the import from "../widgets/style.js" to match the project's
import-sorting rule (alphabetical, case-insensitive) so that the names (rgb,
rgbB, rgbG, rgbR, type Rgb24) appear in the correct sorted order.

In `@packages/core/src/app/widgetRenderer.ts`:
- Around line 541-543: Remove the redundant union types and unused helper:
delete the unused isV2Builder function and replace any parameter or field
annotations that read as redundant unions (e.g., "DrawlistBuilder |
DrawlistBuilder" and "DrawlistBuilder | DrawlistBuilder | DrawlistBuilder") with
the single type DrawlistBuilder; update the builder field declaration (the
member named "builder") to be typed simply as DrawlistBuilder and remove any
references to isV2Builder so dead/unneeded code is eliminated.

In `@packages/core/src/drawlist/builder.ts`:
- Around line 188-191: The validation currently masks state.shape with bitwise
(& 0xff) before checking range, which hides non-finite values; update the
setCursor handling to validate state.shape for finitude and integrality before
coercion (use the existing requireI32 or the same pre-coercion check used for
x/y and zLayer in drawImage) and only then compute const shape = state.shape &
0xff;; if validation fails call this.fail("ZRDL_BAD_PARAMS", ...) as before.
Ensure you reference state.shape, validateParams, requireI32 (or the same check
used for zLayer) and keep the allowed range check for 0–2 after coercion.

In `@packages/core/src/runtime/commit.ts`:
- Around line 1295-1299: The diagnostic object pushed into __commitDiag should
assign detail directly instead of using a redundant type assertion; update the
object literal in the __commitDiag.push call (the one using instanceId,
vnode.kind and childDiffs) to set detail: "children-changed" without the as
"was-dirty" | undefined cast so the value matches the declared union type and
remains consistent with other pushes in this file.

In `@packages/core/src/theme/extract.ts`:
- Around line 7-8: The falsy check on the packed color token (const bgBase =
c["bg.base"] as Rgb24 | undefined; if (!bgBase) return null;) incorrectly treats
a valid black value (0) as missing; change the presence check to explicitly test
for undefined/null (e.g., if (bgBase === undefined) return null or if (bgBase ==
null) return null) so that Rgb24 value 0 is accepted and semantic token
extraction continues; update any similar checks in this module that use
truthiness on packed color tokens (references: bgBase, c["bg.base"], Rgb24).

In `@packages/core/src/widgets/__tests__/graphicsWidgets.test.ts`:
- Around line 169-173: The test "link encodes hyperlink refs in v3 and degrades
to text in v1" incorrectly calls renderBytes with the same
createDrawlistBuilder() for both v3 and v1, so it no longer tests distinct
paths; fix by either constructing v1 with the legacy builder (e.g., use the
legacy builder factory used elsewhere such as createLegacyDrawlistBuilder or
createDrawlistBuilderV1) when calling renderBytes for the v1 variable, or
simplify the test to only assert the v3 behavior by removing the v1 render and
its assertion; update references to ui.link, renderBytes, createDrawlistBuilder
(and the legacy builder name you choose), and parseOpcodes accordingly so the
test again validates two different builder paths or only v3 as intended.

In `@packages/core/src/widgets/__tests__/overlays.typecheck.ts`:
- Around line 26-27: Update the stale `@ts-expect-error` comment above the
declaration of dropdownFrameInvalid: DropdownProps["frameStyle"] to reflect the
current type mismatch (object literal vs numeric packed color) instead of
"missing b component"; locate the const dropdownFrameInvalid definition and
change the rationale text to state that the error is due to using an object {
border: { r: 1, g: 2 } } where a packed numeric color is expected.

In `@packages/core/src/widgets/diffViewer.ts`:
- Around line 232-238: The colors for the diff widget are hardcoded as rgb(...)
literals (addBg, deleteBg, addFg, deleteFg, hunkHeader, lineNumber, border) —
replace these with theme/design-system tokens or semantic variant props so
colors are centrally themeable; locate the color map in diffViewer.ts (the
object containing addBg/deleteBg/etc) and swap each rgb(...) value for the
appropriate token lookup or semantic variant (e.g., using the app theme API or
Badge/Status variants) so addition/deletion/background/foreground/hunk
header/line number/border use theme tokens instead of raw RGBs, keeping the same
semantic mapping.

In `@packages/core/src/widgets/logsConsole.ts`:
- Around line 209-215: LEVEL_COLORS currently uses hardcoded rgb(...) literals;
replace these with theme/design tokens so colors come from the centralized theme
system. Import and use the appropriate token accessors (e.g., your design token
map or helper like getTokenColor/resolveColor) and map each LogLevel key in
LEVEL_COLORS (trace, debug, info, warn, error) to the corresponding semantic
token instead of rgb(...), ensuring the values are converted/typed to Rgb24 (or
use an existing token-to-Rgb24 helper) so types (LogLevel, Rgb24) still match.

In `@packages/core/src/widgets/style.ts`:
- Around line 5-6: The Rgb24 typedef's comment incorrectly claims value 0 is a
reserved/unset sentinel while rgb(r,g,b) encodes black as 0; update the
documentation for Rgb24 to reflect the actual encoding semantics (packed
0x00RRGGBB) and that 0 represents black (rgb(0,0,0)) rather than being reserved,
and ensure any mention of rgb() behavior in the same file (e.g., the rgb
function) is consistent with the revised comment.

In `@packages/core/src/widgets/toast.ts`:
- Around line 172-177: TOAST_COLORS currently hardcodes RGB values (rgb(…)) for
each Toast["type"]); replace those literal RGBs with theme/design tokens or
semantic variants so the toast border colors come from the design system. Update
the TOAST_COLORS mapping to reference the theme token names or semantic props
(e.g., use token keys or a helper that maps toast types to token values) and,
where applicable, switch to using component semantic props like
variant/dsVariant or dsTone on the toast Badge/Callout/StatusIndicator
components instead of raw rgb(...) so palette overrides and theming are
respected.

In `@packages/ink-compat/src/runtime/createInkRenderer.ts`:
- Around line 523-526: The early-return in createInkRenderer's fast path (when
isFirstFrame/forceLayout/prevRoot.dirty checks allow returning
cachedOps/cachedNodes) skips emitting any tracing events; update that branch to
emit/complete the same trace/span used elsewhere in this module (the module's
trace/tracer API) before returning and include timings (commitMs, totalMs and
layoutSkipped: true) so skipped frames appear in traces/metrics; locate the
fast-path using the variables isFirstFrame, forceLayout, prevRoot.dirty,
cachedLayoutTree, cachedOps and cachedNodes and call the existing trace API in
the same manner as other code paths rather than returning immediately.

In `@packages/ink-compat/src/runtime/render.ts`:
- Around line 2517-2543: The appendFileSync call in the frame profiling block
should avoid string concatenation and the use of the any type: build the object
to stringify first, extracting a safely-typed layout profile via a type guard
(e.g., const timings = result.timings as unknown; if (timings && typeof timings
=== 'object' && '_layoutProfile' in timings) { layoutProfile = { _lp: (timings
as any)['_layoutProfile'] } } else layoutProfile = {};), merge that
layoutProfile into the payload, then call appendFileSync(frameProfileFile,
`${JSON.stringify(payload)}\n`) using a template literal; update references to
result.timings/_layoutProfile and result.ops/result.nodes as needed in the
payload construction and remove the inline any cast.
- Around line 1160-1172: The checks treating 0 as "unset" prevent explicit black
(0x000000) from being applied; update the predicates in render.ts so they test
for undefined/null rather than falsy zero: replace `style.fg !== 0` and
`style.bg !== 0` with explicit existence checks (e.g., `style.fg !== undefined`
/ `style.bg !== undefined`) in the normalization block that uses isRgb24,
clampRgb24 and CORE_DEFAULT_FG/CORE_DEFAULT_BG, and likewise replace falsy
checks in styleToSgr (`if (style.fg)` and `if (style.bg)`) with explicit
undefined checks; alternatively, choose and document a distinct sentinel (e.g.,
-1) across the codebase and update usages accordingly so 0 (black) is treated as
a valid color.

In `@packages/ink-compat/src/translation/colorMap.ts`:
- Around line 54-59: The trace code incorrectly treats result as falsy so a
valid black color (packed Rgb24 value 0) is recorded as null; change the
conditional that builds the trace payload to check for undefined (e.g., result
!== undefined) instead of using a truthy check so pushTranslationTrace receives
{ r: rgbR(result), g: rgbG(result), b: rgbB(result) } for valid black parses;
update the block around isTranslationTraceEnabled()/pushTranslationTrace and
reference result, rgbR, rgbG, rgbB, and parseColorInner to ensure only undefined
indicates parse failure.

In `@packages/ink-compat/src/translation/propsToVNode.ts`:
- Around line 15-16: Props color checks treat numeric 0 (black) as falsy; update
the checks in propsToVNode.ts to detect presence explicitly (e.g., use fg !==
undefined && fg !== null or fg != null) instead of if (fg) so 0x000000 is
preserved; apply the same change for bg and borderColor (and any helper like
mapColorProp or serializeColor) wherever you currently use truthy checks to gate
color propagation.

In `@packages/native/vendor/zireael/src/core/zr_drawlist.h`:
- Around line 89-101: Add documentation to the backend native docs (or a new
resource management section) describing the C resource lifecycle and how it
integrates with drawlist execution: explain the purpose and usage of
zr_dl_resources_init, zr_dl_resources_release, zr_dl_resources_swap,
zr_dl_resources_clone, and zr_dl_resources_clone_shallow; describe how
zr_dl_preflight_resources and zr_dl_execute interact with the resource subsystem
during preflight/execute, why preflight takes shallow snapshots for stage
borrowing (zr_image_frame_t image_stage / image_frame_stage) versus when deep
clones are required, and give guidance/examples for correct ownership semantics
and when to call init/release/swap to avoid leaks or double-free. Ensure the
docs clarify the behavioral differences between deep vs shallow clones and when
each function should be used in the persistent drawlist resource lifecycle.

In `@packages/native/vendor/zireael/src/core/zr_engine_present.inc`:
- Around line 341-365: The code clears e->diff_prev_hashes_valid on
copy/clone/fallback errors but zr_engine_swap_diff_hashes_on_commit later
re-enables it, undoing the invalidation; update the logic so a failure in
zr_fb_copy_damage_rects, zr_fb_links_clone_from or the fb_prev shape check
permanently preserves the invalid state across the commit swap. Concretely,
either (A) add/keep a preservation flag (e.g.
e->preserve_diff_prev_hashes_invalidation) set to true when you set
e->diff_prev_hashes_valid = 0u in the failure branches and have
zr_engine_swap_diff_hashes_on_commit check that flag before re-enabling hashes,
or (B) change zr_engine_swap_diff_hashes_on_commit to respect the current
e->diff_prev_hashes_valid (only enable hashes if it is already true) so the
earlier invalidation from the branches (those using zr_fb_copy_damage_rects and
zr_fb_links_clone_from) is not overwritten.

In `@packages/native/vendor/zireael/src/core/zr_framebuffer.c`:
- Around line 208-270: zr_fb_copy_damage_rects copies zr_cell_t (including
style.link_ref) but doesn't sync dst link tables, causing copied link references
to be invalid; after the initial input validation (in function
zr_fb_copy_damage_rects, before the for-loop that copies rects) call
zr_fb_links_reset(dst) and then return or call zr_fb_links_clone_from(dst, src)
as in zr_engine_fb_copy_noalloc to synchronize dst->links and dst->link_bytes
with src; follow the same ordering/pattern used in zr_engine_fb_copy_noalloc
(memcpy then zr_fb_links_reset(dst) and zr_fb_links_clone_from(dst, src)) so
link tables match after damage-region copying.

---

Outside diff comments:
In `@packages/core/src/drawlist/__tests__/builder.cursor.test.ts`:
- Around line 2-10: The test header comment incorrectly states "Correct v2
header version" while the test validates ZR_DRAWLIST_VERSION_V1; update the
block comment in builder.cursor.test.ts to refer to the v1 header version (e.g.,
"Correct v1 header version" or "Correct header version (v1)") and ensure any
other v2 wording in that top comment is changed to match ZR_DRAWLIST_VERSION_V1
so the comment accurately reflects the test's intent.

In `@packages/core/src/drawlist/__tests__/builder.round-trip.test.ts`:
- Around line 385-406: Test titles in builder.round-trip.test.ts incorrectly
reference "v2" while assertions (e.g., comparing h.version to
ZR_DRAWLIST_VERSION_V1 in the tests that call createDrawlistBuilder(),
readHeader(), assert.equal(... ZR_DRAWLIST_VERSION_V1)) verify the v1 protocol;
update those test name strings (for example the tests currently titled "v2
header uses version 2 and correct cmd byte/count totals" and "v2 setCursor
readback preserves payload fields and reserved byte" and any other "v2" titled
tests in the same block) to reflect v1 (or remove the version prefix) so the
descriptions match the asserted ZR_DRAWLIST_VERSION_V1 behavior.

In `@packages/core/src/renderer/__tests__/renderer.damage.test.ts`:
- Around line 486-526: The test harness collapses distinct packed colors because
colorKey() doesn't handle numeric Rgb24 values, causing different packed colors
to become "?" and masking regressions; update colorKey() to detect numeric
packed RGB24 values (e.g., the same packed ints used in renderer.damage.test.ts
and represented by type Rgb24) and return a stable unique string (such as a hex
"#RRGGBB" or "rgb(r,g,b)") instead of "?", so framebuffer comparisons and style
assertions remain precise; modify the implementation used by the tests (where
colorKey is referenced) to convert the packed int into separate R, G, B
components and produce that deterministic representation.

In `@packages/core/src/runtime/commit.ts`:
- Around line 108-151: The textStyleEqual function currently omits the underline
color field from its type and equality checks, causing underline-only style
changes to be treated as equal; update the style shape in textStyleEqual to
include underlineColor?: unknown (alongside fg/bg) and add a corresponding
comparison using colorEqual(a.underlineColor, b.underlineColor) in the returned
boolean expression, referencing the existing textStyleEqual and colorEqual
helpers so underline color differences trigger updates.

In `@packages/core/src/widgets/__tests__/basicWidgets.render.test.ts`:
- Around line 126-139: renderBytes and renderBytesV3 currently call the same
builder (createDrawlistBuilder) so both produce identical protocol output;
change renderBytes to use the v1 drawlist builder so it emits the legacy/public
protocol (e.g., call the v1 factory such as createDrawlistBuilderV1 or the
builder with version:1 options) while leaving renderBytesV3 using
createDrawlistBuilder(), ensuring tests that assert v1 vs v3 differences can
pass.

In `@packages/core/src/widgets/__tests__/styleUtils.test.ts`:
- Around line 37-41: The test is asserting that sanitizeTextStyle returns an
object {r,g,b} for underlineColor but the implementation now returns a packed
Rgb24 integer; update the expectation in the assert.deepEqual call inside the
styleUtils.test (the line using sanitizeTextStyle({ underlineColor: ((1 << 16) |
(2 << 8) | 3) }).underlineColor) to compare against the packed numeric value ((1
<< 16) | (2 << 8) | 3) instead of an { r, g, b } object so the assertion matches
the new Rgb24 output.

In `@packages/core/src/widgets/field.ts`:
- Around line 11-24: The FIELD_ERROR_STYLE constant uses a hardcoded rgb(...)
value for fg; replace that value with the semantic error color token from the
design/theme (e.g., the theme's error/semantic token) by assigning the token to
FIELD_ERROR_STYLE.fg, and remove the now-unused rgb import at the top (import {
rgb } from "./style.js") if nothing else in this file uses it; update the
Object.freeze definition for FIELD_ERROR_STYLE accordingly so the file uses the
theme token instead of rgb(255,0,0).

---

Nitpick comments:
In `@packages/core/src/app/__tests__/rawRender.test.ts`:
- Around line 31-39: The test stub's buildInto() bypasses the single-build guard
present in build(): update buildInto() to mirror build() behavior by checking
the same built flag, returning the identical error shape ({ ok: false, error: {
code: "ZRDL_INTERNAL", detail: "built twice" } } as const) when built is already
true, and otherwise set built = true and return { ok: true, bytes } as const;
keep the same const type assertions and use the same bytes variable so both
build() and buildInto() enforce single-build semantics.

In `@packages/core/src/app/createApp.ts`:
- Around line 206-219: Centralize the drawlist version policy by introducing a
single constant (e.g., ALLOWED_DRAWLIST_VERSIONS or DRAWLIST_VERSION_POLICY with
min/max/current) and use it in readBackendDrawlistVersionMarker (replace the
hard-coded [1..5] checks) and in the startup enforcement logic (the code that
currently checks === 1) so both parsing/validation and startup enforcement
reference the same constant; update readBackendDrawlistVersionMarker and the
startup check to read from that shared constant and adjust the invalidProps
message to reference the policy constant.

In `@packages/core/src/drawlist/__tests__/builder_style_attrs.test.ts`:
- Around line 33-77: Consolidate duplicate dim-attribute tests: keep the
drawText attrs test for drawText (createDrawlistBuilder + drawText +
drawTextAttrs) and replace the two redundant text-run tests ("v1 text-run attrs
include dim..." and "v2 text-run attrs include dim...") with a single text-run
test that adds a blob via addTextRunBlob, calls drawTextRun, builds, and asserts
textRunAttrs for index 0 equals 1<<4 and for index 1 equals
(1<<0)|(1<<1)|(1<<2)|(1<<3); remove the extra v1/v2 variants so the suite
validates the unified builder path using createDrawlistBuilder, addTextRunBlob,
drawTextRun, textRunAttrs, and drawTextAttrs exactly once each.

In `@packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts`:
- Line 10: Rename legacy v6 references to v1 in this test: change the file name
from builder_v6_resources.test.ts to builder_v1_resources.test.ts and update any
in-file artifacts that mention "v6" (including test descriptions, snapshot
names, constants, or import paths) to use "v1" instead; specifically update the
describe string and any identifiers or fixtures referencing "v6" so they read
"v1" (e.g., update "DrawlistBuilder resource inputs" contexts or any variables
like builder_v6_* to builder_v1_*), run tests to ensure imports/snapshots are
found and update snapshot filenames if needed.

In `@packages/core/src/drawlist/__tests__/builder.reset.test.ts`:
- Around line 103-104: Tests still reference "v1"/"v2" but use the unified
createDrawlistBuilder(); rename the test titles (e.g., change "v1 reset clears
prior commands/strings/blobs" to "reset clears prior commands/strings/blobs" and
"v2 reset drops cursor" to "reset drops cursor and string state") so names
reflect the unified builder; update the test descriptions in the files that call
createDrawlistBuilder() (the tests around the occurrences currently labeled with
v1/v2) to remove version prefixes and use the suggested clearer wording.

In `@packages/core/src/pipeline.ts`:
- Around line 34-35: The temporary profiling symbol __layoutProfile should not
be exported from the shared pipeline entrypoint; remove the export statement
"export { __layoutProfile } from './layout/engine/layoutEngine.js'" from the
pipeline entrypoint and instead expose it only from an internal/unstable module
(or behind a clearly marked unstable flag/namespace) so downstream code doesn't
accidentally depend on it; update any consumers to import __layoutProfile from
the new internal/unstable path or the guarded export location.

In `@packages/core/src/renderer/__tests__/renderer.scrollbar.test.ts`:
- Line 508: The test declares const customFg = Object.freeze(((1 << 16) | (2 <<
8) | 3)) but Object.freeze is a no-op on primitives; remove the unnecessary
Object.freeze wrapper around the packed RGB expression so customFg is simply the
numeric value ((1 << 16) | (2 << 8) | 3), leaving the packed 24-bit RGB
representation intact and clearer in the test.

In `@packages/core/src/renderer/styles.ts`:
- Line 9: Replace the hardcoded DISABLED_FG constant with a theme-driven value:
remove or convert const DISABLED_FG = rgb(128, 128, 128) in styles.ts and
instead derive the disabled text color from the theme token at use-site or by
changing DISABLED_FG into a function that accepts the Theme (e.g.,
getDisabledFg(theme) or using theme.colors.disabledText /
theme.tokens.text.disabled). Update callers to pass the theme or use the theme
accessor so colors follow the design tokens rather than a hardcoded RGB.

In `@packages/core/src/theme/resolve.ts`:
- Around line 68-75: Update the JSDoc examples in the resolveColorToken
docstring to show the new Rgb24 packed 24-bit integer return format instead of
the old { r,g,b } object; specifically change the example outputs for
resolveColorToken(darkTheme, "fg.primary") and resolveColorToken(darkTheme,
"error") to their hex-packed Rgb24 values (e.g., 0xe6e1cf and 0xf07178) and
mention "packed RGB24" so the example matches the actual return type of
resolveColorToken and any related types like Rgb24.

In `@packages/core/src/widgets/commandPalette.ts`:
- Around line 284-290: PALETTE_COLORS currently hardcodes rgb(...) values;
replace these with theme token keys so the command palette follows theme
overrides. Update the PALETTE_COLORS export to map keys (background, border,
selectedBg, highlight, placeholder) to appropriate theme tokens or
token-resolving helpers used in the codebase (e.g., use tokens like
token('palette.background') or theme.get('palette.background')), and for
interactive/semantic colors prefer using component props (dsVariant/dsTone or
variant on badges/tags/callouts/status indicators) where those colors are
applied; ensure any consumer of PALETTE_COLORS expects token strings or resolved
values consistently.

In `@packages/core/src/widgets/splitPane.ts`:
- Line 373: DIVIDER_COLOR is currently a hardcoded rgb(80, 80, 80); replace it
with a theme token so the divider color follows theming: introduce or use an
existing theme token (e.g., "divider" or "colors.divider") and have
DIVIDER_COLOR reference that token (with an explicit fallback to rgb(80,80,80)
if the theme token is absent) so consumers like the SplitPane component read
from the theme instead of a fixed Rgb24 value; update usages of DIVIDER_COLOR
accordingly to consume the theme-aware value.

In `@packages/ink-compat/src/_bench_layout_profile.ts`:
- Around line 44-55: The layout profile dump is missing the tracked frameCount
value; update the dumpLayoutCounters output (where the array of lines is built
using c.* values) to include a line like `frameCount: ${c.frameCount}` so the
snapshot includes frameCount alongside layoutNodeCalls, measureNodeCalls, etc.;
locate the array construction in dumpLayoutCounters (references to
c.layoutNodeCalls, c.measureCacheHits, c.textMeasureTotalMs) and insert the
frameCount entry in the list.
- Around line 13-39: The counter fields are duplicated between the
layoutCounters object and resetLayoutCounters, risking drift; create a single
source of truth (e.g., an initialLayoutCounters constant or a function
getInitialLayoutCounters()) and have resetLayoutCounters reuse it (for example
via Object.assign(layoutCounters, initialLayoutCounters) and then increment
layoutCounters.frameCount) instead of manually resetting each field; update
references to layoutCounters and resetLayoutCounters accordingly so
adding/removing counters only requires changing the initializer.

In `@packages/ink-compat/src/translation/propsToVNode.ts`:
- Around line 501-502: NATIVE_PERCENT_PROPS is being instantiated inside
translateBox() causing a new Set allocation on every call; move the declaration
const NATIVE_PERCENT_PROPS = new Set(["width","height","flexBasis"]) to module
scope (top of the file) so translateBox() and any other functions reuse the
single Set instance, leaving all uses of NATIVE_PERCENT_PROPS unchanged.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/core/src/drawlist/__tests__/builder.round-trip.test.ts (1)

381-398: ⚠️ Potential issue | 🟡 Minor

Rename stale v2 test names to match v1 assertions.

These test titles still say “v2” (and one says “version 2”), but the body intentionally validates ZR_DRAWLIST_VERSION_V1. Rename-only cleanup will prevent misleading CI failures.

Proposed rename-only diff
-  test("v2 header uses version 2 and correct cmd byte/count totals", () => {
+  test("v1 header uses version 1 and correct cmd byte/count totals", () => {
@@
-  test("v2 setCursor readback preserves payload fields and reserved byte", () => {
+  test("v1 setCursor readback preserves payload fields and reserved byte", () => {
@@
-  test("v2 multiple cursor commands are emitted in-order", () => {
+  test("v1 multiple cursor commands are emitted in-order", () => {
@@
-  test("v2 mixed frame keeps aligned sections and expected total byte size", () => {
+  test("v1 mixed frame keeps aligned sections and expected total byte size", () => {

Based on learnings: In the Rezi project, public API references and documentation must consistently use the current public protocol version (v1) after the reset.

Also applies to: 400-420, 422-463, 501-529

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/drawlist/__tests__/builder.round-trip.test.ts` around lines
381 - 398, Rename the stale test titles that say "v2" or "version 2" to reflect
v1 so they match the assertions (which check ZR_DRAWLIST_VERSION_V1); update the
test descriptions in the test cases around the assertions and helper calls
(e.g., the test currently named "v2 header uses version 2 and correct cmd
byte/count totals") to mention v1/version 1, and do the same for the other
affected tests referenced (lines ~400-420, 422-463, 501-529) so test names
accurately reflect the verified constant ZR_DRAWLIST_VERSION_V1.
packages/core/src/widgets/__tests__/basicWidgets.render.test.ts (1)

126-138: ⚠️ Potential issue | 🟠 Major

These test assertions will fail because the pre-alpha ABI reset unified v1 and v3 builders.

After commit ede8948 (drawlist: make v1 the only protocol), there is only a single createDrawlistBuilder() implementation. However, the test functions renderBytes() and renderBytesV3() both call the same unified builder, producing identical output. The tests at lines 555–565 and 567–616 expect different behavior:

  • Line 563–564 expect v3 to encode linkUriRef > 0 but v1 to have linkUriRef === 0
  • Line 606–614 expect v1 to have zero values for underlineStyle, underlineColorRgb, and linkUriRef

Both assertions cannot pass with a unified builder that encodes these fields in all cases. Either remove/rename the test functions to reflect that v1 is now the only protocol, or delete the renderBytesV3() helper entirely if it no longer serves a purpose.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/widgets/__tests__/basicWidgets.render.test.ts` around lines
126 - 138, renderBytesV3 duplicates renderBytes but the ABI now uses a single
unified createDrawlistBuilder(), so having a V3-specific helper causes tests to
expect differing outputs that no longer exist; remove the renderBytesV3 function
(and any references) or consolidate tests to use renderBytes only, and update
the assertions that expected v1/v3 differences (e.g., checks for linkUriRef,
underlineStyle, underlineColorRgb) so they match the unified builder behavior;
key symbols: renderBytes, renderBytesV3, createDrawlistBuilder, and the failing
assertions around linkUriRef/underlineStyle/underlineColorRgb.
♻️ Duplicate comments (4)
packages/core/src/widgets/toast.ts (1)

172-177: ⚠️ Potential issue | 🟠 Major

Replace hardcoded toast RGB literals with semantic theme tokens.

Line 174 through Line 177 still hardcode palette values, which bypasses theming and design-system overrides for toast semantics.

As per coding guidelines: "Never hardcode RGB color values; let the theme handle colors via tokens. Use variant props on badges, tags, callouts, status indicators for semantic colors. Use dsVariant/dsTone on buttons and interactive widgets for consistent theming."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/widgets/toast.ts` around lines 172 - 177, TOAST_COLORS
currently hardcodes RGB values (using rgb(...)) which bypasses theming; replace
these literals by mapping Toast["type"] to semantic theme tokens/variants
instead. Update the TOAST_COLORS definition so it references the design-system
theme tokens or a centralized color-variant lookup (e.g., use the project's
semantic token accessor or badge/tag variant mapping) for keys info, success,
warning, error, and remove direct rgb(...) calls; ensure the map still returns
Rgb24 values by converting tokens via the existing theme-to-Rgb24 helper (or the
same helper used elsewhere in widgets) so toast rendering uses theme-driven
colors.
packages/ink-compat/src/runtime/render.ts (1)

1157-1166: ⚠️ Potential issue | 🟠 Major

Treat 0x000000 as a valid color, not “unset”.

Line 1157/Line 1165, Line 1219/Line 1228, and Line 1289/Line 1290 currently treat 0 as absent. With packed Rgb24, 0 is valid black, so black fg/bg gets dropped during normalize/merge/SGR emission.

🔧 Proposed fix
-  if (isRgb24(style.fg) && style.fg !== 0) {
+  if (isRgb24(style.fg)) {
     const fg = clampRgb24(style.fg);
     if (!isSameRgb(fg, CORE_DEFAULT_FG)) {
       normalized.fg = fg;
     }
   }
-  if (isRgb24(style.bg) && style.bg !== 0) {
+  if (isRgb24(style.bg)) {
     const bg = clampRgb24(style.bg);
     if (!isSameRgb(bg, CORE_DEFAULT_BG)) {
       normalized.bg = bg;
     }
   }

-    if (style.fg) {
+    if (style.fg !== undefined) {
       ...
     }
-    if (style.bg) {
+    if (style.bg !== undefined) {
       ...
     }

   const merged: CellStyle = {};
   const bg = overlay.bg ?? base.bg;
   const fg = overlay.fg ?? base.fg;
-  if (bg) merged.bg = bg;
-  if (fg) merged.fg = fg;
+  if (bg !== undefined) merged.bg = bg;
+  if (fg !== undefined) merged.fg = fg;

-      if (cell.style?.bg)
+      if (cell.style?.bg !== undefined)
         entry["bg"] = `${rgbR(cell.style.bg)},${rgbG(cell.style.bg)},${rgbB(cell.style.bg)}`;
-      if (cell.style?.fg)
+      if (cell.style?.fg !== undefined)
         entry["fg"] = `${rgbR(cell.style.fg)},${rgbG(cell.style.fg)},${rgbB(cell.style.fg)}`;

Also applies to: 1219-1230, 1287-1290, 900-903

packages/ink-compat/src/runtime/createInkRenderer.ts (1)

524-530: ⚠️ Potential issue | 🟠 Major

Skipped frames are not traced in the fast path.

When trace is configured, Line 526 returns before any trace event is emitted, so layout-skipped frames disappear from metrics/diagnostics.

Suggested fix
     if (!isFirstFrame && !forceLayout && !prevRoot.dirty && cachedLayoutTree !== null) {
       const totalMs = performance.now() - t0;
+      if (trace) {
+        const textStartedAt = performance.now();
+        const screenText = opsToText(cachedOps, viewport);
+        const textMs = performance.now() - textStartedAt;
+        const opSummary = summarizeOps(cachedOps);
+        const nodeSummary = summarizeNodes(cachedNodes);
+        const textSummary = summarizeText(screenText);
+        trace({
+          renderId,
+          viewport,
+          focusedId: null,
+          tick: 0,
+          timings: { commitMs, layoutMs: 0, drawMs: 0, textMs, totalMs },
+          nodeCount: cachedNodes.length,
+          opCount: cachedOps.length,
+          opCounts: opSummary.opCounts,
+          clipDepthMax: opSummary.clipDepthMax,
+          textChars: textSummary.textChars,
+          textLines: textSummary.textLines,
+          nonBlankLines: textSummary.nonBlankLines,
+          widestLine: textSummary.widestLine,
+          minRectY: nodeSummary.minRectY,
+          maxRectBottom: nodeSummary.maxRectBottom,
+          zeroHeightRects: nodeSummary.zeroHeightRects,
+          detailIncluded: defaultTraceDetail,
+          layoutSkipped: true,
+          ...(defaultTraceDetail ? { nodes: cachedNodes, ops: cachedOps, text: screenText } : {}),
+        });
+      }
       return {
         ops: cachedOps,
         nodes: cachedNodes,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ink-compat/src/runtime/createInkRenderer.ts` around lines 524 - 530,
The fast-return path in createInkRenderer (the block checking isFirstFrame,
forceLayout, prevRoot.dirty, cachedLayoutTree) exits before emitting any trace
events, so frames where layout is skipped are missing from diagnostics; before
returning from that branch, call the configured trace API (e.g., trace(...) or
trace.event/traceFrame depending on your tracer) to record this frame with
timings (totalMs, commitMs, layoutMs: 0, drawMs: 0) and layoutSkipped: true,
using the same t0/performance.now() calculation and include any identifying
metadata (ops/nodes or frame id) so skipped frames appear in traces. Ensure you
guard the call with the existing trace variable (e.g., if (trace) ...) and do
not change the returned payload (ops, nodes, timings).
packages/core/src/runtime/commit.ts (1)

1366-1375: ⚠️ Potential issue | 🟡 Minor

Type assertion on detail is still incorrect.

The assignment detail: "children-changed" as "was-dirty" | undefined casts to a narrower type that doesn't include the actual value being assigned. The value "children-changed" is already a valid member of the detail union type and should be assigned directly.

♻️ Proposed fix
           __commitDiag.push({
             id: instanceId as number,
             kind: vnode.kind,
             reason: "fast-reuse",
-            detail: "children-changed" as "was-dirty" | undefined,
+            detail: "children-changed",
             childDiffs,
             prevChildren: prevChildren.length,
             nextChildren: (nextChildren as readonly RuntimeInstance[]).length,
           });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/runtime/commit.ts` around lines 1366 - 1375, The diagnostic
object pushed into __commitDiag uses an incorrect type assertion for the detail
field; replace the cast '"children-changed" as "was-dirty" | undefined' with a
direct assignment of the proper literal (detail: "children-changed") so the
value matches the declared union type, i.e., update the object literal created
in the __commitDiag.push call (referencing instanceId, vnode.kind, childDiffs,
prevChildren, nextChildren) to assign detail: "children-changed" without the
narrowing as-cast.
🧹 Nitpick comments (10)
packages/core/src/renderer/__tests__/renderer.scrollbar.test.ts (1)

508-508: Unnecessary Object.freeze() on primitive number.

The packed Rgb24 representation is correct: (1 << 16) | (2 << 8) | 3 produces 0x010203. However, Object.freeze() on a primitive number is a no-op since numbers are already immutable—it just returns the number unchanged.

🧹 Simplify by removing unnecessary freeze
-    const customFg = Object.freeze((1 << 16) | (2 << 8) | 3);
+    const customFg = (1 << 16) | (2 << 8) | 3;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/renderer/__tests__/renderer.scrollbar.test.ts` at line 508,
Remove the unnecessary Object.freeze call on the primitive number by changing
the declaration of customFg to a plain numeric assignment (replace const
customFg = Object.freeze((1 << 16) | (2 << 8) | 3); with const customFg = (1 <<
16) | (2 << 8) | 3;), keeping the packed Rgb24 expression intact and leaving the
symbol customFg unchanged elsewhere.
packages/core/src/layout/engine/layoutEngine.ts (1)

59-76: Profiling instrumentation looks good; consider tracking the cleanup.

The __layoutProfile export is well-structured with a clean reset mechanism and default-disabled state that ensures no production overhead. The __ prefix appropriately signals internal/temporary usage.

Since the comment explicitly marks this as temporary ("remove after investigation"), consider opening an issue to track the removal once the performance investigation concludes—this helps prevent temporary instrumentation from becoming permanent technical debt.

Would you like me to open an issue to track the removal of this profiling instrumentation once the investigation is complete?

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/layout/engine/layoutEngine.ts` around lines 59 - 76, Create
a tracker issue in the repo for removing this temporary profiling
instrumentation (mention file packages/core/src/layout/engine/layoutEngine.ts
and the exported symbol __layoutProfile) with a clear title and scope/acceptance
criteria (why it was added, how to validate removal). After creating the issue,
add a short TODO/FIXME comment directly above the __layoutProfile export that
references the new issue number (e.g., "TODO: remove profiling instrumentation —
see `#1234`") so future maintainers can find the task and reason quickly.
packages/core/src/widgets/heatmap.ts (1)

64-93: Consider validating monotonic stop ordering in buildScaleTable().

buildScaleTable() currently assumes stops are sorted by t. A lightweight guard (dev-time assert or early throw) would prevent silent gradient corruption if future scales are added incorrectly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/widgets/heatmap.ts` around lines 64 - 93, The
buildScaleTable function assumes the input stops array is sorted by stop.t and
within [0,1]; add a lightweight validation at the start of buildScaleTable that
checks each stop.t is a finite number in [0,1] and that stops[i].t <=
stops[i+1].t for all consecutive pairs (or throw/assert in dev builds). If
validation fails, throw an informative error (or dev-only assertion) that
references the offending index and values so callers can correct the stop
ordering or bounds; keep the rest of buildScaleTable logic unchanged.
packages/core/src/ui/recipes.ts (1)

40-42: Consider using hex literals for clarity in target color construction.

The bit-shift operations are correct but verbose. Hex literals would be more readable for these well-known color constants.

✨ Suggested simplification
 function adjustBrightness(color: Rgb24, amount: number): Rgb24 {
-  const target = amount > 0 ? (255 << 16) | (255 << 8) | 255 : (0 << 16) | (0 << 8) | 0;
+  const target = amount > 0 ? 0xffffff : 0x000000;
   return blendRgb(color, target, Math.abs(amount));
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/ui/recipes.ts` around lines 40 - 42, In adjustBrightness,
replace the verbose bit-shift construction of the target color with explicit hex
literals for white and black (use 0xFFFFFF for white and 0x000000 for black) so
the intent is clearer; update the ternary that currently builds target to return
those hex constants while keeping the rest of the function (blendRgb,
Math.abs(amount), and Rgb24 usage) unchanged.
packages/core/src/renderer/__tests__/renderer.border.test.ts (1)

537-537: Minor: Object.freeze on primitive is unnecessary.

Object.freeze has no effect on primitive numbers. The packed Rgb24 value is already immutable by nature.

Suggested simplification
-    const pressedFg = Object.freeze((255 << 16) | (64 << 8) | 64);
+    const pressedFg = (255 << 16) | (64 << 8) | 64;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/renderer/__tests__/renderer.border.test.ts` at line 537,
The test defines pressedFg using Object.freeze on a primitive number which is
unnecessary; change the declaration of pressedFg (the const pressedFg =
Object.freeze((255 << 16) | (64 << 8) | 64)) to simply compute and assign the
packed Rgb24 value without Object.freeze (e.g., const pressedFg = (255 << 16) |
(64 << 8) | 64) so the value remains the same while removing the redundant
freeze call in the renderer.border.test.ts test.
packages/core/src/renderer/renderToDrawlist/widgets/renderCanvasWidgets.ts (1)

360-360: Redundant fallback: hashImageBytes cannot return nullish.

Per the code snippet from packages/core/src/widgets/image.ts, hashImageBytes always returns hash >>> 0, which is a number. The ?? 0 fallback after hashImageBytes(analyzed.bytes) is defensive but unnecessary.

Suggested simplification
-      const imageId = readNonNegativeInt(props.imageId) ?? hashImageBytes(analyzed.bytes) ?? 0;
+      const imageId = readNonNegativeInt(props.imageId) ?? hashImageBytes(analyzed.bytes);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/renderer/renderToDrawlist/widgets/renderCanvasWidgets.ts`
at line 360, The current expression uses an unnecessary fallback "?? 0" because
hashImageBytes(analyzed.bytes) always returns a non-nullish number; update the
assignment for imageId to remove that redundant fallback by changing the line
using readNonNegativeInt(props.imageId) and hashImageBytes(analyzed.bytes) only
(e.g., const imageId = readNonNegativeInt(props.imageId) ??
hashImageBytes(analyzed.bytes);), referencing the imageId variable,
readNonNegativeInt, hashImageBytes, and analyzed.bytes to locate and simplify
the code.
packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts (1)

1-50: Rename file to remove v6 reference per protocol reset.

The filename builder_v6_resources.test.ts references the old v6 iteration. Based on learnings, after the protocol reset, public API references must consistently use the current public protocol version (v1) and must not reference v6.

Consider renaming to builder.resources.test.ts or builder_v1_resources.test.ts to align with the protocol reset naming convention.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts` around
lines 1 - 50, Rename the test file that references the old v6 protocol so it no
longer mentions v6: move builder_v6_resources.test.ts to a new name such as
builder.resources.test.ts (or builder_v1_resources.test.ts) and update any test
suite globs or exports if needed so the tests using createDrawlistBuilder,
expectOk, and the described tests continue to run under the new filename; ensure
no remaining references to "v6" remain in the test file or related test
configuration.
packages/ink-compat/src/runtime/createInkRenderer.ts (1)

453-467: Remove temporary unused debug helper before merge.

countSelfDirty is currently unused and explicitly marked temporary; keeping it here adds dead code in a hot-path module.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ink-compat/src/runtime/createInkRenderer.ts` around lines 453 - 467,
Remove the temporary debug helper function countSelfDirty from
createInkRenderer.ts: delete the entire function definition "countSelfDirty"
(including its declaration and body) and any imports/exports or references to it
so there are no dangling identifiers; after removal, run typecheck/build/tests
to ensure nothing else depended on countSelfDirty and fix any remaining
references (e.g., replace with existing public API or inline minimal logic where
actually required).
packages/core/src/runtime/commit.ts (1)

1783-1795: Remove temporary debug tracing before merge.

The comment explicitly marks this as temporary code to "remove after investigation." Debug instrumentation that reads from globalThis should not remain in production code.

♻️ Proposed removal
-    // Temporary debug: trace commit matching (remove after investigation)
-    const commitDebug = globalThis as Record<string, unknown> & {
-      __commitDebug?: unknown;
-      __commitDebugLog?: string[] | undefined;
-    };
-    if (commitDebug.__commitDebug) {
-      const debugLog = commitDebug.__commitDebugLog;
-      if (debugLog) {
-        debugLog.push(
-          `commitNode(${String(instanceId)}, ${vnode.kind}, prev=${prev ? `${prev.vnode.kind}:${String(prev.instanceId)}` : "null"})`,
-        );
-      }
-    }
-
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/runtime/commit.ts` around lines 1783 - 1795, Remove the
temporary global debug tracing block that was added for investigation: delete
the commitDebug declaration and the conditional that checks
commitDebug.__commitDebug and pushes to commitDebug.__commitDebugLog inside
commitNode (the code referencing commitDebug, __commitDebug, __commitDebugLog,
instanceId, vnode, and prev). Ensure no other parts of commitNode depend on
__commitDebug or __commitDebugLog; if any references remain, remove them too so
commitNode contains only production logic.
packages/core/src/widgets/__tests__/basicWidgets.render.test.ts (1)

555-565: Test name is now misleading and assertions are contradictory.

The test "link encodes hyperlink refs on v3 and degrades on v1" no longer reflects reality since both renderBytesV3 and renderBytes use the unified builder. Consider either:

  1. Removing the v1/v3 differentiation test since versioning is unified, or
  2. Renaming and updating assertions to reflect the new unified behavior
♻️ Option: Update test to reflect unified builder behavior
-  test("link encodes hyperlink refs on v3 and degrades on v1", () => {
-    const v3 = renderBytesV3(ui.link("https://example.com", "Docs"), { cols: 40, rows: 4 });
-    const v1 = renderBytes(ui.link("https://example.com", "Docs"), { cols: 40, rows: 4 });
-    assert.equal(parseOpcodes(v3).includes(8), false);
-    assert.equal(parseOpcodes(v1).includes(8), false);
-    assert.equal(parseInternedStrings(v3).includes("https://example.com"), true);
-    const v3Docs = parseDrawTextCommands(v3).find((cmd) => cmd.text === "Docs");
-    const v1Docs = parseDrawTextCommands(v1).find((cmd) => cmd.text === "Docs");
-    assert.equal((v3Docs?.linkUriRef ?? 0) > 0, true);
-    assert.equal(v1Docs?.linkUriRef ?? 0, 0);
+  test("link encodes hyperlink refs with unified builder", () => {
+    const bytes = renderBytes(ui.link("https://example.com", "Docs"), { cols: 40, rows: 4 });
+    assert.equal(parseOpcodes(bytes).includes(8), false);
+    assert.equal(parseInternedStrings(bytes).includes("https://example.com"), true);
+    const docsCmd = parseDrawTextCommands(bytes).find((cmd) => cmd.text === "Docs");
+    assert.equal((docsCmd?.linkUriRef ?? 0) > 0, true);
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/widgets/__tests__/basicWidgets.render.test.ts` around lines
555 - 565, The test "link encodes hyperlink refs on v3 and degrades on v1" is
outdated because renderBytesV3 and renderBytes now use a unified builder; rename
the test (e.g., "link encodes hyperlink refs with unified builder") and update
assertions to expect the unified behavior: both rendered outputs should intern
the URL (parseInternedStrings(...).includes("https://example.com") === true) and
both draw-text commands (from parseDrawTextCommands for each render) should have
linkUriRef > 0 (replace the v1Docs zero-check). Keep or adjust the parseOpcodes
assertions only if they remain meaningful for the unified builder.
🤖 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/bench/src/profile-packed-style.ts`:
- Around line 85-87: Replace the dot-accesses on the index-signature `counters`
with bracket notation and add an ESLint suppression for the dot-notation rule;
e.g. change occurrences like `const merges = counters.style_merges_performed ??
0;` to `// eslint-disable-next-line `@typescript-eslint/dot-notation`` followed by
`const merges = counters['style_merges_performed'] ?? 0;` (and do the same for
`style_objects_created` -> `['style_objects_created']` and `packRgb_calls` ->
`['packRgb_calls']`); this keeps TypeScript happy while silencing the linter
conflict (alternatively, update the `perfSnapshot()`/`counters` type to include
explicit properties if you prefer not to suppress).

In `@packages/core/src/widgets/canvas.ts`:
- Around line 76-83: The current truthy check incorrectly treats numeric color 0
(black) as absent; update the condition around parsedHex in the color parsing
branch so it accepts numeric 0 (e.g., check parsedHex !== null / !== undefined
or use typeof parsedHex === "number"), then return the frozen pixel using
rgbR(parsedHex), rgbG(parsedHex), rgbB(parsedHex), clampU8(...), and a:255
instead of falling back to DEFAULT_SOLID_PIXEL; keep the rest of the logic
(functions rgbR, rgbG, rgbB, clampU8) unchanged.

In `@packages/core/src/widgets/heatmap.ts`:
- Around line 8-44: The palette anchors in heatmap.ts (objects viridis, plasma,
inferno, magma, turbo, grayscale) are using hardcoded rgb(...) literals; replace
those inline RGB values with centralized theme/design tokens (e.g., import the
project's palette/token registry and reference token names or exported color
constants) so the heatmap uses semantic theme colors; keep the same t positions
and Object.freeze wrappers but swap each rgb(...) entry to the corresponding
token reference (or a small mapping that resolves token -> Rgb24) and update any
use of the rgb function in this file to accept token-based values.

In `@packages/ink-compat/src/runtime/createInkRenderer.ts`:
- Around line 506-531: Early-skip ignores viewport changes so a changed
renderOpts.viewport can return stale cachedOps/cachedNodes; add tracking of the
current normalized viewport (e.g., a closure variable lastViewport or
cachedViewport) and compare it to the new viewport from
normalizeViewport(renderOpts.viewport ?? defaultViewport) before taking the
early-skip path in the block using
isFirstFrame/forceLayout/prevRoot.dirty/cachedLayoutTree; if the viewports
differ, treat as not-skippable (invalidate cachedOps/cachedNodes or force
layout), and after a successful commit update the saved cachedViewport to the
new normalized viewport so future frames can correctly early-skip when viewport
is unchanged.

In `@scripts/guardrails.sh`:
- Around line 77-78: The guardrail in scripts/guardrails.sh is using a regex
that misses the actual import and rejects single-quote styles; update the
ripgrep check (the -e pattern used to scan builder.ts) to a quote-agnostic,
properly shell-escaped regex that matches import statements that end with
./writers.gen.js regardless of single or double quotes and that allows typical
import forms (e.g., named/default/side-effect imports); replace the current
pattern string in the rg invocation with this new pattern and ensure it is
passed to rg with -e and appropriate quoting/escaping so the shell and ripgrep
interpret backslashes and quotes correctly.

---

Outside diff comments:
In `@packages/core/src/drawlist/__tests__/builder.round-trip.test.ts`:
- Around line 381-398: Rename the stale test titles that say "v2" or "version 2"
to reflect v1 so they match the assertions (which check ZR_DRAWLIST_VERSION_V1);
update the test descriptions in the test cases around the assertions and helper
calls (e.g., the test currently named "v2 header uses version 2 and correct cmd
byte/count totals") to mention v1/version 1, and do the same for the other
affected tests referenced (lines ~400-420, 422-463, 501-529) so test names
accurately reflect the verified constant ZR_DRAWLIST_VERSION_V1.

In `@packages/core/src/widgets/__tests__/basicWidgets.render.test.ts`:
- Around line 126-138: renderBytesV3 duplicates renderBytes but the ABI now uses
a single unified createDrawlistBuilder(), so having a V3-specific helper causes
tests to expect differing outputs that no longer exist; remove the renderBytesV3
function (and any references) or consolidate tests to use renderBytes only, and
update the assertions that expected v1/v3 differences (e.g., checks for
linkUriRef, underlineStyle, underlineColorRgb) so they match the unified builder
behavior; key symbols: renderBytes, renderBytesV3, createDrawlistBuilder, and
the failing assertions around linkUriRef/underlineStyle/underlineColorRgb.

---

Duplicate comments:
In `@packages/core/src/runtime/commit.ts`:
- Around line 1366-1375: The diagnostic object pushed into __commitDiag uses an
incorrect type assertion for the detail field; replace the cast
'"children-changed" as "was-dirty" | undefined' with a direct assignment of the
proper literal (detail: "children-changed") so the value matches the declared
union type, i.e., update the object literal created in the __commitDiag.push
call (referencing instanceId, vnode.kind, childDiffs, prevChildren,
nextChildren) to assign detail: "children-changed" without the narrowing
as-cast.

In `@packages/core/src/widgets/toast.ts`:
- Around line 172-177: TOAST_COLORS currently hardcodes RGB values (using
rgb(...)) which bypasses theming; replace these literals by mapping
Toast["type"] to semantic theme tokens/variants instead. Update the TOAST_COLORS
definition so it references the design-system theme tokens or a centralized
color-variant lookup (e.g., use the project's semantic token accessor or
badge/tag variant mapping) for keys info, success, warning, error, and remove
direct rgb(...) calls; ensure the map still returns Rgb24 values by converting
tokens via the existing theme-to-Rgb24 helper (or the same helper used elsewhere
in widgets) so toast rendering uses theme-driven colors.

In `@packages/ink-compat/src/runtime/createInkRenderer.ts`:
- Around line 524-530: The fast-return path in createInkRenderer (the block
checking isFirstFrame, forceLayout, prevRoot.dirty, cachedLayoutTree) exits
before emitting any trace events, so frames where layout is skipped are missing
from diagnostics; before returning from that branch, call the configured trace
API (e.g., trace(...) or trace.event/traceFrame depending on your tracer) to
record this frame with timings (totalMs, commitMs, layoutMs: 0, drawMs: 0) and
layoutSkipped: true, using the same t0/performance.now() calculation and include
any identifying metadata (ops/nodes or frame id) so skipped frames appear in
traces. Ensure you guard the call with the existing trace variable (e.g., if
(trace) ...) and do not change the returned payload (ops, nodes, timings).

---

Nitpick comments:
In `@packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts`:
- Around line 1-50: Rename the test file that references the old v6 protocol so
it no longer mentions v6: move builder_v6_resources.test.ts to a new name such
as builder.resources.test.ts (or builder_v1_resources.test.ts) and update any
test suite globs or exports if needed so the tests using createDrawlistBuilder,
expectOk, and the described tests continue to run under the new filename; ensure
no remaining references to "v6" remain in the test file or related test
configuration.

In `@packages/core/src/layout/engine/layoutEngine.ts`:
- Around line 59-76: Create a tracker issue in the repo for removing this
temporary profiling instrumentation (mention file
packages/core/src/layout/engine/layoutEngine.ts and the exported symbol
__layoutProfile) with a clear title and scope/acceptance criteria (why it was
added, how to validate removal). After creating the issue, add a short
TODO/FIXME comment directly above the __layoutProfile export that references the
new issue number (e.g., "TODO: remove profiling instrumentation — see `#1234`") so
future maintainers can find the task and reason quickly.

In `@packages/core/src/renderer/__tests__/renderer.border.test.ts`:
- Line 537: The test defines pressedFg using Object.freeze on a primitive number
which is unnecessary; change the declaration of pressedFg (the const pressedFg =
Object.freeze((255 << 16) | (64 << 8) | 64)) to simply compute and assign the
packed Rgb24 value without Object.freeze (e.g., const pressedFg = (255 << 16) |
(64 << 8) | 64) so the value remains the same while removing the redundant
freeze call in the renderer.border.test.ts test.

In `@packages/core/src/renderer/__tests__/renderer.scrollbar.test.ts`:
- Line 508: Remove the unnecessary Object.freeze call on the primitive number by
changing the declaration of customFg to a plain numeric assignment (replace
const customFg = Object.freeze((1 << 16) | (2 << 8) | 3); with const customFg =
(1 << 16) | (2 << 8) | 3;), keeping the packed Rgb24 expression intact and
leaving the symbol customFg unchanged elsewhere.

In `@packages/core/src/renderer/renderToDrawlist/widgets/renderCanvasWidgets.ts`:
- Line 360: The current expression uses an unnecessary fallback "?? 0" because
hashImageBytes(analyzed.bytes) always returns a non-nullish number; update the
assignment for imageId to remove that redundant fallback by changing the line
using readNonNegativeInt(props.imageId) and hashImageBytes(analyzed.bytes) only
(e.g., const imageId = readNonNegativeInt(props.imageId) ??
hashImageBytes(analyzed.bytes);), referencing the imageId variable,
readNonNegativeInt, hashImageBytes, and analyzed.bytes to locate and simplify
the code.

In `@packages/core/src/runtime/commit.ts`:
- Around line 1783-1795: Remove the temporary global debug tracing block that
was added for investigation: delete the commitDebug declaration and the
conditional that checks commitDebug.__commitDebug and pushes to
commitDebug.__commitDebugLog inside commitNode (the code referencing
commitDebug, __commitDebug, __commitDebugLog, instanceId, vnode, and prev).
Ensure no other parts of commitNode depend on __commitDebug or __commitDebugLog;
if any references remain, remove them too so commitNode contains only production
logic.

In `@packages/core/src/ui/recipes.ts`:
- Around line 40-42: In adjustBrightness, replace the verbose bit-shift
construction of the target color with explicit hex literals for white and black
(use 0xFFFFFF for white and 0x000000 for black) so the intent is clearer; update
the ternary that currently builds target to return those hex constants while
keeping the rest of the function (blendRgb, Math.abs(amount), and Rgb24 usage)
unchanged.

In `@packages/core/src/widgets/__tests__/basicWidgets.render.test.ts`:
- Around line 555-565: The test "link encodes hyperlink refs on v3 and degrades
on v1" is outdated because renderBytesV3 and renderBytes now use a unified
builder; rename the test (e.g., "link encodes hyperlink refs with unified
builder") and update assertions to expect the unified behavior: both rendered
outputs should intern the URL
(parseInternedStrings(...).includes("https://example.com") === true) and both
draw-text commands (from parseDrawTextCommands for each render) should have
linkUriRef > 0 (replace the v1Docs zero-check). Keep or adjust the parseOpcodes
assertions only if they remain meaningful for the unified builder.

In `@packages/core/src/widgets/heatmap.ts`:
- Around line 64-93: The buildScaleTable function assumes the input stops array
is sorted by stop.t and within [0,1]; add a lightweight validation at the start
of buildScaleTable that checks each stop.t is a finite number in [0,1] and that
stops[i].t <= stops[i+1].t for all consecutive pairs (or throw/assert in dev
builds). If validation fails, throw an informative error (or dev-only assertion)
that references the offending index and values so callers can correct the stop
ordering or bounds; keep the rest of buildScaleTable logic unchanged.

In `@packages/ink-compat/src/runtime/createInkRenderer.ts`:
- Around line 453-467: Remove the temporary debug helper function countSelfDirty
from createInkRenderer.ts: delete the entire function definition
"countSelfDirty" (including its declaration and body) and any imports/exports or
references to it so there are no dangling identifiers; after removal, run
typecheck/build/tests to ensure nothing else depended on countSelfDirty and fix
any remaining references (e.g., replace with existing public API or inline
minimal logic where actually required).

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25f82d5 and a559378.

📒 Files selected for processing (66)
  • packages/bench/src/profile-packed-style.ts
  • packages/core/src/animation/__tests__/interpolate.easing.test.ts
  • packages/core/src/animation/interpolate.ts
  • packages/core/src/app/__tests__/partialDrawlistEmission.test.ts
  • packages/core/src/drawlist/__tests__/builder.golden.test.ts
  • packages/core/src/drawlist/__tests__/builder.graphics.test.ts
  • packages/core/src/drawlist/__tests__/builder.round-trip.test.ts
  • packages/core/src/drawlist/__tests__/builder.string-cache.test.ts
  • packages/core/src/drawlist/__tests__/builder.string-intern.test.ts
  • packages/core/src/drawlist/__tests__/builder.style-encoding.test.ts
  • packages/core/src/drawlist/__tests__/builder.text-run.test.ts
  • packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts
  • packages/core/src/drawlist/builder.ts
  • packages/core/src/layout/__tests__/layout.edgecases.test.ts
  • packages/core/src/layout/engine/layoutEngine.ts
  • packages/core/src/renderer/__tests__/focusIndicators.test.ts
  • packages/core/src/renderer/__tests__/recipeRendering.test-utils.ts
  • packages/core/src/renderer/__tests__/renderer.border.test.ts
  • packages/core/src/renderer/__tests__/renderer.damage.test.ts
  • packages/core/src/renderer/__tests__/renderer.scrollbar.test.ts
  • packages/core/src/renderer/__tests__/tableRecipeRendering.test.ts
  • packages/core/src/renderer/__tests__/textStyle.opacity.test.ts
  • packages/core/src/renderer/renderToDrawlist/textStyle.ts
  • packages/core/src/renderer/renderToDrawlist/widgets/editors.ts
  • packages/core/src/renderer/renderToDrawlist/widgets/renderCanvasWidgets.ts
  • packages/core/src/renderer/shadow.ts
  • packages/core/src/renderer/styles.ts
  • packages/core/src/runtime/__tests__/commit.fastReuse.regression.test.ts
  • packages/core/src/runtime/__tests__/hooks.useTheme.test.ts
  • packages/core/src/runtime/commit.ts
  • packages/core/src/theme/__tests__/theme.contrast.test.ts
  • packages/core/src/theme/__tests__/theme.extend.test.ts
  • packages/core/src/theme/__tests__/theme.interop.test.ts
  • packages/core/src/theme/__tests__/theme.resolution.test.ts
  • packages/core/src/theme/__tests__/theme.switch.test.ts
  • packages/core/src/theme/__tests__/theme.test.ts
  • packages/core/src/theme/__tests__/theme.transition.test.ts
  • packages/core/src/theme/blend.ts
  • packages/core/src/theme/contrast.ts
  • packages/core/src/theme/defaultTheme.ts
  • packages/core/src/theme/tokens.ts
  • packages/core/src/ui/__tests__/themed.test.ts
  • packages/core/src/ui/recipes.ts
  • packages/core/src/widgets/__tests__/basicWidgets.render.test.ts
  • packages/core/src/widgets/__tests__/canvas.primitives.test.ts
  • packages/core/src/widgets/__tests__/collections.test.ts
  • packages/core/src/widgets/__tests__/containers.test.ts
  • packages/core/src/widgets/__tests__/graphics.golden.test.ts
  • packages/core/src/widgets/__tests__/overlays.test.ts
  • packages/core/src/widgets/__tests__/overlays.typecheck.ts
  • packages/core/src/widgets/__tests__/renderer.regressions.test.ts
  • packages/core/src/widgets/__tests__/style.attributes.test.ts
  • packages/core/src/widgets/__tests__/style.inheritance.test.ts
  • packages/core/src/widgets/__tests__/style.merge-fuzz.test.ts
  • packages/core/src/widgets/__tests__/style.merge.test.ts
  • packages/core/src/widgets/__tests__/style.utils.test.ts
  • packages/core/src/widgets/__tests__/styleUtils.test.ts
  • packages/core/src/widgets/__tests__/styled.test.ts
  • packages/core/src/widgets/__tests__/table.typecheck.ts
  • packages/core/src/widgets/canvas.ts
  • packages/core/src/widgets/heatmap.ts
  • packages/core/src/widgets/logsConsole.ts
  • packages/core/src/widgets/toast.ts
  • packages/ink-compat/src/runtime/createInkRenderer.ts
  • packages/ink-compat/src/runtime/render.ts
  • scripts/guardrails.sh
✅ Files skipped from review due to trivial changes (1)
  • packages/core/src/layout/tests/layout.edgecases.test.ts
🚧 Files skipped from review as they are similar to previous changes (25)
  • packages/core/src/widgets/tests/styled.test.ts
  • packages/core/src/renderer/tests/textStyle.opacity.test.ts
  • packages/core/src/widgets/tests/overlays.test.ts
  • packages/core/src/theme/tests/theme.transition.test.ts
  • packages/core/src/widgets/logsConsole.ts
  • packages/core/src/drawlist/tests/builder.string-cache.test.ts
  • packages/core/src/widgets/tests/styleUtils.test.ts
  • packages/core/src/theme/tests/theme.contrast.test.ts
  • packages/core/src/runtime/tests/hooks.useTheme.test.ts
  • packages/core/src/widgets/tests/style.merge-fuzz.test.ts
  • packages/core/src/drawlist/tests/builder.graphics.test.ts
  • packages/core/src/animation/tests/interpolate.easing.test.ts
  • packages/core/src/widgets/tests/style.merge.test.ts
  • packages/core/src/widgets/tests/renderer.regressions.test.ts
  • packages/core/src/renderer/tests/focusIndicators.test.ts
  • packages/core/src/drawlist/tests/builder.golden.test.ts
  • packages/core/src/widgets/tests/table.typecheck.ts
  • packages/core/src/theme/tests/theme.resolution.test.ts
  • packages/core/src/drawlist/tests/builder.text-run.test.ts
  • packages/core/src/theme/tests/theme.test.ts
  • packages/core/src/renderer/tests/tableRecipeRendering.test.ts
  • packages/core/src/widgets/tests/containers.test.ts
  • packages/core/src/renderer/shadow.ts
  • packages/core/src/runtime/tests/commit.fastReuse.regression.test.ts
  • packages/core/src/widgets/tests/overlays.typecheck.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant