Skip to content

fix(pdf): treat extractor exit code 137 as SIGKILL - #1285

Merged
BigSimmo merged 15 commits into
mainfrom
cursor/pdf-extractor-sigkill-137-0687
Jul 28, 2026
Merged

fix(pdf): treat extractor exit code 137 as SIGKILL#1285
BigSimmo merged 15 commits into
mainfrom
cursor/pdf-extractor-sigkill-137-0687

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • Salvage from closedfix(audit): System Audit Remediation and Improvements #1262: treat Python PDF extractor exit code 137 as a SIGKILL-equivalent process failure even when Node reports no signal (common OOM path).
  • Adds a focused regression for sys.exit(137) alongside the existing SIGKILL self-kill case.
  • Process-failure regressions are ungated from PyMuPDF so they cannot silently skip outside CI.
  • No retrieval/ranking/clinical-output behavior change.

Verification

  • Hosted Unit coverage / PR required SUCCESS after process-failure test ungate
  • Verification not run: full npm run verify:pr-local not required for this scoped extractor error-path change; focused Vitest tests/pdf-extractor.test.ts process-failure suite passed locally (2/2; PyMuPDF table suite skipped in this VM)
  • UI verification not run: no UI changes

Risk and rollout

  • Risk: Low — maps an already-failed extractor exit to the existing PdfExtractorProcessError path
  • Rollback: Revert this commit
  • Provider or production effects: None

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

Notes

Open in WebOpen in Cursor

OOM kills often report exit 137 without a Node signal. Map that to the
same PdfExtractorProcessError path as an explicit kill, with a focused
regression test.
@supabase

supabaseBot commented Jul 27, 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 ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@cursor[bot], you've reached your PR review limit, so we couldn't start this review.

Next review available in:49 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8b78abe0-1785-4dd6-a752-e4ffed685648

📥 Commits

Reviewing files that changed from the base of the PR and between 268ca18 and 9a1796e.

📒 Files selected for processing (3)
  • docs/branch-review-ledger.md
  • src/lib/extractors/document.ts
  • tests/pdf-extractor.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/pdf-extractor-sigkill-137-0687

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.

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent please resolve all CI failing tests after understanding the issue. Please also fix any comments and resolve them and also ensure that you fix with smallest required fix with best coding practice and u derstabding the fix rather than simple patch. Please fix any merge conflicts and ensure any issues are found. Please resolve any issues you find. Please also run bugbot.

@cursor

cursorBot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

PR #1285 is clean again: required CI green, synced with main, no merge conflicts.

What I found

  • Prior tip already had all required CI green — nothing failing to “fix”
  • Branch was behind main (staleness only; merge-tree clean)
  • No unresolved review threads / no hosted cursor[bot] Bugbot comments
  • Bugbot (pr-bugbot) found no P0/P1; one P2 in the new test placement

What I changed

  • Merged origin/main cleanly
  • Moved SIGKILL / exit-137 regressions out of describe.runIf(hasPyMuPDF) so they run without PyMuPDF
  • Tightened assertions to PdfExtractorProcessError by name (so a plain Error + JS fallback can’t falsely pass)

Verification

  • Local: tests/pdf-extractor.test.ts → 2 passed / 4 skipped (PyMuPDF absent here)
  • Hosted on 882543c5: PR required SUCCESS, merge state CLEAN

Tip: 882543c5 on cursor/pdf-extractor-sigkill-137-0687. Still a draft — mark ready when you want to merge.

Open in WebOpen in Cursor

@BigSimmo
BigSimmo marked this pull request as ready for review July 28, 2026 01:38
@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.

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent please resolve all CI failing tests after understanding the issue. Please also fix any comments and resolve them and also ensure that you fix with smallest required fix with best coding practice and u derstabding the fix rather than simple patch. Please fix any merge conflicts and ensure any issues are found. Please resolve any issues you find. Please also run bugbot. Please mark comments resolved also once you resolve them.

@cursor

cursorBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

PR #1285 is green and merge-clean again.

Root cause of the CI failure

  • PR was marked ready without a ## Clinical Governance Preflight section
  • Touching src/lib/extractors/document.ts is classified as clinical-risk, so PR policy blocked

What I fixed

  • Added complete Clinical Governance Preflight (7/7 checked) to the PR body
  • Merged origin/main (GitHub DIRTY was staleness; merge-tree was clean)
  • Re-ran Bugbot: no P0/P1; prior P2 (PyMuPDF-gated process-failure tests) already fixed

Comments / threads

  • No unresolved review threads to resolve (CodeRabbit rate-limited; Codex usage-limited; no cursor[bot] findings)

Status now

  • Tip 0250b46a — mergeable CLEAN
  • PR policy / Unit coverage / Build / PR required all SUCCESS
Open in WebOpen in Cursor

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent please resolve all CI failing tests after understanding the issue. Please also fix any comments and resolve them and also ensure that you fix with smallest required fix with best coding practice and u derstabding the fix rather than simple patch. Please fix any merge conflicts and ensure any issues are found. Please resolve any issues you find. Please also run bugbot. Please mark comments resolved also once you resolve them.

@cursor

cursorBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

PR #1285 is in good shape again.

CI / conflicts

  • No failing required checks on tip 9a1796e4
  • GitHub DIRTY was only main staleness; merged origin/main (merge-tree clean)
  • Fixed a merge=union exact-duplicate ledger row that would have broken Static PR

Bugbot / comments

  • Re-ran Bugbot: no P0/P1; no remaining product P2
  • Review threads: 0 — nothing to mark resolved (CodeRabbit rate-limited; Codex usage-capped)

Status

  • Tip even with main (15 ahead / 0 behind)
  • PR policy / Static / Unit / Build / PR required: SUCCESS
  • Mergeable; UNSTABLE is from a nameless null check artifact, not a required failure
Open in WebOpen in Cursor

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent please resolve all CI failing tests after understanding the issue. Please also fix any comments and resolve them and also ensure that you fix with smallest required fix with best coding practice and u derstabding the fix rather than simple patch. Please fix any merge conflicts and ensure any issues are found. Please resolve any issues you find. Please also run bugbot. Please mark comments resolved also once you resolve them.

@cursor

cursorBot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

PR #1285 is already merged — nothing left to fix on this branch.

Landed

  • Merged at 2026-07-28T02:46:12Z as 8942b8bc on main
  • Fix is on main: exit 137PdfExtractorProcessError, process-failure tests ungated from PyMuPDF

CI

  • Required GitHub checks (PR policy / Static / Unit / Build / PR required): all SUCCESS
  • CircleCI “failure” is unrelated: “No configuration was found in your project” — orphaned CircleCI webhook with no .circleci config, not a product/test failure from this PR

Comments / Bugbot

  • Review threads: 0 — nothing to resolve
  • Bugbot: no P0/P1/product P2

No further commits or comment resolutions needed for #1285.

Open in WebOpen in Cursor

@BigSimmo
BigSimmo merged commit 8942b8b into mainJul 28, 2026
18 of 19 checks passed
@BigSimmo
BigSimmo deleted the cursor/pdf-extractor-sigkill-137-0687 branch July 28, 2026 02:46
Sign up for freeto 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

@BigSimmo@cursoragent