Repository navigation
feat(engine): extract php symbols with structure awareness - #369
Conversation
|
Implements #88 |
ZG Retrieval BenchmarkRetrieval-only · PR base vs head✅ Complete Base: zvec-ai/zvec-grep@e4082245bcc861ed92a25203f7e3f2f4cfdbb063 Head: GrzegorzDrozd/zvec-grep@c833298ef1381b4abce2a9bd60b42da96ae838ce Each cell is base / head / Δ. Δ = head − base; all five metrics use the 0–1 scale and higher is better. Both versions use the same frozen harness, queries, gold labels and local embedding models, with fresh indexes and five calls per query/mode. Quality is report-only; no regression threshold is applied.
SWE-QA nDCG uses repository macro averaging; the other suites use query means. Suites are reported separately. Full base/head reports and per-call evidence are available in the run artifacts. Embedding: local. e2e: skipped. This comment is updated by automatic PR Retrieval runs. |
egolearner
left a comment
egolearner
left a comment
There was a problem hiding this comment.
Reusing the existing language adapter fits the PHP support requested in #88. The main issue is loss of previously searchable template and top-level PHP content once a declaration is found. The inline comments also cover existing-index upgrades, declaration metadata, and outlines.
Please include a companion update to both language versions of the supported-content guide, as required by CONTRIBUTING.md. The published guides still classify PHP as plain-text chunks. English guide · Chinese guide
Address review on zvec-ai#369. - Index source between PHP declarations as plain-text chunks, so templates, imports and top-level statements stay searchable. - Give each split constant or property element its own signature. - Keep ternary true branches in signatures. - Name by-reference promoted parameters without the & and $. - Keep the receiver or class in PHP call targets in outlines. - Parse stored PHP entities with the full grammar behind a synthetic open tag, so ?>text<?php switches stay valid.
PHP files were discovered but fell through to plain-text chunks. They now parse with tree-sitter-php 0.25.1 and index classes, interfaces, traits, enums, enum cases, functions, methods, properties, constants and braced namespaces with scopes, signatures and docblocks. - Property and constant statements that declare several elements split into one entity per element. A lone element keeps the whole statement, as Java fields do. - Promoted constructor parameters are indexed as properties on the class, since the walk never descends into a method. - Signatures drop attributes and PHP 8.4 property hook bodies. - Traits classify as interfaces, matching Rust traits. - Outlines parse stored entities with the PHP-only grammar, because they lack an open tag, and recognise member, nullsafe and static calls. - zg --help file-types lists php under structured code. Statement-form namespaces (namespace App;) have no body, so their declarations are not scoped under them. Visibility stays unset, matching the other languages.
Address review on zvec-ai#369. - Index source between PHP declarations as plain-text chunks, so templates, imports and top-level statements stay searchable. - Give each split constant or property element its own signature. - Keep ternary true branches in signatures. - Name by-reference promoted parameters without the & and $. - Keep the receiver or class in PHP call targets in outlines. - Parse stored PHP entities with the full grammar behind a synthetic open tag, so ?>text<?php switches stay valid.
afa85d7 to
6169d9b
Compare
|
Addressed all inline comments in the latest commit. For the companion docs, I'll open a zvec-web PR updating the supported-content guide: PHP moves to structured code with .phtml listed, plus a note that existing workspaces need zg --index --rebuild to get PHP symbols. I'll link it here once it's up. |
|
Doc update: zvec-ai/zvec-web#82 |
egolearner
left a comment
egolearner
left a comment
There was a problem hiding this comment.
The previous review threads are resolved. Two additional P2 issues remain in comment coverage and callable summaries.
Address second review on zvec-ai#369 and findings from adversarial local tests. - Cover a declaration's docblock from the declaration, so comments between split constant or property elements stay searchable. - Skip calls that run nothing at the call site: first-class callables (record(...)), anonymous classes, immediately invoked closures and new static, new self or new parent. - Skip attribute arguments, which run on reflection. - Build receiver call names from tokens, so layout and comments no longer leak into them and dynamic names keep their closing brace. - Record PHP member, nullsafe and static calls as graph edges with their receiver, as Java does. Outlines and graph share one rule for what counts as a call. - Scan every element of a split PHP statement for calls, so each initializer is attributed to its own element. - Let complete top-level PHP functions skip the method wrapper in outlines, which the synthetic open tag used to prevent. - Stop joining receiver tokens once a name exceeds the line limit. The graph does not yet resolve receiver-qualified or cross-file references in any language, so PHP method edges stay pending until the resolver supports them.
|
Hi @egolearner :) Pushed fixes for both comments. While testing this round I also ran a larger local corpus of unusual PHP through the real zg binary. It found a few more outline issues, which this push fixes as well:
Known limitations I left alone: declarations nested inside a function body aren't extracted, as in the other languages. isset, empty and eval show up as calls because the grammar models them that way. |
egolearner
left a comment
egolearner
left a comment
There was a problem hiding this comment.
One P2 issue remains in PHP call-graph resolution.
|
Thanks for the fixes and the additional PHP coverage. I've reviewed the updates and resolved the previous review threads. The documented rebuild requirement and companion docs update work for this PR, and the known limitations you listed can stay outside its scope. I left one additional comment on the call-graph fallback: dropping an unsupported receiver can bind a method call to an unrelated global function. Preserving the receiver and leaving the target unresolved, or omitting an unsupported edge, would address this without requiring broader PHP name or type resolution. Please also add a regression test covering the final partitioned graph with a same-named global function. |
Address third review on zvec-ai#369. - Keep the receiver on every PHP member, nullsafe and static call edge. A receiver the resolver cannot read, such as \Audit or $this->factory(), keeps its source spelling, so the call stays pending instead of binding to a same-named global function. - Drop edges for dynamic method names such as $this->{'record'}(). - Use the bare method name in outlines when the receiver defines a class or closure inline, so its source no longer leaks into the call name. - Skip calls of another call's result, such as make()(). The inner call is still reported.
|
@egolearner ready :) |
|
Thanks for adding PHP support and for working through the review feedback. I appreciate the extra regression coverage and the companion documentation update. The review is complete, and I'm preparing to merge this. Thanks again for the contribution! |
|
CI is currently failing in CI is testing the merge with the latest Could you merge the latest |
Set package: None in the PHP metadata test, since CodeMetadata gained a package field on main. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
|
Merged the latest main into the branch and set package: None in the PHP metadata test. fmt, clippy and the engine and CLI tests pass locally on the merged tree. I also checked the PHP call edges against the new resolver: self::record() inside a class now resolves to that class's method, and $this, static and parent receivers stay pending, so nothing binds to a same-named global function. |
PHP files were discovered but fell through to plain-text chunks. They now parse with the full tree-sitter-php 0.25.1 grammar, so .phtml templates that mix HTML and PHP work too, and index classes, interfaces, traits, enums, enum cases, functions, methods, properties, constants and braced namespaces with scopes, signatures and docblocks.
Statement-form namespaces (namespace App;) have no body, so their declarations are not scoped under them. Visibility stays unset, matching the other languages. PHP has no import or heritage vocabulary in the code graph yet, so it uses the generic resolution.
Existing workspaces already hold PHP files as plain-text chunks, and unchanged files are not re-extracted. Run zg --index --rebuild to pick up PHP symbols.
The supported-content guide update will follow in a zvec-web PR.