diff --git a/gitnexus/src/core/index-freshness.ts b/gitnexus/src/core/index-freshness.ts index 8af3ad73a..f063e446c 100644 --- a/gitnexus/src/core/index-freshness.ts +++ b/gitnexus/src/core/index-freshness.ts @@ -10,6 +10,41 @@ export const INDEX_INCOMPLETE_REASONS = [ export type IndexIncompleteReason = (typeof INDEX_INCOMPLETE_REASONS)[number]; +/** + * Fraction of the pipeline's relationship count that must survive into the DB + * before the write counts as collapsed. Deliberately generous: this detects + * "most of the graph did not persist" (the reported case lost ~91%), not a + * per-edge reconciliation. + */ +export const GRAPH_WRITE_COLLAPSE_RATIO = 0.5; + +/** + * Below this many relationships the ratio is meaningless — a handful of edges + * lost to legitimate filtering would trip it — so small repos are exempt. + */ +export const GRAPH_WRITE_COLLAPSE_MIN_EDGES = 100; + +/** + * Decide whether a finished write collapsed, comparing what the pipeline + * produced against what the DB hands back. + * + * A RATIO, not equality: some relationship types do not round-trip one-for-one + * and `--pdg` writes MORE rows into the same table, so demanding equality would + * fire on healthy runs. Only a collapse is a defect. + * + * FAIL-SAFE at `expected === 0`: an implementation that offloads relationships + * out of memory may not be able to report a total, and a false "your index is + * broken" is worse than a missed one. + */ +export function detectGraphWriteCollapse( + expected: number, + persisted: number, +): { expected: number; persisted: number } | undefined { + if (expected < GRAPH_WRITE_COLLAPSE_MIN_EDGES) return undefined; + if (persisted >= expected * GRAPH_WRITE_COLLAPSE_RATIO) return undefined; + return { expected, persisted }; +} + /** Stable machine-readable reasons an index cannot be certified complete. */ export function getIndexIncompleteReasons( meta: diff --git a/gitnexus/src/core/ingestion/utils/ast-helpers.ts b/gitnexus/src/core/ingestion/utils/ast-helpers.ts index 64eab074f..9b243d044 100644 --- a/gitnexus/src/core/ingestion/utils/ast-helpers.ts +++ b/gitnexus/src/core/ingestion/utils/ast-helpers.ts @@ -1103,8 +1103,15 @@ export interface ObjectLiteralBindingInfo { * * Set by {@link findMemberAssignmentOwnerInfo} so a prototype method keys as * `Foo.bar` — without it two constructors in one file that each define - * `bar` collapse onto a single `Method::bar` id. Left undefined by - * {@link findObjectLiteralBindingInfo}, whose ids stay exactly as they were. + * `bar` collapse onto a single `Method::bar` id. + * + * {@link findObjectLiteralBindingInfo} sets it ONLY when the caller opts in + * via `includeOwnerName`. Its `Method` ids must stay exactly as they were — + * qualifying them would rewrite every object-literal method id in every + * indexed repo — but object-literal KEYS (indexed since A1/A5) genuinely + * need it: two config objects in one file sharing a key name otherwise + * collapse onto a single `Property::` id, merging two distinct + * settings into one symbol. */ ownerName?: string; } @@ -1161,6 +1168,13 @@ const BLOCK_SCOPE_BOUNDARY_TYPES = new Set([ export const findObjectLiteralBindingInfo = ( node: SyntaxNode, filePath: string, + options?: { + /** + * Also return `ownerName` so the member qualifies as `.`. + * Opt-in because turning it on for `Method` would rewrite existing ids. + */ + readonly includeOwnerName?: boolean; + }, ): ObjectLiteralBindingInfo | null => { // ── Phase A: walk up from node, count `object` ancestors, find declarator let current: SyntaxNode | null = node; @@ -1218,6 +1232,7 @@ export const findObjectLiteralBindingInfo = ( const ownerLabel = declaration?.type === 'variable_declaration' ? 'Variable' : 'Const'; return { ownerId: generateId(ownerLabel, `${filePath}:${nameNode.text}`), + ...(options?.includeOwnerName === true ? { ownerName: nameNode.text } : {}), }; }; diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index 3a80eaa71..029ffab83 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -2345,10 +2345,23 @@ const processFileGroup = ( // syntax rather than from an ancestor walk, and both are language-shaped // helpers behind the provider's own label decision — shared code here // only asks "does this Method name an owner". + // `Property` joins `Method` here because object-literal KEYS are now + // indexed (A1/A5), and a key is owned by the object that holds it exactly + // as a literal's function-valued member is. Without it, two config + // objects in one file sharing a key name (`httpConfig.timeoutMs` and + // `dbConfig.timeoutMs`) generate the same `Property::timeoutMs` id + // and COLLAPSE INTO ONE node — two distinct settings become one symbol, + // and the merged name then looks workspace-unique to name inference, + // which resolves reads of it to a node representing both. const objectLiteralOwnerInfo = - !enclosingClassId && nodeLabel === 'Method' && definitionNode + !enclosingClassId && (nodeLabel === 'Method' || nodeLabel === 'Property') && definitionNode ? (findMemberAssignmentOwnerInfo(definitionNode, file.path) ?? - findObjectLiteralBindingInfo(definitionNode, file.path)) + findObjectLiteralBindingInfo(definitionNode, file.path, { + // Only `Property` opts into the qualifier; `Method` ids must stay + // byte-identical or every object-literal method in every indexed + // repo changes id. + includeOwnerName: nodeLabel === 'Property', + })) : null; // #1978: hoisted ABOVE qualifiedName/node-id (load-bearing order) so a diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 83837201c..96c939304 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -9,6 +9,7 @@ * wrapper or server worker) is responsible for process lifecycle. */ +import { detectGraphWriteCollapse } from './index-freshness.js'; import path from 'path'; import fs from 'fs/promises'; import { randomUUID } from 'node:crypto'; @@ -502,20 +503,6 @@ export interface AnalyzeResult { // Class-neutral lead, reused for the missing-dependency degrade path (#2383 F2): // its remedy already explains that reinstalling will NOT help, so appending the // generic "install with network access" tail below would contradict it. -/** - * Fraction of the pipeline's relationship count that must survive into the DB - * before the write is treated as a collapse. Deliberately generous: this is a - * catastrophe detector for "most of the graph did not persist" (the reported - * case lost ~91%), not a reconciliation of every edge. - */ -const GRAPH_WRITE_COLLAPSE_RATIO = 0.5; - -/** - * Below this many relationships the ratio is meaningless — a handful of edges - * lost to legitimate filtering would trip it — so small repos are exempt. - */ -const GRAPH_WRITE_COLLAPSE_MIN_EDGES = 100; - const FTS_UNAVAILABLE_LEAD = 'FTS extension unavailable; skipping search-index creation.'; const FTS_UNAVAILABLE_MESSAGE = `${FTS_UNAVAILABLE_LEAD} ` + @@ -2552,11 +2539,7 @@ async function runFullAnalysisInner( // relationships out of memory may no longer be able to report a total, and // a false "your index is broken" is worse than a missed one. const expectedRelationships = pipelineResult.graph.relationshipCount; - const graphWriteCollapsed = - expectedRelationships >= GRAPH_WRITE_COLLAPSE_MIN_EDGES && - stats.edges < expectedRelationships * GRAPH_WRITE_COLLAPSE_RATIO - ? { expected: expectedRelationships, persisted: stats.edges } - : undefined; + const graphWriteCollapsed = detectGraphWriteCollapse(expectedRelationships, stats.edges); if (graphWriteCollapsed) { log( `Warning: graph write incomplete — the pipeline produced ${expectedRelationships} ` + diff --git a/gitnexus/test/fixtures/lang-resolution/javascript-object-properties/ambiguous.js b/gitnexus/test/fixtures/lang-resolution/javascript-object-properties/ambiguous.js new file mode 100644 index 000000000..c63a0f414 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/javascript-object-properties/ambiguous.js @@ -0,0 +1,15 @@ +// Two DIFFERENT config objects that share a key name. A read of that name +// through an untyped receiver could mean either one, so the unique-name pass +// must emit nothing rather than pick — the safety property that keeps name +// inference from over-connecting on generic keys (id, name, data). +export const httpConfig = { + sharedTimeoutMs: 1000, +}; + +export const dbConfig = { + sharedTimeoutMs: 2000, +}; + +export function readsAmbiguous(cfg) { + return cfg.sharedTimeoutMs; +} diff --git a/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts b/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts index 23be3652a..e2feec99d 100644 --- a/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts +++ b/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts @@ -93,6 +93,21 @@ describe('JavaScript plain-object property access (A1/A5)', () => { expect(readersOf('exitMinAtrMult')).toContain('applyRules'); }); + // The safety property. Name inference is only defensible because it refuses + // to choose between candidates: two objects sharing a key name means a read + // through an untyped receiver could mean either, and a wrong edge in the + // pre-edit safety gate is worse than a missing one. Without this, the pass + // would silently link generic keys (id, name, data) across unrelated objects. + it('emits NOTHING when two objects share the key name', () => { + expect(readersOf('sharedTimeoutMs')).toEqual([]); + }); + + it('still indexes both ambiguous keys as nodes — only the EDGE is withheld', () => { + // The symbols must remain findable; it is the inference that is unsafe, + // not the definitions. + expect(propertyNames().filter((n) => n === 'sharedTimeoutMs')).toHaveLength(2); + }); + it('marks a name-inferred edge at reduced confidence, not as precise', () => { const inferred = getRelationships(result, 'ACCESSES').filter( (e) => e.target === 'exitMinAtrMult' && (e.rel.reason ?? '').includes('unique-name'), diff --git a/gitnexus/test/unit/index-freshness-graph-collapse.test.ts b/gitnexus/test/unit/index-freshness-graph-collapse.test.ts index cdaaecb9f..bcff4d732 100644 --- a/gitnexus/test/unit/index-freshness-graph-collapse.test.ts +++ b/gitnexus/test/unit/index-freshness-graph-collapse.test.ts @@ -15,10 +15,58 @@ */ import { describe, it, expect } from 'vitest'; import { + detectGraphWriteCollapse, getIndexIncompleteReasons, + GRAPH_WRITE_COLLAPSE_MIN_EDGES, + GRAPH_WRITE_COLLAPSE_RATIO, INDEX_INCOMPLETE_REASONS, } from '../../src/core/index-freshness.js'; +describe('detectGraphWriteCollapse (B2 detection)', () => { + it('flags the reported field failure (23009 built, 2170 persisted)', () => { + expect(detectGraphWriteCollapse(23009, 2170)).toEqual({ expected: 23009, persisted: 2170 }); + }); + + it('flags a missing relation table, which reads back as zero persisted', () => { + expect(detectGraphWriteCollapse(23009, 0)).toEqual({ expected: 23009, persisted: 0 }); + }); + + it('stays silent on a healthy write', () => { + expect(detectGraphWriteCollapse(23009, 23009)).toBeUndefined(); + }); + + it('stays silent when MORE rows persist than the call graph built (--pdg)', () => { + // PDG layers write into the same table, so persisted > expected is normal. + expect(detectGraphWriteCollapse(1000, 4000)).toBeUndefined(); + }); + + it('is fail-safe when the expected count is unavailable', () => { + // An implementation that offloads relationships out of memory may report 0; + // a false "your index is broken" is worse than a missed one. + expect(detectGraphWriteCollapse(0, 0)).toBeUndefined(); + expect(detectGraphWriteCollapse(0, 5000)).toBeUndefined(); + }); + + it('exempts small repos where the ratio is meaningless', () => { + const justUnder = GRAPH_WRITE_COLLAPSE_MIN_EDGES - 1; + expect(detectGraphWriteCollapse(justUnder, 0)).toBeUndefined(); + }); + + it('applies exactly at the minimum-edge boundary', () => { + expect(detectGraphWriteCollapse(GRAPH_WRITE_COLLAPSE_MIN_EDGES, 0)).toEqual({ + expected: GRAPH_WRITE_COLLAPSE_MIN_EDGES, + persisted: 0, + }); + }); + + it('treats the ratio as inclusive — exactly at threshold is not a collapse', () => { + const expected = 1000; + const atThreshold = expected * GRAPH_WRITE_COLLAPSE_RATIO; + expect(detectGraphWriteCollapse(expected, atThreshold)).toBeUndefined(); + expect(detectGraphWriteCollapse(expected, atThreshold - 1)).toBeDefined(); + }); +}); + describe('graph-write-collapsed incomplete reason (B2)', () => { it('is part of the stable reason vocabulary', () => { expect(INDEX_INCOMPLETE_REASONS).toContain('graph-write-collapsed');