refactor(ingestion): delete legacy resolution context + tiered-lookup plumbing (RING4-2, #943)#2033
Conversation
…(RING4-2 abhigyanpatwari#943) Pins the current processRoutesFromExtracted edge-emission behavior (which had no direct coverage) before migrating it off the legacy ResolutionContext.resolve tiered lookup. Locks edge target, reason, and confidence values.
…try (RING4-2 abhigyanpatwari#943) Migrate processRoutesFromExtracted off the legacy ResolutionContext.resolve tiered lookup onto model.types.lookupClassByName (global class resolution) + model.symbols.lookupExactAll (same-file method lookup). Drops the TIER_CONFIDENCE dependency for a fixed ROUTE_EDGE_CONFIDENCE constant matching the prior global-tier confidence. Characterization tests (6) stay green — behavior preserved.
…RING4-2 abhigyanpatwari#943) Removes the legacy tiered name resolution — resolve/resolveUncached, TieredCandidates, ResolutionTier, TIER_CONFIDENCE, walkBindingChain, the package-dir index, the per-file resolve cache, and tier-hit stats. The context is now a thin holder for the live SemanticModel plus the (now-dead) per-file import maps, which the follow-up prune removes. Deletes the dedicated resolution-context.test.ts and symbol-resolver.test.ts (both exercised the removed .resolve tiered lookup). Full unit suite green (the 3 analyze worker-pool tests are pre-existing load flakes — pass isolated).
…thesis (RING4-2 abhigyanpatwari#943) The per-file importMap / namedImportMap / packageMap / moduleAliasMap that fed the retired tiered resolver are now dead — nothing reads them (IMPORTS edges come from scope-resolution's imports-to-edges bridge, independent of these maps). Removes: - wildcard-synthesis.ts (synthesized the dead namedImportMap/moduleAliasMap) - import-processor's resolution path (processImports/processImportsFromExtracted/ wireImplicitImports/buildImportResolutionContext), keeping only the live preprocessImportPath path-cleanup helper - the parse-impl orchestration that drove them The parse phase now threads its SemanticModel to scope-resolution directly (parseOutput.model) instead of wrapping it in the resolution context. Deletes the obsolete wildcard/import-processor unit tests; trims the dead processImports cases from sequential-language-availability (processParsing coverage kept).
…ng (RING4-2 abhigyanpatwari#943) Completes the legacy-resolution retirement. With the tiered resolver gone, the entire per-file import-extraction chain is dead — its only consumer was the deleted ResolutionContext.resolve, and scope-resolution emits IMPORTS edges from its own finalized ImportEdges: - delete model/resolution-context.ts (the legacy context); the parse phase now hands its SemanticModel to scope-resolution as parseOutput.model - delete the named-bindings/ extractors + the namedBindingExtractor provider hook (built the dead NamedImportMap) across all 8 providers + the worker - delete the orphaned implicitImportWirer hook + Swift implementation + providersWithImplicitWiring (scope-resolution owns implicit imports now) - drop the dead ExtractedImport type + worker/sequential import accumulation (result.imports / WorkerExtractedData.imports) - import-processor.ts and its preprocessImportPath helper are now unreferenced Deletes the obsolete named-bindings + preprocessImportPath unit tests. tsc clean; full unit suite green (3 analyze worker-pool tests are pre-existing load flakes); 1229 import/cross-file/resolver integration tests pass incl. the wildcard-import languages (Go/Ruby/C++/Swift) that previously used synthesis.
… machinery (RING4-2 abhigyanpatwari#943)
Code-review autofixes from the multi-agent pass: - delete orphaned dead code the deletion missed: swift.ts groupSwiftFilesByTarget + SwiftPackageConfig import (live copy is target-grouping.ts), import-resolvers EMPTY_INDEX export (no consumers after the importCtx reset was removed) - scrub stale comments referencing deleted symbols (processImports, preprocessImportPath, moduleAliasMap, NamedImportMap/PackageMap, wildcard-synthesis) and fix a broken comment fragment in parse-impl.ts - document the intentional global-resolution convergence for route controllers (the import-scoped tier was deleted with the resolver): confidence flattens 0.9→0.5 but resolved edges stay at the 0.5 process-trace/community gate; only the narrow imported-controller-with-unresolved-method guessed edge crosses it - add an overloaded-method characterization case pinning lookupExactAll[0]
|
@magyargergo is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
✨ PR AutofixFound fixable formatting / unused-import issues across 49 changed lines. Comment |
|
/autofix |
|
✅ No autofix to apply — formatter found nothing. |
CI Report✅ All checks passed Pipeline Status
Test Results
✅ All 10473 tests passed 16 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
magyargergo
left a comment
magyargergo
left a comment
There was a problem hiding this comment.
Tri-review — PR #2033 (RING4-2: delete legacy resolution context)
Methods & engines. Codex (the one genuinely independent engine — live) + CE/GitNexus Claude lanes (correctness, adversarial, maintainability, testing, performance; risk + test/CI). Honest breakdown: only Codex is a different engine — the rest is "consistent across personas," not independent confirmation. The two gitnexus-* lanes ended mid-investigation; their domains (production risk, test/CI) were covered by the CE correctness/adversarial/testing lanes.
What the review validated (credit). The route-resolution migration is byte-for-byte equivalent to the legacy tiered lookup on everything except controller resolution — method resolution (lookupExactAll[0] == legacy same-file tier), edge id/confidence math (× 0.8, reason:'laravel-route'), overload [0] selection, and null handling are unchanged (correctness + adversarial). The parse-impl orchestrator unwind, the parseOutput.model threading, and the worker import-skip control flow are consistent — no live consumer reads a deleted symbol (correctness). The deletion is net-faster (performance: the old route path ran an uncached 4-tier lookup; the new path is two O(1) registry hits; the removed wildcard-synthesis + inert suffix-index building were pure overhead). The new characterization test is strong (7/7, asserts exact target + confidence) and no deleted test covered surviving behavior (testing). The ~4100-line deletion is verified dead (grep gate zero, tsc clean, 6660 unit + 1229 integration green incl. the wildcard-import languages that used the deleted synthesis).
Findings (2 inline)
-
[P2 · Codex + adversarial, cross-engine] Route migration drops edges for import-disambiguated controllers. The controller is resolved by global short name (
lookupClassByName, skip if ≠ 1). The legacy resolver disambiguated controllers via the routes-fileusebinding at the import-scoped tier, so two cases that previously emitted a correct edge now silently drop it — an aliased import (use …OrderController as Orders; [Orders::class,'index'], Codex F1) and a globally-duplicated short name +useimport (adversarial #1). The JSDoc's "matches the legacy global-tier ambiguity guard" is inaccurate for these (the legacy path resolved them at the import-scoped tier). Only missing edges result — never a wrong target — and the patterns are uncommon, so impact is narrow; but the documented trade-off undersells it. (inline oncall-processor.ts) -
[P2 · maintainability, coordinator-verified]
importSemanticsis now dead.wildcard-synthesis.ts(deleted here) was its sole consumer; HEAD has zero readers, yet the field is still declared, defaulted, set by 7 providers, and carries a JSDoc describing the deleted synthesis. Same class of leftover as thegroupSwiftFilesByTarget/EMPTY_INDEXitems already cleaned in this PR. (inline onlanguage-provider.ts)
Lower-priority (no inline)
- Stale JSDoc/comments referencing deleted machinery remain in
parse-impl.ts(the module docstring +runChunkedParseAndResolveJSDoc still list the removed import-resolution / wildcard-synthesis / heritage steps; inline comment ~:631) and theImportSemanticsJSDoc (maintainability M2–M4, P2/P3 cleanup). - Two uncovered side-paths in the route emitter — the
onProgress50-route batch boundary, and same-file sibling-class method[0]selection without anownerIdfilter (matches the legacy same-file tier; Laravel's one-class-per-file convention makes it moot). Testing P3, not regressions.
Refuted / not found
No correctness, performance, or test-coverage defects. Adversarial found no wrong edge; correctness found the migration semantically sound; performance found no regression.
CI
Green on all 26 code checks (tests across ubuntu/windows/macOS, quality lint/typecheck/format, build, e2e, analyze, gitleaks, review). The single "fail" is Vercel deploy-authorization — an infra check, not a code gate.
Automated multi-tool digest (Codex + Claude lanes). Only Codex is an independent engine; weigh accordingly and verify before acting.
| if (controllerResolved.tier === 'global' && controllerResolved.candidates.length > 1) continue; | ||
| // Controller resolves by global class name. Refuse ambiguous matches — | ||
| // mirrors the legacy global-tier "candidates.length > 1 → skip" guard. | ||
| const controllerDefs = model.types.lookupClassByName(route.controllerName); |
There was a problem hiding this comment.
[P2 · Codex + CE-adversarial, cross-engine] Route migration drops edges for import-disambiguated controllers. lookupClassByName(route.controllerName) resolves the controller by global short name and skips on length !== 1. The legacy ctx.resolve() disambiguated controllers via the routes-file use binding at the import-scoped tier, so two cases that previously emitted a correct edge now silently drop it:
- (a) aliased import —
use App\Http\Controllers\OrderController as Orders;+Route::get('/orders', [Orders::class, 'index']);→controllerName='Orders'(laravel.ts:182) →lookupClassByName('Orders')is[]→ no edge (Codex F1). - (b) globally-duplicated short name —
App\…\OrderController+Admin\OrderControllerwith auseimport →length===2→continue(adversarial Add support for Ollama as a local inference backend #1).
The JSDoc above (…matches the legacy *global*-tier ambiguity guard) is inaccurate for these: the legacy path resolved import-disambiguated controllers at the import-scoped tier and emitted the edge at 0.9 — it never reached the global guard. Only missing edges result (never a wrong target), and the patterns are uncommon, so impact is narrow.
Fix: correct the JSDoc to cover the import-disambiguated case; optionally restore alias/qualified-name disambiguation (thread the route file's use map or fall back to a qualified lookup when length > 1); add characterization tests for use … as X and same-short-name controllers. [code-read]
…wari#943) From the PR abhigyanpatwari#2033 tri-review (Codex + CE lanes): - delete the now-dead importSemantics provider field + ImportSemantics type (wildcard-synthesis.ts was its sole consumer; zero readers remain) across language-provider.ts + 7 providers + DEFAULTS - correct the processRoutesFromExtracted JSDoc: the import-disambiguated controller skip is STRICTER than the legacy global-tier guard (the legacy import-scoped tier resolved aliased / same-short-name controllers and emitted the edge); document the aliased-import missed-edge case explicitly - add an aliased-controller characterization test pinning the documented global-resolution convergence (no edge for an aliased/unresolvable controller name) - scrub stale parse-impl.ts docstrings/comments that still listed the removed import-resolution / wildcard-synthesis / heritage passes Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…er resolution (abhigyanpatwari#943) Adds ExtractedRoute.controllerQualifiedName: the Laravel route extractor now builds the routes file's `use`-import alias map (local→normalized dot-joined FQN, via splitNamespaceUseDeclaration) and captures inline qualified ::class references, threading the disambiguating FQN through every route. Normalized via the shared normalizeQualifiedName so it matches the type registry's key shape (issue abhigyanpatwari#1982). Foundation for qualified-first route→controller resolution (U2).
…higyanpatwari#943) processRoutesFromExtracted now resolves the controller via model.types.lookupClassByQualifiedName(route.controllerQualifiedName) when the extractor disambiguated it (aliased use / same-short-name / inline FQN), falling back to the short-name lookupClassByName (which still skips on ambiguity). This restores the route→controller CALLS edges the PR abhigyanpatwari#2033 tri-review (Codex F1 + ce-adversarial) found dropped, without re-adding the deleted per-file import map. Method resolution, guessed-id, and confidence are unchanged. JSDoc rewritten to qualified-first precedence; the aliased characterization test flips from no-edge to edge; adds duplicated-name-disambiguated + stale-FQN-fallback cases.
…tion + PSR-4 disambiguation (abhigyanpatwari#943) Adds an integration test that parses real namespaced PHP controllers + a routes file through the worker pipeline and asserts the route CALLS edges target the correct namespaced controller — the authoritative gate the unit tests can't be (hand-built models). It surfaced that PHP's statement-form `namespace X;` leaves the structure-phase qualifiedName as the SHORT name, so lookupClassByQualifiedName misses; resolveControllerByQualifiedName now adds a PSR-4 file-path disambiguation (FQN namespace tail ↔ file directory tail) to pick the right same-short-name controller. Forces the worker path (workerThresholdsForTest) since route extraction is worker-only.
…rpus (abhigyanpatwari#943) The laravel route-resolution fixture lived under lang-resolution/php-laravel-routes, which the php scope-capture golden + benchmark both glob (lang-resolution/php-*), drifting their fingerprints. The fixture is for route resolution, not php scope-capture parity, so rename it to lang-resolution/laravel-route-resolution to decouple it. Reverts the golden's php-laravel-routes entries; bench scope-capture --check passes (php back to baseline).
2 participants
What & why
RING4-2 finishes the call-resolution DAG retirement started by RING4-1 (#942). It deletes the legacy tiered name resolver —
ResolutionContext.resolve,TieredCandidates,walkBindingChain, the package-dir index,TIER_CONFIDENCE— and all the now-dead import-map plumbing that existed only to feed it.Registry.lookup/resolveTypeRef(the scope-resolution registry) are now the only resolution entry points.Closes #943. Depends on RING4-1 (#942), already merged.
The one behavioral migration
.resolve()had a single surviving live consumer:processRoutesFromExtracted(Laravel framework route → controller-methodCALLSedges). It now resolves the controller by global class name viamodel.types.lookupClassByNameand the method within the controller's file viamodel.symbols.lookupExactAll— behavior-equivalent to the global tier the worker path always hit. A characterization test (test/unit/call-processor-routes.test.ts) was written first to pin the existing edge-emission behavior, then the migration kept it green.Intentional convergence (documented in
call-processor.ts): the legacy resolver could also resolve a controller at the import-scoped tier (0.9) via the now-deleted per-file named-import map. Without it, route controllers resolve globally at a flat 0.5. The 0.5 process-trace (MIN_TRACE_CONFIDENCE) and large-graph community (MIN_CONFIDENCE_LARGE) gates use>= 0.5, so resolved route edges (at exactly 0.5) are unaffected; only the narrow imported-controller-with-unresolved-method guessed edge drops 0.72 → 0.4 below the gate — an accepted loss for an already-heuristic edge whose method could not be resolved.Why the rest is safe to delete
With
.resolve()gone, nothing readsimportMap/namedImportMap/packageMap/moduleAliasMap— IMPORTS edges are emitted independently by the scope-resolution phase (graph-bridge/imports-to-edges.ts) from finalizedImportEdges, and implicit imports by each resolver'semitImplicitImportEdgeshook. That makes the whole per-file import-extraction chain dead:model/resolution-context.ts(the context) — deleted; the parse phase now threads itsSemanticModelto scope-resolution directly asparseOutput.modelpipeline-phases/wildcard-synthesis.ts— deleted (built the deadnamedImportMap/moduleAliasMap)import-processor.tsresolution path +named-bindings/extractors (8 providers) + thenamedBindingExtractorprovider hook — deletedimplicitImportWirerhook + Swift implementation — deleted (scope-resolution owns implicit imports)result.importsextraction + theExtractedImporttype — deletedbuildTypeEnv/TypeEnvironment(receiver-type filtering),buildCollisionGroups, and theimportPathPreprocessorhook are preserved (live or independent of the deleted machinery).Verification
TieredCandidates/ResolutionContext.resolve/walkBindingChain/packageDirIndex/TIER_CONFIDENCEoutside git history.tsc --noEmitclean.dist6816 KB → 5760 KB (−1056 KB, ~15.5%); net source −4148 LOC (267 insertions / 4415 deletions).Review
Reviewed by a multi-agent pass (correctness, adversarial, maintainability, testing). Autofixes applied in-PR: removed orphaned dead code the deletion missed (
groupSwiftFilesByTarget,EMPTY_INDEX), scrubbed stale comments referencing deleted symbols, documented the route-resolution convergence, and added an overloaded-method characterization case. No residual actionable work.🤖 Generated with Claude Code