From ee5275cd9fae8db64f945a0c8b59e1c19ac402c3 Mon Sep 17 00:00:00 2001 From: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Date: Sun, 11 Oct 2026 07:33:57 -0700 Subject: [PATCH] fix(artifacts): count a changed partial-inventory root once (#1854) When partial inventories omitted node details, the root got an unknown availability row and the differing manifest root digests appended a second, changed content row, so one logical path was counted as both changed and unknown. Fold the manifest content change into an existing unknown or changed root row instead, keeping its dimensions, occurrence and artifact identities, and citations; contradiction and presence rows are left as they are. Fixes #1844 Co-authored-by: Claude Opus 5.5 --- src/domain/artifactComparison.ts | 45 +++++++--- .../artifactComparisonSummary.test.ts | 86 ++++++++++++++++++- 2 files changed, 114 insertions(+), 17 deletions(-) diff --git a/src/domain/artifactComparison.ts b/src/domain/artifactComparison.ts index f7c27cdcf..323a9077f 100644 --- a/src/domain/artifactComparison.ts +++ b/src/domain/artifactComparison.ts @@ -123,11 +123,10 @@ export const compareArtifacts = ( ], leftCovered && rightCovered, ); - const rootChange = rootHashChange(left.inventory, right.inventory, changes, [ + applyRootHashChange(left.inventory, right.inventory, changes, [ ...left.evidence.map(({ evidence_id: id }) => id), ...right.evidence.map(({ evidence_id: id }) => id), ]); - if (rootChange !== null) changes.push(rootChange); const summary = summarize( left.inventory, right.inventory, @@ -441,27 +440,45 @@ const hasCoverageLimitation = (limitations: readonly string[]): boolean => !/integrity contradiction\(s\) were recorded/u.test(limitation), ); -const rootHashChange = ( +// Differing manifest root digests prove a root content change even when node +// details are unavailable. Fold that fact into an existing root row so one +// logical path keeps one classification. +const applyRootHashChange = ( left: InventorySet, right: InventorySet, - changes: readonly ArtifactChange[], + changes: ArtifactChange[], evidenceLinks: readonly string[], -): ArtifactChange | null => { - if (left.manifest.root_sha256 === right.manifest.root_sha256) return null; - if ( - changes.some( - (change) => - change.logical_path === "." && change.dimensions.includes("content"), +): void => { + if (left.manifest.root_sha256 === right.manifest.root_sha256) return; + const index = changes.findIndex((change) => change.logical_path === "."); + const existing = changes[index]; + if (existing !== undefined) { + if ( + existing.dimensions.includes("content") || + (existing.classification !== "unknown" && + existing.classification !== "changed") ) - ) - return null; + return; + changes[index] = { + ...existing, + classification: "changed", + dimensions: comparisonDimensionSchema.options.filter( + (dimension) => + dimension === "content" || existing.dimensions.includes(dimension), + ), + evidence_links: [ + ...new Set([...existing.evidence_links, ...evidenceLinks]), + ], + }; + return; + } const leftRoot = left.occurrences.find( (occurrence) => occurrence.logical_path === ".", ); const rightRoot = right.occurrences.find( (occurrence) => occurrence.logical_path === ".", ); - return { + changes.push({ classification: "changed", logical_path: ".", dimensions: ["content"], @@ -470,5 +487,5 @@ const rootHashChange = ( left_artifact_id: leftRoot?.artifact_id ?? null, right_artifact_id: rightRoot?.artifact_id ?? null, evidence_links: [...evidenceLinks], - }; + }); }; diff --git a/tests/boundary/filesystem/artifactComparisonSummary.test.ts b/tests/boundary/filesystem/artifactComparisonSummary.test.ts index bbb721bf7..5042c916c 100644 --- a/tests/boundary/filesystem/artifactComparisonSummary.test.ts +++ b/tests/boundary/filesystem/artifactComparisonSummary.test.ts @@ -9,8 +9,17 @@ import { createEvidence } from "../../../src/domain/evidence.js"; import { jsonValueSchema } from "../../../src/domain/jsonValue.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; -const observe = async (path: string) => { - const inventory = await inventoryArtifact(path); +const observe = async ( + path: string, + omit: { readonly nodes?: boolean; readonly occurrences?: boolean } = {}, +) => { + const observed = await inventoryArtifact(path); + const inventory = { + ...observed, + ...(omit.nodes === true ? { nodes: [] } : {}), + ...(omit.occurrences === true ? { occurrences: [] } : {}), + }; + const partial = omit.nodes === true || omit.occurrences === true; return createEvidence( { path, @@ -22,7 +31,7 @@ const observe = async (path: string) => { operation: "inventory_artifact", parameters: {}, result: jsonValueSchema.parse(inventory), - confidence: "observed", + confidence: partial ? "derived" : "observed", authority: "shipped-artifact", }, ); @@ -60,4 +69,75 @@ describe("artifact comparison summary", () => { }); expect(comparison.changes).toHaveLength(3); }); + + const writeChangedFiles = async () => { + const root = await createTestTempDirectory("rea-artifact-partial-root-"); + const leftPath = join(root, "left.txt"); + const rightPath = join(root, "right.txt"); + await Promise.all([ + writeFile(leftPath, "before"), + writeFile(rightPath, "after"), + ]); + return { leftPath, rightPath }; + }; + + it("counts a changed root once when partial inventories omit node details", async () => { + const { leftPath, rightPath } = await writeChangedFiles(); + const [left, right] = await Promise.all([ + observe(leftPath, { nodes: true }), + observe(rightPath, { nodes: true }), + ]); + const comparison = compareArtifacts(left, right); + + expect(comparison.status).toBe("truncated"); + expect(comparison.summary).toMatchObject({ changed: 1, unknown: 0 }); + expect(comparison.changes).toEqual([ + expect.objectContaining({ + logical_path: ".", + classification: "changed", + dimensions: ["content", "availability"], + left_occurrence_id: expect.stringMatching(/^occ_/u), + right_occurrence_id: expect.stringMatching(/^occ_/u), + evidence_links: [left.evidence_id, right.evidence_id], + }), + ]); + }); + + it("reports a root content change when partial inventories omit every member", async () => { + const { leftPath, rightPath } = await writeChangedFiles(); + const [left, right] = await Promise.all([ + observe(leftPath, { nodes: true, occurrences: true }), + observe(rightPath, { nodes: true, occurrences: true }), + ]); + const comparison = compareArtifacts(left, right); + + expect(comparison.status).toBe("truncated"); + expect(comparison.summary).toMatchObject({ changed: 1 }); + expect(comparison.changes).toEqual([ + expect.objectContaining({ + logical_path: ".", + classification: "changed", + dimensions: ["content"], + }), + ]); + }); + + it("does not infer a content change between identical partial inventories", async () => { + const { leftPath } = await writeChangedFiles(); + const [left, right] = await Promise.all([ + observe(leftPath, { nodes: true }), + observe(leftPath, { nodes: true }), + ]); + const comparison = compareArtifacts(left, right); + + expect(comparison.status).toBe("truncated"); + expect(comparison.summary).toMatchObject({ changed: 0, unknown: 1 }); + expect(comparison.changes).toEqual([ + expect.objectContaining({ + logical_path: ".", + classification: "unknown", + dimensions: ["availability"], + }), + ]); + }); });