fix(extract): receiver-qualify Go method QNs

[ilyabrykau-orca]

What does this PR do?

Fixes #1909. Stacked on #1907 (shares the Go test scaffolding in tests/test_extraction.c; the first commit here is #1907's — review only the last commit).

Go concrete methods carried a flat package QN (proj.pkg.method, receiver ignored), so every same-name method in a package collided on one QN and the graph upsert kept exactly one node — on the repo measured in #1909: 20 Process/Name methods → 2 nodes, 9 Task() methods fused into one chimera node carrying all nine bodies' call edges, 19 structs DEFINES_METHOD-linked to a single shared node, and a _test.go mock outranking the production method in the dedupe tie-break. The upsert's own comment calls kind-disambiguated QNs "the real cure".

The change — same shape as Go interface members and the C++ out-of-line method path directly below it in extract_func_def:

1. internal/cbm/extract_defs.c — when the receiver type resolves, def.qualified_name = parent_class + "." + name (proj.pkg.Storage.Close); go_receiver_type_name becomes the shared cbm_go_receiver_type_name (declared in helpers.h, following the cbm_cpp_out_of_line_parent_class precedent) so both sides of the contract use one formula.
2. internal/cbm/extract_unified.c (compute_func_qn) — mirror branch for Go method_declaration, so method-body calls keep exact source attribution instead of degrading to the calls_find_source File-node fallback.
3. No resolver changes needed — consumers already agree:
   • pxc_build_lsp_def passes the def QN and parent_class (→ receiver_type) verbatim into the Go LSP registries, so lsp_type_dispatch / lsp_embed_dispatch / interface-satisfaction emissions follow the new QN automatically (all lrp_go_s* probes stay GREEN, untouched).
   • check_go_class_implements explicitly supports class-qualified method QNs (its path (b) reconstructs <ClassQN>.<method>; its comment describes the flat QN as the anomaly).

Side effect (also #1909): resolve_same_module's exact module.name hash no longer matches concrete methods, which removes the confidence-0.9 false edges where an interface-typed call (pipeline.Process(event)) bound to an unrelated same-package method.

Tests:

• extract_go_method_receiver_qualified_qn — two same-name methods on different receivers get distinct, receiver-qualified QNs with matching parent_class; free functions keep the flat QN.
• extract_go_no_filename_in_module_qn updated: the method expectation becomes proj.myapp.db.Conn.Query (its actual contract — no filename segment in the QN — still asserted).
• Full scripts/test.sh leg green (ASan+UBSan, "All tests passed"), including all lsp_resolution_probe Go scenarios and the fix(pipeline): suppress weak short-name matches for Go selector calls #1907 pipeline test, with zero probe changes.

Note for maintainers: this changes extracted QNs for Go methods, so existing graphs need a reindex to pick up the new shape (flagged in #1909 per CONTRIBUTING's indexing-change rule).

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects
  unsigned commits (DCO, see CONTRIBUTING.md)
• Tests pass locally (scripts/test.sh — full leg, ASan+UBSan, "All tests passed")
• Lint passes (git clang-format --diff clean on changed lines; clang-tidy/cppcheck via CI)
• New behavior is covered by a test (reproduce-first for bug fixes)

[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.

[ilyabrykau-orca]

Rebased onto current main (stacked on the rebased #1907). No semantic changes; full test leg green. Post-fix field numbers for this change are in the #1906 census comment (method-node recovery incl. generic receivers: #1906 (comment)).

[DeusData]

Reviewed the last commit. The bug is real and badly worth fixing, the shape of the fix is right, and there is one measurement I want before it lands.

The problem is as bad as you say. store.c:257 declares UNIQUE(project, qualified_name) and the node upsert is ON CONFLICT(project, qualified_name) DO UPDATE SET, so a flat proj.pkg.method genuinely makes every same-name method in a package one row. Nine Task() methods fusing into a single node carrying all nine bodies' call edges is not a cosmetic count problem — that node is a chimera whose outgoing edges are attributed to code that never made them. And a _test.go mock outranking the production method in the dedupe tie-break is the kind of thing that quietly poisons every answer downstream.

Receiver-qualifying is the right cure, matching the Go interface-member and C++ out-of-line paths already in extract_func_def, and lifting go_receiver_type_name to a shared cbm_go_receiver_type_name so both sides of the contract compute one formula is exactly right — two copies of that formula would drift.

The measurement I want

You describe this as a side effect, in one line:

resolve_same_module's exact module.name hash no longer matches concrete methods, which removes the confidence-0.9 false edges where an interface-typed call bound to an unrelated same-package method.

I think that line is doing more work than it looks. resolve_same_module (registry.c:790) builds its candidate as module_qn + "." + callee_name. Before this change a call to Close inside proj.pkg hit proj.pkg.Close at confidence 0.90. After it, the method is proj.pkg.Storage.Close and that candidate misses — and the suffix retry at :795 builds the same shape, so it misses too.

So same_module stops resolving every Go concrete-method call, not only the wrong ones. The true ones do not disappear; they fall through to resolve_name_lookup, which picks a winner by import distance project-wide at a much lower confidence. That is the failure mode this repo has been bitten by before: a blocked high-confidence match can lose a correct edge and mint a wrong one in the same step, and the net can be negative even when every individual removal was justified.

You measured the false edges removed. I would like the other half measured too, and there is already an instrument for it — the per-language CALLS census with its dumped per-edge sets. The Go leg of that is the right baseline. Concretely, what I am asking for is:

• Go CALLS edge count before and after, and
• the strategy histogram — specifically how many edges moved out of same_module and where they landed.

If the answer is "the LSP type-dispatch path already covers them, so the demotions are few and the removals are many", that settles it and the PR merges on the strength of the numbers rather than on the argument. If a meaningful number of true edges demote to import-distance guessing, that is worth knowing before this is in main rather than after.

The lrp_go_s* probes staying green is real evidence and I am not dismissing it — but they are a fixed scenario set, and the population this affects is every Go method call in a repo without LSP coverage on the receiver.

Two smaller things

This shifts a baseline other PRs are being judged against. There is an open cluster of call-resolution PRs that we have decided to judge against one shared census precisely because whichever lands first makes the rest unreadable on their own numbers. A change to Go method QNs moves that baseline for the Go leg. Not an objection — just something I would rather sequence deliberately than discover afterwards.

Your reindex note is right and needs to travel further than the PR. A QN-shape change means existing graphs pick up the new shape only on rebuild, and that belongs in the release notes, not only in #1909. I will make sure it gets there.

Status

ms is DIRTY — this and both its stack neighbours (#1907, #1915) have conflicts. main was broken for a few hours today and was repaired an hour ago by #1993, so now is a good moment to rebase the stack.

Thanks for the measurements you did bring — "20 methods → 2 nodes, 19 structs linked to one shared node" is what turns this from a plausible tidy-up into an obvious defect.

[ilyabrykau-orca]

Rebased onto current main as part of the 1907→1913→1915→1936 stack; no source conflicts, test anchors re-anchored. Full scripts/test.sh green on the stack head.

[ilyabrykau-orca]

Here is the measurement — and it caught something real, so thank you for insisting on it.

The census (orca-runtime-sensor, Go→Go CALLS, isolated one-shot indexes)

	main	+#1907	+#1907+#1913	full stack (+#1915+#1936)
total	21018	14615	14812	13815
lsp_direct	8896	8896	9130	9124
lsp_type_dispatch	2551	2551	2640	2640
lsp_interface_dispatch	274	274	287	287
same_module	931	931	117	145
field_type_hint	676	676	1205	186
unique_name	2944	940	936	936
suffix_match	4554	155	180	180

Per-edge migration of #1907's 929 same_module keys (caller + callee-name identity, so chimera retargeting doesn't count as loss): 108 kept same_module (receiver still resolvable flat), 444 re-landed (308 field_type_hint, 97 lsp_strategy_cross_file, 39 unique_name), 377 lost entirely. The lost population is exactly the calls whose receiver cannot be typed — the ones same_module was binding at 0.90 by package-coincidence, which is this PR's thesis. The LSP strategies keep ≥96% of their edges and end above the pre-#1913 baseline. Net caller→callee pairs across the leg: +41.

What the first run of this census caught

My first measurement showed lsp_type_dispatch losing 568 edges and lsp_direct 105 — every sample was a method-body caller ((r *Repo) Install calling r.helper()). Root cause: go_lsp.c's process_function still set the enclosing-function QN to the bare package.func form; its own comment declared the contract ("must be the QN the textual call events and the defs pass produce"), and this PR had moved that shared form to package.Type.method on the other two sides but not this one. Every LSP resolution sourced from inside a method lost its caller join in cbm_pipeline_find_lsp_resolution and fell back — or died.

Amended into this PR (it is the same one-formula claim): process_function now receiver-qualifies via the shared cbm_go_receiver_type_name, with a reproduce-first pipeline test (pipeline_go_method_caller_keeps_lsp_join: same-receiver method call with a colliding decoy method in another package, RED without the go_lsp hunk, GREEN with it). The table above is measured with the fix; without it the leg read 14156 total with lsp_direct at 8263.

One upstream test adopted the new shape: parallel_go_cross_package_field_chain_resolves matched edges by the flat handler.PlaceOrder fragment; the edge it guards is intact — OrderHandler.PlaceOrder → OrderService.PlaceOrder, still lsp_type_dispatch at 0.95, now receiver-qualified on both endpoints (which also confirms the cross-package chain path under the join fix) — so its fragments moved to the qualified form.

On the baseline-shift point: agreed — these numbers are from the stack in its merge order (#1907 → #1913 → #1915 → #1936), so whichever leg lands first, the others' columns above stay the readable reference.

Full scripts/test.sh green on the amended stack head; branch force-pushed with the rebase.

[DeusData]

Census received, and thank you for insisting on measuring rather than arguing it — it caught exactly the thing I was worried about.

Where this stands: the result changes the shape of Go method qualified names (pkg.Type.method where main has pkg.method) and drops 377 same_module edges in exchange for the LSP-side gains. That is a graph-contract decision, not a review nit, and the standing rule here is that call-resolution changes are judged per language against one census rather than one PR at a time. So this is now a direction call on our side, with your census as the primary input — I will bring the answer back to this thread.

Please hold any further rebase of the stack (#1907 → #1913 → #1915 → #1936) until that call is made. main has moved again since your last one and the stack conflicts, but rebasing now would cost you a pass that may need redoing depending on the decision. Nothing else is needed from you meanwhile.

[DeusData]

Thank you for holding the rebase while we measured this ourselves — here is the result, and the decision.

We ran our own census on kubernetes/kubernetes @ 8c078bc3 with four builds (the merge-base, #1907 alone, #1907+#1913, the whole stack), diffed the full Go CALLS edge sets, and hand-classified 150 sampled edges against the source.

build	Go CALLS	what changed
base	318,635
#1907 alone	237,347 (−25.5%)	purely subtractive; all suffix_match/unique_name; 80% of sampled removed edges were false (os.ReadFile → a struct field, fmt.Errorf → a test fake, time.Sleep → *SleepAction)
+ #1913	254,821	+32,007 Method nodes the base had fused into one per name (we found base edges whose recorded call lines lie past end-of-file — the fusion bug in the flesh). On the lost side 18/25 sampled drops were false and 3 were actively corrected.
whole stack	235,519	#1936's last commit halves field_type_hint, lifting gained-edge precision from 48% to 64%

Decision: the direction is accepted. Please rebase #1907 first, then #1913 onto main; #1915 and #1936 follow once those two are in. We'll take each through the normal gates as it goes green.

Two things from the measurement, so you have the full picture:

1. We could not reproduce the 377. Our same_module true-loss for fix(extract): receiver-qualify Go method QNs #1913 vs base is 8,584 (net −7,207 after 1,377 gained), and no counting definition we tried — same-file only, same-package-dir only, bare callees, distinct targets — lands anywhere near 377. Could you say which corpus and which exact query produced it? It would tell us whether one of the two instruments is off.
2. Five defect shapes surfaced in the fix(extract): receiver-qualify Go method QNs #1913 sample that are not visible at the 377 level. Two are precision regressions (a cross-package StatusStrategy.ValidateUpdate suffix collision; metav1.Object.SetNamespace rebinding to meta.MetadataAccessor.SetNamespace with the wrong arity — seen twice) and three are recall losses (type-alias receivers such as type FeatureSet = bitmap; an interface-typed var holding a local fake; build-tag duplicate methods defeating unique_name). None of these block your PRs — we are taking them ourselves as follow-ups on top of fix(extract): receiver-qualify Go method QNs #1913, with the census as the test fixture source, and will credit this work in them.

The full dumps stay on our side if you want any of the sampled edges with file:line.

[ilyabrykau-orca]

Rebased onto current main behind #1907 (stack order per your decision). Conflicts were test-anchor drift only: the extraction fixture now sits after main's new Python local-binding tests, and the parallel field-chain assertion keeps the receiver-qualified fragments (OrderHandler.PlaceOrder → .service.OrderService.PlaceOrder). Full scripts/test.sh green locally (ASan+UBSan), git clang-format --diff clean. The 377-instrument answer is two comments up.

[DeusData]

Heads-up: I cancelled this PR's CI run on purpose a minute ago. It was sharing the hosted-runner pool with tonight's v0.11.0 release build, and the release needed the slots. Nothing about the PR is wrong. I will re-run the full CI as soon as the release is out, so any red you see right now is the cancellation, not your change. Thanks for your patience, and for the steady stream of Go fixes.

[DeusData]

Thank you, @ilyabrykau-orca. We've merged the updated #1907 head, which carries current main, into this branch from our side, keeping both sets of tests in tests/test_pipeline.c. Please pull before your next push.

The QN shape matches the 15 September direction. Methods become pkg.Recv.method, generic receivers are unwrapped, and free functions stay flat. On kubernetes/apiserver, Go Method nodes go from 3,248 to 4,700 and now equal the DEFINES_METHOD count: the fused-method bug is gone. Every production hunk is bound by a test (we neutered each one: the extract_defs, extract_unified and go_lsp hunks each turn tests red). The merge result passes 962 tests plus the 3,199 in the other Go-fixture suites, and lint-ci is clean.

Two asks:

1. Please bump CBM_INDEX_FORMAT_VERSION to 2 (src/store/store.h, with a line like 2 = Go method QNs are receiver-qualified (#1913)). The comment in store.h asks for a bump whenever the QN scheme of an already-written graph changes, and this is that case. We reproduced the consequence: an index built on main and then refreshed incrementally with this branch ends up mixed. It has 768 receiver-qualified and 2,490 flat Go method nodes, and the old fused nodes keep their stale edges. With the bump, an old index goes through the full-reindex path once. tests/test_index_format.c uses the constant symbolically, so no test changes should be needed. We'll cover the one-time rebuild in the release notes.
2. Tighten pipeline_go_method_caller_keeps_lsp_join. It asserts Install → helper by name only, but the fixture deliberately contains other.Decoy.helper, so a wrong binding to the decoy would still pass. Asserting the target file (svc/helper.go) or QN (…svc.Repo.helper) would pin it, ideally with an lsp_* strategy.

Nits, optional:

• src/pipeline/pass_semantic.c around line 264 still describes Go methods as carrying a flat QN.
• go_lsp.c builds the QN by concatenation, while the other two sites use cbm_fqn_compute_source_lang. They agree today, and the tests hold them together.

On the question we raised on 1 September, how the two changes interact: on the same corpus, #1907 plus #1913 lose about 74 true same-package edges (about 0.45% of Go CALLS). With same_module missing, those fall to unique_name, which penalises a same-package callee as not import-reachable, and #1907's guard then drops them. We think the right fix is small: treat the caller's own package as import-reachable. That can be a follow-up on top of this stack and doesn't block it.

Thank you. Once the bump and the assertion are in, this is ready.

[DeusData]

Thank you, @ilyabrykau-orca. Receiver-qualified Go method QNs fix the fused-method bug at its root, and your census on this thread is what let us take the direction decision on numbers. As on #1907, we finished the review asks on your branch ourselves so you do not need another rebase. Your commit is unchanged; ours sit on top, signed off:

1. Merged the updated fix(pipeline): suppress weak short-name matches for Go selector calls #1907 head, which carries current main (5f66c867) and the field-type-hint fix described below. The only conflict was the shared test anchor in tests/test_pipeline.c, and both sides are kept.
2. CBM_INDEX_FORMAT_VERSION 3 -> 4 (src/store/store.h; main had reached 3 since the review asked for 2), with the line 4 #1913: Go method QNs are receiver-qualified. An index written before this change now goes through one full rebuild instead of mixing flat and receiver-qualified method nodes. We will state the one-time rebuild in the release notes.
3. Tighter pipeline_go_method_caller_keeps_lsp_join. The edge is now pinned by QN on both ends (…svc.Repo.Install -> …svc.Repo.helper) with an lsp_* strategy, and the test asserts that nothing binds the decoy other.Decoy.helper.
4. One finding from our review of the updated head, fixed here. The templ grammar is Go-derived (method_declaration with a receiver), so templ methods reached the same receiver branch in extract_defs.c. They got a receiver-qualified def QN, while templ's call-scope QN (compute_func_qn) stays flat, so calls inside a templ method named a QN no node carried. The rewrite is now Go-only, and extract_templ_method_qn_matches_call_scope pins def QN == call scope.
5. A small hardening, also from that review. cbm_fqn_compute_source_lang returns NULL only when the arena is exhausted, and both receiver-qualified branches passed that NULL to %s. Both sides now fall back to the flat QN together, so a def and its calls still agree. This touches the allocation-failure path only.
6. The stale flat-QN comment in pass_semantic.c is updated. We left the go_lsp.c concatenation as it is: it matches the other two sites, and the tests hold all three together.
7. The Go doc-link layer from main now follows the new QN scheme. It is a consumer of method QNs too.
   • Its tests had pinned the fused flat node ((A) Close and (B) Close sharing alpha.Close). They now pin each receiver-qualified method node and its edge, so doc links to same-named methods of different receivers resolve.
   • Its model still let every method claim the flat node <dir>.<name>. A type, function or value sharing its name with a method in the same directory was therefore reported as a graph gap. main reports the same gap, because there the method really did take that node. So this completes your change rather than fixing anything it broke.
   • Methods no longer claim a flat name (doclink_go.c, doc_links_go.c). On terraform, the 9 graph-gap rows become 8 correct doc-link edges, e.g. [AbsProviderConfig] now lands on the type rather than the method RootProviderConfig.AbsProviderConfig. vault and prometheus are unchanged.

Your change is what turns the Go LSP's type dispatch into graph edges. On the 13 measured repos, the calls that scip-go confirms as correct lsp_type_dispatch edges go from 20,642 to 30,750, and lsp_direct from 56,924 to 60,105. Precision rises from 0.900 (#1907 alone) to 0.944.

How we measured the stack

Each of these was indexed on 13 Go repos with the parallel resolver, and on caddy + prometheus with the sequential one as well:

• main (9f85f104)
• fix(pipeline): suppress weak short-name matches for Go selector calls #1907 alone
• fix(pipeline): suppress weak short-name matches for Go selector calls #1907 + fix(extract): receiver-qualify Go method QNs #1913
• each of the two again with the fix described under "One more change"

Every repo was indexed twice per binary, and both runs were identical for every edge type. Precision and recall come from scip-go 0.2.7 as an independent oracle. Precision is counted only on the 6 repos where scip-go also records external references (on the other 7 it records none, so a call bound to the wrong project function would look correct there). Recall covers all 13.

build	precision (6 repos)	recall (13 repos)
main	0.632	0.925
#1907	0.864	0.889
#1907 + fix	0.900	0.888
#1907 + #1913	0.845	0.893
#1907 + #1913 + fix	0.944	0.873

Per repo, main -> the full stack with the fix:

repo	precision	recall
caddy	0.547 -> 0.820	0.842 -> 0.904
golang/tools	0.485 -> 0.910	0.942 -> 0.921
grpc-go	0.621 -> 0.914	0.897 -> 0.869
terraform	0.703 -> 0.981	0.894 -> 0.820
helm	0.611 -> 0.937	0.901 -> 0.922
prometheus	0.647 -> 0.970	0.929 -> 0.878

The honest part is the recall trade. Overall recall goes from 0.925 to 0.873. The largest drops are vault (0.906 -> 0.792, −11.4 points) and terraform (−7.4 points). Most of what the guard removes was wrong, but not all of it.

The true calls it loses are method calls on values whose receiver type the Go LSP does not infer: field chains (c.router.Route), method-return chains (client.Sys().Mount) and locals bound by := (runner.Stop). A weak short-name guess happened to get these right, and the guard drops such guesses by design. Of the pairs newly missed on vault, terraform, caddy and prometheus, 2 out of 4,381 are package-qualified calls; all the rest have this shape.

One caveat on reading those numbers: #1913 changes what the oracle can match. Receiver-qualified QNs let scip-go map 20,912 more caller/callee pairs (237,806 vs 216,894), so recall only compares within one QN scheme. Winning those calls back is on us, not on you. Receiver typing in the Go LSP (field chains, method-return chains, := from project constructors) is our own follow-up.

One more change: the parallel resolver's field-type hint for Go

Running both resolvers side by side showed that the stack made them disagree. Here is CALLS from the parallel / sequential resolver:

build	caddy	prometheus
main	9,964 / 9,955	38,206 / 38,067
#1907	7,814 / 7,680	30,050 / 28,406
#1907 + #1913	8,391 / 7,578	30,014 / 25,142
#1907 + fix	7,682 / 7,680	28,498 / 28,406
#1907 + #1913 + fix	7,580 / 7,578	25,220 / 25,142

The cause was ours, not yours. In the review asks we kept field_type_hint out of your guard as "receiver-aware". It is not. try_field_type_hint runs only in the parallel resolver. It capitalizes the receiver variable name and accepts any candidate whose QN contains it, so t.Run(...) on a *testing.T became T and bound a project TBench.Run. The sequential resolver dropped the same weak pick through your guard.

Receiver-qualified QNs (#1913) put type names into many more QNs, which made the hint fire far more often. The typed field call in lrp_go_s8 (s.repo.Find()) never needed the hint; the Go LSP resolves it.

The fix (e554a2ab, on #1907) has two parts:

• Go no longer takes the hint.
• The guard treats field_type_hint as weak again, like the shared list.

Both pipeline twins now assert that t.Run binds no project Run, and the parallel twin fails without the fix. On scip-go's call sites, the fix costs and removes the following:

on top of	correct calls lost	wrong calls removed
#1907	17	3,927
#1907 + #1913	2,048	12,434

No Go field_type_hint edges are left.

Thank you again for the care in this stack, and especially for chasing down the third QN site on your own.

[DeusData]

Merged current main (1346ac4e, which now includes #2681 and #2686) so the branch is mergeable again. The only conflict was the test registration list in tests/test_registry.c, and both sides are kept. Affected suites are green locally; CI is re-running.

[DeusData]

CI caught two PDF doc-link tests (tests/test_doc_links_pdf.c:1092, :1175) that still named a Go method by its flat QN (pkg.server.Serve, pkg.time.Now). With the receiver-qualified scheme that node is pkg.server.Handler.Serve / pkg.time.Provider.Now. aa4775f updates the three lookups. That includes the time.Now must-not-bind check, which would otherwise have passed vacuously against a node that no longer exists. The PDF resolver needs no change: a Go method now binds through its QN like any other member. The rule for owner-less QNs stays for templ methods, and its comment is updated. The branch also takes the refreshed #1907 base (current main 658639e). The whole doc-link family (15 suites, 282 tests) is green locally; CI is re-running.