From 59047c4e18667463ac61d1a712c7fce5784f465a Mon Sep 17 00:00:00 2001 From: Randy Wilson Date: Fri, 21 Aug 2026 02:47:22 -0400 Subject: [PATCH] Fix the indexer lifecycle, keep git history intact, and make the MCP surface truthful Indexer: the Project node is now created before file linking, so HAS_FILE edges exist and incremental indexing can finally skip unchanged files. Git sync handles root commits (empty-tree diff), symlinked roots (realpath boundary check with caller-namespace edge paths), and joins git paths against the repo root instead of the indexed root. Tier-2 language registration is now actually called. Re-indexing a changed file no longer deletes the File node, so MODIFIED_IN history survives incremental runs and the file watcher. MCP truthfulness: all eight documented knowledge actions are reachable (six were rejected by the enum), recall's temporal and speaker params are declared in the schema, and a doc-consistency test keeps every description example honest. The profile route and codebase persona no longer query phantom properties (f.path, f.language), match project paths on directory boundaries so sibling projects cannot leak into each other, and reject relative paths instead of returning silently empty. Labels: one canonical label taxonomy in @codegraph/types replaces nine hand-copied lists across graph, core, api, and dashboard, with tests locking each deliberate subset. Node ids for Commit, MarkdownDocument, Section, CodeBlock, Link, and Entity now key on their real MERGE identity instead of colliding on empty symbol fields. Co-Authored-By: Claude Fable 5 --- packages/api/package.json | 1 + .../api/src/__tests__/profile-route.test.ts | 56 ++++ packages/api/src/__tests__/profile.test.ts | 131 +++++++- packages/api/src/__tests__/search.test.ts | 15 +- packages/api/src/routes/profile.ts | 130 +++++++- .../src/__tests__/gitsync-repo-root.test.ts | 126 +++++++ .../src/__tests__/gitsync-root-commit.test.ts | 91 +++++ .../gitsync-symlink-namespace.test.ts | 113 +++++++ ...er-git-edges-preserved.integration.test.ts | 120 +++++++ .../indexer-project-link-order.test.ts | 210 ++++++++++++ ...er-single-file-preserves-file-node.test.ts | 132 ++++++++ .../__tests__/indexer-tier2-languages.test.ts | 172 ++++++++++ .../src/__tests__/services-helpers.test.ts | 166 ++++++++++ packages/core/src/embed-nodes.ts | 11 +- packages/core/src/gitSync.ts | 109 +++++- packages/core/src/indexer.ts | 47 ++- packages/core/src/pipeline/pipeline.ts | 23 +- packages/core/src/services/helpers.ts | 149 ++++++++- packages/dashboard/package.json | 1 + .../components/dashboard/embedding-badge.tsx | 6 +- .../src/__tests__/label-constants.test.ts | 179 ++++++++++ .../__tests__/remove-file-contents.test.ts | 144 ++++++++ packages/graph/src/drivers/falkordb-shared.ts | 24 +- packages/graph/src/fileTree.ts | 5 +- packages/graph/src/operations.ts | 54 ++- packages/graph/src/queries.ts | 43 ++- .../mcp-server/src/__tests__/codebase.test.ts | 205 ++++++++++++ .../knowledge-persona-actions.test.ts | 313 ++++++++++++++++++ packages/mcp-server/src/personas/codebase.ts | 127 ++++++- packages/mcp-server/src/personas/knowledge.ts | 86 ++++- packages/types/package.json | 4 +- packages/types/src/__tests__/labels.test.ts | 137 ++++++++ packages/types/src/index.ts | 1 + packages/types/src/labels.ts | 193 +++++++++++ packages/types/tsconfig.json | 2 +- pnpm-lock.yaml | 9 + 36 files changed, 3212 insertions(+), 123 deletions(-) create mode 100644 packages/api/src/__tests__/profile-route.test.ts create mode 100644 packages/core/src/__tests__/gitsync-repo-root.test.ts create mode 100644 packages/core/src/__tests__/gitsync-root-commit.test.ts create mode 100644 packages/core/src/__tests__/gitsync-symlink-namespace.test.ts create mode 100644 packages/core/src/__tests__/indexer-git-edges-preserved.integration.test.ts create mode 100644 packages/core/src/__tests__/indexer-project-link-order.test.ts create mode 100644 packages/core/src/__tests__/indexer-single-file-preserves-file-node.test.ts create mode 100644 packages/core/src/__tests__/indexer-tier2-languages.test.ts create mode 100644 packages/core/src/__tests__/services-helpers.test.ts create mode 100644 packages/graph/src/__tests__/label-constants.test.ts create mode 100644 packages/graph/src/__tests__/remove-file-contents.test.ts create mode 100644 packages/mcp-server/src/__tests__/codebase.test.ts create mode 100644 packages/mcp-server/src/__tests__/knowledge-persona-actions.test.ts create mode 100644 packages/types/src/__tests__/labels.test.ts create mode 100644 packages/types/src/labels.ts diff --git a/packages/api/package.json b/packages/api/package.json index f90a4046..640a7f96 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -23,6 +23,7 @@ "@codegraph/core": "workspace:*", "@codegraph/graph": "workspace:*", "@codegraph/logger": "workspace:*", + "@codegraph/types": "workspace:*", "@hono/node-server": "^1.19.14", "hono": "^4.12.23" }, diff --git a/packages/api/src/__tests__/profile-route.test.ts b/packages/api/src/__tests__/profile-route.test.ts new file mode 100644 index 00000000..a1c85ae3 --- /dev/null +++ b/packages/api/src/__tests__/profile-route.test.ts @@ -0,0 +1,56 @@ +/** + * Route-level coverage for GET /api/profile's projectPath boundary safety. + * + * getProfile()'s own tests (profile.test.ts) prove the filter and validation + * logic in isolation. This file proves the Hono route actually wires that + * validation in: a relative projectPath must come back as a 400 with a + * plain-text error, and must never reach the graph. A pure-function test of + * validateProjectPath alone cannot see that wiring bug; only calling the + * route can (same reasoning as search-route.test.ts). + * + * `@codegraph/core` is mocked so this never touches a real graph. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('@codegraph/core', () => ({ + codeGraphService: { getGraphStats: vi.fn() }, + getGraphClient: vi.fn(), +})); + +import { codeGraphService, getGraphClient } from '@codegraph/core'; +import { profileRoutes } from '../routes/profile'; + +const mockedGetGraphStats = vi.mocked(codeGraphService.getGraphStats); +const mockedGetGraphClient = vi.mocked(getGraphClient); + +describe('GET /api/profile: projectPath validation', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns 400 for a relative projectPath and never touches the graph', async () => { + const res = await profileRoutes.request('/api/profile?projectPath=relative/path'); + expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error).toMatch(/absolute/i); + expect(mockedGetGraphClient).not.toHaveBeenCalled(); + }); + + it('returns 200 for an absolute projectPath', async () => { + mockedGetGraphStats.mockResolvedValue({ + totalNodes: 1, + totalEdges: 0, + nodesByType: { File: 1 }, + edgesByType: {}, + largestFiles: [], + mostConnected: [], + } as never); + mockedGetGraphClient.mockResolvedValue({ + roQuery: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + } as never); + + const res = await profileRoutes.request('/api/profile?projectPath=/abs/path'); + expect(res.status).toBe(200); + }); +}); diff --git a/packages/api/src/__tests__/profile.test.ts b/packages/api/src/__tests__/profile.test.ts index e2254091..fa78e330 100644 --- a/packages/api/src/__tests__/profile.test.ts +++ b/packages/api/src/__tests__/profile.test.ts @@ -1,5 +1,24 @@ import { describe, it, expect, vi } from 'vitest'; -import { getProfile, type ProfileService } from '../routes/profile'; +import { getProfile, validateProjectPath, type ProfileService } from '../routes/profile'; + +/** + * The true File node property set, as upserted by packages/graph/src/schema.ts + * (fileToNodeProps) and packages/graph/src/operations.ts (UPSERT_FILE, + * BATCH_UPSERT_FILES, BATCH_CREATE_FILES). File nodes have `filePath` and + * `extension`, never `path` or `language`, which do not exist on the node. + */ +const REAL_FILE_PROPERTIES = [ + 'filePath', + 'name', + 'extension', + 'loc', + 'lastModified', + 'hash', + 'sourcePipeline', + 'sourceTask', + 'processedAt', +]; +const PHANTOM_FILE_PROPERTIES = ['f.path', 'f.language']; describe('codebase profile', () => { function makeMockService(overrides?: Partial): ProfileService { @@ -12,11 +31,11 @@ describe('codebase profile', () => { if (cypher.includes('callCount')) { return { data: [{ name: 'parseProject', callCount: 12 }] }; } - if (cypher.includes('f.language')) { - return { data: [{ name: 'TypeScript', fileCount: 98 }] }; + if (cypher.includes('f.extension')) { + return { data: [{ extension: 'ts', fileCount: 98 }] }; } if (cypher.includes('lastModified')) { - return { data: [{ filePath: '/src/x.ts', lastModified: Date.now() }] }; + return { data: [{ filePath: '/src/x.ts', lastModified: '2026-08-01T00:00:00.000Z' }] }; } if (cypher.includes('Entity')) { return { data: [{ text: 'JWT auth', type: 'Decision', createdAt: Date.now() }] }; @@ -53,6 +72,38 @@ describe('codebase profile', () => { it('populates recentFiles from query results', async () => { const profile = await getProfile(makeMockService(), { projectPath: '/test' }); expect(profile.dynamic.recentFiles[0]?.filePath).toBe('/src/x.ts'); + expect(profile.dynamic.recentFiles[0]?.lastModified).toBe('2026-08-01T00:00:00.000Z'); + }); + + it('maps the extension-grouped languages query to display names', async () => { + const profile = await getProfile(makeMockService(), { projectPath: '/test' }); + expect(profile.static.languages[0]?.name).toBe('TypeScript'); + expect(profile.static.languages[0]?.fileCount).toBe(98); + }); + + it('never references phantom File properties (f.path, f.language) in generated Cypher', async () => { + const service = makeMockService(); + await getProfile(service, { projectPath: '/test' }); + const calls = (service.query as ReturnType).mock.calls; + for (const [cypher] of calls as [string, unknown][]) { + for (const phantom of PHANTOM_FILE_PROPERTIES) { + expect(cypher).not.toContain(phantom); + } + } + }); + + it('every f. reference in File-scoped queries is a real File node property', async () => { + const service = makeMockService(); + await getProfile(service, { projectPath: '/test' }); + const calls = (service.query as ReturnType).mock.calls; + for (const [cypher] of calls as [string, unknown][]) { + if (!cypher.includes('(f:File)')) continue; + const refs = cypher.match(/\bf\.(\w+)/g) ?? []; + for (const ref of refs) { + const prop = ref.slice(2); + expect(REAL_FILE_PROPERTIES).toContain(prop); + } + } }); it('is fast — completes in under 200ms when service is mocked', async () => { @@ -87,3 +138,75 @@ describe('codebase profile', () => { } }); }); + +describe('validateProjectPath', () => { + it('accepts an absolute path', () => { + expect(validateProjectPath('/abs/path')).toEqual({ valid: true }); + }); + + it('accepts undefined (no filter requested)', () => { + expect(validateProjectPath(undefined)).toEqual({ valid: true }); + }); + + it('rejects a relative path', () => { + const result = validateProjectPath('relative/path'); + expect(result.valid).toBe(false); + }); +}); + +describe('projectPath boundary safety', () => { + function makeMockService(overrides?: Partial): ProfileService { + return { + getStats: vi.fn().mockResolvedValue({ nodes: 2310, edges: 5500, files: 142 }), + query: vi.fn().mockResolvedValue({ data: [] }), + ...overrides, + }; + } + + it('rejects a relative projectPath instead of silently returning an empty profile', async () => { + await expect( + getProfile(makeMockService(), { projectPath: 'relative/path' }), + ).rejects.toThrow(/absolute/i); + }); + + it('never calls the service when projectPath is relative', async () => { + const service = makeMockService(); + await expect(getProfile(service, { projectPath: 'relative/path' })).rejects.toThrow(); + expect(service.getStats).not.toHaveBeenCalled(); + expect(service.query).not.toHaveBeenCalled(); + }); + + it('normalizes a trailing slash before building the filter params', async () => { + const service = makeMockService(); + await getProfile(service, { projectPath: '/tmp/x/project/' }); + const calls = (service.query as ReturnType).mock.calls; + const fileCall = calls.find(([cypher]) => (cypher as string).includes('(f:File)')); + expect(fileCall).toBeDefined(); + const [, params] = fileCall as [string, Record]; + expect(params['projectPath']).toBe('/tmp/x/project'); + expect(params['projectPathPrefix']).toBe('/tmp/x/project/'); + }); + + it('the generated filter cannot match a sibling directory sharing the same prefix', async () => { + const service = makeMockService(); + await getProfile(service, { projectPath: '/tmp/x/project' }); + const calls = (service.query as ReturnType).mock.calls; + const fileCall = calls.find(([cypher]) => (cypher as string).includes('(f:File)')); + expect(fileCall).toBeDefined(); + const [, params] = fileCall as [string, Record]; + + // Reproduces FalkorDB's `= $projectPath OR STARTS WITH $projectPathPrefix` + // semantics against the exact params the code built, so this fails if the + // implementation ever regresses to a plain STARTS WITH. + const matchesFilter = (filePath: string): boolean => + filePath === params['projectPath'] || filePath.startsWith(params['projectPathPrefix'] as string); + + // The leak this guards against: a plain `STARTS WITH "/tmp/x/project"` + // also matches the unrelated sibling directory "/tmp/x/project-extra". + expect(matchesFilter('/tmp/x/project-extra/leaked.ts')).toBe(false); + // A real file inside the project must still match. + expect(matchesFilter('/tmp/x/project/src/index.ts')).toBe(true); + // The project root itself must match too. + expect(matchesFilter('/tmp/x/project')).toBe(true); + }); +}); diff --git a/packages/api/src/__tests__/search.test.ts b/packages/api/src/__tests__/search.test.ts index f7006269..a8958154 100644 --- a/packages/api/src/__tests__/search.test.ts +++ b/packages/api/src/__tests__/search.test.ts @@ -20,13 +20,18 @@ import { describe, it, expect } from 'vitest'; import { resolveTypeFilter, typeFilterNotice } from '../routes/search'; +import { SYMBOL_LABELS } from '@codegraph/types'; // Mirrors what /api/embeddings/status reports for this repository's own -// indexed graph: the seven embeddable code-symbol types, plus Commit, +// indexed graph: the seven embeddable code-symbol types (SYMBOL_LABELS, +// the shared source of truth in packages/types/src/labels.ts), plus Commit, // TypeRef, Project and Metadata, which the vector-search allowlist never -// covered but the Cypher fallback path has always been able to match. -const KNOWN_LABELS = new Set([ - 'File', 'Function', 'Class', 'Interface', 'Variable', 'Type', 'Component', +// covered but the Cypher fallback path has always been able to match. The +// extra four are graph-structure labels specific to this fixture, not a +// canonical subset, so they stay spelled out here rather than living in +// @codegraph/types. +const KNOWN_LABELS = new Set([ + ...SYMBOL_LABELS, 'Commit', 'TypeRef', 'Project', 'Metadata', ]); @@ -74,7 +79,7 @@ describe('resolveTypeFilter', () => { }); it('accepts every label the fallback query defaults to', () => { - for (const label of ['File', 'Function', 'Class', 'Interface', 'Variable', 'Type', 'Component']) { + for (const label of SYMBOL_LABELS) { expect(resolveTypeFilter(label, KNOWN_LABELS)).toEqual({ kind: 'labels', labels: [label] }); } }); diff --git a/packages/api/src/routes/profile.ts b/packages/api/src/routes/profile.ts index 2fc54bf3..dbbb4b36 100644 --- a/packages/api/src/routes/profile.ts +++ b/packages/api/src/routes/profile.ts @@ -8,6 +8,7 @@ */ import { Hono } from 'hono'; +import { isAbsolute } from 'node:path'; import { safeErrorMessage } from '../safe-error'; export const profileRoutes = new Hono(); @@ -24,11 +25,67 @@ export interface CodebaseProfile { languages: Array<{ name: string; fileCount: number }>; }; dynamic: { - recentFiles: Array<{ filePath: string; lastModified: number }>; + /** lastModified is the File node's ISO 8601 timestamp string, not epoch millis. */ + recentFiles: Array<{ filePath: string; lastModified: string }>; recentEntities: Array<{ text: string; type: string; createdAt: number }>; }; } +// ============================================================================ +// Extension -> language display name +// ============================================================================ + +/** + * File nodes only carry an `extension` property (see fileToNodeProps in + * packages/graph/src/schema.ts) - there is no `language` property in the + * graph. This maps common bare extensions (no leading dot) to a + * human-readable language name for display; anything not listed falls back + * to the bare extension itself. + */ +const EXTENSION_LANGUAGE_MAP: Record = { + ts: 'TypeScript', + tsx: 'TypeScript', + js: 'JavaScript', + jsx: 'JavaScript', + mjs: 'JavaScript', + cjs: 'JavaScript', + py: 'Python', + go: 'Go', + rs: 'Rust', + java: 'Java', + rb: 'Ruby', + php: 'PHP', + c: 'C', + h: 'C', + cpp: 'C++', + cc: 'C++', + cxx: 'C++', + hpp: 'C++', + cs: 'C#', + swift: 'Swift', + kt: 'Kotlin', + kts: 'Kotlin', + scala: 'Scala', + sh: 'Shell', + bash: 'Shell', + md: 'Markdown', + json: 'JSON', + yaml: 'YAML', + yml: 'YAML', + html: 'HTML', + css: 'CSS', + scss: 'SCSS', + sql: 'SQL', + vue: 'Vue', + svelte: 'Svelte', +}; + +/** Maps a bare file extension to a display language name, falling back to the extension itself. */ +export function languageNameForExtension(extension: string | null | undefined): string { + if (!extension) return 'Unknown'; + return EXTENSION_LANGUAGE_MAP[extension] ?? extension; +} + export interface ProfileService { getStats(): Promise<{ nodes: number; edges: number; files: number }>; query( @@ -37,6 +94,29 @@ export interface ProfileService { ): Promise<{ data: unknown[] }>; } +// ============================================================================ +// projectPath validation +// ============================================================================ + +/** + * Reject a projectPath that isn't absolute instead of letting it silently + * build a filter that matches nothing. A relative path never matches an + * indexed File's filePath (those are always absolute), so the old behavior + * was a quiet empty profile with no sign anything was wrong. + */ +export function validateProjectPath( + projectPath: string | undefined, +): { valid: true } | { valid: false; error: string } { + if (!projectPath) return { valid: true }; + if (!isAbsolute(projectPath)) { + return { + valid: false, + error: 'projectPath must be an absolute path: a relative path would not match any indexed file', + }; + } + return { valid: true }; +} + // ============================================================================ // Core function (also used by MCP codebase.profile action) // ============================================================================ @@ -53,15 +133,30 @@ export async function getProfile( const limit = opts.limit ?? 10; const projectPath = opts.projectPath; - // FalkorDB filter clauses - const nodeFilter = projectPath - ? 'WHERE n.filePath STARTS WITH $projectPath' + const pathCheck = validateProjectPath(projectPath); + if (!pathCheck.valid) { + throw new Error(pathCheck.error); + } + + // Strip a trailing slash so the prefix match below is boundary-safe: without + // this, a filter built from "/proj/" would look for the literal (and + // never-occurring) prefix "/proj//". + const normalizedProjectPath = projectPath ? projectPath.replace(/\/+$/, '') : undefined; + const projectPathPrefix = normalizedProjectPath ? `${normalizedProjectPath}/` : undefined; + + // FalkorDB filter clauses. A plain `STARTS WITH $projectPath` also matches + // a sibling directory that merely shares the prefix (projectPath + // "/x/project" would match "/x/project-extra/file.ts" too), so require + // either an exact match on the root itself or containment under "root/". + const nodeFilter = normalizedProjectPath + ? 'WHERE (n.filePath = $projectPath OR n.filePath STARTS WITH $projectPathPrefix)' : ''; - const fileFilter = projectPath - ? 'WHERE f.path STARTS WITH $projectPath' + const fileFilter = normalizedProjectPath + ? 'WHERE (f.filePath = $projectPath OR f.filePath STARTS WITH $projectPathPrefix)' : ''; const params: Record = { - projectPath: projectPath ?? null, + projectPath: normalizedProjectPath ?? null, + projectPathPrefix: projectPathPrefix ?? null, limit, }; @@ -96,7 +191,7 @@ export async function getProfile( service .query( `MATCH (f:File) ${fileFilter} - RETURN f.language AS name, count(f) AS fileCount + RETURN f.extension AS extension, count(f) AS fileCount ORDER BY fileCount DESC LIMIT $limit`, params, ) @@ -105,7 +200,7 @@ export async function getProfile( service .query( `MATCH (f:File) ${fileFilter} - RETURN f.path AS filePath, f.lastModified AS lastModified + RETURN f.filePath AS filePath, f.lastModified AS lastModified ORDER BY f.lastModified DESC LIMIT $limit`, params, ) @@ -121,15 +216,19 @@ export async function getProfile( .catch(() => ({ data: [] as unknown[] })), ]); + const languages = (langsRes.data as Array<{ extension: string; fileCount: number }>).map( + (row) => ({ name: languageNameForExtension(row.extension), fileCount: row.fileCount }), + ); + return { stats: rawStats, static: { topImports: topImportsRes.data as Array<{ name: string; importCount: number }>, topCallers: topCallersRes.data as Array<{ name: string; callCount: number }>, - languages: langsRes.data as Array<{ name: string; fileCount: number }>, + languages, }, dynamic: { - recentFiles: recentFilesRes.data as Array<{ filePath: string; lastModified: number }>, + recentFiles: recentFilesRes.data as Array<{ filePath: string; lastModified: string }>, recentEntities: recentEntitiesRes.data as Array<{ text: string; type: string; createdAt: number }>, }, }; @@ -143,6 +242,15 @@ export async function getProfile( profileRoutes.get('/api/profile', async (c) => { try { const projectPath = c.req.query('projectPath') || undefined; + + // Validate before touching the graph: a relative path can never match an + // indexed File's filePath, so let this fail loudly instead of returning + // a silently empty profile. + const pathCheck = validateProjectPath(projectPath); + if (!pathCheck.valid) { + return c.json({ error: pathCheck.error }, 400); + } + const limitStr = c.req.query('limit'); const limit = limitStr ? parseInt(limitStr, 10) : undefined; diff --git a/packages/core/src/__tests__/gitsync-repo-root.test.ts b/packages/core/src/__tests__/gitsync-repo-root.test.ts new file mode 100644 index 00000000..7d7decf7 --- /dev/null +++ b/packages/core/src/__tests__/gitsync-repo-root.test.ts @@ -0,0 +1,126 @@ +/** + * Regression test for the repo-root join bug in syncGitHistory(). + * + * git reports changed-file paths (via `git diff` / `--name-status`) relative + * to the REPOSITORY root, not relative to whatever cwd the command was + * invoked from. syncGitHistory() used to build `${repoPath}/${file.file}`, + * where repoPath is the INDEXED root passed in by the caller. When the + * indexed directory is a subdirectory of the actual git repo (a package in a + * monorepo), that join produces a path that duplicates the subdirectory + * segment and never matches any real File node, so MODIFIED_IN edges + * silently fail to attach. + * + * This test drives a real temporary git repo (git init in a tmp dir) so the + * paths git reports are genuine repo-root-relative paths, and mocks only the + * graph ops layer, so it needs no live FalkorDB. + */ + +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import type { GraphClient } from '@codegraph/graph'; + +// --------------------------------------------------------------------------- +// Mocks. vi.mock is hoisted; mutable state the factory closes over must be +// created via vi.hoisted(). +// --------------------------------------------------------------------------- + +const opsMocks = vi.hoisted(() => ({ + upsertCommit: vi.fn().mockResolvedValue(undefined), + createModifiedInEdge: vi.fn().mockResolvedValue(undefined), + createIntroducedInEdgesForFile: vi.fn().mockResolvedValue(0), + createDeletedInEdgesForFile: vi.fn().mockResolvedValue(0), +})); + +vi.mock('@codegraph/graph', () => ({ + createOperations: vi.fn().mockReturnValue(opsMocks), +})); + +// Import after mocks are declared. +import { syncGitHistory } from '../gitSync'; + +function git(cwd: string, args: string[]): void { + execFileSync('git', args, { cwd, stdio: 'pipe' }); +} + +const fakeClient = { + graph: null, + graphName: 'test', + dialect: {}, + // getMetadata/setMetadata in gitSync.ts talk to the raw client directly. + query: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + roQuery: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + ensureIndexes: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), +} as unknown as GraphClient; + +let repoRoot: string; +let indexedRoot: string; + +beforeAll(() => { + // realpathSync matters here: on macOS, os.tmpdir() returns a path under a + // symlink (/var/folders -> /private/var/folders), and `git rev-parse + // --show-toplevel` resolves to the real path. Canonicalizing up front keeps + // our expected paths in the test in sync with what the fix computes. + repoRoot = realpathSync(mkdtempSync(join(tmpdir(), 'codegraph-gitsync-'))); + git(repoRoot, ['init', '-q']); + git(repoRoot, ['config', 'user.email', 'test@example.com']); + git(repoRoot, ['config', 'user.name', 'Test']); + + // First commit has no parent, so `git diff ^ ` cannot be + // computed for it (expected, non-fatal) -- keep it minimal. + writeFileSync(join(repoRoot, '.gitkeep'), ''); + git(repoRoot, ['add', '-A']); + git(repoRoot, ['commit', '-q', '-m', 'initial']); + + // Second commit adds a file under a subdirectory (the "indexed root", as + // if this were one package in a monorepo) AND a file at the repo root + // (outside the indexed root, so it must be skipped, not mislinked). + indexedRoot = join(repoRoot, 'packages', 'sub'); + mkdirSync(indexedRoot, { recursive: true }); + writeFileSync(join(indexedRoot, 'foo.ts'), 'export const x = 1;\n'); + writeFileSync(join(repoRoot, 'README.md'), '# root file\n'); + git(repoRoot, ['add', '-A']); + git(repoRoot, ['commit', '-q', '-m', 'add files']); +}); + +afterAll(() => { + rmSync(repoRoot, { recursive: true, force: true }); +}); + +beforeEach(() => { + opsMocks.upsertCommit.mockClear(); + opsMocks.createModifiedInEdge.mockClear(); + opsMocks.createIntroducedInEdgesForFile.mockClear(); + opsMocks.createDeletedInEdgesForFile.mockClear(); +}); + +describe('syncGitHistory: joins git paths against the repo root, not the indexed subdirectory', () => { + it('links MODIFIED_IN edges using the real absolute path of files under the indexed root', async () => { + const result = await syncGitHistory(indexedRoot, fakeClient, { maxCommits: 10, includeStats: true }); + + expect(result.errors).toEqual([]); + expect(result.commitsProcessed).toBe(2); + + const modifiedPaths = opsMocks.createModifiedInEdge.mock.calls.map((call) => call[0] as string); + + // foo.ts lives under the indexed root; the correct absolute path is + // indexedRoot/foo.ts, matching how the indexer resolves File nodes. + // The old code instead built `${indexedRoot}/packages/sub/foo.ts` + // (doubling the subdirectory), which never matches any File node. + expect(modifiedPaths).toContain(resolve(indexedRoot, 'foo.ts')); + expect(modifiedPaths).not.toContain(resolve(indexedRoot, 'packages', 'sub', 'foo.ts')); + }); + + it('skips files outside the indexed root instead of mislinking them', async () => { + await syncGitHistory(indexedRoot, fakeClient, { maxCommits: 10, includeStats: true }); + + const modifiedPaths = opsMocks.createModifiedInEdge.mock.calls.map((call) => call[0] as string); + + // README.md sits at the repo root, outside packages/sub. It must not be + // turned into any absolute path and passed to createModifiedInEdge. + expect(modifiedPaths.some((p) => p.includes('README.md'))).toBe(false); + }); +}); diff --git a/packages/core/src/__tests__/gitsync-root-commit.test.ts b/packages/core/src/__tests__/gitsync-root-commit.test.ts new file mode 100644 index 00000000..0291cbac --- /dev/null +++ b/packages/core/src/__tests__/gitsync-root-commit.test.ts @@ -0,0 +1,91 @@ +/** + * Regression test: syncGitHistory() silently produced zero MODIFIED_IN / + * INTRODUCED_IN edges for a repository's root (first, parent-less) commit. + * + * The per-commit diff used `git.diffSummary(['^', hash])` and + * `git.raw(['diff', '--name-status', '^', hash])`. A root commit has + * no parent, so `^` is not a valid revision and git exits 128. Both + * calls were wrapped in `.catch(() => null)` / `.catch(() => '')`, so the + * failure was swallowed: the Commit node was still created and + * commitsProcessed still incremented, but no file edges were written for + * that commit's files. + * + * This test drives a real temporary git repo whose very first commit adds + * files, and mocks only the graph ops layer, so it needs no live FalkorDB. + */ + +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import type { GraphClient } from '@codegraph/graph'; + +// --------------------------------------------------------------------------- +// Mocks. vi.mock is hoisted; mutable state the factory closes over must be +// created via vi.hoisted(). +// --------------------------------------------------------------------------- + +const opsMocks = vi.hoisted(() => ({ + upsertCommit: vi.fn().mockResolvedValue(undefined), + createModifiedInEdge: vi.fn().mockResolvedValue(undefined), + createIntroducedInEdgesForFile: vi.fn().mockResolvedValue(0), + createDeletedInEdgesForFile: vi.fn().mockResolvedValue(0), +})); + +vi.mock('@codegraph/graph', () => ({ + createOperations: vi.fn().mockReturnValue(opsMocks), +})); + +// Import after mocks are declared. +import { syncGitHistory } from '../gitSync'; + +function git(cwd: string, args: string[]): void { + execFileSync('git', args, { cwd, stdio: 'pipe' }); +} + +const fakeClient = { + graph: null, + graphName: 'test', + dialect: {}, + // getMetadata/setMetadata in gitSync.ts talk to the raw client directly. + query: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + roQuery: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + ensureIndexes: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), +} as unknown as GraphClient; + +let repoRoot: string; + +beforeAll(() => { + // realpathSync matters on macOS: os.tmpdir() returns a path under a + // symlink (/var/folders -> /private/var/folders). + repoRoot = realpathSync(mkdtempSync(join(tmpdir(), 'codegraph-gitsync-root-'))); + git(repoRoot, ['init', '-q']); + git(repoRoot, ['config', 'user.email', 'test@example.com']); + git(repoRoot, ['config', 'user.name', 'Test']); + + // The FIRST commit in this repo adds files directly (no prior commit to + // diff against), which is exactly the case that used to be swallowed. + writeFileSync(join(repoRoot, 'foo.ts'), 'export const x = 1;\n'); + git(repoRoot, ['add', '-A']); + git(repoRoot, ['commit', '-q', '-m', 'root commit']); +}); + +afterAll(() => { + rmSync(repoRoot, { recursive: true, force: true }); +}); + +describe('syncGitHistory: root commit diffing', () => { + it('produces MODIFIED_IN and INTRODUCED_IN edges for files added in the very first commit', async () => { + const result = await syncGitHistory(repoRoot, fakeClient, { maxCommits: 10, includeStats: true }); + + expect(result.commitsProcessed).toBe(1); + + const modifiedPaths = opsMocks.createModifiedInEdge.mock.calls.map((call) => call[0] as string); + expect(modifiedPaths).toContain(resolve(repoRoot, 'foo.ts')); + + const introducedPaths = opsMocks.createIntroducedInEdgesForFile.mock.calls.map((call) => call[0] as string); + expect(introducedPaths).toContain(resolve(repoRoot, 'foo.ts')); + }); +}); diff --git a/packages/core/src/__tests__/gitsync-symlink-namespace.test.ts b/packages/core/src/__tests__/gitsync-symlink-namespace.test.ts new file mode 100644 index 00000000..e27c4d53 --- /dev/null +++ b/packages/core/src/__tests__/gitsync-symlink-namespace.test.ts @@ -0,0 +1,113 @@ +/** + * Regression test: syncGitHistory() dropped every file on macOS because of a + * symlink mismatch between two "the same directory" paths. + * + * `git rev-parse --show-toplevel` returns a symlink-RESOLVED path. On macOS, + * os.tmpdir() lives under /var/folders, which is a symlink to + * /private/var/folders, so a repo created under os.tmpdir() reports its + * toplevel as /private/var/folders/... . The indexed root the caller passes + * in (indexer.ts never calls realpath) stays /var/folders/... . Joining + * git's repo-root-relative paths onto the resolved repoRoot and then taking + * `relative(indexedRoot, resolvedPath)` therefore starts with '..' for + * every file, even files genuinely inside the indexed root, so the + * boundary-skip check silently dropped all of them. + * + * File nodes are stored using whatever (possibly unresolved) root path the + * caller supplied to the indexer (see createFileEntityFromContent() / + * resolve(rootPath, f) in indexer.ts) - so the fix must resolve symlinks + * only to make the boundary comparison correct, and must map the edge's + * filePath back into the caller's ORIGINAL namespace so it still matches + * File.filePath byte-for-byte. + * + * Deliberately does NOT call realpathSync on the fixture paths (unlike + * gitsync-root-commit.test.ts), because that's what a real caller does. + */ + +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import type { GraphClient } from '@codegraph/graph'; + +// --------------------------------------------------------------------------- +// Mocks. vi.mock is hoisted; mutable state the factory closes over must be +// created via vi.hoisted(). +// --------------------------------------------------------------------------- + +const opsMocks = vi.hoisted(() => ({ + upsertCommit: vi.fn().mockResolvedValue(undefined), + createModifiedInEdge: vi.fn().mockResolvedValue(undefined), + createIntroducedInEdgesForFile: vi.fn().mockResolvedValue(0), + createDeletedInEdgesForFile: vi.fn().mockResolvedValue(0), +})); + +vi.mock('@codegraph/graph', () => ({ + createOperations: vi.fn().mockReturnValue(opsMocks), +})); + +// Import after mocks are declared. +import { syncGitHistory } from '../gitSync'; + +function git(cwd: string, args: string[]): void { + execFileSync('git', args, { cwd, stdio: 'pipe' }); +} + +const fakeClient = { + graph: null, + graphName: 'test', + dialect: {}, + // getMetadata/setMetadata in gitSync.ts talk to the raw client directly. + query: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + roQuery: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + ensureIndexes: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), +} as unknown as GraphClient; + +// Deliberately unresolved - mirrors what a real caller (indexer.ts) passes. +let repoRoot: string; +let indexedRoot: string; + +beforeAll(() => { + repoRoot = mkdtempSync(join(tmpdir(), 'codegraph-gitsync-symlink-')); + git(repoRoot, ['init', '-q']); + git(repoRoot, ['config', 'user.email', 'test@example.com']); + git(repoRoot, ['config', 'user.name', 'Test']); + + // First commit has no parent (root commit); keep it minimal. + writeFileSync(join(repoRoot, '.gitkeep'), ''); + git(repoRoot, ['add', '-A']); + git(repoRoot, ['commit', '-q', '-m', 'initial']); + + // Second, ordinary commit adds a file under a subdirectory that stands in + // for "the indexed root" (one package in a monorepo checkout). + indexedRoot = join(repoRoot, 'packages', 'sub'); + mkdirSync(indexedRoot, { recursive: true }); + writeFileSync(join(indexedRoot, 'foo.ts'), 'export const x = 1;\n'); + git(repoRoot, ['add', '-A']); + git(repoRoot, ['commit', '-q', '-m', 'add foo.ts']); +}); + +afterAll(() => { + rmSync(repoRoot, { recursive: true, force: true }); +}); + +describe('syncGitHistory: preserves the caller original path namespace', () => { + it('creates MODIFIED_IN edges even when the indexed root sits under a symlink, using a filePath that matches File.filePath', async () => { + const result = await syncGitHistory(indexedRoot, fakeClient, { maxCommits: 10, includeStats: true }); + + expect(result.commitsProcessed).toBe(2); + + // This is exactly the string indexer.ts's createFileEntityFromContent() + // would store on the File node: resolve(indexedRoot, 'foo.ts'), using + // the caller's ORIGINAL (possibly symlinked) indexedRoot, never a + // realpath-resolved one. + const expectedFilePath = resolve(indexedRoot, 'foo.ts'); + + const modifiedPaths = opsMocks.createModifiedInEdge.mock.calls.map((call) => call[0] as string); + expect(modifiedPaths).toContain(expectedFilePath); + + const introducedPaths = opsMocks.createIntroducedInEdgesForFile.mock.calls.map((call) => call[0] as string); + expect(introducedPaths).toContain(expectedFilePath); + }); +}); diff --git a/packages/core/src/__tests__/indexer-git-edges-preserved.integration.test.ts b/packages/core/src/__tests__/indexer-git-edges-preserved.integration.test.ts new file mode 100644 index 00000000..057f9458 --- /dev/null +++ b/packages/core/src/__tests__/indexer-git-edges-preserved.integration.test.ts @@ -0,0 +1,120 @@ +/** + * Regression test: an incremental reindex of a CHANGED file destroyed every + * git-history edge (MODIFIED_IN, INTRODUCED_IN, DELETED_IN, HAS_FILE) that + * had already been synced onto that file's File node. + * + * indexProject()'s incremental path called ops.removeFileAndCleanup(file) + * before re-upserting a changed file. Its REMOVE_FILE_NODE Cypher is + * `MATCH (f:File {filePath}) OPTIONAL MATCH (f)-[c:CONTAINS]->() DELETE c, f` + * -- deleting the File node cascades away every edge attached to it, not + * just CONTAINS. HAS_FILE gets recreated by the very next chunk-loop step, + * but syncGitHistory() only walks commits after its saved checkpoint + * (Metadata node `lastCommitSynced:`), so MODIFIED_IN edges from + * commits that were already synced before this reindex are gone forever. + * + * This test drives the real indexProject() against a real temporary git + * repo and a real (embedded, no server) FalkorDBLite graph, because the bug + * is about actual Cypher DELETE-vs-MERGE semantics -- a mocked ops layer + * cannot distinguish "File node destroyed and rebuilt" from "File node + * updated in place", only a real graph engine can. + * + * Sequence: index the repo at commit A (git sync captures commit A's + * MODIFIED_IN edge), edit the file, commit B, incremental reindex (git sync + * captures commit B's edge). Both edges must still be present afterward. + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { createClient, resolveEmbeddedBinaryPaths, type GraphClient } from '@codegraph/graph'; +import { indexProject } from '../indexer'; + +// The embedded driver ships binaries for darwin-arm64 and linux-x64 only. +const describeIfAvailable = resolveEmbeddedBinaryPaths() ? describe : describe.skip; + +function git(cwd: string, args: string[]): void { + execFileSync('git', args, { cwd, stdio: 'pipe' }); +} + +describeIfAvailable('indexProject: incremental reindex preserves prior git-history edges', () => { + let client: GraphClient; + let dataDir: string; + let repoRoot: string; + let filePath: string; + let previousEmbeddingProvider: string | undefined; + + beforeAll(async () => { + // ensureIndexes() needs to know the embedding dimension even when this + // test passes embeddings: false to indexProject() itself; 'none' skips + // vector indexes entirely so no provider/API key is needed here. + previousEmbeddingProvider = process.env['CODEGRAPH_EMBEDDING_PROVIDER']; + process.env['CODEGRAPH_EMBEDDING_PROVIDER'] = 'none'; + + dataDir = await mkdtemp(join(tmpdir(), 'cg-git-edges-')); + client = await createClient({ + driver: 'falkordblite', + databasePath: dataDir, + graphName: 'git_edges_regression', + } as never); + + repoRoot = mkdtempSync(join(tmpdir(), 'codegraph-git-edges-repo-')); + git(repoRoot, ['init', '-q']); + git(repoRoot, ['config', 'user.email', 'test@example.com']); + git(repoRoot, ['config', 'user.name', 'Test']); + + filePath = resolve(repoRoot, 'foo.ts'); + writeFileSync(filePath, 'export function foo(): number {\n return 1;\n}\n'); + git(repoRoot, ['add', '-A']); + git(repoRoot, ['commit', '-q', '-m', 'commit A']); + }, 60_000); + + afterAll(async () => { + await client?.close(); + if (dataDir) await rm(dataDir, { recursive: true, force: true }); + if (repoRoot) rmSync(repoRoot, { recursive: true, force: true }); + if (previousEmbeddingProvider === undefined) { + delete process.env['CODEGRAPH_EMBEDDING_PROVIDER']; + } else { + process.env['CODEGRAPH_EMBEDDING_PROVIDER'] = previousEmbeddingProvider; + } + }); + + it('keeps commit A and commit B MODIFIED_IN edges after an incremental reindex edits the file', async () => { + // First index: brand-new project, full/CREATE path. Git sync captures + // commit A's MODIFIED_IN edge. + const first = await indexProject(repoRoot, { + client, + includePatterns: ['*.ts'], + embeddings: false, + force: true, + }); + expect(first.success).toBe(true); + + // Edit the file and commit again. + writeFileSync(filePath, 'export function foo(): number {\n return 2;\n}\n'); + git(repoRoot, ['add', '-A']); + git(repoRoot, ['commit', '-q', '-m', 'commit B']); + + // Incremental reindex: foo.ts's hash changed, so it goes through the + // changed-file cleanup-and-reupsert path. Git sync only walks commit B + // (commit A is already past the saved checkpoint). + const second = await indexProject(repoRoot, { + client, + includePatterns: ['*.ts'], + embeddings: false, + force: false, + }); + expect(second.success).toBe(true); + + const result = await client.roQuery<{ hash: string }>( + `MATCH (f:File {filePath: $filePath})-[:MODIFIED_IN]->(c:Commit) RETURN c.hash AS hash`, + { params: { filePath } }, + ); + + const hashes = (result.data ?? []).map((row) => row.hash); + expect(hashes).toHaveLength(2); + }); +}); diff --git a/packages/core/src/__tests__/indexer-project-link-order.test.ts b/packages/core/src/__tests__/indexer-project-link-order.test.ts new file mode 100644 index 00000000..5ceaf95c --- /dev/null +++ b/packages/core/src/__tests__/indexer-project-link-order.test.ts @@ -0,0 +1,210 @@ +/** + * Regression tests for the HAS_FILE ordering bug in indexProject(). + * + * ops.linkProjectFiles() MATCHes the Project node by id (see + * BATCH_LINK_PROJECT_FILES in packages/graph/src/operations.ts: it uses + * OPTIONAL MATCH plus a WHERE filter, so a missing Project node just means + * the MERGE never runs, no error). indexProject() used to call + * ops.upsertProject() only once, at the very end of indexing, after the + * per-file chunk loop had already called ops.linkProjectFiles(). Two + * scenarios were broken: + * + * 1. Indexing a brand-new project: the Project node doesn't exist in the + * graph at all until the final upsertProject() call, so every + * linkProjectFiles() call during the run finds nothing to attach to. + * 2. A full reindex (force: true) of an existing project: deleteProject() + * DETACH DELETEs the Project node, and nothing recreated it before the + * chunk loop ran linkProjectFiles() again. + * + * These tests mock the pipeline (parsing) and the graph ops layer so they + * run without tree-sitter or a live FalkorDB, and assert only on call + * order, which is what the bug actually breaks. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { randomUUID } from 'node:crypto'; +import type { GraphClient } from '@codegraph/graph'; +import type { ExtractedEntities, FileEntity, ParsedFileEntities, ProjectEntity } from '@codegraph/types'; + +// --------------------------------------------------------------------------- +// Mocks. vi.mock is hoisted; factory functions must be self-contained, so +// mutable state the factories close over is created via vi.hoisted(). +// --------------------------------------------------------------------------- + +const opsMocks = vi.hoisted(() => { + const callOrder: string[] = []; + return { + callOrder, + getProjectByRoot: vi.fn(), + getProjectFileHashes: vi.fn().mockResolvedValue([]), + getEmbeddingHashesForFiles: vi.fn().mockResolvedValue(new Map()), + upsertProject: vi.fn().mockImplementation(async (project: ProjectEntity) => { + callOrder.push(`upsertProject:${project.id}`); + }), + deleteProject: vi.fn().mockImplementation(async (id: string) => { + callOrder.push(`deleteProject:${id}`); + }), + removeFileAndCleanup: vi.fn().mockResolvedValue(undefined), + removeFileContents: vi.fn().mockResolvedValue(undefined), + batchUpsertBulk: vi.fn().mockResolvedValue(undefined), + batchCreateBulk: vi.fn().mockResolvedValue(undefined), + linkProjectFiles: vi.fn().mockImplementation(async (projectId: string) => { + callOrder.push(`linkProjectFiles:${projectId}`); + }), + linkProjectFile: vi.fn().mockImplementation(async (projectId: string) => { + callOrder.push(`linkProjectFile:${projectId}`); + }), + batchUpsertDocuments: vi.fn().mockResolvedValue(undefined), + }; +}); + +vi.mock('@codegraph/graph', () => ({ + createOperations: vi.fn().mockReturnValue(opsMocks), +})); + +vi.mock('../pipeline', () => ({ + initParser: vi.fn().mockResolvedValue(undefined), + parseFile: vi.fn(), + parseCode: vi.fn().mockReturnValue({ rootNode: {}, sourceCode: '', language: 'typescript' }), + getLanguageForExtension: vi.fn().mockReturnValue('typescript'), + createFileEntityFromContent: vi.fn().mockImplementation((filePath: string): FileEntity => ({ + path: filePath, + name: filePath.split('/').pop() ?? filePath, + extension: 'ts', + loc: 1, + lastModified: new Date().toISOString(), + hash: 'fakehash', + })), + extractEntitiesForFile: vi.fn().mockReturnValue({ + imports: [], + functions: [], + classes: [], + interfaces: [], + variables: [], + types: [], + components: [], + } satisfies ExtractedEntities), + buildParsedFileEntities: vi.fn().mockImplementation((file: FileEntity): ParsedFileEntities => ({ + file, + functions: [], + classes: [], + interfaces: [], + variables: [], + types: [], + components: [], + imports: [], + callEdges: [], + importsEdges: [], + extendsEdges: [], + implementsEdges: [], + rendersEdges: [], + hasMethodEdges: [], + hasPropertyEdges: [], + typeRefs: [], + hasParamEdges: [], + returnsEdges: [], + usesTypeEdges: [], + })), + registerPlugins: vi.fn(), + registerTier2Languages: vi.fn().mockResolvedValue({ registered: [], skipped: [] }), + countEntities: vi.fn().mockReturnValue(0), + countEdges: vi.fn().mockReturnValue(0), + isMarkdownFile: vi.fn().mockReturnValue(false), + getSupportedExtensions: vi.fn().mockReturnValue(['.ts']), + DEFAULT_IGNORE_PATTERNS: [], +})); + +// Import after mocks are declared. +import { indexProject } from '../indexer'; + +const fakeClient = { + graph: null, + graphName: 'test', + dialect: {}, + query: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + roQuery: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + ensureIndexes: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), +} as unknown as GraphClient; + +let projectDir: string; + +beforeEach(() => { + projectDir = mkdtempSync(join(tmpdir(), 'codegraph-indexer-link-order-')); + writeFileSync(join(projectDir, 'foo.ts'), 'export const x = 1;\n'); + opsMocks.callOrder.length = 0; + opsMocks.getProjectByRoot.mockReset(); + opsMocks.upsertProject.mockClear(); + opsMocks.linkProjectFiles.mockClear(); + opsMocks.deleteProject.mockClear(); +}); + +afterEach(() => { + rmSync(projectDir, { recursive: true, force: true }); +}); + +describe('indexProject: Project node must exist before linkProjectFiles', () => { + it('upserts the Project node before linking files for a brand-new project', async () => { + opsMocks.getProjectByRoot.mockResolvedValue(null); + + const result = await indexProject(projectDir, { + client: fakeClient, + includePatterns: ['*.ts'], + embeddings: false, + gitSync: false, + force: false, + }); + + expect(result.success).toBe(true); + expect(opsMocks.linkProjectFiles).toHaveBeenCalledTimes(1); + + const upsertIdx = opsMocks.callOrder.findIndex((c) => c.startsWith('upsertProject:')); + const linkIdx = opsMocks.callOrder.findIndex((c) => c.startsWith('linkProjectFiles:')); + + expect(upsertIdx).toBeGreaterThanOrEqual(0); + expect(linkIdx).toBeGreaterThanOrEqual(0); + expect(upsertIdx).toBeLessThan(linkIdx); + + // The id used to link files must be the same id that was upserted. + const linkedProjectId = opsMocks.callOrder[linkIdx]!.split(':')[1]; + const upsertedProjectId = opsMocks.callOrder[upsertIdx]!.split(':')[1]; + expect(linkedProjectId).toBe(upsertedProjectId); + }); + + it('re-upserts the Project node after deleteProject() during a full reindex, before linking files', async () => { + const existingProject: ProjectEntity = { + id: randomUUID(), + name: 'fixture', + rootPath: projectDir, + createdAt: new Date().toISOString(), + lastParsed: new Date().toISOString(), + fileCount: 1, + }; + opsMocks.getProjectByRoot.mockResolvedValue(existingProject); + + const result = await indexProject(projectDir, { + client: fakeClient, + includePatterns: ['*.ts'], + embeddings: false, + gitSync: false, + force: true, + }); + + expect(result.success).toBe(true); + expect(opsMocks.deleteProject).toHaveBeenCalledWith(existingProject.id); + expect(opsMocks.linkProjectFiles).toHaveBeenCalledTimes(1); + + const deleteIdx = opsMocks.callOrder.findIndex((c) => c.startsWith('deleteProject:')); + const linkIdx = opsMocks.callOrder.findIndex((c) => c.startsWith('linkProjectFiles:')); + const upsertAfterDeleteIdx = opsMocks.callOrder.findIndex( + (c, i) => c.startsWith('upsertProject:') && i > deleteIdx, + ); + + expect(deleteIdx).toBeGreaterThanOrEqual(0); + expect(upsertAfterDeleteIdx).toBeGreaterThan(deleteIdx); + expect(upsertAfterDeleteIdx).toBeLessThan(linkIdx); + }); +}); diff --git a/packages/core/src/__tests__/indexer-single-file-preserves-file-node.test.ts b/packages/core/src/__tests__/indexer-single-file-preserves-file-node.test.ts new file mode 100644 index 00000000..045105b6 --- /dev/null +++ b/packages/core/src/__tests__/indexer-single-file-preserves-file-node.test.ts @@ -0,0 +1,132 @@ +/** + * Regression test: indexSingleFile() has the same File-node-destroying bug + * as indexProject()'s incremental batch path, for the same reason. + * + * indexSingleFile() is the code path the file watcher uses for its + * `onFileChanged` event, which fires for edits to files that already exist + * in the graph (not just brand-new files: see onFileRemoved in + * mcp-server/src/index.ts and configureProjects.ts, which is the separate, + * true-deletion path and still calls removeFileAndCleanup() directly). + * indexSingleFile() used to call ops.removeFileAndCleanup(filePath) before + * re-upserting, which would destroy that file's MODIFIED_IN/HAS_FILE edges + * exactly like the batch path did. It must use ops.removeFileContents() + * instead, so the File node is refreshed in place rather than rebuilt. + * + * This test mocks the pipeline and the graph ops layer (the actual Cypher + * semantics of removeFileContents() are proven separately, against a real + * graph, in packages/graph/src/__tests__/remove-file-contents.test.ts and + * packages/core/src/__tests__/indexer-git-edges-preserved.integration.test.ts) + * -- this one only checks indexSingleFile() calls the right operation. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { GraphClient } from '@codegraph/graph'; +import type { ExtractedEntities, FileEntity, ParsedFileEntities } from '@codegraph/types'; + +// --------------------------------------------------------------------------- +// Mocks. vi.mock is hoisted; mutable state the factory closes over must be +// created via vi.hoisted(). +// --------------------------------------------------------------------------- + +const opsMocks = vi.hoisted(() => ({ + removeFileAndCleanup: vi.fn().mockResolvedValue(undefined), + removeFileContents: vi.fn().mockResolvedValue(undefined), + batchUpsert: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('@codegraph/graph', () => ({ + createOperations: vi.fn().mockReturnValue(opsMocks), +})); + +vi.mock('../pipeline', () => ({ + initParser: vi.fn().mockResolvedValue(undefined), + parseFile: vi.fn().mockResolvedValue({ rootNode: {}, sourceCode: '', language: 'typescript' }), + parseCode: vi.fn().mockReturnValue({ rootNode: {}, sourceCode: '', language: 'typescript' }), + getLanguageForExtension: vi.fn().mockReturnValue('typescript'), + createFileEntityFromContent: vi.fn().mockImplementation((filePath: string): FileEntity => ({ + path: filePath, + name: filePath.split('/').pop() ?? filePath, + extension: 'ts', + loc: 1, + lastModified: new Date().toISOString(), + hash: 'fakehash', + })), + extractEntitiesForFile: vi.fn().mockReturnValue({ + imports: [], + functions: [], + classes: [], + interfaces: [], + variables: [], + types: [], + components: [], + } satisfies ExtractedEntities), + buildParsedFileEntities: vi.fn().mockImplementation((file: FileEntity): ParsedFileEntities => ({ + file, + functions: [], + classes: [], + interfaces: [], + variables: [], + types: [], + components: [], + imports: [], + callEdges: [], + importsEdges: [], + extendsEdges: [], + implementsEdges: [], + rendersEdges: [], + hasMethodEdges: [], + hasPropertyEdges: [], + typeRefs: [], + hasParamEdges: [], + returnsEdges: [], + usesTypeEdges: [], + })), + registerPlugins: vi.fn(), + registerTier2Languages: vi.fn().mockResolvedValue({ registered: [], skipped: [] }), + countEntities: vi.fn().mockReturnValue(0), + countEdges: vi.fn().mockReturnValue(0), + isMarkdownFile: vi.fn().mockReturnValue(false), + getSupportedExtensions: vi.fn().mockReturnValue(['.ts']), + DEFAULT_IGNORE_PATTERNS: [], +})); + +// Import after mocks are declared. +import { indexSingleFile } from '../indexer'; + +const fakeClient = { + graph: null, + graphName: 'test', + dialect: {}, + query: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + roQuery: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + ensureIndexes: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), +} as unknown as GraphClient; + +let projectDir: string; +let filePath: string; + +beforeEach(() => { + projectDir = mkdtempSync(join(tmpdir(), 'codegraph-single-file-preserve-')); + filePath = join(projectDir, 'foo.ts'); + writeFileSync(filePath, 'export const x = 1;\n'); + opsMocks.removeFileAndCleanup.mockClear(); + opsMocks.removeFileContents.mockClear(); +}); + +afterEach(() => { + rmSync(projectDir, { recursive: true, force: true }); +}); + +describe('indexSingleFile: reindexing a changed file must not destroy the File node', () => { + it('calls removeFileContents (not removeFileAndCleanup) before re-upserting', async () => { + const result = await indexSingleFile(filePath, projectDir, fakeClient, false); + + expect(result.success).toBe(true); + expect(opsMocks.removeFileContents).toHaveBeenCalledWith(filePath); + expect(opsMocks.removeFileAndCleanup).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/__tests__/indexer-tier2-languages.test.ts b/packages/core/src/__tests__/indexer-tier2-languages.test.ts new file mode 100644 index 00000000..0dbee65d --- /dev/null +++ b/packages/core/src/__tests__/indexer-tier2-languages.test.ts @@ -0,0 +1,172 @@ +/** + * Regression test: registerTier2Languages() had no call site. + * + * packages/core/src/pipeline/pipeline.ts exports registerTier2Languages(), + * which registers the 29 tier-2 tree-sitter languages (Ruby, Kotlin, Swift, + * C, C++, ...) with the language registry. Nothing in the indexing flow + * called it, so getSupportedExtensions() (used to build file-discovery glob + * patterns) never included tier-2 extensions, and those files were never + * discovered, let alone parsed. + * + * These tests mock the pipeline module and the graph ops layer so they run + * without tree-sitter grammars or a live FalkorDB, and assert only that + * indexProject()/indexSingleFile() actually call registerTier2Languages(), + * before file discovery happens. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { GraphClient } from '@codegraph/graph'; +import type { ExtractedEntities, FileEntity, ParsedFileEntities } from '@codegraph/types'; + +// --------------------------------------------------------------------------- +// Mocks. vi.mock is hoisted; mutable state the factory closes over must be +// created via vi.hoisted(). +// --------------------------------------------------------------------------- + +const pipelineMocks = vi.hoisted(() => { + const callOrder: string[] = []; + return { + callOrder, + registerTier2Languages: vi.fn().mockImplementation(async () => { + callOrder.push('registerTier2Languages'); + return { registered: [], skipped: [] }; + }), + getSupportedExtensions: vi.fn().mockImplementation(() => { + callOrder.push('getSupportedExtensions'); + return ['.ts']; + }), + }; +}); + +const opsMocks = vi.hoisted(() => ({ + getProjectByRoot: vi.fn().mockResolvedValue(null), + getProjectFileHashes: vi.fn().mockResolvedValue([]), + getEmbeddingHashesForFiles: vi.fn().mockResolvedValue(new Map()), + upsertProject: vi.fn().mockResolvedValue(undefined), + deleteProject: vi.fn().mockResolvedValue(undefined), + removeFileAndCleanup: vi.fn().mockResolvedValue(undefined), + removeFileContents: vi.fn().mockResolvedValue(undefined), + batchUpsertBulk: vi.fn().mockResolvedValue(undefined), + batchCreateBulk: vi.fn().mockResolvedValue(undefined), + linkProjectFiles: vi.fn().mockResolvedValue(undefined), + linkProjectFile: vi.fn().mockResolvedValue(undefined), + batchUpsert: vi.fn().mockResolvedValue(undefined), + batchUpsertDocuments: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('@codegraph/graph', () => ({ + createOperations: vi.fn().mockReturnValue(opsMocks), +})); + +vi.mock('../pipeline', () => ({ + initParser: vi.fn().mockResolvedValue(undefined), + parseFile: vi.fn().mockResolvedValue({ rootNode: {}, sourceCode: '', language: 'typescript' }), + parseCode: vi.fn().mockReturnValue({ rootNode: {}, sourceCode: '', language: 'typescript' }), + getLanguageForExtension: vi.fn().mockReturnValue('typescript'), + createFileEntityFromContent: vi.fn().mockImplementation((filePath: string): FileEntity => ({ + path: filePath, + name: filePath.split('/').pop() ?? filePath, + extension: 'ts', + loc: 1, + lastModified: new Date().toISOString(), + hash: 'fakehash', + })), + extractEntitiesForFile: vi.fn().mockReturnValue({ + imports: [], + functions: [], + classes: [], + interfaces: [], + variables: [], + types: [], + components: [], + } satisfies ExtractedEntities), + buildParsedFileEntities: vi.fn().mockImplementation((file: FileEntity): ParsedFileEntities => ({ + file, + functions: [], + classes: [], + interfaces: [], + variables: [], + types: [], + components: [], + imports: [], + callEdges: [], + importsEdges: [], + extendsEdges: [], + implementsEdges: [], + rendersEdges: [], + hasMethodEdges: [], + hasPropertyEdges: [], + typeRefs: [], + hasParamEdges: [], + returnsEdges: [], + usesTypeEdges: [], + })), + registerPlugins: vi.fn(), + registerTier2Languages: pipelineMocks.registerTier2Languages, + countEntities: vi.fn().mockReturnValue(0), + countEdges: vi.fn().mockReturnValue(0), + isMarkdownFile: vi.fn().mockReturnValue(false), + getSupportedExtensions: pipelineMocks.getSupportedExtensions, + DEFAULT_IGNORE_PATTERNS: [], +})); + +// Import after mocks are declared. +import { indexProject, indexSingleFile } from '../indexer'; + +const fakeClient = { + graph: null, + graphName: 'test', + dialect: {}, + query: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + roQuery: vi.fn().mockResolvedValue({ data: [], metadata: [] }), + ensureIndexes: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), +} as unknown as GraphClient; + +let projectDir: string; +let filePath: string; + +beforeEach(() => { + projectDir = mkdtempSync(join(tmpdir(), 'codegraph-indexer-tier2-')); + filePath = join(projectDir, 'foo.ts'); + writeFileSync(filePath, 'export const x = 1;\n'); + pipelineMocks.callOrder.length = 0; + pipelineMocks.registerTier2Languages.mockClear(); + pipelineMocks.getSupportedExtensions.mockClear(); +}); + +afterEach(() => { + rmSync(projectDir, { recursive: true, force: true }); +}); + +describe('indexProject calls registerTier2Languages before file discovery', () => { + it('invokes registerTier2Languages, before getSupportedExtensions is used to build discovery patterns', async () => { + const result = await indexProject(projectDir, { + client: fakeClient, + embeddings: false, + gitSync: false, + }); + + expect(result.success).toBe(true); + expect(pipelineMocks.registerTier2Languages).toHaveBeenCalledTimes(1); + + const tier2Idx = pipelineMocks.callOrder.indexOf('registerTier2Languages'); + const firstDiscoveryIdx = pipelineMocks.callOrder.indexOf('getSupportedExtensions'); + + expect(tier2Idx).toBeGreaterThanOrEqual(0); + expect(firstDiscoveryIdx).toBeGreaterThanOrEqual(0); + expect(tier2Idx).toBeLessThan(firstDiscoveryIdx); + }); +}); + +describe('indexSingleFile calls registerTier2Languages', () => { + it('invokes registerTier2Languages before parsing the file', async () => { + const result = await indexSingleFile(filePath, projectDir, fakeClient, false); + + expect(result.success).toBe(true); + expect(pipelineMocks.registerTier2Languages).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/core/src/__tests__/services-helpers.test.ts b/packages/core/src/__tests__/services-helpers.test.ts new file mode 100644 index 00000000..684e6d20 --- /dev/null +++ b/packages/core/src/__tests__/services-helpers.test.ts @@ -0,0 +1,166 @@ +import { describe, it, expect } from 'vitest'; +import { getLabelFromLabels, generateNodeId, ALL_LABELS } from '../services/helpers'; +import { SYMBOL_LABELS, NODE_LABELS } from '@codegraph/types'; + +/** + * getLabelFromLabels used to classify against a hand-copied 8-label allowlist + * (the 7 embeddable code-symbol labels plus 'Import') instead of the full set + * of real node labels. A Commit, MarkdownDocument, Section, CodeBlock or Link + * node fell through the "not found" branch and defaulted to 'File', which + * generateNodeId then turns into a nonsensical id built from a File-shaped + * key on a node that has no filePath at all. + * + * This path is reachable only through CodeGraphService.getNodesPaginated + * when a caller explicitly requests one of those types (e.g. + * `types: ['Commit']`); it is exported but currently uncalled in production. + * The fix widens classification to the full canonical NODE_LABELS set. + */ +describe('getLabelFromLabels', () => { + it('classifies a Commit node as Commit, not File', () => { + expect(getLabelFromLabels(['Commit'])).toBe('Commit'); + }); + + it('classifies MarkdownDocument, Section, CodeBlock and Link correctly', () => { + expect(getLabelFromLabels(['MarkdownDocument'])).toBe('MarkdownDocument'); + expect(getLabelFromLabels(['Section'])).toBe('Section'); + expect(getLabelFromLabels(['CodeBlock'])).toBe('CodeBlock'); + expect(getLabelFromLabels(['Link'])).toBe('Link'); + }); + + it('still classifies every one of the original 8 recognized labels correctly', () => { + for (const label of [...SYMBOL_LABELS, 'Import'] as const) { + expect(getLabelFromLabels([label])).toBe(label); + } + }); + + it('classifies every NODE_LABELS member', () => { + for (const label of NODE_LABELS) { + expect(getLabelFromLabels([label])).toBe(label); + } + }); + + it('still falls back to File for a label list with no recognized NodeLabel value', () => { + expect(getLabelFromLabels(['External'])).toBe('File'); + expect(getLabelFromLabels([])).toBe('File'); + }); +}); + +/** + * generateNodeId used to build every non-File id from the same + * name/filePath/startLine-or-line scheme, regardless of label. That scheme + * is the real identity for the seven symbol labels (see + * packages/graph/src/schema.ts's MERGE keys and generateNodeId), but Commit, + * MarkdownDocument, Section, CodeBlock, Link and Entity nodes carry none of + * those properties, so every node of a given one of those labels collapsed + * onto the exact same id (e.g. every Commit became "Commit::0"). + * + * Real identity per label, confirmed against packages/graph/src/schema.ts + * (CommitNodeProps, MarkdownDocumentNodeProps, SectionNodeProps, + * CodeBlockNodeProps, LinkNodeProps) and the MERGE keys in + * packages/graph/src/operations.ts (BATCH_UPSERT_* / commit upsert queries) + * and packages/graph/src/knowledge-operations.ts (Entity upsert): + * - Commit: MERGE (c:Commit {hash}) -> hash + * - MarkdownDocument: MERGE (d:MarkdownDocument {path}) -> path + * - Section: MERGE (s:Section {filePath, startLine}) -> filePath + startLine + * - CodeBlock: MERGE (cb:CodeBlock {filePath, startLine}) -> filePath + startLine + * - Link: MERGE (l:Link {filePath, line, target}) -> filePath + line + target + * - Entity: MERGE (n:Entity {text, type}) -> text + type + * The seven symbol labels (File, Function, Class, Interface, Variable, + * Type, Component) keep the existing name/filePath/startLine-or-line scheme, + * which already matches their own MERGE keys. + */ +describe('generateNodeId: Commit uses hash, not the name/filePath/line scheme', () => { + // Real CommitNodeProps shape (packages/graph/src/schema.ts): hash, message, + // author, email, date. Deliberately no `name` or `filePath` field, since a + // real Commit node never carries either. + const realCommitProps = (hash: string) => ({ + hash, + message: 'fix: something', + author: 'Randy Wilson', + email: 'randy@example.com', + date: '2026-08-20T00:00:00Z', + }); + + it('builds an id containing the commit hash, not a File-shaped id', () => { + const label = getLabelFromLabels(['Commit']); + const id = generateNodeId(label, realCommitProps('abc123')); + expect(id).not.toMatch(/^File:/); + expect(id).toContain('abc123'); + }); + + it('gives two different Commit nodes two different ids', () => { + const idA = generateNodeId('Commit', realCommitProps('aaa111')); + const idB = generateNodeId('Commit', realCommitProps('bbb222')); + expect(idA).not.toBe(idB); + }); +}); + +describe('generateNodeId: MarkdownDocument, Section, CodeBlock, Link use their real MERGE keys', () => { + it('keys MarkdownDocument by path', () => { + // Real MarkdownDocumentNodeProps shape: path, name, title, frontmatter, hash, lastModified. + const idA = generateNodeId('MarkdownDocument', { path: '/docs/a.md', name: 'a.md', title: null, frontmatter: null, hash: 'h1', lastModified: '2026-01-01' }); + const idB = generateNodeId('MarkdownDocument', { path: '/docs/b.md', name: 'a.md', title: null, frontmatter: null, hash: 'h1', lastModified: '2026-01-01' }); + expect(idA).not.toBe(idB); + expect(idA).toContain('/docs/a.md'); + }); + + it('keys Section by filePath + startLine, not name (Sections have no name)', () => { + // Real SectionNodeProps shape: heading, level, filePath, startLine, endLine. + const idA = generateNodeId('Section', { heading: 'Intro', level: 1, filePath: '/docs/a.md', startLine: 1, endLine: 5 }); + const idB = generateNodeId('Section', { heading: 'Intro', level: 1, filePath: '/docs/a.md', startLine: 20, endLine: 25 }); + expect(idA).not.toBe(idB); + }); + + it('keys CodeBlock by filePath + startLine, not name (CodeBlocks have no name)', () => { + // Real CodeBlockNodeProps shape: language, content, filePath, startLine, endLine. + const idA = generateNodeId('CodeBlock', { language: 'ts', content: 'const a = 1;', filePath: '/docs/a.md', startLine: 3, endLine: 5 }); + const idB = generateNodeId('CodeBlock', { language: 'ts', content: 'const a = 1;', filePath: '/docs/a.md', startLine: 10, endLine: 12 }); + expect(idA).not.toBe(idB); + }); + + it('keys Link by filePath + line + target, not name (Links have no name)', () => { + // Real LinkNodeProps shape: text, target, isInternal, filePath, line, anchor. + const idA = generateNodeId('Link', { text: 'here', target: '/other.md', isInternal: true, filePath: '/docs/a.md', line: 4, anchor: null }); + const idB = generateNodeId('Link', { text: 'here', target: '/another.md', isInternal: true, filePath: '/docs/a.md', line: 4, anchor: null }); + expect(idA).not.toBe(idB); + }); +}); + +describe('generateNodeId: Entity uses text + type (its real MERGE key), reachable via getNeighborsImpl', () => { + it('gives two different Entity nodes two different ids', () => { + const idA = generateNodeId('Entity', { text: 'Acme Corp', type: 'Organization' }); + const idB = generateNodeId('Entity', { text: 'Beta Corp', type: 'Organization' }); + expect(idA).not.toBe(idB); + }); + + it('distinguishes same-text Entities of different types', () => { + const idA = generateNodeId('Entity', { text: 'Acme', type: 'Organization' }); + const idB = generateNodeId('Entity', { text: 'Acme', type: 'Product' }); + expect(idA).not.toBe(idB); + }); +}); + +describe('generateNodeId: symbol labels keep the existing name/filePath/startLine-or-line scheme', () => { + it('still builds ids from name, filePath and startLine for Function/Class/etc.', () => { + const id = generateNodeId('Function', { name: 'doThing', filePath: '/src/a.ts', startLine: 10 }); + expect(id).toBe('Function:/src/a.ts:doThing:10'); + }); + + it('still builds the single-key File id', () => { + expect(generateNodeId('File', { filePath: '/src/a.ts' })).toBe('File:/src/a.ts'); + }); +}); + +describe('generateNodeId: a label with no established identity contract falls back to an explicit unknown marker', () => { + it('marks Import (never materialized as a real node; see schema.ts, no MERGE for :Import) as unknown rather than guessing', () => { + const id = generateNodeId('Import', { source: '../foo' }); + expect(id).toContain('unknown'); + expect(id).not.toMatch(/^Import::/); + }); +}); + +describe('ALL_LABELS', () => { + it('matches SYMBOL_LABELS exactly', () => { + expect([...ALL_LABELS].sort()).toEqual([...SYMBOL_LABELS].sort()); + }); +}); diff --git a/packages/core/src/embed-nodes.ts b/packages/core/src/embed-nodes.ts index 0de5e957..41456631 100644 --- a/packages/core/src/embed-nodes.ts +++ b/packages/core/src/embed-nodes.ts @@ -9,6 +9,7 @@ import { createLogger } from '@codegraph/logger'; import { createOperations, type GraphClient } from '@codegraph/graph'; +import { SYMBOL_LABELS } from '@codegraph/types'; import type { FunctionParam } from '@codegraph/types'; import { buildFunctionEmbeddingText, @@ -307,9 +308,13 @@ const textBuilders: Record) // Public API // ============================================================================ -const ALL_NODE_TYPES: EmbeddableNodeType[] = [ - 'File', 'Function', 'Class', 'Interface', 'Variable', 'Type', 'Component', -]; +// SYMBOL_LABELS is the shared source of truth (packages/types/src/labels.ts) +// and matches EmbeddableNodeType exactly (7 code-symbol labels). Not +// EMBEDDABLE_LABELS (which adds 'Entity'): knowledge-graph Entity nodes are +// embedded through a separate path, not this file, so there is no +// query/mapper/builder for them anywhere below, and adding 'Entity' here +// would break queryNodesForType's lookup. +const ALL_NODE_TYPES: EmbeddableNodeType[] = [...SYMBOL_LABELS]; /** * Generate and store embeddings for all graph nodes that lack them. diff --git a/packages/core/src/gitSync.ts b/packages/core/src/gitSync.ts index e2c64e06..d966dc5d 100644 --- a/packages/core/src/gitSync.ts +++ b/packages/core/src/gitSync.ts @@ -9,6 +9,8 @@ import simpleGit, { type SimpleGit, type LogResult, type DefaultLogFields } from import { createOperations, type GraphClient } from '@codegraph/graph'; import type { CommitEntity } from '@codegraph/types'; import { createLogger } from '@codegraph/logger'; +import { relative, resolve, join } from 'node:path'; +import { realpath } from 'node:fs/promises'; const logger = createLogger({ namespace: 'core:gitSync' }); @@ -71,6 +73,34 @@ async function setMetadata(client: GraphClient, key: string, value: string): Pro // Git Sync Implementation // ============================================================================ +/** + * Git's well-known empty tree object hash (SHA-1 of an empty tree). Used as + * the "before" state when diffing a repository's root commit, which has no + * parent and so cannot be diffed with the usual `^` syntax. + */ +const EMPTY_TREE_HASH = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'; + +/** + * Resolve the revision to diff a commit against: its parent (`^`) for + * a normal commit, or the empty tree for a root commit. A root commit has + * no parent, so `^` is not a valid revision and `git diff` exits 128 - + * that failure used to be silently swallowed by a blanket .catch(), which + * meant a repo's first commit never got MODIFIED_IN/INTRODUCED_IN edges for + * its files. + */ +async function resolveDiffBase(git: SimpleGit, commitHash: string): Promise { + try { + const revList = await git.raw(['rev-list', '--parents', '-n', '1', commitHash]); + // Output is " [ ...]" - a root commit has no parent, so + // there is exactly one token. + const tokens = revList.trim().split(/\s+/).filter(Boolean); + return tokens.length > 1 ? `${commitHash}^` : EMPTY_TREE_HASH; + } catch (err) { + logger.warn(`Could not determine parent commit for ${commitHash}, assuming it has one: ${err instanceof Error ? err.message : String(err)}`); + return `${commitHash}^`; + } +} + /** * Sync git history for a repository into the graph. * Creates Commit nodes and MODIFIED_IN edges from Files to Commits. @@ -144,6 +174,35 @@ export async function syncGitHistory( const ops = createOperations(client); + // git reports file paths (via `git diff` / `--name-status`) relative to + // the REPOSITORY root, not relative to repoPath, which may be a + // subdirectory of the repo (e.g. one package in a monorepo checkout). + // Resolve the real repo root once so every commit's paths can be turned + // into the same absolute path the indexer used when it created File + // nodes, instead of naively joining repoPath with git's relative path. + const repoRoot = (await git.revparse(['--show-toplevel'])).trim(); + const indexedRoot = resolve(repoPath); + + // `git rev-parse --show-toplevel` resolves symlinks. On macOS, + // os.tmpdir() lives under /var/folders, itself a symlink to + // /private/var/folders, so a repo created under it reports repoRoot as + // /private/var/folders/... while indexedRoot (built from whatever the + // caller passed in, unresolved) stays /var/folders/... . Comparing those + // two directly makes every file look "outside" the indexed root, since + // relative() sees two different-looking paths even though they name the + // same directory. Resolve indexedRoot's real path once, purely for that + // boundary comparison - never for the filePath written onto edges, + // which must stay in the caller's original (possibly symlinked) + // namespace to match File.filePath (see indexer.ts, which never calls + // realpath either). + let realIndexedRoot: string; + try { + realIndexedRoot = await realpath(indexedRoot); + } catch (err) { + logger.warn(`Could not resolve real path for indexed root ${indexedRoot}, using it as-is: ${err instanceof Error ? err.message : String(err)}`); + realIndexedRoot = indexedRoot; + } + // Determine starting point for incremental sync let fromCommit = sinceCommit; if (!fromCommit) { @@ -196,15 +255,25 @@ export async function syncGitHistory( await ops.upsertCommit(commitEntity); commitsProcessed++; - // Get files changed in this commit with status (A=added, M=modified, D=deleted) + // Get files changed in this commit with status (A=added, M=modified, D=deleted). + // diffBase is the commit's parent, or the empty tree for a root commit + // (see resolveDiffBase) - a root commit has no parent to diff against. + const diffBase = await resolveDiffBase(git, commit.hash); + const diffSummary = await git - .diffSummary([`${commit.hash}^`, commit.hash]) - .catch(() => null); + .diffSummary([diffBase, commit.hash]) + .catch((err: unknown) => { + logger.warn(`Could not compute diff stats for commit ${commit.hash}: ${err instanceof Error ? err.message : String(err)}`); + return null; + }); // Get name-status for INTRODUCED_IN / DELETED_IN detection const nameStatus = await git - .raw(['diff', '--name-status', `${commit.hash}^`, commit.hash]) - .catch(() => ''); + .raw(['diff', '--name-status', diffBase, commit.hash]) + .catch((err: unknown) => { + logger.warn(`Could not compute name-status for commit ${commit.hash}: ${err instanceof Error ? err.message : String(err)}`); + return ''; + }); // Parse name-status into a map: filePath → status const statusMap = new Map(); @@ -217,7 +286,29 @@ export async function syncGitHistory( if (diffSummary) { for (const file of diffSummary.files) { - const absolutePath = `${repoPath}/${file.file}`; + // file.file is relative to repoRoot (git's convention), not to + // repoPath. Resolve it against repoRoot to get the file's real + // (symlink-resolved) absolute path, since repoRoot itself came + // from `git rev-parse --show-toplevel`, which resolves symlinks. + const resolvedAbsolutePath = resolve(repoRoot, file.file); + + // Boundary check against the equally-resolved indexed root, so + // a symlink difference between the two (e.g. macOS os.tmpdir()) + // can't make every file look like it's outside the project. + const relativeToIndexedRoot = relative(realIndexedRoot, resolvedAbsolutePath); + if (relativeToIndexedRoot.startsWith('..')) { + // Genuinely outside the indexed root: belongs to a different + // part of the repo (e.g. a sibling package) and can't + // correspond to a File node here. + continue; + } + + // Map back into the caller's ORIGINAL (possibly unresolved) + // path namespace for the edge itself, so it byte-matches + // File.filePath, which indexer.ts builds from the unresolved + // root it was given (see createFileEntityFromContent()). + const filePath = join(indexedRoot, relativeToIndexedRoot); + const linesAdded = includeStats ? (file as { insertions?: number }).insertions : undefined; @@ -226,7 +317,7 @@ export async function syncGitHistory( : undefined; try { - await ops.createModifiedInEdge(absolutePath, commit.hash, linesAdded, linesRemoved); + await ops.createModifiedInEdge(filePath, commit.hash, linesAdded, linesRemoved); edgesCreated++; } catch { // File may not be in the graph (not indexed) — expected @@ -236,7 +327,7 @@ export async function syncGitHistory( const status = statusMap.get(file.file); if (status === 'A') { try { - await ops.createIntroducedInEdgesForFile(absolutePath, commit.hash); + await ops.createIntroducedInEdgesForFile(filePath, commit.hash); } catch { // Entities may not exist yet } @@ -245,7 +336,7 @@ export async function syncGitHistory( // DELETED_IN: file was deleted in this commit if (status === 'D') { try { - await ops.createDeletedInEdgesForFile(absolutePath, commit.hash); + await ops.createDeletedInEdgesForFile(filePath, commit.hash); } catch { // Entities may already be gone } diff --git a/packages/core/src/indexer.ts b/packages/core/src/indexer.ts index 486cec60..4b648079 100644 --- a/packages/core/src/indexer.ts +++ b/packages/core/src/indexer.ts @@ -20,6 +20,7 @@ import { extractEntitiesForFile, buildParsedFileEntities, registerPlugins, + registerTier2Languages, countEntities, countEdges, isMarkdownFile, @@ -227,9 +228,14 @@ export async function indexProject( }; } - // Register language plugins + initialize parser + // Register language plugins + initialize parser. Tier-2 languages (Ruby, + // Kotlin, Swift, C, C++, ...) must be registered before file discovery + // below: getSupportedExtensions() only returns extensions for languages + // already in the registry, so skipping this call means tier-2 source + // files are never even discovered, let alone parsed. registerPlugins(); await initParser(); + await registerTier2Languages(); // Discover source files (git ls-files when available, glob fallback) const gitignorePatterns = await loadGitignorePatterns(rootPath); @@ -289,6 +295,13 @@ export async function indexProject( lastParsed: now, fileCount: 0, }; + // Persist the Project node now, before any file processing. + // linkProjectFiles() MATCHes the Project by id, so if the node doesn't + // exist yet (brand-new project, or right after deleteProject() clears it + // below) the MATCH silently finds nothing and no HAS_FILE edges get + // created. The final upsertProject() call further down still runs, to + // persist fileCount/lastParsed once those are known. + await ops.upsertProject(project); // ---------------------------------------------------------------- // Incremental: build hash map of previously indexed files @@ -401,9 +414,12 @@ export async function indexProject( logger.info('Full reindex: clearing existing project data for fast CREATE path'); await ops.deleteProject(existingProject.id); } - // Re-create the project node after clear + // Re-create the project node after clear. deleteProject() above DETACH + // DELETEs it, so it must exist again before the chunk loop below calls + // linkProjectFiles(), or every HAS_FILE edge silently fails to attach. if (useCreatePath) { project.createdAt = now; + await ops.upsertProject(project); } // Pipelined parse + upsert for code files @@ -417,10 +433,15 @@ export async function indexProject( await ops.batchCreateBulk(chunk.map(r => r.built)); } else { // Incremental: clean up old entities before re-upserting. - // removeFileAndCleanup removes the File node + CONTAINS edges, then - // deletes orphaned entities that have no incoming cross-file edges. - // This prevents stale nodes when functions move lines or get deleted. - await Promise.all(chunk.map(r => ops.removeFileAndCleanup(r.file))); + // removeFileContents() detaches CONTAINS edges and deletes orphaned + // entities that have no incoming cross-file edges, preventing stale + // nodes when functions move lines or get deleted. Unlike + // removeFileAndCleanup(), it leaves the File node itself (and its + // MODIFIED_IN / HAS_FILE / EXPORTS edges) untouched, since this + // file's content changed, it was not deleted from disk. The + // upsertBulk call below then updates that same File node in place + // (MERGE), instead of a fresh node replacing a deleted one. + await Promise.all(chunk.map(r => ops.removeFileContents(r.file))); await ops.batchUpsertBulk(chunk.map(r => r.built)); } await ops.linkProjectFiles(project.id, chunk.map(r => r.file)); @@ -692,9 +713,11 @@ export async function indexSingleFile( return { success: true, entities: entityCount, edges: edgeCount }; } - // Register language plugins + initialize parser + // Register language plugins + initialize parser (tier-2 too, so a + // single-file re-index of e.g. a .rb or .kt file resolves correctly) registerPlugins(); await initParser(); + await registerTier2Languages(); // Read file once, reuse for parsing and entity creation const [fileStat, content] = await Promise.all([ @@ -720,8 +743,14 @@ export async function indexSingleFile( // Skip non-exported variables parsed.variables = parsed.variables.filter(v => v.isExported); - // Clean up old entities before re-upserting (prevents stale nodes when code moves/deletes) - await ops.removeFileAndCleanup(filePath); + // Clean up old entities before re-upserting (prevents stale nodes when + // code moves/deletes). This path re-indexes a file whose content + // changed (see onFileChanged in the watcher integrations) -- it was not + // deleted from disk, so use removeFileContents() rather than + // removeFileAndCleanup(), which would destroy this File node's + // MODIFIED_IN / HAS_FILE / EXPORTS edges along with it. True deletions + // go through removeFileAndCleanup() directly, from onFileRemoved. + await ops.removeFileContents(filePath); await ops.batchUpsert(parsed); // Embedding pass — deferred (background) or blocking diff --git a/packages/core/src/pipeline/pipeline.ts b/packages/core/src/pipeline/pipeline.ts index b049cc0c..dd6b0d5b 100644 --- a/packages/core/src/pipeline/pipeline.ts +++ b/packages/core/src/pipeline/pipeline.ts @@ -24,6 +24,9 @@ import { languageRegistry } from './registry'; import { stat, readFile } from 'node:fs/promises'; import { basename, extname } from 'node:path'; import { createHash } from 'node:crypto'; +import { createLogger } from '@codegraph/logger'; + +const logger = createLogger({ namespace: 'Core:Pipeline' }); // ============================================================================ // Plugin Registration @@ -80,9 +83,23 @@ export async function registerTier2Languages(): Promise<{ const registerAllLanguages = mod.registerAllLanguages as ( registry: { register(plugin: any): void } ) => Promise<{ registered: string[]; skipped: string[] }>; - return await registerAllLanguages(languageRegistry); - } catch { - // @codegraph/plugin-languages not installed — tier-2 unavailable + const result = await registerAllLanguages(languageRegistry); + // registerAllLanguages() already isolates a per-language grammar load + // failure (it just lands the language in `skipped`, see + // packages/plugin-languages/src/grammar-loader.ts), so one bad grammar + // never aborts registration of the rest. Log the outcome instead of + // swallowing it, so a missing or broken grammar is visible somewhere. + if (result.skipped.length > 0) { + logger.warn(`Tier-2 languages unavailable (grammar not installed or failed to load): ${result.skipped.join(', ')}`); + } + if (result.registered.length > 0) { + logger.info(`Tier-2 languages registered: ${result.registered.join(', ')}`); + } + return result; + } catch (err) { + // @codegraph/plugin-languages not installed, or registration itself + // crashed. Tier-2 is unavailable, but tier-1 indexing must continue. + logger.warn(`Tier-2 language registration failed, continuing with tier-1 languages only: ${err instanceof Error ? err.message : String(err)}`); return { registered: [], skipped: [] }; } } diff --git a/packages/core/src/services/helpers.ts b/packages/core/src/services/helpers.ts index e35b0e18..e4347382 100644 --- a/packages/core/src/services/helpers.ts +++ b/packages/core/src/services/helpers.ts @@ -5,17 +5,19 @@ import type { CypherDialect } from '@codegraph/graph'; import type { NodeLabel } from '@codegraph/types'; +import { SYMBOL_LABELS, resolveNodeLabel } from '@codegraph/types'; /** Build OR-separated label check expression */ export function labelOr(dialect: CypherDialect, alias: string, labels: string[]): string { return labels.map(l => dialect.labelCheckExpr(alias, l)).join(' OR '); } -export const ALL_LABELS = ['File', 'Function', 'Class', 'Interface', 'Variable', 'Component', 'Type']; - -export const VALID_LABELS: NodeLabel[] = [ - 'File', 'Function', 'Class', 'Interface', 'Variable', 'Type', 'Component', 'Import', -]; +// SYMBOL_LABELS is the shared source of truth (packages/types/src/labels.ts). +// This was a hand-copied 7-item array before; kept as its own mutable +// `string[]` (rather than exporting SYMBOL_LABELS directly) because +// graph-data-service.ts passes it straight into labelOr's `labels: string[]` +// parameter. +export const ALL_LABELS: string[] = [...SYMBOL_LABELS]; export function extractNodeProps(node: Record): Record { if (node['properties'] && typeof node['properties'] === 'object') { @@ -24,17 +26,136 @@ export function extractNodeProps(node: Record): Record VALID_LABELS.includes(l as NodeLabel)); - return (found as NodeLabel) ?? 'File'; + return resolveNodeLabel(labels) ?? 'File'; +} + +/** True string property lookup: rejects missing, non-string, and empty values. */ +function stringProp(props: Record, key: string): string | undefined { + const value = props[key]; + return typeof value === 'string' && value.length > 0 ? value : undefined; } -export function generateNodeId(label: NodeLabel, props: Record): string { - if (label === 'File') { - return `File:${props['filePath'] ?? ''}`; +/** True numeric property lookup: rejects missing and non-number values (0 is valid). */ +function numberProp(props: Record, key: string): number | undefined { + const value = props[key]; + return typeof value === 'number' ? value : undefined; +} + +/** + * Build a unique, stable id for a node from the property that actually + * identifies its label, not a one-size-fits-all name/filePath/line guess. + * + * Real identity per label, confirmed against packages/graph/src/schema.ts's + * node-props types and the MERGE keys in packages/graph/src/operations.ts + * and packages/graph/src/knowledge-operations.ts: + * - File: filePath alone (existing single-key scheme, unchanged) + * - Function, Class, Interface, Variable, Type, Component: name plus + * filePath plus startLine (or line for Variable). Existing scheme, + * unchanged, and already matches these labels' own MERGE keys. + * - Commit: MERGE (c:Commit {hash}), so the identity is hash. + * - MarkdownDocument: MERGE (d:MarkdownDocument {path}), so the identity is path. + * - Section: MERGE (s:Section {filePath, startLine}), so the identity is + * filePath plus startLine. Sections have no `name` property. + * - CodeBlock: MERGE (cb:CodeBlock {filePath, startLine}), so the identity + * is filePath plus startLine. CodeBlocks have no `name` property. + * - Link: MERGE (l:Link {filePath, line, target}), so the identity is + * filePath plus line plus target. Links have no `name` property. + * - Entity: MERGE (n:Entity {text, type}), so the identity is text plus + * type. Entity is not a NodeLabel value (it is a knowledge-graph label, + * not a code-graph one), but a real Entity node reaches this function + * today through graph-data-service.ts's getNeighborsImpl, which builds + * `nodeLabel` straight from the database's own `labels(neighbor)[0]` + * string rather than through getLabelFromLabels. + * + * The old version used the symbol-label scheme (name, filePath, and + * startLine or line) for every label, including these. None of Commit, + * MarkdownDocument, Section, CodeBlock, Link or Entity carry those + * properties, so every node of a given one of those labels collapsed onto + * the same id (every Commit became "Commit::0", regardless of which commit + * it was), a real collision, not just an ugly id, since callers use this id + * to tell nodes apart. A label with no identity contract established here + * (there is no MERGE for an Import node anywhere in packages/graph, so one + * is never actually reachable) falls back to an explicit "unknown" marker + * instead of silently reusing a scheme that does not apply to it. + */ +export function generateNodeId(label: NodeLabel | 'Entity', props: Record): string { + switch (label) { + case 'File': + return `File:${stringProp(props, 'filePath') ?? ''}`; + + case 'Function': + case 'Class': + case 'Interface': + case 'Variable': + case 'Type': + case 'Component': { + const name = stringProp(props, 'name') ?? ''; + const filePath = stringProp(props, 'filePath') ?? ''; + const line = numberProp(props, 'startLine') ?? numberProp(props, 'line') ?? 0; + return `${label}:${filePath}:${name}:${line}`; + } + + case 'Commit': { + const hash = stringProp(props, 'hash'); + return hash ? `Commit:${hash}` : 'Commit:unknown'; + } + + case 'MarkdownDocument': { + const path = stringProp(props, 'path'); + return path ? `MarkdownDocument:${path}` : 'MarkdownDocument:unknown'; + } + + case 'Section': { + const filePath = stringProp(props, 'filePath'); + const startLine = numberProp(props, 'startLine'); + return filePath !== undefined && startLine !== undefined + ? `Section:${filePath}:${startLine}` + : 'Section:unknown'; + } + + case 'CodeBlock': { + const filePath = stringProp(props, 'filePath'); + const startLine = numberProp(props, 'startLine'); + return filePath !== undefined && startLine !== undefined + ? `CodeBlock:${filePath}:${startLine}` + : 'CodeBlock:unknown'; + } + + case 'Link': { + const filePath = stringProp(props, 'filePath'); + const line = numberProp(props, 'line'); + const target = stringProp(props, 'target'); + return filePath !== undefined && line !== undefined && target !== undefined + ? `Link:${filePath}:${line}:${target}` + : 'Link:unknown'; + } + + case 'Entity': { + const text = stringProp(props, 'text'); + const type = stringProp(props, 'type'); + return text !== undefined && type !== undefined + ? `Entity:${type}:${text}` + : 'Entity:unknown'; + } + + // 'Import' (never materialized as a graph node; no MERGE for :Import + // exists anywhere in packages/graph) and any other label this function + // does not yet have an identity contract for. + default: + return `${label}:unknown`; } - const name = props['name'] ?? ''; - const filePath = props['filePath'] ?? ''; - const line = props['startLine'] ?? props['line'] ?? 0; - return `${label}:${filePath}:${name}:${line}`; } diff --git a/packages/dashboard/package.json b/packages/dashboard/package.json index 2146258a..1080c471 100644 --- a/packages/dashboard/package.json +++ b/packages/dashboard/package.json @@ -11,6 +11,7 @@ "typecheck": "tsc --noEmit" }, "dependencies": { + "@codegraph/types": "workspace:*", "@radix-ui/react-separator": "1.1.8", "@radix-ui/react-slot": "1.2.4", "@radix-ui/react-tabs": "1.1.13", diff --git a/packages/dashboard/src/components/dashboard/embedding-badge.tsx b/packages/dashboard/src/components/dashboard/embedding-badge.tsx index 05699067..f63095ab 100644 --- a/packages/dashboard/src/components/dashboard/embedding-badge.tsx +++ b/packages/dashboard/src/components/dashboard/embedding-badge.tsx @@ -2,6 +2,7 @@ import { useEffect, useState, useCallback } from 'react' import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' import { API_URL } from '@/lib/api' +import { EMBEDDABLE_LABELS } from '@codegraph/types' interface EmbeddingLabel { @@ -22,8 +23,9 @@ export function EmbeddingBadge() { if (!res.ok) return const data = await res.json() const labels = (data.labels ?? []) as EmbeddingLabel[] - // Only count embeddable node types (code symbols, not git/markdown structure) - const embeddable = new Set(['File', 'Function', 'Class', 'Interface', 'Variable', 'Type', 'Component', 'Entity']) + // Only count embeddable node types (code symbols, not git/markdown structure). + // EMBEDDABLE_LABELS is the shared source of truth (packages/types/src/labels.ts). + const embeddable = new Set(EMBEDDABLE_LABELS) const relevant = labels.filter(l => embeddable.has(l.label)) const total = relevant.reduce((s, l) => s + l.total, 0) const embedded = relevant.reduce((s, l) => s + l.withEmbedding, 0) diff --git a/packages/graph/src/__tests__/label-constants.test.ts b/packages/graph/src/__tests__/label-constants.test.ts new file mode 100644 index 00000000..6b045e28 --- /dev/null +++ b/packages/graph/src/__tests__/label-constants.test.ts @@ -0,0 +1,179 @@ +import { describe, it, expect, vi } from 'vitest'; +import { + SYMBOL_LABELS, + REFERENCEABLE_LABELS, + EMBEDDABLE_LABELS, + SUMMARY_LABELS, + ALL_GRAPH_LABELS, +} from '@codegraph/types'; +import { createQueries } from '../queries'; +import { createOperations } from '../operations'; +import { getIndexSummary } from '../fileTree'; +import { ensureSchemaImpl } from '../drivers/falkordb-shared'; +import { falkorDialect } from '../drivers/falkordb'; +import type { GraphClient, QueryOptions, QueryResult } from '../client'; + +/** + * Locks in the exact label sets used by every call site that was refactored + * to import from packages/types instead of hand-copying a string array. + * These sites had already drifted apart before the refactor (one had + * 'External', another didn't; one had 'Entity', another didn't) so this + * test exists to make sure the shared constant, not a fresh copy-paste + * mistake, is what each site is actually using. + */ + +function makeFakeClient(): GraphClient & { calls: string[] } { + const calls: string[] = []; + return { + graph: null, + graphName: 'test', + dialect: falkorDialect, + calls, + async query(cypher: string, _options?: QueryOptions): Promise> { + calls.push(cypher); + return { data: [] as T[], metadata: [] }; + }, + async roQuery(cypher: string, _options?: QueryOptions): Promise> { + calls.push(cypher); + return { data: [] as T[], metadata: [] }; + }, + async ensureIndexes(): Promise {}, + } as unknown as GraphClient & { calls: string[] }; +} + +/** Every label the falkordb dialect would check for, as `alias:Label` tokens. */ +function labelTokens(alias: string, labels: readonly string[]): string[] { + return labels.map((l) => `${alias}:${l}`); +} + +describe('queries.ts label sets (GET_FULL_GRAPH_NODES / GET_FULL_GRAPH_EDGES)', () => { + it('GET_FULL_GRAPH_NODES filters on exactly REFERENCEABLE_LABELS (SYMBOL_LABELS + External)', async () => { + const client = makeFakeClient(); + await createQueries(client).getFullGraph(); + + const nodesQuery = client.calls.find((c) => c.includes('RETURN n,')); + expect(nodesQuery).toBeDefined(); + + for (const token of labelTokens('n', REFERENCEABLE_LABELS)) { + expect(nodesQuery).toContain(token); + } + // Nothing beyond REFERENCEABLE_LABELS should show up as an n: