scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-07 02:58:02 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 8
861 commits 117 branches 1040 tags 252 MiB
Commit graph

2 commits

Author SHA1 Message Date
Gergő Magyar
e944f90879
chore(shared): apply Ring 2 SHARED review follow-ups in one diff (#964) 
* chore(shared): apply Ring 2 SHARED review follow-ups in one diff

Aggregates all actionable follow-ups from the 9 Ring 2 SHARED PRs
(#949–#963) before proceeding to Ring 2 PKG. No behavior changes;
docstring edits, test refinements, and one structural cleanup.

## #913 (DefIndex / ModuleScopeIndex / QualifiedNameIndex)
  - Rename `freezeIndex` → `wrapIndex` across all three index builders.
    The old name implied `Object.freeze` on the wrapper, which we never
    applied; `wrapIndex` more accurately describes the lightweight
    readonly-interface wrap. Safety surface (frozen bucket arrays,
    frozen miss-empty array, readonly Maps) is unchanged.
  - Document in `buildModuleScopeIndex` JSDoc that callers must
    pre-normalize `filePath` keys (no path-separator canonicalization
    happens here). Prevents silent cross-platform misses.
  - Add an explicit hit-path freeze assertion in
    `qualified-name-index.test.ts` (the existing test covered only the
    miss-path `EMPTY` array).

## #914 (MethodDispatchIndex)
  - Differentiate the C3 and BFS test cases: both tests now use
    distinct MRO orderings so they prove the materializer stores
    whatever order the `computeMro` callback produces (not that C3 and
    BFS yield identical output).
  - Add `implementsOfCalls` counter in the first-write-wins test, and
    document the call-count contract in `MethodDispatchInput.implementsOf`
    JSDoc: `implementsOf` fires **per occurrence** in `input.owners`
    (not per unique owner); `computeMro` fires at most once per unique
    owner. Callers with expensive `implementsOf` implementations should
    pre-dedupe `owners`.

## #916 (resolveTypeRef)
  - Document the deliberate exclusion of `'Type'` from `TYPE_KINDS`
    (verified no extractor in `gitnexus/src/core/ingestion/` emits
    `type: 'Type'` for annotation-relevant symbols).
  - Rename the namespace-origin test from `'resolves ...'` to
    `'returns null for a namespace-origin binding whose def is not a
    type kind'`, matching the failure-case intent.

## #918 (shadow diff + aggregate)
  - Remove the partial re-export `export type { ShadowAgreement, ShadowDiff };`
    from `aggregate.ts` — it omitted `ShadowCallsite` and diverged
    from the top-level barrel. Consumers import all three from the
    `gitnexus-shared` entry point.
  - Fix the invalid `'wildcard'` evidence kind in `diff.test.ts` fixture
    (that kind is not a valid `ResolutionEvidence.kind`). Replaced with
    `'global-name'`, a real kind the test treats identically.

## #912 (ScopeTree / PositionIndex / makeScopeId)
  - Document the touching-boundary semantics on `PositionIndex.atPosition`:
    when siblings share a boundary point, the right (later-start) sibling
    wins per the existing innermost-wins sort contract.
  - Resolve the layer-inversion flagged by review: move `ScopeLookup`
    from `resolve-type-ref.ts` to `types.ts` (its natural home in the
    data-model layer). `scope-tree.ts` now imports `ScopeLookup` from
    `types.js` directly; the old re-export from `resolve-type-ref.ts`
    is removed per repo convention (`feedback_no_reexport`). Barrel
    export moved alongside.

## #917 (ClassRegistry / MethodRegistry / FieldRegistry)
  - Replace the dangling "try a name-match among class-like defs"
    comment in `lookupReceiverType` with explicit prose that callers
    must pre-resolve via `resolveTypeRef` if they want richer semantics.
    No behavior change — the function already returned `undefined` on
    ambiguous/missing qnames.
  - Fix `tieBreakKey.origin` default for pure Step-2 candidates.
    Type-binding-only hits no longer falsely inherit `'local'` from
    `ensureCandidate`'s neutral default; they now demote to `'import'`
    on their first type-binding hit, and only a later Step-1 lexical
    hit can upgrade them back to `'local'`. Keeps the Appendix B
    cascade faithful to the true origin.
  - Document `'global-name'` in `evidence.ts`: currently reserved for
    Ring 3's byName global index; `lookupCore` never emits it today.
    The weight stays live so `composeEvidence` remains exhaustive over
    the origin union.
  - Rename the mislabeled Step-7 test from `'confidence DESC is the
    primary key'` (which actually tested hard-shadow baseline) to
    `'inner scope shadows outer, yielding single result'`, and add a
    separate test that actually exercises multi-candidate confidence
    ordering (local vs wildcard at the same scope).

## Verification
  - `tsc --noEmit` clean (both `gitnexus-shared` and `gitnexus`)
  - `gitnexus-shared` build clean
  - Combined scope-resolution / model / shadow suite: **260/260 pass**
    (+1 from the new multi-candidate ordering test in #917)

## Not addressed (non-actionable)
  - #949 CI "failure with zero failing tests": pre-existing Swift Node 22
    grammar flake unrelated to #910 scope.
  - #950: the two non-blocking findings were already addressed in
    follow-up commit `cbac32ba` (ParsedImport discriminated union +
    `ScopeId | null` on the two hooks).
  - #915: the five in-scope findings were already addressed in
    follow-up commit `54515a7e` (dead code, unused params, multi-hop
    docs, cap-hit test, stats granularity).
  - #915 LanguageProvider.resolveImportTarget signature divergence +
    `findDefById` O(F×D) perf: tracked separately as follow-up issues
    for the Ring 3 migration window.

* chore(shared): address ce:review findings on the follow-up diff

ce:review (interactive) on PR #964 surfaced two P2s and several P3s. This
commit applies all `safe_auto` fixes + both manual tests in-line so the
PR ships with a cleaner review trail.

## P2 fixes

- **Complete `freezeIndex` → `wrapIndex` rename.** The prior commit renamed
  3 of 5 sibling index files; `method-dispatch-index.ts` and
  `position-index.ts` still carried the old name. Now all 5 helpers use
  the consistent `wrapIndex` naming.
  (maintainability + project-standards reviewers both flagged this.)

- **Add regression tests for the `recordTypeBindingHit` origin demotion.**
  The prior commit introduced the `tieBreakKey.origin = 'import'`
  demotion for Step-2-only candidates without a direct test. Added:
    - `registries.test.ts`: two Step-2-only siblings under the same
      interface, asserting deterministic DefId.localeCompare tie-break
      AND the stronger invariant that composeEvidence never emits a
      where-found signal for Step-2-only candidates (no `signals.origin`).
    - `position-index.test.ts`: touching-boundary test proving the
      right-sibling-wins rule documented in the new JSDoc.
  (testing + kieran-typescript + api-contract reviewers all flagged these gaps.)

## P3 fixes

- Fix wrong comment in `recordTypeBindingHit` that claimed Step 1 could
  later upgrade a demoted origin. Step 1 runs BEFORE Step 2 — the actual
  upgrade path is Step 3 (`seedFromOwnerScopedContributor`). Comment now
  describes execution order correctly.

- Fix inaccurate "re-exported there" comment in `index.ts`. `types.ts`
  *defines* ScopeLookup natively; it's not a re-export. Phrasing now
  says "defined in types.ts and exported from the type-export block
  above — not from this module."

- Update stale `scope-tree.ts` file-header prose that still referenced
  `ScopeLookup` as living in #916/resolve-type-ref.ts. Now points to
  `./types.js` with a cross-ref to both #916 and #917 consumers.

- Expand `atPosition` touching-boundary JSDoc to name the mechanism
  (backward scan through start-sorted array) so readers can trace the
  binary-search code to the claim.

- Add breadcrumb to `aggregate.ts` module header pointing future readers
  to `./diff.ts` / the top-level barrel for `ShadowAgreement`,
  `ShadowCallsite`, and `ShadowDiff`.

- Remove unnecessary non-null assertion in `recordTypeBindingHit`. Local
  `const existingMroDepth = ...` lets TS narrow to `number` in the
  else-branch, eliminating the `!` without behavior change.

## Verification

- `tsc --noEmit` clean (both `gitnexus-shared` and `gitnexus`)
- `gitnexus-shared` build clean
- Combined scope-resolution / model / shadow suite: **262/262 pass** (+2
  from the new origin-demotion + touching-boundary regression tests)
 2026-04-18 18:40:29 +01:00
Gergő Magyar
1bf9fb4ef1
feat(shared): ClassRegistry / MethodRegistry / FieldRegistry + 7-step lookup (#917, RFC #909 Ring 2 SHARED) (#963) 
Capstone of Ring 2 SHARED. Implements RFC §4 — the shared, scope-aware
resolution surface the rest of the semantic model feeds into.

## Modules (`gitnexus-shared/src/scope-resolution/registries/`)

  - `context.ts`         — `RegistryContext` bundling ScopeTree / DefIndex
                           / QualifiedNameIndex / ModuleScopeIndex /
                           MethodDispatchIndex + provider hooks.
                           Narrows Ring 1's opaque `RegistryContributor`
                           to concrete `OwnerScopedContributor`.
  - `tie-breaks.ts`      — `compareByConfidenceWithTiebreaks`, the RFC
                           Appendix B cascade: confidence DESC → scope
                           depth ASC → MRO depth ASC → ORIGIN_PRIORITY
                           ASC → DefId.localeCompare.
  - `evidence.ts`        — `composeEvidence(signals)` / `confidenceFromEvidence`.
                           Translates raw walk signals into the typed
                           `ResolutionEvidence[]` using authoritative
                           `EvidenceWeights`. No magic numbers.
  - `lookup-qualified.ts`— RFC §4.5. Qualified-name fast path consumed
                           by `resolveTypeRef` dotted fallback and by
                           Step 6 of lookup-core.
  - `lookup-core.ts`     — The 7-step canonical algorithm. Pure. Param-
                           eterized by `CoreLookupParams`.
  - `{class,method,field}-registry.ts`
                         — Thin wrappers over `lookupCore` that fix
                           `acceptedKinds` + `useReceiverTypeBinding` per
                           kind. `buildClassRegistry` / `buildMethodRegistry`
                           / `buildFieldRegistry` factory functions.

## RFC §4.2 algorithm contract (honored verbatim)

  1. Lexical scope-chain walk. Hard shadow on any `scope.bindings.has(name)`
     regardless of kind survivorship.
  2. Type-binding resolution (methods/fields only, opt-in via
     `useReceiverTypeBinding`). MRO walk via `MethodDispatchIndex.mroFor`.
     MRO-depth-decayed weight via `typeBindingWeightAtDepth`.
  3. Owner-scoped contributor — when the caller knows the receiver owner,
     its direct members merge in as `origin: 'local'`.
  4. Kind filter — `acceptedKinds` per registry; `kind-match` evidence
     at weight 0 is always emitted for debuggability.
  5. Arity filter — `provider.arityCompatibility` per candidate. When at
     least one compatible candidate exists, incompatibles are dropped;
     otherwise the −0.15 penalty alone disambiguates (they stay in the
     result, just ranked lower).
  6. Global fallback — fires only when Steps 1-3 produced NO candidates
     AND the name is dotted. Delegates to `lookupQualified`.
  7. Rank + tie-break — evidence list sorted by the Appendix B cascade.

## §4.7 invariants asserted in tests

  - No tier vocabulary in the return type (`Resolution`, not `TierXResult`).
  - Confidence is per-candidate (not per-tier).
  - Shadowing is a HARD filter; globals are consulted ONLY when lexically
    empty.
  - Caller can read `[0]` for one-shot answers.
  - `Resolution.confidence` is capped at 1.0.
  - `kind-match` is always emitted (weight 0).

## Unresolved-import + dynamic-unresolved evidence shape

  - `BindingRef.via.linkStatus === 'unresolved'` applies the
    `unlinkedImportMultiplier` (0.5×) to the where-found signal only.
    Corroborators (`arity-match`, `owner-match`, `type-binding`) remain
    unaffected — the RFC §4v2 capped-signal rule applies per-signal, not
    per-candidate.
  - `BindingRef.via.kind === 'dynamic-unresolved'` adds a degraded
    `dynamic-import-unresolved` evidence signal at weight 0.02.

## Tests (28 in registries.test.ts, 259/259 combined)

Organized per RFC §4.2 step so a regression localizes to the step it broke:

  - Step 1: local + walk-to-parent + hard-shadow + origin=import
  - Step 2: explicit receiver type-binding + MRO depth decay on ancestor
  - Step 3: owner-scoped contributor + owner-match
  - Step 5: drop-incompatible-when-compatible-exists + soft-penalty-when-all-
            incompatible + unknown-when-no-provider
  - Step 6: global-qualified fires only when lexically empty + never for
            non-dotted names + not consulted when lexical hit exists
  - Step 7: tie-break cascade (inner shadows outer; defId.localeCompare
            final)
  - Corroborators: unresolved-import 0.5× cap per-signal + dynamic-
            unresolved 0.02 degraded signal
  - §4.5: lookupQualified kind filter + empty on miss + deterministic defId
          order for partial classes
  - §4.7: invariants — confidence per-candidate, capped at 1.0, kind-match
          always present, [0]-for-one-shot

## Known follow-up optimizations

`collectOwnedMembers` in `lookup-core.ts` iterates `defs.byId.values()`
for each MRO hop — O(D) per call. Acceptable for Ring 2 fixtures; a
by-owner index should land before Ring 3 migrates large-workspace
languages. Tracked alongside the existing `findDefById` follow-up from
#915 review.

## Module placement

All under `gitnexus-shared/src/scope-resolution/registries/` — consistent
with the Ring 2 SHARED folder layout (#912/#913/#914/#915/#916/#918).
Slight deviation from the issue's `gitnexus-shared/src/registries/`
suggestion for consistency with siblings.

## Part of

- Parent: #909
- Depends on (code): #910, #911, #912, #913, #914, #915, #916, #918.
- Closes the Ring 2 SHARED delivery band. Unblocks Ring 2 PKG (#919–#925
  bridges to the gitnexus/ CLI package) and Ring 3 language migrations.
 2026-04-18 17:58:26 +01:00