Index JavaScript in inline <script> blocks of a PHP file (#2320) - #3627
ayushcodes10 wants to merge 6 commits into
Conversation
The PHP tree sitter grammar treats a span that is not PHP, such as an inline script block, as opaque markup, so any JavaScript declared there was entirely absent from the graph: functions, calls, and everything else in that region. extract_php now runs a second pass after the PHP extraction. Every part of the source that is not a script body is blanked to spaces, preserving newlines so line numbers stay accurate, and the masked source is parsed with the JS grammar the same way the Vue extractor already masks everything outside its own script block. The two node sets are merged onto the one file node, so a symbol declared in the script block looks like any other symbol in the file. A script tag with only a src attribute and no body masks to an empty region and contributes nothing, and a file with no script tag at all skips the second pass entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers the issue's exact reproduction (two functions plus the call edge between them), that line numbers survive the masking pass, that a script tag with only a src attribute contributes nothing without crashing, and that a plain PHP file with no script tag is unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![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 4 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Indexes JavaScript declared inside inline <script> blocks of .php files, which tree-sitter-php otherwise treats as opaque markup and skips entirely. extract_php runs a second JS parse over _php_mask_to_script_blocks, which blanks everything outside script bodies while preserving line numbers, then merges the JS symbols onto the same file node; PHP wins on same-name collisions. Files with no <script> tag short-circuit before the JS pass and any extraction error is swallowed rather than failing the file.
Worth a look
- JS edges appended without checking node id existence may reference dropped nodes —
graphify/extract.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
- Script tags inside PHP strings are parsed as inline JS —
graphify/extract.py:2572· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Merged JS edges can attach to retained PHP node after node-id collision —
graphify/extract.py:2611· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Bare except silently discards JS extraction failures —
graphify/extract.py:2612· 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 — 2568 functions depend on the 1014 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 645 callers, 45 callees - new:
_rebuild_code()— 129 callers, 54 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 30 more — each is listed as a finding
Verification — 2568 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: 2393 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
286 of 286 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_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_build.py— impact, 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_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_external_call_guards.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-safety- … and 236 more
non-code file(s) changed (
CHANGELOG.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 (
CHANGELOG.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 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: no capturable inputs from the test suite; property tier: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 38 more finding(s) on lines outside this diff (see the check run).
A reviewer on this PR pointed out that when a JS symbol's id collides with an existing PHP node (same name, one file, resolved in favor of the PHP node), the merge still appended every JS edge unconditionally. An edge meant for the discarded JS symbol silently attached to the unrelated PHP node sharing its id instead of vanishing with it. The file node's own always on collision (both passes deliberately reuse its id) is excluded from this so an unrelated JS symbol's own edges sourced from the file node are unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers a same name PHP/JS function pair in one file: exactly one surviving node, the JS call to the colliding name absent as a calls edge entirely rather than misattached, and the unrelated JS symbol's own contains edge from the shared file node still present. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Good catch — fixed. When a JS symbol's id collided with an existing PHP node (same name, resolved in favor of the PHP node), every JS edge was still being merged in unconditionally, so an edge meant for the discarded JS symbol silently attached to the unrelated PHP node sharing its id. Now edges touching a dropped JS node's id are skipped too, with the file node's own always on collision (both passes deliberately reuse its id, by design) excluded from that so unrelated JS symbols' edges from the shared file node are unaffected. Added a regression test with a same name PHP/JS pair. |
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
Indexes JavaScript declared in inline <script> blocks of .php files: extract_php now runs a second JS pass over the source with everything outside script bodies blanked to spaces (line numbers preserved) via _php_mask_to_script_blocks, and merges those symbols onto the same file node, skipping the pass entirely when no <script> is present. On a same-name PHP/JS id collision it keeps the PHP node and drops every edge touching the discarded JS id, so no stray edge misattaches to the unrelated PHP node — while preserving the shared file node's own contains edges to surviving JS symbols. A <script src=...> with no body masks to empty JS and contributes nothing.
Worth a look
- Bare except swallows all errors, hiding JS extraction failures —
graphify/extract.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
- PHP extractor treats script tags in PHP comments as executable JS —
graphify/extract.py:2550· 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 — 2570 functions depend on the 1016 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 645 callers, 45 callees - new:
_rebuild_code()— 129 callers, 54 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 30 more — each is listed as a finding
Verification — 2570 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: 2395 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
286 of 286 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_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_build.py— impact, 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_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_external_call_guards.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-safety- … and 236 more
non-code file(s) changed (
CHANGELOG.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 (
CHANGELOG.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 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: no capturable inputs from the test suite; property tier: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 38 more finding(s) on lines outside this diff (see the check run).
|
Re: the two remaining advisory findings. "Bare except swallows all errors, hiding JS extraction failures": checked, and this matches an existing, deliberate convention already in this codebase — "PHP extractor treats script tags in PHP comments as executable JS" ( |
1 participant
Summary
Fixes #2320. `.php` files that mix PHP with an inline `<script>` block only had their PHP half indexed — any JavaScript declared inside `<script>…</script>` was silently absent from the graph, so `query`/`explain`/`path` couldn't see it. This matters for classic server-rendered PHP apps, where a large share of behavior lives in inline `<script>` blocks.
Root cause
`extract_php` delegates straight to the generic tree-sitter path with no handling for embedded regions. `tree-sitter-php`'s grammar treats non-PHP spans as opaque text, so JS inside them is never parsed.
Fix
Reuses the masking pattern `extract_vue` already established for the same class of problem (an embedded `<script>` region a host grammar can't see into): after the normal PHP extraction, everything outside `<script>` bodies is blanked to spaces (preserving `\r`/`\n` so line numbers stay accurate), and the masked full source is parsed with the JS grammar via `_extract_generic(path, _JS_CONFIG, source_override=...)`. The two node sets merge onto the one file node — a symbol declared in the script block looks like any other symbol in the file.
Two edge cases handled:
A PHP echo tag inside the JS region (`const MODE = ;`) doesn't crash the JS parser and the rest of the block still extracts correctly, so the placeholder-substitution refinement the issue mentioned as a nice-to-have wasn't needed.
Test plan
🤖 Generated with Claude Code