Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 15 additions & 0 deletions src/core/contracts/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
}

/**
* 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<T>(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;
}

/**
Expand Down
68 changes: 68 additions & 0 deletions src/core/engine/rule-engine.test.ts
Original file line number Diff line number Diff line change
@@ -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>): AnalysisNode {
return {
id: "1:1",
name: "TestNode",
type: "FRAME",
visible: true,
...overrides,
};
}

function makeFile(overrides?: Partial<AnalysisFile>): 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
);
});
});
10 changes: 6 additions & 4 deletions src/core/engine/rule-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<string, unknown>();

// Find target node if specified
let rootNode = file.document;
Expand Down Expand Up @@ -177,6 +175,7 @@ export class RuleEngine {
0,
[],
0,
analysisState,
undefined,
undefined
);
Expand Down Expand Up @@ -221,6 +220,7 @@ export class RuleEngine {
depth: number,
path: string[],
componentDepth: number,
analysisState: Map<string, unknown>,
parent?: AnalysisNode,
siblings?: AnalysisNode[]
): void {
Expand All @@ -247,6 +247,7 @@ export class RuleEngine {
maxDepth,
path: nodePath,
siblings,
analysisState,
};

// Run each rule on this node
Expand Down Expand Up @@ -297,6 +298,7 @@ export class RuleEngine {
depth + 1,
nodePath,
currentComponentDepth + 1,
analysisState,
node,
node.children
);
Expand Down
1 change: 1 addition & 0 deletions src/core/rules/ai-readability/invisible-layer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ function makeContext(overrides?: Partial<RuleContext>): RuleContext {
componentDepth: 0,
maxDepth: 10,
path: ["Page", "Section"],
analysisState: new Map(),
...overrides,
};
}
Expand Down
25 changes: 16 additions & 9 deletions src/core/rules/component/index.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -33,12 +30,17 @@ function makeFile(
};
}

/** Each test gets a fresh analysisState to isolate dedup state */
let analysisState: Map<string, unknown>;

function makeContext(overrides?: Partial<RuleContext>): RuleContext {
return {
file: makeFile(),
depth: 2,
componentDepth: 0,
maxDepth: 10,
path: ["Page", "Frame"],
analysisState,
...overrides,
};
}
Expand All @@ -49,7 +51,7 @@ function makeContext(overrides?: Partial<RuleContext>): RuleContext {

describe("missing-component-description", () => {
beforeEach(() => {
resetMissingComponentDescriptionState();
analysisState = new Map();
});

it("returns null for non-INSTANCE nodes", () => {
Expand Down Expand Up @@ -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: "" },
Expand All @@ -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();
});
Expand Down
70 changes: 26 additions & 44 deletions src/core/rules/component/index.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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<string>();
const seenStage4ComponentIds = new Set<string>();
/** 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<string> {
return getAnalysisState(context, SEEN_STAGE1_KEY, () => new Set<string>());
}

function getSeenStage4(context: RuleContext): Set<string> {
return getAnalysisState(context, SEEN_STAGE4_KEY, () => new Set<string>());
}

const missingComponentDef: RuleDefinition = {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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<string>();
/** State key for per-analysis deduplication via RuleContext.analysisState */
const SEEN_MISSING_DESC_KEY = "missing-component-description:seenComponentIds";

function getSeenMissingDescription(context: RuleContext): Set<string> {
return getAnalysisState(context, SEEN_MISSING_DESC_KEY, () => new Set<string>());
}

const missingComponentDescriptionDef: RuleDefinition = {
id: "missing-component-description",
Expand All @@ -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,
Expand All @@ -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();
}
Loading
Loading