perf(web): saving the thread list cache no longer re-encodes every thread

[bompus]

Saving the thread list cache in the web client now encodes only the threads that changed. With 1,417 real threads (4.6 MB), a save after one thread changes drops from 791 ms to 11 ms of UI-thread work in Chrome, and the saved bytes are identical.

Problem

Every saveShell in the web client ran a full Schema.encodeEffect over every thread row, even when one row had changed. The shell cache saves every 10 seconds while threads update, so a large thread list kept the UI thread busy for most of a second each time. encodeShellSnapshotForCache had kept this cheap before the V2 orchestrator (#2829) removed it.

Change

• Mobile's row-reuse encoder (makeStoredShellSnapshotEncoder, perf(mobile): reuse encoded rows in shell cache saves #16129) moves from apps/mobile/src/connection/shell-cache-encoding.ts to packages/client-runtime/src/platform/shellCacheEncoding.ts, with its tests. Shell rows are immutable and kept by reference, so the encoder remembers each row's encoding and only runs the row codec for new rows.
• Web saveShell uses it and yields between chunks with a MessageChannel task. A zero-delay timer is clamped to 4 ms after a few nested calls and throttled to about once a second in background tabs, and the shell flush also runs on disconnect.
• Mobile imports the encoder from client-runtime and keeps its default setTimeout(0) yield. Its behavior is unchanged.
• The output stays byte-for-byte equal to Schema.fromJsonString(StoredOrchestrationShellSnapshot). The web client now keeps one encoded copy of each live row, as mobile already does: about 15 MB of heap for 1,600 rows with 10 pull request links each.

Scope and approval

Fixes #18926, which was triaged as an accepted regression. The triage comment asked for exactly this scope: move the encoder into client-runtime, call it from saveShell, keep byte equality with a test like mobile's, keep mobile's yield, use a browser-friendly yield on web, and leave payload size for #18907.

Verification

• Main's 9 encoder tests moved verbatim into packages/client-runtime/src/platform/shellCacheEncoding.test.ts: byte equality, row reuse, chunked yields, invalid rows, concurrent saves, interruption and failed encodes. 9/9 pass. The mobile store tests that stay behind pass (8/8), as do web storage.test.ts (10/10).
• Measured above in the T3 browser panel (Chrome 154) on an isolated dev server seeded from a copy of real data. The page loaded both encoders and timed them on the same 1,417-thread snapshot the app had just saved. The new encoder's output matched main's byte for byte on every save.
• vp run typecheck in client-runtime, web and mobile, plus targeted lint and format: clean.
• Two independent review rounds (Claude Opus 5.5 and GPT-6.1 Sol) found no issues.
• Not measured in Safari or the packaged desktop app.

Original work by @bompus, taken over and finished by maintainers.

Claude Opus 5.5 and Claude Fable 5.1 in Claude Code, with review by GPT-6.1 Sol in Codex.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR not approvable — This production-path change replaces web shell-cache encoding with shared memoization and browser task yielding, while relocating the existing mobile encoder and adding focused tests. It also adds a file-level directive suppressing two static-analysis diagnostics, so the change requires human review.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 88a582e9-cc8e-4d39-a862-c49f40ab46a5

📥 Commits

Reviewing files that changed from the base of the PR and between 1e02b3d and 212601b.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: c78907b4-6651-424c-80a3-9d3ea24cdc1b

📥 Commits

Reviewing files that changed from the base of the PR and between da89192 and 1e02b3d.

📒 Files selected for processing: 4

• apps/mobile/src/connection/environment-cache-store.test.ts
• apps/web/src/connection/storage.ts
• packages/client-runtime/src/platform/shellCacheEncoding.test.ts
• packages/client-runtime/src/platform/shellCacheEncoding.ts

💤 Files with no reviewable changes: 1

• apps/mobile/src/connection/environment-cache-store.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

The shared shell snapshot encoder now accepts a caller-supplied yield effect. The web client supplies a MessageChannel-based browser-task yield, and the mobile client imports the encoder from the client-runtime platform package.

Changes: Shell snapshot encoding

Layer / File(s)	Summary
Configurable cached encoder packages/client-runtime/src/platform/shellCacheEncoding.ts, packages/client-runtime/src/platform/shellCacheEncoding.test.ts, packages/client-runtime/src/platform/index.ts	The encoder accepts an optional yield effect and defaults to a zero-delay timer. Tests cover schema-equivalent output, chunk yields, interruption, validation failures, concurrent saves, row reuse, and recovery after failure. The platform index re-exports the encoder.
Web and mobile integration apps/web/src/connection/storage.ts, apps/mobile/src/connection/environment-cache-store.ts, apps/mobile/src/connection/environment-cache-store.test.ts	The web client supplies a MessageChannel-based yield to the encoder and closes the receiving port on interruption. The mobile client imports the encoder from the platform package. Its removed encoder tests are covered by the new client-runtime tests.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant saveShell
  participant makeStoredShellSnapshotEncoder
  participant yieldToBrowserTask
  participant MessageChannel
  saveShell->>makeStoredShellSnapshotEncoder: encode stored shell snapshot
  makeStoredShellSnapshotEncoder->>yieldToBrowserTask: yield during encoding
  yieldToBrowserTask->>MessageChannel: wait for browser task message
  MessageChannel-->>yieldToBrowserTask: deliver message
  yieldToBrowserTask-->>makeStoredShellSnapshotEncoder: resume encoding

Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 1e02b

The reviewed snapshot-encoding change is mergeable after normal checks; no unresolved behavior issue was established.

Security architecture risk: ⚪ Minimal · up to 1e02b

The shared encoder retains schema validation and environment-specific storage without adding permissions or a remotely callable operation. The inspected persistence flow preserves save ordering and writes only after encoding succeeds. No material security risk introduced or worsened by this change was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed behavior affects shell-cache serialization across environments handled by a client storage instance. The compared code adds no credential, network, filesystem, or server-side authority; the MessageChannel carries only a local scheduling notification.

Trust Boundaries and Controls

• observed — The injected yield effect comes from application configuration, not snapshot data. Web retains environment-keyed storage and checks the decoded envelope’s environment ID before returning a cached snapshot. The shared row cache does not cache environment envelopes.

Resilience and Maintainability Implications

• inferred — Failure or interruption before persistence can leave only successfully validated rows cached in memory, not a partially written snapshot. Repeating an encode safely reuses those rows under the immutable-row invariant. Timer cancellation and browser receiving-port cleanup are present in source; invalid persisted caches retain their discard path.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		Issue [#18926] requires shared row-reuse encoding, web integration, byte-equal output, the existing mobile yield, and a browser-appropriate web yield. The PR moves makeStoredShellSnapshotEncoder to …
Out of Scope Changes check		The changes stay within [#18926]. The shared export, mobile import update, test relocation, row-reuse implementation, and browser task yield directly support the issue. The PR does not change payload …
Title check		The title clearly and concisely identifies the main web performance change: saving the thread list cache no longer re-encodes unchanged threads.
Description check		The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the regression, identifies issue #18926, documents implementation details, reports foc…

✨ Finishing Touches: 💡 1

🧪 Generate unit tests (beta)

• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[juliusmarminge]

Note

Claude Opus 5.5 responding on behalf of Julius.

Taking over this one. I will push follow-up commits to this branch on top of yours and merge main in rather than rebasing.