Repository navigation
Conversation
PHP enums were extracted (enum node plus a case_of edge per case) but no
USE of a case produced an edge: `Status::Active`, `Status::Active->label()`
and `match ($x) { Status::Draft => .. }` are class_constant_access_expression
nodes, and the only handling resolved the scope against labels of the same
file, so a cross-file access emitted nothing.
The extractor now parks each `Scope::MEMBER` access as a raw_calls entry
(php_const_ref, no callee, so every call pass skips it) and a new
php_class_constant_refs resolver binds it corpus-wide: a member that is a case
of the uniquely named scope enum gets a references_constant edge to the case
node (EXTRACTED, case-sensitive match); any other member links to the scope
type, as the in-file path already did. self/static resolve to the enclosing
type; parent, $var:: and unresolved/ambiguous scopes get no edge.
The static-call resolver also binds the implicit enum statics from/tryFrom/
cases(), which have no method node, to the enum (a user-defined method of the
same name still wins). AST cache schema 20 -> 21.
Because the build keeps one edge per undirected node pair, a method naming a
constant of its own class (`self::LIMIT`) would replace the class's `method`
edge for that method. `references_constant` is therefore added to the generic
relations that never displace a specific one.
|
Thanks for the pull request, @if12is. 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.
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.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; formal verification proved 0 of 2 changed function(s) (2 not verified).
Formal verification. PR-changed functions: 0/2 verified (0 proven, 0 may-equivalent, 0 distinguished) · 2 not verified (1 vacuous, 1 unsupported).
Not verified on this run: \_resolve\_php\_member\_calls (vacuous: never exercised), \_extract\_generic (unsupported).
Graphify review — findings
Adds corpus-level resolution of PHP Scope::MEMBER accesses (Status::Active, match arms, Status::Active->label()), which are now parked as php_const_ref raw calls and bound by a new _resolve_php_class_constant_refs pass. A member that is a case of a uniquely named in-corpus enum gets a references_constant edge to the case node, any other member links to the scope type, and self/static resolve to the enclosing type. parent, $var:: and unresolved or ambiguous scopes get no edge. Binds implicit enum statics from, tryFrom and cases() to the enum with a calls edge, since they have no method node. Treats references_constant as a generic relation in the edge-collapse logic, so a specific edge like a method's own class membership wins when both land on the same node pair. The AST cache schema goes from 20 to 21, so cached PHP results that lack the parked references are re-extracted.
No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 5235 functions depend on the 937 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 989 callers, 59 callees - worse:
build_from_json()— 269 callers, 22 callees - worse:
walk_calls()— 1 callers, 33 callees
Verification — 5235 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: 4687 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
437 of 437 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 387 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
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
· 3 more finding(s) on lines outside this diff (see the check run).
|
I tried this branch on a private Laravel codebase (~3.4k PHP files, AST pass only) and compared it with the current release (0.9.93). Same corpus, same settings: So no One gap remains, and I think it is out of scope here: accesses that sit in a class-level initializer, not inside a method body, still produce no edge. In our code, one permissions catalog class holds about 80 enum-case accesses in a Thanks for the PR. |
Branch:
upstream/php-enum-case-refs(base:upstream/v8at b90b9df). One commit.Related: #4022 (closed: PHP 8.1 enum cases were dropped; this PR builds on the
case_ofedges thatfix added) and #2615 (open, PHP consolidated). It does not close either.
Problem
PHP enums are extracted (an enum node plus a
case_ofedge per case), but no use of a case producedan edge.
Status::Active,Status::Active->label()andmatch ($x) { Status::Draft => ... }areclass_constant_access_expressionnodes, and the only handling resolved the scope against labelsdeclared in the same file. A cross-file access emitted nothing, so on a real application almost no
file-to-case relation existed. The implicit enum statics
Status::from(),tryFrom()andcases()have no method node either, so the static-call resolver had nothing to bind them to.
Implementation
graphify/extractors/engine.py: eachScope::MEMBERaccess is parked as araw_callsentry(
php_const_ref, with no callee, so every call pass skips it). The list is used because it is theone whose
caller_nidandsource_filethe id remapping and cache portability code alreadyrewrite.
graphify/extract.py: new resolverphp_class_constant_refsbinds them corpus-wide. A member thatis a case of the uniquely named scope enum gets a
references_constantedge to the case node(
EXTRACTED; the member match is case-sensitive). Any other member links to the scope type, as thein-file path already did.
self/staticresolve to the enclosing type;parent,$var::andunresolved or ambiguous scopes get no edge. The static-call resolver binds
from/tryFrom/cases()to the enum when the enum declares no method of that name (a user-defined one wins).graphify/build.py:references_constantis added to_GENERIC_RELATIONS. The graph keeps one edgeper undirected node pair, and
self::LIMITinside a method links that method to its own class, thesame pair as the class's
methodedge; without this the new edge replaces it.graphify/cache.py: AST cache schema 20 -> 21 (raw call output changes).ARCHITECTURE.md: paragraph on the rule.The scope class is found by its unqualified name (case-insensitive, as PHP class names are) and must
be unique in the corpus. This PR stands alone on
v8. If the PHP class-name resolution PR(
upstream/php-class-name-resolution) is merged first, the scope should go through that resolver so asame-named enum in another namespace is never picked; that is a small follow-up in the same function
and is not included here to keep the two changes independent.
Verification
New tests:
tests/test_php_enum_case_refs.py(8 tests: cross-file case access, case match iscase-sensitive and never guessed,
self::Caseinside the enum, plain class constant links to theclass, unresolved scope yields no edge,
from/tryFrom/cases, a user-definedfromwins, anda built-graph test that a method naming its own class's constant keeps its
methodedge) plusreferences_constantadded toGENERICintests/test_relation_collapse_precedence.py.Fails before, passes after (unmodified
graphify/ofupstream/v8, both test files):Full checks on this branch:
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.Before/after on two real Laravel applications (private copies,
uv run graphify update <copy> --forcefrom this branch and from bare
upstream/v8):references_constantedgescallsedges (addsEnum::from/tryFrom/cases)methodedges lost to areferences_constanton the same pairNo existing edge pair disappears in either application (the pair-level diff of the two graphs shows
only additions, plus the 8
methodedges restored in app A).Limitations
and get no edge (see the follow-up note above).
parent::Xand$var::Xare not resolved.Enum::cases()etc. are linked to the enum, not to a method node, because none exists.