Skip to content

perf: remove serial dead weight from the answer hot path (round 2, phase 3) #480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BigSimmo merged 1 commit into from
Jul 11, 2026
Merged

perf: remove serial dead weight from the answer hot path (round 2, phase 3) #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions src/app/api/answer/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,23 +83,27 @@ export async function POST(request: Request) {
const access = await publicAccessContext(request, supabase);
const publicOnly = !access.authenticated && !isLocalNoAuthMode();

const rateLimit = await consumeSubjectApiRateLimit({
supabase,
subject: access.rateLimitSubject,
bucket: "answer",
allowInMemoryFallbackOnUnavailable: allowRateLimitInMemoryFallbackOnUnavailable(),
});
// 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([

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep scope resolution behind the rate-limit gate

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 👍 / 👎.

consumeSubjectApiRateLimit({
supabase,
subject: access.rateLimitSubject,
bucket: "answer",
allowInMemoryFallbackOnUnavailable: allowRateLimitInMemoryFallbackOnUnavailable(),
}),
resolveSearchScope({
supabase,
ownerId: access.ownerId,
publicOnly,
documentIds: answerBody.documentIds ?? (answerBody.documentId ? [answerBody.documentId] : undefined),
filters: answerBody.filters,
}),
]);
if (rateLimit.limited) {
return rateLimitJsonResponse("Too many answer requests. Retry shortly.", rateLimit);
}

const scope = await resolveSearchScope({
supabase,
ownerId: access.ownerId,
publicOnly,
documentIds: answerBody.documentIds ?? (answerBody.documentId ? [answerBody.documentId] : undefined),
filters: answerBody.filters,
});
if (scope.documentIds?.length === 0) {
return NextResponse.json({
answer:
Expand Down
38 changes: 29 additions & 9 deletions src/app/api/answer/stream/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,13 @@ function buildDemoStreamAnswer(body: AnswerBody, fallbackReason?: string) {
};
}

function streamAnswer(body: AnswerBody, ownerId?: string, signal?: AbortSignal, publicOnly = false) {
function streamAnswer(
body: AnswerBody,
ownerId?: string,
signal?: AbortSignal,
publicOnly = false,
scopePromise?: ReturnType<typeof resolveSearchScope>,
) {
const encoder = new TextEncoder();

return new Response(
Expand All @@ -156,13 +162,14 @@ function streamAnswer(body: AnswerBody, ownerId?: string, signal?: AbortSignal,
send("progress", { stage: "retrieving", message: "Searching indexed documents." });
const scope = isDemoMode()
? null
: await resolveSearchScope({
supabase: createAdminClient(),
ownerId,
publicOnly,
documentIds: body.documentIds ?? (body.documentId ? [body.documentId] : undefined),
filters: body.filters,
});
: await (scopePromise ??
resolveSearchScope({
supabase: createAdminClient(),
ownerId,
publicOnly,
documentIds: body.documentIds ?? (body.documentId ? [body.documentId] : undefined),
filters: body.filters,
}));
if (scope?.documentIds?.length === 0) {
send("final", {
answer:
Expand Down Expand Up @@ -265,6 +272,19 @@ export async function POST(request: Request) {
const access = await publicAccessContext(request, supabase);
const publicOnly = !access.authenticated && !isLocalNoAuthMode();

// Kick off the read-only scope resolution alongside the rate-limit consume;
// the stream awaits it (and the limit still rejects before retrieval). The
// detached catch marks a pre-await rejection handled — the stream's own
// await still observes and reports the real error.
const scopePromise = resolveSearchScope({
supabase,
ownerId: access.ownerId,
publicOnly,
documentIds: body.documentIds ?? (body.documentId ? [body.documentId] : undefined),
filters: body.filters,
});
void scopePromise.catch(() => undefined);

const rateLimit = await consumeSubjectApiRateLimit({
supabase,
subject: access.rateLimitSubject,
Expand All @@ -273,7 +293,7 @@ export async function POST(request: Request) {
});
if (rateLimit.limited) return rateLimitStream(rateLimit);

return streamAnswer(body, access.ownerId, request.signal, publicOnly);
return streamAnswer(body, access.ownerId, request.signal, publicOnly, scopePromise);
} catch (error) {
if (error instanceof AuthenticationError) {
return unauthorizedResponse(error);
Expand Down
23 changes: 16 additions & 7 deletions src/lib/rag-cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,10 @@ type SharedCacheMissReason =
| "expired"
| "indexing_version_mismatch"
| "dependency_version_mismatch"
| "unknown_filter_miss";
| "unknown_filter_miss"
// Placeholder while the off-path classification probe is still in flight;
// telemetry is patched with the real reason when it resolves (getSharedCachedSearch).
| "pending_probe";

function sharedCacheSelector(
supabase: ReturnType<typeof createAdminClient>,
Expand Down Expand Up @@ -316,14 +319,16 @@ export async function getSharedCachedSearch(
queryVariants: string[] = [],
): Promise<
| { kind: "hit"; results: SearchResult[]; telemetry: SearchTelemetry }
| { kind: "miss"; reason: SharedCacheMissReason }
// `deferredReason` classifies a generic miss (no_entry/expired/version mismatch)
// via a follow-up metadata query that must NOT block retrieval: the caller
// starts with `reason` ("pending_probe") and patches telemetry when it lands.
| { kind: "miss"; reason: SharedCacheMissReason; deferredReason?: Promise<SharedCacheMissReason> }
| null
> {
if (args.skipCache || env.RAG_SEARCH_CACHE_TTL_MS <= 0) return null;
const normalizedQuery = retrievalPlanCacheQuery(args, queryClass, queryVariants);
const indexingVersion = await cacheIndexingVersion(args);
async function probeSharedCacheMissReason(reasonFromLookup?: SharedCacheMissReason): Promise<SharedCacheMissReason> {
if (reasonFromLookup) return reasonFromLookup;
async function probeSharedCacheMissReason(): Promise<SharedCacheMissReason> {
try {
const supabase = createAdminClient();
let probeQuery = supabase
Expand Down Expand Up @@ -361,11 +366,15 @@ export async function getSharedCachedSearch(
indexingVersion,
normalizedQuery,
).maybeSingle();
if (error) return { kind: "miss", reason: await probeSharedCacheMissReason("cache_lookup_error") };
if (!data?.payload) return { kind: "miss", reason: await probeSharedCacheMissReason() };
if (error) return { kind: "miss", reason: "cache_lookup_error" };
if (!data?.payload) {
// Do not await the classification probe on the hot path — it costs a full
// extra rag_response_cache round trip before retrieval can start.
return { kind: "miss", reason: "pending_probe", deferredReason: probeSharedCacheMissReason() };
}
const payload = data.payload as { results?: SearchResult[]; telemetry?: Partial<SearchTelemetry> };
if (!Array.isArray(payload.results)) {
return { kind: "miss", reason: await probeSharedCacheMissReason("cache_payload_invalid") };
return { kind: "miss", reason: "cache_payload_invalid" };
}
return {
kind: "hit",
Expand Down
Loading
Loading