fix(php): keep aliased class-import identity in multi-namespace files - #3469
fix(php): keep aliased class-import identity in multi-namespace files#3469DivyamTalwar wants to merge 6 commits into
Conversation
Problem PHP grouped function and const imports acquire class-style graph identities. Their imports edges can be redirected to sourceless FQNs or unrelated classes, and same-named class and symbol imports can collapse into one wrong target. Root cause The PHP import boundary discarded effective import kind before emitting edges. Tree-sitter stores homogeneous group kinds on namespace_use_declaration and homogeneous comma-list kinds only on the first clause, while mixed groups keep kind on each symbol clause. The PHP namespace resolver also classified clauses without this inherited kind. Once class and symbol edges shared a bare ID, the PHP FQN path or generic unique-stub rewire treated both as class references. Approach Derive effective kind from the clause, its declaration, or the first governed clause when the internal PHP extractor creates each edge. Mark function/const edges before deduplication, propagate the same kind when building the class-use map, and skip only marked targets during generic stub rewiring. Public extract_php strips the marker immediately; internal aggregate dispatch retains it through cache and resolution, then removes it before returning the graph. This preserves same-line class/symbol imports, mixed groups, multi-namespace files, and legitimate class relations while retaining symbol targets. Rejected alternatives reconstructed kind from line/target pairs, disabled class FQN resolution, or deleted bad stubs after other relations were corrupted. Verification INITIAL RED: FF [100%] FAILED tests/test_php_type_resolution.py::test_php_grouped_symbol_aliases_are_not_class_imports[function] FAILED tests/test_php_type_resolution.py::test_php_grouped_symbol_aliases_are_not_class_imports[const] 2 failed in 0.14s SHARED-STUB RED: FFFF [100%] 4 failed, 1 warning in 0.15s PRODUCER-IDENTITY RED: FFF [100%] FAILED tests/test_php_type_resolution.py::test_php_symbol_import_does_not_hide_same_named_class_import FAILED tests/test_php_type_resolution.py::test_php_mixed_group_preserves_same_named_class_and_function_imports FAILED tests/test_php_type_resolution.py::test_php_symbol_import_survives_multi_namespace_resolution_skip 3 failed, 1 warning in 0.35s PUBLIC-EXTRACTOR RED: F [100%] FAILED tests/test_php_type_resolution.py::test_php_single_file_extractor_hides_symbol_import_marker 1 failed, 1 warning in 0.14s GREEN: ................... [100%] 19 passed, 1 warning in 0.13s PHP LANGUAGE CONTROLS: ................... [100%] 19 passed, 388 deselected, 1 warning in 0.11s FINAL TEST-ONLY MUTATION: FFFFFFF [100%] FAILED tests/test_php_type_resolution.py::test_php_grouped_symbol_aliases_are_not_class_imports[function] FAILED tests/test_php_type_resolution.py::test_php_grouped_symbol_aliases_are_not_class_imports[const] FAILED tests/test_php_type_resolution.py::test_php_grouped_symbol_import_does_not_share_class_rewire[function] FAILED tests/test_php_type_resolution.py::test_php_grouped_symbol_import_does_not_share_class_rewire[const] FAILED tests/test_php_type_resolution.py::test_php_symbol_import_does_not_hide_same_named_class_import FAILED tests/test_php_type_resolution.py::test_php_mixed_group_preserves_same_named_class_and_function_imports FAILED tests/test_php_type_resolution.py::test_php_symbol_import_survives_multi_namespace_resolution_skip 7 failed in 0.59s mutation_test_exit=1 The repository gate is run after this commit so its record can certify the final commit SHA; the run artifacts carry the gate result and record path. Impact Grouped, homogeneous comma-separated, aliased, same-line, mixed, and multi-namespace function/const imports retain symbol targets and no longer influence class-style resolution. No dependency, public API, or output schema changes. Risk / rollback Internal AST/cache edges temporarily carry a private marker; public direct and aggregate extraction paths are covered for cleanup. Roll back with git revert HEAD while this commit is the branch tip. Closes none
Problem
When a PHP file declares more than one namespace, the resolver deliberately
skips file-level namespace resolution. An aliased class import loses its
imported identity at that point and keeps a bare provisional endpoint, which
generic same-label rewiring is then free to redirect to an unrelated class
that happens to share the alias.
Approach
Three regressions at the resolver seam, each failing for a distinct reason:
- keeps_imported_identity: a class import whose local alias collides with an
unrelated internal class in a two-namespace file. The edge must land on the
imported definition, so both an incomplete endpoint and a wrong-class
endpoint fail visibly.
- identity_is_per_edge: two imports sharing an imported basename must resolve
independently. Any file-wide map keyed by name or by target id collapses
them and fails here.
- does_not_retarget_symbol_import: a control. A declaration-level function
import must not be redirected through class-import identity, so a fix for
the above cannot widen into the grouped-symbol path.
Verification
All three fail on this commit; the nineteen existing PHP resolution tests pass.
.venv/bin/python -m pytest tests/test_php_type_resolution.py -q
3 failed, 19 passed
Observed: the aliased import at L3 targets bare "foo" rather than the sourced
Vendor\Foo definition, while the function import at L4 is already correct.
Problem
An aliased PHP class import in a file with more than one namespace resolves to
a bare provisional endpoint instead of the class it imports. Generic same-label
rewiring can then redirect that endpoint to an unrelated internal class that
shares the alias, so the graph records an import edge pointing at the wrong
definition.
Root cause
Class imports carried a single bare endpoint. The imported fully-qualified name
and the local alias existed only in a temporary file-level use map, and the
multi-namespace bailout discards that map before edges are processed. Once it
is gone there is nothing left that distinguishes the imported class from any
other symbol with the same tail.
Approach
Move identity ownership to the producer. Each class import edge now carries its
own imported fully-qualified name in metadata, including any group-use prefix,
recorded at extraction time where the syntax is still in hand. Resolution reads
only that per-edge value, and does so before the file-level namespace gate, so
the multi-namespace bailout no longer destroys it.
Two properties follow from putting identity on the edge rather than in a shared
map. Repeated aliases with the same imported basename resolve independently,
because nothing correlates them. Declaration-level function and const imports
are classified at extraction and carry no class provenance, so they are not
redirected through this path.
An earlier draft used a file-wide map keyed by imported tail and local alias.
It was discarded: it could not distinguish repeated aliases across namespace
blocks, and it could redirect a same-named function or const import to a class.
Full namespace-block resolution for inheritance and references remains out of
scope. Ambiguous identities are left unresolved rather than guessed.
Verification
.venv/bin/python -m pytest tests/test_php_type_resolution.py -q
22 passed
Reverting either source file independently returns the same three failures,
so both halves are load-bearing:
reverted graphify/extractors/resolution.py -> 3 failed, 19 passed
reverted graphify/extract.py -> 3 failed, 19 passed
ruff and py_compile pass on all three changed files.
Impact
No dependency change and no public output schema change. Import edges for PHP
class imports gain a target_fqn metadata key; existing consumers that ignore
unknown metadata keys are unaffected.
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 3 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Distinguishes PHP use function/use const symbol imports from class imports so they no longer collapse onto same-named classes: _import_php tags symbol-import edges with _php_symbol_import (detected via _php_import_kind) and stamps a target_fqn on unresolved class imports (via _php_import_fqn), while _resolve_php_type_references repoints class imports by their recorded FQN and skips symbol imports, and _record_use_clause treats grouped/declaration-level function/const kinds as non-class uses. Routes .php extraction and shebang dispatch through _extract_php_with_symbol_markers to preserve the marker, with extract_php and _rewire_unique_stub_nodes stripping _php_symbol_import so symbol imports are exempt from stub target rewiring and the marker never leaks into output.
Worth a look
- _php_symbol_import marker consumed in _rewire before resolution can read it —
graphify/extractors/resolution.py:3364· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _php_symbol_import marker persists to output for edges not touched by rewire remap —
graphify/extract.py:2831· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _php_symbol_import marker leaks to output when resolution short-circuits on target_fqn —
graphify/extractors/resolution.py:3357· 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 — 2093 functions depend on the 394 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 571 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
_resolve_js_module_path()— 34 callers, 9 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - …and 46 more — each is listed as a finding
Verification — 2093 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: 1928 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
104 of 262 test file(s) selected (40%) 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_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_extract.py— impacttests/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_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_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— impacttests/test_inherited_field_receivers.py— impacttests/test_java_member_calls.py— impacttests/test_java_type_resolution.py— impacttests/test_js_callback_calls.py— impact- … and 54 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\_php.
The verifier did not have enough to check extract\_php, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_import\_php.
The verifier did not have enough to check \_import\_php, 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: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_rewire\_unique\_stub\_nodes.
The verifier did not have enough to check \_rewire\_unique\_stub\_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: not verifiable: the input domain has 81 values but only 9 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
Could not verify: Could not verify \_resolve\_php\_type\_references.
The verifier did not have enough to check \_resolve\_php\_type\_references, 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: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
· 54 more finding(s) on lines outside this diff (see the check run).
Problem
An independent adversarial review of this branch rejected it, and it was right.
`metadata.target_fqn` is a SHARED metadata key: C# `using` directives and other
language extractors stamp it too. `_resolve_php_type_references` is handed EVERY
edge in the graph, not only PHP ones, so the per-edge FQN lookup this branch added
was consuming other languages' values.
Measured. In any scan containing one namespaced PHP file, a Kotlin
`import external.lib.Widget` had its target rewritten from `widget` to
`external_lib_widget`, and a sourceless node labelled `external.lib.Widget` was
materialised that the base revision never produced.
Root cause
The only thing confining this function to PHP was the `ref_file not in ns_by_file`
gate. The new check has to run BEFORE that gate, because emptying `ns_by_file` is
exactly what the multi-namespace bailout does and that bailout is the bug being
fixed. Hoisting the check above the gate also hoisted it out of the language scoping,
and nothing else re-established it.
Approach
Ask for provenance explicitly rather than inheriting it from control flow.
`_is_php_source` mirrors the suffix set `extract.py` already selects PHP files on,
including its `.blade.php` exclusion, and the FQN is consumed only for an edge a PHP
file produced. The multi-namespace behaviour this branch exists to fix is untouched,
because that has never depended on the gate the check now sits above.
Verification
python -m pytest tests/test_php_type_resolution.py -q
23 passed
The review's own reproduction, before and after:
base kotlin target 'widget', no external.lib.Widget node
before this fix kotlin target 'external_lib_widget', node materialised
after this fix kotlin target 'widget', no external.lib.Widget node
Mutation: deleting the single `_is_php_source(ref_file)` clause turns
test_php_import_identity_does_not_touch_other_languages red and leaves the other 22
green, so the guard is load-bearing.
ruff and py_compile pass.
Impact
Restores base behaviour for every non-PHP import edge. PHP class-import identity is
unchanged. No dependency or public output schema change.
Claude-Session: https://claude.ai/code/session_01C8DVxdS8oqWSm8bBur74s9
Correction pushed: the PHP resolver was rewriting other languages' import edgesAn adversarial review of this branch rejected it, and it was right. The defect
The check has to run before the Measured, in a scan containing one namespaced PHP file plus
C# The fixProvenance is now required explicitly rather than inherited from control flow. VerificationDeleting the single Full suite on the branch head: 5499 passed, 13 skipped. Worth noting for reviewers: CI was green on the previous head and the pull request was marked mergeable throughout. A cross-language rebind that no existing test covers is invisible to the suite, which is why this needed a separate pass rather than a re-run. |
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.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Distinguishes PHP use function/use const symbol imports from class imports so they keep their own identity instead of being repointed or collapsed into class nodes: _import_php now stamps _php_symbol_import and, for plain class uses, records a target_fqn for later resolution, while _extract_php_with_symbol_markers preserves the marker through extraction (the public extract_php strips it) and _rewire_unique_stub_nodes skips remapping marked edges. Fixes a cross-language misresolution where _resolve_php_type_references treated the shared metadata.target_fqn key as PHP's, repointing non-PHP imports (e.g. a Kotlin import external.lib.Widget) whenever a namespaced PHP file was in the scan; the FQN repoint now runs only when _is_php_source confirms the edge came from a PHP file and before the multi-namespace bailout that empties ns_by_file.
Worth a look
- resolution.py reads _php_symbol_import but extract.py strips it before resolution in the extract_php path —
graphify/extractors/resolution.py:3383· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Accidental .venv symlink committed into repository —
.venv:1· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Committed absolute .venv symlink —
.venv:1· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _php_symbol_import marker never stripped from function/const import edges in namespaced files —
graphify/extractors/resolution.py:3384· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- C# import assertion is vacuous —
tests/test_php_type_resolution.py· 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 — 2098 functions depend on the 399 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 572 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
_resolve_js_module_path()— 34 callers, 9 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - …and 46 more — each is listed as a finding
Verification — 2098 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: 1933 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
262 of 262 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/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.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_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— impact, 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_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.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— full-run-safetytests/test_chunking.py— 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_cluster.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— impact, full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— impact, full-run-safetytests/test_cross_language_call_resolution.py— impact, full-run-safetytests/test_cross_repo_member_calls.py— impact, full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— impact, full-run-safetytests/test_csharp_enum_members.py— impact, full-run-safetytests/test_csharp_field_generic_args.py— impact, full-run-safetytests/test_csharp_generic_callsites.py— impact, full-run-safety- … and 212 more
non-code file(s) changed (
.venv) → running the full suite for safety (a code graph can't see config/fixture/data deps)
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\_php.
The verifier did not have enough to check extract\_php, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_import\_php.
The verifier did not have enough to check \_import\_php, 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: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_rewire\_unique\_stub\_nodes.
The verifier did not have enough to check \_rewire\_unique\_stub\_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: not verifiable: the input domain has 81 values but only 9 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
Could not verify: Could not verify \_resolve\_php\_type\_references.
The verifier did not have enough to check \_resolve\_php\_type\_references, 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: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
· 54 more finding(s) on lines outside this diff (see the check run).
Problem
The previous commit added a `.venv` symlink pointing at an absolute path on one
developer's laptop. Anyone else checking this branch out gets a dangling link, and
any tooling that resolves it either fails or silently reaches outside the checkout.
Root cause
`.gitignore` carries `.venv/` with a trailing slash, which matches directories only.
A symlink is stored as mode 120000, which git does not treat as a directory, so the
pattern never applied and `git add -A` took the link. The ignore rule is correct for
a real virtualenv directory; it simply cannot see this shape.
Approach
Remove the symlink from the index. `.gitignore` is left alone: the existing rule is
right for the case it was written for, and widening it is the repository owner's call
rather than something to slip into an unrelated fix.
Verification
git ls-tree -r HEAD --name-only | grep -x '.venv' (no output)
python -m pytest tests/test_php_type_resolution.py -q
23 passed
Impact
Removes a file that could never have worked on another machine. No source change.
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 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Distinguishes PHP function/const imports from class imports so symbol imports (including grouped and aliased use clauses) keep their own identity and are no longer collapsed onto class stubs during stub rewiring, tagging them with an internal _php_symbol_import marker that extract_php strips before returning. Confines metadata.target_fqn repointing to PHP-produced edges via _is_php_source, fixing a cross-language regression where a namespaced PHP file in the same scan caused Kotlin/C# imports carrying the shared target_fqn key to be repointed to invented external stubs. Adds _php_import_fqn to qualify grouped imports with their namespace prefix, and routes .php dispatch through _extract_php_with_symbol_markers so resolution sees the markers.
Worth a look
- _extract_php_with_symbol_markers exposed via _DISPATCH leaks internal _php_symbol_import key to public extract output —
graphify/extract.py:5777· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _php_symbol_import marker leaks into final edges when resolution repoints via target_fqn —
graphify/extractors/resolution.py:3373· 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 — 2098 functions depend on the 399 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 572 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
_resolve_js_module_path()— 34 callers, 9 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - …and 46 more — each is listed as a finding
Verification — 2098 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: 1933 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
104 of 262 test file(s) selected (40%) 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_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_extract.py— impacttests/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_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_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— impacttests/test_inherited_field_receivers.py— impacttests/test_java_member_calls.py— impacttests/test_java_type_resolution.py— impacttests/test_js_callback_calls.py— impact- … and 54 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\_php.
The verifier did not have enough to check extract\_php, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_import\_php.
The verifier did not have enough to check \_import\_php, 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: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_rewire\_unique\_stub\_nodes.
The verifier did not have enough to check \_rewire\_unique\_stub\_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: not verifiable: the input domain has 81 values but only 9 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
Could not verify: Could not verify \_resolve\_php\_type\_references.
The verifier did not have enough to check \_resolve\_php\_type\_references, 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: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
· 54 more finding(s) on lines outside this diff (see the check run).
Summary
An aliased PHP class import loses its imported identity when the file declares more than one namespace, and can then be rewired to an unrelated class that happens to share the alias.
The resolver deliberately skips file-level namespace resolution for multi-namespace files. Class imports carried only a bare provisional endpoint, while the imported fully-qualified name and the local alias lived in a temporary file-level use map that the multi-namespace bailout discards. Once that map is gone, nothing distinguishes the imported class from any other symbol with the same tail, so generic same-label rewiring is free to pick the wrong one.
This moves identity ownership to the producer. Each class import edge carries its own imported FQN in metadata, recorded at extraction where the syntax is still available, group-use prefixes included. Resolution reads only that per-edge value, and reads it before the file-level namespace gate.
Two properties follow from putting identity on the edge rather than in a shared map:
functionandconstimports are classified at extraction and carry no class provenance, so they are never redirected through this path.Depends on #3466
This branch is stacked on
harness/bf-phpalias/fix-a, the head of #3466. Both changes touch_import_phpand the same region of_resolve_php_type_references, and they are two arms of one condition: #3466 acts when the import kind isfunctionorconst, this change acts when the kind is neither. Opening them independently would have produced two patches that conflict on every hunk.The diff shown against
v8therefore includes #3466's two commits. Once #3466 merges, this reduces to its own two commits. Please merge #3466 first.What changed
graphify/extract.py_php_import_fqnhelper;_import_phpnow branches on import kindgraphify/extractors/resolution.pytests/test_php_type_resolution.pyTests
Three regressions, each failing for a distinct reason:
keeps_imported_identity— a class import whose alias collides with an unrelated internal class in a two-namespace file. Asserting the imported definition makes both an incomplete endpoint and a wrong-class endpoint fail visibly.identity_is_per_edge— two imports sharing an imported basename must resolve independently. Any file-wide map keyed by name or by target id collapses them.does_not_retarget_symbol_import— a control. A declaration-level function import must not be redirected through class-import identity, so this fix cannot widen into fix(php): give grouped function and const imports their own identity #3466's grouped-symbol path.How to verify
The first commit adds the tests alone and is red; the second is green.
Reverting either source file on its own returns the same three failures, so both halves are load-bearing.
Full suite on the branch head: 5499 passed, 13 skipped.
ruffandpy_compileclean on all three changed files.Risk and rollback
No dependency change and no public output schema change. PHP class-import edges gain a
target_fqnmetadata key; consumers that ignore unknown metadata keys are unaffected. Full namespace-block resolution for inheritance and references is deliberately out of scope, and ambiguous identities are left unresolved rather than guessed. Reverting the two source hunks restores the previous behaviour.