fix(resolution): stop silent edge loss when a same-key reference group straddles the resolution batch boundary#1266
Open
w0lan wants to merge 1 commit into
Open
Conversation
…top batch-boundary edge loss (colbymchenry#1265) The batched resolver (resolveAndPersistBatched) reads pending refs in offset-0 LIMIT/OFFSET pages and, after each batch, deleted the resolved rows keyed only on (from_node_id, reference_name, reference_kind). But edge identity is (source, target, kind, line, col) (idx_edges_identity), so one caller invoking the same target from many call sites is many distinct rows AND edges. When such a same-key group is larger than the batch size, the 3-tuple delete wiped the whole group — including tail rows a later batch had not read yet — silently dropping their edges while the index still reported pending=0. Key the delete (and, for symmetry, markReferencesFailed) on the full call-site identity including line/col. line/col are NOT NULL in unresolved_refs, so plain equality suffices. Updates all call sites (resolveAndPersist, resolveAndPersistListYielding, resolveAndPersistBatched) and the query key types. Tests: the regression test covers three scenarios — the straddling same-key group (edges preserved across the batch boundary), an exact-fit control (group fills the batch exactly; passes pre- and post-fix), and the retry-list path (resolveAndPersistListYielding resolving a subset of a same-key group must not clobber the pending siblings it never touched). Adds a CHANGELOG entry under [Unreleased]. Fixes colbymchenry#1265
Author
|
Field confirmation on a production Elixir codebase (the repo where this was originally noticed): an ExUnit test module calling the same helper from 26 call sites had 17 edges before this fix and exactly 26 after (total edges +9, node count unchanged, |
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.
Fixes #1265.
Summary
On
codegraph index(the batched resolution path, defaultbatchSize=5000),edges are silently lost whenever one caller invokes the same resolvable target
from more call sites than fit in the remainder of a batch. The first batch
resolves its slice of the group and creates its edges — then deletes the
entire key group from
unresolved_refs, including the rows a later batchhas not read yet. Those references vanish before resolution; their edges are
never created, and the index reports a clean, complete state (
pending=0,failed=0), so nothing signals the loss.Found while validating the Elixir support PR on a production codebase: a test
module calling one helper 26 times produced only 17
callsedges on thebatched path, and the count changed with batch size. The bug itself is
language-agnostic and reproduces on clean v1.4.1 in TypeScript — full analysis,
deterministic reproduction, and loss table in #1265.
Fix
Key
deleteSpecificResolvedReferencesandmarkReferencesFailedon the fullcall-site identity —
(from_node_id, reference_name, reference_kind, line, col).line/colareNOT NULLinunresolved_refs, so plain equalitysuffices; they already flow through the resolver (
ref.original.line/column).All three call-site paths updated (
resolveAndPersist,resolveAndPersistListYielding,resolveAndPersistBatched). A straddlinggroup's tail now stays pending and is resolved by the next batch, which
preserves the drain-termination invariant from #1187 (every processed row
still leaves the pending set) and composes with the #1240 failed-refs retry.
The retry path turned out to be affected too, not just latently: the 3-tuple
delete has no status filter, so
resolveAndPersistListYieldingresolving asubset of a same-key group also wiped that key's still-pending rows (pre-fix
it dropped 8 of 10 remaining pending rows in the new test).
No supporting index added: the existing
idx_unresolved_from_name(from_node_id, reference_name)narrows the delete to the key group andSQLite filters line/col within it; adding a composite index would cost a
schema-migration version bump for marginal gain outside pathological group
sizes. Happy to add it here or as a follow-up if you prefer.
Verification
__tests__/batch-boundary-edge-loss.test.ts, threescenarios: a 15-call-site same-key group with an injected
batchSize=10(8/15 edges before the fix, 15/15 after); an exact-fit control (group fills
the batch exactly; passes pre- and post-fix); and the retry-path scenario
above. Each bug-pinning scenario was watched fail on the unfixed code first.
calls/referencesedges when a same-key reference group straddles the 5000-row resolution batch boundary #1265 on the rebuilt CLIproduces 8000/8000 edges (4998 before),
pending=0, failed=0.of the final diff (verdict: approve). It independently reproduced the
regression test's red-on-revert and confirmed via grep that no other
3-tuple-keyed mutation of
unresolved_refsremains; its one non-blockingnote is the composite-index follow-up mentioned above.
Impact
Rare but unbounded and invisible: ordinary code seldom has one function
calling one resolvable target thousands of times, but generated code, dispatch
tables, and repetitive test files do (observed in the field: 26→17 edges on a
production Elixir repo; synthetic: 3002 edges lost at K=8000) — and
statusreports the index as complete throughout.