Skip to content

refactor(core): split runtime commit pipeline by concern#283

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

refactor(core): split runtime commit pipeline by concern#283
RtlZeroMemory merged 1 commit intomainfrom
codex/split-runtime-commit

Conversation

@RtlZeroMemory
Copy link
Copy Markdown
Owner

@RtlZeroMemory RtlZeroMemory commented Mar 17, 2026

Summary

  • Split packages/core/src/runtime/commit.ts into internal helper modules under packages/core/src/runtime/commit/.
  • Kept the public API unchanged.
  • No intended behavior change.

Why

  • Reduce the runtime commit monolith after splitting app/runtime orchestration and widget rendering.

Validation

  • npm run lint
  • npm run typecheck
  • npm run build
  • node scripts/run-tests.mjs --filter "packages/core/dist/runtime/__tests__/"
  • node scripts/run-tests.mjs --filter "packages/core/dist/app/__tests__/widgetRenderer"
  • node scripts/run-tests.mjs --filter "packages/core/dist/__tests__/integration/"
  • node scripts/run-tests.mjs

PTY / Frame Audit Evidence

  • Deterministic viewport used for starship PTY: stty rows 68 cols 300
  • Starship worker run: REZI_STARSHIP_EXECUTION_MODE=worker REZI_STARSHIP_DEBUG=1 REZI_FRAME_AUDIT=1 npx tsx packages/create-rezi/templates/starship/src/main.ts
  • Starship audit report: backend_submitted=5689, worker_payload=5689, worker_accepted=5689, worker_completed=5689, hash_mismatch_backend_vs_worker=0
  • Starship route summary: bridge submitted=4921 completed=4921, comms submitted=768 completed=768
  • Starship debug log captured these exercised paths: cycle-theme, go-comms, comms-hail, toggle-command-palette, quit
  • Starship layout logs showed route transition bridge -> comms and showToastOverlay toggling on and off during the run
  • Animation lab transition run: REZI_FRAME_AUDIT=1 npx tsx packages/create-rezi/templates/animation-lab/src/main.ts
  • Animation lab audit report: backend_submitted=1849, worker_payload=1849, worker_accepted=1849, worker_completed=1849, hash_mismatch_backend_vs_worker=0, DRAW_CANVAS(8): 9245

Summary by CodeRabbit

Refactor

  • Restructured internal runtime commit system for improved code organization and maintainability.
  • Consolidated type definitions and centralized validation utilities for core rendering operations.
  • Enhanced consistency and type safety across the widget rendering pipeline.
  • Optimized equality checking mechanisms for more efficient change detection.

@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

📝 Walkthrough

Walkthrough

This pull request restructures the runtime commit pipeline by decomposing a monolithic commit.ts file into specialized modules. It centralizes type definitions in shared.ts and extracts equality comparisons, composite rendering, container reconciliation, error boundary handling, exit transitions, and validation logic into dedicated modules. The main commit.ts file is reduced from 1851 to 59 lines via re-exports, preserving the behavioral surface while reorganizing internal implementation.

Changes

Cohort / File(s) Summary
Type Centralization
packages/core/src/runtime/commit/shared.ts
New module defining all commit-related types (RuntimeInstance, CommitDiagEntry, CommitFatal, CommitOk, CommitResult, CommitCtx, CommitNodeFn, CommitContainerFn) and constants (NODE_ENV, DEV_MODE, layout thresholds, exit transition duration, focus container kinds).
Equality & Diagnostics
packages/core/src/runtime/commit/equality.ts
New module providing VNode equality utilities (leafVNodeEqual, canFastReuseContainerSelf, diagWhichPropFails), runtime inspection utilities (compositePropsEqual, evaluateAppStateSelections, runtimeChildrenChanged, hasDirtyChild), and diagnostic container __commitDiag with push/reset methods.
Composite Rendering
packages/core/src/runtime/commit/composite.ts
New module managing composite widget rendering: currentCompositeTheme extracts active theme, resolveCompositeChildTheme merges parent/child themes, readCompositeColorTokens resolves color tokens, executeCompositeRender orchestrates full composite render lifecycle including theme/state management, depth validation, hook context creation, and error handling.
Container Reconciliation
packages/core/src/runtime/commit/container.ts
New module handling node path management and container/composite VNode reconciliation: appendNodePath/formatNodePath build hierarchical paths, rewriteCommittedVNode adapts VNode content based on kind, commitContainer performs full reconciliation with theme stack management, fast-path reuse optimization, exit animation scheduling, and dirty flag computation.
Error Boundary Handling
packages/core/src/runtime/commit/errorBoundary.ts
New module for error boundary fallback rendering: captureErrorBoundaryState creates frozen error state objects, commitErrorBoundaryFallback validates fallback function, executes it with error context, and delegates rendering with robust exception handling.
Exit Transitions & Cleanup
packages/core/src/runtime/commit/transitions.ts
New module managing exit animation scheduling: collectSubtreeInstanceIds and markCompositeSubtreeStale traverse/update subtrees, deleteLocalStateForSubtree and createDeferredLocalStateCleanup handle per-node state cleanup, readExitTransition/resolveExitAnimationState extract/normalize transition specs, tryScheduleExitAnimation orchestrates exit scheduling with timing and metadata.
Widget Validation
packages/core/src/runtime/commit/validation.ts
New module providing commit-time validation: warnDev/widgetPathEntry/formatWidgetPath for diagnostics, ensureInteractiveId/ensureFocusContainerId validate widget IDs with duplicate detection and length constraints, commitChildrenForVNode/readVNodeKey/isContainerVNode/isFocusContainerVNode provide type guards and traversal.
Main Commit Refactor
packages/core/src/runtime/commit.ts
Reduced from 1851 to 59 lines: re-exports CommitDiagEntry/CommitFatal/CommitOk/CommitResult/PendingExitAnimation/RuntimeInstance from shared.ts, re-exports __commitDiag from equality.ts, updates commitVNodeTree opts signature to reference CompositeCommitRuntime and CommitErrorBoundaryController types.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • PR #189: Both PRs modify the runtime exit-animation commit path, introducing/using PendingExitAnimation, exitTransition specs, and tryScheduleExitAnimation logic with pending exit animation scheduling through CommitCtx.
  • PR #264: Both PRs refactor container and fragment handling in the commit surface, introducing commitContainer, rewriteCommittedVNode, and isContainerVNode with updated reconciliation semantics.
  • PR #196: Both PRs modify commit-time widget ID validation logic in ensureInteractiveId, including whitespace rejection, duplicate detection, and length constraints with related MAX_INTERACTIVE_ID_LENGTH constant.

Poem

A rabbit hops through commits with care,
Splitting one big file into structures fair,
Types find their home in shared.ts today,
Composite renders, containers at play,
Error bounds catch what falls astray! 🐰✨

🚥 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 'refactor(core): split runtime commit pipeline by concern' directly and clearly describes the main change: splitting a monolithic commit module into separate concern-based modules.

✏️ 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-runtime-commit
📝 Coding Plan
  • Generate coding plan for human review comments

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/core/src/runtime/commit/equality.ts (1)

451-470: Consider extending fast-reuse coverage to additional container types.

canFastReuseContainerSelf only supports fast-reuse checks for fragment, box, row, column, focusZone, focusTrap, and themed. However, isContainerVNode includes 12 additional container kinds: grid, layers, field, tabs, accordion, breadcrumb, pagination, splitPane, panelGroup, resizablePanel, modal, and layer.

These additional container types will always return false from fast-reuse checks, falling through to the slower "new-instance" path even when props haven't changed. Adding prop equality functions for these containers could improve rendering performance in views using these widgets.

Example: Adding fast-reuse support for grid
case "grid":
  return gridPropsEqual(prev.props, (next as typeof prev).props);

This pattern can be applied to the other missing container kinds alongside their corresponding prop equality functions.

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

In `@packages/core/src/runtime/commit/equality.ts` around lines 451 - 470,
canFastReuseContainerSelf currently only handles a subset of container kinds;
extend its switch to include the other container kinds (grid, layers, field,
tabs, accordion, breadcrumb, pagination, splitPane, panelGroup, resizablePanel,
modal, layer) and return the result of their corresponding prop-equality
functions (e.g., gridPropsEqual(prev.props, (next as typeof prev).props),
layersPropsEqual(...), fieldPropsEqual(...), etc.). Add/import any missing
prop-equality helpers and ensure each new case compares prev.props and
next.props in the same pattern used for fragment/box/row, so unchanged props can
take the fast-reuse path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/core/src/runtime/commit/equality.ts`:
- Around line 451-470: canFastReuseContainerSelf currently only handles a subset
of container kinds; extend its switch to include the other container kinds
(grid, layers, field, tabs, accordion, breadcrumb, pagination, splitPane,
panelGroup, resizablePanel, modal, layer) and return the result of their
corresponding prop-equality functions (e.g., gridPropsEqual(prev.props, (next as
typeof prev).props), layersPropsEqual(...), fieldPropsEqual(...), etc.).
Add/import any missing prop-equality helpers and ensure each new case compares
prev.props and next.props in the same pattern used for fragment/box/row, so
unchanged props can take the fast-reuse path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 957b27e6-b6b1-4f30-9c3e-501001f39758

📥 Commits

Reviewing files that changed from the base of the PR and between 743e804 and 0921e82.

📒 Files selected for processing (8)
  • packages/core/src/runtime/commit.ts
  • packages/core/src/runtime/commit/composite.ts
  • packages/core/src/runtime/commit/container.ts
  • packages/core/src/runtime/commit/equality.ts
  • packages/core/src/runtime/commit/errorBoundary.ts
  • packages/core/src/runtime/commit/shared.ts
  • packages/core/src/runtime/commit/transitions.ts
  • packages/core/src/runtime/commit/validation.ts

@RtlZeroMemory RtlZeroMemory merged commit 3a17dda into main Mar 17, 2026
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