Make public listings faster and keep requests within Bunny limits - #1820
Conversation
📝 WalkthroughWalkthroughChangesThe PR centralizes public group visibility and bookability, batches group-listing reads, adds a per-request database round-trip guard, updates bulk database operations, and replaces the former cold-boot profiler with balanced bundle-load and Public liveness and data access
Database budgets and bulk operations
Cold-start benchmarks
Benchmark documentation
Estimated code review effort: 4 (Complex) | ~75 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 8
🤖 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 `@AGENTS.md`:
- Around line 423-424: Update the benchmark script reference in the AGENTS.md
guidance to use the full scripts/bench/cold-start/first-request.ts path, while
preserving the existing bundle-load.ts reference and surrounding rules.
In `@scripts/bench/cold-start/bundle-load.ts`:
- Around line 79-82: Extract the robots.txt response body and content-type value
used by the writeTextFile call into a shared constant, such as in support.ts,
and reuse that constant in both bundle-load.ts and measure-import.ts's
expectedBody check. Remove the duplicated literal while preserving the existing
response content and validation behavior.
- Line 1: Define and export shared robots.txt body and content-type constants in
support.ts, then update bundle-load.ts generation of hello.js and
measure-import.ts expectedBody/content-type assertions to import and reuse them,
removing the duplicated magic literals.
In `@scripts/bench/cold-start/first-request.ts`:
- Around line 206-217: Update balancedRequestSlope to validate that runs.length
is evenly divisible by LATENCIES_MS.length before constructing cycles, and fail
immediately when a remainder exists. Preserve the existing per-cycle slicing,
slope averaging, and median calculation for valid inputs.
In `@scripts/bench/cold-start/measure-import.ts`:
- Around line 31-48: Share the benchmark response literals between
measure-import.ts and bundle-load.ts instead of defining expectedBody and the
content type independently. Extract and reuse constants for the exact robots.txt
body and content type that bundle-load.ts writes into hello.js, while preserving
the existing response validation in the first-request flow.
In `@src/features/public/group-liveness.ts`:
- Around line 272-278: Extend loadPublicGroups or introduce a variant that
returns both the filtered groups and the membersByGroup data fetched during
loadBookableGroupIds. Update src/features/public/group-liveness.ts#L272-L278 to
expose that data, src/features/public/order.ts#L123-L137 to reuse it instead of
calling getVisibleGroupMembersByGroupIds for packageGroups, and
src/features/public/pages.ts#L177-L198 to pass through and reuse the members
from the earlier loadPublicGroups call rather than refetching them.
- Around line 41-53: Update getVisibleGroupMembers to delegate to the existing
visibleGroupMembersFrom batch implementation, passing the single group and its
active listings, rather than using visibleGroupMembers directly. Move
getVisibleGroupMembers below visibleGroupMembersFrom and membersOf, or hoist
those const declarations so the delegation is valid; remove the duplicated
singular visibility path if no longer needed.
In `@src/shared/db/groups.ts`:
- Around line 238-243: Bound the concurrency of the mapParallel operation that
decrypts rows in the listingsWithGroups flow. Configure or wrap mapParallel so
decryptListingWithCount runs with a fixed, reasonable per-request concurrency
limit, while preserving the existing row transformation and result ordering
behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 316dfbf4-a002-45fa-909d-11afeb6257fd
📒 Files selected for processing (31)
AGENTS.mdTODO.mddocs/cold-start.mdscripts/bench/cold-start/bundle-load.tsscripts/bench/cold-start/first-request-child.tsscripts/bench/cold-start/first-request.tsscripts/bench/cold-start/measure-import.tsscripts/bench/cold-start/serve-request.tsscripts/bench/cold-start/serve-root.tsscripts/bench/cold-start/strip-lib.tsscripts/bench/cold-start/support.tsscripts/profile-cold-boot.tssrc/features/admin/group-page-data.tssrc/features/api/listings.tssrc/features/api/payment-processing/cancel.tssrc/features/feeds.tssrc/features/public/discovery.tssrc/features/public/group-liveness.tssrc/features/public/groups.tssrc/features/public/order-js.tssrc/features/public/order.tssrc/features/public/pages.tssrc/features/public/site-nav.tssrc/features/public/ticket-routes.tssrc/fp.tssrc/shared/db/attendees/capacity.tssrc/shared/db/groups.tstest/fp.test.tstest/lib/server-public/listings-query-scaling.test.tstest/scripts/cold-start-strip.test.tstest/scripts/cold-start-support.test.ts
💤 Files with no reviewable changes (3)
- scripts/bench/cold-start/serve-root.ts
- scripts/profile-cold-boot.ts
- TODO.md
stefan-burke
changed the title
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/lib/server-public/listings-query-scaling.test.ts (1)
164-175: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReuse
batchedMemberQueriesand assert uniqueness instead of.find().Lines 165-169 duplicate the exact SQL-prefix literal already encoded in
batchedMemberQueries(lines 36-41). Beyond the duplication, using.find()here never verifies there's only one such query — the test is named "projects a shared listing once" but doesn't assert the count, so a regression that emits the grouped query twice would slip through undetected, unlike the sibling tests at lines 124, 139, and 146 which do assert.length.♻️ Proposed fix
const seen = await recordListingsPage(names); - const memberQuery = seen.find((sql) => - sql.startsWith( - "SELECT json_group_array(groupListing.group_id) AS group_ids,", - ), - ); + const memberQueries = batchedMemberQueries(seen); + expect(memberQueries.length).toBe(1); + const [memberQuery] = memberQueries; expect(memberQuery).toContain("GROUP BY listing.id");As per coding guidelines, "Eliminate code duplication by extracting shared helpers or curried factories; do not evade jscpd with structural changes or
jscpd:ignore" and tests must "Write strong mutation-resistant assertions... do not rely on truthiness or presence-only assertions."🤖 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 `@test/lib/server-public/listings-query-scaling.test.ts` around lines 164 - 175, Update the test’s grouped-query lookup to reuse the existing batchedMemberQueries prefix/helper instead of duplicating the SQL literal. Assert that exactly one matching query is emitted before validating its GROUP BY and ORDER BY clauses, preserving the existing query-content assertions.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@test/lib/server-public/listings-query-scaling.test.ts`:
- Around line 164-175: Update the test’s grouped-query lookup to reuse the
existing batchedMemberQueries prefix/helper instead of duplicating the SQL
literal. Assert that exactly one matching query is emitted before validating its
GROUP BY and ORDER BY clauses, preserving the existing query-content assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aa2d1c64-d63d-400b-975a-37aeb5d50519
📒 Files selected for processing (15)
AGENTS.mdscripts/bench/cold-start/bundle-load.tsscripts/bench/cold-start/first-request.tsscripts/bench/cold-start/measure-import.tsscripts/bench/cold-start/support.tssrc/features/api/listings.tssrc/features/feeds.tssrc/features/public/group-liveness.tssrc/features/public/order-js.tssrc/features/public/order.tssrc/features/public/pages.tssrc/shared/types.tssrc/ui/templates/public/order-gallery.tsxtest/lib/server-public/listings-query-scaling.test.tstest/scripts/cold-start-support.test.ts
|
Addressed the incremental CodeRabbit review in 4390e8f. The shared-listing test now uses |
stefan-burke
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68e557a1ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const firstPages = await queryBatch( | ||
| tables.map((table) => tablePageStatement(table, 0, pageSize)), | ||
| ); |
There was a problem hiding this comment.
When several tables have close to BACKUP_PAGE_SIZE rows, this batches the first page for every table into one libsql response. That removes the per-table response-size bound that exportTable relies on to avoid libsqld's payload cap, so a large but otherwise valid backup can fail before the per-table keyset loop gets a chance to continue. Please keep first-page reads in small bounded batches, or let each table page through exportTable independently.
Useful? React with 👍 / 👎.
# Conflicts: # src/shared/db/backup.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/admin/listings-parents.ts (1)
333-380: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBatch the remaining parent and clone reads too.
getChildrenForParents/getParentsForChildrenalready hydrate edges in bulk, butaddChildrenToParentstill does onegetListingWithCount(parentId)and onelistingChildren.getIds(parentId)per outside parent. UsegetListingsWithCountsByIds([...])andlistingChildren.getIdsByKeys([...])before the loop so larger duplications stay clear of the round-trip cap.🤖 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 `@src/features/admin/listings-parents.ts` around lines 333 - 380, Batch the outside-parent reads in remapDuplicatedGroupEdges before iterating byOutsideParent: fetch all parent listings with getListingsWithCountsByIds and all existing child IDs with listingChildren.getIdsByKeys, then pass the hydrated parent and children data into addChildrenToParent or its equivalent. Remove the per-parent getListingWithCount and listingChildren.getIds calls while preserving existing children and error aggregation.
🤖 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 `@src/shared/proxy-members.ts`:
- Around line 3-6: The proxyMembers signature currently accepts overrides as an
unconstrained object, so overridden members are not checked against T. Update
the overrides parameter type to tie matching properties to T while retaining
support for extra marker-symbol properties such as GUARDED_CLIENT, and preserve
the existing proxyMembers behavior.
- Around line 13-19: Update the get trap in proxyMembers to pass inner as the
receiver when reading passthrough properties with Reflect.get, while preserving
override handling and method binding. Ensure accessor reads such as closed
execute with the wrapped target’s private-field brand.
In `@test/shared/proxy-members.test.ts`:
- Around line 1-56: Add direct getOwnPropertyDescriptor coverage to the
proxyMembers tests: add one test that retrieves a descriptor for a replacement
member and verifies the replacement branch, plus one test for an unchanged
member that verifies the target branch. Use Object.getOwnPropertyDescriptor on
the proxied object and assert the returned descriptors reflect the respective
values.
---
Outside diff comments:
In `@src/features/admin/listings-parents.ts`:
- Around line 333-380: Batch the outside-parent reads in
remapDuplicatedGroupEdges before iterating byOutsideParent: fetch all parent
listings with getListingsWithCountsByIds and all existing child IDs with
listingChildren.getIdsByKeys, then pass the hydrated parent and children data
into addChildrenToParent or its equivalent. Remove the per-parent
getListingWithCount and listingChildren.getIds calls while preserving existing
children and error aggregation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 13723018-5fc8-4f66-acb6-3997b01c20ab
📒 Files selected for processing (23)
AGENTS.mdTODO.mddocs/cold-start.mdscripts/bench/cold-start/first-request-child.tsscripts/bench/cold-start/first-request.tssrc/features/admin/attendee-refunds.tssrc/features/admin/bulk-actions.tssrc/features/admin/listings-parents.tssrc/shared/db/backup.tssrc/shared/db/client.tssrc/shared/db/libsql-call.tssrc/shared/db/listings/records.tssrc/shared/db/migrations.tssrc/shared/db/migrations/schema-sync.tssrc/shared/db/query-log.tssrc/shared/proxy-members.tssrc/shared/subrequest-budget.tstest/lib/server-refunds-bulk.test.tstest/shared/db/client/round-trip-limit.test.tstest/shared/db/listings/queries.test.tstest/shared/db/query-log.test.tstest/shared/proxy-members.test.tstest/test-utils/record-queries.ts
| export const proxyMembers = <T extends object>( | ||
| target: T, | ||
| overrides: object, | ||
| ): T => |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
overrides: object erases override-signature checking against the wrapped target.
Because overrides is typed as a bare object rather than something tied to T, TypeScript can't catch a mismatched override signature. For example, test/test-utils/record-queries.ts's batch override narrows mode?: "write" | "read" while Client.batch's real signature accepts TransactionMode (which also includes "deferred") — this compiles silently today, and would only surface as a behavioural surprise if a caller ever passes "deferred".
A stricter type such as Partial<T> & Record<PropertyKey, unknown> would preserve the ability to inject marker symbols (like GUARDED_CLIENT in client.ts) while still checking that any real member of T you override matches its actual signature.
🤖 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 `@src/shared/proxy-members.ts` around lines 3 - 6, The proxyMembers signature
currently accepts overrides as an unconstrained object, so overridden members
are not checked against T. Update the overrides parameter type to tie matching
properties to T while retaining support for extra marker-symbol properties such
as GUARDED_CLIENT, and preserve the existing proxyMembers behavior.
| get(inner, property, receiver) { | ||
| if (Reflect.has(overrides, property)) { | ||
| return Reflect.get(overrides, property); | ||
| } | ||
| const value = Reflect.get(inner, property, receiver); | ||
| return typeof value === "function" ? value.bind(inner) : value; | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the libsql-client-ts hrana/http Transaction implementation use private class fields (#) for its closed getter?
💡 Result:
The HttpTransaction implementation in libsql-client-ts (specifically within the hrana/http implementation) does not use a private class field (the # syntax) for its own closed getter [1]. Instead, the HttpTransaction class overrides the closed getter to delegate directly to the underlying Hrana stream object's closed property [1]: override get closed: boolean { return this.#stream.closed; } In this implementation, the HttpTransaction class uses a private class field (#stream) to store the reference to the hrana.HttpStream object [1], but the closed getter itself is a public getter that returns the value of the closed property belonging to that stream [1].
Citations:
🏁 Script executed:
git ls-files src/shared/proxy-members.ts src/shared/db/client.ts
wc -l src/shared/proxy-members.ts src/shared/db/client.ts
cat -n src/shared/proxy-members.tsRepository: chobbledotcom/tickets
Length of output: 1716
🏁 Script executed:
rg -n "proxyMembers|Proxy\\(" src/shared -g '!**/*.map'Repository: chobbledotcom/tickets
Length of output: 500
🌐 Web query:
libsql-client-ts hrana Client getter private field closed accessor proxy brand check
💡 Result:
In the @libsql/hrana-client TypeScript library, the client's state is managed using a private field, typically denoted as #closed, which stores the error that caused the client to close or remains undefined if the client is active [1]. A public getter named closed provides access to this state, returning a boolean indicating whether the client is closed [2][3]. Internally, this getter checks the status of the private #closed field. Regarding your query about a "proxy brand check," the @libsql/hrana-client library does not explicitly implement a proxy-based brand check pattern for the Client class itself. However, the library is designed for internal use, with @libsql/client being the recommended, high-level interface for most developers [4][5][6]. The library uses standard TypeScript access modifiers and private fields (#) to encapsulate its internal state, preventing direct external manipulation of the underlying Hrana protocol connection. When a client or stream operation is attempted while the client is closed, the library throws a ClosedError [7][1], which is a specific error class designed to handle this state. If you are developing code that interacts with the Hrana protocol, you should rely on the documented.close method to terminate connections and check the.closed getter to verify the client's status [2][4][8].
Citations:
- 1: DaBigBlob/hrana-client@4540fb4
- 2: https://libsql.org/hrana-client-ts/classes/Client.html
- 3: https://github.com/tursodatabase/libsql-client-ts/blob/main/packages/libsql-client/src/http.ts
- 4: https://github.com/libsql/hrana-client-ts
- 5: https://registry.npmjs.org/@libsql/hrana-client
- 6: https://github.com/libsql/hrana-client-ts/
- 7: https://github.com/libsql/hrana-client-ts/blob/95a1d21/src/errors.ts
- 8: https://registry.npmjs.org/hrana-client
Pass the inner target as the receiver for passthrough accessors proxyMembers() forwards the proxy as the receiver to Reflect.get/Reflect.set, but the wrapped hrana Client and HttpTransaction accessors read private fields. Reading .closed on these proxies can throw a brand-check TypeError; use inner as the receiver for passthrough members.
🤖 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 `@src/shared/proxy-members.ts` around lines 13 - 19, Update the get trap in
proxyMembers to pass inner as the receiver when reading passthrough properties
with Reflect.get, while preserving override handling and method binding. Ensure
accessor reads such as closed execute with the wrapped target’s private-field
brand.
| import { expect } from "@std/expect"; | ||
| import { describe, test } from "@std/testing/bdd"; | ||
| import { proxyMembers } from "#shared/proxy-members.ts"; | ||
|
|
||
| describe("proxyMembers", () => { | ||
| test("returns replacement members", () => { | ||
| const proxied = proxyMembers({ value: 1 }, { value: 2 }); | ||
| expect(proxied.value).toBe(2); | ||
| }); | ||
|
|
||
| test("forwards unchanged values", () => { | ||
| const proxied = proxyMembers({ value: 1 }, {}); | ||
| expect(proxied.value).toBe(1); | ||
| }); | ||
|
|
||
| test("binds unchanged methods to their target", () => { | ||
| const target = { | ||
| read() { | ||
| return this.value; | ||
| }, | ||
| value: 3, | ||
| }; | ||
| const read = proxyMembers(target, {}).read; | ||
| expect(read()).toBe(3); | ||
| }); | ||
|
|
||
| test("updates a replacement without changing its target", () => { | ||
| const target = { value: 1 }; | ||
| const proxied = proxyMembers(target, { value: 2 }); | ||
| proxied.value = 3; | ||
| expect(proxied.value).toBe(3); | ||
| expect(target.value).toBe(1); | ||
| }); | ||
|
|
||
| test("updates an unchanged target member", () => { | ||
| const target = { value: 1 }; | ||
| const proxied = proxyMembers(target, {}); | ||
| proxied.value = 2; | ||
| expect(target.value).toBe(2); | ||
| }); | ||
|
|
||
| test("defines a replacement property on the replacements", () => { | ||
| const target = { value: 1 }; | ||
| const proxied = proxyMembers(target, { value: 2 }); | ||
| Object.defineProperty(proxied, "value", { configurable: true, value: 3 }); | ||
| expect(proxied.value).toBe(3); | ||
| expect(target.value).toBe(1); | ||
| }); | ||
|
|
||
| test("defines an unchanged property on the target", () => { | ||
| const target = { value: 1 }; | ||
| const proxied = proxyMembers(target, {}); | ||
| Object.defineProperty(proxied, "value", { configurable: true, value: 2 }); | ||
| expect(target.value).toBe(2); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== proxy-members implementation ==\n'
file="src/shared/proxy-members.ts"
wc -l "$file"
cat -n "$file"
printf '\n== search for getOwnPropertyDescriptor usage in tests ==\n'
rg -n "getOwnPropertyDescriptor|proxyMembers" test src -g '!**/dist/**' -g '!**/build/**'
printf '\n== locate proxyMembers tests ==\n'
fd -a "proxy-members.test.ts" test srcRepository: chobbledotcom/tickets
Length of output: 3507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/shared/proxy-members.ts"
printf '\n== %s ==\n' "$file"
wc -l "$file"
cat -n "$file"
printf '\n== search for getOwnPropertyDescriptor usage in tests ==\n'
rg -n "getOwnPropertyDescriptor|proxyMembers" test src -g '!**/dist/**' -g '!**/build/**'
printf '\n== locate proxyMembers tests ==\n'
fd -a "proxy-members.test.ts" test srcRepository: chobbledotcom/tickets
Length of output: 3506
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== test/test-utils/record-queries.ts ==\n'
wc -l test/test-utils/record-queries.ts
cat -n test/test-utils/record-queries.ts
printf '\n== where record-queries utility is used ==\n'
rg -n "record-queries|recordQueries" test src
printf '\n== any Object.getOwnPropertyDescriptor / getOwnPropertyDescriptors around proxyMembers call sites ==\n'
rg -n "Object\\.getOwnPropertyDescriptor|Object\\.getOwnPropertyDescriptors|defineProperties|proxyMembers\\(" test srcRepository: chobbledotcom/tickets
Length of output: 5674
Add direct getOwnPropertyDescriptor coverage
The current tests hit get, set, and defineProperty, but not the descriptor trap. Add one case for a replacement member and one for an unchanged member so both branches in proxyMembers are exercised.
🤖 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 `@test/shared/proxy-members.test.ts` around lines 1 - 56, Add direct
getOwnPropertyDescriptor coverage to the proxyMembers tests: add one test that
retrieves a descriptor for a replacement member and verifies the replacement
branch, plus one test for an unchanged member that verifies the target branch.
Use Object.getOwnPropertyDescriptor on the proxied object and assert the
returned descriptors reflect the respective values.
Source: Learnings
…t-limit fallback Address two Codex P2 review threads on #1827: 1. 'Defer stray deletion until after saving webhook credentials': setupWebhookEndpointImpl no longer deletes old endpoints. It creates the new endpoint only and returns. The caller (settings-stripe.ts) saves the new endpoint ID + secret to the DB FIRST, then calls the new cleanupOldWebhookEndpoints function to delete stale same-URL endpoints. A DB-save failure leaves the old endpoint (whose secret matches the DB) alive — webhooks keep delivering instead of losing the only signed endpoint mid-replacement. 2. 'Add a cleanup fallback for endpoint-limit failures': If Stripe rejects the create because the account is at its webhook endpoint cap (~16), setup now deletes same-URL strays (keeping the recorded endpoint intact so webhooks keep delivering if the retry also fails) and retries the create. This lets admins recover from the duplicate-endpoint state without manual Stripe dashboard work. Extracted createCheckoutWebhook, sameUrlEndpointIdsExcept, and deleteEndpointsBestEffort as shared helpers to keep jscpd at 0%. Tests rewritten for the two-phase flow: setup creates only (no deletes), cleanup deletes after DB save, endpoint-limit fallback deletes strays and retries, keeps recorded endpoint during recovery. Also merges origin/main (PRs #1820–#1824) and resolves TODO.md conflict preserving both branches' entries.
1 participant
Summary
TODO.mdwith their trigger and a clear fix.Measured result
The same 12-group, 2-package fixture was measured with eight fresh processes at each latency:
/listingsdatabase calls fell from 119 to 30.Raw samples and benchmark limits are in
docs/cold-start.md. The benchmark no longer uses speculative 50 or 100 ms database delays.Request safety
Bunny allows 50 subrequests per request. The database client now counts statements, batches, transaction operations, migration batches, scripts, and replica syncs in request scope. Statements inside one batch count once. Routes that also call payment, email, storage, or other providers should target no more than 40 database calls.
The audit also found larger follow-up work in package carts, registration webhooks, multi-entry check-in, order availability, site assignment, old migrations, large backups, bulk email, CSV export, seed generation, and two group-admin reads. These remain explicit follow-ups rather than hidden failures.
Validation
deno task precommitThe request benchmark uses local SQLite with deterministic simulated latency. Bundle load and request work remain separate measurements and are not presented as one production end-to-end time.
Summary by CodeRabbit