mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-24 00:51:53 +00:00
dd3527327d
* fix: link object literal methods to exported bindings
* fix(ingestion): bridge object-literal value receivers in scope-resolution (PR #1718 review)
Addresses adversarial production-readiness review on PR #1718 / issue #1358:
- F1 (caller resolution) — setting `ownerId` on object-literal method symbols
alone is not sufficient; the scope-resolution receiver-bound resolver only
consults class-like or type-annotated bindings, so lowercase value receivers
(`export const fooService = {...}; fooService.getUser(...)`) never reach the
owner-indexed lookup. Adds a Case 5 value-receiver bridge in
receiver-bound-calls.ts that resolves the receiver name as a Const/Variable
binding, translates its def to the canonical graph node id, and emits the
CALLS edge via the owner-indexed method registry.
- F2 (boundary guard) — rewrites findObjectLiteralBindingInfo as an explicit
two-phase AST walk: Phase A tracks object-literal depth (returns null for
nested literals and pre-declarator function/class boundaries — IIFE
patterns); Phase B walks the declarator's ancestors and rejects function,
class, and block-statement containers (if / for / while / try / catch /
switch / etc.) before reaching program/export_statement. Prevents false
HAS_METHOD edges for locally-scoped or block-scoped object literals.
- F4 — drops the dead `ownerName` field from ObjectLiteralBindingInfo.
Constraint: TS/JS are scope-resolution migrated per RFC #909; the legacy
Call-Resolution DAG (call-processor.ts) is intentionally left untouched.
Tests:
- test/integration/ast-helpers-object-literal-binding.test.ts (13 cases) —
pins helper semantics: happy paths, function/arrow/class-ctor boundaries,
nested literals, block scope (if / for-of / try), IIFE, assignment
expressions without declarator.
- test/integration/object-literal-owner-resolution.test.ts (9 cases) —
drives the full pipeline against an on-disk fixture: sequential CALLS edge
emission (issue #1358 proof), worker-mode parity, negative local binding,
and nested-literal attribution boundary.
Full sweep: 2958/2958 integration + 6056/6056 unit tests pass.
* refactor(ingestion): address code-review findings on object-literal owner resolution
Multi-agent code review on the prior commit surfaced 7 actionable findings,
all walked through and applied here. None change observable behavior for
issue #1358's fix; all harden correctness, predicate stability, and test
signal.
- #1 (P1 / 3-reviewer corroboration): Case 5 in receiver-bound-calls.ts no
longer hand-builds graph.addRelationship + a dedup key. New
tryEmitEdgeWithExplicitTargetId in edges.ts takes a pre-resolved target
id (the canonical Method nodeId from the parser) and reuses every
invariant of tryEmitEdge: dedup-key format, collapse-flag honoring,
caller-id resolution, rel-id shape, mapReferenceKindToEdgeType for
read/write ACCESSES. This also lands the adversarial reviewer's "F2"
follow-up (hardcoded type: 'CALLS' for non-call sites) for free.
- #2 (P2 cross-reviewer): findValueBindingInScope's predicate inverted
from denylist ("not class-like and not callable") to explicit allowlist
matching reconcileOwnership's registration set:
Const | Variable | Property | Static. Extracted as isOwnableValueLabel
so future NodeLabel additions require an explicit opt-in.
- #6 (P2): walkScopeChain<T>() extracted; both findClassBindingInScope
and findValueBindingInScope now route through it. Local scope.bindings
are exhausted BEFORE lookupBindingsAt (imported/augmented) at every
scope level — preserves JavaScript lexical scoping where a local const
shadows an imported binding of the same name. Behavior was already
correct in findClassBindingInScope but was implicit; now it is the
walker's explicit, documented contract.
- #7 (P2): scope-walker duplication closed. findClassBindingInScope and
findValueBindingInScope reduce to thin wrappers over walkScopeChain
with their respective predicate. findClassBindingInScope keeps its
qualifiedNames + dotted-name fallback tail.
- #3 (P2): parse-worker.ts hoists `const ownerId = enclosingClassId ??
objectLiteralOwnerInfo?.ownerId` once before the symbol push, dropping
the duplicated coalesce + `as string` cast. Matches the cast-free
pattern at parsing-processor.ts:793. HAS_METHOD emit site reuses the
same hoisted local.
- #4 (P2): object-literal-owner-resolution.test.ts Test A's CALLS-edge
assertion no longer matches by name alone. .toEqual now pins the
canonical target id (Method:src/service.ts:getUser#1 via generateId),
confidence (0.85), and reason ('import-resolved'). A regression that
emits the edge at confidence=0, with the wrong reason, or against a
phantom Method node now fails the test.
- #5 (P2): worker-parity test adds a CI tripwire — when CI=1 and
dist/parse-worker.js is missing, throw at module top with a clear
message. Locally, skipIf(!hasDistWorker) keeps the fast-iteration
experience; CI cannot pass with U3 (worker-path ownerId) unverified.
Verification: tsc --noEmit clean. Targeted regression sweep on
ast-helpers-object-literal-binding (13), object-literal-owner-resolution
(9), has-method (60), cross-file-binding (40) — 122/122 pass. Full unit
sweep: 6056/6056. Integration suite: 1 pre-existing Windows-flake in
worker-pool.test.ts (passes 28/28 in isolation) unrelated to this diff.
* refactor(scope-resolution): align Const label emission with legacy DAG (PR #1718 review F1)
Eliminates the architectural fragility surfaced by PR #1718's adversarial review
Finding 1. Previously, normalizeNodeLabel('const') returned 'Variable' while
the legacy DAG parse phase emits 'Const' graph nodes (via @definition.const
capture for lexical_declaration). PR #1718's Case 5 value-receiver bridge
resolved correctly only because resolveDefGraphId happened to fall back to
simpleKey after the qualified-key miss — accidental correctness.
After this change, scope-resolution defs for `const x = ...` declarations
report def.type === 'Const', matching the graph node label. resolveDefGraphId's
qualified-key path now hits on the first try; the simple-key fallback is no
longer load-bearing for value receivers and can be tightened in future without
silently breaking Case 5.
Audit completeness verification:
- Grep `\bVariable\b` across src/core/ingestion/scope-resolution/ surfaced two
consumer sites that already accept both labels: reconcile-ownership.ts:101+168
(`def.type === 'Variable' || def.type === 'Const' || ...`) and
walkers.ts:207 isOwnableValueLabel (`Const | Variable | Property | Static`).
No language hook in src/core/ingestion/languages/ branches on
`def.type === 'Variable'` for what's actually a const declaration.
- Sentinel stress test (the full unit + integration suite run with the
renamed label in place): 6137/6137 unit tests pass; 2967/2967 integration
tests pass. One pre-existing Windows-only flake on worker-pool.test.ts when
run alongside the full integration suite (passes 28/28 in isolation,
unrelated to scope-extractor — same flake observed before this diff).
The variable mapping (`'variable' → 'Variable'`) is preserved for `var`
declarations, matching the legacy DAG's `@definition.variable` capture for
variable_declaration. The split now mirrors the parse-phase capture
distinction exactly.
Per plan docs/plans/2026-05-21-002-feat-pr1718-followups-class-instance-and-label-normalization-plan.md
U4 + U5. T1 (class-instance singleton resolution from issue #1358's second
sub-case) is deferred to a standalone pre-plan investigation, not shipped
here.
* test(ingestion): add regression coverage for issue #1358 singleton sub-cases
Closes the remaining sub-cases of issue #1358 surfaced by PR #1718's
adversarial review (Finding 4, NOTED): the class-instance singleton
(`export const fooService = new FooService();`) and the factory-pattern
singleton (`export const fooService = makeFooService();`).
Pre-plan investigation (per docs/plans/2026-05-21-002 § "Pre-Plan
Investigation Task (T1)") confirmed Outcome A for both patterns — they
already resolve end-to-end through scope-resolution's
`@type-binding.constructor` capture (languages/typescript/query.ts:489-511)
+ `propagateImportedReturnTypes` chain-follow
(scope-resolution/passes/imported-return-types.ts:114) + receiver-bound
Case 4 simple typeBinding lookup (receiver-bound-calls.ts:625). The
mechanism was wired correctly before this session; the regression-net
wasn't.
This test pins the behavior:
- Pattern 1: `caller → FooService.getUser` CALLS edge with
confidence 0.85 and reason 'import-resolved'
- Pattern 2: same edge shape via factory chain-follow (the
`@type-binding.alias` capture for `const u = find()` style)
Both assertions use exact `.toEqual([{...}])` shape pinning so a future
regression that targets a phantom Method node, emits at lower confidence,
or drops the cross-file import-resolved reason fails loudly.
Verification: 5/5 pass, 127/127 in targeted regression sweep including
object-literal-owner-resolution.test.ts, ast-helpers-object-literal-
binding.test.ts, has-method.test.ts, and cross-file-binding.test.ts.
No production code change. The class methods get a class-qualified node id
(`Method:src/service.ts:FooService.getUser#1`) distinguishing them from
same-name methods on other classes — distinct from the bare-name node id
shape PR #1718's object-literal case uses.
* test(resolvers): add class-instance + factory-pattern singleton coverage for TS/JS (issue #1358)
Closes the remaining sub-cases of issue #1358 surfaced by PR #1718's
adversarial review (Finding 4). PR #1718 fixed object-literal-shorthand
singletons (`export const fooService = { getUser() {} }`); this commit adds
parallel coverage for the two other singleton shapes that resolve through
the existing scope-resolution chain:
// Pattern 1 — class-instance singleton
export class FooService { getUser(id) { ... } }
export const fooService = new FooService();
// Pattern 2 — factory-pattern singleton
export class FooService { getUser(id) { ... } }
export function makeFooService() { return new FooService(); }
export const fooService = makeFooService();
Pre-plan investigation (per local plan docs/plans/2026-05-21-002 § "Pre-Plan
Investigation Task (T1)") confirmed Outcome A — both patterns already
resolve end-to-end through:
- `@type-binding.constructor` capture (languages/{typescript,javascript}/
query.ts) seeds `fooService → FooService` at parse time
- `propagateImportedReturnTypes` (scope-resolution/passes/
imported-return-types.ts:114) mirrors the typeBinding cross-file
- Receiver-bound Case 4 simple typeBinding lookup
(scope-resolution/passes/receiver-bound-calls.ts:625) MRO-walks
FooService and emits the CALLS edge to getUser
Tests added per language × pattern (5 each, 10 total):
- node existence (Class, Method, Function, Const, plus Function for the
factory pattern's `makeFooService`)
- HAS_METHOD edge from class to method (class-instance variant)
- CALLS edge from caller to `getUser` with `targetFilePath: 'src/service.{ts,js}'`,
`reason: 'import-resolved'`, `confidence: 0.85` — exact `.toEqual([{...}])`
shape pinning so a regression that emits at lower confidence or drops the
cross-file reason fails loudly
Fixtures placed under the existing `test/fixtures/lang-resolution/` convention.
Tests appended to `test/integration/resolvers/{typescript,javascript}.test.ts`,
matching the in-file pattern of every other resolver scenario.
Also supersedes and removes the standalone
`test/integration/class-instance-and-factory-singleton-resolution.test.ts`
introduced earlier in this PR session (`0df91b77`) — the proper home for
language-resolver scenarios is the per-language resolver test file alongside
similar fixtures (`javascript-self-this-resolution`, `javascript-cross-file`,
`typescript-tsconfig-paths`, etc.). One canonical location for the scenario,
not two.
Verification: 10/10 new singleton tests pass; 297/297 full TS+JS resolver
suite pass (no regression in any existing resolver test).
* test(resolvers): gate TS/JS singleton tests behind scope-resolution parity (CI run 26223603426)
The class-instance and factory-pattern singleton CALLS-edge resolution
tests added in
|
||
|---|---|---|
| .. | ||
| call-routing | ||
| group | ||
| import-resolution | ||
| integrations | ||
| mcp | ||
| model | ||
| named-bindings | ||
| scope-resolution | ||
| shadow | ||
| workers | ||
| ai-context.test.ts | ||
| analyze-api.test.ts | ||
| analyze-community-skills-gate.test.ts | ||
| analyze-embeddings-limit.test.ts | ||
| analyze-heap-respawn.test.ts | ||
| analyze-job.test.ts | ||
| analyze-no-stats-bridge.test.ts | ||
| analyze-respawn-progress-terminal.test.ts | ||
| analyze-wal-error.test.ts | ||
| analyze-worker-pool-size.test.ts | ||
| analyze-worker-timeout.test.ts | ||
| api-file-route.test.ts | ||
| api-graph-streaming.test.ts | ||
| api-query-readonly-wiring.test.ts | ||
| api-readonly-wiring.test.ts | ||
| ast-cache.test.ts | ||
| ast-utils.test.ts | ||
| binding-accumulator.test.ts | ||
| bm25-search.test.ts | ||
| call-attribution-issue-1166.test.ts | ||
| call-extraction.test.ts | ||
| call-form.test.ts | ||
| call-processor.test.ts | ||
| calltool-dispatch.test.ts | ||
| chunker.test.ts | ||
| cli-commands.test.ts | ||
| cli-index-help.test.ts | ||
| cli-message.test.ts | ||
| cobol-copy-expander.test.ts | ||
| cobol-preprocessor.test.ts | ||
| cohesion-consistency.test.ts | ||
| community-processor.test.ts | ||
| compatible-stdio-transport.test.ts | ||
| cors.test.ts | ||
| cpp-ue-preprocessor.test.ts | ||
| cross-file-impl.test.ts | ||
| cross-file.test.ts | ||
| csv-escaping.test.ts | ||
| cursor-hook.test.ts | ||
| dart-import-resolver.test.ts | ||
| dart-type-extractor.test.ts | ||
| detect-changes-worktree.test.ts | ||
| embedder.test.ts | ||
| embedding-chunking.test.ts | ||
| embedding-config.test.ts | ||
| embedding-pipeline.test.ts | ||
| entry-point-scoring.test.ts | ||
| esm-extension-resolution.test.ts | ||
| eval-formatters.test.ts | ||
| exact-search.test.ts | ||
| expo-routes.test.ts | ||
| extract-element-type-from-string.test.ts | ||
| extract-generic-type-args.test.ts | ||
| fetch-reason-parsing.test.ts | ||
| field-extraction.test.ts | ||
| framework-detection.test.ts | ||
| git-clone.test.ts | ||
| git-utils.test.ts | ||
| git.test.ts | ||
| graph.test.ts | ||
| group-service-not-found.test.ts | ||
| has-method.test.ts | ||
| heritage-extraction.test.ts | ||
| heritage-map.test.ts | ||
| heritage-processor.test.ts | ||
| hf-env.test.ts | ||
| hooks.test.ts | ||
| http-embedder.test.ts | ||
| hybrid-search.test.ts | ||
| ignore-service.test.ts | ||
| impact-batching-grouping.test.ts | ||
| impact-confidence.test.ts | ||
| import-processor.test.ts | ||
| import-resolver-factory.test.ts | ||
| incremental-file-hash.test.ts | ||
| incremental-orchestration.test.ts | ||
| incremental-parse-cache.test.ts | ||
| incremental-shadow-candidates.test.ts | ||
| incremental-subgraph-extract.test.ts | ||
| index-repo-command.test.ts | ||
| ingestion-utils.test.ts | ||
| jcl-parser.test.ts | ||
| kotlin-scope-captures.test.ts | ||
| language-skip.test.ts | ||
| lazy-action.test.ts | ||
| lbug-adapter-wal-schema.test.ts | ||
| lbug-checkpoint-lifecycle.test.ts | ||
| lbug-checkpoint.test.ts | ||
| lbug-config-wal.test.ts | ||
| lbug-embedding-hashes.test.ts | ||
| lbug-extension-loader.test.ts | ||
| lbug-pool-win-fts-probe.test.ts | ||
| lbug-readonly-error.test.ts | ||
| local-backend-maxbuffer.test.ts | ||
| logger.test.ts | ||
| max-file-size.test.ts | ||
| mcp-stdout-sentinel.test.ts | ||
| mcp-wal-feedback.test.ts | ||
| method-extraction.test.ts | ||
| method-props.test.ts | ||
| mro-processor.test.ts | ||
| noise-filter.test.ts | ||
| parse-diff-hunks.test.ts | ||
| parse-impl-chunk-concurrency.test.ts | ||
| parse-impl-deferred-extraction.test.ts | ||
| parse-impl-env-reads.test.ts | ||
| parse-impl-fallback.test.ts | ||
| parse-impl-progress-monotonic.test.ts | ||
| parse-impl-worker-lazy-cache.test.ts | ||
| parser-loader.test.ts | ||
| parsing-worker-fallback.test.ts | ||
| phase-timer.test.ts | ||
| pipeline-exports.test.ts | ||
| pipeline-runner.test.ts | ||
| platform-capabilities.test.ts | ||
| pool-wal-recovery.test.ts | ||
| process-processor.test.ts | ||
| publish.test.ts | ||
| query-fts-parameterization.test.ts | ||
| query-params.test.ts | ||
| rate-limit.test.ts | ||
| receiver-extraction.test.ts | ||
| registry-primary-flag.test.ts | ||
| rel-csv-split.test.ts | ||
| repo-manager-ensure-ignore-readonly.test.ts | ||
| repo-manager-finalize-invariant.test.ts | ||
| repo-manager.test.ts | ||
| resolve-enclosing-owner.test.ts | ||
| resources.test.ts | ||
| route-tool-detection.test.ts | ||
| ruby-self-call.test.ts | ||
| run-analyze-fts-repair.test.ts | ||
| run-analyze.test.ts | ||
| safe-parse.test.ts | ||
| schema.test.ts | ||
| security.test.ts | ||
| semantic-chunk-search.test.ts | ||
| sequential-language-availability.test.ts | ||
| server-cors-stack.test.ts | ||
| server-validation.test.ts | ||
| server.test.ts | ||
| setup-codex.test.ts | ||
| setup-jsonc.test.ts | ||
| setup.test.ts | ||
| shape-check.test.ts | ||
| shared-type-extractors.test.ts | ||
| sibling-clone-drift.test.ts | ||
| sidecar-recovery.test.ts | ||
| skill-gen.test.ts | ||
| skip-git-cli.test.ts | ||
| staleness.test.ts | ||
| stdout-silence.test.ts | ||
| structure-processor.test.ts | ||
| suffix-index-ambiguity.test.ts | ||
| symbol-resolver.test.ts | ||
| symbol-table.test.ts | ||
| text-generator.test.ts | ||
| tool-direct-cli.test.ts | ||
| tool-process-linking.test.ts | ||
| tools.test.ts | ||
| topological-sort.test.ts | ||
| transitive-include-closure.test.ts | ||
| tree-sitter-queries.test.ts | ||
| type-env.test.ts | ||
| utils.test.ts | ||
| variable-extraction.test.ts | ||
| vue-sfc-extractor.test.ts | ||
| web-ui-serving.test.ts | ||
| wiki-flags.test.ts | ||
| wiki-llm-client.test.ts | ||
| wiki-mermaid-sanitizer.test.ts | ||
| wildcard-synthesis.test.ts | ||
| worker-pool-cumulative-timeout.test.ts | ||
| worker-pool-options.test.ts | ||
| worker-pool-resilience.test.ts | ||
| worker-pool-slot-generation.test.ts | ||
| worker-pool-transferlist.test.ts | ||
| worker-pool-windows-quarantine.test.ts | ||