Skip to content

fix(ingestion): give the enrichment repair function a caller, a reach, and monitoring - #2548

Closed
BigSimmo wants to merge 22 commits into
mainfrom
claude/enrichment-staging
Closed

fix(ingestion): give the enrichment repair function a caller, a reach, and monitoring#2548
BigSimmo wants to merge 22 commits into
mainfrom
claude/enrichment-staging

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Summary

#W98GR7 recorded four claims about the enrichment pipeline. Two hold, one holds by a different mechanism than recorded, and the central technical premise is refuted by the code on main. This PR fixes what is real and corrects the record on what is not.

Refuted, and why no ordering change is made

The issue says supabase/functions/indexing-v3-agent "deletes an artifact family before calling OpenAI and re-inserting, one family at a time, never staged-then-swapped", so a provider outage leaves the family permanently empty. On main it does the opposite. In all four writers — upsertMemoryCardsFromSections, upsertSectionIndexUnits, upsertVisualArtifacts, upsertCoreEmbeddingFields — the embeddingBatch await completes before sql.begin is entered, and the delete and re-insert share one Postgres transaction. A provider outage therefore throws before any delete happens, and a failed insert rolls the delete back. tests/indexing-v3-agent.test.ts did not actually pin that ordering — it pinned delete-before-insert inside the transaction, which is a weaker and different claim — so this PR adds the assertion that does. See "Three things review corrected" below.

The Edge Function is not modified here. The ledger row is corrected instead.

One caveat stated rather than buried: this checkout is shallow and the audit commits the issue cites are not reachable, so whether the file was fixed since the audit or the claim was inaccurate from the outset is not established. What is established is what the code does now.

Confirmed, and fixed

The repair function was invoked by nothing. Every repo-wide hit for repair_strict_enrichment_gate_batch is the migration, the schema mirror, generated types, drift bookkeeping, docs, or a schema-text assertion. It now has an operator caller.

And wiring up a caller alone would not have worked — this is the finding that came out of building it. repair_strict_enrichment_gate_batch touches documents.metadata, document_index_quality and ingestion_jobs, and never touches indexing_v3_agent_jobs, which is the table claim_indexing_v3_agent_jobs actually reads. It could not have unstuck a stuck document however often it ran. 20260902120000 adds that reset: completed where the gate passes, back to claimable with a fresh attempt budget where it does not, each repair stamping a counter so a document repaired again and again is visible rather than looping silently.

Terminal states are excluded from claim eligibility forever — by two different mechanisms, not the one recorded. claim_indexing_v3_agent_jobs excludes needs_enrichment_artifacts by name in its status not in (...) filter, while failed is excluded through attempt_count < max_attempts, because agentFailureDecision only writes failed once attempts are spent. A monitor that looked only at the status name would under-report.

No monitoring existed. needs_enrichment_artifacts appeared in no script and no workflow, so a stuck document reported as indexed with an empty artifact family and nothing counted it — silent corruption, not a crash. check:enrichment-health counts the three permanently-excluded states plus gate-failing documents. It reads indexing_v3_agent_jobs itself rather than the indexing_v3_agent_status mirror on documents.metadata: a divergence between recorded state and reality is the whole subject of this issue, so the check reads the table the claim RPC reads.

Design notes

The operator script follows scripts/cleanup-abandoned-reindex-generations.ts, the repo's established shape for a repair RPC that touches live clinical rows: dry run by default, mutating only under --apply plus a confirmation, health-probed first. It is deliberately not wired to a worker loop, a scheduled workflow, or an admin route. cli-utils.ts's confirm() returns false on a non-TTY, so --apply without --yes cannot mutate in a piped or detached context.

check:enrichment-health is provider-backed and is deliberately absent from verify:cheap, verify:pr-local and every CI job, so it cannot fire unattended.

Decision logic lives in src/lib/enrichment-repair.ts so it is unit-testable without a database; the scripts own only the provider I/O.

Three things review corrected, recorded because the first version of this branch shipped all three

The migration was built from the wrong baseline. schema.sql carries repair_strict_enrichment_gate_batch twice: the original 20260625033425 body, and the later 20260712171500 "codify live ahead" body which is the one that wins on replay and is what is actually deployed. They differ by more than whitespace — the original declares v_processing_lock_timeout and preserves a fresh-locked processing row's metadata; the deployed one does neither. Rebuilding from the superseded copy would have shipped that as an undeclared second behaviour change and moved a def_hash that 20260819110500 pins, for reasons unrelated to this issue. The migration is now a true minimal delta on the deployed body, and supabase/schema.sql carries a byte-identical copy — verified programmatically, not by eye.

The reset needed its own lease guard. gate_passed is a structural fact about which artifacts exist, and request_indexing_v3_enrichment re-queues a document without clearing them, so it stays true for the whole of a new run. Without a guard the reset would match a document mid-run and clear status, locked_by and locked_at underneath it — and request_ingestion_reindex_if_agent_idle, which decides "is the agent active?" from exactly those columns, would then approve a concurrent reindex over the same artifact tables. The deployed body has no lease-age guard anywhere, so the new CTE carries its own.

The refutation's premise was not actually pinned by a test. tests/indexing-v3-agent.test.ts asserted delete-before-insert inside the transaction — which would stay green if the embedding call were moved inside sql.begin, precisely the arrangement the issue describes. A new assertion pins the ordering the refutation rests on: embeddingBatch completes before sql.begin is entered, and no second embedding call appears inside the transaction.

Two further review findings applied: the dry run now reruns the function's own candidate predicate instead of counting gate-failing indexed documents (a different set in both directions — a gate-passing document with disagreeing recorded state is a candidate, and a gate-failing one already recorded as pending is not — so a preview reporting "0 failing" while apply repaired 50 was possible), and both the preview and the confirmation prompt now say what --apply actually costs. The script also refuses to apply against any project ref but sjrfecxgysukkwxsowpy.

What --apply actually does, stated plainly

For a gate-failing document the repair queues a pending ingestion_jobs row, and worker/main.ts ignores the incoming stage — so that is a full re-ingestion: download, extract or OCR, chunk, OpenAI embeddings, image captioning. Real provider spend, and a cross-border transfer of clinical document text. It takes the atomic reindex path (isAtomicReindexCandidate is status === "indexed", which every candidate is), so the old generation stays live until the new one commits and no document is unsearchable in between. That belongs in the operator's line of sight, and it is now in both the dry-run output and the confirmation prompt.

Verification

  • npm run typecheck — clean.
  • npm run lint — clean.
  • npm run check:migration-role — passed.
  • npm run check:function-grantsOK — all 36 SECURITY DEFINER public function(s) are revoked from PUBLIC ... and none are re-opened by a grant to PUBLIC/anon.
  • npm run docs:check-scripts1193 npm-run reference(s) resolve to real scripts.
  • npm run docs:check-indexall 64 repository roots/modules/routes and all schema tables are indexed.
  • npm run docs:update — inventory regenerated and committed (285 script files, 286 npm scripts).
  • tests/enrichment-repair.test.ts and tests/indexing-v3-agent.test.tsTest Files 2 passed (2) / Tests 32 passed (32).
  • Full offline unit suite — Test Files 1 failed | 947 passed (948). The single failure is tests/drift-detection.test.ts, the known drift-manifest staleness described below; nothing else is red.
  • npm run format — run, and the result is committed.
  • Reviewed before push by the ingestion-worker-reviewer and clinical-governance-reviewer subagents on this exact diff.

On the honesty of the Edge Function's test coverage, since this PR relies on it for the refutation: tests/indexing-v3-agent.test.ts is two different things. Real unit tests of the pure logic extracted into behavior.tsagentFailureDecision, deferralDecision, completionGateFromRow and siblings — which execute and assert real return values. And static source-text assertions against index.ts that slice the raw file and do string containment and ordering checks; they do not execute it and cannot, because it is Deno-runtime code that Vitest cannot load. There are no Deno tests anywhere under supabase/functions. So the ordering claim rests on reading the source plus a static pin, not on behavioural coverage, and it would be wrong to imply otherwise. The new assertion is the same kind — a stronger static pin, not behavioural proof.

Verification not run: npm run check:enrichment-health and npm run repair:enrichment-gate are the new scripts and both read or write the live clinical database — neither was run. npm run check:supabase-project, check:drift and check:migration-history are provider-backed and were not run. supabase migration up --local could not be run: the Supabase CLI is not installed here, and CI's Migration replay job is skipped on draft pull requests. No statement in 20260902120000 has been executed or parsed anywhere, because there is no Docker daemon and no Postgres in this environment.

tests/drift-detection.test.ts is red on this branch for the same reason as its sibling: supabase/schema.sql changed and supabase/drift-manifest.json cannot be regenerated without Docker. It was not hand-edited to make the check pass.

pr-policy classifies this diff clinicalRisk: true, operationalRisk: true, the latter because package.json gains two script entries. It emits the advisory note that operational-risk changes are bundled with clinical ones; splitting a two-line npm-script registration into its own PR would leave the scripts it registers unreachable, so they are kept together deliberately.

Risk and rollout

  • Risk: medium. The migration recreates one repair function with one added CTE; everything else is byte-identical to the deployed body (20260712171500's), verified programmatically. Applying it also changes the function's def_hash, which supabase/drift-manifest.json currently pins to the old value — regenerating the manifest is what keeps the post-merge live-drift run clean, and is step 2 below. The function is SECURITY INVOKER, granted to service_role only, and is invoked by nothing automatically — so applying the migration changes no behaviour on its own. The behaviour only changes when an operator runs the repair script deliberately.
  • Rollback: revert the commit and apply a follow-up migration restoring the prior function body. The two new scripts are additive and can simply stop being run.
  • Provider or production effects: merging applies 20260902120000 to the live clinical database within seconds — the Supabase GitHub integration has "Deploy to production" enabled with production branch main. Merge only inside a window the owner has approved. The two new scripts make provider calls only when an operator invokes them.
  • RAG impact: no retrieval behaviour change — ingestion repair tooling, monitoring, and a repair-RPC migration; no retrieval, ranking, selection or answer-generation surface is touched.

On whether merging deploys Edge Functions in this repository — the question asked before this work started. Migrations deploy automatically on merge; Edge Functions appear not to, but that is not provable from the repository. No workflow contains supabase functions deploy (all eleven Supabase-mentioning workflows checked); docs/db-maintenance.md and docs/disaster-recovery-runbook.md both give the function deploy as an explicit operator CLI command inside an approved change window; docs/process-hardening.md records a specific past operator-run function deploy by version number; and AGENTS.md documents the migration auto-deploy toggle in detail while saying nothing about functions — which is exactly where that fact would live. The residual ambiguity: the Supabase GitHub integration has a separate function-deploy toggle, and nothing committed proves its state. A dashboard read of that setting would settle it, the same check AGENTS.md records was done for the migration toggle on 2026-08-21. It does not gate this PR, since the Edge Function is not modified — but it must be confirmed before anyone assumes a merged function change is live.

Clinical Governance Preflight

  • 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

The reasoning behind each. Nothing here changes what is cited, how sources are rendered, or how an answer is verified; the diff is ingestion repair tooling. No document workflow is introduced — the repair returns an existing stuck document to the enrichment path it was already meant to take, and no new content is ingested. No Supabase environment value is edited, so check:supabase-project was deliberately not run. Both new scripts are server-only Node entry points using createAdminClient(); neither is reachable from a route or the browser, and errors go through safeErrorLogDetails. Demo mode is untouched. Item 6 is what the change is about: a document with an incomplete artifact family currently reports as fully indexed, and the monitoring makes that conservative rather than silent. On SaMD: no clinical decision-support behaviour changes — a repaired document is re-enriched by the existing pipeline through the existing gates, and nothing bypasses the strict enrichment gate.

Notes

Do not merge this PR. It is a draft and is labelled hold and do-not-merge. Auto-merge is not enabled and must not be.

Approval still needed, and from whom — Josh, the repository owner:

  1. An approved live-database window for 20260902120000. Merging is the apply step; there is no separate one. Merge only inside a window he has approved.
  2. npm run drift:manifest on a machine with Docker, committed, before this leaves draft.
  3. Separate approval before either new script is ever run against production. check:enrichment-health is a read; repair:enrichment-gate --apply writes to live clinical document rows and re-queues enrichment work that will make OpenAI calls.
  4. A dashboard read of the Supabase GitHub integration's Edge Function deploy setting, to close the ambiguity above. Not a blocker for this PR.

The cross-route race in docs/ingestion-state-machine.md R24d — deep-memory.ts deleting the same artifact families unscoped, racing the route that writes them — is a real defect on a different code path, and is very likely the defect this issue's ordering claim was reaching for. It is queued rather than folded in here.

After this lands, npm run issues:reconcile applies the two queued inbox records to the canonical ledger from its own serialized branch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N8JUuufwbFHy8PsgbztmGN


Generated by Claude Code


Note

High Risk
Changes a live clinical-database repair function (auto-deploys on merge) and adds operator paths that can trigger full re-ingestion with OpenAI spend; safety relies on dry-run defaults, lease guards, and explicit apply confirmation.

Overview
Addresses #W98GR7 by making the previously uncalled repair_strict_enrichment_gate_batch operable and able to unstick documents that were permanently excluded from claim_indexing_v3_agent_jobs.

Migration 20260902120000 extends the repair RPC with a reset_agent_jobs CTE (returns stuck indexing_v3_agent_jobs to claimable or completed state) and adds 45-minute lease / fresh-pending guards on completed_open_jobs and deferred_open_jobs so repair does not disturb in-flight or just-queued re-ingestions. supabase/schema.sql and drift-manifest.json are updated to match.

Operator tooling: npm run repair:enrichment-gate (dry-run by default, --apply with confirmation, project-ref guard) invokes the RPC with a full-corpus preview via selectStrictGateRepairCandidates. npm run check:enrichment-health counts terminal/stuck agent states and gate-failing indexed docs (provider-backed, not in CI). Shared decision logic lives in src/lib/enrichment-repair.ts with tests/enrichment-repair.test.ts.

Tests/docs/ledger: indexing-v3-agent.test.ts adds a static pin that OpenAI embeddingBatch runs before the delete/insert transaction (refuting the original ordering claim without changing the Edge Function). Branch-review and outstanding-issues inbox entries record the clinical-governance review and queue the separate deep-memory.ts race as P2.

Reviewed by Cursor Bugbot for commit 3b751e5. Configure here.

…, and monitoring

#W98GR7 recorded four claims. Two hold, one holds by a different mechanism than
recorded, and the central technical premise is refuted by the code on main.

REFUTED: "deletes an artifact family BEFORE calling OpenAI and re-inserting,
never staged-then-swapped". In all four writers in
supabase/functions/indexing-v3-agent/index.ts — upsertMemoryCardsFromSections,
upsertSectionIndexUnits, upsertVisualArtifacts, upsertCoreEmbeddingFields — the
embeddingBatch await completes before sql.begin is entered, and the delete and
re-insert share one Postgres transaction. A provider outage aborts before any
delete happens, and a failed insert rolls the delete back. So no ordering change
is made here; the ledger row is corrected instead.

The existing test did NOT pin that, and the first version of this change claimed
it did. tests/indexing-v3-agent.test.ts asserted delete-before-insert INSIDE the
transaction, which would stay green if the embedding call moved inside
sql.begin — precisely the arrangement the issue describes. A new assertion pins
the ordering the refutation actually rests on, including that no second
embeddingBatch call appears inside the transaction.

CONFIRMED, and fixed: repair_strict_enrichment_gate_batch is invoked by nothing.
It now has an operator caller — dry run by default, mutating only under --apply
plus a confirmation, health-probed first, following
scripts/cleanup-abandoned-reindex-generations.ts. Deliberately not wired to a
worker loop, a scheduled workflow, or an admin route.

CONFIRMED with the mechanism corrected: both terminal states are excluded from
claim eligibility forever, but by two different routes.
claim_indexing_v3_agent_jobs excludes needs_enrichment_artifacts by name, while
'failed' is excluded through `attempt_count < max_attempts`.

NEW, and the reason wiring up a caller alone would not have worked: the repair
function touches documents.metadata, document_index_quality and ingestion_jobs
and never touches indexing_v3_agent_jobs — the table the claim RPC reads. It
could not have unstuck a stuck document however often it ran. 20260902120000
adds that reset.

Two things about that migration are worth stating, because the first version of
it got both wrong.

Its baseline is the body currently DEPLOYED — the one codified by
20260712171500 and mirrored at schema.sql's later copy — not the original
20260625033425 body that schema.sql still carries earlier in the file as a
superseded copy. Those differ by more than whitespace: the original declares
v_processing_lock_timeout and preserves a fresh-locked processing row's
metadata, the deployed one does neither. Rebuilding from the wrong copy shipped
that as an undeclared second behaviour change and would have moved a def_hash
20260819110500 pins for reasons unrelated to this issue.

And the reset needs its own lease guard, because the deployed body has none.
gate_passed is a structural fact about which artifacts exist, and
request_indexing_v3_enrichment re-queues a document without clearing them, so it
stays true for the whole of a new run. Without the guard the reset would match a
document mid-run and clear status, locked_by and locked_at underneath it, and
request_ingestion_reindex_if_agent_idle — which decides "is the agent active?"
from exactly those columns — would then approve a concurrent reindex over the
same artifact tables.

CONFIRMED, and fixed: no monitoring. check:enrichment-health counts the three
permanently-excluded states and the gate-failing documents. It reads
indexing_v3_agent_jobs itself rather than the metadata mirror: a divergence
between recorded state and reality is the whole subject of this issue. It reads
the live database, so it is confirmation-gated and is in no CI job.

The dry run now reruns the function's own candidate predicate rather than
approximating it. Counting gate-failing indexed documents was a different set in
both directions — a gate-passing document with disagreeing recorded state IS a
candidate, a gate-failing one already recorded as pending is NOT — and a preview
that says "0 failing" while apply repairs 50 is worse than no preview. It also
now says what apply costs: each gate-failing document is queued for a full
re-ingestion with OpenAI embedding and caption calls, and the script refuses to
apply against any project ref but the expected one.

Decision logic lives in src/lib/enrichment-repair.ts so it is unit-testable
without a database; the scripts own only the provider I/O.

The cross-route race in docs/ingestion-state-machine.md R24d — deep-memory.ts
deleting the same artifact families unscoped — is a real defect on a different
code path, and is queued rather than folded in here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N8JUuufwbFHy8PsgbztmGN
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 08698d9d-3638-4ed7-beaf-251c79afe13e


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@supabase

supabase Bot commented Sep 2, 2026

Copy link
Copy Markdown

Updates to Preview Branch (claude/enrichment-staging) ↗︎

Deployments Status Updated
Database Fri, 04 Sep 2026 12:01:38 UTC
Services Fri, 04 Sep 2026 12:01:38 UTC
APIs Fri, 04 Sep 2026 12:01:38 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 Fri, 04 Sep 2026 12:01:40 UTC
Migrations Fri, 04 Sep 2026 12:01:42 UTC
Seeding Fri, 04 Sep 2026 12:01:43 UTC
Edge Functions Fri, 04 Sep 2026 12:01:45 UTC

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

@BigSimmo
BigSimmo marked this pull request as ready for review September 2, 2026 05:59
@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_39f48354-1a82-4972-895a-e318bf438bfc)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 769cd00e1d

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

Comment thread scripts/repair-strict-enrichment-gate.ts Outdated
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-04T10:52:22.000486Z 6317345 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Resolves the one conflict, in the generated data/repo-awareness-snapshot.json,
by regenerating it with npm run snapshot:repo-awareness rather than by hand.
check:repo-awareness-snapshot confirms it is in step (204 pages, 575 documents,
2664 reviews).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N8JUuufwbFHy8PsgbztmGN
Both found in review on PR #2548.

repair_strict_enrichment_gate_batch cleared ingestion_jobs lease columns with no
lease-age guard on completed_open_jobs or deferred_open_jobs. During an atomic
reindex the old artifact generation stays live, so gate_passed can be true while
a job is actively processing; marking it completed and clearing locked_at/locked_by
let the worker keep mutating artifacts after losing ownership. The new
reset_agent_jobs CTE already carried a 45-minute guard -- the same predicate now
covers all three CTEs that write a lock column. The function had no caller before
this branch, which is why the hazard was latent; adding one is what makes it
reachable, so the fix belongs here.

The operator script's dry run read the 500 oldest indexed documents and filtered
locally, while apply calls the RPC, which filters the whole corpus and only then
limits. A corpus whose oldest page was healthy previewed as zero candidates and
then queued real re-ingestions -- OpenAI spend against live clinical documents --
that the operator never saw. The preview now pages the view and stops once the
limit is met. The predicate deliberately stays in selectStrictGateRepairCandidates
rather than moving into PostgREST filters: neq drops NULLs while the TS predicate
reads NULL as not-completed, and that difference undercounts.

Tests pin both: the paging contract and the single-predicate rule as source
assertions, plus a fixture whose only candidate sits past the old 500-row window.
Verified red against the pre-fix script.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N8JUuufwbFHy8PsgbztmGN
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 3 failed job(s):

  • Unit coveragenot baselined: this job did NOT run on the main comparison below (path-scoped skip), so that run says nothing about it either way. Treat the comparison as absent, not green, and inspect the failing step.
  • Migration replaynot baselined: this job did NOT run on the main comparison below (path-scoped skip), so that run says nothing about it either way. Treat the comparison as absent, not green, and inspect the failing step.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #15508 (success). That run's conclusion is an aggregate and did not exercise Unit coverage, Migration replay.

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

…ging

# Conflicts:
#	data/outstanding-issues-snapshot.json
#	data/repo-awareness-snapshot.json
@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_cadfb40d-18e6-43d7-821c-e5099224ef81)

BigSimmo commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

Merge conflict resolved; one red check remains and needs a hand off this branch

The conflict is fixed. This branch went mergeable_state: dirty twice while main advanced, both times on generated files — data/repo-awareness-snapshot.json and data/outstanding-issues-snapshot.json. Resolved by regenerating them with npm run snapshot:repo-awareness and npm run snapshot:issues rather than editing by hand, and confirmed in step:

[repo-awareness] in step with data/repo-awareness-snapshot.json (204 pages, 576 documents, 2664 reviews)
[snapshot] in step with data/outstanding-issues-snapshot.json (70 open, 3 pending)

Head 5bb51908 merges cleanly and CI is running again. That recurrence is structural, not incidental — every PR regenerates those files — and is already tracked as #Y090R5.

What will still fail: tests/drift-detection.test.ts, because this PR changes supabase/schema.sql and supabase/drift-manifest.json is stale.

Why I have not fixed it: npm run drift:manifest replays the schema into a supabase/postgres Docker container. This session's container has the docker binary but no daemon (/var/run/docker.sock absent, confirmed), and no local Postgres. schema_sha256 must never be hand-edited — a hand-written hash turns the gate green over a stale snapshot, which is worse than the red.

The fix: take the drift-manifest artifact that the Migration replay job uploads on every run of this branch and commit it as supabase/drift-manifest.json, or run npm run drift:manifest on a machine with Docker. Pushes here use the guard's documented SKIP_DRIFT_GUARD=1 override in the meantime.

Not for merge. Labelled hold and do-not-merge. It carries a migration that reaches the live clinical database within seconds of merge, so it needs an approved database window; and the new repair script and health check read live Supabase, so each needs separate approval before it is ever run against production.


Generated by Claude Code

…ging

# Conflicts:
#	data/outstanding-issues-snapshot.json
#	data/repo-awareness-snapshot.json
#	docs/scripts-index.md
@BigSimmo
BigSimmo enabled auto-merge (squash) September 2, 2026 09:17
…ging

# Conflicts:
#	data/repo-awareness-snapshot.json
@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_f8a52a3c-d2e7-470a-9845-0f2824e30307)

@BigSimmo
BigSimmo disabled auto-merge September 2, 2026 10:18
…ging

# Conflicts:
#	data/repo-awareness-snapshot.json
#	docs/scripts-index.md
@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_f69d846d-615a-4536-b9d4-19241724f644)

…ging

# Conflicts:
#	data/repo-awareness-snapshot.json
#	docs/scripts-index.md
@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_4f088264-b497-4edc-8cea-7418065ff957)

…ging

# Conflicts:
#	data/repo-awareness-snapshot.json
#	docs/scripts-index.md
@cursor

cursor Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_ee79e040-01bf-400b-a944-01263d17a0c4)

…ging

# Conflicts:
#	data/repo-awareness-snapshot.json
#	docs/branch-review-index.md
#	docs/scripts-index.md
@cursor

cursor Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_4ca01c86-a572-4511-8132-b6883a253a55)

BigSimmo commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Synced this branch with main (2aa57e55b) to clear the merge conflict that was stopping CI from running at all. The conflict was in three generated files — data/repo-awareness-snapshot.json, docs/branch-review-index.md, docs/scripts-index.md — so it was resolved by taking main's versions and regenerating with npm run docs:update, never by hand. No source file in this PR was touched: the diff against main is still the same 14 files it was before.

Verified after the resolution: docs:check-inventory, docs:check-scripts, check:repo-awareness-snapshot, ledger:index:check and check:outstanding-issues-snapshot all pass, as do check:ledger-write-discipline and check:diff-integrity on the merge commit.

One thing is still outstanding, and it is this PR's own, not the merge's. The push was blocked by the drift guard:

✖ drift
  supabase/schema.sql changed but supabase/drift-manifest.json is stale (check:drift would fail).
  schema sha:   a84c02220dd56f3ba784185717dd3f6bb0350f878e74a3833b65dee103783095
  manifest sha: 1d0bc22c4e2fd9f2f45faa372c8dd958d55e785d17438920d436a295a9501875

Two of this branch's own commits (769cd00e1, 0152c8474) change supabase/schema.sql — 67 added lines, alongside the new ..._strict_enrichment_gate_unsticks_agent_jobs.sql migration — and supabase/drift-manifest.json is still identical to main, so it was never regenerated to match. That is very likely part of why PR required was already red before this sync.

The fix is npm run drift:manifest, committed alongside. It could not be run here — this cloud container has no usable Docker. So the sync was pushed with SKIP_DRIFT_GUARD=1, with the repository owner's explicit approval for that one push, precisely so CI can run and show the real state instead of nothing. The override applies to the push only and resolves nothing: check:drift will still fail until the manifest is regenerated on a machine with Docker.

Worth restating for whoever picks this up: this PR carries a migration, and in this repository merging a migration reaches the live clinical database within seconds. Merging is a deployment decision, not a code decision, and should not happen on a stale drift manifest.


Generated by Claude Code

@BigSimmo
BigSimmo enabled auto-merge (squash) September 4, 2026 09:37
@BigSimmo
BigSimmo marked this pull request as draft September 4, 2026 10:19
auto-merge was automatically disabled September 4, 2026 10:19

Pull request was converted to draft

# Conflicts:
#	data/repo-awareness-snapshot.json
#	docs/branch-review-index.md
@BigSimmo
BigSimmo marked this pull request as ready for review September 4, 2026 10:46
@cursor

cursor Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_770f2f84-0f40-4fc9-8ccc-681d6efdfcb6)

@BigSimmo
BigSimmo enabled auto-merge (squash) September 4, 2026 10:47

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6317345fcb

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

Comment thread scripts/repair-strict-enrichment-gate.ts Outdated
BigSimmo and others added 5 commits September 4, 2026 19:06
… limit

Two P1 findings from a second review round on this PR, verified against
the code before fixing.

completed_open_jobs and deferred_open_jobs matched any ingestion_jobs row
in ('pending', 'processing'), and the 45-minute lease-age exemption added
in the first review round only covers 'processing'. A 'pending' row is not
only a stuck one: an atomic reindex queues its new job while the OLD
generation's artifacts (and so gate_passed) are still live, so a
just-queued, entirely legitimate reindex sits in 'pending' for the instant
before a worker claims it. A repair run landing in that window marked the
job completed (or relabelled it deferred) and the worker never performed
it. Both CTEs now also exclude a 'pending' row whose created_at -- set
once at insert, never touched again -- is within the same 45-minute
window the lease guards already use. supabase/schema.sql's later
(deployed) copy of the function gets the identical change; the earlier
superseded 20260625033425 copy is deliberately left untouched, since it
exists only as a historical record of the pre-fix body.

repair-strict-enrichment-gate.ts accepted --limit=0 or a negative value as
finite, so the paging loop's own exit condition (candidates.length <
limit) was already true before its first iteration -- the internal clamp
inside selectStrictGateRepairCandidates never runs, because the loop that
calls it never runs. The preview reported zero candidates while the RPC's
own p_limit clamp still applied to one live, unpreviewed document. The
script now clamps the raw arg to the RPC's own 1..500 range before the
loop, so preview and apply always agree.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015uRvqm44emrLDMyPF7Pame
…ng-guard comments

Both safety gaps described for PR #2548 (unclamped --limit in
repair-strict-enrichment-gate.ts, and completed_open_jobs/deferred_open_jobs
matching a freshly-queued pending ingestion_jobs row) were already fixed on
this branch by commit 9a2792d, which landed both the --limit clamp and the
45-minute created_at guard on pending rows for both CTEs.

That commit updated the guard's SQL logic identically in the migration and
in supabase/schema.sql's mirror, but the six lines of explanatory comment it
added alongside the guard in the migration were not copied into the
schema.sql copy. The PR description states schema.sql carries a
byte-identical copy of the deployed function body "verified programmatically,
not by eye" -- comparing the two function bodies directly showed this was no
longer true after that commit: the executable predicate matched, but the
comment text did not, so the two were not actually byte-identical as claimed.

This adds the same two comment blocks to schema.sql's copy of
repair_strict_enrichment_gate_batch so the migration and its mirror are
byte-identical again, confirmed by extracting both CREATE OR REPLACE
FUNCTION ... $function$; bodies and diffing them.

No SQL logic changes in either file -- comment-only.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENQDEPFuwDZNoNssvV1PTc
@BigSimmo
BigSimmo marked this pull request as ready for review September 5, 2026 16:39
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@cursor

cursor Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_2f4be075-2a15-442f-be47-5cfb5944b773)

Copilot AI 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.

🟡 Changes recommended

The new reset_agent_jobs logic can still clobber freshly queued pending enrichment runs (missing a pending-freshness guard), creating a race/cancellation risk in production.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR makes the strict enrichment-gate repair path operable and effective by adding an operator caller + monitoring, and by extending repair_strict_enrichment_gate_batch to also reset stuck indexing_v3_agent_jobs rows (closing the gap where “repair” could never make a document claimable again).

Changes:

  • Add operator scripts for repair (repair:enrichment-gate) and monitoring (check:enrichment-health) plus shared pure logic in src/lib/enrichment-repair.ts.
  • Extend the Supabase repair RPC via migration 20260902120000 (and schema mirror + drift manifest) to reset stuck agent-job rows and add lease/freshness guards around job reconciliation.
  • Add tests to pin the Edge Function’s “OpenAI before transaction” ordering and to validate repair/monitoring logic and the new SQL guard shapes.
File summaries
File Description
tests/indexing-v3-agent.test.ts Adds a stronger static pin that embeddingBatch completes before sql.begin in all four writers.
tests/enrichment-repair.test.ts Adds unit tests covering repair summary/formatting, health verdicts, candidate selection, and SQL guard invariants.
supabase/schema.sql Mirrors the updated repair RPC body and adds reset_agent_jobs + new job guards.
supabase/migrations/20260902120000_repair_strict_enrichment_gate_unsticks_agent_jobs.sql Updates repair_strict_enrichment_gate_batch to reset stuck indexing_v3_agent_jobs rows and adds lease/freshness guards.
supabase/drift-manifest.json Updates schema hash and function def_hash pin for drift detection.
src/lib/enrichment-repair.ts Introduces pure shaping logic for repair and health-check output + preview candidate predicate.
scripts/repair-strict-enrichment-gate.ts Adds the operator-run repair caller (dry-run default; --apply + confirmation + project-ref guard).
scripts/check-enrichment-artifact-health.ts Adds a provider-backed monitoring script that counts stuck/at-risk enrichment states and gate failures.
package.json Exposes the new scripts as npm run repair:enrichment-gate and npm run check:enrichment-health.
docs/scripts-index.md Updates script counts after adding the new scripts.
docs/outstanding-issues-inbox/9f4217c2-b63d-43cc-a81e-41bb2579359b.json Queues a follow-up issue about deep-memory.ts unscoped deletes racing enrichment artifacts.
docs/outstanding-issues-inbox/29f57740-548d-4855-90cd-91e255609398.json Updates the #W98GR7 record with corrected findings and references.
docs/branch-review-records/ebd992696b935640a5c8e9462c1741a00a0c7497f04384ec810c415b29d80be2.record.md Records a prior clinical-governance review outcome for this line of work.
docs/branch-review-index.md Updates the index to include the new review record(s).
data/repo-awareness-snapshot.json Updates the snapshot with the new review record(s) metadata.
Review details
  • Files reviewed: 15/15 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +323 to +327
and not (
a.status = 'processing'
and a.locked_at is not null
and a.locked_at >= now() - make_interval(mins => 45)
)
Comment thread supabase/schema.sql
Comment on lines +7543 to +7547
and not (
a.status = 'processing'
and a.locked_at is not null
and a.locked_at >= now() - make_interval(mins => 45)
)

BigSimmo commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #2702, rebuilt as one fresh commit on current main with migration 20260907041700 and the missing pending-row freshness guard for reset_agent_jobs. This stale branch is now 253 commits behind current main; its migration number and concurrency behavior should not be merged. Closing this PR in favour of the held draft replacement.

@BigSimmo BigSimmo closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants