From e714559a7e097ee7a37ed618dfa51f1753b7d529 Mon Sep 17 00:00:00 2001 From: Randy Wilson Date: Sun, 23 Aug 2026 21:37:46 -0400 Subject: [PATCH] Give Commit and TypeRef nodes their identities in the file subgraph The file detail panel 500ed when a contained symbol had a git-history neighbor: Commit nodes are keyed by hash, never id, and the identity derivation did not know that. Commits now derive Commit: to match the service layer, TypeRefs keep their persisted ids instead of being coerced to a false File identity, and a truly unidentifiable neighbor surfaces in an identityErrors entry instead of failing the whole panel. Co-Authored-By: Claude Fable 5 --- .../file-subgraph-commit-identity.test.ts | 123 ++++++++++++++++++ packages/graph/src/queries.ts | 67 +++++++++- 2 files changed, 183 insertions(+), 7 deletions(-) create mode 100644 packages/graph/src/__tests__/file-subgraph-commit-identity.test.ts diff --git a/packages/graph/src/__tests__/file-subgraph-commit-identity.test.ts b/packages/graph/src/__tests__/file-subgraph-commit-identity.test.ts new file mode 100644 index 0000000..d1e834d --- /dev/null +++ b/packages/graph/src/__tests__/file-subgraph-commit-identity.test.ts @@ -0,0 +1,123 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { mkdtemp, rm } from 'node:fs/promises'; +import { createClient, type GraphClient } from '../client'; +import { createQueries, type GraphQueries } from '../queries'; +import { resolveEmbeddedBinaryPaths } from '../drivers/falkordblite'; + +const describeIfAvailable = resolveEmbeddedBinaryPaths() ? describe : describe.skip; + +describeIfAvailable('File subgraph Commit identity', () => { + let client: GraphClient; + let queries: GraphQueries; + let dataDir: string; + + beforeAll(async () => { + dataDir = await mkdtemp('/tmp/cg-fc-'); + client = await createClient({ + driver: 'falkordblite', + databasePath: dataDir, + graphName: 'file_subgraph_commit_identity', + }); + queries = createQueries(client); + }, 30_000); + + afterAll(async () => { + await client?.close(); + if (dataDir) await rm(dataDir, { recursive: true, force: true }); + }, 15_000); + + it('uses the persisted Commit hash for a contained symbol history relationship', async () => { + const filePath = '/repo/history.ts'; + const symbolId = 'sym:v1:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; + const commitHash = '4915f38093c05dbafb5f780e18a5979b0d779c2e'; + await client.query(` + CREATE (file:File {filePath: $filePath, name: 'history.ts'}) + CREATE (symbol:Interface {id: $symbolId, name: 'HistoryCoverage', filePath: $filePath, startLine: 1}) + CREATE (commit:Commit { + hash: $commitHash, + message: 'Add history coverage', + author: 'Randy Wilson', + email: 'author@example.com', + date: '2026-08-23T20:20:56-04:00' + }) + CREATE (file)-[:CONTAINS]->(symbol) + CREATE (symbol)-[:INTRODUCED_IN]->(commit) + `, { params: { filePath, symbolId, commitHash } }); + + const result = await queries.getFileSubgraph(filePath); + + expect(result.nodes).toEqual(expect.arrayContaining([ + expect.objectContaining({ + id: `Commit:${commitHash}`, + label: 'Commit', + displayName: commitHash, + }), + ])); + expect(result.edges).toEqual(expect.arrayContaining([ + expect.objectContaining({ + source: symbolId, + target: `Commit:${commitHash}`, + label: 'INTRODUCED_IN', + }), + ])); + }); + + it('surfaces a hashless Commit without discarding the identifiable file subgraph', async () => { + const filePath = '/repo/malformed-history.ts'; + const symbolId = 'sym:v1:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; + await client.query(` + CREATE (file:File {filePath: $filePath, name: 'malformed-history.ts'}) + CREATE (symbol:Interface {id: $symbolId, name: 'MalformedHistory', filePath: $filePath, startLine: 1}) + CREATE (commit:Commit {message: 'Missing hash'}) + CREATE (file)-[:CONTAINS]->(symbol) + CREATE (symbol)-[:INTRODUCED_IN]->(commit) + `, { params: { filePath, symbolId } }); + + const result = await queries.getFileSubgraph(filePath) as Awaited< + ReturnType + > & { + identityErrors?: Array<{ labels: string[]; edgeType: string; message: string }>; + }; + + expect(result.nodes.map((node) => node.id)).toEqual([ + `File:${filePath}`, + symbolId, + ]); + expect(new Set(result.edges.map((edge) => edge.label))).toEqual(new Set(['CONTAINS'])); + expect(result.identityErrors).toEqual([{ + labels: ['Commit'], + edgeType: 'INTRODUCED_IN', + message: 'Graph node is missing a persisted id', + }]); + }); + + it('preserves a related TypeRef persisted id and runtime label', async () => { + const filePath = '/repo/type-ref.ts'; + const symbolId = 'sym:v1:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc'; + const typeRefId = 'prim::typescript::string'; + await client.query(` + CREATE (file:File {filePath: $filePath, name: 'type-ref.ts'}) + CREATE (symbol:Function {id: $symbolId, name: 'usesString', filePath: $filePath, startLine: 1}) + CREATE (typeRef:TypeRef {id: $typeRefId, name: 'string', language: 'typescript', isPrimitive: true}) + CREATE (file)-[:CONTAINS]->(symbol) + CREATE (symbol)-[:USES_TYPE]->(typeRef) + `, { params: { filePath, symbolId, typeRefId } }); + + const result = await queries.getFileSubgraph(filePath); + + expect(result.nodes).toEqual(expect.arrayContaining([ + expect.objectContaining({ + id: typeRefId, + label: 'TypeRef', + displayName: 'string', + }), + ])); + expect(result.edges).toEqual(expect.arrayContaining([ + expect.objectContaining({ + source: symbolId, + target: typeRefId, + label: 'USES_TYPE', + }), + ])); + }); +}); diff --git a/packages/graph/src/queries.ts b/packages/graph/src/queries.ts index c95fbfa..c23066f 100644 --- a/packages/graph/src/queries.ts +++ b/packages/graph/src/queries.ts @@ -23,12 +23,25 @@ import type { // Type Guards and Helpers // ============================================================================ -type DashboardNodeLabel = NodeLabel | 'Entity'; +type DashboardNodeLabel = NodeLabel | 'Entity' | 'TypeRef'; + +type FileSubgraphIdentityError = { + labels: string[]; + edgeType: string; + message: string; +}; + +type FileSubgraphResult = SubgraphData & { + identityErrors?: FileSubgraphIdentityError[]; +}; function getLabelFromLabels(labels: string[]): DashboardNodeLabel { if (labels.includes('Entity')) { return 'Entity'; } + if (labels.includes('TypeRef')) { + return 'TypeRef'; + } // resolveNodeLabel is the shared classifier (packages/types/src/labels.ts): // it walks `labels` in DB order and returns the first recognized @@ -101,6 +114,10 @@ function persistedNodeId(props: Record, label?: DashboardNodeLa const filePath = typeof props['filePath'] === 'string' ? props['filePath'] : ''; return `File:${filePath}`; } + if (label === 'Commit') { + const hash = typeof props['hash'] === 'string' ? props['hash'] : ''; + if (hash.length > 0) return `Commit:${hash}`; + } const id = props['id']; if (typeof id === 'string' && id.length > 0) { return id; @@ -113,6 +130,10 @@ function persistedNodeId(props: Record, label?: DashboardNodeLa throw new Error('Graph node is missing a persisted id'); } +function isMissingPersistedNodeIdError(error: unknown): error is Error { + return error instanceof Error && error.message === 'Graph node is missing a persisted id'; +} + function nodeToGraphNode(node: Record, labels: string[], dialect?: CypherDialect): GraphNode { const actualLabels = extractLabels(node, labels, dialect); const props = extractNodeProps(node, dialect); @@ -123,7 +144,7 @@ function nodeToGraphNode(node: Record, labels: string[], dialec return { id, label, - displayName: (props['name'] as string) ?? (props['filePath'] as string) ?? (props['text'] as string) ?? 'unknown', + displayName: (props['name'] as string) ?? (props['filePath'] as string) ?? (props['text'] as string) ?? (props['hash'] as string) ?? 'unknown', filePath: (props['filePath'] as string), data: dashboardProps, } as unknown as GraphNode; @@ -1159,10 +1180,11 @@ class GraphQueriesImpl implements GraphQueries { } @trace() - async getFileSubgraph(filePath: string): Promise { + async getFileSubgraph(filePath: string): Promise { const nodes: GraphNode[] = []; const edges: GraphEdge[] = []; const nodeIds = new Set(); + const identityErrors: FileSubgraphIdentityError[] = []; const result = await this.client.roQuery<{ f: Record; @@ -1187,7 +1209,18 @@ class GraphQueriesImpl implements GraphQueries { // Add contained entity if (row.e) { - const entityNode = nodeToGraphNode(row.e, row.labels, this.dialect); + let entityNode: GraphNode; + try { + entityNode = nodeToGraphNode(row.e, row.labels, this.dialect); + } catch (error) { + if (!isMissingPersistedNodeIdError(error)) throw error; + identityErrors.push({ + labels: row.labels, + edgeType: 'CONTAINS', + message: error.message, + }); + continue; + } if (!nodeIds.has(entityNode.id)) { nodes.push(entityNode); nodeIds.add(entityNode.id); @@ -1205,7 +1238,18 @@ class GraphQueriesImpl implements GraphQueries { // Add related entities and edges if (row.related && row.relatedLabels && row.edgeType && row.r) { - const relatedNode = nodeToGraphNode(row.related, row.relatedLabels, this.dialect); + let relatedNode: GraphNode; + try { + relatedNode = nodeToGraphNode(row.related, row.relatedLabels, this.dialect); + } catch (error) { + if (!isMissingPersistedNodeIdError(error)) throw error; + identityErrors.push({ + labels: row.relatedLabels, + edgeType: row.edgeType, + message: error.message, + }); + continue; + } if (!nodeIds.has(relatedNode.id)) { nodes.push(relatedNode); nodeIds.add(relatedNode.id); @@ -1230,9 +1274,18 @@ class GraphQueriesImpl implements GraphQueries { } if (centerId !== undefined) { - return { nodes, edges, centerId }; + return { + nodes, + edges, + centerId, + ...(identityErrors.length > 0 ? { identityErrors } : {}), + }; } - return { nodes, edges }; + return { + nodes, + edges, + ...(identityErrors.length > 0 ? { identityErrors } : {}), + }; } @trace()