mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-10 03:27:59 +00:00
This commit is contained in:
Gergő Magyar
GitHub
parent
f5a2e6a248
commit
905b7dfa21
8 changed files with 459 additions and 123 deletions
|
|
@ -26,7 +26,6 @@ import {
|
|||
type EmbeddableNode,
|
||||
type SemanticSearchResult,
|
||||
type ModelProgress,
|
||||
type EmbeddingContext,
|
||||
EMBEDDABLE_LABELS,
|
||||
isShortLabel,
|
||||
LABEL_METHOD,
|
||||
|
|
@ -80,7 +79,7 @@ const ensureVectorExtensionAvailable = async (): Promise<boolean> => {
|
|||
* invalidate existing vectors, such as metadata/header shape changes,
|
||||
* structural container context changes, or preceding-context formatting rules.
|
||||
*/
|
||||
export const EMBEDDING_TEXT_VERSION = 'v2';
|
||||
export const EMBEDDING_TEXT_VERSION = 'v4';
|
||||
|
||||
/**
|
||||
* Compute a stable content fingerprint for an embeddable node.
|
||||
|
|
@ -255,6 +254,42 @@ export interface EmbeddingPipelineResult {
|
|||
semanticMode: 'vector-index' | 'exact-scan';
|
||||
}
|
||||
|
||||
/**
|
||||
* DELETE stale embedding rows for the given nodeIds so they can be re-inserted.
|
||||
*
|
||||
* Kuzu forbids SET on vector-indexed properties; DELETE-then-INSERT is the
|
||||
* sanctioned pattern. A `"does not exist"` error means the rows are already gone
|
||||
* (safe to proceed); any other error risks vector-index corruption, so it
|
||||
* propagates and aborts the pipeline.
|
||||
*
|
||||
* Called per-batch (just before each batch's INSERT), not once up front — see
|
||||
* the caller comment / KTD7: an up-front bulk delete of every stale row leaves
|
||||
* the whole index deleted-not-reinserted if the re-embed is interrupted. Per-batch
|
||||
* interleaving bounds that window to a single batch.
|
||||
*/
|
||||
const deleteStaleEmbeddingRows = async (
|
||||
executeWithReusedStatement: (
|
||||
cypher: string,
|
||||
paramsList: Array<Record<string, any>>,
|
||||
) => Promise<void>,
|
||||
nodeIds: string[],
|
||||
): Promise<void> => {
|
||||
if (nodeIds.length === 0) return;
|
||||
try {
|
||||
await executeWithReusedStatement(
|
||||
`MATCH (e:${EMBEDDING_TABLE_NAME} {nodeId: $nodeId}) DELETE e`,
|
||||
nodeIds.map((nodeId) => ({ nodeId })),
|
||||
);
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
if (!msg.includes('does not exist')) {
|
||||
throw new Error(
|
||||
`[embed] Failed to delete stale embedding rows — aborting to prevent vector-index corruption: ${msg}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Run the embedding pipeline
|
||||
*
|
||||
|
|
@ -263,11 +298,9 @@ export interface EmbeddingPipelineResult {
|
|||
* @param onProgress - Callback for progress updates
|
||||
* @param config - Optional configuration override
|
||||
* @param skipNodeIds - Optional set of node IDs that already have embeddings (incremental mode)
|
||||
* @param context - Optional repo/server context for metadata enrichment
|
||||
* @param existingEmbeddings - Optional map of nodeId → contentHash for incremental mode.
|
||||
* Nodes whose hash matches are skipped; nodes with a changed hash are DELETE'd
|
||||
* and re-embedded; nodes not in the map are embedded fresh.
|
||||
|
||||
*/
|
||||
export const runEmbeddingPipeline = async (
|
||||
executeQuery: (cypher: string) => Promise<any[]>,
|
||||
|
|
@ -278,7 +311,6 @@ export const runEmbeddingPipeline = async (
|
|||
onProgress: EmbeddingProgressCallback,
|
||||
config: Partial<EmbeddingConfig> = {},
|
||||
skipNodeIds?: Set<string>,
|
||||
context?: EmbeddingContext,
|
||||
existingEmbeddings?: Map<string, string>,
|
||||
): Promise<EmbeddingPipelineResult> => {
|
||||
const finalConfig = resolveEmbeddingConfig(config);
|
||||
|
|
@ -321,21 +353,16 @@ export const runEmbeddingPipeline = async (
|
|||
// Phase 2: Query embeddable nodes
|
||||
let nodes = await queryEmbeddableNodes(executeQuery);
|
||||
|
||||
// Apply context metadata
|
||||
if (context?.repoName) {
|
||||
for (const node of nodes) {
|
||||
node.repoName = context.repoName;
|
||||
node.serverName = context.serverName;
|
||||
}
|
||||
}
|
||||
|
||||
// Incremental mode: compare content hashes, delete stale rows, skip fresh ones.
|
||||
// Computed hashes for stale nodes are cached so batchInsertEmbeddings can reuse them
|
||||
// (avoids double computation).
|
||||
const computedStaleHashes = new Map<string, string>();
|
||||
// Stale rows are DELETE'd per-batch (just before each batch's INSERT) rather
|
||||
// than all up front — see U6 / KTD7. `staleNodeIds` is consulted inside the
|
||||
// batch loop; it stays empty in full (non-incremental) mode so no deletes fire.
|
||||
const staleNodeIds = new Set<string>();
|
||||
if (existingEmbeddings && existingEmbeddings.size > 0) {
|
||||
const beforeCount = nodes.length;
|
||||
const staleNodeIds: string[] = [];
|
||||
nodes = nodes.filter((n) => {
|
||||
const existingHash = existingEmbeddings.get(n.id);
|
||||
if (existingHash === undefined) {
|
||||
|
|
@ -346,40 +373,16 @@ export const runEmbeddingPipeline = async (
|
|||
if (currentHash !== existingHash) {
|
||||
// Content changed — cache hash for reuse during insert, mark for DELETE + re-embed
|
||||
computedStaleHashes.set(n.id, currentHash);
|
||||
staleNodeIds.push(n.id);
|
||||
staleNodeIds.add(n.id);
|
||||
return true;
|
||||
}
|
||||
// Hash matches — skip (fresh); no need to cache hash for skipped nodes
|
||||
return false;
|
||||
});
|
||||
|
||||
// DELETE stale embedding rows so they can be re-inserted
|
||||
// (Kuzu forbids SET on vector-indexed properties; DELETE-then-INSERT is the sanctioned pattern)
|
||||
if (staleNodeIds.length > 0) {
|
||||
if (isDev) {
|
||||
logger.info(`🔄 Deleting ${staleNodeIds.length} stale embedding rows for re-embed`);
|
||||
}
|
||||
try {
|
||||
await executeWithReusedStatement(
|
||||
`MATCH (e:${EMBEDDING_TABLE_NAME} {nodeId: $nodeId}) DELETE e`,
|
||||
staleNodeIds.map((nodeId) => ({ nodeId })),
|
||||
);
|
||||
} catch (err) {
|
||||
// "does not exist" = rows already gone — safe to proceed.
|
||||
// All other errors risk vector-index corruption (Kuzu requires DELETE-before-INSERT
|
||||
// for vector-indexed properties) — propagate so the pipeline aborts cleanly.
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
if (!msg.includes('does not exist')) {
|
||||
throw new Error(
|
||||
`[embed] Failed to delete stale embedding rows — aborting to prevent vector-index corruption: ${msg}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (isDev) {
|
||||
logger.info(
|
||||
`📦 Incremental embeddings: ${beforeCount} total, ${existingEmbeddings.size} cached, ${staleNodeIds.length} stale, ${nodes.length} to embed`,
|
||||
`📦 Incremental embeddings: ${beforeCount} total, ${existingEmbeddings.size} cached, ${staleNodeIds.size} stale, ${nodes.length} to embed`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
@ -504,6 +507,12 @@ export const runEmbeddingPipeline = async (
|
|||
}
|
||||
}
|
||||
|
||||
// U6 / KTD7: delete this batch's stale rows immediately before its inserts,
|
||||
// so an interrupted re-embed loses at most one batch (not the whole index).
|
||||
// Preserves Kuzu's required DELETE-before-INSERT for vector-indexed rows.
|
||||
const batchStaleIds = batch.filter((n) => staleNodeIds.has(n.id)).map((n) => n.id);
|
||||
await deleteStaleEmbeddingRows(executeWithReusedStatement, batchStaleIds);
|
||||
|
||||
// Embed chunk texts in sub-batches to control memory
|
||||
const EMBED_SUB_BATCH = finalConfig.subBatchSize;
|
||||
for (let si = 0; si < allTexts.length; si += EMBED_SUB_BATCH) {
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
/**
|
||||
* Text Generator Module
|
||||
*
|
||||
* Generates enriched embedding text from code nodes with metadata.
|
||||
* Generates compact, description-forward embedding text from code nodes.
|
||||
* Supports chunkable labels (Function/Method with AST chunking),
|
||||
* Class-specific structural text, and short-node direct embed.
|
||||
*
|
||||
|
|
@ -58,33 +58,51 @@ const cleanContent = (content: string): string => {
|
|||
};
|
||||
|
||||
/**
|
||||
* Build metadata header for a node
|
||||
* Compact location signal for the embedding header: the last 1-2 path segments
|
||||
* (immediate parent dir + basename), never the full deep path.
|
||||
*
|
||||
* #2333 / PR #2334 tri-review: U1 dropped the location entirely, which regressed
|
||||
* path/service-qualified semantic search (e.g. `billing/handler` vs
|
||||
* `identity/handler` in a monorepo) — and FTS indexes only name/content/description,
|
||||
* never `filePath`, so there is no keyword backfill. The bounded form restores the
|
||||
* discriminating tokens (service dir + filename-concept) at a fraction of the
|
||||
* dilution the full path caused.
|
||||
*/
|
||||
const buildMetadataHeader = (node: EmbeddableNode, config: Partial<EmbeddingConfig>): string => {
|
||||
const boundedLocation = (filePath: string): string => {
|
||||
const segments = filePath.replace(/\\/g, '/').split('/').filter(Boolean);
|
||||
return segments.slice(-2).join('/');
|
||||
};
|
||||
|
||||
/**
|
||||
* Build a compact, description-forward header for embedding text.
|
||||
*
|
||||
* Issue #2333 (sub-issue of #2326), Option A: lead the embedding text with the
|
||||
* symbol name + doc-comment description and drop the low-signal metadata lines
|
||||
* (`Repo`/`Server`/`Export` and the verbose full `Path`). For short doc comments
|
||||
* those lines used to be ~25-30% of the embedding text, diluting the description's
|
||||
* semantic weight in the vector and weakening description-shaped search — worst
|
||||
* for CJK, where a complete concept is often 4-20 characters.
|
||||
*
|
||||
* A *bounded* location signal (last 1-2 path segments) is kept after the
|
||||
* description — see `boundedLocation` for why the full path drop was reversed.
|
||||
*
|
||||
* Full metadata is unaffected: it lives on the graph node properties, which is
|
||||
* what display/context tools read. Only the embedding text changes here.
|
||||
*
|
||||
* Option B (reorder only, keep metadata) was rejected — mean-pooled embeddings
|
||||
* weight by token proportion, not position, so reordering alone barely moves the
|
||||
* signal. Option C (a separate description-only embedding + hybrid merge) is
|
||||
* deferred to follow-up; build it only if Option A proves insufficient against
|
||||
* real measurement. Any change to this template MUST bump EMBEDDING_TEXT_VERSION.
|
||||
*/
|
||||
const buildEmbeddingHeader = (node: EmbeddableNode, config: Partial<EmbeddingConfig>): string => {
|
||||
const parts: string[] = [];
|
||||
|
||||
// Label + name
|
||||
parts.push(`${node.label}: ${node.name}`);
|
||||
|
||||
// Repo name
|
||||
if (node.repoName) {
|
||||
parts.push(`Repo: ${node.repoName}`);
|
||||
}
|
||||
|
||||
// Server name (optional)
|
||||
if (node.serverName) {
|
||||
parts.push(`Server: ${node.serverName}`);
|
||||
}
|
||||
|
||||
// Full file path
|
||||
parts.push(`Path: ${node.filePath}`);
|
||||
|
||||
// Export status
|
||||
if (node.isExported !== undefined) {
|
||||
parts.push(`Export: ${node.isExported}`);
|
||||
}
|
||||
|
||||
// Description (truncated)
|
||||
// Description hoisted above everything else so its semantic signal dominates
|
||||
// the embedding vector and is never the part lost to token-limit truncation.
|
||||
if (node.description) {
|
||||
const maxLen = config.maxDescriptionLength ?? DEFAULT_EMBEDDING_CONFIG.maxDescriptionLength;
|
||||
const truncated = truncateDescription(node.description, maxLen);
|
||||
|
|
@ -93,6 +111,16 @@ const buildMetadataHeader = (node: EmbeddableNode, config: Partial<EmbeddingConf
|
|||
}
|
||||
}
|
||||
|
||||
// Bounded location signal — placed after the description so the description
|
||||
// still leads the vector. Restores path/service disambiguation lost when the
|
||||
// full Path line was dropped (FTS does not index filePath to backfill it).
|
||||
if (node.filePath) {
|
||||
const loc = boundedLocation(node.filePath);
|
||||
if (loc) {
|
||||
parts.push(`Loc: ${loc}`);
|
||||
}
|
||||
}
|
||||
|
||||
return parts.join('\n');
|
||||
};
|
||||
|
||||
|
|
@ -102,7 +130,7 @@ const generateCodeBodyText = (
|
|||
config: Partial<EmbeddingConfig>,
|
||||
prevTail?: string,
|
||||
): string => {
|
||||
const header = buildMetadataHeader(node, config);
|
||||
const header = buildEmbeddingHeader(node, config);
|
||||
const parts = [header];
|
||||
if (prevTail) {
|
||||
parts.push(`[preceding context]: ...${cleanContent(prevTail)}`);
|
||||
|
|
@ -128,7 +156,7 @@ const generateStructuralTypeText = (
|
|||
chunkIndex?: number,
|
||||
prevTail?: string,
|
||||
): string => {
|
||||
const header = buildMetadataHeader(node, config);
|
||||
const header = buildEmbeddingHeader(node, config);
|
||||
const parts: string[] = [header];
|
||||
const isFirstChunk = chunkIndex === undefined || chunkIndex === 0;
|
||||
const cleanedContent = cleanContent(node.content);
|
||||
|
|
@ -253,7 +281,7 @@ export const generateEmbeddingText = (
|
|||
prevTail?: string,
|
||||
): string => {
|
||||
if (isShortLabel(node.label)) {
|
||||
const header = buildMetadataHeader(node, config);
|
||||
const header = buildEmbeddingHeader(node, config);
|
||||
const cleaned = cleanContent(node.content);
|
||||
return `${header}\n\n${cleaned}`;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -289,14 +289,6 @@ export interface CachedEmbedding {
|
|||
contentHash?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Context info for embedding pipeline (repo/server metadata enrichment)
|
||||
*/
|
||||
export interface EmbeddingContext {
|
||||
repoName?: string;
|
||||
serverName?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Model download progress from transformers.js
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -1369,21 +1369,6 @@ export async function runFullAnalysis(
|
|||
}
|
||||
}
|
||||
|
||||
const { readServerMapping } = await import('./embeddings/server-mapping.js');
|
||||
// Mirror the registry's name-resolution chain so the server-mapping
|
||||
// lookup key stays aligned with the final registry name (#1259):
|
||||
// --name → remote-derived → canonical-root basename
|
||||
// (preserved-alias is intentionally NOT consulted here — server
|
||||
// mappings are addressed by the operationally-meaningful name the
|
||||
// user configures, not by a sticky registry-only alias they may not
|
||||
// know about. The previous canonical-only logic ignored both --name
|
||||
// and remote-derived names, silently breaking server-mapping for
|
||||
// anyone with a `--name` alias or remote-named repo.)
|
||||
const projectName =
|
||||
options.registryName ??
|
||||
getInferredRepoName(repoPath) ??
|
||||
path.basename(resolveRepoIdentityRoot(repoPath));
|
||||
const serverName = await readServerMapping(projectName);
|
||||
const embeddingResult = await runEmbeddingPipeline(
|
||||
executeQuery,
|
||||
executeWithReusedStatement,
|
||||
|
|
@ -1399,7 +1384,6 @@ export async function runFullAnalysis(
|
|||
},
|
||||
{},
|
||||
cachedEmbeddingNodeIds.size > 0 ? cachedEmbeddingNodeIds : undefined,
|
||||
{ repoName: projectName, serverName },
|
||||
existingEmbeddings,
|
||||
);
|
||||
if (embeddingResult.semanticMode === 'exact-scan') {
|
||||
|
|
|
|||
|
|
@ -1767,7 +1767,6 @@ export const createServer = async (port: number, host: string = '127.0.0.1') =>
|
|||
},
|
||||
{}, // config: use defaults
|
||||
undefined, // skipNodeIds
|
||||
undefined, // context
|
||||
existingEmbeddings,
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -105,10 +105,11 @@ describe('embedding-chunking integration', () => {
|
|||
|
||||
const text = generateEmbeddingText(node, chunks[0].text);
|
||||
expect(text).toContain('Function: test');
|
||||
expect(text).toContain('Repo: my-project');
|
||||
expect(text).toContain('Server: my-service');
|
||||
expect(text).toContain('Export: true');
|
||||
expect(text).toContain('function hello()');
|
||||
// #2333: verbose metadata is no longer part of embedding text.
|
||||
expect(text).not.toContain('Repo: my-project');
|
||||
expect(text).not.toContain('Server: my-service');
|
||||
expect(text).not.toContain('Export: true');
|
||||
});
|
||||
|
||||
it('long function produces multiple chunks', () => {
|
||||
|
|
@ -282,7 +283,7 @@ describe('embedding-chunking integration', () => {
|
|||
expect(secondText).toContain('age: u32,');
|
||||
});
|
||||
|
||||
it('metadata is present in every chunk', () => {
|
||||
it('header is present in every chunk', () => {
|
||||
const longContent = 'x'.repeat(3000);
|
||||
const node = makeNode({
|
||||
content: longContent,
|
||||
|
|
@ -294,9 +295,11 @@ describe('embedding-chunking integration', () => {
|
|||
|
||||
for (const chunk of chunks) {
|
||||
const text = generateEmbeddingText(node, chunk.text);
|
||||
// The compact header (name, + description when present) repeats on every
|
||||
// chunk so each chunk keeps its identity; #2333 dropped the metadata lines.
|
||||
expect(text).toContain('Function: test');
|
||||
expect(text).toContain('Repo: test-repo');
|
||||
expect(text).toContain('Path: src/test.ts');
|
||||
expect(text).not.toContain('Repo: test-repo');
|
||||
expect(text).not.toContain('Path: src/test.ts');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -101,11 +101,29 @@ describe('contentHashForNode', () => {
|
|||
expect(contentHashForNode(original)).not.toBe(contentHashForNode(edited));
|
||||
});
|
||||
|
||||
it('changes when filePath differs', () => {
|
||||
const a = makeNode({ filePath: 'src/a.ts' });
|
||||
const b = makeNode({ filePath: 'src/b.ts' });
|
||||
// Different filePaths lead to different embedding text ⇒ different hashes
|
||||
expect(contentHashForNode(a)).not.toBe(contentHashForNode(b));
|
||||
it('depends on the bounded location (last 1-2 segments) but not the deep path prefix (#2333 U3)', () => {
|
||||
// U3 reinstated a BOUNDED location signal (last 1-2 path segments) in the
|
||||
// embedding header, so the hash now tracks that signal — but only it, not the
|
||||
// full deep prefix. Same last-2-segments ⇒ identical embedding text ⇒ identical
|
||||
// hash, even with a totally different prefix.
|
||||
const samePrefixA = makeNode({ filePath: 'src/very/deep/nested/svc/Impl.ts' });
|
||||
const samePrefixB = makeNode({ filePath: 'other/svc/Impl.ts' });
|
||||
expect(contentHashForNode(samePrefixA)).toBe(contentHashForNode(samePrefixB));
|
||||
|
||||
// Different last segments (e.g. a real service-folder move) ⇒ different bounded
|
||||
// location ⇒ different hash, so the re-embed correctly picks up the new location.
|
||||
const billing = makeNode({ filePath: 'billing/handler.ts' });
|
||||
const identity = makeNode({ filePath: 'identity/handler.ts' });
|
||||
expect(contentHashForNode(billing)).not.toBe(contentHashForNode(identity));
|
||||
});
|
||||
|
||||
it('is independent of repoName/serverName/isExported (#2333 — dropped from header)', () => {
|
||||
// #2333 dropped these three (alongside filePath) from the embedding header.
|
||||
// The hash must not depend on them; if any were re-added to the header, this
|
||||
// assertion flips and flags the silent re-coupling before it ships.
|
||||
const a = makeNode({ repoName: 'repo-a', serverName: 'svc-a', isExported: true });
|
||||
const b = makeNode({ repoName: 'repo-b', serverName: 'svc-b', isExported: false });
|
||||
expect(contentHashForNode(a)).toBe(contentHashForNode(b));
|
||||
});
|
||||
|
||||
it('produces identical hash regardless of config vs finalConfig when config is empty', () => {
|
||||
|
|
@ -116,7 +134,7 @@ describe('contentHashForNode', () => {
|
|||
});
|
||||
|
||||
it('exports a text template version marker', () => {
|
||||
expect(EMBEDDING_TEXT_VERSION).toBe('v2');
|
||||
expect(EMBEDDING_TEXT_VERSION).toBe('v4');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -290,7 +308,6 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
onProgress,
|
||||
{},
|
||||
undefined, // skipNodeIds
|
||||
undefined, // context
|
||||
existingEmbeddings,
|
||||
);
|
||||
|
||||
|
|
@ -326,7 +343,6 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
onProgress,
|
||||
{},
|
||||
undefined, // skipNodeIds
|
||||
undefined, // context
|
||||
existingEmbeddings,
|
||||
);
|
||||
|
||||
|
|
@ -402,7 +418,6 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
onProgress,
|
||||
{},
|
||||
undefined,
|
||||
undefined,
|
||||
new Map(),
|
||||
);
|
||||
|
||||
|
|
@ -410,9 +425,19 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
const classText = embeddedTexts.find((text) => text.includes('Class: Parser'));
|
||||
const enumText = embeddedTexts.find((text) => text.includes('Enum: Status'));
|
||||
|
||||
expect(classText).toContain('Export: true');
|
||||
// #2333 dropped Export/metadata from embedding text, but the description
|
||||
// assertions still prove the positional column mapping is correct. The Class
|
||||
// row carries isExported at index 7 and description at index 8; the Enum row
|
||||
// has no isExported column (description at index 7), exercising the other
|
||||
// mapping branch. The toContain checks below are the primary guard: an
|
||||
// off-by-one would put the boolean from index 7 into description, so the real
|
||||
// text would be absent, failing here.
|
||||
expect(classText).toContain('Parses typed payloads.');
|
||||
expect(enumText).not.toContain('Export:');
|
||||
// Header-integrity guard (#2333 U5): the embedding text must start with the
|
||||
// `Label: name` header. A positional mis-map that corrupted the header line
|
||||
// (e.g. the name column shifting) is caught here directly, instead of via the
|
||||
// old narrow `not.toContain('\ntrue')` coincidence.
|
||||
expect(classText).toMatch(/^Class: Parser\n/);
|
||||
expect(enumText).toContain('Represents user status.');
|
||||
});
|
||||
|
||||
|
|
@ -435,7 +460,6 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
onProgress,
|
||||
{},
|
||||
undefined, // skipNodeIds
|
||||
undefined, // context
|
||||
existingEmbeddings,
|
||||
);
|
||||
|
||||
|
|
@ -468,7 +492,6 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
onProgress,
|
||||
{},
|
||||
undefined, // skipNodeIds
|
||||
undefined, // context
|
||||
existingEmbeddings,
|
||||
);
|
||||
|
||||
|
|
@ -481,6 +504,87 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
expect(createCalls.length).toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
it('deletes each batch stale rows interleaved with its insert, not all up front (#2333 U6)', async () => {
|
||||
mockEmbedderSetup();
|
||||
|
||||
const n1 = makeNode({ id: 'Function:a:src/a.ts', name: 'a', filePath: 'src/a.ts' });
|
||||
const n2 = makeNode({ id: 'Function:b:src/b.ts', name: 'b', filePath: 'src/b.ts' });
|
||||
// Both stale (hash mismatch) → both re-embed.
|
||||
const existingEmbeddings = new Map<string, string>([
|
||||
[n1.id, 'wronghash1'],
|
||||
[n2.id, 'wronghash2'],
|
||||
]);
|
||||
|
||||
const executeQuery = mockExecuteQuery([n1, n2]);
|
||||
const executeWithReusedStatement = mockExecuteWithReusedStatement();
|
||||
|
||||
const { runEmbeddingPipeline } =
|
||||
await import('../../src/core/embeddings/embedding-pipeline.js');
|
||||
|
||||
await runEmbeddingPipeline(
|
||||
executeQuery,
|
||||
executeWithReusedStatement,
|
||||
onProgress,
|
||||
{ batchSize: 1 }, // one node per batch → two batches
|
||||
undefined, // skipNodeIds
|
||||
existingEmbeddings,
|
||||
);
|
||||
|
||||
// U6 / KTD7: per-batch interleaving means TWO separate DELETE calls (one per
|
||||
// batch), not one up-front bulk delete of both stale rows.
|
||||
const deleteCalls = stmtCalls.filter((c) => c.cypher.includes('DELETE'));
|
||||
expect(deleteCalls.length).toBe(2);
|
||||
|
||||
// Ordering proof: batch 1's INSERT lands BEFORE batch 2's DELETE. An up-front
|
||||
// bulk delete would put both DELETEs before any INSERT, failing this — so an
|
||||
// interrupted re-embed can lose at most one batch, never the whole index.
|
||||
const insertN1 = stmtCalls.findIndex(
|
||||
(c) => c.cypher.includes('CREATE') && c.params.some((p) => p.nodeId === n1.id),
|
||||
);
|
||||
const deleteN2 = stmtCalls.findIndex(
|
||||
(c) => c.cypher.includes('DELETE') && c.params.some((p) => p.nodeId === n2.id),
|
||||
);
|
||||
expect(insertN1).toBeGreaterThanOrEqual(0);
|
||||
expect(deleteN2).toBeGreaterThanOrEqual(0);
|
||||
expect(insertN1).toBeLessThan(deleteN2);
|
||||
});
|
||||
|
||||
it('deletes only stale nodes — new and unchanged nodes are never deleted (#2333 U6)', async () => {
|
||||
mockEmbedderSetup();
|
||||
|
||||
const unchanged = makeNode({ id: 'Function:u:src/u.ts', name: 'u', filePath: 'src/u.ts' });
|
||||
const stale = makeNode({ id: 'Function:s:src/s.ts', name: 's', filePath: 'src/s.ts' });
|
||||
const brandNew = makeNode({ id: 'Function:n:src/n.ts', name: 'n', filePath: 'src/n.ts' });
|
||||
const unchangedHash = contentHashForNode(unchanged, DEFAULT_EMBEDDING_CONFIG);
|
||||
const existingEmbeddings = new Map<string, string>([
|
||||
[unchanged.id, unchangedHash], // hash matches → skipped, no delete
|
||||
[stale.id, 'wronghash'], // hash mismatch → deleted + re-embed
|
||||
// brandNew absent from the map → new → embedded, no delete
|
||||
]);
|
||||
|
||||
const executeQuery = mockExecuteQuery([unchanged, stale, brandNew]);
|
||||
const executeWithReusedStatement = mockExecuteWithReusedStatement();
|
||||
|
||||
const { runEmbeddingPipeline } =
|
||||
await import('../../src/core/embeddings/embedding-pipeline.js');
|
||||
|
||||
await runEmbeddingPipeline(
|
||||
executeQuery,
|
||||
executeWithReusedStatement,
|
||||
onProgress,
|
||||
{ batchSize: 1 },
|
||||
undefined, // skipNodeIds
|
||||
existingEmbeddings,
|
||||
);
|
||||
|
||||
const deletedIds = stmtCalls
|
||||
.filter((c) => c.cypher.includes('DELETE'))
|
||||
.flatMap((c) => c.params.map((p) => p.nodeId));
|
||||
expect(deletedIds).toContain(stale.id);
|
||||
expect(deletedIds).not.toContain(brandNew.id);
|
||||
expect(deletedIds).not.toContain(unchanged.id);
|
||||
});
|
||||
|
||||
it('calls createVectorIndex even when zero nodes need embedding after filter', async () => {
|
||||
mockEmbedderSetup();
|
||||
|
||||
|
|
@ -501,7 +605,6 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
onProgress,
|
||||
{},
|
||||
undefined, // skipNodeIds
|
||||
undefined, // context
|
||||
existingEmbeddings,
|
||||
);
|
||||
|
||||
|
|
@ -623,7 +726,6 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
onProgress,
|
||||
{ chunkSize: 90, overlap: 0 },
|
||||
undefined,
|
||||
undefined,
|
||||
new Map(),
|
||||
);
|
||||
|
||||
|
|
@ -678,7 +780,6 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
onProgress,
|
||||
{ chunkSize: CLASS_CHUNK_SIZE, overlap: CLASS_OVERLAP },
|
||||
undefined,
|
||||
undefined,
|
||||
new Map(),
|
||||
);
|
||||
|
||||
|
|
@ -714,7 +815,6 @@ describe('runEmbeddingPipeline incremental filter', () => {
|
|||
onProgress,
|
||||
{},
|
||||
undefined, // skipNodeIds
|
||||
undefined, // context
|
||||
existingEmbeddings,
|
||||
),
|
||||
).rejects.toThrow('vector-index corruption');
|
||||
|
|
|
|||
|
|
@ -19,32 +19,30 @@ const baseNode: EmbeddableNode = {
|
|||
|
||||
describe('text-generator', () => {
|
||||
describe('generateEmbeddingText', () => {
|
||||
it('includes metadata header for Function', () => {
|
||||
it('leads with name and code, dropping verbose metadata lines (#2333)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
isExported: true,
|
||||
repoName: 'backend-user-ms',
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
// Compact embedding header: name + code remain.
|
||||
expect(text).toContain('Function: parseJSON');
|
||||
expect(text).toContain('Repo: backend-user-ms');
|
||||
expect(text).toContain('Path: src/utils/parser.ts');
|
||||
expect(text).toContain('Export: true');
|
||||
expect(text).toContain('function parseJSON');
|
||||
// Low-signal metadata lines are intentionally excluded from embedding text.
|
||||
expect(text).not.toContain('Repo: backend-user-ms');
|
||||
expect(text).not.toContain('Path: src/utils/parser.ts');
|
||||
expect(text).not.toContain('Export: true');
|
||||
});
|
||||
|
||||
it('includes Server line when serverName is set', () => {
|
||||
it('excludes the Server line from embedding text even when serverName is set (#2333)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
repoName: 'backend-user-ms',
|
||||
serverName: 'user-service',
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('Server: user-service');
|
||||
});
|
||||
|
||||
it('omits Server line when serverName is undefined', () => {
|
||||
const text = generateEmbeddingText(baseNode, baseNode.content);
|
||||
expect(text).not.toContain('Server: user-service');
|
||||
expect(text).not.toContain('Server:');
|
||||
});
|
||||
|
||||
|
|
@ -57,6 +55,179 @@ describe('text-generator', () => {
|
|||
expect(text).toContain('This function parses JSON text');
|
||||
});
|
||||
|
||||
// #2333: short doc comments must not be diluted by metadata. The description
|
||||
// is hoisted directly under the name, ahead of the code body, and the
|
||||
// low-signal metadata lines are dropped from embedding text entirely.
|
||||
it('hoists a short English description above the code body (#2333)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
label: 'Method',
|
||||
name: 'updateMaterialExpiryDate',
|
||||
description: 'validate user',
|
||||
isExported: false,
|
||||
repoName: 'my-project',
|
||||
content:
|
||||
'function updateMaterialExpiryDate(paramMap) {\n // ... a long method body ...\n return doWork(paramMap);\n}',
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('validate user');
|
||||
// Description appears before the code body.
|
||||
expect(text.indexOf('validate user')).toBeLessThan(text.indexOf('return doWork'));
|
||||
// Metadata noise removed.
|
||||
expect(text).not.toContain('Repo: my-project');
|
||||
expect(text).not.toContain('Path:');
|
||||
expect(text).not.toContain('Export:');
|
||||
});
|
||||
|
||||
it('hoists a short CJK description above the code body (#2333)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
label: 'Method',
|
||||
name: 'updateMaterialExpiryDate',
|
||||
description: '更新物料有效期',
|
||||
isExported: false,
|
||||
repoName: 'my-project',
|
||||
content:
|
||||
'function updateMaterialExpiryDate(paramMap) {\n // ... a long method body ...\n return doWork(paramMap);\n}',
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('更新物料有效期');
|
||||
expect(text.indexOf('更新物料有效期')).toBeLessThan(text.indexOf('return doWork'));
|
||||
expect(text).not.toContain('Repo: my-project');
|
||||
expect(text).not.toContain('Path:');
|
||||
expect(text).not.toContain('Export:');
|
||||
});
|
||||
|
||||
it('hoists description in short-label nodes too (#2333)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
label: 'Const',
|
||||
name: 'MAX_RETRIES',
|
||||
description: 'retry ceiling',
|
||||
content: 'const MAX_RETRIES = 5;',
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('Const: MAX_RETRIES');
|
||||
expect(text).toContain('retry ceiling');
|
||||
expect(text.indexOf('retry ceiling')).toBeLessThan(text.indexOf('const MAX_RETRIES = 5;'));
|
||||
expect(text).not.toContain('Path:');
|
||||
});
|
||||
|
||||
it('keeps structural Methods/Properties lines under the compact header (#2333)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
label: 'Class',
|
||||
name: 'Parser',
|
||||
description: 'JSON parser',
|
||||
repoName: 'my-project',
|
||||
methodNames: ['parseJSON', 'validate'],
|
||||
fieldNames: ['options', 'cache'],
|
||||
content: `class Parser {
|
||||
options: ParserOptions;
|
||||
private cache: Map<string, any>;
|
||||
parseJSON(text: string) { return JSON.parse(text); }
|
||||
validate() { return true; }
|
||||
}`,
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('Class: Parser');
|
||||
expect(text).toContain('JSON parser');
|
||||
// Structural signal must survive the compact-header change.
|
||||
expect(text).toContain('Methods: parseJSON, validate');
|
||||
expect(text).toContain('Properties: options, cache');
|
||||
// Description is hoisted ahead of the structural lines (ordering guard for
|
||||
// the structural path, mirroring the function/method ordering checks).
|
||||
expect(text.indexOf('JSON parser')).toBeLessThan(text.indexOf('Container:'));
|
||||
expect(text.indexOf('JSON parser')).toBeLessThan(text.indexOf('Methods:'));
|
||||
// Metadata noise still dropped.
|
||||
expect(text).not.toContain('Repo: my-project');
|
||||
});
|
||||
|
||||
it('emits no description line and no metadata when description is absent (#2333)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
isExported: true,
|
||||
repoName: 'my-project',
|
||||
description: undefined,
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
// Header is the name line, then the bounded location line, then a blank
|
||||
// line, then the code body — no stray empty description line, no verbose
|
||||
// metadata.
|
||||
expect(
|
||||
text.startsWith('Function: parseJSON\nLoc: utils/parser.ts\n\nfunction parseJSON'),
|
||||
).toBe(true);
|
||||
expect(text).not.toContain('Repo:');
|
||||
expect(text).not.toContain('Export:');
|
||||
// Only the bounded last-1-2 segments — never the verbose deep path.
|
||||
expect(text).not.toContain('Path:');
|
||||
expect(text).not.toContain('src/utils/parser.ts');
|
||||
});
|
||||
|
||||
// U3 (#2333 PR #2334 tri-review): a BOUNDED location signal (last 1-2 path
|
||||
// segments) is reinstated so path/service-qualified semantic search keeps a
|
||||
// discriminator, since FTS does not index filePath.
|
||||
it('emits a bounded location (last 2 segments), not the full deep path (#2333 U3)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
label: 'Method',
|
||||
name: 'updateMaterialExpiryDate',
|
||||
filePath: 'src/main/java/com/example/service/MaterialServiceImpl.java',
|
||||
content: 'function updateMaterialExpiryDate() { return doWork(); }',
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('Loc: service/MaterialServiceImpl.java');
|
||||
// The deep prefix is dropped entirely.
|
||||
expect(text).not.toContain('src/main/java/com/example');
|
||||
});
|
||||
|
||||
it('emits just the basename for a root-level file (#2333 U3)', () => {
|
||||
const node: EmbeddableNode = { ...baseNode, filePath: 'index.ts' };
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('Loc: index.ts');
|
||||
// No leading slash and no stray "undefined/" prefix from slicing one segment.
|
||||
expect(text).not.toContain('Loc: /index.ts');
|
||||
expect(text).not.toContain('undefined');
|
||||
});
|
||||
|
||||
it('disambiguates same-named symbols in different service folders (#2333 U3)', () => {
|
||||
const billing = generateEmbeddingText(
|
||||
{ ...baseNode, name: 'handler', filePath: 'billing/handler.ts' },
|
||||
'function handler() {}',
|
||||
);
|
||||
const identity = generateEmbeddingText(
|
||||
{ ...baseNode, name: 'handler', filePath: 'identity/handler.ts' },
|
||||
'function handler() {}',
|
||||
);
|
||||
expect(billing).toContain('Loc: billing/handler.ts');
|
||||
expect(identity).toContain('Loc: identity/handler.ts');
|
||||
// The two embedding texts differ — the regression the tri-review flagged
|
||||
// (both collapsing to identical vectors) is fixed.
|
||||
expect(billing).not.toBe(identity);
|
||||
});
|
||||
|
||||
it('keeps the description ahead of the location signal (#2333 U3)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
label: 'Method',
|
||||
name: 'doThing',
|
||||
description: 'batch import rows',
|
||||
filePath: 'svc/importer.ts',
|
||||
content: 'function doThing() { return run(); }',
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
// description leads, then the location line, then the code body.
|
||||
expect(text.indexOf('batch import rows')).toBeLessThan(text.indexOf('Loc: svc/importer.ts'));
|
||||
expect(text.indexOf('Loc: svc/importer.ts')).toBeLessThan(text.indexOf('return run()'));
|
||||
});
|
||||
|
||||
it('normalizes Windows path separators in the location signal (#2333 U3)', () => {
|
||||
const node: EmbeddableNode = { ...baseNode, filePath: 'src\\svc\\Foo.ts' };
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('Loc: svc/Foo.ts');
|
||||
expect(text).not.toContain('\\');
|
||||
});
|
||||
|
||||
it('generates short node text for TypeAlias without chunking', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
|
|
@ -167,6 +338,56 @@ describe('text-generator', () => {
|
|||
expect(text).toContain('struct User {');
|
||||
});
|
||||
|
||||
// U5 (#2333 PR #2334): Interface and Struct route through the same
|
||||
// generateStructuralTypeText path as Class, so the description-forward
|
||||
// ordering must hold for them too — guards against a future per-label
|
||||
// specialization silently reordering the header.
|
||||
it('keeps an Interface description ahead of its structural lines (#2333 U5)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
label: 'Interface',
|
||||
name: 'Handler',
|
||||
description: 'event handler contract',
|
||||
methodNames: ['handle', 'validate'],
|
||||
fieldNames: ['name'],
|
||||
content: `interface Handler {
|
||||
handle(event: Event): void;
|
||||
readonly name: string;
|
||||
}`,
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('Interface: Handler');
|
||||
expect(text).toContain('event handler contract');
|
||||
expect(text).toContain('Methods: handle, validate');
|
||||
expect(text).toContain('Properties: name');
|
||||
expect(text.indexOf('event handler contract')).toBeLessThan(text.indexOf('Container:'));
|
||||
expect(text.indexOf('event handler contract')).toBeLessThan(text.indexOf('Methods:'));
|
||||
expect(text).toContain('Loc: utils/parser.ts');
|
||||
expect(text).not.toContain('Repo:');
|
||||
});
|
||||
|
||||
it('keeps a Struct description ahead of its structural lines (#2333 U5)', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
label: 'Struct',
|
||||
name: 'User',
|
||||
description: 'user record',
|
||||
fieldNames: ['name', 'age'],
|
||||
content: `struct User {
|
||||
name: String,
|
||||
age: u32,
|
||||
}`,
|
||||
};
|
||||
const text = generateEmbeddingText(node, node.content);
|
||||
expect(text).toContain('Struct: User');
|
||||
expect(text).toContain('user record');
|
||||
expect(text).toContain('Properties: name, age');
|
||||
expect(text).toContain('Container: struct User {');
|
||||
expect(text.indexOf('user record')).toBeLessThan(text.indexOf('Container:'));
|
||||
expect(text.indexOf('user record')).toBeLessThan(text.indexOf('Properties:'));
|
||||
expect(text).toContain('Loc: utils/parser.ts');
|
||||
});
|
||||
|
||||
it('keeps compact container context on later structural chunks', () => {
|
||||
const node: EmbeddableNode = {
|
||||
...baseNode,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue