fix(extract): skip the process pool up front when spawn can't re-import __main__ (#3669) - #3754
abhay-codes07 wants to merge 1 commit into
Conversation
…rt __main__ (Graphify-Labs#3669) Under the spawn start method (the Windows default) each worker re-imports the parent's __main__ by its __file__ path. A stdin (`… | python -`), `python -c`, or REPL caller has no such file — the path is missing or `<stdin>` — so every worker dies during bootstrap and the pool raises BrokenProcessPool before any work is done. The shipped SKILL.md pipes a heredoc into the interpreter, so on Windows this is the path most users take, not an edge case: every skill-driven run prints one full worker traceback per core, then a fallback warning that blames a missing `if __name__ == "__main__":` guard — a fix the caller cannot apply, because there is no file to put a guard in. Detect the unusable-__main__ case before creating the pool: when the start method is spawn (or the platform is Windows) and __main__ has no importable file, run sequentially and print one note that names the real cause (stdin/-c/ REPL) and the remedy (write the step to a .py file, or pass parallel=False). A script that DOES have a __main__ file but omits the guard is a different failure this cannot catch here; it still surfaces via the existing BrokenProcessPool fallback, whose __main__-guard message is now accurate for that remaining case. Tests cover the detector (stdin, REPL-without-__file__, a real script, and the fork start method) and that extract() with >= _PARALLEL_THRESHOLD uncached files and an unusable __main__ never attempts the pool yet still produces correct output. All five fail without the guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds a preflight check that runs AST extraction sequentially when a spawn-based worker pool can't bootstrap. _spawn_cannot_reimport_main returns true under the spawn start method (or on Windows) whenever __main__.__file__ is missing or not a real file — the stdin/python -c/REPL case — and extract uses it to skip the pool entirely for parallel runs above _PARALLEL_THRESHOLD, printing a note pointing at writing the step to a .py file or passing parallel=False. A script with a real __main__ file but no if __name__ guard is not caught here and still falls back via BrokenProcessPool.
Worth a look
- macOS default spawn is missed when start method is unset —
graphify/extract.py:6623· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2329 functions depend on the 676 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 656 callers, 46 callees - new:
_rebuild_code()— 142 callers, 54 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 30 more — each is listed as a finding
Verification — 2329 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 2154 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
121 of 290 test file(s) selected (42%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_external_call_guards.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_type_resolution.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_elixir_import_resolution.py— impacttests/test_extract.py— impact, changed-testtests/test_extract_cache_location.py— impacttests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impacttests/test_forwarding_review_findings.py— impacttests/test_go_builtin_call_targets.py— impacttests/test_go_interface_methods.py— impacttests/test_go_qualified_resolution.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_imported_export_forwarding.py— impacttests/test_incremental.py— impacttests/test_indirect_call_arrow_single_param_shadow.py— impacttests/test_indirect_call_block_scoped_shadow.py— impacttests/test_indirect_call_catch_binding_shadow.py— impacttests/test_indirect_call_external_import_shadow.py— impacttests/test_indirect_call_for_of_binding_shadow.py— impacttests/test_indirect_call_function_expression_shadow.py— impacttests/test_indirect_call_nested_closure_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_inferred_confidence_rubric.py— impact- … and 71 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
· 38 more finding(s) on lines outside this diff (see the check run).
Fixes #3669.
What
Under the spawn start method (the Windows default; macOS since 3.8), each
ProcessPoolExecutorworker re-imports the parent's__main__by its__file__path. A stdin (… | python -),python -c, or REPL caller has no such file — the path is missing or<stdin>— so every worker dies during bootstrap and the pool raisesBrokenProcessPoolbefore any work is done.The shipped
SKILL.mdpipes a heredoc into the interpreter (@'…'@ | & (Get-Content …\.graphify_python) -), so on Windows this is the path most users take, not an edge case. Every skill-driven run:if __name__ == "__main__":guard — a fix the caller cannot apply, because there is no file to put a guard in.Extraction then completes correctly but sequentially — so the one path most Windows users take silently loses parallelism and looks like it crashed.
Fix
Detect the unusable-
__main__case before creating the pool. When the start method is spawn (or the platform is Windows) and__main__has no importable file, run sequentially and print a single note that names the real cause (stdin/-c/REPL) and the remedy (write the step to a.pyfile, or passparallel=False).A script that does have a
__main__file but omits the guard is a different failure this cannot catch here; it still surfaces through the existingBrokenProcessPoolfallback — whose__main__-guard message is now accurate for that remaining case, since the stdin/-c/REPL cases no longer reach it.Verification
Five tests, all of which fail without the guard:
_spawn_cannot_reimport_main()for a stdin caller (__file__ == "<stdin>"), a REPL (__file__absent), a real script (usable → not skipped), and the fork start method (usable → not skipped).extract()with ≥_PARALLEL_THRESHOLDuncached files and an unusable__main__never attempts the pool yet still produces correct nodes, with the explanatory note on stderr.The existing parallel/sequential/pool tests stay green; the one unrelated failure on this machine (
test_c_include_out_of_root_...) fails identically on cleanv8.Scope
This silences the false failure and keeps correctness. Restoring parallelism for the skill on Windows (by writing the step to a temp
.pyinstead of piping via stdin) is aSKILL.mdchange and out of scope here.