feat(proto): index Protocol Buffers as a contract, not an asset - #1659
Open
ferrine wants to merge 2 commits into
Open
feat(proto): index Protocol Buffers as a contract, not an asset#1659ferrine wants to merge 2 commits into
ferrine wants to merge 2 commits into
Conversation
`callers`, `callees` and `impact` each carried their own symbol filter,
comparing the user's query against the BARE node name only:
node.name === symbol || node.name.endsWith('.' + symbol)
That fails in two opposite directions in the same repository.
A bare name OVER-reports. Every same-named definition passes the filter
and their results are unioned under one heading — "Callers of group" can
list callers that belong to an entirely different `group`, with nothing
saying the name was ambiguous. Collisions cluster on short generic names
(`group`, `num`, `parse`), so this bites hardest exactly where the verbs
would otherwise be most useful.
A qualified name UNDER-reports. `Foo.Bar.baz` can never equal a bare
`baz`, so every candidate fails the filter, and the guarded fallback
takes whichever node full-text search ranked first — or reports "not
found" for a symbol that plainly exists. It only ever appeared to work
when FTS happened to return exactly one hit.
All three now resolve through graph/symbol-lookup, which the MCP tools
share, so a verb cannot drift from the matcher again:
- the exact-name index is consulted first and is authoritative. It is
complete and uncapped, whereas FTS ranks, truncates, and tokenises
`::` away — so resolution no longer depends on search ranking. FTS
stays as the fallback for the fuzzy cases it is good at.
- `matchesSymbol` gains a boundary-aligned suffix match under a
canonical separator. Splitting on every separator assumes no scope
component contains one, which is false for any language whose module
names are themselves dotted: the stored `A.B::c` can never equal the
split-and-rejoined `A::B::c`, so a precise query resolved to nothing.
- an ambiguous bare name still aggregates (an interface method and its
overrides are usually all wanted) but the union is now disclosed, with
the matched definitions named and a qualified spelling that narrows
it. `--json` gains a `targets` array and an `ambiguous` flag.
`matchesSymbol` moves out of mcp/tools.ts unchanged apart from the new
stage; the tool path delegates to it, so its existing coverage applies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Bsi9EH64kMisnik4E1gf7
A `.proto` is the contract layer of a polyglot repository: one field is implemented again in every generated language, each of those sites is machine-written and must never be hand-edited, and none of it was visible to the graph. Indexing it makes a field a symbol you can ask questions about — and makes "what else moves when this changes" answerable from the one place the answer is actually written down. Extracted: messages (including nested), enums and their values, fields, `oneof` members, `map` types, services, `rpc`s (including `stream`), imports, and `reserved`. Names are protobuf's own fully-qualified names (`acme.reporting.v1.Measurement.observed_at`), which is both what a user would type and what a generator uses. Field types and rpc request / response messages become `references`, so `callers` on a message lists the fields and RPCs that depend on it. Two properties are modelled on purpose, because they are where protobuf's real defects live and neither survives a naive extraction: - THE TAG NUMBER IS PART OF A FIELD'S IDENTITY. Renaming a field at the same tag is wire-compatible; changing its TYPE at the same tag and name is a silent mis-decode that every single-language check passes. The tag is recorded as a marker, not just left in prose, so a check can read it without re-parsing the declaration. - `reserved` IS SEMANTIC. A retired number must never be re-used and a reader touching a reserved field is a defect, so reservations — numbers, ranges and names — are kept as symbols rather than discarded as syntax. They are deliberately not `field` nodes, so a reservation can never be mistaken for a live field. Implemented as a standalone scanner (proto-extractor.ts) rather than a vendored grammar, following the Liquid / Razor / MyBatis precedent. The IDL is small and effectively frozen, so the usual reason to want a grammar — tracking an evolving syntax surface — does not apply, and it avoids shipping another megabyte of wasm whose silent absence is its own failure mode. Comments are blanked offset-preserving before scanning, skipping string literals so a URL in an option value is not mistaken for a comment. Verified against upstream protos: descriptor.proto (proto2, groups, extensions, 17 reserved statements), pubsub.proto, struct.proto, timestamp.proto and grpc health.proto all parse with zero errors, and the message / enum / service / rpc counts match the files exactly. Depends on the qualified-lookup fix: without it a protobuf FQN is a dotted container name, which the old matcher could not match at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013Bsi9EH64kMisnik4E1gf7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branch:
ferrine:feat/protobuf-language→mainStacked on:
fix/symbol-lookup-consistency(PR #1) — a protobuf FQN is adotted container name and is unaddressable without that fix.
Size: +848 over #1 · Tests:
__tests__/proto-extraction.test.ts(31 new)Why
A
.protois the only place a wire shape is authored, and it was not in thegraph at all — so the one file that governs every generated tier was the one
file you could not ask about.
What changed
New standalone extractor (
src/extraction/proto-extractor.ts, no tree-sittergrammar — protobuf's grammar is small and stable enough to scan directly,
following the precedent of the
liquid/dfm/vueextractors).Indexed: messages including nested ones, enums and their values, fields,
oneofmembers as fields of the enclosing message,maptypes, services andtheir
rpcs includingstream, andimports. Names are the protobuffully-qualified names, so
acme.reporting.v1.Measurement.observed_atissomething you can look up.
Two details are modelled deliberately:
tag=Nmarker, not just prose in thesignature). Renaming a field at the same tag is harmless; changing its type at
the same tag is a silent wire break — a check for that has to read the tag
without re-parsing.
reservednumbers, ranges and names are kept as symbols (constant,never
field), so a retired field stays findable instead of vanishing fromthe graph — which is the question "is this number off-limits?" that reservation
exists to answer.
Comment stripping is offset-preserving and string-literal aware, so a
//insidea default string value doesn't truncate the declaration.
Also indexes Python type stubs (
.pyi) — real checked-in API surface, and forprotobuf the only place per-field Python declarations exist at all.
Notes for review
codegraph.jsonneeds no configuration;.protois picked up by extension.referencesedges to the declarations they use, somessage-to-message dependencies are traversable within the proto tier.