Repository navigation
Ingestion hardening: Retry route and heartbeat (part a) - #3707
Conversation
Audit 2026-10-20 ING-A-03: one failed heartbeat cancelled the whole attempt and discarded its paid work while the 45-minute lease still had most of its time left. The heartbeat writes nothing, so an uncertain or transiently rejected heartbeat is now tolerated until none has been confirmed for a third of the stale window. A lost lease, an invalid response or a confirmed non-transient rejection still revokes at once. Every other mutation still revokes on any uncertain outcome. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
…overy RPC Audit 2026-10-20 DB-01: Retry called retry_ingestion_job_if_idle, which skips the document authority lock, the managed-evidence hold and the sibling-job check. The route now reads the job's current row state and calls recover_ingestion_job, the same atomic path the operator recovery scripts use, guarded on that state. Each refusal becomes a plain 409 message the admin panel already shows. No database change: the older function stays in place, now unused by the app. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
Audit 2026-10-20 CH-04: without WORKER_ADMIN_SOURCE_RUNTIME_MODULE there is no scanner verdict, so every ingestion provider call is refused and each job fails as scanner_unavailable. The worker now logs one warning at start-up saying so (never the value), and the runbook's re-enable checklist names that warning. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
…up ledger Audit 2026-10-20 ING-A-05: delete_document_if_idle copies the title into storage_cleanup_jobs, and nothing cleared it, though the delete route itself treats titles as possibly identifying. The delete route and the storage janitor now clear it once the document row is gone. An aborted delete leaves the live document's title alone. The privacy impact assessment's retention table gains the row. Clearing rows that finished earlier needs a database change and is not part of this commit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
…by OCR Audit 2026-10-20 ING-B-07: OCR can list a value apart from its qualifier or misread a decimal, and nothing at answer time said a passage's document came from a scan. A new governance caution (ocr_text_source, warning severity, never a refusal) appears whenever a supporting source's index quality records OCR pages. No retrieval, ranking or extraction change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
…ising ones Audit 2026-10-20 RAG-A-02: table-fact grouping replaced a chunk's match only when a later fact scored higher, so lower-ranked facts were dropped and about one structured row survived per chunk. Every fact is now kept, deduplicated by id and capped at five, while the chunk keeps its best fact's score. Scores and ordering are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
|
Warning Review limit reachedYour organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Next included review available in 15 minutes. View limit details
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
…-read to the owner OCR_TEXT now sorts after other warnings of the same severity so it cannot push DOCUMENT_CAUTION or REVIEW_DUE out of the 8-item cap (Gitar). The Retry route's post-recovery re-read now joins documents!inner on the owner, and both queries are declared in the derived-query inventory (owner-scope phase 2). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
…meter (ING-B-09) PyMuPDF leaves a vertically merged label cell blank on later rows, so those facts were labelled with the table title. The last label now carries into clinical_parameter, with metadata.row_label_inherited_from recording where it came from. row_label, cells and chunk text keep the extracted value. Also formats the Retry route test mock. RAG impact: behaviour change for table facts of newly indexed documents only; before/after canary belongs around the re-index. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
…ng search data ING-A-02: reindex-health counts processing jobs whose lease expired on their final attempt directly, instead of relying on the 25-oldest preview, and the delete refusal no longer tells the admin to stop a worker that does not exist. EVAL-09: reindex-health also counts indexed documents with no chunks and chunks missing an embedding, and lists any non-zero count under integrityProblems. ING-A-04: the re-index runbook states the eval gate is advisory and by hand. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
document_chunks.embedding is NOT NULL, so the missing-embedding count was always 0 and gave false assurance. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
|
Static PR checks is red on Generated by Claude Code |
…ce tests Owner approved by decision card 21 Oct 2026. Not a reseal or clinical sign-off. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
|
Owner wording approval recorded: at 05:18 UTC on 21 Oct 2026, in the project thread, Josh (BigSimmo) said "yes" to both texts as written.
This covers the wording only. The hazard-control seal on Generated by Claude Code |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
BigSimmo has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f7b568347
ℹ️ 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".
Restores searchTableFactCandidates' inline grouping exactly as on main and drops tests/table-fact-grouping.test.ts. The grouping change moves to a separate draft PR so its canary pair can run on its own. worker/table-facts.ts is kept.
|
Auto-fix isn't included in your plan ↗ |
…t tolerance by the lease Owner-approved split (21 Oct 2026, 14:42 AWST). The OCR caution, scanner start-up warning, table-fact label carry-forward and deleted-document title changes move to their own PR off main; restored here to main's versions by a normal commit. startIngestionHeartbeat now takes the job lease and refuses to start unless the tolerance is positive and strictly shorter than it, so an attempt always stands down before another worker can reclaim the job. Tests cover the guard and revocation inside the lease. Source-contract tests are updated to the fail-closed shape of runIngestionMutation (it still revokes by default; only the heartbeat opts out).
Code Review ✅ Approved 2 closed / 2 findings🟡 Medium risk · Heartbeat failures are tolerated until a bounded timeout, which could delay worker cancellation during outages; admin retries also now use the recovery RPC. Ingestion hardening for the worker heartbeat, admin Retry route, OCR source cautions, table-fact row labels and reindex-health counts. Two issues were resolved: the OCR caution could push document-caution warnings out of the 8-item cap, and the chunks_missing_embedding count could never be non-zero because the column is NOT NULL. No open findings remain. ✅ 2 closed✅ Edge Case: OCR caution can push document-caution warnings out of the 8-item cap
✅ Quality: chunks_missing_embedding can never be non-zero: column is NOT NULL
VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Approved For more verification details, comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
BigSimmo
changed the title
Keep every matching table row per chunk (RAG-A-02, split from #3707)
…cts-titles OCR caution, scanner warning, table-fact labels and deleted-document titles (split from #3707)
Before: a single missed worker check-in threw away a whole job's paid work. The admin Retry button used the older, less careful recovery step. A job stranded on its final attempt could hide behind old failed jobs in the health probe.
After:
startIngestionHeartbeattakes the job lease and refuses to start unless the tolerance is positive and strictly shorter than it (owner decision, 21 Oct 14:42), so an attempt always stands down before another worker can reclaim the job. Tests cover this.recover_ingestion_jobpath. Both of its job reads joindocuments!inneron the owner and are declared in the derived-query inventory. Refusals are clear 404/409 responses.reindex-healthcounts jobs stranded on their final attempt directly. The delete refusal no longer tells the admin to stop a worker that does not exist.reindex-healthcounts indexed documents with no chunks and chunks with no embedding, and lists any non-zero count underintegrityProblems.No
supabase/**change.RAG impact: no behaviour change. Retrieval, ranking and answers are untouched; this is worker lease handling, the Retry route and ops reporting.
Clinical Governance Preflight: no clinical content, cautions, thresholds or refusal logic in this PR (the OCR caution moved to #3715).
Evidence (local, offline): focused Vitest passes for
worker-ingestion-attempt,worker-shadow-extraction,private-access-routes,worker-admin-source-runtime,ingestion,source-governance,table-factsandtable-fact-ranking. Typecheck is clean, and the owner-scope check passes both phases. The newreindex-healthcounts have no unit test, because the script reads the live database. Hosted CI has not yet been observed.Left out:
🤖 Generated with Claude Code
https://claude.ai/code/session_011qMFt8SZBmLuAmkH6ko6nd
Split (owner-approved, 21 Oct 2026): this PR is now part (a), the Retry route plus heartbeat (with the stranded-job and integrity reporting). Part (b), the OCR caution, scanner warning, table-fact label carry-forward (ING-B-09) and deleted-document titles, moved to #3715. The table-row grouping (RAG-A-02) moved to #3711.