fix(extractors): share the in-flight scan root via base, not graphify.extract (#3666) - #3758
abhay-codes07 wants to merge 1 commit into
Conversation
….extract (Graphify-Labs#3666) `extractors/markdown.py` read the scan root of the extraction in flight with a function-local `import graphify.extract` + `getattr(_extract, "_XAML_ACTIVE_EXTRACT_ROOT", None)`. That violates MIGRATION.md invariant Graphify-Labs#4 ("never import from graphify.extract inside this package; direction is extract.py -> extractors/"), and the `getattr(..., None)` meant a rename of the module global would silently disable Markdown wikilink (Obsidian vault) resolution rather than fail loudly. Move the ambient scan root onto `extractors/base.py` — where the extractors legitimately live — as `set_active_scan_root()` / `active_scan_root()`. `_safe_extract_with_xaml_root` now publishes the root there (still saving and restoring for nested extraction), the two XAML readers and the Markdown vault lookup read it through the accessor, and `markdown.py` imports it in the correct direction. The `_XAML_ACTIVE_EXTRACT_ROOT` module global in extract.py is gone; nothing else referenced it. Behavior is unchanged — the same process-global root is set around each extraction and read by the same sites. Tests assert the extractor no longer imports graphify.extract (fails on the old code), that the set/get/restore contract nests correctly, and that markdown's accessor IS the base accessor. The markdown, XAML/dotnet, and language suites stay green; the only failures on this machine are the pre-existing v8 baseline set (c-include determinism, wikilink-unicode, two watch cwd-deleted cases), which fail identically on clean v8. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
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
Moves the in-flight scan root from a module global in graphify.extract to extractors.base, exposed via set_active_scan_root (returns the prior value so nested extraction can restore it) and active_scan_root, so the XAML C# project clamp and Markdown link resolution read the corpus boundary without importing back into the parent module. _safe_extract_with_xaml_root now publishes and restores the root through these accessors, and the Markdown extractor aliases _active_scan_root to the base function instead of doing a function-local import graphify.extract + getattr, which failed silently on a rename. Adds a test guarding that extractors/markdown.py never imports graphify.extract and that set/get/restore nests correctly.
Worth a look
- Global active scan root races across concurrent extractions —
graphify/extractors/base.py:18· 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 — 3288 functions depend on the 291 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 655 callers, 45 callees - new:
_rebuild_code()— 142 callers, 54 callees - new:
build_from_json()— 218 callers, 20 callees - new:
build_merge()— 76 callers, 14 callees - new:
_extract_generic()— 18 callers, 29 callees - new:
extract_bash()— 45 callers, 10 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - …and 108 more — each is listed as a finding
Verification — 3288 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: 2842 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
165 of 291 test file(s) selected (57%) via static blast radius.
tests/test_analyze.py— impacttests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_build.py— impacttests/test_build_merge_dedup_scope.py— impacttests/test_build_merge_hyperedges_and_prune.py— impacttests/test_build_merge_shrink_guard.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_carried_hyperedge_remap.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cli_export.py— impacttests/test_cluster.py— impacttests/test_confidence.py— impacttests/test_corrupt_graph_json.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_dart.py— impacttests/test_dedup.py— impacttests/test_dedup_remaps_hyperedges.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_evidence_binding.py— impacttests/test_export.py— impacttests/test_export_control_characters.py— impacttests/test_external_stub_endpoints.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extraction_spec_ids.py— impacttests/test_extractor_scan_root_context.py— impact, changed-testtests/test_falkordb_integration.py— impacttests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impact- … and 115 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 \_safe\_extract\_with\_xaml\_root.
The verifier did not have enough to check \_safe\_extract\_with\_xaml\_root, 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 `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_xaml\_csharp\_class\_nodes.
The verifier did not have enough to check \_xaml\_csharp\_class\_nodes, 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 `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_xaml\_project\_root.
The verifier did not have enough to check \_xaml\_project\_root, 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 `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 2 grounded finding(s) anchored inline below; 114 more finding(s) on lines outside this diff (see the check run).
| @@ -5879,7 +5882,7 @@ def _xaml_project_root(path: Path) -> Path: | |||
| def _xaml_csharp_class_nodes(path: Path) -> dict[str, list[dict]]: | |||
There was a problem hiding this comment.
_xaml_csharp_class_nodes()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| set_active_scan_root(previous_root) | ||
|
|
||
|
|
||
| def _extract_single_file(args: tuple) -> tuple[int, dict]: |
There was a problem hiding this comment.
_extract_single_file()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Fixes #3666 (part 1 — the concrete invariant violation).
What
extractors/markdown.pyread the scan root of the extraction in flight by reaching back into its parent module:Two problems:
graphify.extractinside this package. Import direction is strictly extract.py → extractors/." A deferred, function-local import is still a package→parent dependency.getattr(..., None)means a rename of the module global silently disables Markdown wikilink (Obsidian vault) resolution instead of failing loudly.Fix
Move the ambient scan root onto
extractors/base.py— where the extractors already live — asset_active_scan_root()/active_scan_root():_safe_extract_with_xaml_rootpublishes the root there (still saving/restoring the previous value, so nested extraction is unaffected)._xaml_project_root,_xaml_csharp_class_nodes) and the Markdown vault lookup read it through the accessor.markdown.pyimportsactive_scan_rootfrombase— the correct direction; a rename now fails at import, not silently._XAML_ACTIVE_EXTRACT_ROOTglobal inextract.pyis removed; nothing else referenced it.Behavior is unchanged: the same process-global root is set around each extraction and read by the same sites.
Verification
New tests (all fail on the unpatched code):
markdown.pyimportsgraphify.extractnowhere (AST scan for top-level and function-local imports).set_active_scan_rootreturns the previous value and nests/restores correctly._active_scan_rootisbase.active_scan_root.The markdown, dotnet/XAML, and language suites stay green (937 passed in the touched-area sweep); the only failures on this machine are the pre-existing
v8baseline set (c-include determinism, wikilink-unicode, twotest_watchcwd-deleted cases), which fail identically on cleanv8.Scope
This fixes the concrete invariant violation (part 1 of the issue). The issue's part 2 — the facade re-exporting ~166 private symbols, and the
resolutionvs*_resolutionnaming — is a broader architectural observation about the ongoingextract.py → extractors/split, deliberately left out of this focused fix.