refactor(node): split node backend by concern#284
Merged
RtlZeroMemory merged 1 commit intomainfrom Mar 17, 2026
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis pull request refactors the Node backend's internal state management by extracting monolithic inline logic into dedicated, modular state objects and helpers. Frame tracking, debug operations, execution mode selection, and frame transport are moved from embedded code into separate modules with structured APIs for state initialization, lifecycle management, and error handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
📝 Coding Plan
Comment Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9425d625-1727-476a-bd72-72c50c139942
📒 Files selected for processing (7)
packages/node/src/backend/nodeBackend.tspackages/node/src/backend/nodeBackend/debugChannel.tspackages/node/src/backend/nodeBackend/executionMode.tspackages/node/src/backend/nodeBackend/frameTracking.tspackages/node/src/backend/nodeBackend/frameTransport.tspackages/node/src/backend/nodeBackend/shared.tspackages/node/src/backend/nodeBackendInline.ts
Comment on lines
+40
to
+65
| export function selectNodeBackendExecutionMode( | ||
| input: NodeBackendExecutionModeSelectionInput, | ||
| ): NodeBackendExecutionModeSelection { | ||
| const { requestedExecutionMode, fpsCap } = input; | ||
| const resolvedExecutionMode: "worker" | "inline" = | ||
| requestedExecutionMode === "inline" | ||
| ? "inline" | ||
| : requestedExecutionMode === "worker" | ||
| ? "worker" | ||
| : fpsCap <= 30 | ||
| ? "inline" | ||
| : "worker"; | ||
| return { | ||
| resolvedExecutionMode, | ||
| selectedExecutionMode: resolvedExecutionMode, | ||
| fallbackReason: null, | ||
| }; | ||
| } | ||
|
|
||
| export function assertWorkerEnvironmentSupported(nativeShimModule: string | undefined): void { | ||
| if (nativeShimModule !== undefined) return; | ||
| if (hasInteractiveTty()) return; | ||
| throw new ZrUiError( | ||
| "ZRUI_BACKEND_ERROR", | ||
| 'Worker backend requires a TTY when using @rezi-ui/native. Use `executionMode: "inline"` for headless runs or pass `nativeShimModule` in test harnesses.', | ||
| ); |
There was a problem hiding this comment.
Handle headless auto fallback here.
selectNodeBackendExecutionMode() never uses hasAnyTty or nativeShimModule, so selectedExecutionMode can never differ from resolvedExecutionMode and fallbackReason is dead. In headless executionMode: "auto" runs with fpsCap > 30, createNodeBackendInternal() will still choose the worker backend and then fail at start() instead of selecting inline, and nativeShimModule: "" also bypasses the environment check.
Suggested fix
export function selectNodeBackendExecutionMode(
input: NodeBackendExecutionModeSelectionInput,
): NodeBackendExecutionModeSelection {
- const { requestedExecutionMode, fpsCap } = input;
+ const { requestedExecutionMode, fpsCap, nativeShimModule, hasAnyTty } = input;
const resolvedExecutionMode: "worker" | "inline" =
requestedExecutionMode === "inline"
? "inline"
: requestedExecutionMode === "worker"
? "worker"
: fpsCap <= 30
? "inline"
: "worker";
+ const hasNativeShim =
+ typeof nativeShimModule === "string" && nativeShimModule.length > 0;
+ const canUseWorker = hasNativeShim || hasAnyTty;
+ const selectedExecutionMode =
+ requestedExecutionMode === "auto" && resolvedExecutionMode === "worker" && !canUseWorker
+ ? "inline"
+ : resolvedExecutionMode;
return {
resolvedExecutionMode,
- selectedExecutionMode: resolvedExecutionMode,
- fallbackReason: null,
+ selectedExecutionMode,
+ fallbackReason:
+ selectedExecutionMode !== resolvedExecutionMode
+ ? "worker backend requires a TTY or native shim; falling back to inline"
+ : null,
};
}
export function assertWorkerEnvironmentSupported(nativeShimModule: string | undefined): void {
- if (nativeShimModule !== undefined) return;
+ if (typeof nativeShimModule === "string" && nativeShimModule.length > 0) return;
if (hasInteractiveTty()) return;
Owner
Author
There was a problem hiding this comment.
Keeping this unchanged in this PR. The extracted selectNodeBackendExecutionMode() and assertWorkerEnvironmentSupported() logic are mechanically preserved from origin/main, and this refactor is intentionally scoped to extraction/wiring only. Changing auto to headless-fallback inline would be a behavioral change to worker/inline selection semantics, which is out of scope for this backend split. The current docs in docs/backend/node.md also describe auto as fpsCap <= 30 => inline, otherwise worker. If we want headless auto fallback, that should land as a separate behavior PR with updated tests/docs.
There was a problem hiding this comment.
@RtlZeroMemory, understood — preserving the extracted logic verbatim from origin/main is the right call for a refactor-only PR. I'll drop the suggestion.
Would you like me to open a follow-up issue to track:
- Headless
automode fallback to inline when no TTY/native-shim is present (hasAnyTty/nativeShimModuleintegration inselectNodeBackendExecutionMode()). - Tightening the
assertWorkerEnvironmentSupportedguard to reject empty-stringnativeShimModule(i.e.,nativeShimModule.length > 0).
Both changes would need updated tests and a docs/backend/node.md update before landing.
( ˶ᵔ ᵕ ᵔ˶ )
✏️ Learnings added
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 284
File: packages/node/src/backend/nodeBackend/executionMode.ts:40-65
Timestamp: 2026-03-17T13:58:17.665Z
Learning: In `packages/node/src/backend/nodeBackend/executionMode.ts`, `selectNodeBackendExecutionMode()` and `assertWorkerEnvironmentSupported()` are mechanically extracted from `origin/main` with no behavioral changes. The `auto` mode resolves solely via `fpsCap <= 30 => inline`, otherwise `worker` — headless auto-fallback (using `hasAnyTty`/`nativeShimModule`) is intentionally deferred to a separate behavior PR with updated tests and docs. Do not flag missing headless-fallback logic as a bug in extraction/refactor PRs targeting this file.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 281
File: packages/core/src/app/createApp/eventLoop.ts:97-125
Timestamp: 2026-03-17T08:37:02.317Z
Learning: In `packages/core/src/app/createApp/eventLoop.ts`, the `commitUpdates()` catch block intentionally calls `options.fatalNowOrEnqueue` without rethrowing after a thrown state updater, allowing the current turn to continue. This control flow is carried over unchanged from the pre-split `createApp.ts` and is considered correct semantics for this runtime path. Do not flag it as a bug in extraction/refactor PRs.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: For rendering/layout/theme regressions, include mandatory live PTY validation: run app in PTY with deterministic viewport, exercise routes, capture REZI_FRAME_AUDIT logs, analyze with frame-audit-report.mjs, include concrete evidence in report
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 281
File: packages/core/src/app/createApp/renderLoop.ts:327-341
Timestamp: 2026-03-17T08:37:29.774Z
Learning: In `packages/core/src/app/createApp/renderLoop.ts`, the post-submit callback ordering is intentional: `emitFocusChangeIfNeeded()`, `emitInternalRenderMetrics()`, and `emitInternalLayoutSnapshot()` run *before* `scheduleFrameSettlement()` is attached in both the raw and widget render branches. This ordering is preserved from the pre-split `createApp.ts` on `origin/main`. Reordering would change `framesInFlight`/ack timing and callback sequencing in the runtime orchestrator. Do not flag this as a bug in extraction/refactor PRs.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to packages/core/src/runtime/{commit.ts,reconcile.ts,router/wheel.ts}|packages/core/src/app/createApp.ts|packages/core/src/layout/**|packages/core/src/renderer/**|packages/core/src/drawlist/**|packages/core/src/binary/** : Extra care required when modifying danger zones: runtime commit/reconcile, wheel router, app creation, layout engine, renderer, drawlist, and binary modules; must verify runtime invariants and reconciliation behavior
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Risk triage: docs-only updates (low risk) need lint/grep validation; widget prop renames (medium risk) need compile + affected tests + docs parity; runtime/router/reconcile/layout/drawlist changes (high risk) need full test suite + integration coverage + PTY evidence
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Agent coordination: assign clear ownership by path/purpose, keep one agent as integration owner, split by independent concerns (API/type changes, runtime behavior, JSX parity, docs)
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 216
File: scripts/drawlist-spec.ts:1-3
Timestamp: 2026-02-26T10:29:26.658Z
Learning: In the Rezi project, public API references and documentation must consistently use the current public protocol version (v1) after the reset, and must not reference the old v6 iteration. Review tests, scripts, and docs (including scripts/drawlist-spec.ts) to replace any v6 mentions with v1, and add a short note in release/docs where protocol versions are referenced to reflect the reset.
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
packages/node/src/backend/nodeBackend.tsinto internal helper modules.nodeBackend.ts/nodeBackendInline.tscycle.Why
Reduce the worker-backend monolith and remove a real architectural cycle before touching the worker entrypoint.
Validation
npm install(required in the fresh worktree so repo tooling and native deps were present)npm run lintnpm run typechecknpm run buildnode scripts/run-tests.mjs --filter "packages/node/dist/__tests__/"node scripts/run-tests.mjs --filter "packages/node/dist/__e2e__/"(matched 0 files in this repo's test runner)node scripts/run-e2e.mjsnpm run build:nativenode scripts/run-tests.mjsPTY / Frame Audit Evidence
300x68, then resized to120x40REZI_STARSHIP_EXECUTION_MODE=worker REZI_FRAME_AUDIT=1 npx tsx packages/create-rezi/templates/starship/src/main.tsnode scripts/frame-audit-report.mjs /tmp/rezi-frame-audit.ndjson --latest-pidrecords=17067pid_filter=3791backend_submitted=812worker_payload=812worker_accepted=812worker_completed=812hash_mismatch_backend_vs_worker=0missing_worker_payload=0missing_worker_accepted=0missing_worker_completed=0engineering: submitted=544 completed=544 avgBytes=58970.0 avgCmds=645.0bridge: submitted=255 completed=255 avgBytes=115692.4 avgCmds=826.7crew: submitted=13 completed=13 avgBytes=27740.9 avgCmds=471.6go-engineering,go-crew,cycle-theme,quit, and resize to120x40Summary by CodeRabbit
New Features
selectNodeBackendExecutionModefunction for public use in execution mode selection.Refactor