Repository navigation
Conversation
…:class PHP class references only became edges when a `use` imported them, and `Foo::class` mostly did not, because a class-constant access linked only to a label declared in the same file. Same-namespace classes (no `use` needed), leading-backslash FQCNs and `self`/`static` were not resolved either. Class-constant accesses and type hints now record the name as written (`_php_raw`) and `_resolve_php_type_references` resolves it once: `use` alias, same namespace, or absolute FQCN, to the one declaration with that FQN (EXTRACTED). Names imported from outside the corpus keep the shared FQN-labelled external stub; an unresolved `::class` is dropped, never guessed. `self`/`static` map to the enclosing class. Property defaults and top-level statements (routes, bootstrap) are now walked, attributed to the class and file node. Incremental builds resolve against unchanged files via the resolution context. AST cache schema 20 -> 21. Because the build keeps one edge per undirected node pair, the many new `references_constant` edges (a method naming its own class, a file that both imports and `::class`-references a class) would replace the `method` / `imports` edge on the same pair. `references_constant` is therefore added to the generic relations that never displace a specific one. Not covered: `::class` in method parameter defaults, qualified names in `new \A\B()`, multi-namespace files.
Pages\ListX::route() (a use-imported namespace prefix, or a namespace-relative name) bound by the scope's last segment: ambiguous when each module has its own Pages\ListX (no edge), and wrong when the name is a vendor class sharing a label with an in-corpus one. The scope now goes through _php_fqn_from_raw, as Foo::class does since the class-name resolution change this builds on, and the member-call resolver binds to exactly the class that name denotes (none when it is not in the corpus). Resolution-time change only (the annotation is added by the corpus pass, not by the per-file extractor), so the AST cache schema is not bumped beyond the parent change.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
![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 3 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; formal verification proved 0 of 5 changed function(s) (1 sampled, not proven, 4 not verified).
Formal verification. PR-changed functions: 1/5 verified (0 proven, 1 may-equivalent, 0 distinguished) · 4 not verified (3 vacuous, 1 unsupported).
Not verified on this run: \_resolve\_php\_member\_calls (vacuous: never exercised), \_extract\_generic (unsupported), \_php\_collect\_type\_refs (vacuous: never exercised), \_resolve\_php\_type\_references (vacuous: never exercised).
Graphify review — findings
Adds PHP name resolution for Foo::class, class constants, type hints and static-call scopes, binding each to the one declaration reached through a use alias, the same namespace, or a leading-\ FQCN instead of matching on the bare class name. The extractor records the name as written on each edge, and extract drops any ::class edge the resolver could not tie to a declaration or import, so if the resolver step fails those edges are removed rather than kept. Makes references_constant a generic relation in the pair-collapse so a specific method or imports edge survives, and bumps the AST cache schema to 21 to retire results without the written name.
Worth a look
- PHP resolver call passes 6 args; failure is swallowed and then drops every ::class edge —
graphify/extract.py:10137· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Unresolved
_php_rawedges keep dangling placeholder targets —graphify/extractors/resolution.py:4278· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Global namespace blocks are ignored when detecting multi-namespace PHP files —
graphify/extractors/resolution.py:3993· Escalate · medium- NOT verified (no proof, no reproducing execution) — 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 — 5504 functions depend on the 1115 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 992 callers, 59 callees - worse:
build_from_json()— 269 callers, 22 callees - worse:
_extract_generic()— 20 callers, 47 callees - worse:
walk()— 1 callers, 77 callees - worse:
walk_calls()— 1 callers, 33 callees - worse:
_resolve_php_type_references()— 2 callers, 9 callees
Verification — 5504 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: 4956 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
438 of 438 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— 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— full-run-safetytests/test_alias_folding.py— impact, full-run-safetytests/test_analyze.py— impact, 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— impact, full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— impact, full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_backend_opt_in.py— impact, full-run-safetytests/test_benchmark.py— impact, full-run-safetytests/test_benchmark_raw_graph.py— impact, full-run-safetytests/test_benchmark_real_query.py— impact, full-run-safetytests/test_blade_extractor.py— impact, full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_located_semantic_identity.py— impact, full-run-safetytests/test_build_merge_dedup_scope.py— impact, full-run-safetytests/test_build_merge_hyperedges_and_prune.py— impact, full-run-safetytests/test_build_merge_shrink_guard.py— impact, 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— impact, 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— impact, full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— impact, full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cloud_cta.py— full-run-safetytests/test_cluster.py— impact, full-run-safetytests/test_cluster_ambiguous_scale.py— full-run-safetytests/test_cluster_exclude_hubs.py— full-run-safety- … and 388 more
non-code file(s) changed (
ARCHITECTURE.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 (
ARCHITECTURE.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.
Formal verification
Could not verify: Could not verify \_resolve\_php\_member\_calls.
The verifier did not have enough to check \_resolve\_php\_member\_calls, 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: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
No difference found (not proven): No behavior difference found in extract (not a proof).
The verifier ran both versions of extract on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, 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 `config` is annotated `LanguageConfig` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_php\_collect\_type\_refs.
The verifier did not have enough to check \_php\_collect\_type\_refs, 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 234 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 \_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: no capturable inputs from the test suite; property tier: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
· 1 grounded finding(s) anchored inline below; 5 more finding(s) on lines outside this diff (see the check run).
There was a problem hiding this comment.
_resolve_php_type_references()
fans out to 9 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Branch:
upstream/php-static-call-scope. Stacked onupstream/php-class-name-resolution(the class-name resolution PR): it uses that PR's
_php_fqn_from_rawscope andusemaps. Open itafter that PR, or against that branch; its own diff is one commit and 3 files. Retarget it to
v8once the first PR is merged. Related: #2615 (open, PHP consolidated); it does not close it.
Problem
A PHP static call whose scope is qualified,
Pages\ListX::route(), bound by the scope's last segmentonly (
ListX). With ause App\Admin\Panels\XPanel\Pages;import, or a namespace-relative namesuch as
YPanel\Pages\ListX, that is wrong in two ways:Pages\ListX), the name isambiguous and no edge is made;
(
Vendor\Panels\Pages\Dashboard::route()vsApp\Dashboard), the call binds to the in-corpusclass, which is the wrong class.
Implementation
graphify/extractors/resolution.py(_resolve_php_type_references, the same pass that resolvesFoo::class): for each static-call raw call (is_member_callandphp_scoped) in a file the passhas namespace facts for, the scope goes through
_php_fqn_from_raw(use alias, same namespace,absolute FQCN) and the result, the node id of the declaration or
""for "no in-corpusdeclaration", is stored on the raw call as
php_scope_nid.self,static,parentand$varscopes are left alone.
graphify/extract.py(_resolve_php_member_calls): whenphp_scope_nidis present, the call bindsto exactly that type; an empty value means no edge. Without the annotation the old bare-name path
runs unchanged.
the per-file extractor.
ARCHITECTURE.md: one sentence added to the parent PR's paragraph.Verification
New tests:
tests/test_php_static_call_alias_scope.py(3 tests: an alias-qualified scope binds to thenamed class when two classes share the last segment; a namespace-relative qualified scope binds to
the class that name denotes; a scope in a vendor namespace gets no edge, even though an in-corpus
class has the same last segment).
Fails before, passes after. "Before" is this branch with the single commit's source changes reverted
(
git checkout HEAD~1 -- graphify, i.e. the parent PR only):Full checks on this branch (includes the parent PR):
Bare
upstream/v8(b90b9df) gives1 failed, 8469 passed, 15 skipped, ruff clean and the same641 errorsfrom pyright. The one failure is pre-existing and identical onv8:tests/test_skill_no_backtick_substitution.py::test_control_a_backticked_unknown_command_is_caught.Real-project evidence: none. On three private Laravel/Filament applications (erp, easm, dent) the
callsedge set built from this branch is identical to the one built from the parent PR, so I haveno measured before/after to report; the behaviour is established by the synthetic fixtures above,
which are shaped after the Filament
Pages\ListX::route()pattern.Limitations
usemaps).namespaceblocks are skipped by the resolver, as in the parent PR; the oldbare-name path applies to them.
Depends on #4626 (this branch is stacked on it; review that one first).