Skip to content

test(resolution): guard nested receiver call evidence (#1794) - #1844

Closed
bompus wants to merge 3 commits into
colbymchenry:mainfrom
bompus:fix/upstream-nested-call-evidence
Closed

test(resolution): guard nested receiver call evidence (#1794)#1844
bompus wants to merge 3 commits into
colbymchenry:mainfrom
bompus:fix/upstream-nested-call-evidence

Conversation

@bompus

@bompus bompus commented Sep 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

The fix for #1794 landed upstream in #1862. This PR now carries only the regression test written for it.

The test runs for ts, tsx, js and jsx. Each case calls .get(readKey()) on a nested receiver: optional access, a computed key, a numeric index, a quoted key, a comment inside the receiver, a non-ASCII name, an imported root and a namespace import. It checks that the readKey() argument call survives and that none of these calls binds to the imported object or to an unrelated project get method.

Upstream has no test for #1794, so nothing currently stops the #1790 regression from coming back.

Validation

Merged with upstream e63fe2ec on WSL/Linux, Node 24, native kernel: resolution.test.ts -t 1794 passes 4 of 4.

Related: #1566, #1790, #1794, #1862.

@danusha2345 danusha2345 mentioned this pull request Sep 11, 2026
Closed
This was referenced Sep 11, 2026
Closed
Merged
bompus added a commit to bompus/codegraph that referenced this pull request Sep 11, 2026
@bompus
docs(design): kernel-only extraction, resolution binding model, green…
0331195 
…field Rust sketch (#16)

Three linked design docs recording the 2026-09-11 assessment of a Rust
rewrite against the fork's PR history, the espn-draft retrieval research
and the query audit log:

- kernel-only-extraction-plan.md: remove the WASM path; phases for the
  error-recovery flip, SFC extractors calling the kernel, a parse-tree
  service for the three read-time consumers, tail-language port/drop
  decisions, and golden dumps replacing the parity oracle.
- resolution-binding-model-plan.md: a per-file bindings table emitted by
  extraction replaces the five regex families that answer export and
  binding status (colbymchenry#1721) and ends the colbymchenry#1566/colbymchenry#1790/colbymchenry#1794/colbymchenry#1844 cycle.
- greenfield-rust-core-sketch.md: what a from-scratch core must
  reproduce, what it would buy and cost, and why it is deferred behind
  the two incremental plans.
bompus added a commit to bompus/codegraph that referenced this pull request Sep 12, 2026
@bompus
test(eval): precision scoring and a pinned-corpus edge gate for resol…
4514754 
…ution

Phase 0 of docs/design/resolution-binding-model-plan.md.

The evaluation runner scored recall only, so a wrong edge — the thing
the resolution PR chain (colbymchenry#1713, colbymchenry#1718, colbymchenry#1746, colbymchenry#1844) exists to remove —
was invisible to it. scoring.ts gains scoreEdgeCase: an EdgeCase names
a kind, a target endpoint (file suffix + symbol) and optionally a source,
and expects the edge absent (known-wrong) or present (control).
evaluation-edge-scoring.test.ts pins it on a synthetic project with the
bare-import and sealed-module shapes and proves it flags a violated case
and a missing endpoint.

edge-cases.ts encodes the edges the PR bodies named on the commits they
measured (full SHAs): vite 8492422b (colbymchenry#1713 self-import + getEnv, colbymchenry#1718
two fuzzy nested calls, colbymchenry#1746 sealed defineConfig -> test-stacktrace.js
vite, one relative-import control), vitest 7c818153 (evaluatedModules),
svelte 5895c637 (bundle). npm run eval:precision -- <corpus> fetches the
pinned commit, indexes, scores, prints the resolved-edge histogram by
resolver (the LOST/GAINED methodology) and writes a report; the three
baseline reports at codegraph 1498c2f are committed:
vite 28,893 edges / fuzzy 16, vitest 75,035 / 28, svelte 70,432 / 159;
every absent case held, the control held.
@bompus bompus mentioned this pull request Sep 12, 2026
Merged
bompus added a commit to bompus/codegraph that referenced this pull request Sep 12, 2026
@bompus
test(eval): precision scoring and a pinned-corpus edge gate for resol…
72a5703 
…ution (#27)

Phase 0 of docs/design/resolution-binding-model-plan.md.

The evaluation runner scored recall only, so a wrong edge — the thing
the resolution PR chain (colbymchenry#1713, colbymchenry#1718, colbymchenry#1746, colbymchenry#1844) exists to remove —
was invisible to it. scoring.ts gains scoreEdgeCase: an EdgeCase names
a kind, a target endpoint (file suffix + symbol) and optionally a source,
and expects the edge absent (known-wrong) or present (control).
evaluation-edge-scoring.test.ts pins it on a synthetic project with the
bare-import and sealed-module shapes and proves it flags a violated case
and a missing endpoint.

edge-cases.ts encodes the edges the PR bodies named on the commits they
measured (full SHAs): vite 8492422b (colbymchenry#1713 self-import + getEnv, colbymchenry#1718
two fuzzy nested calls, colbymchenry#1746 sealed defineConfig -> test-stacktrace.js
vite, one relative-import control), vitest 7c818153 (evaluatedModules),
svelte 5895c637 (bundle). npm run eval:precision -- <corpus> fetches the
pinned commit, indexes, scores, prints the resolved-edge histogram by
resolver (the LOST/GAINED methodology) and writes a report; the three
baseline reports at codegraph 1498c2f are committed:
vite 28,893 edges / fuzzy 16, vitest 75,035 / 28, svelte 70,432 / 159;
every absent case held, the control held.
@maxmilian maxmilian mentioned this pull request Sep 13, 2026
Closed
@bompus
test(resolution): keep compound receiver evidence without guessing ta…
0b2c1c5 
…rgets (colbymchenry#1794)

Rebased onto main after colbymchenry#1862: the extraction mechanism from the original
revision is superseded (upstream's normalized qualified-chain callee covers
the retention), but this matrix still adds value — 8 receiver shapes
(optional chains, computed keys, comments, unicode, imported and namespace
roots) across ts/tsx/js/jsx asserting argument calls survive while the
nested member itself stays unlinked. Verified green on current main.
@bompus
bompus force-pushed the fix/upstream-nested-call-evidence branch from 95dccf9 to 0b2c1c5 Compare September 15, 2026 14:58
@bompus

bompus commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main as a tests-only change (verified green: the 4 matrix variants pass unmodified).

What changed in the rebase and why: #1862 landed the same retention behavior with a different encoding (normalized qualified chain + validation, vs this PR's raw receiver text), plus the resolver-side guard. The original revision's source mechanism (isTsJsNestedCall, the raw-text callee, the version bump) is superseded, so I dropped all of it rather than re-litigating the encoding.

What this keeps: the compound-receiver matrix (holder?.values, computed keys, comments, unicode names, imported and namespace roots × ts/tsx/js/jsx) asserting argument calls survive while the nested member stays unlinked — coverage #1862's tests don't pin at the resolution level. Its parity-test expectations asserted the raw-text encoding and cannot hold under #1862, so those went with the mechanism.

@bompus bompus changed the title fix(extraction): retain nested call evidence without guessed targets test(resolution): guard nested receiver call evidence (#1794) Sep 28, 2026
@colbymchenry colbymchenry mentioned this pull request Oct 23, 2026
Merged
colbymchenry pushed a commit that referenced this pull request Oct 23, 2026
@bompus @claude @colbymchenry
test(resolution): calls on nested receivers keep their evidence and g…
3750921 
…uess no target

A member call on a nested receiver, like holder?.values.get(readKey()),
has failed both ways. Before PR #1790, each such call was linked by its
bare method name, to any project method called get. PR #1790 stopped
that by dropping every call on a chain rooted at a name, which also
dropped the call sites effects are drawn from (issue #1794). PR #1862
brought the dotted chains back as unresolved references; shapes its
chain pattern rejects, like holder[0].get(), are still dropped.

The test indexes eight receiver shapes in .ts, .tsx, .js and .jsx:
optional access, computed and numeric keys, a quoted key, a comment
inside the receiver, a non-ASCII name, an imported root and a namespace
import. It checks that the readKey() argument call survives, that
nothing links to Collision.get or to the imported object, and that the
three dotted chains (holder.values.get, imported.values.get and
data.holder.values.get) are still in the index, unresolved.

That last check is ours. As first written, the test passed on PR
1790's commit de5adba, so it could not see the drop issue 1794 is
about. With it, the test fails 4 of 4 on de5adba (the chains are
gone) and 4 of 4 on its parent 72c1ff1 (every shape linked
Collision::get), with the kernel and with CODEGRAPH_KERNEL=0, and
passes 4 of 4 on main d02a88d with both. On main's code, putting the
drop back fails it, as it fails release-main-regressions' retention
test; letting the shapes the chain pattern rejects fall back to the
bare method name fails only this test. The non-ASCII name is written
with U+00F6 and U+00E9, Latin-1 letters, not homoglyphs. Test-only, so
no CHANGELOG entry.

Original PR #1844 by @bompus.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@colbymchenry

colbymchenry commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thanks @bompus, this landed in #2867 with you as the commit author.

While proving it we found that, as written, the test passed on #1790's own commit. #1790 dropped those calls, and a dropped call and a kept-but-unresolved one look the same when you only check callees. So we added a short check that the dotted chains (holder.values.get, imported.values.get, data.holder.values.get) are still recorded as unresolved references. Now it fails both before #1790 (the Collision.get guesses) and at #1790 (the drop), and your eight receiver shapes catch a fallback to bare-name guessing that no other test does. Closing this in favor of #2867, which is now merged.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bompus @colbymchenry