scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-11-10 08:27:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(scope-resolution): close the gaps a four-lens review found in the #2881 change

Browse source 
Correctness review found no defect in the shipped resolvers — the `endsWith`
rewrites, the C# empty-prefix guard, the memo's purity, the `stem` vs `norm`
derivation, the Map re-`set` during iteration and Go's `substring` arithmetic
were each attacked with running code and each held. Everything below is a gap
in what the change ASSERTS, measures or claims.

UNGATED BEHAVIOUR, now covered:

  - `import-resolvers/go.ts` had no test at all. Its rule changed, the shared
    bench drives the indexed leg rather than this one, and a revert was caught
    by nothing. `go-package-resolve.test.ts` pins the membership rule and, more
    usefully, pins that Go's two independent legs agree on it — they disagreed
    before #2881 and a divergence here means the LanguageProvider hook and the
    ScopeResolver hook hold different views of a package.
  - The memo and the compaction are output-identical, so no fingerprint sees
    them and reverting either leaves both benches green. `kotlin-index-internals
    .test.ts` asserts them directly: the memo hit path against the miss path,
    two directories sharing a component-suffix keeping separate buckets (the one
    way a coarser memo key could move an answer), and that the bucket handed out
    is the cached, frozen, compacted array on both the sliced and the skipped
    path. The comment claiming this was "asserted structurally" previously
    pointed at nothing in the repo.

GATES:

  - Six ratio budgets in `bench/import-target` were slack: the measurements they
    bound got faster and the numbers were left alone. kotlin depth 3.4 -> 2.8,
    go 1.6 -> 1.4, csharp 2.2 -> 2.0, java 2.2 -> 2.1, kotlin collide_scaling
    1.8 -> 1.65, go 5.5 -> 5.1, each holding the headroom the old value
    expressed. The absolute ms ceilings are deliberately untouched: they carry
    runner-contention headroom, and a ratio is runner-speed-invariant where a
    millisecond is not. This is the failure the branch already fixed one
    directory over and missed here.
  - The `csharp_csproj` heap re-baseline is REVERTED. Base and branch both
    measure ~73.10e6 three runs each; the recorded 73703384 was simply not
    reproducible, and re-recording it would have dropped that language's derived
    floor 0.8% for no reason belonging to this change.
  - kotlin's collide arm was blind to the rule it was re-baselined for — a full
    revert of the Kotlin guards left both its fingerprints unmoved, because
    `com/example/models` is not a suffix of `…/models/inner/models`. Deepened to
    repeat the whole queried path; those two fingerprints are the only ones that
    moved for it. The same deepening on the java and kotlin UNIQUE arms was
    measured and REVERTED: ten more fingerprints, java's heap reading up 43%,
    and no coverage gained, because progressive stripping lands those queries on
    the same file either way.

SIMPLIFICATION:

  - `go.ts` now states the predicate as ends-with like its three siblings,
    instead of keeping the `indexOf` shape with `lastIndexOf` swapped in.
  - C# csproj step 2's direct-child filter is dead for a non-empty prefix —
    `getFilesInDir`'s keys ARE segment-aligned directory suffixes, so it cannot
    reject, and measurement agrees over 12 008 pairs. Only the empty-prefix case
    does work, and only that case remains.
  - `addChild` had one call site left; inlined. The memo's double read of its
    own lookup is gone. The V8 byte accounting duplicated verbatim between the
    resolver comment and the baselines note now lives only in the note.
  - Four copies of the same ternary in the csproj parity harness collapse onto
    one hoisted `dirTrail`; two locals in the java harness were named for the
    branch that was deleted.

CLAIMS THAT WERE WRONG:

  - `package-dir-index.ts` said "the four resolvers agree again". It is six, and
    the sixth is the evidence: `import-resolvers/jvm.ts` has answered the same
    question with `lastIndexOf` since #488, so before #2881 Java's and Kotlin's
    LanguageProvider hook and their ScopeResolver hook disagreed about which
    files a package holds.
  - The `uniqueDir` docblock claimed the last segment IS the query granularity
    for csharp/java/kotlin. They query the whole dotted path first and reach the
    tail only through stripping — which is why the partial-revert control fires
    on the go arm alone, now stated instead of implied.
  - Three parity harnesses described themselves as verbatim copies of the
    pre-change implementations; they were edited by this branch, so they are
    re-derivations of the current spec, a weaker claim their headers now make.
  - The shared harness header still listed the removed rule as current, the
    `DIRS` docblock still justified shapes by a divergence that no longer
    exists, and `measure.mjs`'s tier-two docblock plus `_heap_bound_note` still
    counted nine bounded languages when `HEAP_BOUNDED` derives to three — this
    branch had dutifully updated a kotlin bound in a list no gate reads.
  - `_blind_spot` told the next reader to build a repeated-leaf arm that already
    exists in the sibling bench, with a budget that already fails the swap.

Both baselines are also re-serialized to preserve each note's original escaping,
undoing ~20 KB of no-op churn an earlier revision introduced by round-tripping
the JSON.

Refs #2881.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W8QYxYvd5ntikRUvpLZD5e
This commit is contained in:
Gergo Magyar 2026-08-11 20:07:10 +00:00
parent bebdda7850
commit 5ee4753d44
13 changed files with 409 additions and 128 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
gitnexus/bench/import-target/baselines.json
View file

File diff suppressed because one or more lines are too long

60
gitnexus/bench/import-target/measure.mjs
View file

@ -730,10 +730,17 @@ function unwiredLanguage(where, lang) {
* exactly one entry. A nested same-name directory in one repo slice is the
* shape the first-`indexOf` tie-break used to reject (see package-dir-index.ts);
* #2881 removed that tie-break from every resolver that had it, so the go,
* csharp, java and kotlin arms all resolve their `d % 7` slice now. Each of
* those four languages repeats the shape at whatever granularity ITS query uses
* — the last segment for csharp/java/kotlin, the whole package path for go —
* because a repeat the query cannot even ask about leaves the arm blind.
* csharp, java and kotlin arms all resolve their `d % 7` slice now.
*
* A repeat the query cannot ask about leaves the arm blind, which is why go's
* slice repeats the WHOLE package path: a Go import addresses `src/pkg{d}`, and
* `…/internal/pkg{d}` does not end with that, so the old rule was never even
* reached and every go arm sat still through the fix. Java, C# and Kotlin query
* the whole dotted path FIRST and only fall back to the tail through
* progressive stripping, so their slices — which repeat the last segment only —
* move through that fallback rather than the primary query. The consequence is
* measured and worth knowing: a partial revert that reinstates first-occurrence
* only for multi-segment package paths is caught on the go arm alone.
*/
function uniqueDir(lang, d, i) {
// Go's nested slice repeats the WHOLE queried path (`src/pkg{d}`), not just
@ -743,6 +750,15 @@ function uniqueDir(lang, d, i) {
// when #2881 removed that rule, which would have shipped a widened bucket
// with no bench coverage while C# and Java were re-baselined for it.
if (lang === 'go') return d % 7 === 0 ? `src/pkg${d}/internal/src/pkg${d}` : `src/pkg${d}`;
// The repeat is the whole queried path (`App.Ns{d}` -> `App/Ns{d}`), not just
// the leaf: C# queries the full dotted path first and only reaches the leaf
// through progressive stripping, so a leaf-only repeat exercises the fallback
// rather than the primary query (#2881).
// Leaf-only repeat, deliberately: this layout is shared with the
// `csharp_csproj` arm, whose configs mint `dirPrefix` against `src/Ns{d}`, so
// deepening it to the full `App/Ns{d}` query path resolves that arm to ZERO
// and breaks its same-workload invariant. C# therefore exercises the removed
// rule through progressive stripping rather than through its primary query.
if (lang === 'csharp') return d % 7 === 0 ? `src/Ns${d}/Sub/Ns${d}` : `src/Ns${d}`;
if (lang === 'dart') return d % 3 === 0 ? `lib/feature${d}` : `pkg/feature${d}`;
if (lang === 'kotlin') {
@ -803,7 +819,16 @@ function collideDir(lang, d, i) {
if (lang === 'dart') return `pkg${d}/lib/src`;
if (lang === 'kotlin') {
return d % 7 === 0
? `mod${d}/src/main/kotlin/com/example/models/inner/models`
? // Repeats the WHOLE queried path (`com.example.models`), not just the
// `models` leaf. With a leaf-only repeat this arm was structurally
// blind to the #2881 rule: a full revert of the Kotlin guards left both
// collide fingerprints unmoved, because `com/example/models` is not a
// suffix of `…/models/inner/models` and the query never reached the
// rule. Deepening it is the only corpus edit in this file that buys
// coverage — the same deepening applied to the java and kotlin UNIQUE
// arms was measured and reverted, because progressive stripping lands
// those queries on the same file either way.
`mod${d}/src/main/kotlin/com/example/models/inner/com/example/models`
: `mod${d}/src/main/kotlin/com/example/models`;
}
if (lang === 'php') return `svc${d}/src/Models`;
@ -1841,8 +1866,9 @@ const HEAP_PROBE_TARGET = {
javascript: 'vendor0/lib/missing',
python: 'vendor0.deep.missing',
c: 'vendor0/missing.h',
// The nine below are the BOUNDED tier — see `HEAP_BOUNDED`. Same rule as the
// eight above: a spelling `uniqueTarget` already mints for that language, and
// The entries below cover the BOUNDED tier — see `HEAP_BOUNDED`, which
// derives to cobol, swift and rust; the rest were promoted. Same rule as the
// budgeted ones above: a spelling `uniqueTarget` already mints for that language, and
// one that MISSES, so the reading is the index and the cascade runs to the
// end. Chosen from the miss family that reaches furthest into each cascade:
// - `go` takes the GOPATH fallback, one `filesDirectlyInPkgDir` per path
@ -2599,9 +2625,12 @@ for (const lang of HEAP_BUDGETED) {
* TIER TWO, the bounded arms: ONE comparison, and what it is a comparison FOR.
*
* `heap_bound_bytes` is the "exclusion still holds" bound. It does not claim
* these nine indexes are small enough, which is what a ceiling claims about a
* these indexes are small enough, which is what a ceiling claims about a
* budgeted one; it claims each is still the SIZE the decision to leave it out
* was taken on. The re-entry condition the MEMORY section states — "if any of
* was taken on. `HEAP_BOUNDED` derives to THREE today — cobol, swift, rust.
* The prose below still counts nine because six were promoted to tier one
* after it was written; read the counts as history, and `HEAP_BOUNDED` itself
* as the answer. The re-entry condition the MEMORY section states — "if any of
* the four ever diverges in what it ASKS, it earns an arm the same way" — is a
* claim about growth, and this is the only thing in the file that can see it.
*
@ -2609,13 +2638,12 @@ for (const lang of HEAP_BUDGETED) {
* because it builds nothing, so any floor at all would be a floor on noise and
* `1.5 x 0 B` is 0 — its bound is ABSOLUTE (1 MiB) for the same reason: a
* multiplier on 16 B fails on the first byte of anything. The other eight are
* stable enough today to floor (0.24% peak-to-peak at worst over five runs) and
* two of them — kotlin at 40.82 MiB and dart at 7.47 — are larger than budgeted
* arms, so a floor there would be worth having. That is a promotion to tier one,
* with a ceiling and a recorded reading, and it is not this change: a floor
* without them would assert "still measuring" against a number nothing else
* bounds. What this tier is NOT is a weaker version of tier one — it is the
* different question, asked of every language instead of eight.
* stable enough today to floor (0.24% peak-to-peak at worst over five runs).
* The two this paragraph named as floor candidates, kotlin and dart, TOOK that
* promotion: both now carry a ceiling and a recorded reading in tier one, which
* is what the paragraph said the promotion had to be. What this tier is NOT is a
* weaker version of tier one — it is a different question, asked of the
* languages tier one does not ask it of.
*/
const heapBoundScope =
`That leaves the arm bounded by nothing, which is the state all nine of these were in before ` +

12
gitnexus/bench/kotlin-import-target/baselines.json
View file

@ -1,15 +1,15 @@
{
"_comment": "Baselines for bench/kotlin-import-target/measure.mjs --check. `fingerprint` is a sha256 over every `fileSet | fromFile | targetRaw -> result` record the correctness corpus resolves, in BOTH file-set iteration orders; it is a CORRECTNESS gate, so drift means Kotlin import resolution started returning a different file set and IMPORTS/CALLS edges moved in every Kotlin repository. Explain it, never re-baseline to make CI green. `cases` and `non_null` are asserted beside it because a shrunken or hollowed corpus produces a perfectly valid fingerprint over a smaller surface \u2014 all three are one re-baseline, never separate ones. `scaling_budget`, `depth_budget` and `small_ms_ceiling` are timing gates and carry deliberate headroom for shared CI runners.",
"_provenance": "RE-BASELINED ONCE, DELIBERATELY, IN #2881. The previous value ebf1790bf1d42dad483a51f2cbdeb2351e493b9e8236e4eedeef592dd81e2c5c (13256 non-null) was the PRE-INDEX implementation's, and the index that replaced it in #2872 reproduced it byte for byte \u2014 that is what made #2872 a performance change. #2881 changes resolution on purpose: `getKotlinFileIndex` no longer requires the parent directory to be the FIRST occurrence of that name in the path, so a file whose package directory name repeats higher in its own path is now a child of that package (`data/src/main/kotlin/com/example/data/Repo.kt` IS a child of `data`, and `import data.helper` resolves instead of returning null). The drift was not read off the new code and accepted; the corpus was dumped from both implementations and diffed record by record. Of 19968 distinct records, 235 moved: 54 NULL -> resolved (the fix itself, and exactly the +54 in non_null, 13256 -> 13310) and 181 answers that changed within a now-larger `dirChildren` bucket \u2014 149 where the first-child tier picks a different member and 32 where a fan-out array gained entries. ZERO buckets lost a member, zero results were dropped, and in all 149 reselections the new answer's parent directory is the queried package segment. The corpus itself is untouched, which is why `cases` is unchanged at 20106 \u2014 the fingerprint is over the same surface as the value it replaces.",
"_gate_controls": "The gate is only worth its baseline if a plausible regression moves it, so each arm was checked against the mutation it exists to catch, with the resolver otherwise untouched. All values below are against the CURRENT baseline (#2881, guards removed + per-directory key memo + bucket compaction). Caught: skipping the dirChildren component walk above depth 8 (fingerprint 41bb550b76d4\u2026, non_null 13310 -> 12800); capping a dirChildren bucket at 17 entries (a7681945b752\u2026, non_null UNCHANGED \u2014 the fingerprint is the only arm that sees it, and note the compaction pass now rewrites those same buckets, so this control was re-run after it); capping suffixByStem key depth at 7 (d24b8a2bd822\u2026, non_null unchanged); and a HALF fix that drops only the `startsWith` guard while keeping the `indexOf` first-occurrence check (836977b83bf0\u2026, non_null 13310 -> 13282), which leaves every mid-path repeat such as `top/data/mid/data/Repo.kt` broken and is the mutation #2881 itself makes plausible. Added with the memo: keying `dirKeys` on the directory's LAST SEGMENT instead of its full path (36a4e9dad313\u2026, non_null 13310 -> 13305) \u2014 the memo's whole safety argument is that its key determines the key SET a directory contributes, so a coarser key silently hands one directory another's bucket list, and that is the one way this optimization can move an answer. Also caught, with the RESOLVER untouched and only the corpus edited: dropping the competing file from the exact-beats-earlier-suffix case and emptying the repeated-directory case (44df5093ee\u2026). All of these passed silently before this corpus carried deep paths, packages above 16 files, queries against suffix keys deeper than 7, and the file set inside the hashed record. Re-check them after any corpus edit \u2014 a corpus that stops spanning an axis takes the gate with it. NOTE what no fingerprint control here can catch: the memo and the compaction are both invisible to this bench by design (identical output), so their correctness rests on a structural comparison of the three maps \u2014 key insertion order, bucket order, frozen-ness \u2014 over 1234 corpora in both iteration orders, not on this gate.",
"_comment": "Baselines for bench/kotlin-import-target/measure.mjs --check. `fingerprint` is a sha256 over every `fileSet | fromFile | targetRaw -> result` record the correctness corpus resolves, in BOTH file-set iteration orders; it is a CORRECTNESS gate, so drift means Kotlin import resolution started returning a different file set and IMPORTS/CALLS edges moved in every Kotlin repository. Explain it, never re-baseline to make CI green. `cases` and `non_null` are asserted beside it because a shrunken or hollowed corpus produces a perfectly valid fingerprint over a smaller surface — all three are one re-baseline, never separate ones. `scaling_budget`, `depth_budget` and `small_ms_ceiling` are timing gates and carry deliberate headroom for shared CI runners.",
"_provenance": "RE-BASELINED ONCE, DELIBERATELY, IN #2881. The previous value ebf1790bf1d42dad483a51f2cbdeb2351e493b9e8236e4eedeef592dd81e2c5c (13256 non-null) was the PRE-INDEX implementation's, and the index that replaced it in #2872 reproduced it byte for byte — that is what made #2872 a performance change. #2881 changes resolution on purpose: `getKotlinFileIndex` no longer requires the parent directory to be the FIRST occurrence of that name in the path, so a file whose package directory name repeats higher in its own path is now a child of that package (`data/src/main/kotlin/com/example/data/Repo.kt` IS a child of `data`, and `import data.helper` resolves instead of returning null). The drift was not read off the new code and accepted; the corpus was dumped from both implementations and diffed record by record. Of 19968 distinct records, 235 moved: 54 NULL -> resolved (the fix itself, and exactly the +54 in non_null, 13256 -> 13310) and 181 answers that changed within a now-larger `dirChildren` bucket — 149 where the first-child tier picks a different member and 32 where a fan-out array gained entries. ZERO buckets lost a member, zero results were dropped, and in all 149 reselections the new answer's parent directory is the queried package segment. The corpus itself is untouched, which is why `cases` is unchanged at 20106 — the fingerprint is over the same surface as the value it replaces.",
"_gate_controls": "The gate is only worth its baseline if a plausible regression moves it, so each arm was checked against the mutation it exists to catch, with the resolver otherwise untouched. All values below are against the CURRENT baseline (#2881, guards removed + per-directory key memo + bucket compaction). Caught: skipping the dirChildren component walk above depth 8 (fingerprint 41bb550b76d4…, non_null 13310 -> 12800); capping a dirChildren bucket at 17 entries (a7681945b752…, non_null UNCHANGED — the fingerprint is the only arm that sees it, and note the compaction pass now rewrites those same buckets, so this control was re-run after it); capping suffixByStem key depth at 7 (d24b8a2bd822…, non_null unchanged); and a HALF fix that drops only the `startsWith` guard while keeping the `indexOf` first-occurrence check (836977b83bf0…, non_null 13310 -> 13282), which leaves every mid-path repeat such as `top/data/mid/data/Repo.kt` broken and is the mutation #2881 itself makes plausible. Added with the memo: keying `dirKeys` on the directory's LAST SEGMENT instead of its full path (36a4e9dad313…, non_null 13310 -> 13305) — the memo's whole safety argument is that its key determines the key SET a directory contributes, so a coarser key silently hands one directory another's bucket list, and that is the one way this optimization can move an answer. Also caught, with the RESOLVER untouched and only the corpus edited: dropping the competing file from the exact-beats-earlier-suffix case and emptying the repeated-directory case (44df5093ee…). All of these passed silently before this corpus carried deep paths, packages above 16 files, queries against suffix keys deeper than 7, and the file set inside the hashed record. Re-check them after any corpus edit — a corpus that stops spanning an axis takes the gate with it. NOTE what no fingerprint control here can catch: the memo and the compaction are both invisible to this bench by design (identical output), so their correctness rests on a structural comparison of the three maps — key insertion order, bucket order, frozen-ness — over 1234 corpora in both iteration orders, not on this gate.",
"fingerprint": "d91110bee389891c313811c5b4bae61d909561156e1458d38d487be969f0059c",
"cases": 20106,
"non_null": 13310,
"scaling_budget": 1.6,
"depth_budget": 2.0,
"small_ms_ceiling": 40,
"_scaling_note": "(t_large/t_small)/(1600/400). ~1.0 is linear. OBSERVED BAND: 0.99-1.04 on a 12-core dev box, small arm ~6 ms. Read that band as a floor, not a spec \u2014 independent runs on other hardware during review came out 0.954-1.014, 0.965-1.036 and ~0.95-1.08, so a 1.2 reading is noise and should be re-run, not investigated. IMPORTS_PER_FILE is sized so the small arm lands in the ms rather than the ~2 ms a first revision measured, where timer granularity and JIT warm-up, not scaling, set the number; bench/cpp-qualified-ns documents the same artifact. TRIAGE: every timing arm here is a TIMING signal \u2014 RE-RUN IT on an idle machine before investigating; runner contention dominates. The fingerprint arm is the opposite: deterministic, a re-run never changes it, and it must never be wished away. FLOOR CHECK: the pre-index implementation \u2014 i.e. exactly the regression this gate exists to catch \u2014 measures ratio 3.737 on this corpus (2207.8 ms small, 33003.5 ms large, one cold run) against ~1.0 for the index. Independent review runs measured its floor at 3.905-4.297. Treat the absolute times as an order of magnitude only: the floor arm is one cold run because best-of-seven against a quadratic implementation costs minutes, while the index arm is best-of-seven after two warmups.",
"_depth_note": "deep_ms/shallow_ms at a FIXED file count, paths 24 components against 8. scaling_ratio divides the file count out, so it is scale-invariant and structurally cannot see a cost that grows with path depth instead \u2014 and the two loops the index is built from are depth loops (one suffixByStem entry per '/' in a stem, one dirChildren pass per component of dir). OBSERVED BAND, five runs each on one box: 1.44-1.51 before #2881; 1.27-1.40 after its guard removal, which deleted two string comparisons per component of every dir; 1.20-1.26 after the same issue's per-directory key memo, which turns that whole component walk from once-per-FILE into once-per-DIRECTORY. Both movements are per-depth work, which is why this arm sees them and the file-count arm does not. The BUDGET moved with the band both times \u2014 2.4 -> 2.2 -> 2.0 \u2014 holding the ~1.6x headroom over the band's top that 2.4 expressed against the original; left at 2.4 it would quietly have become 1.9x, which is how a gate goes slack without anyone deciding to loosen it. Note what this budget is NOT for: a revert of #2881 scores ~1.5 and passes at any of those numbers, and that is correct \u2014 reverting it restores a resolution bug, which is the FINGERPRINT's job to catch, not a timing arm's. It sits above 1.0 legitimately: 3x the depth is 3x the suffix keys per file, so the build genuinely does more work; what the budget forbids is that growing faster than the depth ratio itself.",
"_scaling_note": "(t_large/t_small)/(1600/400). ~1.0 is linear. OBSERVED BAND: 0.99-1.04 on a 12-core dev box, small arm ~6 ms. Read that band as a floor, not a spec — independent runs on other hardware during review came out 0.954-1.014, 0.965-1.036 and ~0.95-1.08, so a 1.2 reading is noise and should be re-run, not investigated. IMPORTS_PER_FILE is sized so the small arm lands in the ms rather than the ~2 ms a first revision measured, where timer granularity and JIT warm-up, not scaling, set the number; bench/cpp-qualified-ns documents the same artifact. TRIAGE: every timing arm here is a TIMING signal — RE-RUN IT on an idle machine before investigating; runner contention dominates. The fingerprint arm is the opposite: deterministic, a re-run never changes it, and it must never be wished away. FLOOR CHECK: the pre-index implementation — i.e. exactly the regression this gate exists to catch — measures ratio 3.737 on this corpus (2207.8 ms small, 33003.5 ms large, one cold run) against ~1.0 for the index. Independent review runs measured its floor at 3.905-4.297. Treat the absolute times as an order of magnitude only: the floor arm is one cold run because best-of-seven against a quadratic implementation costs minutes, while the index arm is best-of-seven after two warmups.",
"_depth_note": "deep_ms/shallow_ms at a FIXED file count, paths 24 components against 8. scaling_ratio divides the file count out, so it is scale-invariant and structurally cannot see a cost that grows with path depth instead — and the two loops the index is built from are depth loops (one suffixByStem entry per '/' in a stem, one dirChildren pass per component of dir). OBSERVED BAND, five runs each on one box: 1.44-1.51 before #2881; 1.27-1.40 after its guard removal, which deleted two string comparisons per component of every dir; 1.20-1.26 after the same issue's per-directory key memo, which turns that whole component walk from once-per-FILE into once-per-DIRECTORY. Both movements are per-depth work, which is why this arm sees them and the file-count arm does not. The BUDGET moved with the band both times — 2.4 -> 2.2 -> 2.0 — holding the ~1.6x headroom over the band's top that 2.4 expressed against the original; left at 2.4 it would quietly have become 1.9x, which is how a gate goes slack without anyone deciding to loosen it. Note what this budget is NOT for: a revert of #2881 scores ~1.5 and passes at any of those numbers, and that is correct — reverting it restores a resolution bug, which is the FINGERPRINT's job to catch, not a timing arm's. It sits above 1.0 legitimately: 3x the depth is 3x the suffix keys per file, so the build genuinely does more work; what the budget forbids is that growing faster than the depth ratio itself.",
"_ceiling_note": "small_ms_ceiling is an ABSOLUTE bound, because scaling_ratio is a ratio and a constant-factor regression that grows both arms equally passes it. Measured during review: a full workspace scan reintroduced on 1-in-16 imports is caught by the ratio (1.814), but at 1-in-32 it passes at 1.490 while running 2.8x slower in absolute terms. 40 ms against an observed 5.9-6.1 ms leaves ~6x of headroom for a loaded shared runner while still catching that shape.",
"_blind_spot": "WHAT THIS BENCH CANNOT SEE, measured rather than guessed. Its scaling corpus gives every module a UNIQUE package leaf (`com/example/mod{N}`), so a `dirChildren` query matches exactly one directory. That makes it blind to any cost that grows with the number of DIRECTORIES sharing a queried segment \u2014 the shape a real Kotlin monorepo has, where 200 modules each hold `data`, `ui` and `domain`. Established by building the reuse this file's memo argues against: swapping `dirChildren` for the shared `import-resolvers/package-dir-index.ts` (with its first-occurrence rule off) is OUTPUT-IDENTICAL \u2014 same fingerprint, same cases, same non_null, 0 divergences over 107948 answers \u2014 and on THIS corpus it costs only 1.37x-1.50x and passes every arm here. On a repeated-leaf corpus the same swap measures 13.5x per first-child query, 409x per fan-out, and 8114x on `import data.*` at 200 matching directories (it merges and SORTS every candidate, per import), for 3.1x-5.7x end to end and a bench-style scaling_ratio of 3.465 against this file's 1.6 budget \u2014 i.e. back to the pre-index quadratic floor of 3.737. A change that regresses this resolver to the very shape the bench exists to catch would go GREEN here. The trade it buys is real and also measured: 26.2% less retained memory, 12.18 MiB at 32000 files. If that memory is ever wanted, the shape to build is per-suffix keys -> DIRECTORY lists plus files-per-directory (8.29 MiB against 15.87 measured, single-directory query still one hash lookup) \u2014 and the corpus needs a repeated-leaf arm FIRST, or the gate will not be watching."
"_blind_spot": "WHAT THIS BENCH CANNOT SEE, measured rather than guessed. Its scaling corpus gives every module a UNIQUE package leaf (`com/example/mod{N}`), so a `dirChildren` query matches exactly one directory. That makes it blind to any cost that grows with the number of DIRECTORIES sharing a queried segment — the shape a real Kotlin monorepo has, where 200 modules each hold `data`, `ui` and `domain`. Established by building the reuse this file's memo argues against: swapping `dirChildren` for the shared `import-resolvers/package-dir-index.ts` (with its first-occurrence rule off) is OUTPUT-IDENTICAL — same fingerprint, same cases, same non_null, 0 divergences over 107948 answers — and on THIS corpus it costs only 1.37x-1.50x and passes every arm here. On a repeated-leaf corpus the same swap measures 13.5x per first-child query, 409x per fan-out, and 8114x on `import data.*` at 200 matching directories (it merges and SORTS every candidate, per import), for 3.1x-5.7x end to end and a bench-style scaling_ratio of 3.465 against this file's 1.6 budget — i.e. back to the pre-index quadratic floor of 3.737. A change that regresses this resolver to the very shape the bench exists to catch would go GREEN here. The trade it buys is real and also measured: 26.2% less retained memory, 12.18 MiB at 32000 files. If that memory is ever wanted, the shape to build is per-suffix keys -> DIRECTORY lists plus files-per-directory (8.29 MiB against 15.87 measured, single-directory query still one hash lookup) — and the repeated-leaf arm to measure it against already exists one directory over: bench/import-target's kotlin `collide` layout puts `com/example/models` under 200 modules at the 1600-file scale, with `collide_scaling_budget` 1.8 against a measured 1.081. The swap scores 3.465 there. So the gate for this decision is that arm, not a new one here; what this file lacks is only a repeated-leaf arm of its own, which would be duplicated coverage."
}

34
gitnexus/src/core/ingestion/import-resolvers/csharp.ts
View file

@ -287,23 +287,27 @@ export function resolveCSharpImportInternal(
// 2. Try as directory: all .cs files directly inside (namespace import)
if (index) {
const dirFiles = index.getFilesInDir(dirPrefix, '.cs');
// `getFilesInDir` already answers "directly inside a directory `D` where
// `D === dirPrefix || D.endsWith('/' + dirPrefix)`" — its keys ARE
// segment-aligned directory suffixes. So for a non-empty `dirPrefix` the
// direct-child re-check this loop used to run cannot reject anything, and
// measurement agrees: zero rejections over 12 008 (prefix, candidate)
// pairs. It rejected before #2881 only because it asked `indexOf` for the
// FIRST `/<dirPrefix>/`, which is the rule that issue removed.
//
// The empty prefix is the exception and keeps a real filter. `getDirMap`
// emits an empty directory suffix for any path whose first slash is its
// last — including a doubled slash, `a//X.cs` — so the bucket is not
// "one directory deep" on its own, and step 3 answers that same query
// from `singleSegmentDirs`, which is. Filtering on `D` holding no slash
// is what keeps steps 2 and 3 in agreement.
for (const f of dirFiles) {
const normalized = f.replace(/\\/g, '/');
// Direct child of a directory ENDING with `dirPrefix` — the same
// predicate as before minus "…and that occurrence is the FIRST one".
// An empty `dirPrefix` keeps `indexOf`: its needle is a bare '/', and
// step 3 answers that query from `singleSegmentDirs` ("exactly one
// directory deep"), which only the FIRST occurrence expresses. With
// `lastIndexOf` there, an empty prefix would accept every `.cs` that
// sits in any directory and step 2 would diverge from step 3.
const needle = dirPrefix + '/';
const prefixIdx =
dirPrefix === '' ? normalized.indexOf(needle) : normalized.lastIndexOf(needle);
if (prefixIdx < 0) continue;
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
if (!afterDir.includes('/')) {
results.push(f);
if (dirPrefix === '') {
const normalized = f.replace(/\\/g, '/');
const lastSlash = normalized.lastIndexOf('/');
if (lastSlash < 0 || normalized.slice(0, lastSlash).includes('/')) continue;
}
results.push(f);
}
if (results.length > 0) return results;
}

24
gitnexus/src/core/ingestion/import-resolvers/go.ts
View file

@ -40,20 +40,16 @@ export function resolveGoPackage(
for (let i = 0; i < normalizedFileList.length; i++) {
// Prepend '/' so paths like "internal/auth/service.go" match suffix "/internal/auth/"
const normalized = '/' + normalizedFileList[i];
// File must be directly in the package directory (not a subdirectory)
if (
normalized.includes(pkgSuffix) &&
normalized.endsWith('.go') &&
!normalized.endsWith('_test.go')
) {
// `lastIndexOf`, not `indexOf`: `pkgSuffix` is '/'-anchored on both sides,
// so the LAST occurrence is the file's own parent. `indexOf` asked for the
// first, which made `a/pkg/b/pkg/x.go` not a member of `pkg` (#2881).
const afterPkg = normalized.substring(normalized.lastIndexOf(pkgSuffix) + pkgSuffix.length);
if (!afterPkg.includes('/')) {
matches.push(allFileList[i]);
}
}
if (!normalized.endsWith('.go') || normalized.endsWith('_test.go')) continue;
// The file's PARENT directory ends with the package path — the same
// predicate `package-dir-index.ts` states, in the same `endsWith` form.
// This used to ask `indexOf` for the FIRST `/<pkg>/` and then check that
// nothing after it held a slash, which made `a/pkg/b/pkg/x.go` not a member
// of `pkg` (#2881). Writing it as ends-with rather than swapping in
// `lastIndexOf` also drops the second scan the `includes` guard needed.
const lastSlash = normalized.lastIndexOf('/'); // >= 0: `normalized` starts with '/'
if (!normalized.slice(0, lastSlash + 1).endsWith(pkgSuffix)) continue;
matches.push(allFileList[i]);
}
return matches;

12
gitnexus/src/core/ingestion/import-resolvers/package-dir-index.ts
View file

@ -27,8 +27,16 @@
* `internal/…/internal`, `Models/…/Models`, and the reported shape
* `data/src/main/kotlin/com/example/data/Repo.kt`, where `import data.helper`
* resolved to null. Kotlin was fixed first, in its own `dirChildren`
* (`languages/kotlin/import-target.ts`); this index and the C# csproj index
* followed, so the four resolvers agree again.
* (`languages/kotlin/import-target.ts`); this index, the C# csproj index and
* the legacy `go.ts` scan followed.
*
* The strongest evidence that the rule was accidental is that a sixth
* implementation of the same question never had it. `import-resolvers/jvm.ts`
* answers "files directly inside a directory ending with <packagePath>" for
* Java and Kotlin wildcard imports, and has used `lastIndexOf` since #488 — it
* is live, wired as `importResolver` by `languages/{java,kotlin}.ts`. So before
* #2881 the Java and Kotlin LanguageProvider hook and their ScopeResolver hook
* disagreed about which files a package holds. Now all six agree.
*
* The length guard the `indexOf` form needed is gone with it: `endsWith` is
* false for a shorter `D` instead of comparing -1 to -1.

44
gitnexus/src/core/ingestion/languages/kotlin/import-target.ts
View file

@ -201,9 +201,11 @@ const getKotlinFileIndex = perFileSet((allFilePaths: ReadonlySet<string>): Kotli
* directory, in the order the per-file walk produced, and every later file in
* that directory finds those keys already present — so the key set, the Map's
* key insertion order and every bucket's order are what the per-file form
* produced. Asserted structurally, not just through the fingerprint: the
* fingerprint only observes the index through the four resolver tiers, so a
* key-order move no corpus query reaches would survive it.
* produced. `kotlin-index-internals.test.ts` asserts that directly, over the
* built maps rather than through the resolver: the correctness fingerprint
* sees this index only through the four tiers, and `dirChildren` is only ever
* read by `.get(key)`, so a Map key-ORDER move has no consumer and no
* fingerprint could catch one.
*
* Dropped with this frame, so it costs nothing retained.
*/
@ -234,10 +236,9 @@ const getKotlinFileIndex = perFileSet((allFilePaths: ReadonlySet<string>): Kotli
if (dirEnd < 0) continue; // repo-root file has no package directory
const dir = stem.slice(0, dirEnd);
const memo = dirKeys.get(dir);
// The file's own directory always qualifies: the old scan's `atRoot` branch
// matched `norm.startsWith(dir + '/')` and found no '/' after it.
const keys = memo ?? [dir];
let keys = dirKeys.get(dir);
// Every component-suffix of the directory also qualifies: `s` is a suffix
// of `dir` starting after a '/', so `dir` ends with `/s` by construction
@ -270,14 +271,18 @@ const getKotlinFileIndex = perFileSet((allFilePaths: ReadonlySet<string>): Kotli
// resolving to null on the very shape this fixes. The bucket is either
// right or it is not; there is no version of it that is right for one
// consumer and wrong for the other.
if (memo === undefined) {
if (keys === undefined) {
keys = [dir];
for (let i = 0; i < dirEnd; i++) {
if (dir[i] !== '/') continue;
keys.push(dir.slice(i + 1));
if (dir[i] === '/') keys.push(dir.slice(i + 1));
}
dirKeys.set(dir, keys);
}
for (const key of keys) addChild(dirChildren, key, raw);
for (const key of keys) {
const bucket = dirChildren.get(key);
if (bucket === undefined) dirChildren.set(key, [raw]);
else bucket.push(raw);
}
}
// `findKotlinPackageFiles` hands a bucket straight out of the index — the
@ -290,15 +295,12 @@ const getKotlinFileIndex = perFileSet((allFilePaths: ReadonlySet<string>): Kotli
// later import in the run. Freezing makes the contract true at runtime, so a
// future mutation is a loud TypeError instead of a silent edge move.
//
// COMPACTED as they are frozen. `addChild` mints a bucket as `[raw]` and
// `push`es the rest, and V8 grows an array's backing store by
// `old + old/2 + 16`, so the SECOND child takes a 1-slot store to 17 and the
// eighteenth takes it to 41 — and every bucket then retains its overshoot for
// the life of the index. `slice()` allocates exactly `length`. Same fix and
// same accounting as the python `byBasename` sentence in
// `bench/import-target/baselines.json`: contents byte-identical, 5.13 MiB
// smaller at 32 000 paths, 11.2% of what that bench's kotlin heap arm reads
// (61 144 buckets x 88 B; 52.9% of those buckets' slots were empty).
// COMPACTED as they are frozen. A bucket is minted as `[raw]` and pushed
// into, and V8 overshoots the backing store when it grows, so every bucket
// retains slack for the life of the index; `slice()` allocates exactly
// `length`. Contents are byte-identical. The byte accounting and the
// comparison to the python `byBasename` fix live once, in
// `bench/import-target/baselines.json`.
//
// `length === 1` is skipped because a bucket that never grew is ALREADY exact
// — `[raw]` allocates one slot — so slicing it allocates a second array to
@ -320,12 +322,6 @@ const getKotlinFileIndex = perFileSet((allFilePaths: ReadonlySet<string>): Kotli
return { exactByStem, suffixByStem, dirChildren };
});
function addChild(dirChildren: Map<string, string[]>, dir: string, raw: string): void {
const bucket = dirChildren.get(dir);
if (bucket === undefined) dirChildren.set(dir, [raw]);
else bucket.push(raw);
}
/** Mutable view of the buckets, used only while building — the index exposes
* them as `readonly` and freezes them before it is cached. */
type MutableDirChildren = Map<string, string[]>;

15
gitnexus/test/unit/import-resolvers/csharp-csproj-parity.test.ts
View file

@ -102,6 +102,11 @@ function legacyResolveCSharpImportInternal(
if (suffixResult) return [suffixResult];
}
// Shared by steps 2 and 3 — the same needle, and since #2881 the same
// `indexOf`-only-when-empty rule, so it is declared once rather than
// re-derived per step.
const dirTrail = dirPrefix + '/';
// 2. Try as directory: all .cs files directly inside (namespace import)
if (index) {
const dirFiles = index.getFilesInDir(dirPrefix, '.cs');
@ -112,9 +117,8 @@ function legacyResolveCSharpImportInternal(
// `indexOf`: its needle is a bare '/', and step 3 answers that query
// from the one-directory-deep set, which only the first occurrence
// expresses.
const needle2 = dirPrefix + '/';
const prefixIdx =
dirPrefix === '' ? normalized.indexOf(needle2) : normalized.lastIndexOf(needle2);
dirPrefix === '' ? normalized.indexOf(dirTrail) : normalized.lastIndexOf(dirTrail);
if (prefixIdx < 0) continue;
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
if (!afterDir.includes('/')) {
@ -126,7 +130,6 @@ function legacyResolveCSharpImportInternal(
// 3. Linear scan fallback for directory matching
if (results.length === 0) {
const dirTrail = dirPrefix + '/';
for (let i = 0; i < normalizedFileList.length; i++) {
const normalized = normalizedFileList[i];
if (!normalized.endsWith('.cs')) continue;
@ -195,13 +198,14 @@ function skipStep3WhenIndexed(
if (suffixResult) return [suffixResult];
}
const dirTrail = dirPrefix + '/';
if (index) {
const dirFiles = index.getFilesInDir(dirPrefix, '.cs');
for (const f of dirFiles) {
const normalized = f.replace(/\\/g, '/');
const needle2 = dirPrefix + '/';
const prefixIdx =
dirPrefix === '' ? normalized.indexOf(needle2) : normalized.lastIndexOf(needle2);
dirPrefix === '' ? normalized.indexOf(dirTrail) : normalized.lastIndexOf(dirTrail);
if (prefixIdx < 0) continue;
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
if (!afterDir.includes('/')) {
@ -212,7 +216,6 @@ function skipStep3WhenIndexed(
continue;
}
const dirTrail = dirPrefix + '/';
for (let i = 0; i < normalizedFileList.length; i++) {
const normalized = normalizedFileList[i];
if (!normalized.endsWith('.cs')) continue;

122
gitnexus/test/unit/import-resolvers/go-package-resolve.test.ts Normal file
View file

@ -0,0 +1,122 @@
/**
* Coverage for `resolveGoPackage` (`import-resolvers/go.ts`), which had none.
*
* Go resolves package imports through two independent legs. The ScopeResolver
* leg (`languages/go/import-target.ts`) answers from `buildPackageDirIndex`;
* this one is the LanguageProvider leg, wired through `configs/go.ts`, and it
* is still a per-import scan. #2881 changed its membership rule — a directory
* whose name repeats higher in the path is now a member — and review found the
* change reached production with nothing watching it: `bench/import-target`'s
* go arm drives the indexed leg only, and no test called this function.
*
* These cases pin the rule and the two legs' agreement on it, so a revert fails
* here rather than silently moving Go IMPORTS edges in every repository that
* nests a package name inside itself (`internal/…/internal` is the shape Go
* actually produces).
*/
import { describe, expect, it } from 'vitest';
import {
resolveGoPackage,
resolveGoPackageDir,
} from '../../../src/core/ingestion/import-resolvers/go.js';
import { resolveGoImportTarget } from '../../../src/core/ingestion/languages/go/import-target.js';
const MOD = { modulePath: 'example.com/mod' } as const;
function resolve(files: readonly string[], importPath: string): string[] {
const normalized = files.map((f) => f.replace(/\\/g, '/'));
return resolveGoPackage(importPath, MOD as never, normalized, files);
}
/** The indexed leg, for the agreement arm. */
function indexed(files: readonly string[], importPath: string): readonly string[] {
const got = resolveGoImportTarget(importPath, 'main.go', new Set(files), MOD);
return got === null ? [] : Array.isArray(got) ? got : [got];
}
describe('resolveGoPackage', () => {
it('returns every .go file directly inside the package directory', () => {
const files = ['internal/auth/service.go', 'internal/auth/token.go', 'internal/auth/sub/x.go'];
expect(resolve(files, 'example.com/mod/internal/auth')).toEqual([
'internal/auth/service.go',
'internal/auth/token.go',
]);
});
it('a package directory nested inside a same-named one IS a member (#2881)', () => {
// The scan asked `indexOf` for the FIRST `/pkg/` and then required nothing
// after it to hold a slash, so this resolved to nothing. Both halves of the
// shape: the repeat leading the path, and the repeat mid-path.
expect(resolve(['pkg/src/go/pkg/repo.go'], 'example.com/mod/pkg')).toEqual([
'pkg/src/go/pkg/repo.go',
]);
expect(resolve(['a/pkg/b/pkg/x.go'], 'example.com/mod/pkg')).toEqual(['a/pkg/b/pkg/x.go']);
expect(resolve(['svc/internal/sub/internal/x.go'], 'example.com/mod/internal')).toEqual([
'svc/internal/sub/internal/x.go',
]);
});
it('a repeated name that is not the parent directory is still not a member', () => {
// The rule is "the parent directory ends with the package path", not
// "the package path appears anywhere".
expect(resolve(['a/pkg/b/x.go'], 'example.com/mod/pkg')).toEqual([]);
expect(resolve(['internal/auth/sub/x.go'], 'example.com/mod/internal/auth')).toEqual([]);
});
it('multi-segment package paths match on the whole run, not the last segment', () => {
const files = ['a/internal/models/b/internal/models/user.go', 'a/models/other.go'];
expect(resolve(files, 'example.com/mod/internal/models')).toEqual([
'a/internal/models/b/internal/models/user.go',
]);
});
it('_test.go files are a different package and never match', () => {
expect(
resolve(
['internal/auth/service.go', 'internal/auth/service_test.go'],
'example.com/mod/internal/auth',
),
).toEqual(['internal/auth/service.go']);
});
it('non-.go files never match, and the RAW path is returned for backslashes', () => {
expect(
resolve(['internal/auth/README.md', 'internal/auth/x.go'], 'example.com/mod/internal/auth'),
).toEqual(['internal/auth/x.go']);
expect(resolve(['internal\\auth\\x.go'], 'example.com/mod/internal/auth')).toEqual([
'internal\\auth\\x.go',
]);
});
it('an import outside the module, or the module root itself, resolves to nothing here', () => {
expect(resolve(['internal/auth/x.go'], 'github.com/other/repo/internal/auth')).toEqual([]);
// The root package is the caller's `findRootPackageFiles` leg, not this one.
expect(resolve(['main.go'], 'example.com/mod')).toEqual([]);
expect(resolveGoPackageDir('example.com/mod', MOD as never)).toBeNull();
expect(resolveGoPackageDir('example.com/mod/internal/auth', MOD as never)).toBe(
'/internal/auth/',
);
});
it('agrees with the indexed leg on the repeated-name shapes', () => {
// The two legs are independent implementations of one rule. Before #2881
// they agreed on the wrong answer; they must agree on the right one, or
// Go's LanguageProvider and ScopeResolver hooks disagree about which files
// a package holds.
for (const files of [
['pkg/src/go/pkg/repo.go'],
['a/pkg/b/pkg/x.go'],
['a/pkg/b/x.go'],
['internal/auth/service.go', 'internal/auth/token.go'],
['a/internal/models/b/internal/models/user.go'],
]) {
for (const target of [
'example.com/mod/pkg',
'example.com/mod/internal/auth',
'example.com/mod/internal/models',
]) {
expect(resolve(files, target)).toEqual([...indexed(files, target)]);
}
}
});
});

35
gitnexus/test/unit/scope-resolution/import-target-index-parity.test.ts
View file

@ -10,19 +10,26 @@
* through anything the type system or the existing tests can see:
*
* - Go sorts the root-package leg and does NOT sort the package-dir leg;
* - Go and C# both take the FIRST occurrence of `/<segment>/` in the path, so
* a directory nested inside a same-named directory does not match;
* - Go and C# answer when the file's PARENT directory ends with the queried
* segment. Both took the FIRST occurrence of `/<segment>/` until #2881, so a
* directory nested inside a same-named one did not match;
* - C#'s `resolveDirectMatch` lets a whole-path match win over a suffix match
* found EARLIER in iteration order, while `resolveByProgressiveStripping`
* takes whichever comes first;
* - Dart tries `lib/<rel>` fully before bare `<rel>`, and compares raw paths
* (no backslash normalization) on both legs.
*
* So this file keeps verbatim copies of the pre-change implementations and
* asserts the new ones agree with them on a deterministic corpus built to force
* exactly those cases. The copies are the specification; if a future change
* makes one of these fail, the resolver's OUTPUT moved and the graph's edges
* move with it.
* So this file keeps copies of the pre-change implementations and asserts the
* new ones agree with them on a deterministic corpus built to force exactly
* those cases. The copies are the specification; if a future change makes one of
* these fail, the resolver's OUTPUT moved and the graph's edges move with it.
*
* They were verbatim until #2881, which deliberately changed one rule and so had
* to edit them too. Read them now as an independent re-derivation of the CURRENT
* spec, not as a frozen record of what shipped before the hoist — a weaker claim,
* and the reason the hand-built arm below pins ABSOLUTE expectations as well as
* differential ones: a differential where both sides were edited together proves
* only self-consistency.
*
* The second half asserts the index is built once per file set rather than once
* per import, by counting how often the Set is iterated. It is the DETERMINISTIC
@ -57,7 +64,7 @@ import { csharpSuffixFallbackAllowed } from '../../../src/core/ingestion/csharp-
import { DART_HERITAGE_PREFIX } from '../../../src/core/ingestion/languages/dart/interpret.js';
import { CountingSet } from '../../helpers/counting-file-set.js';
// ─── verbatim pre-change implementations ─────────────────────────────────────
// ─── pre-change implementations, minus the rule #2881 removed ────────────────
function legacyFindRootPackageFiles(allFilePaths: ReadonlySet<string>): string[] {
const result: string[] = [];
@ -303,11 +310,17 @@ function mix(n: number): number {
}
/**
* Directory shapes, chosen so the corpus contains every case where the naive
* "does the dir end with the segment" rewrite diverges from the original
* first-`indexOf` predicate: a directory name nested inside itself
* Directory shapes, chosen so the corpus contains every case where the two
* candidate predicates disagree: a directory name nested inside itself
* (`pkg/pkg`, `a/pkg/b/pkg`), the same leaf under several parents (collision
* tie-breaks), an absolute-rooted layout, and the repo root.
*
* The nested shapes were originally here to prove the "does the dir end with
* the segment" rewrite was NOT safe, because the shipped predicate additionally
* required the first `indexOf` occurrence. #2881 removed that requirement and
* made the ends-with form the shipped one, so these shapes now pin the removal
* instead — same shapes, opposite verdict, and still the only ones that can
* tell the two apart.
*/
const DIRS = [
'',

18
gitnexus/test/unit/scope-resolution/java-import-target-parity.test.ts
View file

@ -25,8 +25,9 @@
* - a wildcard import drops its trailing `.*` before any of that runs;
* - paths are compared normalized (`\` → `/`) but returned RAW.
*
* So this file keeps a VERBATIM copy of the pre-change implementation — the
* `resolveJavaImportTarget` that shipped before #2908, scans and all — and
* So this file keeps a copy of the pre-change implementation — the
* `resolveJavaImportTarget` that shipped before #2908, scans and all, minus the
* one rule #2881 deliberately removed (see tie-break 3) — and
* asserts the new one agrees with it, both on hand-built corpora built to force
* exactly those cases and on a generated corpus replayed under three insertion
* orders — order being the only channel most of these tie-breaks travel on.
@ -49,7 +50,7 @@ import type { ParsedImport, WorkspaceIndex } from 'gitnexus-shared';
import { resolveJavaImportTarget } from '../../../src/core/ingestion/languages/java/import-target.js';
import { CountingSet } from '../../helpers/counting-file-set.js';
// ─── verbatim pre-change implementation ──────────────────────────────────────
// ─── pre-change implementation, minus the rule #2881 removed ─────────────────
interface LegacyJavaResolveContext {
readonly fromFile: string;
@ -84,8 +85,7 @@ function legacyResolveJavaImportTarget(
let exactFile: string | null = null;
let suffixFile: string | null = null;
let directoryChild: string | null = null;
const dirPrefix = `${pathLike}/`;
const suffixDirPrefix = `/${dirPrefix}`;
const suffixDirPrefix = `/${pathLike}/`;
for (const raw of ctx.allFilePaths) {
const f = raw.replace(/\\/g, '/');
@ -121,8 +121,7 @@ function legacyResolveJavaImportTarget(
if (tail === '') continue;
const tailFile = `${tail}.java`;
const tailSuffix = `/${tailFile}`;
const tailDir = `${tail}/`;
const tailSuffixDir = `/${tailDir}`;
const tailSuffixDir = `/${tail}/`;
let tailDirectChild: string | null = null;
for (const raw of ctx.allFilePaths) {
const f = raw.replace(/\\/g, '/');
@ -230,6 +229,11 @@ const HAND_CASES: readonly Case[] = [
target: 'com.example',
},
{
// Kept beside `self-nested-directory-answers-the-outer-package` even though
// both now resolve to the same file: this one addresses the FULL path and
// hits the exact-file/suffix tier, that one addresses the outer package and
// hits the directory-child tier. Before #2881 the pair separated a hit from
// a null; it now separates two tiers, which is why it is still two cases.
label: 'self-nested-directory-matches-full-path',
files: ['com/example/com/example/Deep.java'],
target: 'com.example.com.example',

16
gitnexus/test/unit/scope-resolution/kotlin/kotlin-import-target-parity.test.ts
View file

@ -146,13 +146,6 @@ describe('resolveKotlinImportTarget — index parity', () => {
expect(resolve(['top/data/Repo.kt'], 'data.something')).toEqual(['top/data/Repo.kt']);
});
it('a repeated name that is NOT the parent directory stays out of the bucket', () => {
// The rule is "the parent directory is named `s`", not "`s` appears
// anywhere in the path" — dropping the two guards must not widen it that
// far. `data` is a component of the path but the parent is `mid`.
expect(resolve(['top/data/mid/Repo.kt'], 'data.something')).toBeNull();
});
it('the package directory is derived from the normalized path, not the raw one', () => {
// A backslash path whose `dirChildren` key must come from the normalized
// form: reading the raw path's last separator instead would make the whole
@ -164,8 +157,15 @@ describe('resolveKotlinImportTarget — index parity', () => {
expect(resolve(['win\\pkg\\A.kt'], 'pkg.someFunction')).toEqual(['win\\pkg\\A.kt']);
});
it('a path starting with the directory name is not a child of it unless direct', () => {
it('a name that is not the PARENT directory stays out of the bucket', () => {
// The rule is "the parent directory is named `s`", not "`s` appears
// anywhere in the path" — dropping the two guards must not widen it that
// far. Leading and mid-path, since those were the two positions the guards
// distinguished; the new implementation has no positional logic at all, so
// one case would do, and the second is kept only because it is the exact
// shape the widened cases above use with the last segment changed.
expect(resolve(['data/sub/Repo.kt'], 'data.something')).toBeNull();
expect(resolve(['top/data/mid/Repo.kt'], 'data.something')).toBeNull();
expect(resolve(['data/Repo.kt'], 'data.something')).toEqual(['data/Repo.kt']);
});

107
gitnexus/test/unit/scope-resolution/kotlin/kotlin-index-internals.test.ts Normal file
View file

@ -0,0 +1,107 @@
/**
* Structural guard for the two `getKotlinFileIndex` optimizations added in
* #2881 — the per-directory key memo and the bucket compaction.
*
* Both are output-identical by construction, which is why they exist and also
* why nothing else in the repo watches them. `bench/kotlin-import-target`'s
* correctness fingerprint observes the index only through the four resolver
* tiers, so it cannot see a Map key-ORDER move, and reverting either
* optimization leaves every arm of both benches green (the heap ceiling is a
* 1.5x bound, not an equality). Without this file the memo's safety argument —
* "the key list is a pure function of `dir`, so interning it per directory
* cannot change the key set, the key insertion order or any bucket's order" —
* is asserted nowhere.
*
* The index is module-private, so these assertions run through the resolver's
* observable surface and reconstruct what they need:
*
* - bucket CONTENTS and ORDER come from the fan-out tier, which hands out the
* bucket array itself;
* - bucket IDENTITY across calls proves the memoization, and `Object.isFrozen`
* proves the compaction pass ran over the array actually handed out;
* - the memo HIT path is reached only by a second file in the same directory,
* which is the case a single-file corpus never exercises.
*
* A key-order assertion is deliberately absent: `dirChildren` is only ever read
* by `.get(key)`, so Map key order has no consumer and pinning it would assert
* an implementation detail nothing depends on.
*/
import { describe, expect, it } from 'vitest';
import type { ParsedImport } from 'gitnexus-shared';
import { resolveKotlinImportTarget } from '../../../../src/core/ingestion/languages/kotlin/import-target.js';
function bucket(files: readonly string[], targetRaw: string, set = new Set(files)) {
const parsed = { kind: 'named', localName: 'X', importedName: 'X', targetRaw } as ParsedImport;
return resolveKotlinImportTarget(parsed, { fromFile: 'App.kt', allFilePaths: set } as never);
}
describe('getKotlinFileIndex internals (#2881)', () => {
it('the memo hit path produces the same bucket as the miss path', () => {
// Every file after the first in `pkg/` takes the memo, so a divergence
// between the two paths shows up as a missing or reordered member. The
// per-file form and the memoized form must agree element for element.
const files = ['a/b/pkg/One.kt', 'a/b/pkg/Two.kt', 'a/b/pkg/Three.kt', 'a/b/pkg/Four.kts'];
expect(bucket(files, 'pkg.someTopLevelFun')).toEqual(files);
expect(bucket(files, 'b.pkg.someTopLevelFun')).toEqual(files);
expect(bucket(files, 'a.b.pkg.someTopLevelFun')).toEqual(files);
});
it('two directories sharing a component-suffix keep separate buckets', () => {
// The memo is keyed on the full `dir`. Keying it on the last segment — the
// one way this optimization can move an answer — would hand `x/pkg`'s key
// list to `y/pkg` and merge them.
const files = ['x/pkg/One.kt', 'y/pkg/Two.kt'];
expect(bucket(files, 'x.pkg.fn')).toEqual(['x/pkg/One.kt']);
expect(bucket(files, 'y.pkg.fn')).toEqual(['y/pkg/Two.kt']);
// `pkg` alone is a component-suffix of both, so it legitimately holds both,
// in file-set iteration order.
expect(bucket(files, 'pkg.fn')).toEqual(files);
});
it('the bucket handed out is frozen, compacted and the same object every call', () => {
// `slice()` in the freeze loop must not break the caching contract: the
// fan-out tier hands out the cached array, and the first-child tier reads
// `[0]` from the same one, so a downstream `sort()` would reorder the index
// itself. Freezing makes that a loud TypeError.
const set = new Set(['pkg/One.kt', 'pkg/Two.kt']);
const first = bucket([], 'pkg.fn', set) as readonly string[];
const second = bucket([], 'pkg.fn', set) as readonly string[];
expect(first).toBe(second);
expect(Object.isFrozen(first)).toBe(true);
expect(() => (first as string[]).push('pkg/Three.kt')).toThrow(TypeError);
});
it('a single-child bucket is frozen too, on the length === 1 skip path', () => {
// Compaction skips `slice()` for a bucket that never grew. That branch must
// still freeze, or exactly the packages with one file stay mutable.
const set = new Set(['solo/One.kt']);
const only = bucket([], 'solo.fn', set) as readonly string[];
expect(only).toEqual(['solo/One.kt']);
expect(Object.isFrozen(only)).toBe(true);
});
it('a package larger than V8 s first growth step keeps every member in order', () => {
// 40 files takes the backing store through 1 -> 17 -> 41, so this is the
// shape compaction actually reclaims. Nothing may be lost or reordered.
const files = Array.from({ length: 40 }, (_, i) => `big/Item${i}.kt`);
expect(bucket(files, 'big.fn')).toEqual(files);
});
it('the memo keys on the NORMALIZED directory while storing raw paths', () => {
// `dir` is now sliced from `stem` rather than from `norm`. Both are the
// backslash-normalized form and an extension holds no '/', so the last
// separator is the same character at the same index — but only the KEY is
// normalized; the memo must not leak that into the stored value, which
// stays the raw path the file set holds.
expect(bucket(['win\\pkg\\A.kt', 'win\\pkg\\B.kt'], 'win.pkg.fn')).toEqual([
'win\\pkg\\A.kt',
'win\\pkg\\B.kt',
]);
// Same directory reached by its component-suffix, i.e. through the memo's
// second and later keys rather than the full-dir key.
expect(bucket(['win\\pkg\\A.kt', 'win\\pkg\\B.kt'], 'pkg.fn')).toEqual([
'win\\pkg\\A.kt',
'win\\pkg\\B.kt',
]);
});
});