fix: harden Codex auto-resolve review guard

[BigSimmo]

Summary

• keep Codex auto-resolve dedupe scoped to the PR instead of head SHA
• skip review-thread replies and auto-resolve request comments to avoid self-trigger loops
• narrow AGENTS instructions to actionable Codex findings for the current PR head
• make pure-review ledger updates opt-in instead of automatic mutations
• add a CI/static guard to prevent reintroducing SHA-scoped dedupe or broad triggers

Verification

• npm run check:codex-autofix-workflow
• npx prettier --check .github/workflows/codex-autofix-review-comments.yml .github/workflows/ci.yml AGENTS.md docs/codex-review-protocol.md scripts/check-codex-autofix-workflow.mjs package.json
• git diff --check

Notes

• Preserved unrelated dirty local files in the primary worktree; the pushed commit was replayed through a clean detached worktree at the remote branch head.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds try/catch error handling for 403 responses when listing PR comments in a Codex autofix workflow, introduces a validation script and CI step guarding that workflow's structure, and updates AGENTS.md/docs to scope auto-resolve requests to actionable findings and restrict branch-review-ledger updates during pure reviews.

Changes

Codex autofix workflow hardening and guard

Layer / File(s)	Summary
403 error handling for comment listing .github/workflows/codex-autofix-review-comments.yml	Wraps issues.listComments pagination in try/catch; 403 responses log a warning and return early, other errors are re-thrown; minor blank-line formatting near comment-creation error handling.
Workflow guard script and CI wiring scripts/check-codex-autofix-workflow.mjs, package.json, .github/workflows/ci.yml	Adds a script validating forbidden trigger patterns and required concurrency/marker scoping and resolve-skip guard in the autofix workflow file; wires it as check:codex-autofix-workflow npm script and adds a "Codex auto-resolve workflow guard" CI verify step.
AGENTS.md auto-resolve scoping update AGENTS.md	Updates the automatic resolve trigger and primary PR command text to scope Codex resolve requests to actionable findings for the triggering PR/head instead of resolving all review comments.
Branch review ledger and protocol mutation rules docs/branch-review-ledger.md, docs/codex-review-protocol.md	Adds a new ledger document describing lookup procedure and record schema; updates mutation rules to prohibit ledger updates during pure review unless explicitly requested, instead suggesting the ledger row in the response.

Estimated code review effort: 2 (Simple) | ~12 minutes

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Verification Claims	❌ Error	FAIL: The PR description/summary use vague verification labels (“parse check: passed”) instead of explicit “Ran : passed” wording.	Reword every verification note to an exact command/check plus result, e.g. “Ran npm run X: passed” or “Not run: reason.”
Docstring Coverage	⚠️ Warning	Docstring coverage is 2.00% which is insufficient. The required threshold is 70.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (9 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	No committed secrets, tokens, keys, .env files, caches, or build artifacts were added; the PR only changes docs, workflow YAML, package.json, and one validation script.
Risky Git Or Deployment Actions	✅ Passed	No changed file recommends force-push, hard reset, destructive clean, branch deletion, or deployment without confirmation; the edits add guardrails and safe-only guidance.
Supabase Project And Schema Safety	✅ Passed	No Supabase project refs or schema/RLS/policy/migration changes were introduced; the diff only updates workflow/docs and a guard script.
Runtime And Package Manager Integrity	✅ Passed	PASS: PR only adds a guard script and CI check; package.json still pins npm@11.17.0/Node 24.x, .npmrc keeps engine-strict=true, and CI still uses npm ci + package-lock.
Api Route Failure Handling	✅ Passed	PASS: The workflow now wraps issues.listComments in a 403-aware try/catch, soft-skips on auth failure, and rethrows non-403 errors; createComment is handled similarly.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: hardening the Codex auto-resolve review guard.
Description check	✅ Passed	The description follows the repository template with Summary, Verification, and Notes, and includes relevant validation steps.

✨ 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/autoresolve-listcomments-403

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch codex/autoresolve-listcomments-403

---

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

[supabase]

Updates to Preview Branch (codex/autoresolve-listcomments-403) ↗︎

Deployments	Status	Updated
Database	✅	Thu, 09 Jul 2026 12:20:54 UTC
Services	✅	Thu, 09 Jul 2026 12:20:54 UTC
APIs	✅	Thu, 09 Jul 2026 12:20:54 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks	Status	Updated
Configurations	✅	Thu, 09 Jul 2026 12:20:56 UTC
Migrations	✅	Thu, 09 Jul 2026 12:20:58 UTC
Seeding	✅	Thu, 09 Jul 2026 12:20:59 UTC
Edge Functions	✅	Thu, 09 Jul 2026 12:21:00 UTC

---

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

[BigSimmo]

@copilot resolve the merge conflicts in this pull request

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (6)

src/lib/rag-retrieval-variants.ts (1)

169-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

New admission/community branch overlaps with the existing comparison branch (Lines 184-187).

Both blocks add "admission community patients"-style variants for overlapping query shapes (single-branch admission+community vs. comparison admission+discharge). The seen set dedupes, so this is not a correctness bug, but the near-duplicate logic adds maintenance surface.

🤖 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-retrieval-variants.ts` around lines 169 - 172, The
admission/community variant block in rag-retrieval-variants overlaps with the
existing admission/discharge comparison branch and duplicates similar addVariant
logic. Consolidate the shared “admission community patients”-style variant
generation into one helper or shared conditional path used by both branches, and
keep the unique query-shape checks in the surrounding logic so the variants are
added from a single source of truth.

.github/workflows/live-drift.yml (1)

46-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider bumping setup-node to the latest major.

actions/setup-node@v5 works, but v6.x is current (node24-based). Not urgent for a weekly/manual workflow.

🤖 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 @.github/workflows/live-drift.yml around lines 46 - 50, The workflow is still
pinned to an older major of actions/setup-node; update the setup-node usage in
the live-drift workflow to the current major release. Locate the existing
actions/setup-node reference and bump it to the latest supported major while
keeping the current node-version-file, cache, and cache-dependency-path settings
unchanged.

src/lib/registry-corpus.ts (2)

431-447: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

bestEffortReembedRegistryRecordAfterEdit reports zero counts even after partial writes.

When reembedRegistryRecordAfterEdit throws mid-batch (see the batching loop above), earlier batches may have already written documents/chunks, but the caught "failed" result here always reports documentCount: 0, chunkCount: 0, masking that partial writes occurred. Combined with the batching rollback gap, this makes it hard for downstream callers to detect and reconcile partial index state.

🤖 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/registry-corpus.ts` around lines 431 - 447,
bestEffortReembedRegistryRecordAfterEdit currently returns a failed result with
zero counts even when reembedRegistryRecordAfterEdit may have already written
some documents/chunks before throwing. Update the error path in
bestEffortReembedRegistryRecordAfterEdit to preserve/report the actual partial
documentCount and chunkCount from the attempted re-embed instead of hardcoding
zeros, using the existing reembedRegistryRecordAfterEdit flow and
RegistryCorpusEmbedResult shape so downstream callers can detect partial writes.

---

360-406: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Near-identical slug lookup logic duplicated three times.

reembedClinicalRegistryRecordBySlug, reembedMedicationRecordBySlug, and reembedDifferentialRecordBySlug repeat the same disabled-check → query-by-owner/slug(/kind) → maybeSingle() → not-found/error pattern. Consider extracting a shared helper (table name + optional kind filter + row caster) to reduce triplication and ease adding future corpus kinds.

🤖 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/registry-corpus.ts` around lines 360 - 406, The slug-based re-embed
flow is duplicated across reembedClinicalRegistryRecordBySlug,
reembedMedicationRecordBySlug, and reembedDifferentialRecordBySlug. Extract the
shared disabled-check, owner/slug lookup, optional kind filter, maybeSingle
handling, and not_found/error logic into a common helper, then have each
function pass its table name and row-specific embed/cast behavior to that
helper. Keep the existing public function names but remove the repeated query
boilerplate.

src/lib/rag.ts (2)

2622-2624: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

No direct test coverage found for the new admission/community gate.

Neither tests/rag-answer-fallback.test.ts nor tests/rag-routing.test.ts (the test files included in this cohort) exercise admission_community_requires_title_rescue or missing_admission_community_title_support. If coverage exists elsewhere (e.g. an existing rag.test.ts) this can be disregarded; otherwise consider adding a regression test for a query matching admission+community intent with/without matching title support.

Also applies to: 2666-2685, 2954-2963

🤖 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 2622 - 2624, Add regression coverage for the new
admission/community routing gate in rag.ts, especially the branch in the
fast-path decision that returns admission_community_requires_title_rescue and
the missing_admission_community_title_support path. Update an existing relevant
test suite such as rag-answer-fallback.test.ts, rag-routing.test.ts, or
rag.test.ts to exercise a query that matches admission+community intent both
with and without title support, and assert the routing reason and fast-path
behavior in the affected helpers like hasAdmissionCommunityLookupIntent and
hasAdmissionCommunityTitleSupport.

---

2666-2685: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Fallback title-match branch is narrower than the titleHit branch.

When match_explanation.titleHit/labelHit is set, support is granted via a general \badmission\b + \bcommunity\b check on the title (order-independent). When it isn't set, the fallback only matches three hardcoded literal phrases ("admission of community patient", "admission community pt", "admission to discharge for community"). A genuinely on-topic title with different word order or phrasing (e.g. "Community Patient Admission Pathway") would fail the fallback and incorrectly report missing title support, even though the same general regex check (used in the titleHit branch) would have matched it. This only degrades to the slower/structured path rather than causing a wrong answer, but the asymmetry looks unintentional.

♻️ Suggested simplification

 function hasAdmissionCommunityTitleSupport(results: SearchResult[]) {
   return results.slice(0, 5).some((result) => {
-    if (result.match_explanation?.titleHit || result.match_explanation?.labelHit) {
-      const title = normalizeDocumentAliasText(`${result.title} ${result.file_name}`);
-      return /\badmission\b/.test(title) && /\bcommunity\b/.test(title);
-    }
-    const title = normalizeDocumentAliasText(`${result.title} ${result.file_name}`);
-    return (
-      title.includes("admission of community patient") ||
-      title.includes("admission community pt") ||
-      title.includes("admission to discharge for community")
-    );
+    const title = normalizeDocumentAliasText(`${result.title} ${result.file_name}`);
+    return /\badmission\b/.test(title) && /\bcommunity\b/.test(title);
   });
 }

🤖 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 2666 - 2685, The fallback branch in
hasAdmissionCommunityTitleSupport is too narrow compared with the
titleHit/labelHit branch, so align both paths by using the same general
normalizeDocumentAliasText plus /\badmission\b/ and /\bcommunity\b/ title check
for all results instead of only matching a few hardcoded phrases. Keep the
existing result slicing and title/file_name concatenation, but remove the
asymmetric literal-only fallback so Community Patient Admission Pathway-style
titles are recognized consistently.

🤖 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 `@docs/branch-review-ledger.md`:
- Around line 21-23: Mark the sample row in the branch review ledger as clearly
placeholder data, since the current example in the table uses a short Reviewed
HEAD value that conflicts with the real lookup flow used by git rev-parse.
Update the example entry in the table to either use a full commit SHA format or
move the sample outside the table, and keep the intent obvious by tying it to
the branch-review example row rather than a real reviewed HEAD value.

In `@docs/codex-review-protocol.md`:
- Around line 21-26: The protocol text is self-contradictory because the
pure-review rules in the mutation section forbid file edits while the ledger
requirement in the same section mandates updating docs/branch-review-ledger.md
after a review. Update the Mutation Rules in docs/codex-review-protocol.md so
that pure-review mode clearly excludes ledger writes, and move the ledger update
requirement into a separate post-review step or explicitly scope it to
non-pure-review workflows; keep the wording aligned with the surrounding
review-mode policy and the docs/branch-review-ledger.md reference.

In `@scripts/embed-registry-records.ts`:
- Around line 123-161: `listEligibleOwnerCounts` is loading full tables from
`clinical_registry_records`, `medication_records`, and `differential_records`
with plain `.select()`, which can hit Supabase/PostgREST’s 1000-row limit and
undercount owners. Update the logic to paginate each query with `.range()` in a
loop or replace the row scans with a server-side aggregate/RPC, and keep the
counting/sorting behavior in `listEligibleOwnerCounts` and `ensureOwnerCount`
unchanged.

---

Nitpick comments:
In @.github/workflows/live-drift.yml:
- Around line 46-50: The workflow is still pinned to an older major of
actions/setup-node; update the setup-node usage in the live-drift workflow to
the current major release. Locate the existing actions/setup-node reference and
bump it to the latest supported major while keeping the current
node-version-file, cache, and cache-dependency-path settings unchanged.

In `@src/lib/rag-retrieval-variants.ts`:
- Around line 169-172: The admission/community variant block in
rag-retrieval-variants overlaps with the existing admission/discharge comparison
branch and duplicates similar addVariant logic. Consolidate the shared
“admission community patients”-style variant generation into one helper or
shared conditional path used by both branches, and keep the unique query-shape
checks in the surrounding logic so the variants are added from a single source
of truth.

In `@src/lib/rag.ts`:
- Around line 2622-2624: Add regression coverage for the new admission/community
routing gate in rag.ts, especially the branch in the fast-path decision that
returns admission_community_requires_title_rescue and the
missing_admission_community_title_support path. Update an existing relevant test
suite such as rag-answer-fallback.test.ts, rag-routing.test.ts, or rag.test.ts
to exercise a query that matches admission+community intent both with and
without title support, and assert the routing reason and fast-path behavior in
the affected helpers like hasAdmissionCommunityLookupIntent and
hasAdmissionCommunityTitleSupport.
- Around line 2666-2685: The fallback branch in
hasAdmissionCommunityTitleSupport is too narrow compared with the
titleHit/labelHit branch, so align both paths by using the same general
normalizeDocumentAliasText plus /\badmission\b/ and /\bcommunity\b/ title check
for all results instead of only matching a few hardcoded phrases. Keep the
existing result slicing and title/file_name concatenation, but remove the
asymmetric literal-only fallback so Community Patient Admission Pathway-style
titles are recognized consistently.

In `@src/lib/registry-corpus.ts`:
- Around line 431-447: bestEffortReembedRegistryRecordAfterEdit currently
returns a failed result with zero counts even when
reembedRegistryRecordAfterEdit may have already written some documents/chunks
before throwing. Update the error path in
bestEffortReembedRegistryRecordAfterEdit to preserve/report the actual partial
documentCount and chunkCount from the attempted re-embed instead of hardcoding
zeros, using the existing reembedRegistryRecordAfterEdit flow and
RegistryCorpusEmbedResult shape so downstream callers can detect partial writes.
- Around line 360-406: The slug-based re-embed flow is duplicated across
reembedClinicalRegistryRecordBySlug, reembedMedicationRecordBySlug, and
reembedDifferentialRecordBySlug. Extract the shared disabled-check, owner/slug
lookup, optional kind filter, maybeSingle handling, and not_found/error logic
into a common helper, then have each function pass its table name and
row-specific embed/cast behavior to that helper. Keep the existing public
function names but remove the repeated query boilerplate.

🪄 Autofix (Beta)

✅ Autofix completed

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3edbc2c9-98d7-4d89-955e-84fa0b5134e0

📥 Commits

Reviewing files that changed from the base of the PR and between 54723c9 and 11d567d.

📒 Files selected for processing (20)

• .github/workflows/codex-autofix-review-comments.yml
• .github/workflows/live-drift.yml
• docs/branch-review-ledger.md
• docs/codex-prompt-playbook.md
• docs/codex-review-protocol.md
• docs/rag-hybrid-findings-and-todo.md
• scripts/embed-registry-records.ts
• src/lib/clinical-search.ts
• src/lib/rag-extractive-answer.ts
• src/lib/rag-retrieval-variants.ts
• src/lib/rag-routing.ts
• src/lib/rag.ts
• src/lib/registry-corpus.ts
• supabase/drift-manifest.json
• supabase/schema.sql
• tests/clinical-search.test.ts
• tests/rag-answer-fallback.test.ts
• tests/rag-routing.test.ts
• tests/registry-corpus.test.ts
• tests/retrieval-query-variants.test.ts

[coderabbitai]

Note

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

Fixes Applied Successfully

Fixed 3 file(s) based on 3 unresolved review comments.

Files modified:

• docs/branch-review-ledger.md
• docs/codex-review-protocol.md
• scripts/embed-registry-records.ts

Commit: 73aab704169e6902481eb5ac5357a4f946849b22

The changes have been pushed to the codex/autoresolve-listcomments-403 branch.

Time taken: 5m 24s

[BigSimmo]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Resolved. Merge commit: f81bdc4

• docs/codex-review-protocol.md, scripts/embed-registry-records.ts, src/lib/rag-routing.ts, src/lib/registry-corpus.ts, supabase/drift-manifest.json — accepted origin/main
• docs/branch-review-ledger.md — kept branch's Prettier-formatted version

Typecheck and Prettier checks pass.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ecd4fd4a0a

ℹ️ 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".