Skip to content

fix(orm): bound aggregate read caches to safe write scopes - #451

Merged
zbeyens merged 8 commits into
mainfrom
fix/orm-memoize-aggregate-bucket-reads
Sep 7, 2026
Merged

zbeyens merged 8 commits into
mainfrom
fix/orm-memoize-aggregate-bucket-reads

Conversation

@MikeyZhang75

@MikeyZhang75 MikeyZhang75 commented Sep 6, 2026 •

Copy link
Copy Markdown
Collaborator
  • Auto release

🐛 Addresses #440 — aggregate read amplification.

🧭 Task plan: docs/plans/440-memoize-aggregate-bucket-and-member-reads.md

🟢 High confidence — local proof passes; hosted checks verified before merge.

Phase 🧪 Tests 🌐 Browser
Reproduced 🔴 Repeated bucket reads; nested writes produce an incorrect indexed count ➖ N/A
Verified 🟢 Bulk bucket probes 1 / 2 / 1; nested mutation, hook and RLS regressions pass 🟢 Aggregate Write costs reference renders

✅ Outcome

Bulk ORM writes reuse aggregate bucket and member rows within uninterrupted
statements. A 12-row insert or delete probes one bucket; an update moving rows
between two key tuples probes two. Indexed counts remain correct across nested
mutations, including calls made inside lifecycle hooks and RLS policies.

🏗️ Design

Statement builders own cache lifetime. User lifecycle and policy callbacks
expire cached rows and suspend reuse until the callback settles, including
reentrant statements. Aggregate maintenance updates cached post-images after
writes and invalidates cleared rows. Query reads continue to read storage.

⚠️ Caveat

  • Separate statements reload rows. Callback-heavy writes can perform more reads.
  • Member reads remain proportional to distinct documents within each safe segment.
  • Aggregate writes remain eager; deferred-write batching belongs to fix(orm): batch aggregate writes within safe statement scopes #454.
  • Extrema reads remain outside this read-cache change.
  • Release is disabled for this merge.

🧪 Verified

bun check passes: 1403 Bun tests, 1014 Vitest tests, 124 CLI tests, eight
fixture comparisons and generated-app runtime checks. Focused proof covers
66 integration/memo cases plus three cache-lifetime exception tests. Root
typecheck, lint, package build, intent validation/staleness and rendered docs
inspection pass. Final P1 review and exact-head GitHub receipts are recorded
before merge.

Bulk statements reconciled aggregate buckets per document: a 40-row update
issued ~80 index probes against one aggregate_bucket row. Every document is
reconciled in its own change hook, so folding the delta array removes nothing;
the fix is transaction lifetime for the two point lookups.

- memoize getBucketByKey/getMemberByDoc on the createOrmTransactionMemo anchor,
  write path only. readPlanBuckets stays unmemoized: its 25-wide fan-out can
  store a pre-write row over a write-through entry, for no measured gain
- write through on every delta writer; patch merges onto the whole prior row so
  keyHash survives for the extrema reads
- thread docId + the member post-image through AggregateMembershipDelta; the
  patch/delete member writes carried no docId, so write-through was otherwise
  unimplementable and would have corrupted counts
- clearCountIndexChunk retires entries exactly; it deletes without going
  through the delta machinery and its callers only bump state afterwards

12-row bulk update across two orgs: 2 bucket probes, was 24. Values unchanged.
@changeset-bot

changeset-bot Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b62b80c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
kitcn Patch
@kitcn/resend Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
better-convex Ready Ready Preview Sep 7, 2026 1:40pm UTC

Request Review

@zbeyens zbeyens changed the title fix(orm): read each aggregate bucket once per key tuple per transaction fix(orm): bound aggregate read caches to safe write scopes Sep 7, 2026
@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.

@zbeyens

zbeyens commented Sep 7, 2026

Copy link
Copy Markdown
Member

Autoclosure proof receipt — head 1f777a5e0eb386f703afb11a62987751630e1d45.

Local HEAD, fetched refs/pr/451 and live PR head match. Post-push P1 replay
passes: 9 aggregate reconciliation cases, including nested mutation, lifecycle
hook and RLS callback corruption regressions; 11 write-scope/transaction-memo
unit tests. Full bun check passes (1403 Bun, 1014 Vitest, 124 CLI tests,
eight fixture comparisons and generated-app runtime lanes). Frozen branch
P0/P1 autoreview reports no accepted/actionable findings.

Feedback read-back: helper 0 threads / 2 comments / 0 reviews; unfiltered 0
resolved threads / 0 unresolved threads / 3 comments / 0 reviews. All pages
exhausted. Zero actionable P0/P1, zero deferred P2/P3.

Exact item verdicts:

Auto release is unchecked. CI and Preview are still running; this receipt does
not authorize merge until hosted gates pass. Version Packages remains excluded.
The versioned closeout evidence update will require a fresh head-bound replay
and superseding receipt before merge.

@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.

@zbeyens

zbeyens commented Sep 7, 2026

Copy link
Copy Markdown
Member

Autoclosure terminal receipt — head b62b80ca10a54eeaaf87676645b7edecc0fe7d71.

Supersedes the predecessor-head receipt. Local HEAD, fetched refs/pr/451
and live PR head match. After the final material push, P1 replay passes:
9 aggregate reconciliation cases, including all three nested-UDF corruption
regressions, plus 11 write-scope/transaction-memo unit tests.

Full bun check passes: 1403 Bun, 1014 Vitest, 124 CLI tests, eight fixture
comparisons and generated-app runtime lanes. Frozen branch P0/P1 autoreview
reports no accepted/actionable findings. Both goal-plan checks pass.
Exact-head CI passed in 6m16s; Preview and auto-release checkbox checks pass.

Fresh feedback inventory: helper 0 threads / 4 comments / 0 review bodies;
unfiltered 0 resolved threads / 0 unresolved threads / 5 comments / 0 reviews.
All pages exhausted. Zero actionable P0/P1; zero deferred P2/P3.

Exact item verdicts:

Auto release is unchecked. Merge uses [skip release]; Version Packages is
excluded. No further branch mutation is planned. Merge is gated on read-back
of this receipt, repeated receipt/live/fetched/local OID equality, and another
complete helper/raw/all-thread inventory with no unrecorded feedback.

@zbeyens
zbeyens merged commit 80f7609 into main Sep 7, 2026
6 checks passed

This branch was successfully deployed

1 active deployment
Preview — b62b80ca Deployed Sep 7, 2026 by vercel[bot]
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.

2 participants