Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,38 @@ describe("normalizeNodeId", () => {
).toBe("file:src/foo.ts");
});

it("keeps a real prefix when a different reserved word is a middle segment", () => {
// Regression: "endpoint:service:x" is a valid prefix followed by a real
// path segment that happens to be a reserved word. The outer "endpoint"
// prefix must be preserved, not dropped in favour of "service".
expect(
normalizeNodeId("endpoint:service:getUser", { type: "endpoint" }),
).toBe("endpoint:service:getUser");
});

it("is idempotent for IDs whose middle segment is a reserved word", () => {
const once = normalizeNodeId("endpoint:service:getUser", {
type: "endpoint",
});
expect(normalizeNodeId(once, { type: "endpoint" })).toBe(once);
});

it("strips a project-name prefix that collides with a reserved word", () => {
// Regression: when the project name is itself a reserved word ("service")
// and the node's real prefix follows ("file"), the spurious outer prefix
// must be dropped so the canonical "file:src/foo.ts" form is used — not
// left as "service:file:src/foo.ts", which would dangle edges that
// reference the canonical ID.
expect(
normalizeNodeId("service:file:src/foo.ts", { type: "file" }),
).toBe("file:src/foo.ts");
});

it("is idempotent when a reserved-word project prefix is stripped", () => {
const once = normalizeNodeId("service:file:src/foo.ts", { type: "file" });
expect(normalizeNodeId(once, { type: "file" })).toBe(once);
});

it("strips project-name prefix when valid prefix follows", () => {
expect(
normalizeNodeId("my-project:file:src/foo.ts", { type: "file" }),
Expand Down Expand Up @@ -438,6 +470,91 @@ describe("normalizeBatchOutput", () => {
expect(result.edges[0].source).toBe("file:src/bare.ts");
expect(result.edges[0].target).toBe("file:src/target.ts");
});

it("repairs an edge endpoint whose project prefix collides with a reserved word", () => {
// Regression: an edge endpoint "service:file:src/foo.ts" refers to the
// canonical node "file:src/foo.ts", but inferTypeFromId reads the spurious
// reserved-word project prefix "service" as the type. The fallback must
// still resolve it to the existing node rather than drop the edge.
const result = normalizeBatchOutput({
nodes: [
{
id: "file:src/foo.ts",
type: "file",
name: "foo.ts",
filePath: "src/foo.ts",
summary: "Target",
tags: [],
complexity: "simple",
},
{
id: "file:src/bar.ts",
type: "file",
name: "bar.ts",
filePath: "src/bar.ts",
summary: "Source",
tags: [],
complexity: "simple",
},
],
edges: [
{
source: "file:src/bar.ts",
target: "service:file:src/foo.ts",
type: "imports",
direction: "forward",
weight: 0.7,
},
],
});

expect(result.edges).toHaveLength(1);
expect(result.edges[0].target).toBe("file:src/foo.ts");
expect(result.stats.danglingEdgesDropped).toBe(0);
});

it("repairs an edge endpoint with a chain of reserved-word prefixes", () => {
// Regression: an edge endpoint "service:endpoint:file:src/foo.ts" carries
// more than one reserved prefix before the real "file" prefix. Normalizing
// the full id for each candidate type can't collapse the run, so the repair
// must normalize from the candidate prefix segment to resolve it to the
// canonical node "file:src/foo.ts" rather than drop the edge.
const result = normalizeBatchOutput({
nodes: [
{
id: "file:src/foo.ts",
type: "file",
name: "foo.ts",
filePath: "src/foo.ts",
summary: "Target",
tags: [],
complexity: "simple",
},
{
id: "file:src/bar.ts",
type: "file",
name: "bar.ts",
filePath: "src/bar.ts",
summary: "Source",
tags: [],
complexity: "simple",
},
],
edges: [
{
source: "file:src/bar.ts",
target: "service:endpoint:file:src/foo.ts",
type: "imports",
direction: "forward",
weight: 0.7,
},
],
});

expect(result.edges).toHaveLength(1);
expect(result.edges[0].target).toBe("file:src/foo.ts");
expect(result.stats.danglingEdgesDropped).toBe(0);
});
});

describe("normalizeBatchOutput integration", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,17 @@ const TYPE_TO_PREFIX: Record<string, string> = {
/**
* Strips all non-valid prefixes from an ID, returning the bare path
* and the first valid prefix found (if any).
*
* `expectedPrefix` is the canonical prefix for the node's declared type
* (e.g. "file" for a file node). It disambiguates a reserved word that
* appears before the expected prefix — a spurious project-name prefix that
* happens to collide with a reserved word — from a reserved word that is a
* legitimate middle path segment.
*/
function stripToValidPrefix(id: string): { prefix: string | null; path: string } {
function stripToValidPrefix(
id: string,
expectedPrefix?: string,
): { prefix: string | null; path: string } {
let remaining = id;

// Peel off colon-separated segments until we find a valid prefix or run out
Expand All @@ -38,11 +47,22 @@ function stripToValidPrefix(id: string): { prefix: string | null; path: string }

const segment = remaining.slice(0, colonIdx);
if (VALID_PREFIXES.has(segment)) {
// Check for double valid prefix (e.g., "file:file:src/foo.ts")
// Collapse the outer prefix only when the next segment is either:
// - the SAME reserved word — a true duplicate ("file:file:src/foo.ts"), or
// - the node's expected prefix — a spurious project-name prefix that
// collides with a reserved word ("service:file:src/foo.ts" for a file
// node), which must resolve to the canonical "file:src/foo.ts".
// A different reserved word that is NOT the expected prefix
// ("endpoint:service:x" for an endpoint node) is a real path segment and
// must be preserved.
const rest = remaining.slice(colonIdx + 1);
const innerColonIdx = rest.indexOf(":");
if (innerColonIdx > 0 && VALID_PREFIXES.has(rest.slice(0, innerColonIdx))) {
// Double-prefixed — skip the outer, recurse on inner
const innerSegment = innerColonIdx > 0 ? rest.slice(0, innerColonIdx) : "";
if (
innerColonIdx > 0 &&
(innerSegment === segment || innerSegment === expectedPrefix)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle reserved-word project prefixes in edge fallback

When normalizeBatchOutput falls back to normalizing an edge endpoint that is not present in idMap, it infers the type from the malformed endpoint itself. For a project named like a reserved prefix, e.g. an edge endpoint service:file:src/foo.ts pointing to an existing canonical node file:src/foo.ts, inferTypeFromId supplies service, so this check does not strip the outer service segment and the edge remains dangling and is dropped. This regresses the cross-variant edge repair path for the same reserved-word project-prefix case the node normalization now handles.

Useful? React with 👍 / 👎.

) {
// Skip the outer prefix, recurse on the inner one
remaining = rest;
continue;
}
Expand All @@ -69,7 +89,7 @@ export function normalizeNodeId(
if (!trimmed) return trimmed;

const expectedPrefix = TYPE_TO_PREFIX[node.type];
const { prefix, path } = stripToValidPrefix(trimmed);
const { prefix, path } = stripToValidPrefix(trimmed, expectedPrefix);

if (prefix) {
// For step nodes with filePath, reconstruct as step:flowSlug:filePath:stepSlug.
Expand Down Expand Up @@ -191,6 +211,49 @@ function inferTypeFromId(id: string): string {
return "file";
}

/**
* Best-effort repair of an edge endpoint that matches no node ID.
*
* Tries the prefix-inferred type against the full id first (preserving the
* common case), then peels each leading reserved-word segment and normalizes
* from the suffix that begins at it. This recovers a reserved-word project
* prefix — e.g. an edge endpoint `service:file:src/foo.ts` pointing at the
* canonical node `file:src/foo.ts`, where `inferTypeFromId` would treat the
* spurious `service` as the type and fail to strip it. Normalizing from the
* candidate segment (rather than always from the full id) also handles a
* *chain* of reserved prefixes, e.g. `service:endpoint:file:src/foo.ts`, where
* `stripToValidPrefix` can't collapse the run for the `file` candidate and
* every full-id attempt would otherwise leave the edge dangling. Returns the
* original id unchanged when nothing resolves to an existing node.
*/
function resolveEdgeEndpoint(id: string, validNodeIds: Set<string>): string {
// Each candidate pairs a node type with the id suffix to normalize from.
// The first preserves the common case (inferred type, full id); each peeled
// segment then offers its real prefix and the substring that starts there.
const candidates: { type: string; fromId: string }[] = [
{ type: inferTypeFromId(id), fromId: id },
];

let rest = id;
while (true) {
const colonIdx = rest.indexOf(":");
if (colonIdx <= 0) break;
const segment = rest.slice(0, colonIdx);
if (!(segment in PREFIX_TO_TYPE)) break;
rest = rest.slice(colonIdx + 1);
const type = PREFIX_TO_TYPE[segment];
if (!candidates.some((c) => c.type === type && c.fromId === rest)) {
candidates.push({ type, fromId: rest });
}
}

for (const { type, fromId } of candidates) {
const normalized = normalizeNodeId(fromId, { type });
if (validNodeIds.has(normalized)) return normalized;
}
return id;
}

/**
* Normalizes a merged batch output: fixes node IDs and numeric complexity,
* rewrites edge references, deduplicates nodes and edges, and drops dangling edges.
Expand Down Expand Up @@ -280,18 +343,14 @@ export function normalizeBatchOutput(data: {
let newSource = idMap.get(oldSource) ?? oldSource;
let newTarget = idMap.get(oldTarget) ?? oldTarget;

// Fallback: if endpoint not found in idMap, normalize it directly
// (handles cross-variant malformed IDs between nodes and edges).
// Try the edge's implied type first (from prefix), then fall back to "file".
// Fallback: if an endpoint isn't found in idMap, repair it directly
// (handles cross-variant malformed IDs between nodes and edges, including
// reserved-word project prefixes that inferTypeFromId alone can't resolve).
if (!validNodeIds.has(newSource)) {
const inferredType = inferTypeFromId(newSource);
const normalized = normalizeNodeId(newSource, { type: inferredType });
if (validNodeIds.has(normalized)) newSource = normalized;
newSource = resolveEdgeEndpoint(newSource, validNodeIds);
}
if (!validNodeIds.has(newTarget)) {
const inferredType = inferTypeFromId(newTarget);
const normalized = normalizeNodeId(newTarget, { type: inferredType });
if (validNodeIds.has(normalized)) newTarget = normalized;
newTarget = resolveEdgeEndpoint(newTarget, validNodeIds);
}

if (newSource !== oldSource || newTarget !== oldTarget) {
Expand Down