Codex/align existing acls#971
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reached
Next review available in: 19 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 (1)
📝 WalkthroughWalkthroughThe changes add concurrent OpenAI embedding workers with configurable limits, adjust error and rate-limit handling, tighten registry URL metadata types, align Supabase ACLs, and add a partial index for indexed documents. ChangesRuntime behavior and registry routing
Supabase permissions and indexing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant embedTexts
participant WorkerPool
participant OpenAI
Caller->>embedTexts: submit texts and abort signal
embedTexts->>WorkerPool: create bounded batch workers
WorkerPool->>OpenAI: request concurrent embeddings
OpenAI-->>WorkerPool: return indexed batch results
WorkerPool-->>embedTexts: assemble embeddings by global index
embedTexts-->>Caller: return embeddings or first captured error
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…ions and update schema tests
|
@coderabbitai Please give me the exact copy and paste contents to add to the description to ensure this passes. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="BigSimmo/Database"
pr="971"
printf '%s\n' '--- PR metadata and body ---'
gh pr view "$pr" --repo "$repo" --json title,body,files,commits,statusCheckRollup \
--jq '{title, body, files: [.files[].path], commits: [.commits[].messageHeadline], checks: [.statusCheckRollup[]? | {name, conclusion, status}]}'
printf '%s\n' '--- Review comments ---'
gh api "repos/$repo/pulls/$pr/comments?per_page=100" \
--jq '.[] | {user: .user.login, path, line, body}' || true
printf '%s\n' '--- PR conversation comments ---'
gh api "repos/$repo/issues/$pr/comments?per_page=100" \
--jq '.[] | {user: .user.login, body}' || trueLength of output: 11271 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="BigSimmo/Database"
pr="971"
printf '%s\n' '--- Pull-request workflow runs ---'
gh run list --repo "$repo" --branch "codex/align-existing-acls" --limit 30 \
--json databaseId,name,workflowName,displayTitle,status,conclusion,event,headSha \
--jq '.[] | select(.event == "pull_request") | {databaseId, name, workflowName, displayTitle, status, conclusion, headSha}'
printf '%s\n' '--- Failed check-run annotations ---'
gh api "repos/$repo/commits/$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq .headRefOid)/check-runs?per_page=100" \
--jq '.check_runs[] | select(.conclusion == "failure") | {name, details_url, output: {title: .output.title, summary: .output.summary, text: .output.text, annotations_count: .output.annotations_count}}'
printf '%s\n' '--- Policy-related repository configuration ---'
fd -HI '^(pr-policy|.*policy.*|.*pull.*request.*|workflows)$|^package\.json$' .github . 2>/dev/null | head -80Length of output: 4350 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Policy/template outlines ---'
ast-grep outline scripts/pr-policy.mjs --view expanded
ast-grep outline .github/workflows/pr-policy.yml --view expanded 2>/dev/null || true
printf '%s\n' '--- PR-policy workflow and validator ---'
cat -n .github/workflows/pr-policy.yml
sed -n '1,280p' scripts/pr-policy.mjs
printf '%s\n' '--- PR template ---'
cat -n .github/pull_request_template.md
printf '%s\n' '--- Current PR diff summary ---'
git diff --stat origin/main...HEAD
git diff --unified=35 origin/main...HEAD -- \
'supabase/migrations/20260719070000_align_existing_acls.sql' \
'supabase/migrations/20260720170000_add_documents_owner_updated_at_indexed_idx.sql' \
'src/app/api/documents/[id]/signed-url/route.ts' \
'src/lib/openai.ts' \
'src/lib/universal-search.ts' \
'src/lib/registry-corpus-links.ts'Length of output: 43138
## Summary
- Align existing Supabase table and function ACLs to service-role-only access, add an indexed-documents lookup index, and update the schema/drift manifest.
- Improve embedding throughput with bounded concurrent batches, preserve public API errors, and tighten registry-source link typing and rate-limit cleanup behavior.
- No new clinical content, source claims, patient-data workflow, or client-side service-role access is introduced.
## Verification
- Verification not run: local verification commands were not run in this environment; CI is running for the latest commit.
- UI verification not run: the source-actions type-only update does not intentionally change UI behavior; `npm run verify:ui` was not run locally.
- `Build` and `Unit coverage` completed successfully on the prior CI run; confirm the rerun for the latest commit before merge.
- `Semgrep`, `Gitleaks`, `Safety and config checks`, and `GitGuardian Security Checks` completed successfully on the prior CI run; confirm the rerun for the latest commit before merge.
- Migration replay is pending for the latest commit.
- `npm run eval:retrieval:quality` was not run locally; no ranking, scoring, selection, or chunking algorithm change is intended.
- `npm run eval:rag -- --limit 15` and `npm run eval:quality -- --rag-only` were not run locally; no answer-generation or synthesis-prompt change is intended.
- `npm run check:production-readiness` and `npm run check:deployment-readiness` were not run locally; production deployment is not authorized by this PR description.
## Risk and rollout
- Risk: Medium. The migration revokes legacy privileges from all existing public base tables and re-grants DML only to `service_role`; an incorrect assumption about an application role could deny expected database access. Concurrent embedding requests may also increase OpenAI rate-limit pressure if the configured limit is too high.
- Rollback: Revert the application changes and apply a follow-up migration that restores only the explicitly required grants after validating affected roles. If embedding rate limits occur, reduce `OPENAI_EMBEDDING_CONCURRENCY_LIMIT` or revert the concurrency change.
- Provider or production effects: None until an authorized deployment applies the Supabase migrations or changes the embedding concurrency environment setting.
## Clinical Governance Preflight
- [x] Source-backed claims still require linked source verification before clinical use
- [x] No patient-identifiable document workflow was introduced or expanded without explicit governance approval
- [x] Supabase target remains `Clinical KB Database` (`sjrfecxgysukkwxsowpy`)
- [x] Service-role keys and private document access remain server-only
- [x] Demo/synthetic content remains clearly separated from real clinical sources
- [x] Source metadata, review status, and outdated/unknown-source behavior remain conservative
- [x] Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed
## Notes
- The Supabase preview was skipped because the connected project has reached its concurrent preview-branch limit.
- Merge only after the latest CI run, migration replay, and any required local verification are complete.This resolves the specific PR policy blocker. It will not, on its own, fix an unrelated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/openai.ts (1)
368-372: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
as anywith a typed cast.ESLint flags
no-explicit-anyhere; the return type can be narrowed withas PublicApiErrorinstead of discarding type safety entirely.♻️ Proposed fix
if ( error instanceof PublicApiError || (error && typeof error === "object" && "name" in error && error.name === "PublicApiError") ) - return error as any; + return error as PublicApiError;🤖 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/openai.ts` around lines 368 - 372, In the PublicApiError handling branch, replace the `as any` cast with `as PublicApiError` so the return remains explicitly typed and satisfies the no-explicit-any lint rule.Source: Linters/SAST tools
🤖 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/lib/openai.ts`:
- Around line 604-609: Update the worker abort handling in the async worker flow
so an already-aborted options.signal sets firstError to the abort error before
exiting, causing the embedding operation to reject rather than return incomplete
output. Apply the same behavior to the corresponding abort check near the later
worker path, while preserving normal task processing and existing error
propagation.
In `@supabase/migrations/20260719070000_align_existing_acls.sql`:
- Around line 14-15: Update the ACL migration’s table loop around the
REVOKE/GRANT statements to preserve authenticated base privileges for
policy-backed tables such as documents, document_chunks, and document_pages.
Restrict the revoke to service-role-only tables, or explicitly regrant the
required authenticated privileges on affected tables while keeping service_role
access unchanged.
---
Nitpick comments:
In `@src/lib/openai.ts`:
- Around line 368-372: In the PublicApiError handling branch, replace the `as
any` cast with `as PublicApiError` so the return remains explicitly typed and
satisfies the no-explicit-any lint rule.
🪄 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: 81188afe-6b24-492f-b12f-47247d4f3a85
📒 Files selected for processing (14)
src/app/api/documents/[id]/signed-url/route.tssrc/components/clinical-dashboard/source-actions.tsxsrc/lib/api-rate-limit.tssrc/lib/citations.tssrc/lib/env.tssrc/lib/openai.tssrc/lib/registry-corpus-links.tssrc/lib/universal-search.tssupabase/drift-manifest.jsonsupabase/migrations/20260719070000_align_existing_acls.sqlsupabase/migrations/20260720170000_add_documents_owner_updated_at_indexed_idx.sqlsupabase/schema.sqltests/registry-corpus.test.tstests/supabase-schema.test.ts
1 participant
Summary
Verification
npm run verify:pr-localcompleted successfully locally (passed all 328 test files / 2,977 unit tests, built the application tier successfully, and validated RAG fixtures).npm run check:production-readinesscompleted successfully locally with no blocking production-readiness failures.npm run verify:uiwas not run locally.BuildandUnit coveragecompleted successfully on the prior CI run; confirm the rerun for the latest commit before merge.Semgrep,Gitleaks,Safety and config checks, andGitGuardian Security Checkscompleted successfully on the prior CI run; confirm the rerun for the latest commit before merge.npm run eval:retrieval:qualitywas not run locally; no ranking, scoring, selection, or chunking algorithm change is intended.npm run eval:rag -- --limit 15andnpm run eval:quality -- --rag-onlywere not run locally; no answer-generation or synthesis-prompt change is intended.npm run check:deployment-readinesswas not run locally; production deployment is not authorized by this PR description.Risk and rollout
service_role; an incorrect assumption about an application role could deny expected database access. Concurrent embedding requests may also increase OpenAI rate-limit pressure if the configured limit is too high.OPENAI_EMBEDDING_CONCURRENCY_LIMITor revert the concurrency change.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Summary by CodeRabbit
Performance
Reliability
Security