Skip to content

fix(pipeline): bind field_type_hint by owning QN segment, not substring - #1936

Merged
DeusData merged 5 commits into
DeusData:mainfrom
ilyabrykau-orca:fix/go-field-type-hint-owner-segment
Oct 31, 2026
Merged

DeusData merged 5 commits into
DeusData:mainfrom
ilyabrykau-orca:fix/go-field-type-hint-owner-segment

Conversation

@ilyabrykau-orca

@ilyabrykau-orca ilyabrykau-orca commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1927.

What

try_field_type_hint() (parallel resolver) accepted any candidate whose qualified name merely contained the hinted type name as a substring. A single-letter receiver — f.Close(), hint "F" — matched almost every same-named candidate (the method name itself included) and rebound a call whose resolution had already failed to an arbitrary project symbol at confidence 0.85, presented as field_type_hint, the one weak strategy the Go selector guard (#1906) deliberately trusts.

This PR requires the candidate's owning dot-segment — the segment immediately before the method — to equal the hinted type (or its I-prefixed interface form): fth_owner_segment_is() replaces the two strstr() calls.

Measured effect

On the real ~1150-file Go+C repo from #1927, measured with this exact change on the #1906/#1909/#1910 stack (the same table is in the issue body):

strategy before after
field_type_hint 1484 356 (−76%)
single-character receivers 376 0
Go → C/C++ targets 34 0
every other strategy, incl. all lsp_* — unchanged

Nothing relocated: the removed edges were fabricated on top of already-failed resolutions, so removing the substring anchor leaves nothing behind. 63 event.Get* edges moved to the interface that declares the method instead of an arbitrary implementation.

Tests

  • Reproduce-first: pipeline_go_parallel_field_hint_requires_owner_segment (parallel-path fixture, ≥50 files). RED before the fix on this branch: FAIL tests/test_pipeline.c:4941: ASSERT(!(cross_file_call_exists(s, project, "Lookup", "Close"))) — the stdlib f.Close() bound to FileStore.Close because "F" ⊂ the QN. GREEN after.
  • Negative probe (single-char receiver → zero project edges) and positive control (finder.Find() → Finder.Find still hinted) in the same test.
  • lrp_go_s8_field_type_hint stays green — its repo/Repo fixture is segment-exact, the case the hint exists for.
  • Full scripts/test.sh venue leg (ASan+UBSan, all suites + contract steps): green. git clang-format --diff: clean.

Stacking

Stacked on #1907 → #1913 → #1915 (same pattern as those PRs): the test's absence assertions rely on #1907's Go selector guard downstream of the hint, and Go owner segments are receiver-qualified by #1913. The tip commit is the only new change; it applies to main independently.

#1932 tracks the family; #1927 is the direct consequence of #1907/#1913 moving weak-strategy mass into field_type_hint.

@github-actions

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor Author

CI retrigger (no-op amend, identical tree — no rerun rights): the failing job was test-windows CLANG64 2/2 at test_subprocess.c:688 ASSERT(ready) in subprocess_windows_job_object_cancellation_quiesces_descendant_tree — a Windows Job-Object readiness probe with no overlap with this PR's diff (parallel resolver + pipeline test only). No semantic changes.

@ilyabrykau-orca

Copy link
Copy Markdown
Contributor Author

CI retrigger (no-op amend): test-windows-guards again, and this run's log shows the whole Windows harness down — SETUP FAIL: permanent daemon did not start for the crash check, index did not run, ASCII control did not index via CLI — before any guard executed. Tracked with a run matrix in #1952. No semantic changes.

@ilyabrykau-orca
ilyabrykau-orca force-pushed the fix/go-field-type-hint-owner-segment branch from 0f2024c to 6aac10c Compare August 30, 2026 20:26
@DeusData

Copy link
Copy Markdown
Owner

Heads-up: this went red-to-merge because #1937 (fcd8bfc1) and #1940 (61de19bb) landed, not because of anything you changed.

Checked with git merge-tree against current main: the conflict is exactly one file, tests/test_pipeline.c, and it is insertion-anchor drift — both merges added tests to it. Every source file in your diff auto-merges cleanly.

Re-anchor your new test and its RUN_TEST line and the rebase is done. Ping when it is up.

@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Sep 1, 2026
@ilyabrykau-orca
ilyabrykau-orca force-pushed the fix/go-field-type-hint-owner-segment branch from 6aac10c to 27d4d34 Compare September 2, 2026 07:48
@ilyabrykau-orca

Copy link
Copy Markdown
Contributor Author

Rebase is up, as requested. Exactly what your merge-tree predicted: tests/test_pipeline.c insertion-anchor drift only — the new test and its RUN_TEST line re-anchored after the #1937/#1940/#1944 additions; all source files auto-merged. Full scripts/test.sh (ASan+UBSan) green on this head, which carries the whole 1907→1913→1915→1936 stack.

try_field_type_hint accepted any candidate whose qualified name merely
CONTAINED the hinted type name. A single-letter receiver (f.Close(),
hint "F") matched almost every same-named candidate - the method name
itself included - and rebound a call whose resolution had already
failed to an arbitrary project symbol at confidence 0.85, presented as
the one weak strategy the Go selector guard deliberately trusts.

Require the candidate's owning dot-segment - the segment immediately
before the method - to EQUAL the hinted type (or its I-prefixed
interface form). The intended variable-named-after-its-type case keeps
resolving (lrp_go_s8_field_type_hint stays green); the fabricated
bindings lose their only anchor and fall back to the weak short-name
strategies the guards already handle.

Measured on a real ~1150-file Go repo (with the DeusData#1906/DeusData#1909/DeusData#1910
fixes applied): field_type_hint 1484 -> 356 CALLS edges, the
single-character-receiver class 376 -> 0, Go->C/C++ targets 34 -> 0,
and no other strategy absorbed the removed edges. 63 edges moved to
the interface that declares the method instead of an arbitrary
implementation.

Fixes DeusData#1927

Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
@ilyabrykau-orca
ilyabrykau-orca force-pushed the fix/go-field-type-hint-owner-segment branch from 27d4d34 to ac46c17 Compare September 2, 2026 10:01
@DeusData

DeusData commented Sep 3, 2026

Copy link
Copy Markdown
Owner

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.

@DeusData

Copy link
Copy Markdown
Owner

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 → 1915 → 1936 as you have 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; I will ping here once #1915 is in, and a rebase of this branch onto it is then the only step left before review. Thank you for your patience with the long hold — the stack has been rebased cleanly every time we asked, and that is not lost on us.

@DeusData

Copy link
Copy Markdown
Owner

Thanks again for the owner-segment regression and for holding the stack. The recent October 18 update on #1907 now disables the field-type hint for Go, so this PR needs a maintainer scope check against that newer direction before more rebase work. No contributor rework is requested until that is reconciled. Sorry for the delay—this is waiting on maintainer follow-up, and I’ll follow up soon.

Resolve tests/test_pipeline.c as main's file plus this PR's own test hunk; the RUN_TEST line is re-anchored before pipeline_go_multi_init_nodes_survive.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Go no longer runs the parallel field-type hint (e554a2a), so the Go test
of this PR passes with or without the owner-segment fix. C# still reaches
try_field_type_hint, and its PascalCase method names make every candidate
QN contain a one-letter receiver's hint: `w.Write()` on a StreamWriter was
upgraded to an arbitrary project Write as field_type_hint. The test is RED
on main's pass_parallel.c and GREEN with the fix, and its positive control
pins `store.Put()` to Store.Put, never FileStore.Put.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData

Copy link
Copy Markdown
Owner

Thank you, Ilya, for this fix and for your patience while the 1907 -> 1913 -> 1915 stack landed. #1915 is now on main, so we merged current main into your branch: the three stacked commits drop out because main already contains them, and the diff against main is exactly your owner-segment change in pass_parallel.c plus its test (the only conflict was the RUN_TEST line in tests/test_pipeline.c).

Since you wrote this, main stopped running the field-type hint for Go altogether and the Go selector guard now treats it as a weak name guess, so the Go symptom you diagnosed is closed by a different route and your Go test passes with or without the fix today. Your fix still matters for every other language on the parallel path, so we added a C# test on top (afb499b): a one-letter receiver such as w.Write() was upgraded to an arbitrary project Write because "W" is a substring of every candidate's method name. The test fails on main and passes with your change, and store.Put() still binds Store.Put and never FileStore.Put. The pipeline, parallel and extraction suites pass on the merged tree.

We will merge once the required checks are green. Please pull before pushing again, since the branch now carries a merge commit and a test commit from us.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Resolved tests/test_pipeline.c as the main version plus this PR diff: the owner-segment field-hint tests follow the new bare init tests, and both RUN_TEST pairs stay registered (pass_parallel.c merged cleanly).

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData
DeusData merged commit a6d0d6c into DeusData:main Oct 31, 2026
26 checks passed
@DeusData

Copy link
Copy Markdown
Owner

Merged as a6d0d6c - thank you, Ilya! On the parallel path, a field-type hint now upgrades a call only when the receiver type is a whole owner segment of the candidate, so a one-letter receiver such as w.Write() no longer binds to an arbitrary project Write just because "W" appears in its name. Thank you for the precise diagnosis, the Go test, and your patience while the 1907 -> 1913 -> 1915 stack and today's Go init rule (#2862) landed around it; we merged main in once more after #2862 touched the same test file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(pipeline): field_type_hint binds by receiver variable-name substring, not declared type — 1484 edges at conf 0.85 on one Go repo

2 participants