diff --git a/src/core/contracts/rule.ts b/src/core/contracts/rule.ts index ed7fc05c..d6189376 100644 --- a/src/core/contracts/rule.ts +++ b/src/core/contracts/rule.ts @@ -42,6 +42,21 @@ export interface RuleContext { maxDepth: number; path: string[]; siblings?: AnalysisNode[] | undefined; + /** Per-analysis shared state. Created fresh for each analysis run, eliminating module-level mutable state. */ + analysisState: Map; +} + +/** + * Get or initialize per-analysis state for a rule. + * Each key gets its own lazily-initialized state that persists for the duration of one analysis run. + */ +export function getAnalysisState(context: RuleContext, key: string, init: () => T): T { + if (context.analysisState.has(key)) { + return context.analysisState.get(key) as T; + } + const value = init(); + context.analysisState.set(key, value); + return value; } /** diff --git a/src/core/engine/rule-engine.test.ts b/src/core/engine/rule-engine.test.ts new file mode 100644 index 00000000..3dba7dc4 --- /dev/null +++ b/src/core/engine/rule-engine.test.ts @@ -0,0 +1,68 @@ +import { RuleEngine } from "./rule-engine.js"; +import type { AnalysisFile, AnalysisNode } from "../contracts/figma-node.js"; + +// Import rules to register +import "../rules/index.js"; + +function makeNode(overrides?: Partial): AnalysisNode { + return { + id: "1:1", + name: "TestNode", + type: "FRAME", + visible: true, + ...overrides, + }; +} + +function makeFile(overrides?: Partial): AnalysisFile { + return { + fileKey: "test", + name: "Test", + lastModified: "", + version: "1", + document: makeNode({ id: "0:1", name: "Document", type: "DOCUMENT" }), + components: {}, + styles: {}, + ...overrides, + }; +} + +describe("RuleEngine.analyze — per-analysis state isolation", () => { + it("produces identical results when called twice on the same instance", () => { + // Two repeated frames with same name + matching component → missing-component Stage 1 + const frameA = makeNode({ id: "f:1", name: "Button" }); + const frameB = makeNode({ id: "f:2", name: "Button" }); + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [frameA, frameB], + }); + + const file = makeFile({ + document: doc, + components: { + "comp:1": { key: "comp:1", name: "Button", description: "" }, + }, + }); + + const engine = new RuleEngine(); + + const result1 = engine.analyze(file); + const result2 = engine.analyze(file); + + // Both runs should find the same missing-component issue + const missingComp1 = result1.issues.filter( + (i) => i.violation.ruleId === "missing-component" + ); + const missingComp2 = result2.issues.filter( + (i) => i.violation.ruleId === "missing-component" + ); + + expect(missingComp1.length).toBeGreaterThan(0); + expect(missingComp2.length).toBe(missingComp1.length); + expect(missingComp2[0]?.violation.message).toBe( + missingComp1[0]?.violation.message + ); + }); +}); diff --git a/src/core/engine/rule-engine.ts b/src/core/engine/rule-engine.ts index 310acfef..690018e9 100644 --- a/src/core/engine/rule-engine.ts +++ b/src/core/engine/rule-engine.ts @@ -9,7 +9,6 @@ import type { import { supportsDepthWeight } from "../contracts/rule.js"; import { ruleRegistry } from "../rules/rule-registry.js"; import { RULE_CONFIGS } from "../rules/rule-config.js"; -import { resetMissingComponentState, resetMissingComponentDescriptionState } from "../rules/component/index.js"; /** * Analysis issue with calculated score and metadata @@ -146,9 +145,8 @@ export class RuleEngine { * Analyze a Figma file and return issues */ analyze(file: AnalysisFile): AnalysisResult { - // Reset module-level dedup state for rules that track seen patterns - resetMissingComponentState(); - resetMissingComponentDescriptionState(); + // Fresh per-analysis state — eliminates module-level mutable state in rules + const analysisState = new Map(); // Find target node if specified let rootNode = file.document; @@ -177,6 +175,7 @@ export class RuleEngine { 0, [], 0, + analysisState, undefined, undefined ); @@ -221,6 +220,7 @@ export class RuleEngine { depth: number, path: string[], componentDepth: number, + analysisState: Map, parent?: AnalysisNode, siblings?: AnalysisNode[] ): void { @@ -247,6 +247,7 @@ export class RuleEngine { maxDepth, path: nodePath, siblings, + analysisState, }; // Run each rule on this node @@ -297,6 +298,7 @@ export class RuleEngine { depth + 1, nodePath, currentComponentDepth + 1, + analysisState, node, node.children ); diff --git a/src/core/rules/ai-readability/invisible-layer.test.ts b/src/core/rules/ai-readability/invisible-layer.test.ts index ff6d8529..db1a261a 100644 --- a/src/core/rules/ai-readability/invisible-layer.test.ts +++ b/src/core/rules/ai-readability/invisible-layer.test.ts @@ -31,6 +31,7 @@ function makeContext(overrides?: Partial): RuleContext { componentDepth: 0, maxDepth: 10, path: ["Page", "Section"], + analysisState: new Map(), ...overrides, }; } diff --git a/src/core/rules/component/index.test.ts b/src/core/rules/component/index.test.ts index 83597f7e..60507c8a 100644 --- a/src/core/rules/component/index.test.ts +++ b/src/core/rules/component/index.test.ts @@ -1,9 +1,6 @@ import type { RuleContext } from "../../contracts/rule.js"; import type { AnalysisFile, AnalysisNode } from "../../contracts/figma-node.js"; -import { - missingComponentDescription, - resetMissingComponentDescriptionState, -} from "./index.js"; +import { missingComponentDescription } from "./index.js"; // ============================================ // Test helpers @@ -33,12 +30,17 @@ function makeFile( }; } +/** Each test gets a fresh analysisState to isolate dedup state */ +let analysisState: Map; + function makeContext(overrides?: Partial): RuleContext { return { file: makeFile(), depth: 2, + componentDepth: 0, maxDepth: 10, path: ["Page", "Frame"], + analysisState, ...overrides, }; } @@ -49,7 +51,7 @@ function makeContext(overrides?: Partial): RuleContext { describe("missing-component-description", () => { beforeEach(() => { - resetMissingComponentDescriptionState(); + analysisState = new Map(); }); it("returns null for non-INSTANCE nodes", () => { @@ -236,7 +238,7 @@ describe("missing-component-description", () => { expect(result2!.message).toContain("Input"); }); - it("resets deduplication state between test runs via resetMissingComponentDescriptionState", () => { + it("fresh analysisState clears dedup state between analysis runs", () => { const ctx = makeContext({ file: makeFile({ "comp:1": { key: "comp:1", name: "Button", description: "" }, @@ -249,12 +251,17 @@ describe("missing-component-description", () => { ); expect(first).not.toBeNull(); - // After reset, same component should be flaggable again - resetMissingComponentDescriptionState(); + // Fresh analysisState simulates a new analysis run — should flag again + analysisState = new Map(); + const freshCtx = makeContext({ + file: makeFile({ + "comp:1": { key: "comp:1", name: "Button", description: "" }, + }), + }); const second = missingComponentDescription.check( makeNode({ id: "inst:1", type: "INSTANCE", componentId: "comp:1" }), - ctx + freshCtx ); expect(second).not.toBeNull(); }); diff --git a/src/core/rules/component/index.ts b/src/core/rules/component/index.ts index 2049bde8..5ce9ff99 100644 --- a/src/core/rules/component/index.ts +++ b/src/core/rules/component/index.ts @@ -1,4 +1,5 @@ -import type { RuleCheckFn, RuleDefinition } from "../../contracts/rule.js"; +import type { RuleCheckFn, RuleDefinition, RuleContext } from "../../contracts/rule.js"; +import { getAnalysisState } from "../../contracts/rule.js"; import type { AnalysisNode } from "../../contracts/figma-node.js"; import { defineRule } from "../rule-registry.js"; import { getRuleOption } from "../rule-config.js"; @@ -94,26 +95,16 @@ function isInsideInstance(context: { // missing-component (unified 4-stage rule) // ============================================ -/** - * Module-level dedup Sets for missing-component stages. - * These prevent duplicate violations when the same pattern is encountered - * multiple times during a single analysis run. - * - * IMPORTANT: The analysis engine must call resetMissingComponentState() - * before each run to clear stale state (especially in long-running processes - * like the MCP server). See rule-engine.ts analyze() method. - */ -const seenStage1ComponentNames = new Set(); -const seenStage4ComponentIds = new Set(); +/** State keys for per-analysis deduplication via RuleContext.analysisState */ +const SEEN_STAGE1_KEY = "missing-component:seenStage1ComponentNames"; +const SEEN_STAGE4_KEY = "missing-component:seenStage4ComponentIds"; -/** - * Reset deduplication state for missing-component between analysis runs. - * Call this at the start of each analysis if the process is long-running - * (e.g. MCP server mode). - */ -export function resetMissingComponentState(): void { - seenStage1ComponentNames.clear(); - seenStage4ComponentIds.clear(); +function getSeenStage1(context: RuleContext): Set { + return getAnalysisState(context, SEEN_STAGE1_KEY, () => new Set()); +} + +function getSeenStage4(context: RuleContext): Set { + return getAnalysisState(context, SEEN_STAGE4_KEY, () => new Set()); } const missingComponentDef: RuleDefinition = { @@ -142,13 +133,14 @@ const missingComponentCheck: RuleCheckFn = (node, context, options) => { const firstFrame = sameNameFrames?.[0]; if (matchingComponent) { + const seenStage1 = getSeenStage1(context); if ( sameNameFrames && firstFrame !== undefined && sameNameFrames.length >= 2 && - !seenStage1ComponentNames.has(node.name.toLowerCase()) + !seenStage1.has(node.name.toLowerCase()) ) { - seenStage1ComponentNames.add(node.name.toLowerCase()); + seenStage1.add(node.name.toLowerCase()); if (firstFrame === node.id) { return { ruleId: missingComponentDef.id, @@ -247,7 +239,8 @@ const missingComponentCheck: RuleCheckFn = (node, context, options) => { // the designer should use a variant instead. // ======================================== if (node.type === "INSTANCE" && node.componentId) { - if (seenStage4ComponentIds.has(node.componentId)) return null; + const seenStage4 = getSeenStage4(context); + if (seenStage4.has(node.componentId)) return null; const componentDefs = context.file.componentDefinitions; if (!componentDefs) return null; @@ -259,7 +252,7 @@ const missingComponentCheck: RuleCheckFn = (node, context, options) => { const overrides = detectStyleOverrides(master, node); if (overrides.length > 0) { // Only mark as seen when we actually flag — allows other instances to be checked - seenStage4ComponentIds.add(node.componentId); + seenStage4.add(node.componentId); const componentMeta = context.file.components[node.componentId]; const componentName = componentMeta?.name ?? node.name; @@ -434,16 +427,12 @@ export const singleUseComponent = defineRule({ // missing-component-description // ============================================ -/** - * Module-level Set for deduplication across nodes within a single analysis run. - * Tracks componentIds that have already been flagged to avoid duplicate issues - * when many INSTANCE nodes reference the same component. - * - * Note: This Set persists for the lifetime of the module (i.e., the process). - * The analysis engine is expected to clear it between runs if needed, but since - * each CLI invocation starts a fresh process this is safe in practice. - */ -const seenMissingDescriptionComponentIds = new Set(); +/** State key for per-analysis deduplication via RuleContext.analysisState */ +const SEEN_MISSING_DESC_KEY = "missing-component-description:seenComponentIds"; + +function getSeenMissingDescription(context: RuleContext): Set { + return getAnalysisState(context, SEEN_MISSING_DESC_KEY, () => new Set()); +} const missingComponentDescriptionDef: RuleDefinition = { id: "missing-component-description", @@ -466,8 +455,9 @@ const missingComponentDescriptionCheck: RuleCheckFn = (node, context) => { if (componentMeta.description.trim() !== "") return null; // Deduplicate: emit at most one issue per unique componentId - if (seenMissingDescriptionComponentIds.has(componentId)) return null; - seenMissingDescriptionComponentIds.add(componentId); + const seenDesc = getSeenMissingDescription(context); + if (seenDesc.has(componentId)) return null; + seenDesc.add(componentId); return { ruleId: missingComponentDescriptionDef.id, @@ -482,11 +472,3 @@ export const missingComponentDescription = defineRule({ check: missingComponentDescriptionCheck, }); -/** - * Reset deduplication state between analysis runs. - * Call this at the start of each analysis if the process is long-running - * (e.g. MCP server mode). - */ -export function resetMissingComponentDescriptionState(): void { - seenMissingDescriptionComponentIds.clear(); -} diff --git a/src/core/rules/component/missing-component.test.ts b/src/core/rules/component/missing-component.test.ts index 440a345f..ba92fa46 100644 --- a/src/core/rules/component/missing-component.test.ts +++ b/src/core/rules/component/missing-component.test.ts @@ -1,6 +1,6 @@ import type { RuleContext } from "../../contracts/rule.js"; import type { AnalysisFile, AnalysisNode } from "../../contracts/figma-node.js"; -import { missingComponent, resetMissingComponentState } from "./index.js"; +import { missingComponent } from "./index.js"; // ============================================ // Test helpers @@ -32,6 +32,9 @@ function makeFile( }; } +/** Each test gets a fresh analysisState to isolate dedup state */ +let analysisState: Map; + function makeContext(overrides?: Partial): RuleContext { return { file: makeFile(), @@ -39,6 +42,7 @@ function makeContext(overrides?: Partial): RuleContext { componentDepth: 0, maxDepth: 10, path: ["Page", "Section"], + analysisState, ...overrides, }; } @@ -61,7 +65,7 @@ function makeChildFrame( describe("missing-component — Stage 1: Component exists but not used", () => { beforeEach(() => { - resetMissingComponentState(); + analysisState = new Map(); }); it("returns null when no component matches frame name", () => { @@ -170,7 +174,7 @@ describe("missing-component — Stage 1: Component exists but not used", () => { describe("missing-component — Stage 2: Name-based repetition", () => { beforeEach(() => { - resetMissingComponentState(); + analysisState = new Map(); }); it("returns null below minRepetitions threshold", () => { @@ -276,7 +280,7 @@ describe("missing-component — Stage 2: Name-based repetition", () => { describe("missing-component — Stage 3: Structure-based repetition", () => { beforeEach(() => { - resetMissingComponentState(); + analysisState = new Map(); }); it("detects identical sibling structure", () => { @@ -453,7 +457,7 @@ describe("missing-component — Stage 3: Structure-based repetition", () => { describe("missing-component — Stage 4: Instance style overrides", () => { beforeEach(() => { - resetMissingComponentState(); + analysisState = new Map(); }); it("returns null for non-INSTANCE nodes", () => { @@ -705,7 +709,7 @@ describe("missing-component — Stage 4: Instance style overrides", () => { describe("missing-component — General", () => { beforeEach(() => { - resetMissingComponentState(); + analysisState = new Map(); }); it("has correct rule definition metadata", () => { @@ -717,7 +721,7 @@ describe("missing-component — General", () => { expect(def.fix).toBeTruthy(); }); - it("resetMissingComponentState clears dedup state", () => { + it("fresh analysisState clears dedup state", () => { const frameA = makeNode({ id: "f:1", name: "Button" }); const frameB = makeNode({ id: "f:2", name: "Button" }); const doc = makeNode({ @@ -727,23 +731,24 @@ describe("missing-component — General", () => { children: [frameA, frameB], }); - const ctx = makeContext({ - file: makeFile({ - document: doc, - components: { - "comp:1": { key: "comp:1", name: "Button", description: "" }, - }, - }), + const file = makeFile({ + document: doc, + components: { + "comp:1": { key: "comp:1", name: "Button", description: "" }, + }, }); + const ctx = makeContext({ file }); + // First call flags (Stage 1) expect(missingComponent.check(frameA, ctx)).not.toBeNull(); // Deduped expect(missingComponent.check(frameA, ctx)).toBeNull(); - // After reset, should flag again - resetMissingComponentState(); - expect(missingComponent.check(frameA, ctx)).not.toBeNull(); + // Fresh analysisState simulates a new analysis run — should flag again + analysisState = new Map(); + const freshCtx = makeContext({ file }); + expect(missingComponent.check(frameA, freshCtx)).not.toBeNull(); }); }); diff --git a/src/core/rules/custom/custom-rule-loader.test.ts b/src/core/rules/custom/custom-rule-loader.test.ts index 81814afa..58bc6fb1 100644 --- a/src/core/rules/custom/custom-rule-loader.test.ts +++ b/src/core/rules/custom/custom-rule-loader.test.ts @@ -10,8 +10,10 @@ function makeContext(overrides?: Partial): RuleContext { return { file: {} as never, depth: 2, + componentDepth: 0, maxDepth: 5, path: ["Root", "Section"], + analysisState: new Map(), ...overrides, }; }