fix(extract): give each Go init() its own QN - #1915

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.

Rebased onto current main (stacked on the rebased #1913). No semantic changes; full test leg green. Post-fix init-node recovery (29/29) is in the #1906 census comment.

Reviewed the last commit. The diagnosis is right, the safety argument holds, and I have one substantive concern about the suffix itself.

The diagnosis is confirmed by the schema. src/store/store.c:257 declares UNIQUE(project, qualified_name) and the node upsert at :2364 is ON CONFLICT(project, qualified_name) DO UPDATE SET. So a flat proj.pkg.init really does mean every init in a package is the same row, and the last writer wins — which is exactly your 21 → 1.

Your safety claim holds, and I checked it both ways. Nothing in internal/cbm/, the LSP resolvers, or src/pipeline/ matches on the literal "init", so no resolver joins on the plain QN. And since calling init explicitly is illegal in Go, no call site produces init as a callee either. Both directions are clear.

The precedents you cite are real. extract_defs.c:2079 is the Rust cfg twin using "%s#%s", and :3544–:3613 is the GoogleTest derivation, whose comment names the same failure — "causing qualified-name collisions and node loss (#1266)". The shape is established house practice for this problem class.

The concern: the suffix is not stable, and both precedents are

This is the part I would like you to weigh, because it is the one property those two precedents share that yours does not.

The Rust cfg twin derives its suffix from the cfg(...) predicate text, and says why in the code:

Build a compact predicate suffix from the cfg(...) text, dropping whitespace and quotes so the QN stays readable and stable.

GoogleTest derives its from the macro arguments — TEST(Suite, Case) → TEST_Suite_Case. Both are semantic content that does not move when the file is edited.

proj.pkg.init#a.go:L5 embeds a line number. Add an import above that init and its qualified name changes — and since the QN is the node's identity under that UNIQUE constraint, an incremental reindex sees the old node vanish and a new one appear, for an edit that did not touch the function. That is node churn on every unrelated edit above any init, and it lands on the delta-merge path rather than a full rebuild.

The file basename alone will not do it — your own extract_go_multiple_init_disambiguated test requires two inits in one file to differ, and only the line separates them today. But a per-file ordinal would: proj.pkg.init#a.go:1, #a.go:2. That still satisfies that test, and it only churns if you reorder the inits in a file rather than every time you add a line above one.

I am raising this rather than ruling on it — you may have considered ordinals and rejected them for a reason I cannot see from here. If so, say why and I will take it.

Also

The whole stack is currently DIRTY — #1907, #1913 and this one all have conflicts. main was broken for a few hours today (duplicate struct members merged into type_registry.h) and was repaired an hour ago by #1993, so now is a good moment to rebase all three.

Thanks for measuring the loss rather than describing it — "21 decoder-registration inits in one package → 1 node; 29 in the repo → 7 nodes" is what makes this obviously worth fixing.

Rebased onto current main as part of the 1907→1913→1915→1936 stack; only test insertion-anchor drift in tests/test_pipeline.c, re-anchored. Full scripts/test.sh green on the stack head.

You are right, and the ordinal is strictly better — implemented, not argued with.

I had not weighed the incremental path when I chose the line number: on a full rebuild the line suffix is merely ugly, but under delta-merge the QN is node identity, so an insertion above any init would delete-and-recreate the node on an edit that never touched it. Both precedents being edit-stable is not a coincidence; it is the property that makes a QN suffix safe.

Now: pkg.init#file.go:N, where N is the 1-based source-order ordinal among the file's top-level init function_declarations. One formula, computed from the tree by a shared cbm_go_init_ordinal (helpers.c) on both the definition side and the call-scope side (compute_func_qn), so the two cannot drift. Reordering the inits within a file still churns — that is inherent to any content-free discriminator — but nothing else does.

The test now pins the property you asked for, not just distinctness: exact QNs t.init#x.go:1 / t.init#x.go:2, then the same file re-extracted with an import and a declaration inserted above both inits, asserting both QNs are byte-identical to before.

Branch force-pushed with the stack rebase; full scripts/test.sh green on the stack head.

Status pointer, so this thread is not left hanging: the stack is now waiting on a direction call about the Go qualified-name shape surfaced by the #1913 census — details on #1913. Please hold the rebase of this branch along with the rest of the stack until that lands; I will update here.

The update I promised on 3 September, late and with apologies for that: the direction call on #1913 landed on 15 September (the receiver-qualified shape stays, stack order 1907 → 1913 as you already rebased it), so nothing is waiting on a decision any more. #1907 and #1913 are next in line for a branch update and a fresh CI run as soon as the hosted-runner queue drains from tonight's merges; once #1913 is in I will ping here, and a rebase of this branch onto it is then the only step left before review. Thank you for holding the stack steady through all of this, and for the ordinal fix — that reasoning about delta-merge identity was exactly right.

Thank you, @ilyabrykau-orca, for this one and for holding the whole stack steady while #1913 was decided. Stack A is in (#1907 and #1913 landed as 5a4af9c), so instead of asking you for one more rebase we finished the last steps on your branch ourselves, as two commits on top of yours. Your commits are untouched.

You got the heart of it right: one node per init(), an edit-stable per-file ordinal instead of a line number (your reasoning about delta-merge identity was exactly right), and the call-scope mirror in compute_func_qn. We checked it on kubernetes/kubernetes: the graph now holds all 1,024 init functions instead of 700, which matches the source exactly (the only other "func init()" lines there sit inside code-generator templates), and each of the 21 files in code-generator's validators package now shows its own init() calling RegisterTagValidator.

1. 776f6f5 - Merge origin/main into fix/go-init-qn. Your branch still carried the pre-rebase copies of the fix(pipeline): suppress weak short-name matches for Go selector calls #1907 and fix(extract): receiver-qualify Go method QNs #1913 commits. Every one of those hunks resolves to main's final version, so the merged tree is main plus exactly your commit. Your production hunks apply unchanged, and your two tests sit next to the newer Go tests main added at the same spots, with identical bodies.

2. 6e784c3 - fix(lsp): name Go init() callers per file in the Go LSP. This is the third side of the same contract you handled for methods in fix(extract): receiver-qualify Go method QNs #1913. The Go LSP builds its own enclosing-function QN in process_function (internal/cbm/lsp/go_lsp.c) and still produced the flat pkg.init, while the pipeline joins LSP results to textual calls by exact caller QN. So every LSP resolution inside an init body missed the join. On kubernetes, all 499 LSP-resolved calls from init bodies fell back to the name-based resolver, and all 562 function-value references from them (scheme registrations, for example) lost their precise edge. The commit:

• moves the formula into one helper, cbm_go_init_qn, used by the def extractor, compute_func_qn and the Go LSP, so the three cannot drift;
• passes the file path into the Go LSP (GoLSPContext.rel_path, plus a rel_path parameter on cbm_run_go_lsp and both cbm_run_go_lsp_cross entry points, wired through the cross-file dispatch including its per-file fallback);
• computes the ordinal with a tree cursor, because ts_node_named_child(root, i) costs O(i) per call and made the loop quadratic on very wide generated files;
• keeps the flat QN when the base QN is NULL (allocation failure only), like the receiver branch next to it.
  New tests cover the per-file and both cross-file LSP paths, the pipeline in its sequential and parallel venues (three inits in two files, where each init's calls and its function-value reference must land on its own node), and the fallback dispatch. Each one fails with its piece reverted, and the pipeline tests now also cover your compute_func_qn mirror, which no test exercised before.

With both commits, the init edges on kubernetes match main's resolution mix exactly, and the 104 calls that main had merged into one shared init node per package now each start at their own init. This rides the index-format change from #1913, so users who upgrade rebuild once and get both.

Please pull before pushing to this branch again, so these two commits stay on it. Thank you again for measuring the loss instead of describing it, and for the ordinal change, which is what made this safe to land.

Thank you for your patience, @ilyabrykau-orca. CI went green on this branch, but this evening's merges on main touched files this pull request also changes, so that green no longer covered today's main. We merged current main into the branch once more (one merge commit, a3ff99d; your changes are untouched) and will merge once the fresh checks are green. Please pull before pushing again.

Thank you for your patience, @ilyabrykau-orca. A pull request that merged into main a few minutes ago (#2290) changes internal/cbm/cbm.c, which this pull request also changes, so the CI run in progress here could no longer vouch for today's main. We merged current main into your branch once more (one merge commit, 19b57f3; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

Thank you so much for this, @ilyabrykau-orca - giving every Go init() its own per-file QN is exactly the right fix, and your tests made it easy to verify once more.

Main moved on after #2769 (the Go LSP now follows a type alias declared in another file of the same package) landed, so we merged current main into your branch (merge commit 7a9e6c6, signed off). Two things needed care:

1. The only textual conflict was the RUN_TEST list in tests/test_pipeline.c, where both changes added Go tests at the same spot. We kept main's file, re-applied your additions unchanged, and registered your three tests right before the new alias tests.
2. Your branch adds a rel_path parameter to cbm_run_go_lsp_cross and cbm_run_go_lsp_cross_with_registry, and the two new alias tests from fix(go-lsp): follow a type alias declared in another file of the package #2769 still called them the old way, so the merged tree did not compile. Those two calls now pass NULL for rel_path, the same way you updated the existing calls. No production code changed in the merge.

On the merged tree the go_lsp, extraction, pipeline, lsp_resolution_probe, edge_types_probe, c_lsp and parallel suites all pass, including all seven of your new tests and the four new alias tests. A fresh CI run is on its way; we will merge once it is green. Please pull before pushing again so the merge commit is kept.

Thank you for your patience, @ilyabrykau-orca. Main has moved since the last CI run on this branch, and some of the new commits change internal/cbm/extract_defs.c, which this pull request also changes, so that run can no longer vouch for today's main. We merged current main into your branch (one merge commit, 7ee29d8; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

Thank you for your patience, @ilyabrykau-orca. The macOS legs of the last CI run here were cancelled by GitHub itself: it is retiring its macOS 14 runners on November 2 and ran scheduled brownouts beforehand, so those jobs never got a runner. Nothing in your change caused it. Main now runs CI on macOS 15 (#2849), so we merged current main into your branch (one merge commit, 8c62cb3; it merged cleanly and your own change is untouched) to get a run on the new runners. Please pull before pushing again.

Merged as 9faa3e5 - thank you, Ilya! Each Go init() now gets its own per-file qualified name, so a package with several init functions keeps every body and every registration edge instead of collapsing them onto one node. Thank you for the careful fix and the tests, and for your patience through all the main merges along the way. We will pick up your stacked follow-up #1936 next.