-
Notifications
You must be signed in to change notification settings - Fork 0
fix: restore drift manifest metadata to unblock design-audit merge #849
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.
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
Changes from all commits
f9630eb
9294b6f
94b4d11
0caef13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,13 @@ import type { DocumentIndexUnitMatch, DocumentMemoryCard, SearchResult } from "@ | |
| // the floor, which is how the live schema drift (42702) went unnoticed. Log it structurally and, | ||
| // where telemetry is in scope, record the failing RPC + code so it shows up in rag_retrieval_logs. | ||
| export type SupabaseRpcError = { message?: string; code?: string; details?: string; hint?: string } | null; | ||
| type RpcResult<T> = Promise<{ data: T | null; error: SupabaseRpcError }>; | ||
| type AbortableRpc<T> = RpcResult<T> & { | ||
| abortSignal?: (signal: AbortSignal) => RpcResult<T>; | ||
| }; | ||
| type SupabaseRpcClient = { | ||
| rpc: (name: string, rpcArgs: Record<string, unknown>) => AbortableRpc<unknown[]> | PromiseLike<unknown>; | ||
| }; | ||
|
|
||
| function legacyRankFields(versionedName: string) { | ||
| if (versionedName === "match_document_chunks_v2") return ["similarity"]; | ||
|
|
@@ -81,15 +88,20 @@ export async function callVersionedRetrievalRpc<T extends unknown[] = unknown[]> | |
| versionedName: string, | ||
| legacyName: string, | ||
| args: Record<string, unknown>, | ||
| signal?: AbortSignal, | ||
| ): Promise<{ data: T | null; error: SupabaseRpcError }> { | ||
| const client = supabase as unknown as { | ||
| rpc: (name: string, rpcArgs: Record<string, unknown>) => Promise<{ data: T | null; error: SupabaseRpcError }>; | ||
| const client = supabase as unknown as SupabaseRpcClient; | ||
| const executeRpc = async (name: string, rpcArgs: Record<string, unknown>) => { | ||
| const pending = client.rpc(name, rpcArgs) as AbortableRpc<T>; | ||
| const pendingWithAbort = | ||
| signal && typeof pending.abortSignal === "function" ? pending.abortSignal(signal) : pending; | ||
| return await pendingWithAbort; | ||
| }; | ||
| const versioned = await client.rpc(versionedName, args); | ||
| const versioned = await executeRpc(versionedName, args); | ||
| if (versioned && !isMissingRetrievalRpcError(versioned.error)) return versioned; | ||
| const legacyArgs = { ...args }; | ||
| delete legacyArgs.include_public; | ||
| const ownerResult = await client.rpc(legacyName, legacyArgs); | ||
| const ownerResult = await executeRpc(legacyName, legacyArgs); | ||
|
Comment on lines
+91
to
+104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Wire the caller signal through retrieval RPC call sites.
Pass Proposed call-site fix@@ hybrid retrieval call
},
+ args.signal,
);
@@ document-filter fallback call
},
+ args.signal,
);🤖 Prompt for AI Agents |
||
| const ownerFilter = String(args.owner_filter ?? ""); | ||
| if ( | ||
| ownerResult.error || | ||
|
|
@@ -99,7 +111,7 @@ export async function callVersionedRetrievalRpc<T extends unknown[] = unknown[]> | |
| ) { | ||
| return ownerResult; | ||
| } | ||
| const publicResult = await client.rpc(legacyName, { | ||
| const publicResult = await executeRpc(legacyName, { | ||
| ...legacyArgs, | ||
| owner_filter: PUBLIC_OWNER_FILTER_SENTINEL, | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { createAdminClient } from "@/lib/supabase/admin"; | |
| import { retrievalAccessScopeForArgs, retrievalRpcScopeArgs } from "@/lib/owner-scope"; | ||
| import { | ||
| callVersionedRetrievalRpc, | ||
| createChunkLoadCache, | ||
| memoryCardChunkScore, | ||
| mergeSearchResults, | ||
| recordHybridRpcError, | ||
|
|
@@ -11,7 +12,6 @@ import { | |
| searchTableFactCandidates, | ||
| searchTextChunkCandidates, | ||
| withMemoryBoostedCandidates, | ||
| createChunkLoadCache, | ||
| type MemoryCardCache, | ||
| } from "@/lib/rag-candidate-sources"; | ||
| export { | ||
|
|
@@ -120,6 +120,11 @@ export { | |
| retrievalPlanCacheQuery, | ||
| } from "@/lib/rag-cache"; | ||
| import { classifySearchCacheOutcome, recordCacheLookup } from "@/lib/observability/cache-metrics"; | ||
| import { | ||
| recordAnswerOrigination, | ||
| recordAnswerOriginationFinished, | ||
| recordCoalescedAnswerWaiter, | ||
| } from "@/lib/observability/answer-coalescing-metrics"; | ||
| import { buildRagSourceBlock, compactContextText, neutralizeIdentityField } from "@/lib/rag-source-block"; | ||
| export { buildRagSourceBlock, truncateForModel } from "@/lib/rag-source-block"; | ||
| import { | ||
|
|
@@ -423,6 +428,26 @@ function throwIfAborted(signal?: AbortSignal) { | |
| } | ||
| } | ||
|
|
||
| function awaitWithCallerSignal<T>(pending: Promise<T>, signal?: AbortSignal): Promise<T> { | ||
| if (!signal) return pending; | ||
| if (signal.aborted) throw signal.reason ?? new DOMException("The operation was aborted.", "AbortError"); | ||
|
|
||
| return new Promise<T>((resolve, reject) => { | ||
| const onAbort = () => reject(signal.reason ?? new DOMException("The operation was aborted.", "AbortError")); | ||
| signal.addEventListener("abort", onAbort, { once: true }); | ||
|
Comment on lines
+431
to
+437
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Map the relevant area first
ast-grep outline src/lib/rag.ts --view expanded || true
# Read the surrounding implementation around the cited lines
sed -n '400,470p' src/lib/rag.ts
# Read the catch block area mentioned in the comment
sed -n '1360,1425p' src/lib/rag.ts
# Search for tests or other abort-handling helpers
rg -n "awaitWithCallerSignal|AbortError|signal\.reason|aborted" src test tests __tests__ . 2>/dev/null || trueRepository: BigSimmo/Database Length of output: 50373 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Look for non-DOMException abort reasons in the repo
rg -n "abort\\(new Error|abort\\([^)]*reason|abort\\(signal\\.reason|abort\\([^)]*DOMException|AbortController\\(" src tests . 2>/dev/null | sed -n '1,220p'
# Read the rag abort-related test file if present
if [ -f tests/rag-abort-signal.test.ts ]; then
sed -n '1,220p' tests/rag-abort-signal.test.ts
fi
# Show the classifier fallback function with enough context to assess the catch behavior
sed -n '1300,1415p' src/lib/rag.tsRepository: BigSimmo/Database Length of output: 18866 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the route budget test that uses a custom abort reason
sed -n '1,120p' tests/rag-route-budget.test.ts
# Inspect the rag abort signal tests for expected semantics
sed -n '1,180p' tests/rag-abort-signal.test.ts
# Inspect the universal-search wrapper since it forwards caller reasons
sed -n '150,230p' src/lib/universal-search.tsRepository: BigSimmo/Database Length of output: 8031 Preserve non- 🤖 Prompt for AI Agents |
||
| pending.then( | ||
| (value) => { | ||
| signal.removeEventListener("abort", onAbort); | ||
| resolve(value); | ||
| }, | ||
| (error) => { | ||
| signal.removeEventListener("abort", onAbort); | ||
| reject(error); | ||
| }, | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| export type AnswerProgressEvent = { | ||
| stage: | ||
| | "retrieved" | ||
|
|
@@ -1290,6 +1315,7 @@ export async function analyzeQueryWithClassifierFallback( | |
| // owner_filter retrieval will use so grounding can never see documents retrieval cannot. | ||
| corpusGrounding?: { supabase: ReturnType<typeof createAdminClient>; ownerFilter: string | null }; | ||
| ownerId?: string | null; | ||
| signal?: AbortSignal; | ||
| }, | ||
| ) { | ||
| if ( | ||
|
|
@@ -1361,10 +1387,16 @@ export async function analyzeQueryWithClassifierFallback( | |
| } | ||
|
|
||
| try { | ||
| const verdict = await pending; | ||
| const verdict = await awaitWithCallerSignal(pending, opts?.signal); | ||
| storeClassifierVerdictMemo(memoKey, verdict); | ||
| return applyClassifierVerdict(analysis, verdict); | ||
| } catch { | ||
| } catch (error) { | ||
| if ( | ||
| error && | ||
| (error instanceof DOMException || typeof error === "object") && | ||
| (error as { name?: string }).name === "AbortError" | ||
| ) | ||
| throw error; | ||
| // Transport/parse failures are deliberately NOT memoized: fall back to the deterministic | ||
| // analysis for this request only, and let the next request retry the classifier. | ||
| return analysis; | ||
|
|
@@ -2398,6 +2430,7 @@ export async function searchChunksWithTelemetry(args: SearchChunksArgs) { | |
| const queryAnalysis = await analyzeQueryWithClassifierFallback(retrievalQuery, analyzeClinicalQuery(retrievalQuery), { | ||
| corpusGrounding: corpusGroundingScope, | ||
| ownerId: args.ownerId, | ||
| signal: args.signal, | ||
| }); | ||
| throwIfAborted(args.signal); | ||
| if (modeQueryClass) queryAnalysis.queryClass = modeQueryClass; | ||
|
|
@@ -3123,6 +3156,7 @@ export async function answerQuestionWithScope(args: AnswerQuestionWithScopeArgs) | |
| let existing = inflightKey ? answerInflight.get(inflightKey) : undefined; | ||
|
|
||
| while (existing) { | ||
| recordCoalescedAnswerWaiter(); | ||
| await args.onProgress?.({ | ||
| stage: "cached", | ||
| message: "Waiting for an identical cited answer request already in progress.", | ||
|
|
@@ -3154,8 +3188,15 @@ export async function answerQuestionWithScope(args: AnswerQuestionWithScopeArgs) | |
| } | ||
| } | ||
|
|
||
| // Only coalescible requests belong in this process-local signal. Requests | ||
| // that intentionally bypass cache/coalescing must not make a replica look | ||
| // ineffective, and neither keys nor clinical content leave this function. | ||
| if (inflightKey) recordAnswerOrigination(); | ||
| const pending = answerQuestionWithScopeUncoalesced(args, startedAt).finally(() => { | ||
| if (inflightKey) answerInflight.delete(inflightKey); | ||
| if (inflightKey) { | ||
| answerInflight.delete(inflightKey); | ||
| recordAnswerOriginationFinished(); | ||
| } | ||
| }); | ||
| if (inflightKey) answerInflight.set(inflightKey, pending); | ||
| return pending; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: BigSimmo/Database
Length of output: 405
🏁 Script executed:
Repository: BigSimmo/Database
Length of output: 5556
🏁 Script executed:
Repository: BigSimmo/Database
Length of output: 5556
🏁 Script executed:
Repository: BigSimmo/Database
Length of output: 5493
🏁 Script executed:
Repository: BigSimmo/Database
Length of output: 3268
Project-level motion override should stay on reduced motion
contextOptions: { reducedMotion: "no-preference" }on the Chromium project overrides the suite-wide guard inuse, soui-smoke/ui-stresscan still hit transition races. Scope this to the tests that need motion, or keepreducehere.🤖 Prompt for AI Agents