From f719dc46c51a55b3926eec9659d06e7dfb95132b Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:11:41 +0800 Subject: [PATCH] fix(views): select every retained module path without losing identity Use one retained-path owner for module references and items. Preserve selected paths and genuine ambiguity across module kinds; remove the superseded singular helper. Fixes #6790 --- docs/javascript-application-workflows.md | 6 ++ src/domain/analysisView/analysisView.test.ts | 48 ++++++++++- .../analysisView/javascriptApplicationView.ts | 61 +++++++++---- ...riptContentDeduplicatedModulePaths.test.ts | 85 +++++++++++++++++++ 4 files changed, 180 insertions(+), 20 deletions(-) create mode 100644 tests/acceptance/applications/javascriptContentDeduplicatedModulePaths.test.ts diff --git a/docs/javascript-application-workflows.md b/docs/javascript-application-workflows.md index e502cdf5a1..70305b448f 100644 --- a/docs/javascript-application-workflows.md +++ b/docs/javascript-application-workflows.md @@ -40,6 +40,12 @@ combine its fields with the fact's location. Semantic trace results include the canonical context subset referenced by their returned facts, so those facts remain self-contained in the response. +Content-deduplicated module references match every retained observation path. +A path-filtered reference reports the caller-selected path while keeping its +shared node identity. Item selection also accepts any retained path when it +identifies one module; if several module kinds share that path, select the +reported `node_id`. Node-ID selection uses the stable representative path. + ## Retained module facts For canonical identities and retained source-range counts, query the existing diff --git a/src/domain/analysisView/analysisView.test.ts b/src/domain/analysisView/analysisView.test.ts index 6e79299724..a530452abf 100644 --- a/src/domain/analysisView/analysisView.test.ts +++ b/src/domain/analysisView/analysisView.test.ts @@ -27,7 +27,7 @@ import { } from "./analysisViewCoverage.js"; import { sourceMapReference } from "../javascript/javascriptApplicationGraph.fixture.js"; import { createArtifactInspection } from "../artifactInspection.js"; -import { javascriptModulePath } from "./javascriptApplicationView.js"; +import { javascriptModulePaths } from "./javascriptApplicationView.js"; import { inspectAnalysisViewInputSchema, projectAnalysisView, @@ -497,7 +497,7 @@ it("projects JavaScript summary and preserves selected module observations", () if (!item.ok) throw item.error; const moduleNode = analysis.graph.nodes[0]; if (moduleNode === undefined) throw new Error("missing module"); - expect(javascriptModulePath(moduleNode)).toBe("renderer.js"); + expect(javascriptModulePaths(moduleNode)).toEqual(["renderer.js"]); if (item.value.kind !== "item") throw new Error("expected item view"); expect(item.value.item).toMatchObject({ path: "renderer.js", @@ -1369,3 +1369,47 @@ it("preserves selected native facet unavailability instead of an exhausted empty }); } }); + +it("selects a unique content-deduplicated module by any retained path", () => { + const source = analysisViewJavaScriptAnalysisWithSource(); + const original = source.graph.nodes[0]; + const observation = original?.observations[0]; + if (original === undefined || observation === undefined) + throw new Error("Missing module fixture"); + const node = createJavaScriptApplicationNode({ + kind: original.kind, + identity: original.identity, + observations: [ + ...original.observations.map(({ label, properties, evidence }) => ({ + label, + properties, + evidence, + })), + { + evidence: observation.evidence, + label: "renderer-alias.js", + properties: { ...observation.properties, path: "renderer-alias.js" }, + }, + ], + }); + const { graph_id: _graphId, ...graphFacts } = source.graph; + const graph = createJavaScriptApplicationGraph({ + ...graphFacts, + nodes: [node], + }); + const parent = javascriptParent( + analysisViewBindJavaScriptGraphs(source, graph), + ); + for (const path of ["renderer.js", "renderer-alias.js"]) { + const selected = projectAnalysisView(parent, { + kind: "item", + collection: "modules", + selector: { path }, + }); + if (!selected.ok) throw selected.error; + expect(selected.value).toMatchObject({ + kind: "item", + item: { node_id: node.node_id, path }, + }); + } +}); diff --git a/src/domain/analysisView/javascriptApplicationView.ts b/src/domain/analysisView/javascriptApplicationView.ts index 13490df699..25e75b01c1 100644 --- a/src/domain/analysisView/javascriptApplicationView.ts +++ b/src/domain/analysisView/javascriptApplicationView.ts @@ -39,21 +39,27 @@ const jsIncompatible = (detail: string): AnalysisError => }, ]); -/** Exact module path or original source-map reference, when available. */ -export const javascriptModulePath = (node: ApplicationNode): string | null => { - if (node.identity.strategy === "canonical-path") return node.identity.path; +/** Every exact module path or original source-map reference retained by a node. */ +export const javascriptModulePaths = ( + node: ApplicationNode, +): readonly string[] => { + if (node.identity.strategy === "canonical-path") return [node.identity.path]; if (node.identity.strategy === "source-map-original") - return node.identity.original_source; + return [node.identity.original_source]; + const paths = new Set(); for (const observation of node.observations) { const path = observation.properties.path; - if (typeof path === "string") return path; + if (typeof path === "string") { + paths.add(path); + continue; + } const location = observation.evidence.location; if (location.available && location.value.kind === "artifact-path") - return location.value.path; + paths.add(location.value.path); if (location.available && location.value.kind === "source-range") - return location.value.source; + paths.add(location.value.source); } - return null; + return [...paths]; }; interface ModuleEntry { @@ -61,6 +67,7 @@ interface ModuleEntry { readonly kind: JavaScriptModuleReference["kind"]; }; readonly path: string | null; + readonly paths: readonly string[]; } const isModuleNode = (node: ApplicationNode): node is ModuleEntry["node"] => @@ -73,7 +80,10 @@ function* moduleEntries( nodes: readonly ApplicationNode[], ): Generator { for (const node of nodes) { - if (isModuleNode(node)) yield { node, path: javascriptModulePath(node) }; + if (isModuleNode(node)) { + const paths = javascriptModulePaths(node); + yield { node, path: paths[0] ?? null, paths }; + } } } @@ -94,10 +104,20 @@ const modulePage = ( for (const entry of moduleEntries(nodes)) { if (view.where?.kind !== undefined && entry.node.kind !== view.where.kind) continue; - if (view.where?.path !== undefined && entry.path !== view.where.path) + const selectedPath = view.where?.path; + if ( + selectedPath !== undefined && + (selectedPath === null + ? entry.path !== null + : !entry.paths.includes(selectedPath)) + ) continue; + const selectedEntry = + selectedPath !== undefined && selectedPath !== null + ? { ...entry, path: selectedPath } + : entry; if (total >= view.offset && entries.length < view.limit) - entries.push(entry); + entries.push(selectedEntry); total += 1; } return { entries, total }; @@ -147,10 +167,10 @@ const sourceRanges = (node: ApplicationNode): readonly JsonValue[] => : []; }); -const moduleItem = (node: ApplicationNode): JsonValue => ({ +const moduleItem = (node: ApplicationNode, path: string | null): JsonValue => ({ node_id: node.node_id, kind: node.kind, - path: javascriptModulePath(node), + path, identity: node.identity, hashes: [...nodeHashes(node)], exports: [...exportNames(node)], @@ -252,14 +272,14 @@ const parentFields = ( const selectModule = ( modules: readonly ModuleEntry[], selector: Extract["selector"], -): Result => { +): Result => { if ("node_id" in selector) { const matches = modules.filter( ({ node }) => node.node_id === selector.node_id, ); if (matches.length === 1) { const selected = matches[0]; - if (selected !== undefined) return ok(selected.node); + if (selected !== undefined) return ok(selected); } return err( inputError([ @@ -272,10 +292,15 @@ const selectModule = ( ); } if ("path" in selector) { - const matches = modules.filter((entry) => entry.path === selector.path); + const matches = modules.filter((entry) => + selector.path === null + ? entry.path === null + : entry.paths.includes(selector.path), + ); if (matches.length === 1) { const selected = matches[0]; - if (selected !== undefined) return ok(selected.node); + if (selected !== undefined) + return ok({ ...selected, path: selector.path }); } if (matches.length === 0) return err( @@ -403,7 +428,7 @@ export const projectJavaScriptApplicationView = ( return ok({ kind: "item", view, - item: moduleItem(selected.value), + item: moduleItem(selected.value.node, selected.value.path), coverage: completeCoverage(1, modules.length), ...shared, }); diff --git a/tests/acceptance/applications/javascriptContentDeduplicatedModulePaths.test.ts b/tests/acceptance/applications/javascriptContentDeduplicatedModulePaths.test.ts new file mode 100644 index 0000000000..0dff2285d5 --- /dev/null +++ b/tests/acceptance/applications/javascriptContentDeduplicatedModulePaths.test.ts @@ -0,0 +1,85 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect } from "vitest"; + +import { parseEvidence } from "../../../src/domain/evidence.js"; +import { analysisViewResultSchema } from "../../../src/domain/analysisView/analysisView.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest( + "selects each retained path of a content-deduplicated module", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-js-module-alias-view-"); + const source = "export const answer = 42;\n"; + await Promise.all( + ["a.js", "b.js"].map((path) => writeFile(join(root, path), source)), + ); + const analyzed = await cli.run({ + arguments: ["analyze-javascript-application", root, "--format", "json"], + }); + expect(analyzed.exitCode, analyzed.stdout + analyzed.stderr).toBe(0); + const evidence = parseEvidence(analyzed.json); + const evidencePath = join(root, "evidence.json"); + await writeFile(evidencePath, JSON.stringify(evidence)); + const queryPath = join(root, "query.json"); + + const inspect = async (view: Record) => { + await writeFile( + queryPath, + JSON.stringify({ + source: { kind: "evidence-file", path: evidencePath }, + view, + }), + ); + const result = await cli.run({ + arguments: ["inspect-analysis-view", queryPath, "--json"], + }); + expect(result.exitCode, result.stdout + result.stderr).toBe(0); + return analysisViewResultSchema.parse( + parseEvidence(result.json).normalized_result, + ); + }; + + let sharedNodeId: string | undefined; + for (const path of ["a.js", "b.js"]) { + const references = await inspect({ + kind: "javascript", + collection: "modules", + projection: "references", + where: { kind: "javascript-asset", path }, + }); + expect(references).toMatchObject({ + kind: "javascript", + rows: [{ path }], + coverage: { total: 1, examined: 1, exhausted: true }, + }); + if (references.kind !== "javascript") + throw new Error("expected JavaScript module references"); + const nodeId = references.rows[0]?.node_id; + expect(nodeId).toBeDefined(); + if (sharedNodeId === undefined) sharedNodeId = nodeId; + else expect(nodeId).toBe(sharedNodeId); + } + const item = await inspect({ + kind: "item", + collection: "modules", + selector: { node_id: sharedNodeId }, + }); + expect(item).toMatchObject({ + kind: "item", + item: { + node_id: sharedNodeId, + path: "a.js", + observations: expect.arrayContaining([ + expect.objectContaining({ + properties: expect.objectContaining({ path: "a.js" }), + }), + expect.objectContaining({ + properties: expect.objectContaining({ path: "b.js" }), + }), + ]), + }, + }); + }, +);