Skip to content

refactor(node): split engine worker by concern#285

Merged
RtlZeroMemory merged 1 commit intomainfrom
codex/split-engine-worker
Mar 17, 2026
Merged

refactor(node): split engine worker by concern#285
RtlZeroMemory merged 1 commit intomainfrom
codex/split-engine-worker

Conversation

@RtlZeroMemory
Copy link
Copy Markdown
Owner

@RtlZeroMemory RtlZeroMemory commented Mar 17, 2026

Summary

  • Split packages/node/src/worker/engineWorker.ts into internal helper modules.
  • Kept worker protocol and runtime behavior unchanged.
  • No intended behavior change.

Why

  • Reduce the native worker entrypoint monolith after splitting the node backend orchestration.

Validation

  • npm run lint
  • npm run typecheck
  • npm run build
  • node scripts/run-tests.mjs --filter "packages/node/dist/__tests__/"
  • node scripts/run-tests.mjs --filter "packages/node/dist/__e2e__/" (tooling mismatch: run-tests.mjs only discovers dist/**/__tests__, so this matched 0 files)
  • node --test packages/node/dist/__e2e__/runtime-reduced.e2e.test.js packages/node/dist/__e2e__/terminal_io_contract.e2e.test.js
  • node scripts/run-tests.mjs

PTY/frame-audit evidence

  • Deterministic PTY run executed in worker mode via node-pty with viewport 300x68, then resized to 120x40.
  • Exercised startup, frame submission, key input (2, t, 3), resize, and shutdown (q).
  • Frame audit report: records=3660, backend_submitted=174, worker_payload=174, worker_accepted=174, worker_completed=174, hash_mismatch_backend_vs_worker=0, native_summary_records=174, native_header_records=348.
  • Route summary covered bridge, engineering, and crew.
  • Starship debug log captured route/input/resize events including go-engineering, cycle-theme, viewport change 300x68 -> 120x40, go-crew, and quit.

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced frame auditing with native debugging integration.
    • Added performance metrics collection and snapshot reporting.
  • Refactor

    • Restructured engine worker architecture into modular components.
    • Improved frame transport handling and state management.

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 17, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The EngineWorker module transitions from a monolithic implementation to a modular architecture. Large inline implementations for frame auditing, SAB mailbox management, tick scheduling, performance tracking, and message handling are extracted into dedicated modules. Type definitions are centralized in a shared module. Core runtime state is restructured into typed objects that coordinate between worker subsystems.

Changes

Cohort / File(s) Summary
Main Worker Refactoring
packages/node/src/worker/engineWorker.ts
Extracted implementations into dedicated modules; replaced inline types and scalars with typed state objects (runtimeState, frameAuditState, eventState, tickState); delegated message handling to messageHandlers; updated postToMain signature to support optional transfer parameter; changed control flow from linear in-file logic to module delegation.
Shared Infrastructure
packages/node/src/worker/engineWorker/shared.ts
New module defining comprehensive type system: terminal capabilities, debug stats, native API surface, frame structures (transfer/SAB), state containers, and utility helpers; centralizes type safety and interop contracts for worker subsystems.
Frame Auditing
packages/node/src/worker/engineWorker/frameAudit.ts
New module for advanced frame audit integration with native debug API; provides meta tracking (setFrameAuditMeta, emitFrameAudit, deleteFrameAudit), native enable flow (maybeEnableNativeFrameAudit), and native payload drain logic (drainNativeFrameAudit) with batch processing and error handling.
SAB Mailbox Management
packages/node/src/worker/engineWorker/frameMailbox.ts
New module handling SAB-based frame transport lifecycle; provides slot management (releaseSabSlot, releasePendingFrame), frame lifecycle events (postFrameStatus, postFrameAccepted), SAB frame reads (readLatestSabFrame), and mailbox synchronization (syncPendingSabFrameFromMailbox) with audit integration.
Tick Loop Scheduling
packages/node/src/worker/engineWorker/tickLoop.ts
New module implementing tick loop orchestration; provides timer and immediate scheduling (stopTickLoop, scheduleTickNow, scheduleTick), SAB frame wake coordination (armSabFrameWake), and loop startup (startTickLoop) with epoch tracking and asynchronous wait support.
Performance Tracking
packages/node/src/worker/engineWorker/perf.ts
New module for performance sample collection and statistics; maintains a sliding window of samples (perfRecord), computes per-phase percentiles, averages, and worst-case analysis (perfSnapshot) with a 1024-sample cap.
Message Handlers
packages/node/src/worker/engineWorker/messageHandlers.ts
New module providing comprehensive message handler functions for all message types (init, frame, frameKick, config, events, debug, shutdown, perf snapshot); manages engine lifecycle, frame state, event batching, native API calls, and inter-thread communication with centralized error handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 Modular dreams in workers' threads,
State flows clean through shared beds,
Audit, mailbox, tick—each plays its part,
One big file now beats as an orchestra's heart!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: a refactoring that splits the monolithic engine worker module into smaller, focused modules organized by concern.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/split-engine-worker
📝 Coding Plan
  • Generate coding plan for human review comments

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

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

@RtlZeroMemory RtlZeroMemory merged commit 49f2997 into main Mar 17, 2026
31 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant