Skip to content

perf(do): stop over-sending on the live-query WebSocket path - #407

Merged
prisis merged 6 commits into
alphafrom
perf/ws-paginated-page-deltas
Aug 11, 2026
Merged

perf(do): stop over-sending on the live-query WebSocket path#407
prisis merged 6 commits into
alphafrom
perf/ws-paginated-page-deltas

Conversation

@prisis

@prisis prisis commented Aug 11, 2026

Copy link
Copy Markdown
Member

Supersedes #406, which was stacked under this branch and is now folded in and
closed. This PR carries all five commits, rebased onto alpha.

Summary

Live-query fan-out (@lunora/shard-engine, @lunora/do) was sending more bytes
than it needed to, in three ways. All three are fixed here, plus an advisor lint
(@lunora/advisor, @lunora/codegen) for the fourth — the one only the app
author can fix — and a correctness bug found while reviewing the third.

1. Delta-vs-snapshot was decided by row count

A refresh went out as one {type:"delta"} frame per changed row whenever the
diff was expressible, capped only by "no more deltas than rows".

Every delta frame re-pays the whole {"type":"delta","id":…} + "table":… +
cursor/epoch/watermark envelope around one row body — which the snapshot
pays once for the entire list. So the deltas stop being the cheaper encoding
well before they stop being a valid one, and a row count cannot see where.
Measured, 100-row list of ~290-byte rows:

changed snapshot deltas ratio
1 29,060 B 438 B 0.02x
50 29,060 B 21,980 B 0.76x
100 29,060 B 43,970 B across 100 frames 1.51x

The cap was blind in the other direction too: a single delta into a short list
of small rows already costs more than re-sending the list, and it allowed that
as well.

Now both candidates are rendered and the smaller one is sent — stopping the
render as soon as the running delta total reaches the snapshot, so a near-total
change does not allocate envelopes that lose. subscriptionFrames owns the
frame layout and the choice together, so the sizing cannot drift from the format
— it is the format. Ties go to the snapshot, since one frame is also one
ws.send and one client apply.

2. Paginated live queries never diffed at all

.paginate() returns { page, isDone, continueCursor } — an object, so the row
diff rejected it at both ends. Every write re-sent the whole page, and
usePaginatedQuery holds one subscription per loaded page.

Measured, one row updated:

page size before after saved
25 rows 6,597 B 412 B 93.8%
50 rows 13,072 B 412 B 96.8%

Scope caveat: reactive pagination also returns a splitCursor derived from
the page's midpoint row, outside page. An insert or delete that shifts the
midpoint changes it and correctly forces a snapshot — so those numbers describe
row updates, the common live-query case, not every write.

This needs capability negotiation rather than a version bump: a client that
cannot merge into page does not ignore a page delta — applyDelta returns
undefined and the caller replaces the entire query value with the raw delta
object. So connect gains an optional caps: string[], and the server only
diffs a paginated result for a socket that announced "pageDelta".

Fail-closed by construction: absent caps means snapshots, exactly what every
client did before. The seven non-JS SDKs already send connect with no caps,
so they need no changes. The delta frame format is unchanged too.

3. Inserted rows could land in the wrong place

An insert delta carries no position, so the client picks one with a
_creationTime heuristic — but paginateOrderKeys orders a .withIndex() page
by the index fields. The two can legitimately disagree, and
survivorsKeepOrder does not catch it (the survivors did keep their order;
only the newcomer moves). Reproduced end to end:

server order: a,n,b,c
client order: n,a,b,c

This desyncs permanently: the server advances its diff baseline to the value
it believes the client holds, so every later update lands at an index the client
has wrong, and nothing reconciles until a reconnect.

The server now replays the client's placement before sending and falls back to a
snapshot on mismatch — using the same function the client merges with, so the
two cannot drift. The bug predates paginated deltas (a bare
.withIndex().collect() diverged identically), so this fixes that path too.

4. unbounded_collect (new advisor lint)

filter_without_index gates on hasFilter, so the widest read of all —
ctx.db.query("t").collect() with no index and no filter — fell straight
through it. The codegen feeder made that structural: it discarded every
discovered read without a .filter(), so no lint could have seen one.

An unindexed read records a whole-table dependency, so the refresh gate can
never skip it: every write re-runs the query and re-sends the whole table to
every subscribed socket. Two of this repo's own examples trip it.

withGeoIndex also joins the narrowing methods — it was missing while only
filtered reads were reported, and would otherwise have flagged the exact chain
geo_index_unused tells authors to write.

Linked issues

None — found by a WebSocket payload audit of the subscription fan-out path.

Test plan

  • pnpm run lint:affected:types — 74 projects, 0 errors
  • pnpm run lint:eslint@lunora/client, @lunora/do, @lunora/shard-engine, @lunora/advisor, @lunora/codegen
  • pnpm run lint:package-json — 74 sorted
  • @lunora/shard-engine 1128, @lunora/client 657, @lunora/do 539, @lunora/codegen 1202, @lunora/advisor 482
  • bash sdks/run-all.sh — all 7 non-JS SDKs pass, unmodified
  • pnpm run api:check — 47/47
  • Durable Object code changed → the workerd-pool @lunora/do suite is the one above
  • Schema/codegen changed → every examples/* and apps/playground regenerated; no drift after the rebase
  • Protocol spec + golden fixtures updated
  • The ordering fix verified to fail the round-trip test when reverted, and pass with it
  • Re-verified end to end after rebasing onto the current alpha

Reviewers should re-run bash sdks/run-all.sh and the @lunora/do suite.

Checklist

  • Commit messages follow the Conventional Commits style
  • Added or updated tests covering the change
  • Updated relevant docs — protocol/README.md §5.1.1 (normative, for connect.caps and insert placement) and packages/advisor/docs/index.mdx
  • No package.json files in packages/* modified outside the touched package (@lunora/client gains one devDependency, see below)
  • If a new package was added: n/a
  • If migration impact: none — the wire change is additive and opt-in in both directions

Notes for reviewers

Read in commit order — each builds on the last, and the final one fixes a
bug the third exposed.

Start at shared/page-result.ts. The row-list shape, the capability token,
and the insert-placement rule live there because both sides must agree at
runtime. Mirroring a TYPE across the client boundary is safe (drift fails
tsc — that is why MutationDelta is mirrored); mirroring a runtime shape
agreement behind a negotiated capability is not, and the two copies had already
diverged on their record check one commit in.

Two fixture traps, one of which bit me:

  1. clientFrames.connect is asserted for exact equality by all seven SDK
    suites, so caps went into a new connect-with-caps golden.
  2. serverFrames is iterated wholesale by those suites and applied by
    replacement — so the merge goldens went into their own pageDeltaFrames
    key. I put them in serverFrames first and broke all seven;
    sdks/run-all.sh caught it.

@lunora/client gains @lunora/shard-engine as a devDependency for the
round-trip test, which asserts the server's chosen frames and the client's real
applyDelta land on the same value. No cycle, and there is precedent — client
already devDepends on @lunora/do.

Breaking (pre-1.0, alpha): sendDeltaFrames is removed from
@lunora/shard-engine in favour of subscriptionFrames, which returns the
frames rather than sending them. subscriptionListDeltas no longer returns
undefined for a near-total change — it decomposes any expressible diff, and
cost is decided by the sender. SocketAttachment.caps is stored as the resolved
pageDeltas?: boolean instead of unbounded client tokens. All internal to the
shard/engine boundary; no app code imports them.

Frame-type flips are one-directional — the new predicate is a strict subset
of the old, so no socket that previously got a snapshot now gets deltas.

Measured, not changed

subscriptionFrames' cost scales with list length rather than change size
(~0.74 ms per 200-row list per (socket, subscription); at 100 subscribers that
is ~74 ms of single-threaded shard time per write). Inherent to the current
design — identity-dependent queries cannot share a result under RLS — and pinned
by the new bench so a regression shows up.

On CodSpeed

The predecessor branch showed a CodSpeed red for broadcastWhisper to 1024 members and an in-batch re-projection. Neither is touched by any commit here —
no whisper/batch/relation lines in the diff — and CodSpeed's own summary reports
"Different runtime environments detected… may affect the accuracy of the
results"
. Cross-runner noise rather than a regression, but worth a second look
if it reappears here.

Contributor License Agreement

By submitting this pull request, I confirm that my contribution is made under the terms of the project's license and that you can use, modify, copy, and redistribute this contribution under those terms.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AyG8dyPFieGYzRd7y1MMme

Summary by CodeRabbit

  • New Features
    • Added a performance check for unfiltered, unindexed .collect() queries that may resend entire tables to subscribers.
    • Subscription updates now support paginated results and choose the smaller delta or snapshot representation.
    • Improved detection of query filters, indexes, and materialization methods.
  • Bug Fixes
    • Improved merging of live updates for paginated data while preserving pagination details.
    • Added safer fallbacks when ordering or metadata prevents reliable incremental updates.
  • Documentation
    • Documented the new performance check and its impact on live subscriptions.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7613b1f5-fbf2-467f-8878-922b527e6a20

📥 Commits

Reviewing files that changed from the base of the PR and between de77eb6 and 62c0f56.

⛔ Files ignored due to path filters (3)
  • packages/client/__tests__/delta-merge.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/shard-engine/__tests__/subscription-frames.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • shared/page-result.ts is excluded by none and included by none
📒 Files selected for processing (1)
  • packages/shard-engine/src/subscription-delivery.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/shard-engine/src/subscription-delivery.ts

Walkthrough

The PR adds the unbounded_collect advisor lint and query terminal metadata. It also adds page-delta capability negotiation, paginated delta merging, size-based subscription frame selection, and ShardDO delivery integration.

Changes

Advisor query analysis

Layer / File(s) Summary
Query metadata contract
packages/codegen/src/discover-queries.ts, packages/codegen/src/ir.ts, packages/advisor/src/queries.ts
Query discovery retains unfiltered reads and records filter status and recognized terminal methods. The IR and advisor query types expose optional terminal metadata.
Unbounded collect lint
packages/advisor/src/lints/helpers.ts, packages/advisor/src/lints/static/*.ts, packages/advisor/src/index.ts, packages/advisor/docs/index.mdx
The advisor adds shared location and shard helpers, registers unbounded_collect, updates related lints, and documents the new finding. The lint reports unfiltered, unindexed .collect() reads with storage-tier-specific details.

Paginated subscription delivery

Layer / File(s) Summary
Page-delta capability contract
packages/client/src/types.ts, packages/client/src/lunora-client.ts, packages/client/package.json, packages/shard-engine/src/types.ts
Client connect messages and subscription envelopes support capability tokens. The client advertises pageDelta, and ShardDO persists the resolved capability.
Subscription frame rendering
packages/shard-engine/src/subscription-delivery.ts, packages/shard-engine/src/index.ts, packages/shard-engine/__bench__/subscription-frames.bench.ts
Subscription delivery validates paginated results and client ordering, then returns either delta frames or a snapshot based on serialized size. Benchmarks cover change ratios and fallback cases.
ShardDO subscription delivery
packages/do/src/shard-do.ts
ShardDO resolves socket delivery metadata once, renders and sends subscription frames, and advances the baseline only after all frames succeed.
Client paginated delta merge
packages/client/src/delta-merge.ts
The client merges structured deltas into arrays and paginated results while preserving pagination metadata and validating row shapes.

Estimated code review effort: 5 (Critical) | ~90 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ShardDO
  participant SubscriptionFrames
  participant ClientDeltaMerge

  Client->>ShardDO: connect with pageDelta capability
  ShardDO->>SubscriptionFrames: render subscription frames
  SubscriptionFrames-->>ShardDO: snapshot or row-delta frames
  ShardDO-->>Client: send selected frames
  Client->>ClientDeltaMerge: apply row deltas to result
  ClientDeltaMerge-->>Client: updated array or paginated result
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: reducing unnecessary live-query WebSocket payloads.
Description check ✅ Passed The description follows the required template and provides detailed scope, testing, checklist status, reviewer notes, and the required CLA statement.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/ws-paginated-page-deltas

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.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for following the naming conventions! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for confirming the Contributor License Agreement! 🙏

@prisis prisis changed the title perf(do): diff paginated live queries instead of re-sending the page fix(do): diff paginated live queries instead of re-sending the pageq Aug 11, 2026
@prisis prisis changed the title fix(do): diff paginated live queries instead of re-sending the pageq perf(do): diff paginated live queries instead of re-sending the page Aug 11, 2026
prisis and others added 5 commits August 11, 2026 15:39
A live subscription's refresh went out as one `{type:"delta"}` frame per
changed row whenever the diff was expressible, capped only by "no more
deltas than rows". That cap is a row-count proxy for wire cost, and it is
blind in both directions.

Every delta frame re-pays the whole `{"type":"delta","id":…}` + `"table":…`
+ cursor/epoch/watermark envelope around ONE row body, which the snapshot
pays once for the entire list. So the deltas stop being the cheaper encoding
well before they stop being a valid one. Measured on a 100-row list of
~290-byte rows where every row changed: 43,970 bytes across 100 frames
against 29,060 bytes in a single frame — 1.5x the payload and 100 sends
instead of 1, on the shard's write-flush fan-out path. The cap also misses
the opposite case, where a single delta into a short list of small rows
already costs more than re-sending the list.

Render both candidates and send the smaller one. `subscriptionFrames` now
owns the frame layout and the choice together, so the sizing cannot drift
from the format — it IS the format. `subscriptionListDeltas` keeps only the
four expressibility conditions; cost is no longer its call.

BREAKING CHANGE: `sendDeltaFrames` is removed from `@lunora/shard-engine` in
favour of `subscriptionFrames`, which returns the frames to send rather than
sending them. `subscriptionListDeltas` no longer returns `undefined` for a
near-total change — it decomposes any expressible diff.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyG8dyPFieGYzRd7y1MMme
`filter_without_index` gates on `hasFilter`, so the widest read of all —
`ctx.db.query("t").collect()`, no index and no filter — fell straight
through it. The codegen feeder made that structural: it discarded every
discovered read that did not call `.filter()`, so no lint could have seen
one.

The cost is not only the scan. A query's result is what a live subscription
pushes over the WebSocket, and the shard's refresh gate can only skip a
subscription whose reads were confined to index slices. An unindexed
`.collect()` records a whole-table dependency instead, so every write to
the table re-runs the query and re-sends the entire table to every
subscribed socket, individually.

The feeder now reports every read plus the materializing call the chain
ends in, matched against a known terminal set rather than taken as the last
chained method — `.collect().then(...)` keeps the chain walk going, so the
literal last call is a promise combinator, not the terminal.

Also adds `withGeoIndex` to the narrowing methods. It was missing while only
filtered reads were reported, where nothing could observe it; once unfiltered
reads are reported it would flag the idiomatic `withGeoIndex(...).collect()`
as an unbounded scan — the exact chain `geo_index_unused` tells authors to
write.

Two examples trip the new lint on live-subscribed list queries; their
regenerated advisories are included.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyG8dyPFieGYzRd7y1MMme
Two review follow-ups on the frame sizing.

`subscriptionFrames` materialized every delta frame before comparing the
total against the snapshot, so a near-total change — the case the comparison
exists to catch — allocated an envelope per row only to discard the lot.
The running total only grows, so the answer is known the moment it reaches
the snapshot's length. Same comparison, same chosen frames, decided as early
as it can be.

The bench's reordered-survivors case also encoded its snapshot payload
inside the timed body while every other case precomputed it, folding a full
200-row encode plus stringify into a number that is meant to measure the
early-out. The production caller builds that payload once before calling in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyG8dyPFieGYzRd7y1MMme
A `.paginate()` read returns `{ page, isDone, continueCursor }` — an object,
not an array — so the row diff rejected it on both ends: the server's
`collectFramedDeltas` required `Array.isArray`, and so did the client's
`applyDelta`. Every write touching the table therefore re-sent the whole
page, and `usePaginatedQuery` holds one subscription per loaded page, so a
scrolled feed paid that per page. Measured on a one-row edit: 6,597 bytes
for a 25-row page, 13,072 for 50.

The diff now reaches into `page` on both sides, and only when every field
around the page is byte-identical — a moved `continueCursor` or a flipped
`isDone` cannot be described by row deltas, so it still forces a snapshot.
The same one-row edit now costs 412 bytes: 93.8% less at 25 rows, 96.8% at
50.

This needs negotiation rather than a version bump, because a client that
cannot merge into `page` does not IGNORE such a delta — `applyDelta` bails
and the caller replaces the entire query value with the raw delta object.
So the `connect` frame gains an optional `caps` array, and the server only
diffs a paginated result for a socket that announced `pageDelta`.

Fail-closed by construction: no `caps` means snapshots, which is what every
client did before. The seven non-JS SDKs send `connect` with no `caps` at
all, so they are unaffected and need no changes — their conformance suites
pass untouched against the existing `connect` fixture, and the new
capability-announcing form is a separate `connect-with-caps` golden.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyG8dyPFieGYzRd7y1MMme
An `insert` delta carries no position, so the client picks one with a
`_creationTime` heuristic. A read ordered by a `.withIndex()` field does not
order by `_creationTime` — `paginateOrderKeys` orders by the staged index
fields — so the two can legitimately disagree about where a new row goes.
`survivorsKeepOrder` does not catch it: the survivors did keep their order,
only the newcomer is misplaced.

That desyncs permanently, not transiently. The server advances its diff
baseline to the value it believes the client now holds, so every later update
lands in place at an index the client has wrong, and nothing reconciles until
a reconnect. Reproduced end to end: server `a,n,b,c`, client `n,a,b,c`.

The server now replays the client's placement before sending, and falls back
to a snapshot when the result would not match its own order. The replay uses
the same function the client merges with, so the two cannot drift.

The bug predates paginated deltas — a bare `.withIndex().collect()` diverged
the same way — so this fixes that path too, and makes the exactness the
protocol spec claims actually true.

Also from review:

- The row-list shape and the insert rule move to `shared/page-result.ts`.
  Mirroring a TYPE across the client boundary is safe (drift fails `tsc`);
  mirroring a runtime shape agreement behind a negotiated capability is not —
  drift there is silent corruption, which is what the capability exists to
  prevent. The two copies had already diverged on their record check.
- The attachment stores the resolved `pageDeltas` decision instead of the raw
  `caps` array. `caps` is client-supplied and was unbounded, while both
  sibling attachment arrays are explicitly capped; nothing ever read an
  unrecognised token back.
- `socketClientWatermark` takes the attachment rather than re-reading it, so
  `socketDelivery` no longer needs an optional-hint parameter that did not
  work: the refresh path deserialized twice and the seed path three times.
- The delta-vs-snapshot goldens live under their own `pageDeltaFrames` key.
  Adding them to `serverFrames` broke all seven SDK suites, which iterate it
  wholesale and apply by replacement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyG8dyPFieGYzRd7y1MMme
@prisis
prisis force-pushed the perf/ws-paginated-page-deltas branch from 09fc792 to de77eb6 Compare August 11, 2026 13:45
@prisis
prisis changed the base branch from perf/ws-subscription-frame-sizing to alpha August 11, 2026 13:45
@prisis prisis changed the title perf(do): diff paginated live queries instead of re-sending the page perf(do): stop over-sending on the live-query WebSocket path Aug 11, 2026
@prisis

prisis commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
packages/shard-engine/__bench__/subscription-frames.bench.ts (1)

29-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The paginated benchmark does not exercise the new page-delta path, and its description is stale.

subscriptionFrames now gates diffing on previousJson !== undefined && (pageDeltas === true || Array.isArray(nextResult)). The envelope constant at Line 49 omits pageDeltas, so the paginated case still short-circuits before collectFramedDeltas runs.

Two consequences:

  • The docblock claim "fails the Array.isArray precondition immediately" describes the behavior before this PR. The gate is now the capability flag.
  • The paginated diff this PR adds — envelope stableStringify, row indexing, clientOrderMatches insert replay — has no benchmark. That is the hot path the new code introduces.

Keep the capability-absent case, and add a capability-present case so a regression in the paginated diff is visible.

♻️ Proposed change
- * - **not a list** — the paginated `{ page, isDone, continueCursor }` shape
- * `usePaginatedQuery` subscribes to. It fails the `Array.isArray` precondition
- * immediately, so every write re-sends the whole page as a snapshot; the bench
- * records what that rejection costs today.
+ * - **paginated, capability absent** — the `{ page, isDone, continueCursor }`
+ * shape `usePaginatedQuery` subscribes to, on a socket that did not announce
+ * `pageDelta`. The gate rejects it before any diff runs, so every write
+ * re-sends the whole page as a snapshot; the bench records that rejection cost.
+ * - **paginated, capability present** — the same shape on a `pageDelta` socket.
+ * This pays the envelope compare, the row indexing, and the insert-placement
+ * replay, and is the path this diff adds.
-    bench(`paginated { page, isDone, continueCursor } → rejected (not an array)`, () => {
+    bench(`paginated { page, isDone, continueCursor } → rejected (no pageDelta capability)`, () => {
         subscriptionFrames({ ...envelope, nextResult: paginatedNext, previousJson: paginatedPreviousJson, snapshotJson: paginatedSnapshotJson });
     });
+
+    // The capability-gated path: the envelope compare + row diff + insert-order
+    // replay this change introduces. Uses the 1-of-N case so it lands on the
+    // delta branch rather than measuring the snapshot bail-out again.
+    const changedPage = { continueCursor: "c_200", isDone: false, page: buildCase(LIST_LENGTH, 1).nextResult };
+    const changedPageSnapshotJson = JSON.stringify(encodeWire(changedPage));
+
+    bench(`paginated { page, isDone, continueCursor } → deltas (pageDelta capability)`, () => {
+        subscriptionFrames({
+            ...envelope,
+            nextResult: changedPage,
+            pageDeltas: true,
+            previousJson: paginatedPreviousJson,
+            snapshotJson: changedPageSnapshotJson,
+        });
+    });

Also applies to: 89-95

🤖 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/shard-engine/__bench__/subscription-frames.bench.ts` around lines 29
- 32, Update the subscriptionFrames benchmark documentation to describe the
capability flag rather than the Array.isArray precondition, and preserve the
capability-absent case. Extend the envelope used by the paginated benchmark with
pageDeltas enabled, then add a separate capability-present benchmark covering
the paginated diff path through collectFramedDeltas, including envelope
serialization, row indexing, and client-order insert replay.
packages/shard-engine/src/index.ts (1)

334-334: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Export SubscriptionFrameInput from the barrel. subscription-delivery.ts exports the type, but packages/shard-engine/src/index.ts does not. No sendDeltaFrames references remain.

🤖 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/shard-engine/src/index.ts` at line 334, Update the barrel export in
src/index.ts to re-export the SubscriptionFrameInput type from
subscription-delivery.ts alongside the existing awaitWsDrain and related
exports. Do not add sendDeltaFrames.
🤖 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/client/src/delta-merge.ts`:
- Around line 139-156: Update the plain-record validation used by rowListOf and
the server’s isPlainRecord so it accepts only objects whose prototype is
Object.prototype or null, while still requiring an array-valued page. Ensure
applyDelta does not re-wrap class instances or other non-plain objects by
spreading them into plain records.

In `@packages/shard-engine/src/subscription-delivery.ts`:
- Around line 455-459: Update the diff handling around collectFramedDeltas in
subscriptionFrames so an empty framed result is treated like no usable row diff:
return the snapshot when framed is undefined or has zero rows. Preserve the
existing snapshot fallback and ensure zero-delta flushes still emit cursor,
epoch, and lastMutationId metadata.

---

Nitpick comments:
In `@packages/shard-engine/__bench__/subscription-frames.bench.ts`:
- Around line 29-32: Update the subscriptionFrames benchmark documentation to
describe the capability flag rather than the Array.isArray precondition, and
preserve the capability-absent case. Extend the envelope used by the paginated
benchmark with pageDeltas enabled, then add a separate capability-present
benchmark covering the paginated diff path through collectFramedDeltas,
including envelope serialization, row indexing, and client-order insert replay.

In `@packages/shard-engine/src/index.ts`:
- Line 334: Update the barrel export in src/index.ts to re-export the
SubscriptionFrameInput type from subscription-delivery.ts alongside the existing
awaitWsDrain and related exports. Do not add sendDeltaFrames.
🪄 Autofix

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: 0ffe64fe-033a-49da-9877-b3cc555b0bb1

📥 Commits

Reviewing files that changed from the base of the PR and between 1c63c56 and de77eb6.

⛔ Files ignored due to path filters (19)
  • api-snapshots/advisor.api.md is excluded by none and included by none
  • api-snapshots/client.api.md is excluded by none and included by none
  • api-snapshots/codegen.api.md is excluded by none and included by none
  • api-snapshots/shard-engine.api.md is excluded by none and included by none
  • examples/chess/lunora/_generated/shard.ts is excluded by !**/_generated/** and included by none
  • examples/feedback-board/lunora/_generated/shard.ts is excluded by !**/_generated/** and included by none
  • packages/advisor/__tests__/unbounded-collect.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/client/__tests__/delta-merge.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/client/__tests__/delta-round-trip.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/client/__tests__/lunora-client.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/client/__tests__/protocol-conformance.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/codegen/__tests__/discover-queries.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/do/__tests__/shard-do.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/do/__tests__/subscription-data-watermark.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/shard-engine/__tests__/subscription-frames.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml and included by none
  • protocol/README.md is excluded by none and included by none
  • protocol/fixtures/ws-frames.json is excluded by none and included by none
  • shared/page-result.ts is excluded by none and included by none
📒 Files selected for processing (18)
  • packages/advisor/docs/index.mdx
  • packages/advisor/src/index.ts
  • packages/advisor/src/lints/helpers.ts
  • packages/advisor/src/lints/static/filter-on-primary-key.ts
  • packages/advisor/src/lints/static/filter-without-index.ts
  • packages/advisor/src/lints/static/unbounded-collect.ts
  • packages/advisor/src/queries.ts
  • packages/client/package.json
  • packages/client/src/delta-merge.ts
  • packages/client/src/lunora-client.ts
  • packages/client/src/types.ts
  • packages/codegen/src/discover-queries.ts
  • packages/codegen/src/ir.ts
  • packages/do/src/shard-do.ts
  • packages/shard-engine/__bench__/subscription-frames.bench.ts
  • packages/shard-engine/src/index.ts
  • packages/shard-engine/src/subscription-delivery.ts
  • packages/shard-engine/src/types.ts

Comment thread packages/client/src/delta-merge.ts
Comment thread packages/shard-engine/src/subscription-delivery.ts
Two review findings, both on the paginated path this branch added.

A diff can now come back EMPTY while the snapshot still differs. The caller
suppresses a push only when the new snapshot is byte-identical to the last,
but `diffableLists` compares a paginated envelope with `stableStringify` —
key-order insensitive. A result whose non-`page` fields were re-serialized in
a different order therefore reaches the diff with no rows to report and a
changed `snapshotJson`, and `subscriptionFrames` returned no frames at all.
`[].every(Boolean)` is `true`, so the caller advanced its diff baseline while
the client never received that flush's cursor, epoch or watermark — stranding
its resume position and leaving a pending optimistic layer masked until some
later visible change. Reproduced directly; an empty diff now sends the
snapshot, which carries the suffix.

(This was unreachable before: every other comparison on the path is
byte-exact, so an empty diff implied an identical snapshot. The stable
envelope comparison is what opened it.)

Second, the paginated-shape test accepted any non-array object with an array
`page`, including class instances — which the merge then re-spreads into a
plain object, silently changing the value's shape. Both sides now use the wire
codec's own `isPlainObject`. That is the right predicate rather than merely a
stricter one: it is exactly the set of objects that survives the wire, so
anything it rejects could not have reached a client as this shape anyway, and
it replaces the last hand-rolled copy of the test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AyG8dyPFieGYzRd7y1MMme
@netlify

netlify Bot commented Aug 11, 2026

Copy link
Copy Markdown

Deploy Preview for lunorash ready!

Name Link
🔨 Latest commit 62c0f56
🔍 Latest deploy log https://app.netlify.com/projects/lunorash/deploys/6a7b39927adf160008a0f4a2
😎 Deploy Preview https://deploy-preview-407--lunorash.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

*/

import { encodeWire } from "../../../shared/wire-codec";
import { ID_FIELD, insertionIndexFor, PAGE_DELTA_CAPABILITY, PAGE_FIELD } from "../../../shared/page-result";
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.34513% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.01%. Comparing base (95d33d6) to head (62c0f56).
⚠️ Report is 295 commits behind head on alpha.

Files with missing lines Patch % Lines
packages/shard-engine/src/subscription-delivery.ts 96.00% 2 Missing ⚠️
packages/do/src/shard-do.ts 92.30% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            alpha     #407      +/-   ##
==========================================
- Coverage   87.09%   87.01%   -0.08%     
==========================================
  Files        1172     1201      +29     
  Lines       63383    65300    +1917     
  Branches    15447    16023     +576     
==========================================
+ Hits        55202    56822    +1620     
- Misses       7654     7930     +276     
- Partials      527      548      +21     
Files with missing lines Coverage Δ
packages/advisor/src/index.ts 100.00% <ø> (ø)
packages/advisor/src/lints/helpers.ts 100.00% <100.00%> (ø)
.../advisor/src/lints/static/filter-on-primary-key.ts 100.00% <100.00%> (ø)
...s/advisor/src/lints/static/filter-without-index.ts 100.00% <100.00%> (ø)
...ages/advisor/src/lints/static/unbounded-collect.ts 100.00% <100.00%> (ø)
packages/client/src/delta-merge.ts 100.00% <100.00%> (+5.17%) ⬆️
packages/client/src/lunora-client.ts 80.90% <100.00%> (-0.06%) ⬇️
packages/codegen/src/discover-queries.ts 96.42% <100.00%> (+0.13%) ⬆️
packages/do/src/shard-do.ts 85.02% <92.30%> (+0.05%) ⬆️
packages/shard-engine/src/subscription-delivery.ts 87.50% <96.00%> (+44.64%) ⬆️

... and 59 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq

codspeed-hq Bot commented Aug 11, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 12.56%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 252 untouched benchmarks
🆕 5 new benchmarks
⏩ 10 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
flat 3 primitives (the notify.send attribute shape) 62.6 µs 55.6 µs +12.56%
🆕 1 of 200 rows changed N/A 4.1 ms N/A
🆕 100 of 200 rows changed N/A 4.2 ms N/A
🆕 200 of 200 rows changed N/A 4.4 ms N/A
🆕 200 rows, survivors reordered → rejected N/A 1.2 ms N/A
🆕 paginated { page, isDone, continueCursor } → rejected (not an array) N/A 63.4 µs N/A

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing perf/ws-paginated-page-deltas (62c0f56) with alpha (ccc68a3)2

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on alpha (1c63c56) during the generation of this report, so ccc68a3 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@prisis
prisis merged commit 4f8c291 into alpha Aug 11, 2026
62 checks passed
@prisis
prisis deleted the perf/ws-paginated-page-deltas branch August 11, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants