Repository navigation
fix(resolver): an unresolved call never joins a real node - #684
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses issue #661 by preventing unresolved calls from incorrectly joining real nodes when their raw names collide with real node IDs. To achieve this, unresolved targets retain their raw_call: prefix if a collision is detected, and Go builtins are now prefixed with a builtin pseudo-package (e.g., builtin.len) to avoid conflicts with local variables or closures. The query engine has been updated to strip the raw_call: prefix when displaying unresolved targets to users, and comprehensive unit tests have been added to verify these behaviors. I have no feedback to provide as there are no review comments.
An unresolved CALLS or EXTENDS edge targets its bare raw name. When that text equalled a real node's id (a Go local q shadowing package q, a Python parameter named like a module) the resolver minted no virtual node and the edge joined the real one, read as a resolved call: a false caller in impact, counted resolved by validate and coverage. Such a name now keeps the raw_call: prefix (no real id holds one) and gets its own UNKNOWN node named by the bare tail; coverage shows the bare name. Consumers already read the prefix as unresolved. Go builtins resolve to builtin.<name> (STDLIB), as TS uses js_builtins., so a bare len is no longer shared by a resolved builtin call and an unresolved call to a local closure named len. Measured by the consumer map: 1 collision on owner-api, 11 on kafka-go (all accidentally correct; #680), 0 on cgis, raft, crypto, lima, client_golang, owner-web and both snapshots, which are unchanged. Docs: ONTOLOGY.md no longer describes the pre-#49 shape (closes #682). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…allers Review of #684: - _cross_file_inputs_changed treated a stored virtual row with the new node's id as another file's row, so no rebuild ran and the stale bare edge joined the new node (full and incremental ingest disagreed). Virtual rows are no file's. - a bare Go builtin name is an unresolved call to a local; it classifies UNKNOWN, only builtin.<name> is STDLIB. - cohesion no longer reconciles a raw_call: target to a file by suffix. - _unresolved_target is a module function (god-object gate); RAW_CALL_PREFIX lives once, in core.models. - docs: a Go builtin is resolved (1.0), not an unresolved fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
975e1bf to
1cd097d
Compare
The SymbolIndex import added in the review fix changed the resolver domain's call triads, and self-drift scored 0.58 against pipeline_stage (tolerance 0.40). _unresolved_target takes the ResolutionView engine.py already imports. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Delta review of #684: the predeclared-type head check (meant for methods such as error.Error) also caught a bare string(x) on a local closure and called it STDLIB. It now needs a dot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request addresses issue #661 by ensuring that unresolved calls do not incorrectly merge with real nodes sharing the same name (such as local variables shadowing packages or parameters shadowing modules). It implements a mechanism where unresolved targets retain the raw_call: prefix if their name collides with a real node ID. Additionally, Go builtins are now resolved to a qualified builtin.<name> format to prevent collisions with local closures. The PR also updates documentation, adjusts confidence level definitions, refactors the location of RAW_CALL_PREFIX, and adds comprehensive unit tests to verify these behaviors. There are no review comments to evaluate, so no additional feedback is provided.
Summary
Fixes #661 and #682. The fix follows option (b) from the consumer map (see the issue thread).
Problem. An unresolved CALLS or EXTENDS edge targets its bare raw name, and the resolver mints a virtual boundary node for it. When that text equalled the id of a real node,
_ensure_virtual_nodeminted nothing, and the edge joined the real node at 0.8. It then read as a resolved call: a false caller in impact, counted as resolved byvalidateand by trace coverage.qthat shadows packageq, so the raw callq.Getequals the nodeq.Get.vocabulary(1)→ FILEvocabulary).Fix:
ResolverEngine._unresolved_targetis used by the call path and the class path. A raw name that equals a real node id keeps theraw_call:prefix, which no real id carries, and gets its own UNKNOWN virtual node named by the bare tail. Seven consumers already treat the prefix as unresolved (left over from feat(classifier): external/stdlib node classification via virtual nodes #49). Every consumer that decides by namespace is now correct without changes.top_unresolved,sites) show the bare name.builtin.<name>(STDLIB), the way TS usesjs_builtins.. A resolvedlen(...)and an unresolved call to a local closure namedlenno longer share one barelennode, so their edges stop merging.Size, from the consumer map:
There is no schema change. The extraction fingerprint forces a full rebuild on the next ingest.
Test plan
tests/unit/test_unresolved_collision.py: Go local-shadows-package, Golenclosure vs builtin, Python parameter-named-like-module, unresolved class reference, and a store invariant (no CALLS/REFERENCES/EXTENDS edge below 1.0 joins a non-virtual node). All five failed before the fix.test_go_builtins_are_stdlib_and_a_package_function_shadows_one: updated tobuiltin.<name>.check_test_count.py --base-ref origin/main🤖 Generated with Claude Code