fix(core): preserve REPL bindings across sessions#230
Conversation
Persist lexical and TypeScript declarations across Code Mode cells, retain completed failed-cell state, and isolate Host storage in concurrent verification fixtures.
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughCode Mode reusable sessions now preserve broader JavaScript and TypeScript declarations, retain completed mutations after ordinary runtime errors, detect cross-cell declaration conflicts, and dispose sessions when persistence integrity fails. Session diagnostics, guidance, tests, and test storage configuration were updated accordingly. ChangesCode Mode reusable sessions
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant runCodeMode
participant CodeModeSessionManager
participant QuickJsCodeModeReplSession
participant CodeModeDiagnosticsSession
Client->>runCodeMode: submit code with sessionId
runCodeMode->>CodeModeSessionManager: run cell
CodeModeSessionManager->>QuickJsCodeModeReplSession: execute scoped cell
QuickJsCodeModeReplSession-->>CodeModeSessionManager: result and session status
CodeModeSessionManager->>CodeModeDiagnosticsSession: record executed cell
CodeModeSessionManager-->>Client: result and session metadata
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (1)
packages/core/src/code-mode/diagnostics.ts (1)
419-447: 🚀 Performance & Scalability | 🔵 TrivialType-conflict diagnostics recompile the full accumulated declaration set on every diagnose call.
typeDeclarationDiagnostics(viadeclarationDiagnostics) creates a freshts.createProgramover[...previousDeclarations, ...currentDeclarations]on each cell, and this set grows monotonically with the session's lifetime. Combined with the main-program compilation indiagnoseCodeModeTypeScriptand the per-cell program inrecordCell, long-lived REPL sessions will see diagnose latency scale with the number of accumulated type declarations. If sessions are expected to be long, consider bounding retained declarations or caching the compiled declaration program across cells and invalidating only when#typeDeclarationschanges.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/code-mode/diagnostics.ts` around lines 419 - 447, Update typeDeclarationDiagnostics and its declarationDiagnostics caller to avoid rebuilding the full accumulated declaration set on every diagnose call. Cache the compiled declaration program or diagnostics across cells, invalidating the cache only when `#typeDeclarations` changes; alternatively bound retained declarations if that is the established session policy. Preserve conflict deduplication and current diagnostic results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/code-mode/sandbox.ts`:
- Around line 1292-1301: Update the runner’s diagnosticsSession.recordCell
handling to use the settled binding state after
__caplets_settle_failed_declarations runs. Ensure failed const/class
declarations are recorded as let in later-cell diagnostics, while preserving the
original declaration kind for bindings that successfully initialize.
---
Nitpick comments:
In `@packages/core/src/code-mode/diagnostics.ts`:
- Around line 419-447: Update typeDeclarationDiagnostics and its
declarationDiagnostics caller to avoid rebuilding the full accumulated
declaration set on every diagnose call. Cache the compiled declaration program
or diagnostics across cells, invalidating the cache only when `#typeDeclarations`
changes; alternatively bound retained declarations if that is the established
session policy. Preserve conflict deduplication and current diagnostic results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4db20b12-7c71-47c6-b0ba-f2069cae8114
📒 Files selected for processing (14)
.changeset/clever-repls-share.mdpackages/core/src/code-mode/declarations.tspackages/core/src/code-mode/diagnostics.tspackages/core/src/code-mode/runner.tspackages/core/src/code-mode/sandbox.tspackages/core/src/code-mode/sessions.tspackages/core/src/native/tools.tspackages/core/test/attach-service-wiring.test.tspackages/core/test/code-mode-cli.test.tspackages/core/test/code-mode-session.test.tspackages/core/test/code-mode-sessions.test.tspackages/core/test/native.test.tspackages/core/test/remote-control-dispatch.test.tsscripts/check-package-runtime.mjs
|
| Filename | Overview |
|---|---|
| packages/core/src/code-mode/sandbox.ts | Major rewrite replacing snapshot-and-restore with predeclare + scopeProxy + with-statement; adds settleFailedDeclarations for REPL-style error recovery; narrows session-disposal conditions significantly. Well-exercised by the expanded test suite. |
| packages/core/src/code-mode/diagnostics.ts | Tracks valueDeclarations, typeDeclarations, and runtimeKinds separately; adds pre-flight declarationDiagnostics and recordCell for both ok and failed cells. Two minor P2 concerns: projectedTypeDeclarations double-invocation and unbounded interface-merge string growth. |
| packages/core/src/code-mode/sessions.ts | onSuccessfulCell renamed to onExecutedCell; recordExecutedCell now passes settledBindingNames to the diagnostics session. Clean, minimal change. |
| packages/core/test/code-mode-session.test.ts | Large expansion of sandbox-level tests covering const/let/class/enum/namespace persistence, destructured bindings, settled failed declarations, and reversed disposal assertions. |
| packages/core/test/code-mode-sessions.test.ts | New integration tests for the full diagnostics+runtime path with sqlite storage isolation per session. |
| scripts/check-package-runtime.mjs | Adds sqlite storage config to the packaged-runtime smoke check for Host Node storage isolation. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller
participant Runner
participant DiagSession as CodeModeDiagnosticsSession
participant SessionMgr as CodeModeSessionManager
participant QuickJS as QuickJS Sandbox
Caller->>Runner: runCodeMode(code, sessionId)
Runner->>DiagSession: declarationDiagnostics(code)
Note over DiagSession: Check runtimeKinds for block-scoped redeclarations
DiagSession-->>Runner: diagnostics list
alt Diagnostics block execution
Runner-->>Caller: "ok=false, diagnostic_blocked"
else Diagnostics pass
Runner->>SessionMgr: session.run(code)
SessionMgr->>QuickJS: predeclare(declarations)
Note over QuickJS: Checks conflicts, adds to persistBacking, exposes globalThis accessors
QuickJS->>QuickJS: with(scopeProxy) async IIFE with initializeBinding calls
alt Cell succeeds or ordinary runtime error
QuickJS->>QuickJS: settleFailedDeclarations
QuickJS-->>SessionMgr: result with settledBindingNames
SessionMgr->>DiagSession: recordCell(code, declaration, settledBindingNames)
SessionMgr-->>Caller: ok, meta.sessionId preserved
else Timeout or global corruption
QuickJS-->>SessionMgr: session disposed
SessionMgr-->>Caller: "ok=false"
end
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Caller
participant Runner
participant DiagSession as CodeModeDiagnosticsSession
participant SessionMgr as CodeModeSessionManager
participant QuickJS as QuickJS Sandbox
Caller->>Runner: runCodeMode(code, sessionId)
Runner->>DiagSession: declarationDiagnostics(code)
Note over DiagSession: Check runtimeKinds for block-scoped redeclarations
DiagSession-->>Runner: diagnostics list
alt Diagnostics block execution
Runner-->>Caller: "ok=false, diagnostic_blocked"
else Diagnostics pass
Runner->>SessionMgr: session.run(code)
SessionMgr->>QuickJS: predeclare(declarations)
Note over QuickJS: Checks conflicts, adds to persistBacking, exposes globalThis accessors
QuickJS->>QuickJS: with(scopeProxy) async IIFE with initializeBinding calls
alt Cell succeeds or ordinary runtime error
QuickJS->>QuickJS: settleFailedDeclarations
QuickJS-->>SessionMgr: result with settledBindingNames
SessionMgr->>DiagSession: recordCell(code, declaration, settledBindingNames)
SessionMgr-->>Caller: ok, meta.sessionId preserved
else Timeout or global corruption
QuickJS-->>SessionMgr: session disposed
SessionMgr-->>Caller: "ok=false"
end
end
Reviews (2): Last reviewed commit: "fix(core): align failed bindings with di..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Review follow-up on the declaration-diagnostics performance nit: no cache change is needed. |
1 participant
Summary
let,const, class, enum, namespace, and TypeScript declarations across reused Code Mode sessionsVerification
pnpm verifyRelease
@caplets/coreSummary by CodeRabbit
New Features
Bug Fixes
Documentation