rag: add deterministic and opt-in semantic reranking#901
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThis PR adds offline ranking snapshot generation and tuning, query-class feature-fusion weights, separate rank and confidence scores, and ambiguity-only semantic reranking with strict validation, fail-open behavior, cache awareness, and telemetry across RAG retrieval paths. ChangesRanking and semantic reranking
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Search as searchChunksWithTelemetry
participant Reranker as semanticRerankIfAmbiguous
participant Provider as SemanticRerankGenerator
participant Cache as RAG cache
Search->>Reranker: candidate results and eligibility
Reranker->>Provider: bounded evidence with structured schema
Provider-->>Reranker: validated ranking response
Reranker-->>Search: reordered or unchanged results
Search->>Cache: cache final results and telemetry
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d650ecb89b
ℹ️ 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".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/rag.ts (1)
713-732: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the computed rank for results without
score_explanation.When the optional explanation is absent, Line 715 leaves it undefined, so the comparator falls back to the original hybrid score and discards every computed second-stage adjustment. Sort using an internal
rankScorefield, then remove it from the returned result.🤖 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/lib/rag.ts` around lines 713 - 732, Update the second-stage rerank mapping and subsequent sort in the results pipeline to preserve the computed rank when score_explanation is absent. Store the computed rankScore in an internal field for every result, use that field in the comparator instead of falling back to hybrid_score or similarity, then remove the internal field before returning results while retaining the existing explanation updates.
🤖 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 `@scripts/build-ranking-snapshot.ts`:
- Around line 64-69: Update labelMatches so each normalized label alternative is
matched as a standalone token rather than via arbitrary substring matching,
while preserving case-insensitive OR handling and full-word labels. Add
regression cases covering short abbreviations such as “ed”, “im”, and “po” not
matching embedded characters in unrelated words, and retain matches when they
appear as distinct tokens.
In `@scripts/lib/ranking-tuning.ts`:
- Around line 214-224: Update the hard-negative evaluation around
firstRelevantIndex so cases with no relevant candidate are not counted as
hard-negative ordering failures. Track missing-positive retrieval separately (or
reject/repair invalid snapshots), and ensure high-risk failure and
correctHardNegatives metrics only reflect cases where firstRelevantIndex is
nonnegative, including the agitation-im-po-options and flowchart-next-step
fixtures.
- Around line 125-143: Extend the validation in the ranking snapshot parser
before the `return value as RankingSnapshot` cast to cover every field consumed
by evaluation: require boolean `documentMatch` and sanitization values, validate
labels, hard-negative category and risk fields, and ensure `sourceCaseCount`
equals `cases.length`. Update the `hardNegative` check to reject invalid
non-boolean/null values so only valid hard negatives contribute to
`hardNegativeCount`, while preserving the existing candidate hash, grade,
feature, and minimum-count checks.
In `@src/lib/rag.ts`:
- Around line 729-734: Update the final result-ordering flow in the relevant
reranking function to reassign each result’s score_explanation.finalRank after
every reorder, including the second-stage sort and subsequent semantic rerank.
Ensure the same rank refresh runs when semantic reranking is disabled or fails
open, so finalRank always matches the returned array position.
In `@src/lib/semantic-rerank.ts`:
- Around line 174-178: Update the error handling around providerFailureReason
and the generator catch path to distinguish caller cancellation from provider
failures: when args.signal is aborted, re-throw the cancellation error so
retrieval stops, while retaining "timeout" classification and fail-open behavior
for provider or internal timeout errors.
In `@src/lib/types.ts`:
- Around line 429-438: Update scripts/build-ranking-snapshot.ts to use the exact
runtime components from each record’s fusionSignals as the snapshot inputs for
metadata, clinical, and fixed adjustments. Keep the existing aggregate-field
reconstruction only as a fallback for legacy fixtures that lack fusionSignals,
ensuring tuning evaluates the same scoring function as runtime.
---
Outside diff comments:
In `@src/lib/rag.ts`:
- Around line 713-732: Update the second-stage rerank mapping and subsequent
sort in the results pipeline to preserve the computed rank when
score_explanation is absent. Store the computed rankScore in an internal field
for every result, use that field in the comparator instead of falling back to
hybrid_score or similarity, then remove the internal field before returning
results while retaining the existing explanation updates.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 321f501c-e664-4ed7-bba5-5fa5534ec086
📒 Files selected for processing (21)
.env.examplescripts/build-ranking-snapshot.tsscripts/fixtures/rag-ranking-candidate-snapshot.v1.jsonscripts/lib/ranking-tuning.tsscripts/tune-search-weights.tssrc/lib/clinical-search.tssrc/lib/env.tssrc/lib/openai.tssrc/lib/rag-cache.tssrc/lib/rag-contracts.tssrc/lib/rag.tssrc/lib/ranking-config.tssrc/lib/retrieval-selection.tssrc/lib/semantic-rerank.tssrc/lib/types.tstests/clinical-search.test.tstests/rag-second-stage-ranking.test.tstests/ranking-config.test.tstests/ranking-tuning.test.tstests/retrieval-selection.test.tstests/semantic-rerank.test.ts
|
Summary
Git / PR
Testing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 419ebf9197
ℹ️ 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 reordered = args.results.map((result) => | ||
| bandIds.has(result.id) ? sortedBand[replacementIndex++].result : result, | ||
| ); |
There was a problem hiding this comment.
Preserve semantic rerank through answer ranking
When RAG_SEMANTIC_RERANK_ENABLED=true and the model reorders an ambiguous answer search, this code only swaps array positions and leaves each result's rankScore, hybrid_score, and answer-evidence inputs unchanged. answerQuestionWithScopeUncoalesced immediately feeds the returned results to rankAnswerEvidence (src/lib/rag.ts lines 3455-3456), which sorts by its own evidence score and then hybrid_score before using original index (src/lib/answer-ranking.ts lines 238-248), so any semantic reorder where those scores differ is discarded before source selection/answer generation. Propagate a semantic rank signal into the downstream ranking or make the answer path preserve semantic order.
Useful? React with 👍 / 👎.
| reasoningEffort: "none", | ||
| textVerbosity: "low", | ||
| timeoutMs: 3_000, | ||
| maxRetries: 0, | ||
| signal: args.signal, |
There was a problem hiding this comment.
Thread the safety identifier into rerank requests
When semantic reranking is enabled for an authenticated owner and OPENAI_SAFETY_IDENTIFIER_SECRET is set, this new Responses request sends the user's query plus retrieved clinical candidate snippets but omits the HMAC safetyIdentifier. The repo's PIA and OpenAI operations docs state authenticated Responses requests should carry that pseudonym while raw owner IDs are never sent (docs/privacy-impact-assessment.md lines 162-164; docs/openai-rag-operations.md lines 55-56), and the existing classifier/answer/summary provider calls already do so. Add ownerId/safetyIdentifier plumbing for semantic rerank so canary traffic keeps the same privacy and abuse-tracing invariant.
Useful? React with 👍 / 👎.
Summary
RAG_SEMANTIC_RERANK_ENABLED=false; it fails open to deterministic ordering and uses structured output.Verification
npm run test— 315 files, 2,853 tests passed.npm run eval:rag:offline— 36 golden cases, 21 suites, 294 tests passed.npm run typechecknpm run check:production-readiness:ci— READY; only expected missing-local-env warnings.npm run verify:pr-localVerification not run:
npm run verify:pr-localwas not repeated after its constituent full unit, typecheck, lint, offline RAG, and readiness checks passed; hosted CI remains responsible for the clean production build.Live verification not run:
npm run eval:retrieval:qualityand semantic-rerank canary require provider credentials and separate approval. No provider calls were made.UI verification not run: no UI, routing, styling, or browser behavior changed.
Risk and rollout
RAG_SEMANTIC_RERANK_ENABLED=falseto disable the semantic stage independently.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Summary by CodeRabbit
New Features
OPENAI_RERANK_MODELandRAG_SEMANTIC_RERANK_ENABLED.rankScoreand fusion signals).Bug Fixes
rankScore, with updated telemetry and second-stage reranking behavior.Tests