fix(resolve): gate the exact byQualified path; never bind production code to test symbols#117
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…code to test symbols
The unqualified-fallback gates (cross-language, receiver, cross-scope
demotion) only ran on the fallback path. Targets that hit byQualified
exactly skipped them entirely, so a bare Ruby call like `x.count` (emitted
at ConfidenceUnresolved with an unknown receiver) or a constant call like
`I18n.t` exact-matched a same-named JavaScript symbol in a test spec and
resolved at full confidence. On a real Rails app this produced 603
production->test structural edges (calls + references), almost all of them
Ruby production code binding to JS test-spec constants.
Three changes, all extending rules that already existed on the fallback:
- The exact byQualified path now applies the cross-language gate for
calls/tests/references edges, exempting synthetic cross-language targets
(partial:, turbo-*, importmap:, i18n:, route:, ruby-core:) which are
intentional. When the exact name matches only gated-away coincidences the
edge drops to unresolved rather than leaf-falling-back into another.
- A bare target emitted at ConfidenceUnresolved (receiver unknown) skips the
exact shortcut and goes through the gated fallback, which demotes it below
blast's floor instead of letting a leaf coincidence resolve at 0.5.
- filterByTestDirection: a production-source calls/references edge cannot
resolve into a test file. Test files define stubs and helpers that shadow
real (often framework) names; a same-named match there is a coincidence
(a helper's `url_for` binding to FiltersHelperTest#url_for). The gate is
one-directional, so test code still references production and test symbols.
SymbolRef now carries the file Path (already left-joined in the SymbolRefs
query) so the resolver can determine per-file test-ness, mirroring how it
already carries Language.
Verified on a real Rails app: production->test calls/references edges
603 -> 0 and cross-language structural edges 803 -> 0, while same-language
production->production calls (6773) and test->production edges (6083) are
intact and temporal co-change coupling is untouched.
453b949 to
997eeec
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Follow-up to #116. After that fix, a real Rails codebase still showed 603 production→test structural edges (465 calls + 138 references) where production code resolved a call/reference to a symbol defined in a test file. Root cause: the resolver's gates (cross-language, receiver, cross-scope demotion) only ran on the fallback path. Targets that hit
byQualifiedexactly skipped every gate, so:x.count(emitted atConfidenceUnresolved, receiver unknown) exact-matched a same-named JavaScript constant in a test spec and resolved at 0.5;I18n.texact-matched a JS test-helperI18n.tat 1.0.598 of the 603 were cross-language (Ruby production → JS test specs); the other 5 were same-language (a helper's
url_forbinding toFiltersHelperTest#url_for, where the real method is framework-inherited and unindexed).Summary
Extend the gates the fallback already applies to the exact
byQualifiedpath, and add a one-directional production→test gate.Changes
calls/tests/referencesexact matches now run throughfilterByLanguage, exempting intentional synthetic cross-language targets (partial:,turbo-*,importmap:,i18n:,route:,ruby-core:). When the exact name matches only gated-away coincidences, the edge drops to unresolved instead of leaf-falling-back into another coincidence.ConfidenceUnresolved(receiver unknown) skips the exact shortcut and goes through the gated fallback, which demotes it below blast's floor rather than letting a leaf coincidence resolve at 0.5.filterByTestDirection— a production-sourcecalls/referencesedge cannot resolve into a test file. Test files define stubs/helpers that shadow real (often framework) names, so a same-named match there is a coincidence. One-directional: test code still references both production and test symbols.SymbolRef.Path— carried through the existingSymbolRefsleft-join so the resolver can determine per-file test-ness, mirroring how it already carriesLanguage.Architecture / scope notes
isTestPathis a small local copy ofmcpio.IsTestPath.resolveis a low-level package and must not depend on the presentation layer, so the copy is deliberate; hoisting a shared predicate into a base package is a reasonable follow-up, intentionally out of scope here.calls/tests/referencesedges.Verification (real Rails app)
calls+referencesedges ≥0.5: 603 → 0Test Plan
isTestPath,isSyntheticTarget,filterByTestDirection; adapter carriesPathmake cifully green (build, test, lint 0 issues)