Fix starship render regressions and document autonomous PTY debugging#224
Closed
RtlZeroMemory wants to merge 22 commits intomainfrom
Closed
Fix starship render regressions and document autonomous PTY debugging#224RtlZeroMemory wants to merge 22 commits intomainfrom
RtlZeroMemory wants to merge 22 commits intomainfrom
Conversation
- 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>
…trix" This reverts commit 183532e.
…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
…ized-renderer # Conflicts: # packages/core/src/__tests__/integration/integration.dashboard.test.ts # packages/core/src/__tests__/integration/integration.file-manager.test.ts # packages/core/src/__tests__/integration/integration.form-editor.test.ts # packages/core/src/__tests__/integration/integration.reflow.test.ts # packages/core/src/__tests__/integration/integration.resize.test.ts # packages/core/src/__tests__/stress/stress.large-trees.test.ts # packages/core/src/abi.ts # packages/core/src/app/__tests__/partialDrawlistEmission.test.ts # packages/core/src/app/__tests__/rawRender.test.ts # packages/core/src/app/__tests__/widgetRenderer.integration.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.alignment.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.limits.test.ts # packages/core/src/drawlist/__tests__/builder.reset.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.validate-caps.test.ts # packages/core/src/drawlist/__tests__/builder_style_attrs.test.ts # packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts # packages/core/src/drawlist/builder.ts # packages/core/src/drawlist/builderBase.ts # packages/core/src/drawlist/index.ts # packages/core/src/drawlist/types.ts # packages/core/src/index.ts # packages/core/src/renderer/__tests__/focusIndicators.test.ts # packages/core/src/renderer/__tests__/overlay.edge.test.ts # packages/core/src/renderer/__tests__/persistentBlobKeys.test.ts # packages/core/src/renderer/__tests__/recipeRendering.test-utils.ts # packages/core/src/renderer/__tests__/render.golden.test.ts # packages/core/src/renderer/__tests__/renderer.border.test.ts # packages/core/src/renderer/__tests__/renderer.clip.test.ts # packages/core/src/renderer/__tests__/renderer.damage.test.ts # packages/core/src/renderer/__tests__/renderer.partial.perf.test.ts # packages/core/src/renderer/__tests__/renderer.scrollbar.test.ts # packages/core/src/renderer/__tests__/renderer.text.test.ts # packages/core/src/renderer/renderToDrawlist/renderTree.ts # packages/core/src/renderer/renderToDrawlist/widgets/renderCanvasWidgets.ts # packages/core/src/testing/renderer.ts # packages/core/src/theme/__tests__/theme.extend.test.ts # packages/core/src/theme/__tests__/theme.switch.test.ts # packages/core/src/ui/__tests__/themed.test.ts # packages/core/src/widgets/__tests__/basicWidgets.render.test.ts # packages/core/src/widgets/__tests__/graphics.golden.test.ts # packages/core/src/widgets/__tests__/graphicsWidgets.test.ts # packages/core/src/widgets/__tests__/renderer.regressions.test.ts # packages/core/src/widgets/__tests__/style.utils.test.ts # packages/core/src/widgets/__tests__/styleUtils.test.ts # packages/core/src/widgets/__tests__/widgetRenderSmoke.test.ts # packages/ink-compat/src/runtime/createInkRenderer.ts # packages/native/vendor/VENDOR_COMMIT.txt # packages/native/vendor/zireael/include/zr/zr_drawlist.h # packages/native/vendor/zireael/src/core/zr_drawlist.c # packages/native/vendor/zireael/src/core/zr_engine.c # packages/native/vendor/zireael/src/core/zr_engine_present.inc # packages/node/src/backend/nodeBackend.ts # packages/testkit/fixtures/zrdl-v1/golden/draw_text_interned.bin
📝 WalkthroughWalkthroughThis pull request introduces comprehensive framework enhancements including a frame auditing system for diagnostics, native vendor integrity validation, drawlist format upgrades with string/blob definitions and link references, new regression dashboard example, native Zireael vendor updates, and extensive renderer/layout refinements. Changes span documentation, CI automation, core rendering, native FFI, and template updates. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App/WidgetRenderer
participant Audit as Frame Audit
participant Builder as DrawlistBuilder
participant Backend as NodeBackend
participant Worker as EngineWorker
App->>Builder: planResourceStream()
Builder-->>App: plan (strings/blobs/cmds)
App->>Builder: writeBuiltStream(buffer, plan)
Builder-->>App: drawlist bytes
App->>Audit: drawlistFingerprint(bytes)
Audit-->>App: hash32, opcode histogram
App->>Audit: emitFrameAudit("drawlist.built", {...fingerprint})
Audit-->>Audit: log to NDJSON file
App->>Backend: beginFrame() or submitFrame(bytes)
Backend->>Worker: publish frame via SAB
Worker->>Audit: emitFrameAudit("frame.received", {...})
Audit-->>Audit: log per-frame metadata
Worker->>Audit: drawlistFingerprint(received bytes)
Audit-->>Worker: fingerprint
Worker-->>Backend: frame.accepted ACK
Backend-->>App: inFlight promise resolves
Worker->>Audit: emitFrameAudit("frame.completed", {...})
Audit-->>Audit: finalize frame audit
sequenceDiagram
participant CI as CI Workflow
participant Script as check-native-vendor-integrity
participant FS as Filesystem
participant Git as Git
CI->>Script: npm run check:native-vendor
Script->>FS: read packages/native/vendor/VENDOR_COMMIT.txt
FS-->>Script: pinned commit hash
Script->>Git: git rev-parse HEAD:vendor/zireael
Git-->>Script: gitlink commit hash
Script->>FS: read packages/native/build.rs
FS-->>Script: build.rs content
alt pinned == gitlink
Script->>Script: validate build.rs paths
Script-->>CI: ✓ OK + commit short hash
else pinned != gitlink
Script->>Script: collect mismatch details
Script-->>CI: ✗ FAIL + error message
CI->>CI: exit code 1
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d55b8a4e1b
ℹ️ 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".
Comment on lines
757
to
+758
| await app.start(); | ||
| await new Promise<void>(() => {}); | ||
| await stopPromise; |
There was a problem hiding this comment.
A SIGINT/SIGTERM can call stopApp() before startup finishes, which resolves stopPromise and sets stopping=true, but this bootstrap path still unconditionally executes await app.start() afterward. In that sequence, the app can be started after shutdown was requested, and the stopping guard prevents a second shutdown path, leaving backend resources running and the process hanging instead of exiting cleanly. Add a guard/race so startup is skipped once stopping is set.
Useful? React with 👍 / 👎.
Comment on lines
745
to
747
| DrawlistBuilder, | ||
| DrawlistTextPerfCounters, | ||
| DrawlistImageFit, | ||
| DrawlistImageFormat, |
There was a problem hiding this comment.
DrawlistTextPerfCounters is still defined and exported from packages/core/src/drawlist/index.ts, but it was removed from the top-level @rezi-ui/core type export list here. That is a source-compatible API regression: downstream code importing this type from @rezi-ui/core will now fail TypeScript builds after this commit.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/create-rezi/templates/starship/src/main.ts (1)
734-739:⚠️ Potential issue | 🟠 MajorPrevent start-after-stop race during bootstrap.
A signal can call
stopApp()before startup completes, but bootstrap still proceeds intoapp.start(). That can re-enter runtime after shutdown was already requested.🛠️ Proposed fix
await setupDebugTrace(); -await app.start(); -await stopPromise; +if (!stopping) { + await app.start(); + await stopPromise; +} if (stopCode !== 0) { process.exitCode = stopCode; }Based on learnings: all changes must follow
docs/dev/code-standards.md, including async cancellation guards.Also applies to: 756-761
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/create-rezi/templates/starship/src/main.ts` around lines 734 - 739, The onSignal handler can call stopApp() while bootstrap is still running, allowing bootstrap to continue into app.start() after shutdown was requested; modify bootstrap logic so startup checks a cancellation flag or promise set by onSignal/stopApp before and immediately after awaiting any async initialization, and refuse to call app.start() (or abort startup) if shutdownRequested is true; update onSignal/stopApp to set this shared shutdownRequested flag (or resolve a cancellation promise) and ensure app.start() is only invoked when the flag is false (apply same guard to the other startup lines referenced by symbols onSignal, stopApp, and app.start).packages/node/src/worker/engineWorker.ts (1)
1573-1583:⚠️ Potential issue | 🟡 MinorAvoid clearing audit-enabled state when disable fails.
At Line [1582],
nativeFrameAuditEnabledis set tofalseeven ifengineDebugDisablereturns an error code. That can desync worker-side drain behavior from native state.🔧 Proposed fix
- nativeFrameAuditEnabled = false; + if (rc >= 0) { + nativeFrameAuditEnabled = false; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/node/src/worker/engineWorker.ts` around lines 1573 - 1583, The code unconditionally clears nativeFrameAuditEnabled after calling native.engineDebugDisable even when that call failed (rc != 0), which can desync worker vs native state; in the "debug:disable" case (around engineDebugDisable, rc, engineId, nativeFrameAuditEnabled, frameAudit.enabled) only set nativeFrameAuditEnabled = false when engineDebugDisable returned success (e.g., rc === 0), otherwise leave the flag untouched (and preserve existing error handling/return path).packages/core/src/renderer/renderToDrawlist/widgets/collections.ts (1)
1-31:⚠️ Potential issue | 🟡 MinorFix import ordering to unblock CI.
Current import ordering in this block trips the pipeline import-sort check; please reorder this new perf import to match the project’s expected sort/grouping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/renderer/renderToDrawlist/widgets/collections.ts` around lines 1 - 31, The import of emitFrameAudit and FRAME_AUDIT_ENABLED is misordered and fails the import-sort check; move the "import { emitFrameAudit, FRAME_AUDIT_ENABLED } from '../../../perf/frameAudit.js';" line into the main block of local/project imports so it is grouped with the other "../../../..." imports and alphabetically ordered with them (place it among the other non-type imports such as distributeColumnWidths, tableRecipe, treeRecipe, and the widgets/virtualList imports) to satisfy the project's import ordering rules.
🧹 Nitpick comments (14)
packages/core/src/runtime/commit.ts (1)
1337-1341: Gate parent VNode rewrites on child dirtiness to preserve the no-churn fast path.Good fix for stale child wiring. On Line 1340 and Line 1341, the
map(...)+ rewrite now runs every fast-reuse cycle, even when no child changed.♻️ Proposed tweak
- const fastReuseCommittedChildren = prev.children.map((child) => child.vnode); - (prev as { vnode: VNode }).vnode = rewriteCommittedVNode(vnode, fastReuseCommittedChildren); + const childDirty = hasDirtyChild(prev.children); + if (childDirty) { + const fastReuseCommittedChildren = prev.children.map((child) => child.vnode); + (prev as { vnode: VNode }).vnode = rewriteCommittedVNode(vnode, fastReuseCommittedChildren); + } @@ - prev.dirty = hasDirtyChild(prev.children); + prev.dirty = childDirty;🤖 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 1337 - 1341, The rewrite of the parent VNode’s committed children runs every fast-reuse cycle and should be skipped when none of the child wiring changed; guard the work by checking child dirtiness or reference differences before mapping and calling rewriteCommittedVNode. Concretely: in the fast-reuse path around prev.children.map(...) and rewriteCommittedVNode(vnode,...), first test whether any child is dirty or whether prev.children’ committed vnode references differ from the current vnode.children (e.g. prev.children.some(c => c.dirty) || !prev.children.every((c,i) => c.vnode === vnode.children[i])); only if that predicate is true compute fastReuseCommittedChildren and call rewriteCommittedVNode. Ensure you reference the existing symbols prev, prev.children, vnode, fastReuseCommittedChildren, and rewriteCommittedVNode when making the change.packages/core/src/renderer/renderToDrawlist/overflowCulling.ts (1)
90-109: Add focused regression tests for wrapper traversal/culling edge casesThis traversal is in renderer culling logic and uses early exits plus wrapper-gated descent. Please add tests for: pass-through
layerframe styles,focusZone/focusTrap/themed/layerswrapper chains, and shadow object defaults ({}vs explicit zeros).Based on learnings: Changes to these danger zones require extra care due to risk of subtle regressions affecting all rendering:
packages/core/src/renderer/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/renderer/renderToDrawlist/overflowCulling.ts` around lines 90 - 109, Add focused regression tests exercising subtreeCanOverflowBounds traversal: create scenarios that include pass-through "layer" frame styles, wrapper chains (focusZone, focusTrap, themed, layers) to verify isTransparentOverflowWrapper correctly gates descent, and cases where boxShadow is {} versus explicit zero values to ensure hasBoxShadowOverflow treats defaults and zeros equivalently; confirm that hasVisibleOverflow and hasBoxShadowOverflow return true for visible/shadowing children and that traversal still short-circuits on those checks while continuing descent only through transparent wrappers.packages/core/src/widgets/__tests__/style.inheritance.test.ts (1)
35-47: Consider replacing bit-shift literals with named attr constants.The helper is correct, but named constants would make the bit mapping easier to audit and safer to maintain.
Refactor sketch
function computeExpectedAttrs(style: Readonly<TextStyle>): number { + const ATTR_BOLD = 1 << 0; + const ATTR_ITALIC = 1 << 1; + const ATTR_UNDERLINE = 1 << 2; + const ATTR_INVERSE = 1 << 3; + const ATTR_DIM = 1 << 4; + const ATTR_STRIKETHROUGH = 1 << 5; + const ATTR_OVERLINE = 1 << 6; + const ATTR_BLINK = 1 << 7; let attrs = 0; - if (style.bold) attrs |= 1 << 0; - if (style.italic) attrs |= 1 << 1; + if (style.bold) attrs |= ATTR_BOLD; + if (style.italic) attrs |= ATTR_ITALIC; if (style.underline || (style.underlineStyle !== undefined && style.underlineStyle !== "none")) { - attrs |= 1 << 2; + attrs |= ATTR_UNDERLINE; } - if (style.inverse) attrs |= 1 << 3; - if (style.dim) attrs |= 1 << 4; - if (style.strikethrough) attrs |= 1 << 5; - if (style.overline) attrs |= 1 << 6; - if (style.blink) attrs |= 1 << 7; + if (style.inverse) attrs |= ATTR_INVERSE; + if (style.dim) attrs |= ATTR_DIM; + if (style.strikethrough) attrs |= ATTR_STRIKETHROUGH; + if (style.overline) attrs |= ATTR_OVERLINE; + if (style.blink) attrs |= ATTR_BLINK; return attrs >>> 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/widgets/__tests__/style.inheritance.test.ts` around lines 35 - 47, Replace the magic bit-shift literals in computeExpectedAttrs with named constants (e.g., ATTR_BOLD = 1<<0, ATTR_ITALIC = 1<<1, ATTR_UNDERLINE = 1<<2, etc.) declared near the top of the test file, then use those constants inside computeExpectedAttrs instead of the raw 1<<n values so the mapping (bold/italic/underline/inverse/dim/strikethrough/overline/blink) is explicit and easier to audit/maintain; ensure the final return still uses unsigned shift (attrs >>> 0) unchanged.packages/native/src/lib.rs (3)
1709-1731: Inconsistent indentation in test helper.Similar to the struct definitions, this initialization block uses tab characters. Consider normalizing to spaces for consistency with the rest of the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/native/src/lib.rs` around lines 1709 - 1731, The test helper uses tab characters in the struct initialization blocks (e.g., the caps assignment for ffi::plat_caps_t, the limits retrieval via ffi::zr_engine_config_default(), and the initial_term_state for ffi::zr_term_state_t); update those lines to use spaces (matching the file's existing indentation style) so the caps, limits, and initial_term_state initializers are consistently space-indented with the surrounding code.
116-155: Inconsistent indentation: tabs mixed with spaces.These struct definitions use tab characters for indentation while the rest of the file uses spaces. This inconsistency affects readability and may cause issues with automated formatting tools.
🔧 Suggested fix: normalize to spaces
- #[repr(C)] - #[derive(Copy, Clone)] - pub struct zr_terminal_caps_t { + #[repr(C)] + #[derive(Copy, Clone)] + pub struct zr_terminal_caps_t { pub color_mode: u8, ... - } - - #[repr(C)] - #[derive(Copy, Clone)] - pub struct plat_caps_t { - pub color_mode: u8, + } + + #[repr(C)] + #[derive(Copy, Clone)] + pub struct plat_caps_t { + pub color_mode: u8, ... - } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/native/src/lib.rs` around lines 116 - 155, The indentation in the struct definitions zr_terminal_caps_t and plat_caps_t uses tab characters mixed with spaces; replace all leading tab characters inside these struct blocks with the project's standard spaces indentation to match the rest of the file (align field names and types consistently), and reformat the file (e.g., run rustfmt) to ensure consistent spacing throughout.
1855-1858: Hardcoded constant may drift from vendored headers.
LINK_ENTRY_MAX_BYTESappears to derive fromZR_FB_LINK_URI_MAX_BYTES + ZR_FB_LINK_ID_MAX_BYTES(2083 + 2083). If the vendored header values change, this test's assertions could become misleading without any compile-time warning.Consider adding a comment documenting the derivation, or if feasible, importing the constants via FFI to keep them synchronized.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/native/src/lib.rs` around lines 1855 - 1858, The test fb_link_intern_compacts_stale_refs_and_bounds_growth uses a hardcoded LINK_ENTRY_MAX_BYTES (2083 + 2083) that mirrors ZR_FB_LINK_URI_MAX_BYTES + ZR_FB_LINK_ID_MAX_BYTES and may drift from vendored headers; update the test by either (a) adding a clear comment on LINK_ENTRY_MAX_BYTES documenting its derivation from ZR_FB_LINK_URI_MAX_BYTES and ZR_FB_LINK_ID_MAX_BYTES and why those specific values are used, or (b) preferably import the two upstream constants via FFI and compute LINK_ENTRY_MAX_BYTES from them inside the test (referencing the test function name and the LINK_ENTRY_MAX_BYTES symbol) so the values stay synchronized with the vendored headers.packages/core/src/renderer/renderToDrawlist/widgets/renderCanvasWidgets.ts (1)
225-227: Function nameaddBlobAlignedcreates minor semantic confusion about when alignment occurs.While the function does result in 4-byte aligned blobs (via
writeDefBlob'salign4()call during serialization), the name suggests alignment happens at the addition stage. Alignment actually occurs during drawlist finalization. This is correct behavior, but renaming toaddBlobfor clarity would be reasonable since the API contract is maintained at the serialization layer, or document that "Aligned" refers to the final serialized form.🤖 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` around lines 225 - 227, Rename the confusing addBlobAligned API to addBlob (and update any imports/uses) because alignment actually happens during serialization via writeDefBlob().align4(), not at add time; update the export and all call sites to use addBlob, and optionally add a brief comment in renderCanvasWidgets.ts near the new addBlob function clarifying that 4-byte alignment is applied during drawlist finalization/serialization (writeDefBlob.align4()) rather than at addition.packages/create-rezi/templates/starship/src/screens/engineering.ts (1)
394-397: Extract min-height offsets into named constants.The arithmetic works, but the inline
12/10/2values are hard to tune later.♻️ Proposed refactor
+ const CONTROL_ROWS_WITH_SUMMARY = 12; + const CONTROL_ROWS_BASE = 10; + const SINGLE_PANE_OFFSET = 2; + const responsiveDeckMinHeight = Math.max( 16, - contentRows - (showControlsSummary ? 12 : 10) - (showSecondaryPanels ? 0 : 2), + contentRows - + (showControlsSummary ? CONTROL_ROWS_WITH_SUMMARY : CONTROL_ROWS_BASE) - + (showSecondaryPanels ? 0 : SINGLE_PANE_OFFSET), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/create-rezi/templates/starship/src/screens/engineering.ts` around lines 394 - 397, The inline magic numbers in the responsiveDeckMinHeight calculation (the 12, 10 and 2) should be extracted into named constants for clarity and easier tuning: define e.g. CONTROLS_SUMMARY_OFFSET (12), CONTROLS_OFFSET (10) and SECONDARY_PANELS_OFFSET (2) near the top of the module, then update the responsiveDeckMinHeight expression to use these constants with contentRows, showControlsSummary and showSecondaryPanels so the formula reads self-documenting and the offsets can be tuned without changing the arithmetic.packages/core/src/renderer/__tests__/persistentBlobKeys.test.ts (1)
89-94: Consider adding thenullallocation branch test foraddBlobAligned.Current coverage validates the success path well; a failure-path assertion would lock expected behavior when
addBlob(...)returnsnull.Optional follow-up test
@@ test("addBlobAligned forwards exact payload length", () => { const builder = new CountingBuilder(); const blobId = addBlobAligned(builder, new Uint8Array([1, 2, 3])); assert.equal(blobId, 0); assert.deepEqual(builder.blobByteLens, [3]); }); + + test("addBlobAligned propagates null when blob allocation fails", () => { + class NullBlobBuilder extends CountingBuilder { + addBlob(_bytes: Uint8Array): number | null { + return null; + } + } + const builder = new NullBlobBuilder(); + const blobId = addBlobAligned(builder, new Uint8Array([1, 2, 3])); + assert.equal(blobId, null); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/renderer/__tests__/persistentBlobKeys.test.ts` around lines 89 - 94, Add a test that covers the "null allocation" branch of addBlobAligned by creating or mocking a CountingBuilder whose addBlob method returns null, then call addBlobAligned(builder, new Uint8Array([1,2,3])) and assert the function returns null (or the documented failure value) and that builder.blobByteLens was not changed; reference addBlobAligned, addBlob, and CountingBuilder in the test to locate and implement the behavior check.packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts (1)
4-6: Prefer reusing sharedu32test utility to avoid drift.You already have
u32inpackages/core/src/__tests__/drawlistDecode.ts; importing it here would reduce duplicate parsing helpers.🤖 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 4 - 6, Replace the duplicated u32 implementation in builder_v6_resources.test.ts by importing and reusing the shared u32 test helper from the drawlistDecode test module; remove the local function u32 (the one that constructs a DataView and calls getUint32) and add an import for the exported u32 helper from the drawlistDecode test module so all tests use the same parsing utility.packages/core/src/app/createApp.ts (1)
596-603: Remove redundant drawlist-version check after marker parsing.
readBackendDrawlistVersionMarker()already throws for non-1values and only returns1 | null, so this branch is now unreachable.♻️ Proposed cleanup
const backendDrawlistVersion = readBackendDrawlistVersionMarker(backend); - if (backendDrawlistVersion !== null && backendDrawlistVersion !== 1) { - invalidProps( - `backend drawlistVersion=${String( - backendDrawlistVersion, - )} is invalid. Fix: set backend drawlist version marker to 1.`, - ); - }🤖 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 596 - 603, The branch checking backendDrawlistVersion after calling readBackendDrawlistVersionMarker is redundant because readBackendDrawlistVersionMarker already throws for non-1 values and only returns 1 | null; remove the entire if block that references backendDrawlistVersion and invalidProps to eliminate unreachable code (i.e., delete the const check starting with "if (backendDrawlistVersion !== null && backendDrawlistVersion !== 1) { ... }"). Ensure no other logic depends on that removed block and keep the const backendDrawlistVersion assignment if it's still used elsewhere.examples/regression-dashboard/src/__tests__/keybindings.test.ts (1)
5-12: LGTM! Good keybinding resolution coverage.The test validates canonical mappings and includes a negative case for unmapped keys.
Consider adding a case-insensitivity assertion since
resolveDashboardCommanduseskey.toLowerCase()internally:💡 Optional: Add case-insensitivity test
assert.equal(resolveDashboardCommand("t"), "cycle-theme"); assert.equal(resolveDashboardCommand("x"), undefined); + // Case-insensitivity + assert.equal(resolveDashboardCommand("Q"), "quit"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/regression-dashboard/src/__tests__/keybindings.test.ts` around lines 5 - 12, Add a case-insensitivity assertion to the existing keybinding test: call resolveDashboardCommand with an uppercase key (e.g., "Q" or "J") and assert it resolves to the same canonical command as the lowercase variant (e.g., "quit" or "move-down") to verify the internal key.toLowerCase() behavior in resolveDashboardCommand.docs/dev/build.md (1)
38-41: Clarify optionality against the earlier setup step.Line 38 says submodules are only required for sync/audit workflows, but the earlier setup still reads as unconditional. Add a short “TS-only path can skip submodule init” note to remove ambiguity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/dev/build.md` around lines 38 - 41, The paragraph about the vendor/zireael submodule is ambiguous about whether submodule init is required for all developers; update docs/dev/build.md to add a short clarifying note (adjacent to the earlier setup step and in the paragraph mentioning "vendor/zireael" and "Native addon builds") that explicitly states the TS-only development path can skip submodule initialization (i.e., submodule checkout is only required for native addon builds, syncing, or auditing vendored engine updates), and phrase it as a brief optional step so readers know they can skip submodule init for TypeScript-only work.examples/regression-dashboard/src/helpers/formatters.ts (1)
38-46: Consider single-pass iteration for efficiency.
fleetCountsiterates theservicesarray three times. For a dashboard example this is fine, but a single-pass approach would be more efficient if the service list grows large.♻️ Optional single-pass implementation
export function fleetCounts( services: readonly Service[], ): Readonly<{ healthy: number; warning: number; down: number }> { - return Object.freeze({ - healthy: services.filter((service) => service.status === "healthy").length, - warning: services.filter((service) => service.status === "warning").length, - down: services.filter((service) => service.status === "down").length, - }); + const counts = { healthy: 0, warning: 0, down: 0 }; + for (const service of services) { + counts[service.status]++; + } + return Object.freeze(counts); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/regression-dashboard/src/helpers/formatters.ts` around lines 38 - 46, The function fleetCounts currently iterates services three times causing unnecessary work; change fleetCounts to compute healthy/warning/down in a single pass by iterating once (e.g., use a for loop or Array.prototype.reduce over services) and increment the appropriate counters, then return Object.freeze with those counts; update the implementation of fleetCounts to reference the same return shape ({ healthy, warning, down }) and preserve Readonly typing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/dev/live-pty-debugging.md`:
- Around line 24-25: Replace the hardcoded absolute path "/home/k3nig/Rezi" used
in the two doc examples (lines showing the cd command) with a portable
instruction to run the commands from the repository root (e.g., "change into the
repo root" or "from your project root run the build commands"); update both
occurrences (the cd line at ~24 and the similar lines at ~37-38) so contributors
are instructed to be in the repo root rather than using the specific local path.
In `@packages/core/src/__tests__/drawlistDecode.ts`:
- Around line 273-283: The DEF_/FREE_ handlers (e.g., case OP_DEF_STRING and
OP_FREE_STRING) read fields at offsets like cmd.offset + 8/12 and slice payloads
without validating the command length, so add bounds checks before any u32 reads
or bytes.subarray calls: verify that cmd.size (or computed remaining =
bytes.length - cmd.offset) is at least the minimum expected bytes for the
specific opcode (header + field offsets + byteLen) and that dataStart/dataEnd
live within bytes.length; if validation fails, skip the command (or return an
error) instead of calling u32(...) or strings.set/delete. Apply the same
validation pattern to the other DEF_/FREE handlers referenced around lines
326-336 to prevent out-of-bounds reads.
In `@packages/core/src/__tests__/integration/integration.reflow.test.ts`:
- Around line 11-19: The test imports internal helpers (OP_CLEAR, OP_DRAW_TEXT,
OP_FILL_RECT, OP_PUSH_CLIP, parseCommandHeaders, parseDrawTextCommands,
parseInternedStrings) via a relative path; replace that relative import in
integration.reflow.test.ts with an import from the package entrypoint export
(e.g., import these symbols from the package named export such as `@rezi-ui/core`)
and ensure those helpers are re-exported from the package entry module so the
test only imports from the public package API rather than ../drawlistDecode.js.
In `@packages/core/src/app/widgetRenderer.ts`:
- Around line 3963-3982: The begin-frame path must always release the
frameWriter and convert synchronous commit exceptions into a backend error
return: wrap the call to frameWriter.commit(submittedBytes.byteLength) in a
try/catch (and/or finally) so that if commit throws you call
frameWriter.abort(), call perfMarkEnd("drawlist_build", buildToken) and return a
backend failure (e.g. { ok: false, code: "ZRUI_BACKEND_ERROR", detail:
String(err) }); ensure the normal success path still assigns inFlight from the
commit result and that frameWriter.abort() is invoked for all failure paths so
the writer is always released; refer to BACKEND_BEGIN_FRAME_MARKER, beginFrame,
frameWriter, builtInto, and frameWriter.commit in your changes.
In `@packages/core/src/drawlist/__tests__/builder.text-arena.equivalence.test.ts`:
- Around line 101-108: Add explicit upper-bound checks before performing binary
reads to avoid RangeError: in the command loop (where h.cmdOffset, h.cmdBytes,
cmdEnd, off, u16 and u32 are used) validate that off + 2 and off + 8 are <=
cmdEnd (and also <= bytes.length) before calling u16(bytes, off) and u32(bytes,
off + 4), and assert with a clear message if the bounds fail; similarly, in
collectResourceTables (and the blob/header parsing that reads segCount) add
guards that the buffer has at least the expected header length (e.g., bytes
required to read segCount and subsequent fields) before decoding, and assert/die
deterministically instead of allowing a RangeError.
In `@packages/core/src/renderer/__tests__/renderer.text.test.ts`:
- Around line 95-103: The decoder currently silently defaults to a 28-byte
stride when the blob size doesn't match the 40-byte layout; update
decodeTextRunBlob to validate the blob layout and fail fast: compute segCount
and remaining as now, then if remaining === segCount * 40 use stride=40, else if
remaining === segCount * 28 use stride=28, otherwise throw an error (or assert)
describing "unsupported text-run blob layout" including segCount and remaining;
adjust use of stringFieldOffset/byteOffFieldOffset/byteLenFieldOffset as before
based on the chosen stride and apply the same validation for the other checks in
the 106–123 region.
- Around line 2-13: The test imports internal helpers (OP_DEF_BLOB,
OP_DEF_STRING, OP_DRAW_TEXT, OP_DRAW_TEXT_RUN, OP_FREE_BLOB, OP_FREE_STRING,
OP_POP_CLIP, OP_PUSH_CLIP, parseCommandHeaders, parseInternedStrings) from an
internal test path; change the import to come from the package's public test
export (the package export entry that exposes drawlistDecode utilities, e.g.,
the testing surface in the core package) so tests use package-exported symbols
instead of relative internal paths; update the import statement in
renderer.text.test.ts to import those named symbols from the package export
(e.g., `@rezi-ui/core` or the designated test export) and remove the
../../__tests__/drawlistDecode.js relative path.
- Around line 165-168: The OP_FREE_BLOB handling reads a u32 at off + 8 without
validating the command payload length; before calling u32(bytes, off + 8) check
that cmd.size is at least 12 (or otherwise large enough to include 4 bytes at
offset 8) and bail out (skip/throw/log) if it's truncated so you don't read past
the buffer. Update the block handling OP_FREE_BLOB (the condition using
OP_FREE_BLOB, the u32 call, and textRunBlobsByIndex assignment) to perform this
guard using cmd.size and only compute blobId and mutate textRunBlobsByIndex when
the size check passes.
In `@packages/core/src/renderer/renderToDrawlist/overflowCulling.ts`:
- Line 1: The import in overflowCulling.ts pulls RuntimeInstance from an
internal relative module ("../../runtime/commit.js"); change it to use the
package-export surface instead (import the RuntimeInstance type from the public
package export, e.g. the core package like `@rezi-ui/core`) so it follows the repo
rule of package-export imports; update the import statement that references
RuntimeInstance to use the package's public export rather than the internal
path.
In `@packages/core/src/widgets/__tests__/basicWidgets.render.test.ts`:
- Around line 58-76: The loop that maps decoded text (using
parseDecodedDrawTextCommands()) drifts because drawTextIndex is only incremented
for records with size >= 48; change the logic so drawTextIndex advances for
every DRAW_TEXT opcode (opcode === 3) regardless of size so the decoded[]
entries stay aligned, and when reading decodedCmd (decoded[drawTextIndex])
handle the case it may be undefined (e.g., default text "") before pushing the
DrawTextCommand; update the code around the while loop that reads opcode/size
and uses drawTextIndex accordingly.
In `@packages/core/src/widgets/ui.ts`:
- Line 358: The example uses a hardcoded rgb(...) color literal in the ui.icon
call; replace those raw RGB literals with the design-system/semantic tokens
(e.g., use variant/dsVariant/dsTone or a named token like "success" or
"green.500" depending on your token API) so the icon styling follows the theme;
update each example of ui.icon("arrow.right", { style: { fg: rgb(0, 255, 0) } })
(and the other occurrences noted) to pass the appropriate token prop instead of
rgb(...) — for example set the token via the dsTone/dsVariant/dsSize or style.fg
= getToken("...") per your project's token accessor.
In `@packages/create-rezi/templates/starship/src/main.ts`:
- Around line 126-135: The registered callback
frameAuditGlobal.__reziFrameAuditContext currently calls currentRouteId() too
early and can crash if app isn't initialized; update currentRouteId() to guard
against pre-app access by using optional chaining when dereferencing the
router/current route (e.g., use app?.router?.currentRoute()?.id or similar) so
it returns undefined or a safe fallback when app or router is not available,
then keep the frameAuditGlobal.__reziFrameAuditContext registration as-is to
rely on the safe currentRouteId() behavior.
In `@packages/ink-compat/src/runtime/createInkRenderer.ts`:
- Around line 184-192: The current no-op implementation of blitRect in
createInkRenderer should be changed to fail-fast: replace the empty body of
blitRect(_srcX, _srcY, _w, _h, _dstX, _dstY) with a thrown Error (e.g., throw
new Error("blitRect is unimplemented in ink-compat renderer")) so any future
invocation surfaces immediately; reference the blitRect method in
createInkRenderer and include a clear, descriptive message to aid debugging.
In `@packages/native/src/lib.rs`:
- Around line 1939-1969: The test ffi_layout_matches_vendored_headers is missing
an assertion for the new FFI struct zr_fb_link_t (should be 16 bytes); add size
and alignment checks inside that test so the Rust layout matches the vendored
headers — e.g., assert_eq!(size_of::<ffi::zr_fb_link_t>(), 16) and a
corresponding align_of assertion (likely 4) — to prevent memory-corrupting
mismatches for code using zr_fb_link_t (see usages zr_fb_link_intern and
zr_fb_link_lookup).
In `@packages/node/src/backend/nodeBackend.ts`:
- Line 42: Add BackendBeginFrame to the core package root exports and update the
node backend import to use the package root: modify the core export list in
packages core index (add BackendBeginFrame alongside the other backend types) so
it is exported from "@rezi-ui/core", then change the import in nodeBackend
(where it currently imports BackendBeginFrame from "@rezi-ui/core/backend") to
import BackendBeginFrame from "@rezi-ui/core" instead to satisfy the
packages/node import boundary rule.
In `@packages/node/src/backend/nodeBackendInline.ts`:
- Around line 824-892: The catch block currently emits
frameAudit.emit("frame.throw", { detail: safeDetail(err) }) without the
frameSeq, so update that emit to include frameSeq (e.g.
frameAudit.emit("frame.throw", { frameSeq, detail: safeDetail(err) })) so throw
events can be correlated with the submitted frame; use the existing frameSeq
variable declared at the top of the try and keep the same safeDetail(err)
payload.
In `@packages/node/src/frameAudit.ts`:
- Around line 17-19: The audit writer currently calls
appendFileSync/writeFileSync on the path derived from REZI_FRAME_AUDIT_LOG and
may fail silently if nested directories don't exist; before any appendFileSync
or writeFileSync call (the occurrences around REZI_FRAME_AUDIT_LOG and in the
functions that perform frame audit writes), ensure the directory for the audit
file exists by calling mkdirSync(path.dirname(auditPath), { recursive: true })
(or equivalent) and only then perform append/write; also propagate or log errors
instead of swallowing them so failures are visible.
In `@packages/node/src/worker/engineWorker.ts`:
- Around line 604-611: The DataView construction/emit blocks are misformatted;
run the Biome formatter (biome check --fix) or reformat the DataView-related
blocks so they follow project style, specifically the block that creates
dvPayload (new DataView(payload.buffer, payload.byteOffset,
DEBUG_FRAME_RECORD_BYTES)) and the subsequent frameAudit.emit call that
references reason, recordId, u64FromView(dvPayload, 0).toString(), and code;
also fix the similar DataView blocks at the other occurrences (around the
u64FromView usages at the noted ranges) so indentation, spacing, and punctuation
match the rest of the file and pass CI.
In `@scripts/frame-audit-report.mjs`:
- Around line 288-297: The pid summary is built from the unfiltered recordsByPid
so pid_sessions shows unrelated sessions; rebuild or filter recordsByPid from
the already-filtered records (the same filteredRecords/records used at the
earlier filter) before computing sessionRows, or apply the same predicate when
iterating recordsByPid; update the code that produces sessionRows/for-loop
(variables: recordsByPid, sessionRows, pid_sessions, records) to use the
filtered set so pids and pid_sessions reflect the active PID filter.
---
Outside diff comments:
In `@packages/core/src/renderer/renderToDrawlist/widgets/collections.ts`:
- Around line 1-31: The import of emitFrameAudit and FRAME_AUDIT_ENABLED is
misordered and fails the import-sort check; move the "import { emitFrameAudit,
FRAME_AUDIT_ENABLED } from '../../../perf/frameAudit.js';" line into the main
block of local/project imports so it is grouped with the other "../../../..."
imports and alphabetically ordered with them (place it among the other non-type
imports such as distributeColumnWidths, tableRecipe, treeRecipe, and the
widgets/virtualList imports) to satisfy the project's import ordering rules.
In `@packages/create-rezi/templates/starship/src/main.ts`:
- Around line 734-739: The onSignal handler can call stopApp() while bootstrap
is still running, allowing bootstrap to continue into app.start() after shutdown
was requested; modify bootstrap logic so startup checks a cancellation flag or
promise set by onSignal/stopApp before and immediately after awaiting any async
initialization, and refuse to call app.start() (or abort startup) if
shutdownRequested is true; update onSignal/stopApp to set this shared
shutdownRequested flag (or resolve a cancellation promise) and ensure
app.start() is only invoked when the flag is false (apply same guard to the
other startup lines referenced by symbols onSignal, stopApp, and app.start).
In `@packages/node/src/worker/engineWorker.ts`:
- Around line 1573-1583: The code unconditionally clears nativeFrameAuditEnabled
after calling native.engineDebugDisable even when that call failed (rc != 0),
which can desync worker vs native state; in the "debug:disable" case (around
engineDebugDisable, rc, engineId, nativeFrameAuditEnabled, frameAudit.enabled)
only set nativeFrameAuditEnabled = false when engineDebugDisable returned
success (e.g., rc === 0), otherwise leave the flag untouched (and preserve
existing error handling/return path).
---
Nitpick comments:
In `@docs/dev/build.md`:
- Around line 38-41: The paragraph about the vendor/zireael submodule is
ambiguous about whether submodule init is required for all developers; update
docs/dev/build.md to add a short clarifying note (adjacent to the earlier setup
step and in the paragraph mentioning "vendor/zireael" and "Native addon builds")
that explicitly states the TS-only development path can skip submodule
initialization (i.e., submodule checkout is only required for native addon
builds, syncing, or auditing vendored engine updates), and phrase it as a brief
optional step so readers know they can skip submodule init for TypeScript-only
work.
In `@examples/regression-dashboard/src/__tests__/keybindings.test.ts`:
- Around line 5-12: Add a case-insensitivity assertion to the existing
keybinding test: call resolveDashboardCommand with an uppercase key (e.g., "Q"
or "J") and assert it resolves to the same canonical command as the lowercase
variant (e.g., "quit" or "move-down") to verify the internal key.toLowerCase()
behavior in resolveDashboardCommand.
In `@examples/regression-dashboard/src/helpers/formatters.ts`:
- Around line 38-46: The function fleetCounts currently iterates services three
times causing unnecessary work; change fleetCounts to compute
healthy/warning/down in a single pass by iterating once (e.g., use a for loop or
Array.prototype.reduce over services) and increment the appropriate counters,
then return Object.freeze with those counts; update the implementation of
fleetCounts to reference the same return shape ({ healthy, warning, down }) and
preserve Readonly typing.
In `@packages/core/src/app/createApp.ts`:
- Around line 596-603: The branch checking backendDrawlistVersion after calling
readBackendDrawlistVersionMarker is redundant because
readBackendDrawlistVersionMarker already throws for non-1 values and only
returns 1 | null; remove the entire if block that references
backendDrawlistVersion and invalidProps to eliminate unreachable code (i.e.,
delete the const check starting with "if (backendDrawlistVersion !== null &&
backendDrawlistVersion !== 1) { ... }"). Ensure no other logic depends on that
removed block and keep the const backendDrawlistVersion assignment if it's still
used elsewhere.
In `@packages/core/src/drawlist/__tests__/builder_v6_resources.test.ts`:
- Around line 4-6: Replace the duplicated u32 implementation in
builder_v6_resources.test.ts by importing and reusing the shared u32 test helper
from the drawlistDecode test module; remove the local function u32 (the one that
constructs a DataView and calls getUint32) and add an import for the exported
u32 helper from the drawlistDecode test module so all tests use the same parsing
utility.
In `@packages/core/src/renderer/__tests__/persistentBlobKeys.test.ts`:
- Around line 89-94: Add a test that covers the "null allocation" branch of
addBlobAligned by creating or mocking a CountingBuilder whose addBlob method
returns null, then call addBlobAligned(builder, new Uint8Array([1,2,3])) and
assert the function returns null (or the documented failure value) and that
builder.blobByteLens was not changed; reference addBlobAligned, addBlob, and
CountingBuilder in the test to locate and implement the behavior check.
In `@packages/core/src/renderer/renderToDrawlist/overflowCulling.ts`:
- Around line 90-109: Add focused regression tests exercising
subtreeCanOverflowBounds traversal: create scenarios that include pass-through
"layer" frame styles, wrapper chains (focusZone, focusTrap, themed, layers) to
verify isTransparentOverflowWrapper correctly gates descent, and cases where
boxShadow is {} versus explicit zero values to ensure hasBoxShadowOverflow
treats defaults and zeros equivalently; confirm that hasVisibleOverflow and
hasBoxShadowOverflow return true for visible/shadowing children and that
traversal still short-circuits on those checks while continuing descent only
through transparent wrappers.
In `@packages/core/src/renderer/renderToDrawlist/widgets/renderCanvasWidgets.ts`:
- Around line 225-227: Rename the confusing addBlobAligned API to addBlob (and
update any imports/uses) because alignment actually happens during serialization
via writeDefBlob().align4(), not at add time; update the export and all call
sites to use addBlob, and optionally add a brief comment in
renderCanvasWidgets.ts near the new addBlob function clarifying that 4-byte
alignment is applied during drawlist finalization/serialization
(writeDefBlob.align4()) rather than at addition.
In `@packages/core/src/runtime/commit.ts`:
- Around line 1337-1341: The rewrite of the parent VNode’s committed children
runs every fast-reuse cycle and should be skipped when none of the child wiring
changed; guard the work by checking child dirtiness or reference differences
before mapping and calling rewriteCommittedVNode. Concretely: in the fast-reuse
path around prev.children.map(...) and rewriteCommittedVNode(vnode,...), first
test whether any child is dirty or whether prev.children’ committed vnode
references differ from the current vnode.children (e.g. prev.children.some(c =>
c.dirty) || !prev.children.every((c,i) => c.vnode === vnode.children[i])); only
if that predicate is true compute fastReuseCommittedChildren and call
rewriteCommittedVNode. Ensure you reference the existing symbols prev,
prev.children, vnode, fastReuseCommittedChildren, and rewriteCommittedVNode when
making the change.
In `@packages/core/src/widgets/__tests__/style.inheritance.test.ts`:
- Around line 35-47: Replace the magic bit-shift literals in
computeExpectedAttrs with named constants (e.g., ATTR_BOLD = 1<<0, ATTR_ITALIC =
1<<1, ATTR_UNDERLINE = 1<<2, etc.) declared near the top of the test file, then
use those constants inside computeExpectedAttrs instead of the raw 1<<n values
so the mapping (bold/italic/underline/inverse/dim/strikethrough/overline/blink)
is explicit and easier to audit/maintain; ensure the final return still uses
unsigned shift (attrs >>> 0) unchanged.
In `@packages/create-rezi/templates/starship/src/screens/engineering.ts`:
- Around line 394-397: The inline magic numbers in the responsiveDeckMinHeight
calculation (the 12, 10 and 2) should be extracted into named constants for
clarity and easier tuning: define e.g. CONTROLS_SUMMARY_OFFSET (12),
CONTROLS_OFFSET (10) and SECONDARY_PANELS_OFFSET (2) near the top of the module,
then update the responsiveDeckMinHeight expression to use these constants with
contentRows, showControlsSummary and showSecondaryPanels so the formula reads
self-documenting and the offsets can be tuned without changing the arithmetic.
In `@packages/native/src/lib.rs`:
- Around line 1709-1731: The test helper uses tab characters in the struct
initialization blocks (e.g., the caps assignment for ffi::plat_caps_t, the
limits retrieval via ffi::zr_engine_config_default(), and the initial_term_state
for ffi::zr_term_state_t); update those lines to use spaces (matching the file's
existing indentation style) so the caps, limits, and initial_term_state
initializers are consistently space-indented with the surrounding code.
- Around line 116-155: The indentation in the struct definitions
zr_terminal_caps_t and plat_caps_t uses tab characters mixed with spaces;
replace all leading tab characters inside these struct blocks with the project's
standard spaces indentation to match the rest of the file (align field names and
types consistently), and reformat the file (e.g., run rustfmt) to ensure
consistent spacing throughout.
- Around line 1855-1858: The test
fb_link_intern_compacts_stale_refs_and_bounds_growth uses a hardcoded
LINK_ENTRY_MAX_BYTES (2083 + 2083) that mirrors ZR_FB_LINK_URI_MAX_BYTES +
ZR_FB_LINK_ID_MAX_BYTES and may drift from vendored headers; update the test by
either (a) adding a clear comment on LINK_ENTRY_MAX_BYTES documenting its
derivation from ZR_FB_LINK_URI_MAX_BYTES and ZR_FB_LINK_ID_MAX_BYTES and why
those specific values are used, or (b) preferably import the two upstream
constants via FFI and compute LINK_ENTRY_MAX_BYTES from them inside the test
(referencing the test function name and the LINK_ENTRY_MAX_BYTES symbol) so the
values stay synchronized with the vendored headers.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (23)
examples/regression-dashboard/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonpackages/testkit/fixtures/zrdl-v1-graphics/widgets/barchart_highres.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1-graphics/widgets/canvas_primitives.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1-graphics/widgets/heatmap_plasma.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1-graphics/widgets/image_png_contain.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1-graphics/widgets/image_rgba_sixel_cover.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1-graphics/widgets/line_chart.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1-graphics/widgets/link_docs.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1-graphics/widgets/richtext_underline_ext.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1-graphics/widgets/scatter_plot.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1-graphics/widgets/sparkline_highres.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/golden/draw_text_interned.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/button_focus_states.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/divider_horizontal.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/divider_with_label.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/input_basic.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/input_disabled.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/input_focused_inverse.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/layer_backdrop_opaque.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/modal_backdrop_dim.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/spinner_tick_0.binis excluded by!**/*.binpackages/testkit/fixtures/zrdl-v1/widgets/spinner_tick_1.binis excluded by!**/*.bin
📒 Files selected for processing (150)
.github/workflows/ci.ymlAGENTS.mdCLAUDE.mddocs/backend/native.mddocs/dev/build.mddocs/dev/live-pty-debugging.mddocs/dev/repo-layout.mddocs/dev/testing.mddocs/protocol/versioning.mddocs/protocol/zrdl.mdexamples/regression-dashboard/README.mdexamples/regression-dashboard/package.jsonexamples/regression-dashboard/src/__tests__/keybindings.test.tsexamples/regression-dashboard/src/__tests__/reducer.test.tsexamples/regression-dashboard/src/__tests__/render.test.tsexamples/regression-dashboard/src/__tests__/telemetry.test.tsexamples/regression-dashboard/src/helpers/formatters.tsexamples/regression-dashboard/src/helpers/keybindings.tsexamples/regression-dashboard/src/helpers/state.tsexamples/regression-dashboard/src/helpers/telemetry.tsexamples/regression-dashboard/src/main.tsexamples/regression-dashboard/src/screens/overview.tsexamples/regression-dashboard/src/theme.tsexamples/regression-dashboard/src/types.tsexamples/regression-dashboard/tsconfig.jsonmkdocs.ymlpackage.jsonpackages/core/src/__tests__/drawlistDecode.tspackages/core/src/__tests__/integration/integration.dashboard.test.tspackages/core/src/__tests__/integration/integration.file-manager.test.tspackages/core/src/__tests__/integration/integration.form-editor.test.tspackages/core/src/__tests__/integration/integration.reflow.test.tspackages/core/src/__tests__/integration/integration.resize.test.tspackages/core/src/abi.tspackages/core/src/app/__tests__/hotStateReload.test.tspackages/core/src/app/__tests__/inspectorOverlayHelper.test.tspackages/core/src/app/__tests__/resilience.test.tspackages/core/src/app/createApp.tspackages/core/src/app/rawRenderer.tspackages/core/src/app/widgetRenderer.tspackages/core/src/app/widgetRenderer/damageTracking.tspackages/core/src/drawlist/__tests__/builder.alignment.test.tspackages/core/src/drawlist/__tests__/builder.golden.test.tspackages/core/src/drawlist/__tests__/builder.graphics.test.tspackages/core/src/drawlist/__tests__/builder.limits.test.tspackages/core/src/drawlist/__tests__/builder.reset.test.tspackages/core/src/drawlist/__tests__/builder.round-trip.test.tspackages/core/src/drawlist/__tests__/builder.string-cache.test.tspackages/core/src/drawlist/__tests__/builder.string-intern.test.tspackages/core/src/drawlist/__tests__/builder.style-encoding.test.tspackages/core/src/drawlist/__tests__/builder.text-arena.equivalence.test.tspackages/core/src/drawlist/__tests__/builder.text-run.test.tspackages/core/src/drawlist/__tests__/builder.validate-caps.test.tspackages/core/src/drawlist/__tests__/builder_style_attrs.test.tspackages/core/src/drawlist/__tests__/builder_v6_resources.test.tspackages/core/src/drawlist/__tests__/writers.gen.test.tspackages/core/src/drawlist/__tests__/writers.gen.v6.test.tspackages/core/src/drawlist/builder.tspackages/core/src/drawlist/builderBase.tspackages/core/src/drawlist/writers.gen.tspackages/core/src/index.tspackages/core/src/layout/kinds/overlays.tspackages/core/src/perf/frameAudit.tspackages/core/src/renderer/__tests__/overlay.edge.test.tspackages/core/src/renderer/__tests__/persistentBlobKeys.test.tspackages/core/src/renderer/__tests__/render.golden.test.tspackages/core/src/renderer/__tests__/renderer.damage.test.tspackages/core/src/renderer/__tests__/renderer.partial.test.tspackages/core/src/renderer/__tests__/renderer.scrollBlit.benchmark.test.tspackages/core/src/renderer/__tests__/renderer.text.test.tspackages/core/src/renderer/renderToDrawlist/overflowCulling.tspackages/core/src/renderer/renderToDrawlist/widgets/collections.tspackages/core/src/renderer/renderToDrawlist/widgets/containers.tspackages/core/src/renderer/renderToDrawlist/widgets/renderCanvasWidgets.tspackages/core/src/runtime/__tests__/commit.fastReuse.regression.test.tspackages/core/src/runtime/commit.tspackages/core/src/theme/__tests__/theme.extend.test.tspackages/core/src/theme/__tests__/theme.validation.test.tspackages/core/src/theme/resolve.tspackages/core/src/theme/validate.tspackages/core/src/widgets/__tests__/basicWidgets.render.test.tspackages/core/src/widgets/__tests__/graphics.golden.test.tspackages/core/src/widgets/__tests__/graphicsWidgets.test.tspackages/core/src/widgets/__tests__/inspectorOverlay.render.test.tspackages/core/src/widgets/__tests__/pagination.test.tspackages/core/src/widgets/__tests__/renderer.regressions.test.tspackages/core/src/widgets/__tests__/style.inheritance.test.tspackages/core/src/widgets/__tests__/style.merge.test.tspackages/core/src/widgets/__tests__/style.utils.test.tspackages/core/src/widgets/__tests__/styleUtils.test.tspackages/core/src/widgets/__tests__/widgetRenderSmoke.test.tspackages/core/src/widgets/pagination.tspackages/core/src/widgets/ui.tspackages/create-rezi/templates/starship/src/main.tspackages/create-rezi/templates/starship/src/screens/bridge.tspackages/create-rezi/templates/starship/src/screens/cargo.tspackages/create-rezi/templates/starship/src/screens/comms.tspackages/create-rezi/templates/starship/src/screens/crew.tspackages/create-rezi/templates/starship/src/screens/engineering.tspackages/create-rezi/templates/starship/src/screens/settings.tspackages/create-rezi/templates/starship/src/theme.tspackages/ink-compat/src/runtime/createInkRenderer.tspackages/native/README.mdpackages/native/src/lib.rspackages/native/vendor/VENDOR_COMMIT.txtpackages/native/vendor/zireael/include/zr/zr_caps.hpackages/native/vendor/zireael/include/zr/zr_config.hpackages/native/vendor/zireael/include/zr/zr_debug.hpackages/native/vendor/zireael/include/zr/zr_drawlist.hpackages/native/vendor/zireael/include/zr/zr_engine.hpackages/native/vendor/zireael/include/zr/zr_event.hpackages/native/vendor/zireael/include/zr/zr_metrics.hpackages/native/vendor/zireael/include/zr/zr_platform_types.hpackages/native/vendor/zireael/include/zr/zr_result.hpackages/native/vendor/zireael/include/zr/zr_terminal_caps.hpackages/native/vendor/zireael/include/zr/zr_version.hpackages/native/vendor/zireael/src/core/zr_cursor.hpackages/native/vendor/zireael/src/core/zr_damage.cpackages/native/vendor/zireael/src/core/zr_damage.hpackages/native/vendor/zireael/src/core/zr_diff.cpackages/native/vendor/zireael/src/core/zr_drawlist.cpackages/native/vendor/zireael/src/core/zr_engine.cpackages/native/vendor/zireael/src/core/zr_engine_present.incpackages/native/vendor/zireael/src/core/zr_event_pack.cpackages/native/vendor/zireael/src/core/zr_event_pack.hpackages/native/vendor/zireael/src/core/zr_framebuffer.cpackages/native/vendor/zireael/src/core/zr_metrics.cpackages/native/vendor/zireael/src/core/zr_placeholder.cpackages/native/vendor/zireael/src/platform/win32/zr_win32_conpty_test.cpackages/native/vendor/zireael/src/platform/win32/zr_win32_conpty_test.hpackages/native/vendor/zireael/src/unicode/zr_grapheme.hpackages/native/vendor/zireael/src/unicode/zr_unicode_data.hpackages/native/vendor/zireael/src/unicode/zr_unicode_pins.hpackages/native/vendor/zireael/src/unicode/zr_utf8.hpackages/native/vendor/zireael/src/unicode/zr_width.hpackages/native/vendor/zireael/src/unicode/zr_wrap.hpackages/native/vendor/zireael/src/util/zr_arena.hpackages/native/vendor/zireael/src/util/zr_ring.hpackages/native/vendor/zireael/src/util/zr_string_builder.hpackages/native/vendor/zireael/src/util/zr_string_view.hpackages/native/vendor/zireael/src/util/zr_vec.hpackages/node/src/backend/nodeBackend.tspackages/node/src/backend/nodeBackendInline.tspackages/node/src/frameAudit.tspackages/node/src/worker/engineWorker.tsscripts/__tests__/check-native-vendor-integrity.test.mjsscripts/check-native-vendor-integrity.mjsscripts/frame-audit-report.mjsscripts/generate-drawlist-writers.tsvendor/zireael
💤 Files with no reviewable changes (6)
- packages/core/src/abi.ts
- packages/native/vendor/zireael/src/util/zr_string_view.h
- packages/core/src/index.ts
- packages/native/vendor/zireael/src/unicode/zr_unicode_pins.h
- packages/core/src/renderer/tests/renderer.scrollBlit.benchmark.test.ts
- packages/native/vendor/zireael/src/unicode/zr_utf8.h
Comment on lines
+24
to
+25
| cd /home/k3nig/Rezi | ||
| npx tsc -b packages/core packages/node packages/create-rezi |
There was a problem hiding this comment.
Replace hardcoded local path with portable repo-root instructions.
Line 24 and Line 37 use /home/k3nig/Rezi, which won’t work for most contributors.
🛠️ Proposed doc fix
-cd /home/k3nig/Rezi
+cd <path-to-your-Rezi-clone>Apply this at both occurrences.
Also applies to: 37-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/dev/live-pty-debugging.md` around lines 24 - 25, Replace the hardcoded
absolute path "/home/k3nig/Rezi" used in the two doc examples (lines showing the
cd command) with a portable instruction to run the commands from the repository
root (e.g., "change into the repo root" or "from your project root run the build
commands"); update both occurrences (the cd line at ~24 and the similar lines at
~37-38) so contributors are instructed to be in the repo root rather than using
the specific local path.
Comment on lines
+273
to
+283
| case OP_DEF_STRING: { | ||
| const id = u32(bytes, cmd.offset + 8); | ||
| const byteLen = u32(bytes, cmd.offset + 12); | ||
| const dataStart = cmd.offset + 16; | ||
| const dataEnd = dataStart + byteLen; | ||
| strings.set(id, cloneBytes(bytes.subarray(dataStart, dataEnd))); | ||
| break; | ||
| } | ||
| case OP_FREE_STRING: { | ||
| const id = u32(bytes, cmd.offset + 8); | ||
| strings.delete(id); |
There was a problem hiding this comment.
Validate DEF_*/FREE_* command sizes before reading fields.
Both parsers read +8/+12 fields and payload slices without command-specific bounds checks, so malformed streams can consume bytes from adjacent commands and decode incorrect resources.
🐛 Proposed hardening patch
case OP_DEF_STRING: {
+ if (cmd.size < 16) break;
const id = u32(bytes, cmd.offset + 8);
const byteLen = u32(bytes, cmd.offset + 12);
const dataStart = cmd.offset + 16;
const dataEnd = dataStart + byteLen;
+ if (dataEnd > cmd.offset + cmd.size) break;
strings.set(id, cloneBytes(bytes.subarray(dataStart, dataEnd)));
break;
}
case OP_FREE_STRING: {
+ if (cmd.size !== 12) break;
const id = u32(bytes, cmd.offset + 8);
strings.delete(id);
break;
}
@@
case OP_DEF_BLOB: {
+ if (cmd.size < 16) break;
const id = u32(bytes, cmd.offset + 8);
const byteLen = u32(bytes, cmd.offset + 12);
const dataStart = cmd.offset + 16;
const dataEnd = dataStart + byteLen;
+ if (dataEnd > cmd.offset + cmd.size) break;
blobs.set(id, cloneBytes(bytes.subarray(dataStart, dataEnd)));
break;
}
case OP_FREE_BLOB: {
+ if (cmd.size !== 12) break;
blobs.delete(u32(bytes, cmd.offset + 8));
break;
}Also applies to: 326-336
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/__tests__/drawlistDecode.ts` around lines 273 - 283, The
DEF_/FREE_ handlers (e.g., case OP_DEF_STRING and OP_FREE_STRING) read fields at
offsets like cmd.offset + 8/12 and slice payloads without validating the command
length, so add bounds checks before any u32 reads or bytes.subarray calls:
verify that cmd.size (or computed remaining = bytes.length - cmd.offset) is at
least the minimum expected bytes for the specific opcode (header + field offsets
+ byteLen) and that dataStart/dataEnd live within bytes.length; if validation
fails, skip the command (or return an error) instead of calling u32(...) or
strings.set/delete. Apply the same validation pattern to the other DEF_/FREE
handlers referenced around lines 326-336 to prevent out-of-bounds reads.
Comment on lines
+11
to
+19
| import { | ||
| OP_CLEAR, | ||
| OP_DRAW_TEXT, | ||
| OP_FILL_RECT, | ||
| OP_PUSH_CLIP, | ||
| parseCommandHeaders, | ||
| parseDrawTextCommands, | ||
| parseInternedStrings, | ||
| } from "../drawlistDecode.js"; |
There was a problem hiding this comment.
Use a package export instead of an internal relative import path.
This new import from ../drawlistDecode.js creates internal-path coupling in a TypeScript file. Please re-export these helpers from a package entrypoint and import from that export.
As per coding guidelines, "Import from package exports only (@rezi-ui/core, @rezi-ui/node), never from internal paths in TypeScript files."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/__tests__/integration/integration.reflow.test.ts` around
lines 11 - 19, The test imports internal helpers (OP_CLEAR, OP_DRAW_TEXT,
OP_FILL_RECT, OP_PUSH_CLIP, parseCommandHeaders, parseDrawTextCommands,
parseInternedStrings) via a relative path; replace that relative import in
integration.reflow.test.ts with an import from the package entrypoint export
(e.g., import these symbols from the package named export such as `@rezi-ui/core`)
and ensure those helpers are re-exported from the package entry module so the
test only imports from the public package API rather than ../drawlistDecode.js.
Comment on lines
+3963
to
+3982
| const beginFrame = ( | ||
| this.backend as RuntimeBackend & | ||
| Partial<Record<typeof BACKEND_BEGIN_FRAME_MARKER, BackendBeginFrame>> | ||
| )[BACKEND_BEGIN_FRAME_MARKER]; | ||
| if (typeof beginFrame === "function") { | ||
| const frameWriter = beginFrame(); | ||
| if (frameWriter) { | ||
| const builtInto = this.builder.buildInto(frameWriter.buf); | ||
| if (!builtInto.ok) { | ||
| frameWriter.abort(); | ||
| perfMarkEnd("drawlist_build", buildToken); | ||
| return { | ||
| ok: false, | ||
| code: "ZRUI_DRAWLIST_BUILD_ERROR", | ||
| detail: `${builtInto.error.code}: ${builtInto.error.detail}`, | ||
| }; | ||
| } | ||
| submittedBytes = builtInto.bytes; | ||
| inFlight = frameWriter.commit(submittedBytes.byteLength); | ||
| } |
There was a problem hiding this comment.
Handle begin-frame commit failures as backend errors and always release the frame writer.
If frameWriter.commit(...) throws synchronously, this path currently skips explicit writer cleanup and can escape as a generic outer error. Treat it as backend failure and abort the writer before returning.
🛠️ Proposed fix
if (typeof beginFrame === "function") {
const frameWriter = beginFrame();
if (frameWriter) {
const builtInto = this.builder.buildInto(frameWriter.buf);
if (!builtInto.ok) {
frameWriter.abort();
perfMarkEnd("drawlist_build", buildToken);
return {
ok: false,
code: "ZRUI_DRAWLIST_BUILD_ERROR",
detail: `${builtInto.error.code}: ${builtInto.error.detail}`,
};
}
submittedBytes = builtInto.bytes;
- inFlight = frameWriter.commit(submittedBytes.byteLength);
+ try {
+ inFlight = frameWriter.commit(submittedBytes.byteLength);
+ } catch (e: unknown) {
+ frameWriter.abort();
+ perfMarkEnd("drawlist_build", buildToken);
+ return {
+ ok: false,
+ code: "ZRUI_BACKEND_ERROR",
+ detail: describeThrown(e),
+ };
+ }
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/app/widgetRenderer.ts` around lines 3963 - 3982, The
begin-frame path must always release the frameWriter and convert synchronous
commit exceptions into a backend error return: wrap the call to
frameWriter.commit(submittedBytes.byteLength) in a try/catch (and/or finally) so
that if commit throws you call frameWriter.abort(), call
perfMarkEnd("drawlist_build", buildToken) and return a backend failure (e.g. {
ok: false, code: "ZRUI_BACKEND_ERROR", detail: String(err) }); ensure the normal
success path still assigns inFlight from the commit result and that
frameWriter.abort() is invoked for all failure paths so the writer is always
released; refer to BACKEND_BEGIN_FRAME_MARKER, beginFrame, frameWriter,
builtInto, and frameWriter.commit in your changes.
Comment on lines
+101
to
+108
| const cmdEnd = h.cmdOffset + h.cmdBytes; | ||
| let off = h.cmdOffset; | ||
| for (let i = 0; i < h.cmdCount; i++) { | ||
| assert.equal(off < cmdEnd, true, "command cursor in bounds"); | ||
| const opcode = u16(bytes, off); | ||
| const size = u32(bytes, off + 4); | ||
| assert.equal(size >= 8, true, "command size includes header"); | ||
|
|
There was a problem hiding this comment.
Add explicit upper-bound guards before binary reads.
Two reads can currently fail with RangeError instead of a deterministic assertion: command-size overrun in collectResourceTables, and zero/short blob headers before segCount decode.
🔧 Suggested patch
@@
for (let i = 0; i < h.cmdCount; i++) {
assert.equal(off < cmdEnd, true, "command cursor in bounds");
const opcode = u16(bytes, off);
const size = u32(bytes, off + 4);
assert.equal(size >= 8, true, "command size includes header");
+ assert.equal(off + size <= cmdEnd, true, "command size in bounds");
@@
const blobDv = new DataView(blobBytes.buffer, blobBytes.byteOffset, blobBytes.byteLength);
const blobLen = blobBytes.byteLength;
+ assert.equal(blobLen >= 4, true, "blob header in bounds");
const segCount = blobDv.getUint32(0, true);Also applies to: 205-208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/drawlist/__tests__/builder.text-arena.equivalence.test.ts`
around lines 101 - 108, Add explicit upper-bound checks before performing binary
reads to avoid RangeError: in the command loop (where h.cmdOffset, h.cmdBytes,
cmdEnd, off, u16 and u32 are used) validate that off + 2 and off + 8 are <=
cmdEnd (and also <= bytes.length) before calling u16(bytes, off) and u32(bytes,
off + 4), and assert with a clear message if the bounds fail; similarly, in
collectResourceTables (and the blob/header parsing that reads segCount) add
guards that the buffer has at least the expected header length (e.g., bytes
required to read segCount and subsequent fields) before decoding, and assert/die
deterministically instead of allowing a RangeError.
| setTextMeasureEmojiPolicy, | ||
| severityToNum, | ||
| } from "@rezi-ui/core"; | ||
| import type { BackendBeginFrame } from "@rezi-ui/core/backend"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Subpath imports under packages/node/src ==="
rg -n --type ts 'from "@rezi-ui/core/' packages/node/src || echo "No matches found"
echo
echo "=== BackendBeginFrame declarations/exports in core ==="
rg -n --type ts 'BackendBeginFrame' packages/core/src || echo "No matches found"
echo
echo "=== Core root index exports ==="
head -100 packages/core/src/index.ts | cat -nRepository: RtlZeroMemory/Rezi
Length of output: 4468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking what's exported from core root for backend ==="
grep -A 20 'from "./backend' packages/core/src/index.ts | head -25
echo
echo "=== Checking if BackendBeginFrame should be in the export list ==="
rg -n 'export type BackendBeginFrame' packages/core/src/backend.ts
echo
echo "=== Checking backend.ts to see all exports ==="
rg -n 'export (type|const)' packages/core/src/backend.ts | head -20Repository: RtlZeroMemory/Rezi
Length of output: 1668
Add BackendBeginFrame to core root exports and import from package root.
This file uses a subpath import (@rezi-ui/core/backend), which violates the boundary rule for packages/node. Additionally, BackendBeginFrame is not currently exported from the @rezi-ui/core root, so the import requires both changes:
- Add the type to
packages/core/src/index.tsexports (alongside other backend types) - Update the import to use the root package
📝 Changes needed
In packages/core/src/index.ts, add BackendBeginFrame to the backend export block (around line 84-94):
export {
BACKEND_BEGIN_FRAME_MARKER,
BACKEND_DRAWLIST_VERSION_MARKER,
BACKEND_FPS_CAP_MARKER,
BACKEND_MAX_EVENT_BYTES_MARKER,
BACKEND_RAW_WRITE_MARKER,
type BackendRawWrite,
type BackendEventBatch,
+ type BackendBeginFrame,
type RuntimeBackend,
} from "./backend.js";Then in packages/node/src/backend/nodeBackend.ts:
-import type { BackendBeginFrame } from "@rezi-ui/core/backend";
+import type { BackendBeginFrame } from "@rezi-ui/core";Per coding guidelines: packages/node/src/**/*.{ts,tsx} must import from @rezi-ui/core root only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/node/src/backend/nodeBackend.ts` at line 42, Add BackendBeginFrame
to the core package root exports and update the node backend import to use the
package root: modify the core export list in packages core index (add
BackendBeginFrame alongside the other backend types) so it is exported from
"@rezi-ui/core", then change the import in nodeBackend (where it currently
imports BackendBeginFrame from "@rezi-ui/core/backend") to import
BackendBeginFrame from "@rezi-ui/core" instead to satisfy the packages/node
import boundary rule.
Comment on lines
824
to
+892
| try { | ||
| const frameSeq = nextFrameSeq++; | ||
| const fp = frameAudit.enabled ? drawlistFingerprint(drawlist) : null; | ||
| maybeDumpDrawlistBytes("backend-inline", "requestFrame", frameSeq, drawlist); | ||
| if (fp !== null) { | ||
| frameAudit.emit("frame.submitted", { | ||
| frameSeq, | ||
| submitPath: "requestFrame", | ||
| transport: "inline-v1", | ||
| ...fp, | ||
| }); | ||
| frameAudit.emit("frame.submit.payload", { | ||
| frameSeq, | ||
| transport: "inline-v1", | ||
| ...fp, | ||
| }); | ||
| } | ||
| const submitRc = native.engineSubmitDrawlist(engineId, drawlist); | ||
| if (frameAudit.enabled) { | ||
| frameAudit.emit("frame.submit.result", { | ||
| frameSeq, | ||
| submitResult: submitRc, | ||
| }); | ||
| } | ||
| if (submitRc < 0) { | ||
| if (frameAudit.enabled) { | ||
| frameAudit.emit("frame.completed", { | ||
| frameSeq, | ||
| completedResult: submitRc, | ||
| }); | ||
| } | ||
| return Promise.reject( | ||
| new ZrUiError( | ||
| "ZRUI_BACKEND_ERROR", | ||
| `engine_submit_drawlist failed: code=${String(submitRc)}`, | ||
| ), | ||
| ); | ||
| } | ||
| if (frameAudit.enabled) { | ||
| frameAudit.emit("frame.accepted", { frameSeq }); | ||
| } | ||
| const presentRc = native.enginePresent(engineId); | ||
| if (frameAudit.enabled) { | ||
| frameAudit.emit("frame.present.result", { | ||
| frameSeq, | ||
| presentResult: presentRc, | ||
| }); | ||
| } | ||
| if (presentRc < 0) { | ||
| if (frameAudit.enabled) { | ||
| frameAudit.emit("frame.completed", { | ||
| frameSeq, | ||
| completedResult: presentRc, | ||
| }); | ||
| } | ||
| return Promise.reject( | ||
| new ZrUiError("ZRUI_BACKEND_ERROR", `engine_present failed: code=${String(presentRc)}`), | ||
| ); | ||
| } | ||
| if (frameAudit.enabled) { | ||
| frameAudit.emit("frame.completed", { | ||
| frameSeq, | ||
| completedResult: 0, | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| if (frameAudit.enabled) { | ||
| frameAudit.emit("frame.throw", { detail: safeDetail(err) }); | ||
| } |
There was a problem hiding this comment.
Include frameSeq on throw-path audit events.
At Line [891], frame.throw is emitted without frameSeq, so throw events cannot be reliably joined with the corresponding submitted frame.
🔧 Proposed fix
- try {
- const frameSeq = nextFrameSeq++;
+ const frameSeq = nextFrameSeq++;
+ try {
const fp = frameAudit.enabled ? drawlistFingerprint(drawlist) : null;
maybeDumpDrawlistBytes("backend-inline", "requestFrame", frameSeq, drawlist);
if (fp !== null) {
@@
} catch (err) {
if (frameAudit.enabled) {
- frameAudit.emit("frame.throw", { detail: safeDetail(err) });
+ frameAudit.emit("frame.throw", {
+ frameSeq,
+ detail: safeDetail(err),
+ });
}
return Promise.reject(safeErr(err));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| const frameSeq = nextFrameSeq++; | |
| const fp = frameAudit.enabled ? drawlistFingerprint(drawlist) : null; | |
| maybeDumpDrawlistBytes("backend-inline", "requestFrame", frameSeq, drawlist); | |
| if (fp !== null) { | |
| frameAudit.emit("frame.submitted", { | |
| frameSeq, | |
| submitPath: "requestFrame", | |
| transport: "inline-v1", | |
| ...fp, | |
| }); | |
| frameAudit.emit("frame.submit.payload", { | |
| frameSeq, | |
| transport: "inline-v1", | |
| ...fp, | |
| }); | |
| } | |
| const submitRc = native.engineSubmitDrawlist(engineId, drawlist); | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.submit.result", { | |
| frameSeq, | |
| submitResult: submitRc, | |
| }); | |
| } | |
| if (submitRc < 0) { | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.completed", { | |
| frameSeq, | |
| completedResult: submitRc, | |
| }); | |
| } | |
| return Promise.reject( | |
| new ZrUiError( | |
| "ZRUI_BACKEND_ERROR", | |
| `engine_submit_drawlist failed: code=${String(submitRc)}`, | |
| ), | |
| ); | |
| } | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.accepted", { frameSeq }); | |
| } | |
| const presentRc = native.enginePresent(engineId); | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.present.result", { | |
| frameSeq, | |
| presentResult: presentRc, | |
| }); | |
| } | |
| if (presentRc < 0) { | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.completed", { | |
| frameSeq, | |
| completedResult: presentRc, | |
| }); | |
| } | |
| return Promise.reject( | |
| new ZrUiError("ZRUI_BACKEND_ERROR", `engine_present failed: code=${String(presentRc)}`), | |
| ); | |
| } | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.completed", { | |
| frameSeq, | |
| completedResult: 0, | |
| }); | |
| } | |
| } catch (err) { | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.throw", { detail: safeDetail(err) }); | |
| } | |
| const frameSeq = nextFrameSeq++; | |
| try { | |
| const fp = frameAudit.enabled ? drawlistFingerprint(drawlist) : null; | |
| maybeDumpDrawlistBytes("backend-inline", "requestFrame", frameSeq, drawlist); | |
| if (fp !== null) { | |
| frameAudit.emit("frame.submitted", { | |
| frameSeq, | |
| submitPath: "requestFrame", | |
| transport: "inline-v1", | |
| ...fp, | |
| }); | |
| frameAudit.emit("frame.submit.payload", { | |
| frameSeq, | |
| transport: "inline-v1", | |
| ...fp, | |
| }); | |
| } | |
| const submitRc = native.engineSubmitDrawlist(engineId, drawlist); | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.submit.result", { | |
| frameSeq, | |
| submitResult: submitRc, | |
| }); | |
| } | |
| if (submitRc < 0) { | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.completed", { | |
| frameSeq, | |
| completedResult: submitRc, | |
| }); | |
| } | |
| return Promise.reject( | |
| new ZrUiError( | |
| "ZRUI_BACKEND_ERROR", | |
| `engine_submit_drawlist failed: code=${String(submitRc)}`, | |
| ), | |
| ); | |
| } | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.accepted", { frameSeq }); | |
| } | |
| const presentRc = native.enginePresent(engineId); | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.present.result", { | |
| frameSeq, | |
| presentResult: presentRc, | |
| }); | |
| } | |
| if (presentRc < 0) { | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.completed", { | |
| frameSeq, | |
| completedResult: presentRc, | |
| }); | |
| } | |
| return Promise.reject( | |
| new ZrUiError("ZRUI_BACKEND_ERROR", `engine_present failed: code=${String(presentRc)}`), | |
| ); | |
| } | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.completed", { | |
| frameSeq, | |
| completedResult: 0, | |
| }); | |
| } | |
| } catch (err) { | |
| if (frameAudit.enabled) { | |
| frameAudit.emit("frame.throw", { | |
| frameSeq, | |
| detail: safeDetail(err), | |
| }); | |
| } | |
| return Promise.reject(safeErr(err)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/node/src/backend/nodeBackendInline.ts` around lines 824 - 892, The
catch block currently emits frameAudit.emit("frame.throw", { detail:
safeDetail(err) }) without the frameSeq, so update that emit to include frameSeq
(e.g. frameAudit.emit("frame.throw", { frameSeq, detail: safeDetail(err) })) so
throw events can be correlated with the submitted frame; use the existing
frameSeq variable declared at the top of the try and keep the same
safeDetail(err) payload.
Comment on lines
+17
to
+19
| import { appendFileSync, mkdirSync, writeFileSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
| import { performance } from "node:perf_hooks"; |
There was a problem hiding this comment.
Ensure custom audit log directory exists before append.
With nested REZI_FRAME_AUDIT_LOG paths, appendFileSync fails and the catch swallows it, so all audit records are silently dropped.
🛠️ Suggested fix
-import { join } from "node:path";
+import { dirname, join } from "node:path";
@@
let frameAuditDumpCount = 0;
+let frameAuditLogPathPrepared = false;
@@
if (FRAME_AUDIT_LOG_PATH !== null) {
+ if (!frameAuditLogPathPrepared) {
+ mkdirSync(dirname(FRAME_AUDIT_LOG_PATH), { recursive: true });
+ frameAuditLogPathPrepared = true;
+ }
appendFileSync(FRAME_AUDIT_LOG_PATH, `${line}\n`, "utf8");
if (!FRAME_AUDIT_STDERR_MIRROR) {
return;
}Also applies to: 256-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/node/src/frameAudit.ts` around lines 17 - 19, The audit writer
currently calls appendFileSync/writeFileSync on the path derived from
REZI_FRAME_AUDIT_LOG and may fail silently if nested directories don't exist;
before any appendFileSync or writeFileSync call (the occurrences around
REZI_FRAME_AUDIT_LOG and in the functions that perform frame audit writes),
ensure the directory for the audit file exists by calling
mkdirSync(path.dirname(auditPath), { recursive: true }) (or equivalent) and only
then perform append/write; also propagate or log errors instead of swallowing
them so failures are visible.
Comment on lines
+604
to
+611
| } | ||
| if (wrote >= DEBUG_FRAME_RECORD_BYTES) { | ||
| const dvPayload = new DataView(payload.buffer, payload.byteOffset, DEBUG_FRAME_RECORD_BYTES); | ||
| frameAudit.emit("native.frame.summary", { | ||
| reason, | ||
| recordId: recordId.toString(), | ||
| frameId: u64FromView(dvPayload, 0).toString(), | ||
| code, |
There was a problem hiding this comment.
Apply Biome formatter output for DataView blocks.
These ranges match the formatter failure reported in CI; apply biome check --fix (or equivalent) to pass.
Also applies to: 643-644, 672-673
🧰 Tools
🪛 GitHub Actions: ci
[error] 604-611: Formatter would have printed the following content: DataView construction spans multiple lines and formatting changes were suggested (DataView payload conversions and related fields). Run biome check --fix to apply formatting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/node/src/worker/engineWorker.ts` around lines 604 - 611, The
DataView construction/emit blocks are misformatted; run the Biome formatter
(biome check --fix) or reformat the DataView-related blocks so they follow
project style, specifically the block that creates dvPayload (new
DataView(payload.buffer, payload.byteOffset, DEBUG_FRAME_RECORD_BYTES)) and the
subsequent frameAudit.emit call that references reason, recordId,
u64FromView(dvPayload, 0).toString(), and code; also fix the similar DataView
blocks at the other occurrences (around the u64FromView usages at the noted
ranges) so indentation, spacing, and punctuation match the rest of the file and
pass CI.
Comment on lines
+288
to
+297
| if (recordsByPid.size > 0) { | ||
| process.stdout.write(`pids=${recordsByPid.size}\n`); | ||
| process.stdout.write("pid_sessions:\n"); | ||
| const sessionRows = [...recordsByPid.entries()].sort((a, b) => a[0] - b[0]); | ||
| for (const [pid, s] of sessionRows) { | ||
| const modes = [...s.modes].sort().join("|"); | ||
| process.stdout.write( | ||
| ` pid=${pid} records=${s.count} firstTs=${s.firstTs ?? "-"} lastTs=${s.lastTs ?? "-"} modes=${modes}\n`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Filter pid_sessions output when a PID filter is active.
records is filtered at Line 96-Line 98, but pids/pid_sessions are printed from the unfiltered recordsByPid, so filtered reports can show unrelated sessions.
💡 Suggested fix
-if (recordsByPid.size > 0) {
- process.stdout.write(`pids=${recordsByPid.size}\n`);
+const pidSessions = [...recordsByPid.entries()].filter(
+ ([pid]) => pidFilter === null || pid === pidFilter,
+);
+if (pidSessions.length > 0) {
+ process.stdout.write(`pids=${pidSessions.length}\n`);
process.stdout.write("pid_sessions:\n");
- const sessionRows = [...recordsByPid.entries()].sort((a, b) => a[0] - b[0]);
+ const sessionRows = pidSessions.sort((a, b) => a[0] - b[0]);
for (const [pid, s] of sessionRows) {
const modes = [...s.modes].sort().join("|");
process.stdout.write(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (recordsByPid.size > 0) { | |
| process.stdout.write(`pids=${recordsByPid.size}\n`); | |
| process.stdout.write("pid_sessions:\n"); | |
| const sessionRows = [...recordsByPid.entries()].sort((a, b) => a[0] - b[0]); | |
| for (const [pid, s] of sessionRows) { | |
| const modes = [...s.modes].sort().join("|"); | |
| process.stdout.write( | |
| ` pid=${pid} records=${s.count} firstTs=${s.firstTs ?? "-"} lastTs=${s.lastTs ?? "-"} modes=${modes}\n`, | |
| ); | |
| } | |
| const pidSessions = [...recordsByPid.entries()].filter( | |
| ([pid]) => pidFilter === null || pid === pidFilter, | |
| ); | |
| if (pidSessions.length > 0) { | |
| process.stdout.write(`pids=${pidSessions.length}\n`); | |
| process.stdout.write("pid_sessions:\n"); | |
| const sessionRows = pidSessions.sort((a, b) => a[0] - b[0]); | |
| for (const [pid, s] of sessionRows) { | |
| const modes = [...s.modes].sort().join("|"); | |
| process.stdout.write( | |
| ` pid=${pid} records=${s.count} firstTs=${s.firstTs ?? "-"} lastTs=${s.lastTs ?? "-"} modes=${modes}\n`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/frame-audit-report.mjs` around lines 288 - 297, The pid summary is
built from the unfiltered recordsByPid so pid_sessions shows unrelated sessions;
rebuild or filter recordsByPid from the already-filtered records (the same
filteredRecords/records used at the earlier filter) before computing
sessionRows, or apply the same predicate when iterating recordsByPid; update the
code that produces sessionRows/for-loop (variables: recordsByPid, sessionRows,
pid_sessions, records) to use the filtered set so pids and pid_sessions reflect
the active PID filter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
table.layout) to speed future regressionsAGENTS.md,CLAUDE.md, and dev docs navValidation
npx tsc -b packages/core packages/create-rezi packages/nodenpx tsx --test packages/core/src/widgets/__tests__/pagination.test.tsnpx tsx --test packages/create-rezi/templates/starship/src/__tests__/render.test.ts packages/create-rezi/templates/starship/src/__tests__/reducer.test.ts packages/create-rezi/templates/starship/src/__tests__/keybindings.test.ts1..6,t,q), with log analysis via:node scripts/frame-audit-report.mjs /tmp/rezi-frame-audit.ndjson --latest-pidNotes
tmp/untracked in the worktree (not part of this PR).Summary by CodeRabbit
New Features
Documentation
Chores