镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Using fork in OSX now breaks - #577

Open
castelao wants to merge 1 commit into
mainfrom
fix/OSX_fork
Open

castelao wants to merge 1 commit into
mainfrom
fix/OSX_fork

Conversation

@castelao

Copy link
Copy Markdown
Member

Because docling uses PyTorch, which on OSX uses MPS, it silently crashes the subprocess resulting in like a no text found.

It is interesting that this is a new error. That didn't happen before. Since this is an OSX only change, it shouldn't affect our production runs.

Because docling uses PyTorch, which on OSX uses MPS, it silently crashes
the subprocess resulting in like a no text found.

It is interesting that this is a new error. That didn't happen before.
Since this is an OSX only change, it shouldn't affect our production runs.

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.

🟢 Approval recommended

The focused platform check addresses the macOS crash while preserving existing behavior elsewhere.

0 open findings

What changed in this PR

Updates Docling subprocess creation to avoid PyTorch/MPS crashes on macOS.

Changes:

  • Uses spawn on macOS or when fork is unavailable.
  • Retains fork on supported non-macOS platforms.
File Description
compass/​services/​cpu.py Selects a platform-safe multiprocessing start method for Docling.

🧠 Review effort: Balanced


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

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.55%. Comparing base (613ecd1) to head (317ca14).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
compass/services/cpu.py 60.00% 1 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (60.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #577      +/-   ##
==========================================
+ Coverage   66.72%   69.55%   +2.82%     
==========================================
  Files          78       80       +2     
  Lines        7751     8166     +415     
  Branches      788      862      +74     
==========================================
+ Hits         5172     5680     +508     
+ Misses       2404     2301     -103     
- Partials      175      185      +10     
Flag Coverage Δ
unittests 69.55% <60.00%> (+2.82%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ppinchuk ppinchuk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any way we can add a test for this?

Bonus points if we can show that a test fails in CI for this particular case

This branch was successfully deployed

2 active deployments
codecov — 317ca140 Deployed Oct 11, 2026 by castelao via Codecov #525
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fixed a known bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants