From 255bdf6fae66088d8b7b5f4939e5f123761fe3b6 Mon Sep 17 00:00:00 2001 From: abhigyantrumio Date: Tue, 12 May 2026 17:03:48 +0530 Subject: [PATCH] fix(call-processor): port worker-path property enrichment into the sequential pre-pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's pre-pass in 8184439 fixed the Ruby attr_accessor order-dependence, but it copied the OLD in-loop registration logic, not the canonical worker path in parse-worker.ts. That left the sequential and worker paths emitting non-identical Property nodes/symbols for the same source — silently breaking the `incremental ≡ --force` invariant the moment a repo crosses the worker threshold between runs. Two concrete divergences are closed here: * Node id: worker keys Property as `${file}:${className}.${propName}` (qualified). Pre-pass was using `${file}:${propName}` (unqualified). Same source produced different graph ids depending on which path ran. * Field metadata: worker enriches each routed property with `provider.fieldExtractor` + `getFieldInfo`, falling back to `routedFieldInfo.type` for `declaredType` when the routing payload lacks one (e.g. types discovered from `@address = Address.new` ctor assignments rather than YARD `@return [Type]`), and propagates `visibility` / `isStatic` / `isReadonly`. Pre-pass did none of this, so on the sequential path `resolveFieldAccessType` failed to walk chains where the type only came from the FieldExtractor. The pre-pass now mirrors parse-worker.ts:1803-1898 verbatim, with one deliberate difference: the FieldInfo cache is scoped to a single `processCalls` invocation rather than module-level (the worker process is short-lived; the main thread is not, and a module-level cache would leak state between analyze runs). Also drops the now-stale "Defer resolution: Ruby attr_accessor properties are registered during this same loop" comment on `pendingWrites.push` — the rationale is no longer accurate after Copilot's pre-pass, but the deferral is still needed so write-access tracking sees inference that completes during the main loop. Comment updated to reflect that. Verification: * `tsc --noEmit`: 0 errors * test/unit (call-processor, call-routing, field-extraction, ruby-self-call): 224 passing * test/integration (ruby, ruby-sequential-mixin, pipeline-graph-golden): 137 passing Co-Authored-By: Claude Opus 4.7 (1M context) --- gitnexus/src/core/ingestion/call-processor.ts | 134 ++++++++++++++++-- 1 file changed, 121 insertions(+), 13 deletions(-) diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index ce6be749f..ac5b6c9c3 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -42,12 +42,14 @@ import { isVerboseIngestionEnabled } from './utils/verbose.js'; import { yieldToEventLoop } from './utils/event-loop.js'; import { parseSourceSafe } from '../tree-sitter/safe-parse.js'; import { + CLASS_CONTAINER_TYPES, FUNCTION_NODE_TYPES, - findEnclosingClassId, findEnclosingClassInfo, genericFuncName, inferFunctionLabel, } from './utils/ast-helpers.js'; +import type { FieldInfo, FieldExtractorContext } from './field-types.js'; +import type { LanguageProvider } from './language-provider.js'; import { typeTagForId, constTagForId, buildCollisionGroups } from './utils/method-props.js'; import type { MethodInfo } from './method-types.js'; import { @@ -77,6 +79,58 @@ import type { LiteralTypeInferrer } from './type-extractors/types.js'; import type { SyntaxNode } from './utils/ast-helpers.js'; import { logger } from '../logger.js'; + +// ── Property-prepass helpers (parity with parse-worker.ts) ── +// These mirror the sequential-path equivalents in parse-worker.ts so the main- +// thread `processCalls` pre-pass produces byte-identical Property nodes/symbols +// to the worker pool. Drift between the two paths breaks the +// `incremental ≡ --force` invariant the moment a repo crosses the worker +// threshold between runs. + +/** Walk up to the nearest enclosing class/struct/interface AST node. */ +const findEnclosingClassNode = (node: SyntaxNode): SyntaxNode | null => { + let current = node.parent; + while (current) { + if (CLASS_CONTAINER_TYPES.has(current.type)) return current; + current = current.parent; + } + return null; +}; + +/** No-op SymbolTable stub for FieldExtractorContext — matches parse-worker. */ +const NOOP_SYMBOL_TABLE: SymbolTableReader = { + lookupExact: () => undefined, + lookupExactFull: () => undefined, + lookupExactAll: () => [], + lookupCallableByName: () => [], + getFiles: () => [][Symbol.iterator](), + getStats: () => ({ fileCount: 0 }), +}; + +/** + * Extract (and cache) field info for a class node. Cache is passed in so it + * stays scoped to a single `processCalls` invocation rather than leaking + * across analyze runs (worker uses module-level caching because each worker + * process is short-lived; the main thread is not). + */ +const getFieldInfo = ( + classNode: SyntaxNode, + provider: LanguageProvider, + context: FieldExtractorContext, + cache: Map>, +): Map | undefined => { + if (!provider.fieldExtractor) return undefined; + const cacheKey = classNode.startIndex; + const cached = cache.get(cacheKey); + if (cached) return cached; + const result = provider.fieldExtractor.extract(classNode, context); + if (!result?.fields?.length) return undefined; + const map = new Map(); + for (const field of result.fields) map.set(field.name, field); + cache.set(cacheKey, map); + return map; +}; + /** Per-file resolved type bindings for exported symbols. * Populated during call processing, consumed by Phase 14 re-resolution pass. */ export type ExportedTypeMap = Map>; @@ -861,12 +915,15 @@ export const processCalls = async ( } // ── Property-registration pre-pass ── - // Register all properties (e.g. Ruby attr_accessor) in the FieldRegistry - // BEFORE the resolution loop. This ensures cross-file field-type lookups - // (e.g. `user.address.save → Address#save`) succeed regardless of file - // processing order. Without this pre-pass, field type disambiguation fails - // when the declaring file is processed AFTER the consuming file. - for (const { file, language, provider, matches } of prepared) { + // Register all routed properties (e.g. Ruby attr_accessor) BEFORE the + // resolution loop so cross-file field-type lookups (e.g. + // `user.address.save → Address#save`) succeed regardless of file + // processing order. This MUST stay in lockstep with the equivalent + // worker-path block in parse-worker.ts (kind === 'properties') — any + // divergence between the two paths breaks the `incremental ≡ --force` + // invariant once a repo crosses the worker threshold between runs. + const fieldInfoCache = new Map>(); + for (const { file, language, provider, matches, typeEnv } of prepared) { const callRouter = provider.callRouter; if (!callRouter) continue; matches.forEach((match) => { @@ -877,10 +934,42 @@ export const processCalls = async ( if (!callNameNode) return; const routed = callRouter(callNameNode.text, captureMap['call']); if (!routed || routed.kind !== 'properties') return; + + const propEnclosingInfo = findEnclosingClassInfo( + captureMap['call'], + file.path, + provider.resolveEnclosingOwner, + ); + const propEnclosingClassId = propEnclosingInfo?.classId ?? null; + + // Enrich routed properties with FieldExtractor metadata so types + // discovered from constructor assignments (e.g. `@address = Address.new`) + // are propagated even when the routing payload itself lacks declaredType. + let routedFieldMap: Map | undefined; + if (provider.fieldExtractor && typeEnv) { + const classNode = findEnclosingClassNode(captureMap['call']); + if (classNode) { + routedFieldMap = getFieldInfo( + classNode, + provider, + { + typeEnv, + symbolTable: NOOP_SYMBOL_TABLE, + filePath: file.path, + language, + }, + fieldInfoCache, + ); + } + } + const fileId = generateId('File', file.path); - const propEnclosingClassId = findEnclosingClassId(captureMap['call'], file.path); for (const item of routed.items) { - const nodeId = generateId('Property', `${file.path}:${item.propName}`); + const routedFieldInfo = routedFieldMap?.get(item.propName); + const propQualifiedName = propEnclosingInfo + ? `${propEnclosingInfo.className}.${item.propName}` + : item.propName; + const nodeId = generateId('Property', `${file.path}:${propQualifiedName}`); graph.addNode({ id: nodeId, label: 'Property', @@ -892,11 +981,29 @@ export const processCalls = async ( language, isExported: true, description: item.accessorType, + ...(item.declaredType + ? { declaredType: item.declaredType } + : routedFieldInfo?.type + ? { declaredType: routedFieldInfo.type } + : {}), + ...(routedFieldInfo?.visibility !== undefined + ? { visibility: routedFieldInfo.visibility } + : {}), + ...(routedFieldInfo?.isStatic !== undefined + ? { isStatic: routedFieldInfo.isStatic } + : {}), + ...(routedFieldInfo?.isReadonly !== undefined + ? { isReadonly: routedFieldInfo.isReadonly } + : {}), }, }); ctx.model.symbols.add(file.path, item.propName, nodeId, 'Property', { ...(propEnclosingClassId ? { ownerId: propEnclosingClassId } : {}), - ...(item.declaredType ? { declaredType: item.declaredType } : {}), + ...(item.declaredType + ? { declaredType: item.declaredType } + : routedFieldInfo?.type + ? { declaredType: routedFieldInfo.type } + : {}), }); const relId = generateId('DEFINES', `${fileId}->${nodeId}`); graph.addRelationship({ @@ -991,9 +1098,10 @@ export const processCalls = async ( provider, ); const srcId = enclosing || generateId('File', file.path); - // Defer resolution: Ruby attr_accessor properties are registered during - // this same loop, so cross-file lookups fail if the declaring file hasn't - // been processed yet. Collect now, resolve after all files are done. + // Defer resolution so write-access tracking sees the FINAL graph + // state — properties from the pre-pass are present, but receiver-type + // resolution can still depend on inference that completes during the + // main loop. Resolve after all files have been processed. pendingWrites.push({ receiverTypeName, propertyName, filePath: file.path, srcId }); } // Assignment-only capture (no @call sibling): skip the rest of this