Skip to content

Codex/fix 48h review findings#574

Merged
BigSimmo merged 10 commits into
mainfrom
codex/fix-48h-review-findings
Jul 13, 2026
Merged

Codex/fix 48h review findings#574
BigSimmo merged 10 commits into
mainfrom
codex/fix-48h-review-findings

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

Verification

  • npm run verify:pr-local

During development, use npm run verify:cheap as the faster iteration gate before the final PR-local preflight.

  • npm run verify:ui when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed
  • npm run verify:release before release or handoff confidence claims

For retrieval, ranking, selection, chunking, source/citation rendering, or answer-contract changes, verify:pr-local runs eval:rag:offline automatically. Run the offline command directly during iteration before spending a live eval.

  • npm run eval:retrieval:quality (must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changed — CI cannot run it (needs live keys), so run it locally and paste the summary. A metadata/governance-weighting change once buried correct docs (recall 1.0→0.76) and only this eval caught it.
  • npm run eval:rag -- --limit 15 + npm run eval:quality -- --rag-only when answer generation, the synthesis prompt, or answer post-processing changed (grounded-supported must not drop; citation-failure 0)
  • npm run check:production-readiness when clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changed
  • npm run check:deployment-readiness when deployment startup, hosting, or rollout behavior changed

Clinical Governance Preflight

Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

Notes

BigSimmo and others added 8 commits July 13, 2026 14:16
…findings

# Conflicts:
#	docs/branch-review-ledger.md
#	src/lib/rag-extractive-answer.ts
#	tests/bounded-json-body.test.ts
#	tests/private-access-routes.test.ts
#	tests/source-backed-recovery-cross-reference.test.ts
Docstrings generation was requested by @BigSimmo.

* #550 (comment)

The following files were modified:

* `src/app/layout.tsx`
* `src/components/DocumentViewer.tsx`
* `src/lib/api-rate-limit.ts`
* `src/lib/document-viewer-navigation.ts`
* `src/lib/metadata-base.ts`
* `src/lib/public-api-access.ts`

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
@supabase

supabase Bot commented Jul 13, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@BigSimmo

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Improved metadata URL generation across configured sites and Railway deployments.
    • Added reliable document page navigation with normalized page numbers.
  • Bug Fixes
    • Anonymous document uploads now honor both per-caller and global rate limits.
    • Improved anonymous request identity handling behind trusted proxies.
  • Tests
    • Added coverage for metadata origins, document navigation, rate limits, proxy handling, and session-scoped UI storage.

Walkthrough

The changes add request-aware metadata origin resolution, centralize document page URL construction, extend anonymous upload quotas, revise proxy-based identity derivation, and align UI smoke tests with scoped session storage and updated controls.

Changes

Application behavior updates

Layer / File(s) Summary
Metadata origin resolution
src/lib/metadata-base.ts, src/app/layout.tsx, .env.example, tests/production-metadata-source.test.ts
Metadata bases resolve from configured URLs, trusted deployment domains, or development request headers, with production-oriented tests covering these sources and fallbacks.
Anonymous quotas and proxy identity
src/lib/api-rate-limit.ts, src/lib/public-api-access.ts, tests/api-rate-limit-fallback.test.ts, tests/public-api-access.test.ts
Anonymous answer and document_upload requests use bucket-specific global limits, while anonymous subject keys use the last forwarded proxy address.
Document page navigation
src/lib/document-viewer-navigation.ts, src/components/DocumentViewer.tsx, tests/document-viewer-navigation-source.test.ts
A shared helper creates normalized document page URLs, and DocumentViewer uses it for page navigation.
UI smoke test alignment
tests/ui-smoke.spec.ts, tests/ui-tools-collapse.spec.ts
Demo persistence checks use owner-scoped session storage, and the split-pane test targets the updated medication button.
Estimated code review effort: 3 (Moderate) ~25 minutes
🚥 Pre-merge checks | ✅ 8 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The template is present, but Summary, Notes, and the verification checklist are left blank or unchecked. Fill in the Summary, document relevant verification completed, and add concrete notes or follow-up items.
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 70.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is relevant but too vague to convey the main change in the PR. Use a concise, specific title that names the primary fix, such as metadata and request-identity hardening.
✅ Passed checks (8 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Generated And Sensitive Files ✅ Passed Only source/test updates and a documented .env.example template; no secrets, credentials, private keys, or build/generated artifacts were committed.
Verification Claims ✅ Passed The PR description lists verification checkboxes but makes no unsupported claims that tests/builds passed.
Risky Git Or Deployment Actions ✅ Passed No changed file or PR note recommends force-push, hard reset, destructive cleanup, branch deletion, or deployment without confirmation; only safe config/docs/tests updates.
Supabase Project And Schema Safety ✅ Passed Only env/docs/app/test code changed; .env.example still pins NEXT_PUBLIC_SUPABASE_URL and SUPABASE_PROJECT_REF to sjrfecxgysukkwxsowpy, with no schema/RLS/migration edits.
Runtime And Package Manager Integrity ✅ Passed No package-manager or runtime integrity changes: the PR diff touches only app/test files, with no edits to package.json, lockfiles, .npmrc, or Node/npm version settings.
Api Route Failure Handling ✅ Passed No changed API/server-action/provider path lacks deterministic error handling; the new helpers fail closed or return explicit errors.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-48h-review-findings
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch codex/fix-48h-review-findings
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch codex/fix-48h-review-findings

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously requested changes Jul 13, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/document-viewer-navigation-source.test.ts (1)

13-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider covering negative/decimal page inputs.

Current tests cover 0, NaN, and +Infinity, but not a negative finite value (e.g. -5) or a decimal (e.g. 2.7), which exercise the Math.max/Math.trunc branches differently.

🤖 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/document-viewer-navigation-source.test.ts` around lines 13 - 19, Add
cases to the “normalizes invalid page numbers to the first page” test for a
negative finite input such as -5 and a decimal input such as 2.7, asserting the
expected normalized URLs and covering the Math.max and Math.trunc behavior.
🤖 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/layout.tsx`:
- Around line 35-45: Update generateMetadata so headers() is only awaited when
NODE_ENV is not production, matching the allowRequestOrigin condition; pass the
resulting headers value to resolveMetadataBase only in that development path
while preserving static production rendering and existing metadata behavior.

---

Nitpick comments:
In `@tests/document-viewer-navigation-source.test.ts`:
- Around line 13-19: Add cases to the “normalizes invalid page numbers to the
first page” test for a negative finite input such as -5 and a decimal input such
as 2.7, asserting the expected normalized URLs and covering the Math.max and
Math.trunc behavior.
🪄 Autofix (Beta)

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b0abf075-782c-4b23-860e-c690e6cc6664

📥 Commits

Reviewing files that changed from the base of the PR and between 343531b and c8d341c.

📒 Files selected for processing (13)
  • .env.example
  • src/app/layout.tsx
  • src/components/DocumentViewer.tsx
  • src/lib/api-rate-limit.ts
  • src/lib/document-viewer-navigation.ts
  • src/lib/metadata-base.ts
  • src/lib/public-api-access.ts
  • tests/api-rate-limit-fallback.test.ts
  • tests/document-viewer-navigation-source.test.ts
  • tests/production-metadata-source.test.ts
  • tests/public-api-access.test.ts
  • tests/ui-smoke.spec.ts
  • tests/ui-tools-collapse.spec.ts

Comment thread src/app/layout.tsx

Copilot AI commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved in e155d44.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 1 file(s) based on 1 unresolved review comment.

Files modified:

  • src/app/layout.tsx

Commit: f9e0ca42c7b234a894b49bd2958e69ebb2d5810a

The changes have been pushed to the codex/fix-48h-review-findings branch.

Time taken: 12m 38s

Fixed 1 file(s) based on 1 unresolved review comment.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
@BigSimmo
BigSimmo merged commit 5444c49 into main Jul 13, 2026
14 of 15 checks passed
@BigSimmo
BigSimmo deleted the codex/fix-48h-review-findings branch July 13, 2026 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants