perf: remove serial dead weight from the answer hot path (round 2, phase 3)#480
Conversation
Round-2 phase 3 (results-identical orchestration; no retrieval behavior change). - Shared-cache miss classification no longer blocks retrieval: the probe that cost a full extra rag_response_cache round trip on every generic miss now runs off-path and patches telemetry when it resolves (reason starts as "pending_probe"; query logging happens after generation, so the classified value wins in practice). Lookup-derived reasons stay synchronous. - /api/answer runs the rate-limit consume and the read-only scope resolution concurrently instead of serially; the limit still rejects before retrieval or generation starts. /api/answer/stream kicks scope resolution off next to the rate-limit consume and awaits it inside the stream (detached catch marks a pre-await rejection handled; the stream's own await reports the error). - Every retrieval match_* RPC (9 sites: chunk text, trigram correction, document lookup, documents-for-query, table facts, embedding-field hybrid, index-unit hybrid, chunk hybrid, vector fallback) is now bounded by the caller's AbortSignal via a withAbort helper, so a client disconnect cancels in-flight Postgres work and frees the pooled connection. throwIfAborted only checked between stages; aborted RPCs resolve through the existing error branches and the pipeline unwinds at the next stage check. - Anonymous public-scope resolution (a paginated enumeration of every public indexed document per request) is cached for 60s, keyed by client instance (WeakMap) so the production admin singleton shares one entry while tests stay isolated. Only the zero-filter default scope is cached; the resolved object is defensively cloned on read and write. Verified: tsc, eslint, prettier; targeted vitest (search-scope, rag-cache, owner-scope, api-validation-contract, private-access-routes, rag-answer-fallback, answer-telemetry) all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![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: 848fa9248a
ℹ️ 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".
| // Independent given `access`: the rate-limit consume and the read-only | ||
| // scope resolution overlap instead of running serially. The limit still | ||
| // rejects before any retrieval or generation starts. | ||
| const [rateLimit, scope] = await Promise.all([ |
There was a problem hiding this comment.
When a subject is already over the answer limit, this Promise.all still starts and waits for resolveSearchScope before the 429 branch can run. For uncached anonymous scopes, filters, or explicit document IDs, scope resolution can page through documents/labels; if that read is slow or fails, a request that should be a cheap 429 is delayed or becomes a 500, and the stream route now starts the same scope work before its rate-limit check as well. A focused regression test would make the limiter return limited: true and the documents query throw, and assert the route returns 429 without touching scope queries.
Useful? React with 👍 / 👎.
1 participant
Summary
Phase 3 of the round-2 performance initiative (stacked on #479). Results-identical orchestration changes on the answer hot path — no retrieval behavior, ranking, or scoring is touched, so no eval gate applies (per
docs/capacity-review.mdthis PR deliberately stays on the orchestration side of that line).rag_response_cachequery purely to classify the miss reason before retrieval could start. The probe now runs detached and patches telemetry when it lands (pending_probeplaceholder; query logging happens after generation so the classified reason still wins in practice). Saves one serial DB round trip on every cold answer./api/answerand/api/answer/stream. The rate limit still rejects before any retrieval or generation.match_*/correction RPC sites are now bounded by the caller's signal via awithAborthelper — a client disconnect cancels in-flight Postgres work and frees the pooled connection (previouslythrowIfAbortedonly checked between stages, so up to 10–14 RPCs ran to completion for a caller that had already gone). Aborted RPCs resolve through the existing error branches; the pipeline unwinds at the next stage check. Addresses the reliability finding that the fan-out had no server-side time-bounding tied to the request.Verification
tsc,eslint,prettiercleansearch-scope,rag-cache-utils,rag-cache-invalidation,owner-scope,api-validation-contract,private-access-routes,rag-answer-fallback,answer-telemetry— all green🤖 Generated with Claude Code