streams review: Stages 4–7 (browser robustness, perf, dead code, docs)#1479
Merged
Conversation
…Stage 6) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ull-state reparse (Stage 5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- design.md: status banner; fix 1-based offsets + core-event examples, callable subscriber spec, SQLite/chunking storage, live-tail replayAfterOffset default, OPFS mirror; flag never-shipped implementProcessor/connectStream sections as superseded. - README.md: real browser export (withStreamConnectionFromBrowser), sync Disposable, ?path= route map; add append/subscription semantics section. - ADR 0001: supersede per-view-runtime claim with shipped per-(ns,path,slug) singleton model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t (D4) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, lock + GC fixes (Stage 4) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…E4/E7 obsolete/declined Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…adlock Cursor Bugbot (High): ingestWithSelfHeal and connection recovery shared reconnectTimer with conflicting guards — a socket close during the ingest backoff would no-op (reconnectAfter saw the timer armed) and the backoff callback would no-op (stream had been cleared), leaving the runtime stuck disconnected. - Single scheduleReconnect(error, delayMs): tears down synchronously and bumps the connection epoch so the superseded connection's late close/error can't reset an in-flight backoff. Both the close path (1s) and the ingest self-heal (exponential backoff) route through it. - Stop resetting ingestFailureCount on resubscribe: a clean resubscribe doesn't mean the failing batch will apply, so resetting let a poison batch busy-loop at the floor delay. It now only resets on a successful ingest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
@/tmp/pr-comment.md |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9bcb9b6. Configure here.
…enWriter on release Two Cursor Bugbot findings (Medium): - Missing incarnation when meta absent: reconcileLocalMirrorWithServer only rebuilt on an incarnation *mismatch*, skipping a non-empty mirror that had no recorded incarnation (predates incarnation tracking). A server reset() that caught back up to the same maxOffset would then keep stale local rows. Now any case where the local incarnation != server incarnation — including unrecorded — rebuilds, since continuity can't be verified. - Aborted lock stalls writer election: release() aborted a not-yet-granted Web Lock request but whenWriter only resolved inside the grant callback, so it could dangle forever. release() now also settles whenWriter. Every teardown path invalidates ownership (disposed / epoch bump / stream=undefined) synchronously before the resolved .then microtask runs, so the election always bails via ownsRuntime() — it can't act after release. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
@/tmp/pr-comment2.md |
jonastemplestein
added a commit
that referenced
this pull request
Jun 11, 2026
The packages/streams adversarial review is fully shipped — Stages 0/1/3 (#1455/#1458/#1459), Stages 4–7 (#1479), Stage 2 deliberately abandoned. This deletes the now-stale `tasks/streams-review-fixes.md` and removes the dangling pointers to it from the regression-test comments (the finding ids and the PRs in git history are the durable record). Doc/comment-only; no code changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation and comment-only changes; no runtime or test behavior changes. > > **Overview** > Removes the completed **`tasks/streams-review-fixes.md`** tracking doc now that the June 2026 `packages/streams` review work is shipped (Stages 0/1/3 via #1455/#1458/#1459, Stages 4–7 via #1479; Stage 2 abandoned). > > Regression test file headers are updated so they no longer link to that task file. Comments now cite **review finding ids (C*/M*)** and the **merged PRs** as the durable record instead. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 7c33b86. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- CLOUDFLARE_PREVIEW --> ## Environment Config Lease <!-- CLOUDFLARE_PREVIEW_STATE --> <!-- { "apps": { "os": { "appDisplayName": "OS", "appSlug": "os", "status": "deployed", "updatedAt": "2026-06-11T09:38:48.453Z", "headSha": "7c33b86fa2d6f11b18ad8e99bc42c78940c8a623", "message": null, "publicUrl": "https://os.iterate-preview-2.com", "runUrl": "https://github.com/iterate/iterate/actions/runs/27337613329", "shortSha": "7c33b86" } }, "environmentConfigLease": { "dopplerConfig": "preview_2", "leasedUntil": 1781174051088, "leaseId": "82ae23a4-0588-41db-bf69-772ac5bb0d87", "slug": "preview-2", "type": "environment-config-lease" } } --> <!-- /CLOUDFLARE_PREVIEW_STATE --> Lease: `preview-2` Doppler config: `preview_2` Type: `environment-config-lease` Leased until: 2026-06-11T10:34:11.088Z ### OS Status: deployed Commit: `7c33b86` Preview: https://os.iterate-preview-2.com [Workflow run](https://github.com/iterate/iterate/actions/runs/27337613329) Updated: 2026-06-11T09:38:48.453Z <!-- /CLOUDFLARE_PREVIEW --> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Streams review — Stages 4–7 (browser robustness, perf, dead code, docs)
The final batch of the
packages/streamsadversarial review (tasks/streams-review-fixes.md), bundled into one PR per request. Stages 0/1/3 already shipped (#1455, #1458, #1459); Stage 2 (lazy init) was abandoned — #1460's subscriber-presence model appends asubscriber-connectedfact on everysubscribe(), which fundamentally conflicts with "don't initialize storage until the firstappend()".created/wokenstay eager.All findings were re-verified against current
mainafter #1460/#1457 moved the codebase; items those PRs already fixed or made obsolete are called out below.Stage 4 — browser-runtime correctness
ingest, so a browser mirror that fails to apply a batch silently desyncs forever. The browser now self-heals:ingestWithSelfHealresubscribes from the persisted checkpoint with bounded exponential backoff on ingest failure.appendBatch/runtimeStateawait connection readiness (whenStreamReady) instead of throwing "disposed" during a transient reconnect; only throw when actually disposed.ROLLBACKinstream-db.worker.tsbatch()is now in its own try/catch so it can't mask the original error (whichwithBusyRetry/isBusyErrorneed to see).createdAt+ amirror_metatable): rebuild the mirror on incarnation change rather than trusting the offset comparison after areset().AbortSignalon the Web Lock request (released on disposal) + the request rejection is surfaced instead ofvoid-swallowed.#onChange, and equality-check before notifying to avoid spurioususeSyncExternalStorechurn.Stage 5 — performance
browser-event-feedO(n²) write amplification: coalesce to one op perlocal_index(was a cumulative op per event, each re-serializing the whole accumulated array).MAX_GROUP_EVENTS = 200so one dominant event type can't grow a single blob unboundedly.stateSchema.parsein corereduce; state is validated only at the KV/recovery trust boundary now.eventTypes; filtering is server-side). No change.Stage 6 — dead code / elegance (~247 lines deleted)
shared/stream-processors.ts; E2waitForOpen; E3 unreachable circuit-breakerpausedguard; E5 deadwaitForEvent/messageInbox.errormachinery insubscription.ts; E6 redundant circuit-breakerconsumesentries.processor-registered; the sharedcircuit-breaker-typesis a live, separately-imported hierarchy, not dead code). E7 declined — post-streams: subscriber presence facts + reconciler homogenization #1460 the subscribe override genuinely needs to retain the client callback / wireonRpcBroken, so folding it into the genericmakeRpcTargetClassisn't worth it.Stage 7 — docs
design.md: status banner, fixed 1-based offsets + core-event examples, the callable subscriber spec, SQLite/512KB-chunking storage, live-tailreplayAfterOffsetdefault, OPFS mirror; superseded banners on the never-shippedimplementProcessor/connectStreamsections.README.md: real browser export (withStreamConnectionFromBrowser), syncDisposable,?path=route map; new "Append & subscription semantics" section (D5).(namespace, path, slug)singleton model.beforeAppend→validateAppend,afterAppendBatch→processEventBatch).Testing
pnpm typecheck,pnpm lint, node (79) + workers (15) suites, and example-app typecheck all green. Example-app Playwright e2e run locally against a cleanvite dev(real Stream DO via Miniflare): 26 passed.origin/mainmerged in (nopackages/streamsoverlap; the new project-config stream-processor consumers don't touch any deleted exports).🤖 Generated with Claude Code
Note
Medium Risk
Touches browser reconnect, mirror discard, and ingest self-heal paths that affect live viewers and local SQLite mirrors; server stream DO behavior is mostly unchanged aside from core reduce hot-path optimization.
Overview
Closes out Stages 4–7 of the
packages/streamsadversarial review: browser mirror reliability, feed/core hot-path perf, dead-code removal, and doc alignment. Stage 2 (lazy stream init) stays abandoned and is only noted intasks/streams-review-fixes.md.Browser runtime (
stream-browser-store.tsand friends) is reworked so the OPFS mirror can recover instead of silently drifting. Inbound delivery is fire-and-forget, so failedingestnow triggersingestWithSelfHeal(resubscribe from the last checkpoint with capped exponential backoff). Reconnects go through onescheduleReconnectpath with a connection epoch so stale WebSocket callbacks cannot tear down the replacement connection.appendBatch/runtimeStateusecallWhenReady/whenStreamReadyduring transient reconnects instead of throwing “disposed”. Mirror trust uses servercreatedAtas incarnation identity via newmirror_metahelpers instream-browser-db.ts. Web Lockrelease()aborts pending lock requests; the SQLite worker preserves the original batch error ifROLLBACKfails; reactive queries GC earlier, skip unobserved refreshes, and avoid notify when snapshots are unchanged.Performance:
browser-event-feedplanFeedOpscoalesces one SQL op per group row and caps groups atMAX_GROUP_EVENTS(200). Corereduceno longer exit-parses the fullstateSchemaon every appended event.Cleanup: ~247 lines removed (unused
stream-processorsexports,waitForOpen, dead subscriptionwaitForEventmachinery, trimmed circuit-breakerconsumesand an unreachable guard). Docs:README(real browser API,?path=routes, append/subscription semantics),design.mdand ADR 0001 marked superseded where they diverge from shipped code.Reviewed by Cursor Bugbot for commit f52f305. Bugbot is set up for automated code reviews on this repo. Configure here.
Environment Config Lease
No active environment config lease.
OS
Status: released
Commit:
f52f305Preview: https://os.iterate-preview-3.com
Summary: Preview app released.
Workflow run
Updated: 2026-06-10T22:05:11.534Z