diff --git a/understand-anything-plugin/hooks/hooks.json b/understand-anything-plugin/hooks/hooks.json index ce537ce..b8429bb 100644 --- a/understand-anything-plugin/hooks/hooks.json +++ b/understand-anything-plugin/hooks/hooks.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "echo \"$TOOL_INPUT\" | grep -qE 'git\\s+(commit|merge|cherry-pick|rebase)' && [ -f .understand-anything/config.json ] && grep -q '\"autoUpdate\".*true' .understand-anything/config.json && [ -f .understand-anything/knowledge-graph.json ] && echo '[understand-anything] Commit detected with auto-update enabled. You MUST read the file at ${PLUGIN_DIR}/hooks/auto-update-prompt.md and execute its instructions to incrementally update the knowledge graph. Do not ask the user for confirmation — just do it.' || true" + "command": "grep -qE 'git\\s+(commit|merge|cherry-pick|rebase)' <<< \"$TOOL_INPUT\" && [ -f .understand-anything/config.json ] && grep -q '\"autoUpdate\".*true' .understand-anything/config.json && [ -f .understand-anything/knowledge-graph.json ] && echo '[understand-anything] Commit detected with auto-update enabled. You MUST read the file at ${PLUGIN_DIR}/hooks/auto-update-prompt.md and execute its instructions to incrementally update the knowledge graph. Do not ask the user for confirmation — just do it.' || true" } ] } @@ -16,7 +16,7 @@ "hooks": [ { "type": "command", - "command": "[ -f .understand-anything/config.json ] && grep -q '\"autoUpdate\".*true' .understand-anything/config.json && [ -f .understand-anything/meta.json ] && [ -f .understand-anything/knowledge-graph.json ] && [ \"$(sed -n 's/.*\"gitCommitHash\".*\"\\([^\"]*\\)\".*/\\1/p' .understand-anything/meta.json)\" != \"$(git rev-parse HEAD 2>/dev/null)\" ] && echo '[understand-anything] Knowledge graph is stale. You MUST read the file at ${PLUGIN_DIR}/hooks/auto-update-prompt.md and execute its instructions to check for structural changes and update the graph. Do not ask the user for confirmation — just do it.' || true" + "command": "[ -f .understand-anything/config.json ] && grep -q '\"autoUpdate\".*true' .understand-anything/config.json && [ -f .understand-anything/meta.json ] && [ -f .understand-anything/knowledge-graph.json ] && [ \"$(node -p \"JSON.parse(require('fs').readFileSync('.understand-anything/meta.json','utf8')).gitCommitHash\")\" != \"$(git rev-parse HEAD 2>/dev/null)\" ] && echo '[understand-anything] Knowledge graph is stale. You MUST read the file at ${PLUGIN_DIR}/hooks/auto-update-prompt.md and execute its instructions to check for structural changes and update the graph. Do not ask the user for confirmation — just do it.' || true" } ] } diff --git a/understand-anything-plugin/packages/core/src/__tests__/change-classifier.test.ts b/understand-anything-plugin/packages/core/src/__tests__/change-classifier.test.ts index 0a6561a..dd25373 100644 --- a/understand-anything-plugin/packages/core/src/__tests__/change-classifier.test.ts +++ b/understand-anything-plugin/packages/core/src/__tests__/change-classifier.test.ts @@ -46,7 +46,9 @@ describe("classifyUpdate", () => { cosmeticOnlyFiles: ["src/d.ts"], }); - const decision = classifyUpdate(analysis, 50); + // src/ already exists in the project, so adding src/c.ts is not a directory change + const allKnownFiles = ["src/a.ts", "src/b.ts", "src/d.ts", "lib/util.ts"]; + const decision = classifyUpdate(analysis, 50, allKnownFiles); expect(decision.action).toBe("PARTIAL_UPDATE"); expect(decision.filesToReanalyze).toEqual(["src/a.ts", "src/b.ts", "src/c.ts"]); @@ -73,7 +75,8 @@ describe("classifyUpdate", () => { newFiles: ["newdir/file.ts"], }); - const decision = classifyUpdate(analysis, 50); + const allKnownFiles = ["src/existing.ts", "src/other.ts", "lib/util.ts"]; + const decision = classifyUpdate(analysis, 50, allKnownFiles); expect(decision.action).toBe("ARCHITECTURE_UPDATE"); expect(decision.rerunArchitecture).toBe(true); @@ -85,7 +88,34 @@ describe("classifyUpdate", () => { deletedFiles: ["olddir/removed.ts"], }); - const decision = classifyUpdate(analysis, 50); + const allKnownFiles = ["src/existing.ts", "src/other.ts"]; + const decision = classifyUpdate(analysis, 50, allKnownFiles); + + expect(decision.action).toBe("ARCHITECTURE_UPDATE"); + expect(decision.rerunArchitecture).toBe(true); + }); + + it("does NOT trigger ARCHITECTURE_UPDATE for new file in existing directory", () => { + const analysis = makeAnalysis({ + newFiles: ["src/newfile.ts"], + }); + + // src/ is already known via other files in the project + const allKnownFiles = ["src/a.ts", "src/b.ts", "lib/util.ts"]; + const decision = classifyUpdate(analysis, 50, allKnownFiles); + + expect(decision.action).toBe("PARTIAL_UPDATE"); + expect(decision.rerunArchitecture).toBe(false); + }); + + it("triggers ARCHITECTURE_UPDATE for new file in genuinely new directory", () => { + const analysis = makeAnalysis({ + newFiles: ["brand-new-pkg/index.ts"], + }); + + // allKnownFiles only contains src/ and lib/ — no brand-new-pkg/ + const allKnownFiles = ["src/a.ts", "src/b.ts", "lib/util.ts"]; + const decision = classifyUpdate(analysis, 50, allKnownFiles); expect(decision.action).toBe("ARCHITECTURE_UPDATE"); expect(decision.rerunArchitecture).toBe(true); diff --git a/understand-anything-plugin/packages/core/src/__tests__/fingerprint.test.ts b/understand-anything-plugin/packages/core/src/__tests__/fingerprint.test.ts index 4990b54..6cd9533 100644 --- a/understand-anything-plugin/packages/core/src/__tests__/fingerprint.test.ts +++ b/understand-anything-plugin/packages/core/src/__tests__/fingerprint.test.ts @@ -136,6 +136,7 @@ describe("compareFingerprints", () => { imports: [{ source: "./utils", specifiers: ["format"] }], exports: ["main"], totalLines: 30, + hasStructuralAnalysis: true, }; it("returns NONE when content hash is identical", () => { @@ -242,6 +243,7 @@ describe("compareFingerprints", () => { ...baseFp, contentHash: "different", classes: [{ name: "MyClass", methods: ["init"], properties: [], exported: true, lineCount: 30 }], + hasStructuralAnalysis: true, }; const result = compareFingerprints(baseFp, withClass); expect(result.changeLevel).toBe("STRUCTURAL"); @@ -252,16 +254,79 @@ describe("compareFingerprints", () => { const oldFp: FileFingerprint = { ...baseFp, classes: [{ name: "Foo", methods: ["a", "b"], properties: [], exported: true, lineCount: 30 }], + hasStructuralAnalysis: true, }; const newFp: FileFingerprint = { ...baseFp, contentHash: "different", classes: [{ name: "Foo", methods: ["a", "c"], properties: [], exported: true, lineCount: 30 }], + hasStructuralAnalysis: true, }; const result = compareFingerprints(oldFp, newFp); expect(result.changeLevel).toBe("STRUCTURAL"); expect(result.details).toContain("methods changed: Foo"); }); + + it("does NOT mutate input arrays (sort must use spread-copy)", () => { + const oldFp: FileFingerprint = { + ...baseFp, + classes: [{ name: "Foo", methods: ["b", "a"], properties: ["y", "x"], exported: true, lineCount: 30 }], + imports: [{ source: "./utils", specifiers: ["z", "a"] }], + hasStructuralAnalysis: true, + }; + const newFp: FileFingerprint = { + ...baseFp, + contentHash: "different", + classes: [{ name: "Foo", methods: ["b", "a"], properties: ["y", "x"], exported: true, lineCount: 30 }], + imports: [{ source: "./utils", specifiers: ["z", "a"] }], + hasStructuralAnalysis: true, + }; + + // Snapshot original order before comparison + const oldMethodsBefore = [...oldFp.classes[0].methods]; + const oldPropertiesBefore = [...oldFp.classes[0].properties]; + const oldSpecifiersBefore = [...oldFp.imports[0].specifiers]; + const newMethodsBefore = [...newFp.classes[0].methods]; + const newPropertiesBefore = [...newFp.classes[0].properties]; + const newSpecifiersBefore = [...newFp.imports[0].specifiers]; + + compareFingerprints(oldFp, newFp); + + // Arrays must remain in their original order (not sorted in-place) + expect(oldFp.classes[0].methods).toEqual(oldMethodsBefore); + expect(oldFp.classes[0].properties).toEqual(oldPropertiesBefore); + expect(oldFp.imports[0].specifiers).toEqual(oldSpecifiersBefore); + expect(newFp.classes[0].methods).toEqual(newMethodsBefore); + expect(newFp.classes[0].properties).toEqual(newPropertiesBefore); + expect(newFp.imports[0].specifiers).toEqual(newSpecifiersBefore); + }); + + it("classifies as STRUCTURAL when hasStructuralAnalysis is false (no tree-sitter)", () => { + const oldFp: FileFingerprint = { + filePath: "config.yaml", + contentHash: "hash_old", + functions: [], + classes: [], + imports: [], + exports: [], + totalLines: 10, + hasStructuralAnalysis: false, + }; + const newFp: FileFingerprint = { + filePath: "config.yaml", + contentHash: "hash_new", + functions: [], + classes: [], + imports: [], + exports: [], + totalLines: 12, + hasStructuralAnalysis: false, + }; + + const result = compareFingerprints(oldFp, newFp); + expect(result.changeLevel).toBe("STRUCTURAL"); + expect(result.details).toContain("no structural analysis available — conservative classification"); + }); }); describe("analyzeChanges", () => { @@ -282,6 +347,7 @@ describe("analyzeChanges", () => { imports: [], exports: ["main"], totalLines: 30, + hasStructuralAnalysis: true, }, "src/utils.ts": { filePath: "src/utils.ts", @@ -291,6 +357,7 @@ describe("analyzeChanges", () => { imports: [], exports: [], totalLines: 10, + hasStructuralAnalysis: true, }, }, }; diff --git a/understand-anything-plugin/packages/core/src/change-classifier.ts b/understand-anything-plugin/packages/core/src/change-classifier.ts index b7eae92..b14964d 100644 --- a/understand-anything-plugin/packages/core/src/change-classifier.ts +++ b/understand-anything-plugin/packages/core/src/change-classifier.ts @@ -21,6 +21,7 @@ export interface UpdateDecision { export function classifyUpdate( analysis: ChangeAnalysis, totalFilesInGraph: number, + allKnownFiles: string[] = [], ): UpdateDecision { const { newFiles, deletedFiles, structurallyChangedFiles, cosmeticOnlyFiles, unchangedFiles } = analysis; const structuralCount = structurallyChangedFiles.length + newFiles.length + deletedFiles.length; @@ -53,7 +54,7 @@ export function classifyUpdate( } // Check if directory structure changed (new/deleted top-level directories) - const hasDirectoryChanges = detectDirectoryChanges(newFiles, deletedFiles, structurallyChangedFiles); + const hasDirectoryChanges = detectDirectoryChanges(newFiles, deletedFiles, allKnownFiles); if (hasDirectoryChanges || structuralCount > 10) { return { @@ -79,15 +80,16 @@ export function classifyUpdate( /** * Detect if the changes affect the directory structure (new or removed directories). - * Checks if any new/deleted files introduce or remove a top-level source directory. + * Uses all known files in the project as the baseline for existing directories, + * then checks if any new/deleted files introduce or remove a top-level source directory. */ function detectDirectoryChanges( newFiles: string[], deletedFiles: string[], - structurallyChangedFiles: string[], + allKnownFiles: string[], ): boolean { const existingDirs = new Set( - structurallyChangedFiles.map((f) => topDirectory(f)).filter(Boolean), + allKnownFiles.map((f) => topDirectory(f)).filter(Boolean), ); for (const f of newFiles) { diff --git a/understand-anything-plugin/packages/core/src/fingerprint.ts b/understand-anything-plugin/packages/core/src/fingerprint.ts index b2d1d2d..cc66dd0 100644 --- a/understand-anything-plugin/packages/core/src/fingerprint.ts +++ b/understand-anything-plugin/packages/core/src/fingerprint.ts @@ -35,6 +35,7 @@ export interface FileFingerprint { imports: ImportFingerprint[]; exports: string[]; totalLines: number; + hasStructuralAnalysis: boolean; } export interface FingerprintStore { @@ -116,6 +117,7 @@ export function extractFileFingerprint( imports, exports, totalLines, + hasStructuralAnalysis: true, }; } @@ -137,6 +139,16 @@ export function compareFingerprints( return { filePath: newFp.filePath, changeLevel: "NONE", details: [] }; } + // Conservative path: if either fingerprint lacks structural analysis, + // we cannot verify structure didn't change — classify as STRUCTURAL. + if (!oldFp.hasStructuralAnalysis || !newFp.hasStructuralAnalysis) { + return { + filePath: newFp.filePath, + changeLevel: "STRUCTURAL", + details: ["no structural analysis available — conservative classification"], + }; + } + // Compare function signatures const oldFuncNames = new Set(oldFp.functions.map((f) => f.name)); const newFuncNames = new Set(newFp.functions.map((f) => f.name)); @@ -194,10 +206,10 @@ export function compareFingerprints( const oldCls = oldFp.classes.find((c) => c.name === newCls.name); if (!oldCls) continue; - if (JSON.stringify(oldCls.methods.sort()) !== JSON.stringify(newCls.methods.sort())) { + if (JSON.stringify([...oldCls.methods].sort()) !== JSON.stringify([...newCls.methods].sort())) { details.push(`methods changed: ${newCls.name}`); } - if (JSON.stringify(oldCls.properties.sort()) !== JSON.stringify(newCls.properties.sort())) { + if (JSON.stringify([...oldCls.properties].sort()) !== JSON.stringify([...newCls.properties].sort())) { details.push(`properties changed: ${newCls.name}`); } if (oldCls.exported !== newCls.exported) { @@ -206,8 +218,8 @@ export function compareFingerprints( } // Compare imports - const oldImports = oldFp.imports.map((i) => `${i.source}:${i.specifiers.sort().join(",")}`).sort(); - const newImports = newFp.imports.map((i) => `${i.source}:${i.specifiers.sort().join(",")}`).sort(); + const oldImports = oldFp.imports.map((i) => `${i.source}:${[...i.specifiers].sort().join(",")}`).sort(); + const newImports = newFp.imports.map((i) => `${i.source}:${[...i.specifiers].sort().join(",")}`).sort(); if (JSON.stringify(oldImports) !== JSON.stringify(newImports)) { details.push("imports changed"); @@ -265,6 +277,7 @@ export function buildFingerprintStore( imports: [], exports: [], totalLines: content.split("\n").length, + hasStructuralAnalysis: false, }; } } @@ -341,6 +354,7 @@ export function analyzeChanges( imports: [], exports: [], totalLines: content.split("\n").length, + hasStructuralAnalysis: false, }; } diff --git a/understand-anything-plugin/packages/core/src/persistence/index.ts b/understand-anything-plugin/packages/core/src/persistence/index.ts index b553cfd..56db519 100644 --- a/understand-anything-plugin/packages/core/src/persistence/index.ts +++ b/understand-anything-plugin/packages/core/src/persistence/index.ts @@ -64,7 +64,11 @@ export function saveFingerprints(projectRoot: string, store: FingerprintStore): export function loadFingerprints(projectRoot: string): FingerprintStore | null { const filePath = join(projectRoot, UA_DIR, FINGERPRINT_FILE); if (!existsSync(filePath)) return null; - return JSON.parse(readFileSync(filePath, "utf-8")) as FingerprintStore; + try { + return JSON.parse(readFileSync(filePath, "utf-8")) as FingerprintStore; + } catch { + return null; + } } const DEFAULT_CONFIG: ProjectConfig = { autoUpdate: false }; diff --git a/understand-anything-plugin/packages/core/src/persistence/persistence.test.ts b/understand-anything-plugin/packages/core/src/persistence/persistence.test.ts index 97681bc..9629cb9 100644 --- a/understand-anything-plugin/packages/core/src/persistence/persistence.test.ts +++ b/understand-anything-plugin/packages/core/src/persistence/persistence.test.ts @@ -2,8 +2,10 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { mkdtempSync, rmSync, existsSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { saveGraph, loadGraph, saveMeta, loadMeta } from "./index.js"; +import { writeFileSync } from "node:fs"; +import { saveGraph, loadGraph, saveMeta, loadMeta, saveFingerprints, loadFingerprints } from "./index.js"; import type { KnowledgeGraph, AnalysisMeta } from "../types.js"; +import type { FingerprintStore } from "../fingerprint.js"; describe("persistence", () => { let tempDir: string; @@ -115,4 +117,46 @@ describe("persistence", () => { expect(loaded).toBeNull(); }); }); + + describe("saveFingerprints / loadFingerprints", () => { + const sampleFingerprints: FingerprintStore = { + version: "1.0.0", + gitCommitHash: "abc123", + generatedAt: "2026-03-14T00:00:00.000Z", + files: { + "src/index.ts": { + filePath: "src/index.ts", + contentHash: "deadbeef", + functions: [], + classes: [], + imports: [], + exports: [], + totalLines: 10, + hasStructuralAnalysis: false, + }, + }, + }; + + it("should round-trip fingerprints correctly", () => { + saveFingerprints(tempDir, sampleFingerprints); + const loaded = loadFingerprints(tempDir); + + expect(loaded).toEqual(sampleFingerprints); + }); + + it("should return null when no fingerprints file exists", () => { + const loaded = loadFingerprints(tempDir); + expect(loaded).toBeNull(); + }); + + it("should return null when fingerprints.json is corrupted", () => { + const dir = join(tempDir, ".understand-anything"); + // Ensure the directory exists by saving first, then overwrite with garbage + saveFingerprints(tempDir, sampleFingerprints); + writeFileSync(join(dir, "fingerprints.json"), "{{not valid json!!", "utf-8"); + + const loaded = loadFingerprints(tempDir); + expect(loaded).toBeNull(); + }); + }); });