From 5794d98d4491c8e05555e0c0d4a8447030836303 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 21 May 2026 10:21:43 +0100 Subject: [PATCH] refactor(scope-resolution): align Const label emission with legacy DAG (PR #1718 review F1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- gitnexus/src/core/ingestion/scope-extractor.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gitnexus/src/core/ingestion/scope-extractor.ts b/gitnexus/src/core/ingestion/scope-extractor.ts index 746b9d694..09080d2c6 100644 --- a/gitnexus/src/core/ingestion/scope-extractor.ts +++ b/gitnexus/src/core/ingestion/scope-extractor.ts @@ -693,8 +693,14 @@ function normalizeNodeLabel(kindStr: string): SymbolDefinition['type'] | undefin case 'property': return 'Property'; case 'variable': - case 'const': return 'Variable'; + // `const` / `let` declarations align with the legacy DAG parse phase, + // which emits `Const` graph nodes via `@definition.const` capture for + // `lexical_declaration`. Returning `'Const'` here lets resolveDefGraphId's + // qualified-key path succeed for value receivers without relying on the + // simple-key fallback (PR #1718 review Finding 1 / 2026-05-21-002 U4). + case 'const': + return 'Const'; case 'typealias': case 'type_alias': return 'TypeAlias';