fix(ingestion): two-channel binding lifecycle (closes #1066) + scope-resolution I8 hardening#1082
Merged
Merged
Conversation
…oss-namespace siblings (#1066) Two coupled regressions surfaced when analyzing real-world C# repos with large source files (issue #1066): 1. Tree-sitter `parser.parse()` is hard-coded to a 32 KB buffer by default. Any file exceeding that threshold throws `Invalid argument` on the worker re-parse path of `populateCsharpNamespaceSiblings` (and the analogous Python / TypeScript captures fallbacks). 2. After the buffer fix unblocks the AST walk, the hook tries to `push()` onto the inner `BindingRef[]` array fetched from `indexes.bindings` — but `materializeBindings` froze that array via `Object.freeze(refs.slice())`. Result: `Cannot add property N, object is not extensible`. Fixes: - `csharp/captures.ts`, `python/captures.ts`, `typescript/captures.ts`: pass `bufferSize: getTreeSitterBufferSize(sourceText.length)` to `parser.parse()` on the cache-miss path so multi-MB files parse. - `csharp/namespace-siblings.ts`: introduce `cloneBindingBucket` to copy the frozen array before mutating, then `set()` the new array back. This is a working but architecturally compromised workaround (#1050 follow-up will replace it with an explicit augmentation channel — see docs/plans/2026-04-26-001 plan). Tests: - New `csharp-large-cache-miss-resolution` fixture (Models/Services/ Other layout, ~77 KB padded UserService.cs) drives the buffer-size failure end-to-end through worker mode. - `csharp.test.ts`: 4 new regression assertions covering both the parse-time buffer-size failure and the freeze workaround. - Per-language captures unit tests gain "large cache-miss file uses adaptive buffer" coverage (TS, Python, C#). - `csharp-hooks.test.ts`: in-memory freeze regression test that reproduces the `Cannot add property` crash without invoking the C# parser at all. Made-with: Cursor
Step 1 of the binding-augmentation-channel refactor (issue #1066 follow-up). Pure shape change — no consumers yet. Adds a new `readonly bindingAugmentations` field to `ScopeResolutionIndexes` initialized as an empty `Map` by `finalizeScopeModel`. The new channel is the dedicated post-finalize write target for hooks like `populateCsharpNamespaceSiblings`, so `indexes.bindings` can stay frozen and finalize-owned. Behavior unchanged: nothing reads or writes the new field yet. tsc and the full unit suite remain green. Plan: docs/plans/2026-04-26-001-binding-augmentation-channel.md (local only — `docs/plans/` is gitignored). Made-with: Cursor
Step 2 of the binding-augmentation-channel refactor. Introduces a single primitive every walker uses to read both the finalize-owned `indexes.bindings` channel and the post-finalize `indexes.bindingAugmentations` channel. Contract: - Finalized refs come first (preserves existing precedence). - Augmented refs append, deduped by `def.nodeId`. - Empty input on both channels returns a shared frozen empty array. - Single-channel hits return the bucket by reference (no allocation). No consumers are wired yet — Step 3 routes the existing walker primitives through this helper. Augmentations remain empty for every language; behavior of the full suite is unchanged. 8 unit tests pin precedence, dedup, identity for single-channel hits, and the shared-empty-frozen-array sentinel. Made-with: Cursor
…ngsAt Step 3 of the binding-augmentation-channel refactor. Every direct `indexes.bindings.get(...)` consumer in the post-finalize phase is now routed through `lookupBindingsAt` (per-name) or `namesAtScope` + `lookupBindingsAt` (bulk iteration). Routed sites: - `findClassBindingInScope` (walkers.ts) — class-receiver lookups. - `findCallableBindingInScope` (walkers.ts) — free-call lookups. - `findExportedDefByName` (walkers.ts) — module-scope-fallback callable lookups. - `propagateImportedReturnTypes` (passes/imported-return-types.ts) — bulk iteration over an importer's binding entries; switched to `namesAtScope` + per-name `lookupBindingsAt` so post-finalize augmentations are visible to import-derived typeBinding mirrors. Behavior unchanged: augmentations are empty across the suite (Step 4 populates them for C# `populateNamespaceSiblings`). 587 scope-resolution unit tests + 50 integration resolver suites green (4 pre-existing Swift method-implements failures unrelated to this work). Adds `namesAtScope` companion helper for the bulk-iteration callers. Made-with: Cursor
…annel Step 4 of the binding-augmentation-channel refactor. The C# `populateNamespaceSiblings` hook is the only consumer that needed to inject cross-file bindings post-finalize, and prior to this change it cloned the (frozen) finalized `BindingRef[]` arrays through a `cloneBindingBucket` helper, then `set()`-back the new array — a workaround for the `Object.freeze` applied by `finalize-algorithm.ts` (issue #1066 root cause). Architecturally that violated `ScopeResolver` Invariant I8 (which permits post-finalize modifications but not in-place mutation of finalized buckets). It also forced read-side consumers to be aware of the workaround. This change: * Switches the three C# write sites to append into `indexes.bindingAugmentations` via `getAugmentationBucket`. The augmentation channel was added in Step 1 and is mutable by contract: inner `BindingRef[]` arrays here are NEVER frozen. * Deletes `cloneBindingBucket` and `getMutableScopeBindings` (workaround helpers no longer needed). * `lookupBindingsAt` (Step 2) merges the two channels transparently for every walker (Step 3), so behavior is unchanged for callers. * Updates the unit test to assert against both channels: finalized bucket stays frozen and untouched, cross-file siblings show up in augmentations only. Renamed the test accordingly. Validation: * `npx tsc --noEmit` clean. * csharp hooks unit + walkers-augmentations unit + csharp integration resolver suite all green (236/236). * Wider `test/unit/scope-resolution test/integration/resolvers` suite: 2507 pass, only 4 pre-existing Swift METHOD_IMPLEMENTS failures remain (unrelated to this work, present on baseline). Refs: issue #1066, ADR-pending binding-augmentation-channel. Made-with: Cursor
… dev guard Step 5 of the binding-augmentation-channel refactor. Captures the new two-channel binding lifecycle in the contract docs and adds a dev-mode runtime validator so a future hook cannot silently drift back into mutating `indexes.bindings`. Contract changes: * `contract/scope-resolver.ts` — rewrote Invariant I8 to describe the two channels (`indexes.bindings` is finalize-output and immutable post-finalize; `indexes.bindingAugmentations` is the append-only post-finalize channel populated by hooks like `populateNamespaceSiblings`). Documented `lookupBindingsAt` as the read-side merger and pointed at the new validator as the enforcement mechanism. * `gitnexus-shared/src/scope-resolution/types.ts` — extended the module-header lifecycle contract to call out `bindingAugmentations` alongside `ReferenceIndex` as the two structures populated after the freeze. Validator: * New `pipeline/validate-bindings-immutability.ts` mirrors the shape of `validateOwnershipParity` (#909): runs only when `NODE_ENV !== 'production' && VALIDATE_SEMANTIC_MODEL !== '0'`, emits via `onWarn`, never throws. Asserts (a) every inner `BindingRef[]` in `indexes.bindings` is `Object.isFrozen`, and (b) every inner array in `indexes.bindingAugmentations` is NOT frozen. * Wired into `pipeline/run.ts` after both `populateNamespaceSiblings` and `propagateImportedReturnTypes`, before `resolveReferenceSites`. One sweep covers the full post-finalize surface. Tests: * `validate-bindings-immutability.test.ts` — 6 cases pinning happy path, both drift directions, multi-violation accumulation, and both production no-op gates. All scope-resolution + csharp resolver tests green (242/242 in the focused run; matches the wider Step 4 baseline). Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
CI Report✅ All checks passed Pipeline Status
Test Results
✅ All 7469 tests passed 97 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
Tree-sitter buffer sizing is byte-based, so computing adaptive buffers from JavaScript string length under-sized UTF-8-heavy files. Make getTreeSitterBufferSize accept source text directly and compute Buffer.byteLength internally, then update all parse call sites and max-buffer skip checks to use byte length. Add multibyte cache-miss and cap regressions for C#, Python, TypeScript, and the C# namespace-sibling fallback parse path. Made-with: Cursor
magyargergo
changed the title
Add focused unit coverage for augmented-only binding reads across the routed walker helpers and imported-return-type propagation path. Clarify I8 wording around lexical Scope.bindings versus post-finalize index channels, and document the intentional local-only behavior of findExportedDef. Also switch the immutability validator tests to Vitest env stubs, document one intentional validator blind spot, and split C# namespace-sibling tests so UTF-8 parsing and augmentation-channel behavior are asserted independently. Made-with: Cursor
Replace high-cardinality large-file capture fixtures with large padding plus a trailing declaration. This still proves adaptive tree-sitter buffers parse beyond large ASCII and UTF-8-heavy input, without making query matching process thousands of declarations and risking timeouts. Made-with: Cursor
… regressions Add worker-mode resolver integration coverage mirroring the C# #1066 scenario for Python and TypeScript. Each test builds a temp fixture with large ASCII and UTF-8-heavy source padding, then asserts trailing declarations and call edges still resolve after scope-resolution cache-miss reparsing. Made-with: Cursor
Contributor
|
Claude finished @magyargergo's task in 6m 8s —— View job PR #1082 — Adversarial Production Readiness ReviewReview bar for this PR
Understanding of the changeIssue #1066 produced two crashes in C# scope resolution:
The two-channel lifecycle fixes problem 2 architecturally:
Runtime paths affected: Shared contracts affected: What must remain unchanged: C# cross-namespace visibility, Findings[low] Stale regression test comment describes the removed workaround, not the final fix
[low]
|
| Item | Status |
|---|---|
| Issue #1066 fixed through real C# ingestion/scope-resolution path (not just isolated helper) | satisfied — integration test forces worker-pool (cache-miss) at csharp-large-cache-miss-resolution |
Finalized indexes.bindings not mutated post-finalize; cloneBindingBucket fully removed |
satisfied — grep confirms zero remaining references; C# writes exclusively to bindingAugmentations |
Every reader requiring augmented bindings uses lookupBindingsAt / namesAtScope |
satisfied — all four readers confirmed; findExportedDef's intentional bypass is correctly scoped to target-side local-export queries and documented |
| I8 contract is clear, enforceable, consistent across shared types, contract docs, and runtime | satisfied — types.ts, scope-resolution-indexes.ts, scope-resolver.ts I8 section all consistent |
validateBindingsImmutability is production no-op, dev-mode meaningful, correct placement |
satisfied — gated on process.env.NODE_ENV !== 'production' + VALIDATE_SEMANTIC_MODEL !== '0'; runs after all post-finalize hooks, before resolveReferenceSites |
| Adaptive tree-sitter buffer uses UTF-8 byte length; all call sites correct | satisfied — Buffer.byteLength(sourceText, 'utf8'); confirmed in C#, Python, TypeScript captures, parse-worker, sequential parser, call/import/heritage processors |
| C#, Python, TypeScript capture behavior preserved (buffer-size fix only) | satisfied — no other logic changes in captures; bidirectional reversal confirms architectural pieces carry load |
| Tests prove both crash modes and architectural necessity | satisfied — 4 C# regression integration tests + 8 walkers-augmentations unit tests + 6 validate-bindings-immutability unit tests + bidirectional reversal documented in PR |
Validation assessment
Tests added/updated:
walkers-augmentations.test.ts— 8 tests coveringlookupBindingsAtprecedence, dedup, empty fast-path, shared empty array identity, and walker helpers via augmentationsvalidate-bindings-immutability.test.ts— 6 tests covering happy path, mutable bindings violation, frozen augmentations violation, multi-scope counting, production no-op,VALIDATE_SEMANTIC_MODEL=0no-opcsharp.test.ts— 4 integration tests for issue Analysis failed: Phase 'scopeResolution' failed: Cannot add property 2, object is not extensible #1066 exercising the worker-pool pathingestion-utils.test.ts— 9 buffer-size tests covering ASCII, multibyte CJK, boundaries, exact 32 MB capcsharp-hooks.test.ts,csharp-captures.test.ts,python-fixtures.test.ts,typescript-captures.test.ts,imported-return-types.test.ts— targeted unit coverage additions
Validation claimed by PR:
npx tsc --noEmitcleannpx prettier --checkclean (.cursor/hooks.jsonuntracked workspace file warning unrelated)- Full vitest: 7235/7327 locally (6 failures: 4 Swift
METHOD_IMPLEMENTS, 1 Swift method-extraction, 1 LadybugDB lock) - Targeted issue Analysis failed: Phase 'scopeResolution' failed: Cannot add property 2, object is not extensible #1066 regression tests green
- Bidirectional reversal proof (both write-side and read-side revert produce regressions)
Validation verified from diff/CI:
- CI shows 7462/7559 tests pass, 0 failures, 97 skipped — consistent with PR description. The 97 skips are exclusively Swift tests (confirmed by CI detail). The local/CI count discrepancy is explained by newly added tests and Swift skips counted differently locally.
tsc --noEmitconfirmed by CITypecheck: success.- Test count delta (7559 CI vs 7327 local) is accounted for by the ~18 new tests added in commits 2–6 and the 97 Swift tests being skipped on CI vs failing locally.
- The bidirectional reversal (step 7 in the PR plan) is documented with specific error messages — convincing evidence.
Missing/not-verifiable validation:
gitnexus-shared/build correctness: types.ts change is purely additive documentation (no new exports, no structural changes). CI confirms typecheck passes, which validates the shared package is consistent. The DoD §4.4npm run buildingitnexus-shared/is not explicitly called out, but the CIsetup-gitnexusaction handles build ordering per.github/config. Not blocking.- Python/TypeScript large-file cache-miss integration coverage is absent (see finding above). Not blocking.
Final verdict
production-ready
The two-channel binding lifecycle is architecturally correct and fully wired: indexes.bindings is permanently immutable post-finalize, indexes.bindingAugmentations is the only sanctioned mutation channel, and every reader that needs augmented visibility routes through lookupBindingsAt. Reader migration is complete — confirmed by static analysis (zero stale .bindings.get() calls in ingestion code) and by unit tests proving walker helpers see augmentation-only refs. The findExportedDef bypass is correctly scoped and documented. The tree-sitter buffer fix uses Buffer.byteLength at all call sites. The validateBindingsImmutability guard is correctly gated and correctly positioned in the pipeline. CI is clean with zero failures; all 97 skips are pre-existing Swift tests unrelated to this change. Three findings are identified, all LOW severity, none blocking: a stale test comment, a documented validator blind spot, and absent Python/TypeScript large-file integration coverage. The first is a cosmetic fix that can follow in a quick cleanup; the other two are acknowledged design tradeoffs.
Addresses SPARC reviewer feedback on the binding-augmentation channel: - Validator gate is now opt-in outside development. Extract isSemanticModelValidatorEnabled() in utils/env.ts as the single predicate; both validateBindingsImmutability and phase.ts's warn handler share it. Default CLI runs no longer pay the O(binding-buckets) scan, and explicit VALIDATE_SEMANTIC_MODEL=1 now emits warnings even when NODE_ENV is unset. - namesAtScope returns Iterable<string> and zero-allocates when at most one channel is populated (returns Map.keys() directly), only materializing a Set when both channels carry names. The caller-side branching and EMPTY_NAMES escape hatch in propagateImportedReturnTypes are gone -- both helpers handle the empty-augmentation case internally. - C# namespace-siblings header/JSDoc, model JSDoc, I8 contract prose, and the #1066 integration-test header rewritten to say post-finalize fanout appends only to bindingAugmentations; finalized refs come first and win duplicate def.nodeId metadata; local lexical Scope.bindings remains the first-tier shadowing channel. Validator unit-test setup deduplicated via beforeEach and extended with default-CLI no-op + explicit-opt-in cases. Made-with: Cursor
4 tasks
This was referenced Apr 26, 2026
magyargergo
pushed a commit
that referenced
this pull request
Apr 27, 2026
…1085) The csharp-large-cache-miss-resolution fixture added in #1082 reproduces the freeze contract failure via tree-sitter cache-miss reparse on >32 KB files. This adds a complementary trigger for the same root cause that does not depend on file size: a small-file pair where the importer locally declares a class with the same simple name as a sibling reached through `using`. Pre-#1082 path: scope-extractor pre-populates (and freezes) `User` in the importer's Module bindings, then populateCsharpNamespaceSiblings' namespace-import loop calls push() on the frozen array and throws "Cannot add property N, object is not extensible", aborting the whole scopeResolution phase. Post-#1082 the augmentation channel keeps both bindings visible; the local `Collision.App.User` shadows the namespace-imported one per origin precedence, so `Program.Run -> new User()` resolves to the local class. Three assertions: - scopeResolution completes (no throw on the colliding bucket). - both `User` declarations are detected across the two namespaces. - `Program.Run -> User` constructor edge points at App/Program.cs (not Models/User.cs), verifying origin:local shadows origin:namespace. Verified: full csharp.test.ts suite green (207/207). tsc --noEmit clean. Refs: #1066, #1082, #1083 (closed as superseded).
mann1x
added a commit
to mann1x/GitNexus
that referenced
this pull request
Apr 27, 2026
…l scopes (closes abhigyanpatwari#1086) When a C# file consists of a single top-level `namespace_declaration` that ends exactly at EOF (no trailing newline, no leading content outside the namespace's `{}` body), tree-sitter-c-sharp 0.23.1 reports identical byte ranges for `compilation_unit` and `namespace_declaration`. Pre-fix the scope-extractor parent-finder relied on strict containment, so the Module was popped off the stack and the Namespace ended up with `parent === null` → `ScopeTreeInvariantError: non-module-requires-parent` → `extractParsedFile` swallowed the throw and the whole file was dropped from the registry-primary path. Cross-file IMPORTS / CALLS edges originating in or terminating at that file vanished. Hit on three real-world `*.Designer.cs` files in PersistentWindows (`HotKeyWindow.Designer.cs`, `LaunchProcess.Designer.cs`, `DbKeySelect.Designer.cs`) — all have the byte signature `<BOM><CRLF>namespace ... { ... }<EOF>` (last hex = `... 7D 0D 0A 7D`). The fix is a single carve-out in the parent-validity contract: a `Module` may parent a same-range non-`Module` child. The relationship stays acyclic because the carve-out is direction-asymmetric — only Module-as- outer parents a same-range non-Module, never the reverse. Two coordinated changes: * `gitnexus/src/core/ingestion/scope-extractor.ts` — `pass1BuildScopes` now consults a new `canParentScope` helper instead of `rangeStrictlyContains` directly. Sort tie-breaker added so a same- range Module always sorts before a non-Module candidate, ensuring the Module lands on the parent-stack first regardless of tree-sitter capture iteration order. * `gitnexus-shared/src/scope-resolution/scope-tree.ts` — `buildScopeTree`'s `parent-must-contain-child` check now uses the same `canParentScope` carve-out so the validator agrees with the extractor on what a well-formed parent edge looks like. Error message updated to spell out the new contract. `rangeStrictlyContains` keeps its strict semantics in both files — position-index lookups, hook-side range comparisons, and other call sites are unchanged. * `gitnexus/test/fixtures/lang-resolution/csharp-namespace-as-root-no-trailing-newline/` — minimal regression fixture mirroring the PersistentWindows shape: both `Models/User.cs` and `App/Program.cs` end exactly on the closing `}` of their namespace with no trailing newline. The trigger is shape- driven, not size-driven, so the fixture stays small (~250 bytes total). * New `csharp.test.ts` describe block: scope extraction completes for both files, and the cross-file `IMPORTS` edge resolves through the scope-resolution path with `reason: 'csharp-scope: using'`. * `scope-tree.test.ts`: replaced the prior "rejects child ranges identical to the parent" case with three new ones — non-Module parent still rejected at equal range; Module-as-parent of a same-range non- Module accepted (the abhigyanpatwari#1086 carve-out); Module-as-parent of another Module still rejected (the asymmetry guard). * `npx vitest run test/unit/scope-resolution test/integration/resolvers` → 2514 passed / 77 skipped / 0 failed (52 test files). * `npx tsc --noEmit` clean in both `gitnexus/` and `gitnexus-shared/`. * End-to-end on PersistentWindows (after rebuilding the Docker image with this branch): 3 prior `scope extraction failed for *.Designer.cs` warnings → 0. Pre-fix index numbers will be re-checked here once the branch is built and indexed; the existing post-abhigyanpatwari#1082 baseline is 1113 nodes / 2987 edges / 39 clusters / 97 flows. `canParentScope` is language-agnostic. Other languages whose query emits `(compilation_unit) @scope.module` plus a single same-range top-level scope can naturally hit the same byte shape on minimal files; this fix applies to all of them uniformly. Refs: abhigyanpatwari#1086 (issue with full root-cause analysis + 4-case empirical repro through `extractParsedFile`).
magyargergo
pushed a commit
that referenced
this pull request
Apr 27, 2026
…l scopes (closes #1086) (#1087) * fix(scope-resolution): allow same-range Module-as-parent for top-level scopes (closes #1086) When a C# file consists of a single top-level `namespace_declaration` that ends exactly at EOF (no trailing newline, no leading content outside the namespace's `{}` body), tree-sitter-c-sharp 0.23.1 reports identical byte ranges for `compilation_unit` and `namespace_declaration`. Pre-fix the scope-extractor parent-finder relied on strict containment, so the Module was popped off the stack and the Namespace ended up with `parent === null` → `ScopeTreeInvariantError: non-module-requires-parent` → `extractParsedFile` swallowed the throw and the whole file was dropped from the registry-primary path. Cross-file IMPORTS / CALLS edges originating in or terminating at that file vanished. Hit on three real-world `*.Designer.cs` files in PersistentWindows (`HotKeyWindow.Designer.cs`, `LaunchProcess.Designer.cs`, `DbKeySelect.Designer.cs`) — all have the byte signature `<BOM><CRLF>namespace ... { ... }<EOF>` (last hex = `... 7D 0D 0A 7D`). The fix is a single carve-out in the parent-validity contract: a `Module` may parent a same-range non-`Module` child. The relationship stays acyclic because the carve-out is direction-asymmetric — only Module-as- outer parents a same-range non-Module, never the reverse. Two coordinated changes: * `gitnexus/src/core/ingestion/scope-extractor.ts` — `pass1BuildScopes` now consults a new `canParentScope` helper instead of `rangeStrictlyContains` directly. Sort tie-breaker added so a same- range Module always sorts before a non-Module candidate, ensuring the Module lands on the parent-stack first regardless of tree-sitter capture iteration order. * `gitnexus-shared/src/scope-resolution/scope-tree.ts` — `buildScopeTree`'s `parent-must-contain-child` check now uses the same `canParentScope` carve-out so the validator agrees with the extractor on what a well-formed parent edge looks like. Error message updated to spell out the new contract. `rangeStrictlyContains` keeps its strict semantics in both files — position-index lookups, hook-side range comparisons, and other call sites are unchanged. * `gitnexus/test/fixtures/lang-resolution/csharp-namespace-as-root-no-trailing-newline/` — minimal regression fixture mirroring the PersistentWindows shape: both `Models/User.cs` and `App/Program.cs` end exactly on the closing `}` of their namespace with no trailing newline. The trigger is shape- driven, not size-driven, so the fixture stays small (~250 bytes total). * New `csharp.test.ts` describe block: scope extraction completes for both files, and the cross-file `IMPORTS` edge resolves through the scope-resolution path with `reason: 'csharp-scope: using'`. * `scope-tree.test.ts`: replaced the prior "rejects child ranges identical to the parent" case with three new ones — non-Module parent still rejected at equal range; Module-as-parent of a same-range non- Module accepted (the #1086 carve-out); Module-as-parent of another Module still rejected (the asymmetry guard). * `npx vitest run test/unit/scope-resolution test/integration/resolvers` → 2514 passed / 77 skipped / 0 failed (52 test files). * `npx tsc --noEmit` clean in both `gitnexus/` and `gitnexus-shared/`. * End-to-end on PersistentWindows (after rebuilding the Docker image with this branch): 3 prior `scope extraction failed for *.Designer.cs` warnings → 0. Pre-fix index numbers will be re-checked here once the branch is built and indexed; the existing post-#1082 baseline is 1113 nodes / 2987 edges / 39 clusters / 97 flows. `canParentScope` is language-agnostic. Other languages whose query emits `(compilation_unit) @scope.module` plus a single same-range top-level scope can naturally hit the same byte shape on minimal files; this fix applies to all of them uniformly. Refs: #1086 (issue with full root-cause analysis + 4-case empirical repro through `extractParsedFile`). * refactor(scope-resolution): export canParentScope from gitnexus-shared Addresses #1087 review (medium): the helper was previously duplicated byte-for-byte in `scope-extractor.ts` and `scope-tree.ts`. Per DoD "single source of truth in shared", the contract piece belongs in gitnexus-shared (Ring 2 SHARED #912) and the consuming layer should import it. Eliminates the silent-drift surface where a future edit to one copy would produce extractor/validator disagreement on what a well-formed parent edge looks like. Changes: - gitnexus-shared/src/scope-resolution/scope-tree.ts: add `export` to `canParentScope`. - gitnexus-shared/src/index.ts: re-export `canParentScope`. - gitnexus/src/core/ingestion/scope-extractor.ts: remove the local `canParentScope` definition (and its now-unused local copy of `rangeStrictlyContains`), import from `gitnexus-shared`. The local `rangesEqual` stays — it's still used in capture-anchor logic at two unrelated sites. Validation (per DoD §4.4 — both CLI and web consumers verified): - npx tsc --noEmit clean in gitnexus/ and gitnexus-shared/ - cd gitnexus-web && npx tsc -b --noEmit clean - gitnexus-shared `npm run build` clean - Targeted: vitest run test/unit/scope-resolution test/integration/resolvers → 2522 passed / 0 failed / 77 skipped (54 files) - Full suite: vitest run → 7238 passed / 1 failed / 97 skipped. The single failure is `test/unit/ignore-service.test.ts > warns on EACCES but does not throw`, which cannot run when uid=0 (root bypasses POSIX permission checks). Pre-existing on this branch before the refactor; unrelated to scope-resolution.
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
1 participant
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.
Summary
Fixes issue #1066 and refactors the C# cross-file binding fanout so we no longer mutate finalized scope bindings — the original
cloneBindingBucketworkaround violatedScopeResolverInvariant I8 in spirit and was the wrong layer to patch.This PR consolidates everything into one coherent change:
Direct fix for Analysis failed: Phase 'scopeResolution' failed: Cannot add property 2, object is not extensible #1066 (commit
a18f6e1):getTreeSitterBufferSize) on cache-missparse()calls in C#, Python, and TypeScript captures — eliminates theInvalid argumentcrash on large files (>32KB).cloneBindingBuckethelper to copy frozenBindingRef[]arrays before pushing — fixes theCannot add property N, object is not extensiblecrash.Architectural refactor (commits
0a9679b1→c23a4620): the workaround in step 1 contradicted I8. The clean fix is a two-channel binding lifecycle:indexes.bindings— finalize-output channel, deep-frozen post-finalize, never mutated by hooks.indexes.bindingAugmentations— new append-only post-finalize channel for hooks likepopulateNamespaceSiblings.lookupBindingsAthelper.populateNamespaceSiblingsnow writes tobindingAugmentations, notbindings.cloneBindingBucketis deleted.validateBindingsImmutability(gated onNODE_ENV !== 'production' && VALIDATE_SEMANTIC_MODEL !== '0') catches drift if a future hook regresses.contract/scope-resolver.tsrewritten to capture the new contract; lifecycle comment ingitnexus-shared/types.tsextended.Commit-by-commit
a18f6e1fix(csharp): adaptive tree-sitter buffer + frozen-bucket clone for cross-namespace siblings (#1066)0a9679b1refactor(scope-resolution): add bindingAugmentations channel to indexesb88570fdfeat(scope-resolution): add lookupBindingsAt dual-source helpernamesAtScopecompanion + 8 unit tests.40e2864crefactor(scope-resolution): route binding lookups through lookupBindingsAtfindClassBindingInScope/findCallableBindingInScope/findExportedDefByName/propagateImportedReturnTypesgo through the merger.516e03f1refactor(csharp): write namespace siblings to bindingAugmentations channelcloneBindingBucket.c23a4620feat(scope-resolution): tighten I8 + add validateBindingsImmutability dev guardValidation
npx tsc --noEmitclean.npx prettier --checkclean (only.cursor/hooks.jsonwarns — untracked workspace file).METHOD_IMPLEMENTSregressions (verified on baseline pre-PR).AGENTS.md).csharp-large-cache-miss-resolution).walkers-augmentations.test.tscases green (precedence, dedup-by-def.nodeId, empty-arrays-merge, fast-path miss).validate-bindings-immutability.test.tscases green.Bidirectional regression verification (Step 7 of plan)
To prove each architectural piece carries weight, the architecture was reverted in two surgical ways and the regressions were observed before re-applying.
Reverted Step 4 alone (write target back to
indexes.bindings):✅ Same crash as Analysis failed: Phase 'scopeResolution' failed: Cannot add property 2, object is not extensible #1066. Re-applied — green.
Reverted Step 3 alone (read sites bypass
lookupBindingsAt):✅ Augmentations invisible to readers; cross-file C# resolution regresses. Re-applied — green.
Test plan
npx tsc --noEmitnpx prettier --check(touched dirs)npm test)test/integration/resolvers/csharp.test.ts -t "issue #1066"tscon every commit)Notes for reviewers
validateBindingsImmutabilityis a dev-mode-only guard. In production it is a no-op (return 0on the first line).lookupBindingsAtreturns the existing array by reference when only one channel populates it (zero allocation for the common case where augmentations are empty).Closes #1066.