feat: recover privacy-minimized clinical feedback#611
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds APIs for recording answer feedback and document source reviews, with validation, rate limiting, Supabase persistence, cache invalidation, tests, sitemap entries, and interaction IDs in answer responses. ChangesFeedback and source review APIs
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SourceReviewRoute
participant Supabase
participant RAGCache
Client->>SourceReviewRoute: Submit source review
SourceReviewRoute->>Supabase: Record review via RPC
Supabase-->>SourceReviewRoute: Return recorded review
SourceReviewRoute->>RAGCache: Invalidate owner cache
SourceReviewRoute-->>Client: Return 201 response
sequenceDiagram
participant ClinicalDashboard
participant AnswerFeedbackRoute
participant Supabase
ClinicalDashboard->>AnswerFeedbackRoute: Submit interaction and hashed answer feedback
AnswerFeedbackRoute->>Supabase: Insert feedback row
Supabase-->>AnswerFeedbackRoute: Return insertion result
AnswerFeedbackRoute-->>ClinicalDashboard: Return success or error response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3891152cc5
ℹ️ 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. |
|
To use Codex here, create a Codex account and connect to github. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/components/ClinicalDashboard.tsx (1)
2573-2589: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
authBoundFetchinstead of plainfetchfor consistency and cancellation.Every other mutation in this file (
mutateDocumentLabel,retryJob,reindexDocument) goes throughauthBoundFetch, which ties the request to anAbortControllerand validates the auth epoch before committing state. This call uses plainfetchwith no abort signal, so a hung request leavespendingFeedbackset indefinitely (buttons stuck disabled) with no way to cancel on unmount or a superseding auth event.♻️ Proposed fix
- const response = await fetch("/api/answer-feedback", { - method: "POST", - headers: { - "Content-Type": "application/json", - ...authorizationHeader, - }, - body: JSON.stringify({ - interactionId: answer.interactionId, - feedbackCategory: feedbackType, - answerHash, - sourceIds: sourceChunkIds, - citedSourceIds: citedChunkIds, - route: answer.routingMode ?? null, - model: answer.modelUsed ?? null, - providerRequestIds: answer.openAIRequestIds ?? [], - }), - }); + const { response, requestEpoch } = await authBoundFetch("/api/answer-feedback", { + method: "POST", + headers: { + "Content-Type": "application/json", + ...authorizationHeader, + }, + body: JSON.stringify({ + interactionId: answer.interactionId, + feedbackCategory: feedbackType, + answerHash, + sourceIds: sourceChunkIds, + citedSourceIds: citedChunkIds, + route: answer.routingMode ?? null, + model: answer.modelUsed ?? null, + providerRequestIds: answer.openAIRequestIds ?? [], + }), + }); + if (!isAuthEpochCurrent(requestEpoch)) return;🤖 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/components/ClinicalDashboard.tsx` around lines 2573 - 2589, Replace the plain fetch call in the answer-feedback mutation with the file’s existing authBoundFetch helper, passing the same POST options and request body while preserving the authorization headers. Ensure the mutation uses the helper’s cancellation and auth-epoch behavior so pendingFeedback can be cleared when the request is aborted or superseded.tests/answer-feedback-route.test.ts (1)
11-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the duplicate/rate-limited/demo-mode branches too.
Only the happy path is tested. The 409 duplicate-detection branch, 429 rate-limited branch, and demo-mode 400 rejection in
route.tsare untested, and the duplicate branch is exactly what's flagged as fragile insrc/app/api/answer-feedback/route.ts.As per path instructions, "For high-risk code involving authentication, authorization, private data, database access, clinical output, ingestion, provider calls, billing, uploads, or background jobs, review carefully for regressions, missing tests, rollback safety, and conservative failure behavior."
🤖 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 `@tests/answer-feedback-route.test.ts` around lines 11 - 56, Expand the “answer feedback route” tests to cover the 409 duplicate-detection, 429 rate-limit, and 400 demo-mode rejection branches in POST. Reconfigure the existing mocks to trigger each branch, assert the corresponding response status, and verify duplicate handling remains covered as implemented in route.ts.Source: Path instructions
🤖 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/app/api/answer-feedback/route.ts`:
- Around line 66-70: Update the error handling in the answer feedback insertion
flow to detect duplicate submissions by checking error.code for the stable
unique-violation code 23505 instead of matching error.message with a regex.
Preserve the existing 409 response for that code and the PublicApiError path for
other insertion failures.
- Around line 17-20: Update the answer-feedback handler around bodySchema and
its persistence flow to verify interactionId references an existing answer
before storing feedback. Add a server-side answer lookup or ledger relationship
check after UUID validation, reject fabricated or nonexistent IDs, and preserve
the existing uniqueness validation for legitimate feedback.
In `@src/app/api/documents/`[id]/reviews/route.ts:
- Around line 21-26: Update the reviewDate validation schema to parse valid
calendar dates and reject dates after the current date in the RPC’s
Australia/Perth timezone, while preserving nullable and optional behavior.
Locate the route’s RPC invocation to ensure the same date basis is used
consistently, and remove the future-date fixture from source-review-route tests.
In `@src/components/ClinicalDashboard.tsx`:
- Line 2587: Update the providerRequestIds assignment in the answer-feedback
submission to send at most 10 IDs from answer.openAIRequestIds, preserving the
existing empty-array fallback when no IDs are present.
---
Nitpick comments:
In `@src/components/ClinicalDashboard.tsx`:
- Around line 2573-2589: Replace the plain fetch call in the answer-feedback
mutation with the file’s existing authBoundFetch helper, passing the same POST
options and request body while preserving the authorization headers. Ensure the
mutation uses the helper’s cancellation and auth-epoch behavior so
pendingFeedback can be cleared when the request is aborted or superseded.
In `@tests/answer-feedback-route.test.ts`:
- Around line 11-56: Expand the “answer feedback route” tests to cover the 409
duplicate-detection, 429 rate-limit, and 400 demo-mode rejection branches in
POST. Reconfigure the existing mocks to trigger each branch, assert the
corresponding response status, and verify duplicate handling remains covered as
implemented in route.ts.
🪄 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: aa57e9ca-d98c-4877-bc7c-5cf630f4536f
📒 Files selected for processing (12)
docs/site-map.mdsrc/app/api/answer-feedback/route.tssrc/app/api/answer/route.tssrc/app/api/answer/stream/route.tssrc/app/api/documents/[id]/reviews/route.tssrc/components/ClinicalDashboard.tsxsrc/lib/api-rate-limit.tssrc/lib/source-review.tssrc/lib/supabase/database.types.tssrc/lib/types.tstests/answer-feedback-route.test.tstests/source-review-route.test.ts
1 participant
Summary
Why
This recovers the patch-unique application layer from the completed Domain 1 governance branch. The supporting schema, migrations, append-only review RPC, and feedback table are already on
main; obsolete migration and production-posture history was intentionally not carried forward.Areas touched
Verification
npm exec vitest run tests/answer-feedback-route.test.ts tests/source-review-route.test.ts tests/api-rate-limit-fallback.test.ts tests/private-rag-access.test.ts tests/private-access-routes.test.ts tests/rag-answer-fallback.test.ts— 6 files, 161 tests passed after main refreshnpm run typecheck— passed after refreshing the dependency tree for the updated main basenpm run test— 213 files passed, 1 skipped; 1,940 tests passed, 1 skippednpm run eval:rag:offline— fixture schema plus 21 files / 265 tests passednpm run build— production webpack build and client-bundle secret scan passeddemo answer flow reaches a source-backed answer @critical— passedChecks not fully completed
npm run check:production-readinessran locally but reported missing Supabase/OpenAI env configuration in the isolated cleanup worktree; no provider call or live mutation was madeProduction readiness / governance
Summary by CodeRabbit
New Features
Documentation
Tests