fix(pipeline): key the Go Field guard on the recorded selector shape, not reference text

[ilyabrykau-orca]

Summary

Fixes the defect you found in post-merge review of #1944: cbm_go_suppress_bare_field_ref's strchr(ref_name, '.') test is inert because the extractor strips the receiver on every path that reaches the resolver — resolve_lhs_write_name records the trailing field name of a selector LHS, and is_reference_node records the inner field_identifier. The guard was therefore a blanket veto: every Go Field node was unreachable by USAGE/READS/WRITES, genuine selector references included.

As #1962 says, the signal the guard wants is computed at extraction time and discarded. This PR records it and threads it through:

• CBMUsage / CBMReadWrite gain is_member_access. The usage recorder sets it when the reference node is a field_identifier (the member half of a selector); resolve_lhs_write_name reports it through an out-param when it takes the field/member LHS branch. The receiver is stripped either way — the flag is the only surviving record of selector shape.
• cbm_go_suppress_bare_field_ref(is_go, is_member_access, target_label) now refuses a Field bind only for references that were never the member half of a selector. All four resolver sites (sequential + parallel, USAGE + READS/WRITES) pass the recorded flag. Still Go-gated — the C#/Java/C++/Python bare-member shapes stay untouched (cp_reads_writes_cs_static_field still pins that).
• The dead ASSERT_FALSE(..., "t.err", "Field") unit case — an input Go's extractor cannot produce — is replaced by flag-based cases, so the unit test now pins reachable behavior.

RED / GREEN (reproduce-first)

Extended the #1942 fixtures with a sibling-file method doing genuine selector references (t.err = nil, return t.n). With the blanket veto restored, both resolver twins go RED at the new asserts:

FAIL tests/test_pipeline.c:4913: ASSERT(cross_file_edge_exists(s, project, "Reset", "err", "WRITES"))
FAIL tests/test_pipeline.c:4948: ASSERT(cross_file_edge_exists(s, project, "Reset", "err", "WRITES"))

With the fix, both twins are GREEN and the original bare-local negatives (Run's err := … must not bind the field) still hold. Full scripts/test.sh (ASan+UBSan) green.

Field census (same repo as the #1937/#1940/#1944 numbers)

Your cheap falsification, run on current main: zero Field-targeted USAGE/READS/WRITES edges (4497 Go Field nodes, reachable only by CALLS/DEFINES/TESTS) — the prediction was exact.

With this fix, same repo:

	main	this PR
USAGE → Go Field	0	8503
WRITES → Go Field	0	1368
distinct Field nodes reached	0	2220 of 4497

For scale: the pre-#1944 bare-local pollution was 21308 USAGE / 5191 WRITES. The recovered population is the genuine selector subset, not a return of the noise — the bare-local negatives in the fixtures and the flag's default-false both hold that line.

Fixes #1962.

[DeusData]

Approved. I verified the inertness claim against main rather than taking it, and it holds exactly:

• extract_semantic.c:181-187 — resolve_lhs_write_name matches selector_expression/field_access, takes the field (or name) child, and returns cbm_node_text of that node alone. t.err = nil records err.
• extract_usages.c:113/117/121 — is_reference_node accepts field_identifier, the member half of the selector. Same result: err.

So strchr(ref_name, '.') is unconditionally NULL on both paths that reach the guard, cbm_go_suppress_bare_field_ref returns true for every Field target, and no Go struct field was reachable by USAGE/READS/WRITES at all. Your census is the confirming measurement: zero such edges across 4497 Go Field nodes on current main.

The dead test is the real lesson here

tests/test_registry.c:848:

/* A selector-shaped reference may bind a field. */
ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, "t.err", "Field"));

That assertion passes. It has always passed. And it is worthless, because "t.err" is an input the extractor cannot produce — the receiver is stripped before the resolver ever sees the name. The guard had a unit test covering precisely the case that mattered, the test was green, and the behaviour it described never occurred in production.

That is why this survived #1942 and #1944. A test written against a hypothetical input gives exactly the confidence of a test, and none of the coverage. Replacing it with flag-based cases that pin reachable behaviour is the most valuable change in this PR, more than the edges it recovers.

It is also the second time in two days this repo has produced this shape — the other was a suite whose RUN_TESTs sat in a decoy suite that never ran. Worth remembering as a class: when a guard has a passing test and is provably inert, suspect the test's inputs before the guard's logic.

On the fix itself

Recording the signal instead of reconstructing it is the right call. #1962 identified that selector shape is known at extraction time and thrown away; the alternative — reconstructing "was this a selector?" downstream from a name with the receiver already gone — is not recoverable at all. There is no clever resolver-side fix here, which is what makes threading the flag the honest answer rather than the lazy one.

A distinct is_member_access rather than overloading an existing flag matches the precedent set when callee_is_locally_bound was kept separate from is_method: is_method has a documented contract and other readers, and widening it to mean two things corrupts them silently.

Still Go-gated, with all four sites updated. Sequential and parallel, USAGE and READS/WRITES — the twin-resolver split is the standing trap in this area and you covered both, with cp_reads_writes_cs_static_field still pinning that C#/Java/C++/Python bare-member references are untouched.

I also confirmed CBMUsage/CBMReadWrite are in-memory extraction structs — neither appears in sqlite_writer.c or artifact.c — so the two new fields need no index-format bump. Worth stating explicitly in the description, since a reader seeing struct growth will ask.

Before merge

CI is still queued. Once it is green I will merge — this recovers ~10k edges that were being silently discarded, which is squarely graph-quality-first.

Thank you for taking the post-merge finding and coming back with the falsification run first. Reporting the census on main as zero, and calling it "your cheap falsification", is the part that made this reviewable in one pass: you proved the bug existed before proving your fix removed it.

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

Merged as 192f2960. This recovers roughly 9,900 edges that were being silently discarded — 8,503 USAGE and 1,368 WRITES — and it is the most consequential graph-quality fix I have merged today.

The finding I keep coming back to is not the edges, though, it is why this survived two prior PRs. The guard had a unit test for exactly this case:

ASSERT_FALSE(cbm_go_suppress_bare_field_ref(true, "t.err", "Field"));

Green, always — and worthless, because "t.err" is an input the extractor cannot produce. resolve_lhs_write_name returns the text of the field node alone, and is_reference_node records the inner field_identifier; both strip the receiver before the resolver ever sees the name. So strchr(ref_name, '.') was unconditionally NULL and the guard was a blanket veto over every Go Field node — 4,497 of them, reachable by nothing.

Replacing that assertion with flag-based cases that pin reachable behaviour is worth more than the edges, because it is what stops the next version of this bug. I have written the pattern down on our side: when a guard is provably inert, suspect the test's inputs before the guard's logic, and give any guard whose effect should be selective a census — 0 or all is the tell. Your "cheap falsification" run was exactly that census, and you did it before writing the fix.

Recording the signal at extraction time rather than reconstructing it downstream was the right call too: once the receiver is stripped, "was this a selector?" is not recoverable, so there was no clever resolver-side alternative to find.

Before merging I confirmed the 29-commit gap to main was inert — no commit had touched any of your nine files since your base — so the green described exactly what landed.