Repository navigation
feat(extract): link Fortran modules and submodule procedures - #4450
oleksii-tumanov wants to merge 3 commits into
Conversation
Connect qualified module and submodule facts across files while preserving existing IDs. Refresh accepted Fortran facts during updates and remove structural links when parsing or retained source identities are incomplete. AI-Assistance: OpenAI Codex
Retire links for unsupported submodule interfaces and before mixed update writes. Persist verified empty corpora and preserve existing CLI error guards. AI-Assistance: OpenAI Codex
|
Thanks for the pull request, @oleksii-tumanov. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
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 5 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; formal verification proved 0 of 5 changed function(s) (4 not verified).
Formal verification. 1 change(s) alter behavior, breaking input(s) attached. PR-changed functions: 1/5 verified (0 proven, 0 may-equivalent, 1 distinguished) · 4 not verified (2 vacuous, 2 unsupported).
Behavior changes: extract changes behavior, here is the input that shows it.
The verifier found a concrete input on which extract behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"paths":"\['a'\]","cache\_root":"\_\_import\_\_\('pathlib'\)\.Path\('missing/deeper\.txt'\)"\}, the old code produced \{'nodes': \[\], 'edges': \[\], 'input\_tokens': 0, 'output\_tokens': 0, 'failed\_sources': \[\], 'extracted\_sources': \['a'\]\} but the new code produces \{'nodes': \[\], 'edges': \[\], 'input\_tokens': 0, 'output\_tokens': 0, 'failed\_sources': \[\], 'fortran\_identity\_incomplete\_sources': \[\], 'extracted\_sources': \['a'\]\} (outputs first differ at character 99). Paste that input straight into a regression test.
Not verified on this run: dispatch\_command (vacuous: never exercised), extract\_fortran (unsupported), \_rebuild\_code (unsupported), \_reconcile\_existing\_graph (vacuous: never exercised).
Graphify review — findings
Adds qualified Fortran module, submodule and procedure identity facts, with _resolve_fortran_structure linking a parent module or submodule to its child via contains and a procedure implementation to its declaration, but only when both sides are unique, freshly extracted and unambiguous. Every Fortran file always goes through extraction so links rest on complete evidence. If the facts can't be certified complete (for example, the prior graph.json is unreadable), all Fortran structure edges are dropped from the merged output and the built graph instead of kept stale; deleting the last Fortran sources now persists as a valid empty graph rather than failing as empty extraction. Bumps _AST_CACHE_SCHEMA to 16.
Worth a look
- Update recipe calls sys.executable but may never import sys —
graphify/skills/agents/references/update.md:39· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Strict metadata indexing on context nodes can crash extract() —
graphify/extract.py:9010· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Batch-wide gate silently disables Fortran structure edges —
graphify/extract.py:9298· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Incremental rebuild drops Fortran contains/implements edges when only one endpoint is re-extracted —
graphify/extract.py:9022· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Context graph now silently empty when the prior graph.json is unreadable —
graphify/cli.py:4260· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 5007 functions depend on the 2621 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 963 callers, 58 callees - worse:
_rebuild_code()— 185 callers, 61 callees - worse:
main()— 112 callers, 3 callees - worse:
dispatch_command()— 2 callers, 137 callees - worse:
extract_fortran()— 19 callers, 10 callees - worse:
_reconcile_existing_graph()— 1 callers, 14 callees - worse:
walk()— 1 callers, 10 callees - new:
test_generated_mixed_update_reconciles_fortran_before_step4()— 0 callers, 6 callees
Verification — 5007 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: 4953 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
418 of 418 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/pi.test.ts— full-run-safetytests/test_affected_cli.py— impact, full-run-safetytests/test_affected_defines.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_affected_overrides.py— impact, full-run-safetytests/test_agents_platform.py— impact, full-run-safetytests/test_alias_folding.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— impact, full-run-safetytests/test_astro_import_ids.py— impact, full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_benchmark_real_query.py— full-run-safetytests/test_blade_extractor.py— impact, full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_located_semantic_identity.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— impact, full-run-safetytests/test_c_include_resolution.py— impact, full-run-safetytests/test_cache.py— impact, full-run-safetytests/test_cache_stale_import_target.py— impact, full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— impact, full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— impact, full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cloud_cta.py— impact, full-run-safetytests/test_cluster.py— full-run-safetytests/test_cluster_ambiguous_scale.py— full-run-safetytests/test_cluster_exclude_hubs.py— full-run-safetytests/test_cobol_extractor.py— impact, full-run-safety- … and 368 more
non-code file(s) changed (
graphify/skills/agents/references/update.md,graphify/skills/amp/references/update.md,graphify/skills/claude/references/update.md,graphify/skills/claw/references/update.md,graphify/skills/codex/references/update.md…) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
graphify/skills/agents/references/update.md,graphify/skills/amp/references/update.md,graphify/skills/claude/references/update.md,graphify/skills/claw/references/update.md,graphify/skills/codex/references/update.md…) — a coverage gap or a missing link — running the full suite rather than only the selected tests
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.
Docs that may be stale (advisory)
.github/PULL_REQUEST_TEMPLATE.md§ Graphify-specific checklist (lines 33-39): references changed symbolsrunBENCHMARKS.md§ Reproducing (lines 190-203): references changed symbolsrunCHANGELOG.md§ 0.9.90 (2026-10-13) (lines 5-23): references changed symbolsrunCHANGELOG.md§ 0.9.83 (2026-10-09) (lines 145-159): references changed symbolsrebuild,walkCHANGELOG.md§ 0.9.80 (2026-10-07) (lines 185-204): references changed symbolsrunCHANGELOG.md§ 0.9.68 (2026-09-25) (lines 365-377): references changed symbolsrunCHANGELOG.md§ 0.9.59 (2026-09-12) (lines 469-476): references changed symbolsrunCHANGELOG.md§ 0.9.56 (2026-09-07) (lines 503-515): references changed symbolsexecuteCHANGELOG.md§ 0.9.55 (2026-09-05) (lines 516-530): references changed symbolsrebuildCHANGELOG.md§ 0.9.53 (2026-08-30) (lines 545-556): references changed symbolsrun
…and 10 more.
Formal verification
Behavior changes: extract changes behavior, here is the input that shows it.
The verifier found a concrete input on which extract behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"paths":"\['a'\]","cache\_root":"\_\_import\_\_\('pathlib'\)\.Path\('missing/deeper\.txt'\)"\}, the old code produced \{'nodes': \[\], 'edges': \[\], 'input\_tokens': 0, 'output\_tokens': 0, 'failed\_sources': \[\], 'extracted\_sources': \['a'\]\} but the new code produces \{'nodes': \[\], 'edges': \[\], 'input\_tokens': 0, 'output\_tokens': 0, 'failed\_sources': \[\], 'fortran\_identity\_incomplete\_sources': \[\], 'extracted\_sources': \['a'\]\} (outputs first differ at character 99). Paste that input straight into a regression test.
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, 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: not verifiable: all 40 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
Could not verify: Could not verify extract\_fortran.
The verifier did not have enough to check extract\_fortran, 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: not verified: a runtime divergence was observed, but the two versions' package snapshots loaded differently in the sandbox (old: {'fallback': {'graphify.detect': 'ModuleNotFoundError', 'graphify.extractors.engine': 'ModuleNotFoundError', 'graphify.extractors.resolution': 'ModuleNotFoundError'}, 'missing': ['graphify.extractors.models', 'graphify.google_workspace', 'graphify.rcfile']}, new: {'fallback': {'graphify.detect': 'ModuleNotFoundError', 'graphify.extractors.resolution': 'ModuleNotFoundError', 'graphify.security': 'ModuleNotFoundError'}, 'missing': ['graphify.paths', 'graphify.rcfile']}), so it may be an environment artifact rather than the edit
Could not verify: Could not verify \_rebuild\_code.
The verifier did not have enough to check \_rebuild\_code, 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: nondeterministic output (both versions disagreed with themselves on the witness) — not a behaviour change; a function whose output is not a function of its inputs (random / time / uuid / os.urandom, or state carried across calls) cannot be soundly distinguished by execution, so this tier abstains for the whole function
Could not verify: Could not verify \_reconcile\_existing\_graph.
The verifier did not have enough to check \_reconcile\_existing\_graph, 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: not verifiable: all 24 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
· 4 grounded finding(s) anchored inline below; 4 more finding(s) on lines outside this diff (see the check run).
| }) | ||
|
|
||
|
|
||
| def extract( |
There was a problem hiding this comment.
extract()
fans out to 58 callees (efferent coupling); 963 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| walk_calls(child, scope_nid) | ||
|
|
||
| def walk(node, scope_nid: str) -> None: | ||
| def walk(node, scope_nid: str, module_name: str | None = None, |
There was a problem hiding this comment.
walk()
fans out to 10 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| return not ((current - successful) | (persisted - successful - reconciled)) | ||
|
|
||
|
|
||
| def _reconcile_existing_graph( |
There was a problem hiding this comment.
_reconcile_existing_graph()
fans out to 14 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
|
||
|
|
||
| @pytest.mark.parametrize("third_source", ["malformed", "duplicate_declaration"]) | ||
| def test_generated_mixed_update_reconciles_fortran_before_step4( |
There was a problem hiding this comment.
test_generated_mixed_update_reconciles_fortran_before_step4()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Merge current v8 and advance the AST namespace to preserve both current extraction facts and Fortran completeness metadata. AI-Assistance: OpenAI Codex
What does this PR do?
Related to #1803.
When a Fortran module declares a procedure and a separate submodule implements it, the graph now connects the declaration to the implementation with an
implementsedge. It also connects modules to their submodules withcontainsedges, including nested submodules. The nodes stay separate so each keeps its file and line information.These links require unambiguous names and complete supported Fortran facts. If a parse is incomplete or a retained source cannot be checked, updates remove the structural links while keeping ordinary extracted nodes. CLI, watch and generated update instructions apply the same rule, including deletion of the last Fortran files and mixed code/document updates.
Type of change
Verification & Invariants
Cold and warm extraction, ambiguous declarations, partial parses, deletion and repeated updates have regression coverage. Existing node IDs and source locations are preserved. The AST cache schema changes, so the next refresh rebuilds AST entries across languages once; the semantic cache is unchanged. Later updates revisit accepted Fortran files through the AST cache.
This covers supported module/submodule procedure forms. It does not merge nodes, resolve general
useassociations, expand includes or interpret macros. Separate interfaces declared inside submodules are left unlinked until their scope can be represented safely.How was this tested?
uv run --frozen pytest tests/test_languages.py tests/test_extract.py tests/test_extract_cli.py tests/test_watch.py tests/test_cache.py tests/test_skillgen.py -q --tb=short: 1,243 passed.uv run --frozen pytest tests/ -q --tb=short: 8,416 passed, 15 skipped. One unchanged shell-tracing control fails with macOS Bash 3.2 and also fails on pristine current v8.uv run --frozen ruff check .: passed.uv run --frozen pyright --outputjson: 637 existing errors; no added diagnostics against current v8.uv buildanduv run --isolated --frozen pytest tests/test_wheel_packaging.py -q: passed.uv run --frozen graphify update .: passed.A current-extractor cache namespace simulation confirmed 12 cold misses after schema 17 to 18, followed by 12 warm hits and no reparses, identical cold/warm graphs and unchanged semantic cache. Three-run warm medians for 100, 500 and 1,000 Fortran files were 0.146, 0.703 and 1.254 seconds on the same host; the 1,000-file added cost remained below the planned two-second threshold. This is a local fixture measurement, not a historical-runtime or end-to-end CLI benchmark.
Tests ran with unrelated provider credentials removed and no live model calls.
Graphify-specific checklist
uv run python -m tools.skillgen --bless) when changing their source fragments.AI assistance is disclosed separately as
AI-Assistance: OpenAI Codexin the commits.