fix(extract): parse PHP past inline HTML so symbols after ?> reach the graph (#2000)

[harshitaajoshi]

Fixes #2000

What does this PR do?

The vendored PHP grammar is the php_only variant, which has no rule for ?> or the markup after it. In a file that leaves PHP mode (?> <div>...</div> <?php), everything after the first close tag became an ERROR region, so the declarations and calls there never reached the graph and the file was reported parse_partial.

Before parsing, PHP source now goes through cbm_php_mask_inline_html() (new internal/cbm/php_inline_html.c), which blanks the inline-HTML regions:

• markup before the first open tag and between tags becomes spaces
• the first open tag stays, so the tree keeps its php_tag
• later <?php, <?= and <? tags become spaces; after <?= the echoed expression stays as an expression statement, so its calls are kept
• ?> becomes ; , the statement terminator PHP itself treats it as

The rewritten buffer has the same length and every line break, so byte offsets and line numbers still refer to the file on disk, and node text inside PHP code is unchanged. ?> is only treated as a close tag in code and in // / # comments; strings, heredoc, nowdoc and block comments are skipped, #[ is treated as an attribute, <?xml in markup is not an open tag, and nothing after __halt_compiler is touched. A file that starts with an open tag and has no ?> takes a fast path and is parsed exactly as before.

It is applied in two places so both parses see the same bytes:

• extract_file_ex_body in cbm.c, via a small cbm_parse_source() helper
• cbm_run_php_lsp_cross in php_lsp.c, which receives the on-disk source and may re-parse it

No grammar, pipeline pass or MCP tool behavior changes.

Tests

Reproduce-first: the extraction tests were added and run before the fix. Three of them failed on main (delta missing, two missing, and the template file flagged parse_incomplete); all pass with the fix.

tests/test_extraction.c

• php_inline_html_tail_reaches_graph_issue2000: the fixture from the issue; delta is extracted at line 8 and its call to gamma is scoped to delta
• php_inline_html_template_shapes_issue2000: <?= f() ?>, short echo without spaces, if/else/endif and foreach { } split across PHP blocks
• php_inline_html_close_tag_in_literals_issue2000: ?> inside single, double quoted, heredoc, nowdoc and block comment; #[Attr]; // comment ended by ?>; <?xml prolog in markup
• php_inline_html_trailing_close_tag_crlf_issue2000: trailing ?> and CRLF line numbers

tests/test_pipeline.c

• pipeline_php_inline_html_calls_resolve_issue2000: end to end, delta -> gamma (same file) and renderCard -> formatPrice (cross file, both files contain inline HTML) exist as CALLS edges in the store

Suites run with the change: extraction, pipeline, php_lsp, parse_coverage, grammar_regression, grammar_labels, grammar_imports, grammar_probe_a to grammar_probe_d, edge_imports, edge_structural, lsp_resolution_probe, node_creation_probe, convergence_probe, integration, lang_contract, incremental. No failures. I'm relying on CI for the full matrix.

Real repository check

Indexed Automattic/_s (a783f97) with a build of main (66d9c2b) and a build of this branch:

	main	this PR
PHP files reported parse_partial	17	0
Function nodes	29	32
CALLS edges	15	17

The remaining 4 partial files in that repo are SCSS. Recovered: _s_woocommerce_wrapper_after, _s_woocommerce_cart_link_fragment, _s_woocommerce_cart_link and the two CALLS edges into _s_woocommerce_cart_link. No function or edge present on main is missing, and the recovered functions' start_line values match the source file.

Checklist

• Every commit is signed off (git commit -s)
• Tests pass locally (make -f Makefile.cbm test): the suites listed above pass; full run left to CI
• Lint passes (make -f Makefile.cbm lint-ci, plus lint-tidy-diff and scripts/security-audit.sh)
• New behavior is covered by a test (reproduce-first for bug fixes)

Written with the help of Claude Code. I reviewed the change and ran the tests and the repository check above myself.

[DeusData]

Thank you so much, @harshitaajoshi, this is a really careful piece of work. Blanking the markup into a buffer of the same length, rather than touching the vendored grammar, is exactly the right level for #2000. Byte offsets, rows and node text all stay in on-disk coordinates, so nothing downstream (line ranges, snippets, parse-coverage ranges) has to know it happened. We especially appreciated:

• the care with the lexical edge cases: strings, heredoc, nowdoc and block comments versus ///# closing PHP mode, plus #[, <?xml and __halt_compiler;
• mapping ?> to ; the way PHP itself treats it, and keeping the <?= expression so echo calls survive;
• remembering the cross-file LSP entry, so the cached tree and a re-parse agree.

The Automattic/_s before/after table (17 -> 0 partial files, no function or edge lost) is exactly the kind of evidence we love to see.

A few small requests before we merge:

1. Please rename the static cbm_parse_source in internal/cbm/cbm.c. Open PR #2340 introduces a public TSTree *cbm_parse_source(TSParser *, const char *, uint32_t, TSParseOptions) in cbm.h/cbm.c. The two branches merge with no text conflict, but the merged cbm.c then has both definitions and won't compile, and because the text merge is clean nothing flags it before the build. A distinct name, or inlining the language == CBM_LANG_PHP check at the call site, keeps the two PRs independent. Otherwise your masked buffer works fine with #2340.
2. Please add a test that fails if the php_lsp.c masking line is reverted. That line matters most on a path the current tests may not reach. When a file's result has been spilled to disk under the memory budget, it comes back without its cached tree, and cbm_run_php_lsp_cross re-parses the source itself. Without the mask, the tail of an inline-HTML file would lose its cross-file resolution only when that file happened to spill. The graph would then depend on memory pressure, which we treat as a correctness bug. A direct call in tests/test_php_lsp.c with cached_tree = NULL on your #2000 fixture would pin it; tests/test_go_lsp.c (around line 1067) shows the shape. If you find the pipeline test already fails with that line reverted, just tell us and that's fine too.
3. Please add a negative assertion that markup never becomes graph content. Graph quality is our first priority, so alongside "the tail comes back" we'd like a guard that the HTML itself never produces nodes. For example, put code-shaped markup between ?> and <?php (<p>function fake() { return leak(); }</p>) and assert !has_def(r, "Function", "fake") and !has_call(r, "leak"). That pins the "blanked, never parsed" behaviour and catches any future mistake in open-tag detection.

Small nits, take or leave:

• In pipeline_php_inline_html_calls_resolve_issue2000, the fixture loop could simply use write_temp_file(...). That helper opens files with cbm_fopen(..., "wb"), and binary mode keeps the bytes identical on Windows.
• cbm_php_mask_inline_html allocates and copies the whole file before it knows whether anything will change. Copying on the first write would avoid a throwaway copy when the only ?> bytes sit inside strings or comments.

Two shapes we'd expect to still leave a small error region. They're fine for a follow-up, and still far better than today:

• <?= $a, $b ?>;
• <?php switch ($x): ?> followed by <?php case 1: ?> (the leading ; before the first case).

Thanks again. This closes a gap that affects most legacy PHP codebases, and it was a pleasure to review.

[harshitaajoshi]

Thanks a lot for the thorough review! All three requests and both nits are addressed in 5ff3e8d.

1. Rename: the static helper in cbm.c is now extract_parse_bytes, so it no longer collides with the public cbm_parse_source from fix(extraction): parse every file as if its last line were terminated (#2078) #2340. I checked that the new name does not appear in fix(extraction): parse every file as if its last line were terminated (#2078) #2340's diff.
2. php_lsp.c masking line: added phplsp_cross_reparse_reads_past_inline_html_issue2000 in tests/test_php_lsp.c. It calls cbm_run_php_lsp_cross directly with cached_tree = NULL on the PHP: symbols declared after an inline-HTML block never reach the graph, and the whole tail of a valid file is reported parse_partial (0.10.8) #2000 fixture and expects delta -> Mailer.send to resolve. With the masking line reverted it fails (find_resolved_arr(...) (-1) not >= 0); with it in place it passes. To answer your question: the pipeline test does not catch the revert, since that path has a cached tree. One detail I hit on the way: my first version put a class after the markup, and tree-sitter's error recovery happened to rescue it, so the test passed even without the mask. The fixture now uses a free function after the markup, the same shape as in the issue.
3. Markup never becomes graph content: added php_inline_html_markup_never_reaches_graph_issue2000. It puts code-shaped markup before the first tag, between tags (including inside <script> and after an <?xml prolog) and after the last ?>, then asserts none of before, fake, jsFake, xmlFake, Tail or tailFake is defined and no leak* call exists.

Nits:

• The pipeline test now uses write_temp_file(...).
• The mask now copies on first write. A file whose only ?> bytes are inside strings or comments gets its original buffer back without a copy. This is pinned by php_inline_html_mask_copies_only_on_write_issue2000, which also checks the exact masked bytes for a small mixed file.

Agreed on <?= $a, $b ?> and switch (...): ?> <?php case. Both still leave a small error region, and I'm happy to follow up on them separately.

Suites run with this commit: extraction, php_lsp, pipeline, parse_coverage (1002 passed, 0 failed), plus lint-ci and lint-tidy-diff.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Thank you, @harshitaajoshi. This round covers every point from our review, and it's done carefully:

• extract_parse_bytes no longer collides with fix(extraction): parse every file as if its last line were terminated (#2078) #2340's public cbm_parse_source, and the two PRs merge cleanly together.
• The new php_lsp test calls the cross pass with no cached tree, which is exactly the path the pipeline test couldn't reach. Thanks also for spelling out the error-recovery detail you hit on the way.
• The markup-never-reaches-the-graph test covers every place markup can hide, with a positive control.
• The fixture writer now goes through write_temp_file.

The red test-windows-guards check is a known Windows daemon-startup race tracked in #2057, and it has nothing to do with your change.

One optional nit, only if you're touching the branch anyway: php_inline_html_mask_copies_only_on_write_issue2000 can't tell copy-on-write apart from the old behaviour, because the old code also returned the caller's pointer for plain and quoted. Sampling arena.used before and after each call would pin it: unchanged for plain/quoted, grown for mixed.

Next, we build and run the extraction, php_lsp and pipeline suites locally before merging. Nothing else is needed from you.

[harshitaajoshi]

Thanks! I took the optional nit in 43ff3be since it makes the test check what it claims to.

php_inline_html_mask_copies_only_on_write_issue2000 now samples the arena around each call: no bytes allocated for plain and quoted, a copy for mixed. I used cbm_arena_total() rather than arena.used, because used is per block and resets when the arena grows a new one, while the total counts every allocation.

To check it really tells the two apart, I ran the new test against the previous eager-copy php_inline_html.c from 27c8710. It fails there on the quoted case (cbm_arena_total(&arena) == 48, expected before == 0), while the pointer check still passes, which is exactly the gap you pointed out. With the current code extraction and php_lsp pass (673 passed, 0 failed).