Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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,48 @@ 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);
});
});

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,40 @@ function inferTypeFromId(id: string): string {
return "file";
}

/**
* Best-effort repair of an edge endpoint that matches no node ID.
*
* Tries the prefix-inferred type first (preserving the common case), then
* each subsequent leading reserved-word segment as a candidate type. 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, leaving the edge dangling. Returns the original id
* unchanged when nothing resolves to an existing node.
*/
function resolveEdgeEndpoint(id: string, validNodeIds: Set<string>): string {
const candidateTypes: string[] = [inferTypeFromId(id)];

// Add each leading valid-prefix segment's type as an additional candidate,
// so a spurious outer reserved word can be skipped in favour of the real one.
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;
const type = PREFIX_TO_TYPE[segment];
if (!candidateTypes.includes(type)) candidateTypes.push(type);
rest = rest.slice(colonIdx + 1);
}

for (const type of candidateTypes) {
const normalized = normalizeNodeId(id, { type });

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 Normalize from the candidate prefix segment

When an edge endpoint has more than one reserved prefix before the real node prefix, e.g. service:endpoint:file:src/foo.ts pointing at an existing file:src/foo.ts, candidateTypes includes file but this loop still calls normalizeNodeId on the full original ID. For the file candidate, stripToValidPrefix sees service followed by endpoint and preserves service:endpoint:..., so none of the later candidates can resolve and the edge is dropped as dangling; the previous fallback would collapse this chain. The repair needs to normalize from the candidate segment (or otherwise skip earlier reserved segments) rather than always from id.

Useful? React with 👍 / 👎.

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 +334,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