[BobbieBarker]

This PR was written by an AI agent working on my behalf. I certify the DCO sign-off on the commit and answer for the change.

Follow-up to #2370. The registry scores candidates on CBMDefinition.is_test instead of scanning the qualified name, and is_test_qn is deleted.

is_test_qn matched test, Test, mock, stub, spec, fake or Fixture anywhere in the QN, and REG_TEST_PENALTY uses that to pick which definition an unresolved call resolves to. On one 970-file Elixir tree it calls 794 of 20,882 Function and Method nodes test code. 516 match on spec, 268 of those only because inspect contains it: inspect_optional, inspect_record, inspection_actions. latest and contest match test the same way, and a test helper under a directory spelling none of those words is scored as production.

Cost

Four corpora, a binary from this commit's parent and one from this branch, edges diffed on (source QN, target QN, type):

corpus	moved	retargeted	lost (resolution)	lost (similarity)
Elixir, 970 files	920	919	1	0
ripgrep 14.1.1	353	193	30	130
tokio 1.40.0	1,548	1,426	86	36
rust-analyzer 2024-09-30	5,160	4,817	219	124

336 resolution edges are lost across the four, where the new winner is refused by a check the old one passed. The similarity column is SEMANTICALLY_RELATED, which the post-passes rebuild wholesale and which moves on its own. acceptance_spec_runner is a typical retarget: result resolved to critique_report.result and now resolves to operator_inspect.reader.result, the function it calls.

This is a behaviour change in every indexed project, not a signature change.

Ripples

candidate_score loses its -1: ask tri-state and qn_test_flags stops returning NULL, since there is nothing left to recompute from.

Both filtered resolve paths now carry verdicts with their candidates. resolve_multi_with_imports copied a parallel array but guarded it on flags ?; filter_import_reachable carried none, so the fuzzy path passed NULL whenever it had filtered and fell back to the scan silently.

index_under_name drops an entry whose verdict it cannot store rather than leaving the array short, because a missing verdict scores as non-test and would outrank real code.

pipeline_incremental.c seeds from persisted nodes with no CBMDefinition, so it reads the flag out of properties_json via cbm_node_is_test, exported from pass_tests.c.

Tests

REG_TEST_PENALTY had no direct coverage: of the 95 cbm_registry_add call sites in the suite, none registered a symbol the scan would flag, so the suite passes with the scan and without it.

registry_test_verdict_comes_from_the_caller_not_the_qn discriminates: proj.latest.parse against proj.harness.parse. With the scan restored and the signature kept it fails on "proj.harness.parse" != "proj.latest.parse". The other two pin that the penalty exists and that it orders candidates without vetoing them.

144 of 144 suites, 8,254 passed, 0 failed, 10 skipped.

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

[DeusData]

Thank you for quantifying the retargeted and lost edges instead of presenting only a green suite. I checked is_test_qn and candidate scoring on current main; the name-based classification is indeed part of candidate selection. Because this changes winners across languages and indexing modes, we need more time to review the recorded edge changes and incremental behavior before deciding. It is tagged as a parsing-quality bug for deeper review. Thank you for your patience with the full queue.

[BobbieBarker]

Thank you for quantifying the retargeted and lost edges instead of presenting only a green suite. I checked is_test_qn and candidate scoring on current main; the name-based classification is indeed part of candidate selection. Because this changes winners across languages and indexing modes, we need more time to review the recorded edge changes and incremental behavior before deciding. It is tagged as a parsing-quality bug for deeper review. Thank you for your patience with the full queue.

No problem, I think you're doing a great job considering the popularity of the repo and how hard OS is to do right. 👍

[DeusData]

Thanks for the kind follow-up and for quantifying the candidate retargets. The cross-language scoring and incremental behavior still need maintainer review. No further work is requested from you in this sweep.

[DeusData]

Thank you for the test-verdict scoring change and the measured retargets. The cross-language and incremental review remains on us, with no additional request for you. Sorry for the delay—I’ll follow up soon.

[DeusData]

Thank you, Chad, for this careful change, and for measuring it instead of trusting a green suite. You are right about the root problem: an unanchored substring scan over the qualified name is wrong in both directions, and inspect, latest and contest are exactly the false positives that make it unreliable. Taking the extractor's verdict instead of re-deriving it is the right foundation, and carrying the verdicts through the filtered resolve paths fixes a real silent fallback. Sorry it took us this long to come back with a decision; this is the follow-up we promised.

The decision: we want this approach. CBMDefinition.is_test becomes the verdict and is_test_qn goes away. We also measured your branch against its base on a large Go project (gardener, about 2,400 non-test Go files) and found one effect your four corpora could not show. Go projects keep many test doubles in ordinary, non-test files: generated fake clientsets, mock packages and mock_*.go files. The old name scan happened to rank those below real code; the extractor's verdict does not mark them as test code. With the change, about 570 call edges newly resolve into fake or mock files (73 of them from production code, for example an init() whose AddToScheme call now lands in the generated fake clientset), and none move out of them. About 3% of all CALLS edges move on that project. So we would like one anchored rule on top of your verdict, and then a port onto current main:

1. Keep CBMDefinition.is_test as the primary verdict, exactly as you have it, and keep the deletion of is_test_qn.

2. Add a test-double path rule, anchored to whole path segments and file-name prefixes rather than substrings, so it can never match inspect, latest or fakeroot. A candidate counts as test code for this penalty when the repo-relative path of its defining file has a directory segment named exactly fake, mock, mocks, testing or testutil, or when the file name matches mock_*.go or fake_*.go. That is the complete list.

3. Keep the rule in one predicate that takes the defining file's path, next to the scoring in src/pipeline/registry.c and declared in pipeline.h, and have all three registry seeders pass the extractor verdict OR that predicate: pass_definitions.c and pass_parallel.c with the definition's file path, and the incremental seeder in pipeline_incremental.c with cbm_node_is_test(node) and node->file_path. One predicate keeps the full and incremental registries answering the same way. Please keep it out of cbm_is_test_path and cbm_is_test_file: those decide the stored is_test property, TESTS edges and importance, and a generated fake clientset is not a test. This rule only changes how a candidate is ranked when a call is resolved, never what a node is.

4. Port the branch onto current main (rebase or merge, whichever you prefer, with sign-offs as before). The registry has changed a lot since your base and five files conflict today, mostly registry.c. The main differences:

   • registration now goes through cbm_registry_add_lang_in_module (language plus module prefix), with cbm_registry_add and cbm_registry_add_lang as thin wrappers, so is_test needs to be threaded through that function;
   • each name bucket now carries a lang byte array beside is_test, grown in lockstep and sharing is_test_cap, so your drop-on-failed-growth rule in index_under_name should cover both arrays;
   • best_by_import_distance now takes the registry and the language array, and more resolve paths call it. The fuzzy path on main still passes NULL verdicts whenever it has filtered by imports, so your filter_import_reachable fix is still needed and should carry the languages along with the verdicts.

5. Tests that pin the rule, next to your three registry tests:

   • a table test of the predicate: positives such as pkg/client/fake/clientset.go, fake/x.go, internal/mocks/store.go, pkg/testing/fixtures.go, internal/testutil/helpers.go, pkg/mock_store.go and pkg/fake_clock.go; negatives such as pkg/inspect/x.go, pkg/latest/x.go, pkg/fakeroot/x.go, pkg/mockery/x.go, pkg/testingutil/x.go, pkg/mocking.go and pkg/mock_store.py;
   • a resolution test in which a production caller has two same-named candidates, a real one and one defined under fake/ whose extractor verdict is false, and the real one wins, through both the full and the incremental registry (the incremental one is where a missed seeder would show);
   • please check that each new test fails with the rule removed, as you did for registry_test_verdict_comes_from_the_caller_not_the_qn, and that its RUN_TEST line is in the suite.

What we will do on our side:

• Once the ported head is up, we will re-run the same gardener comparison on it and post the numbers here, so you do not need a large Go corpus yourself.
• If the port turns out to be painful, just say so. Maintainer edits are enabled on your branch, so we can take the main merge, or the whole port, on top of your commits with your authorship kept.

Thank you again for the thorough work, and for your patience with the queue.