From 542910ab696dfe854e58e4d39a41fa62d412863b Mon Sep 17 00:00:00 2001 From: let-sunny Date: Wed, 25 Mar 2026 20:02:37 +0900 Subject: [PATCH 1/4] test: add unit tests for scoring.ts and extend rule-engine.ts tests scoring.ts had zero tests despite being the core scoring output module. rule-engine.test.ts had only 1 test (state isolation). This adds: scoring.test.ts (23 tests): - calculateScores: zero issues, severity counting, density weights, diversity penalty, combined formula, floor clamping, nodeCount edge cases - Grade boundaries: S at 100%, F below 50%, gradeToClassName - formatScoreSummary, getCategoryLabel, getSeverityLabel - buildResultJson field validation rule-engine.test.ts (+15 tests): - targetNodeId: subtree scoping, dash-to-colon normalization, missing node error - excludeNodeTypes / excludeNodeNames filtering - enabledRules / disabledRules / config.enabled filtering - depthWeight interpolation (root vs leaf scoring) - Tree traversal: nodeCount, maxDepth, empty document - Error resilience: analysis continues when rules throw - analyzeFile convenience function with custom configs Closes #55 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/core/engine/rule-engine.test.ts | 350 ++++++++++++++++++++++- src/core/engine/scoring.test.ts | 419 ++++++++++++++++++++++++++++ 2 files changed, 768 insertions(+), 1 deletion(-) create mode 100644 src/core/engine/scoring.test.ts diff --git a/src/core/engine/rule-engine.test.ts b/src/core/engine/rule-engine.test.ts index 3dba7dc4..b7363b5d 100644 --- a/src/core/engine/rule-engine.test.ts +++ b/src/core/engine/rule-engine.test.ts @@ -1,5 +1,7 @@ -import { RuleEngine } from "./rule-engine.js"; +import { RuleEngine, analyzeFile } from "./rule-engine.js"; import type { AnalysisFile, AnalysisNode } from "../contracts/figma-node.js"; +import type { RuleConfig, RuleId } from "../contracts/rule.js"; +import { RULE_CONFIGS } from "../rules/rule-config.js"; // Import rules to register import "../rules/index.js"; @@ -27,6 +29,8 @@ function makeFile(overrides?: Partial): AnalysisFile { }; } +// ─── Per-analysis state isolation ───────────────────────────────────────────── + 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 @@ -66,3 +70,347 @@ describe("RuleEngine.analyze — per-analysis state isolation", () => { ); }); }); + +// ─── targetNodeId / findNodeById ────────────────────────────────────────────── + +describe("RuleEngine.analyze — targetNodeId", () => { + it("analyzes only the subtree of the target node", () => { + const targetChild = makeNode({ id: "2:1", name: "TargetChild", type: "FRAME" }); + const targetNode = makeNode({ + id: "1:1", + name: "Target", + type: "FRAME", + children: [targetChild], + }); + const otherNode = makeNode({ id: "3:1", name: "Other", type: "FRAME" }); + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [targetNode, otherNode], + }); + + const file = makeFile({ document: doc }); + const engine = new RuleEngine({ targetNodeId: "1:1" }); + const result = engine.analyze(file); + + // Should count only target subtree nodes (Target + TargetChild = 2) + expect(result.nodeCount).toBe(2); + }); + + it("normalizes dash to colon in node IDs (URL format)", () => { + const targetNode = makeNode({ id: "1:100", name: "Target", type: "FRAME" }); + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [targetNode], + }); + + const file = makeFile({ document: doc }); + // URL format uses "-" instead of ":" + const engine = new RuleEngine({ targetNodeId: "1-100" }); + const result = engine.analyze(file); + + expect(result.nodeCount).toBe(1); + }); + + it("throws when targetNodeId does not exist", () => { + const file = makeFile(); + const engine = new RuleEngine({ targetNodeId: "999:999" }); + + expect(() => engine.analyze(file)).toThrow("Node not found: 999:999"); + }); +}); + +// ─── excludeNodeNames / excludeNodeTypes ────────────────────────────────────── + +describe("RuleEngine.analyze — node exclusion", () => { + it("skips nodes matching excludeNodeTypes", () => { + const textNode = makeNode({ id: "t:1", name: "Label", type: "TEXT" }); + const frameNode = makeNode({ + id: "f:1", + name: "Container", + type: "FRAME", + children: [textNode], + }); + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [frameNode], + }); + + const file = makeFile({ document: doc }); + + // Analyze without exclusion + const resultAll = analyzeFile(file); + // Analyze with TEXT excluded + const resultExcluded = analyzeFile(file, { excludeNodeTypes: ["TEXT"] }); + + // Issues from TEXT nodes should be absent + const textIssuesAll = resultAll.issues.filter( + (i) => i.violation.nodeId === "t:1" + ); + const textIssuesExcluded = resultExcluded.issues.filter( + (i) => i.violation.nodeId === "t:1" + ); + + expect(textIssuesExcluded.length).toBeLessThanOrEqual(textIssuesAll.length); + // If TEXT generated any issues, they should be filtered out + if (textIssuesAll.length > 0) { + expect(textIssuesExcluded.length).toBe(0); + } + }); + + it("skips nodes matching excludeNodeNames pattern", () => { + const ignoredNode = makeNode({ id: "i:1", name: "IgnoreMe", type: "FRAME" }); + const normalNode = makeNode({ id: "n:1", name: "Normal", type: "FRAME" }); + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [ignoredNode, normalNode], + }); + + const file = makeFile({ document: doc }); + const result = analyzeFile(file, { excludeNodeNames: ["IgnoreMe"] }); + + const ignoredIssues = result.issues.filter( + (i) => i.violation.nodeId === "i:1" + ); + expect(ignoredIssues.length).toBe(0); + }); +}); + +// ─── enabledRules / disabledRules filtering ─────────────────────────────────── + +describe("RuleEngine.analyze — rule filtering", () => { + it("runs only enabledRules when specified", () => { + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [ + makeNode({ id: "f:1", name: "Frame 1", type: "FRAME" }), + ], + }); + const file = makeFile({ document: doc }); + + // Enable only default-name rule + const result = analyzeFile(file, { enabledRules: ["default-name"] }); + + // All issues should be from default-name only + for (const issue of result.issues) { + expect(issue.violation.ruleId).toBe("default-name"); + } + }); + + it("excludes disabledRules", () => { + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [ + makeNode({ id: "f:1", name: "Frame 1", type: "FRAME" }), + ], + }); + const file = makeFile({ document: doc }); + + const resultAll = analyzeFile(file); + const resultDisabled = analyzeFile(file, { disabledRules: ["default-name"] }); + + const defaultNameAll = resultAll.issues.filter( + (i) => i.violation.ruleId === "default-name" + ); + const defaultNameDisabled = resultDisabled.issues.filter( + (i) => i.violation.ruleId === "default-name" + ); + + // default-name issues should be absent when disabled + if (defaultNameAll.length > 0) { + expect(defaultNameDisabled.length).toBe(0); + } + }); + + it("skips rules disabled in config (enabled: false)", () => { + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [ + makeNode({ id: "f:1", name: "Frame", type: "FRAME" }), + ], + }); + const file = makeFile({ document: doc }); + + // no-dev-status is disabled by default in RULE_CONFIGS + const result = analyzeFile(file); + const devStatusIssues = result.issues.filter( + (i) => i.violation.ruleId === "no-dev-status" + ); + expect(devStatusIssues.length).toBe(0); + }); +}); + +// ─── calcDepthWeight ────────────────────────────────────────────────────────── + +describe("RuleEngine.analyze — depth weight calculation", () => { + it("applies higher weight at root level (depthWeight interpolation)", () => { + // Build a tree: root FRAME (depth 0) → child FRAME (depth 1) → grandchild (depth 2) + // Rules with depthWeight should score differently at root vs leaf + const grandchild = makeNode({ id: "gc:1", name: "Frame 2", type: "FRAME" }); + const child = makeNode({ id: "c:1", name: "Frame 3", type: "FRAME", children: [grandchild] }); + const root = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [child], + }); + + const file = makeFile({ document: root }); + // Use only no-auto-layout which has depthWeight: 1.5 and is in "layout" (supports depth weight) + const result = analyzeFile(file, { enabledRules: ["no-auto-layout"] }); + + // Find issues at different depths + const issueAtChild = result.issues.find((i) => i.violation.nodeId === "c:1"); + const issueAtGC = result.issues.find((i) => i.violation.nodeId === "gc:1"); + + if (issueAtChild && issueAtGC) { + // Issue closer to root should have higher absolute score (more negative) + expect(Math.abs(issueAtChild.calculatedScore)).toBeGreaterThanOrEqual( + Math.abs(issueAtGC.calculatedScore) + ); + } + }); +}); + +// ─── Tree traversal / node counting ─────────────────────────────────────────── + +describe("RuleEngine.analyze — tree traversal", () => { + it("counts all nodes in the tree", () => { + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [ + makeNode({ + id: "1:1", + name: "Frame", + type: "FRAME", + children: [ + makeNode({ id: "2:1", name: "Text", type: "TEXT" }), + makeNode({ id: "2:2", name: "Rect", type: "RECTANGLE" }), + ], + }), + ], + }); + + const file = makeFile({ document: doc }); + const result = analyzeFile(file); + + // Document + Frame + Text + Rect = 4 + expect(result.nodeCount).toBe(4); + }); + + it("calculates maxDepth correctly", () => { + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [ + makeNode({ + id: "1:1", + name: "L1", + type: "FRAME", + children: [ + makeNode({ + id: "2:1", + name: "L2", + type: "FRAME", + children: [ + makeNode({ id: "3:1", name: "L3", type: "FRAME" }), + ], + }), + ], + }), + ], + }); + + const file = makeFile({ document: doc }); + const result = analyzeFile(file); + + // Depth: Document=0, L1=1, L2=2, L3=3 → maxDepth = 3 + expect(result.maxDepth).toBe(3); + }); + + it("handles empty document (leaf node)", () => { + const file = makeFile(); + const result = analyzeFile(file); + + expect(result.nodeCount).toBe(1); + expect(result.maxDepth).toBe(0); + expect(result.issues).toBeDefined(); + }); +}); + +// ─── Error handling ─────────────────────────────────────────────────────────── + +describe("RuleEngine.analyze — error resilience", () => { + it("continues analysis when a rule throws an error", () => { + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [ + // Frame 1 is a generic name → should trigger default-name + makeNode({ id: "f:1", name: "Frame 1", type: "FRAME" }), + ], + }); + const file = makeFile({ document: doc }); + + // Spy on console.error to verify error is logged + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + // Even if some rules throw, analysis should complete + const result = analyzeFile(file); + expect(result.issues).toBeDefined(); + expect(result.nodeCount).toBe(2); + + consoleSpy.mockRestore(); + }); +}); + +// ─── analyzeFile convenience function ───────────────────────────────────────── + +describe("analyzeFile", () => { + it("returns a valid AnalysisResult", () => { + const file = makeFile(); + const result = analyzeFile(file); + + expect(result.file).toBe(file); + expect(result.analyzedAt).toBeDefined(); + expect(result.issues).toBeInstanceOf(Array); + expect(typeof result.nodeCount).toBe("number"); + expect(typeof result.maxDepth).toBe("number"); + }); + + it("accepts custom configs", () => { + const doc = makeNode({ + id: "0:1", + name: "Document", + type: "DOCUMENT", + children: [makeNode({ id: "f:1", name: "Frame 1", type: "FRAME" })], + }); + const file = makeFile({ document: doc }); + + // Disable all rules via custom configs + const allDisabled: Record = {} as Record; + for (const [id, config] of Object.entries(RULE_CONFIGS)) { + allDisabled[id as RuleId] = { ...config, enabled: false }; + } + + const result = analyzeFile(file, { configs: allDisabled }); + expect(result.issues.length).toBe(0); + }); +}); diff --git a/src/core/engine/scoring.test.ts b/src/core/engine/scoring.test.ts new file mode 100644 index 00000000..52aec091 --- /dev/null +++ b/src/core/engine/scoring.test.ts @@ -0,0 +1,419 @@ +import { calculateScores, formatScoreSummary, gradeToClassName, getCategoryLabel, getSeverityLabel, buildResultJson } from "./scoring.js"; +import type { AnalysisIssue, AnalysisResult } from "./rule-engine.js"; +import type { AnalysisFile, AnalysisNode } from "../contracts/figma-node.js"; +import type { Rule, RuleConfig, RuleViolation } from "../contracts/rule.js"; +import type { Category } from "../contracts/category.js"; +import type { Severity } from "../contracts/severity.js"; + +// ─── Helpers ────────────────────────────────────────────────────────────────── + +function makeRule(overrides: { id: string; category: Category }): Rule { + return { + definition: { + id: overrides.id, + name: overrides.id, + category: overrides.category, + why: "", + impact: "", + fix: "", + }, + check: () => null, + }; +} + +function makeConfig(severity: Severity, score = -5): RuleConfig { + return { severity, score, enabled: true }; +} + +function makeViolation(ruleId: string): RuleViolation { + return { ruleId, nodeId: "1:1", nodePath: "Root > Node", message: "test" }; +} + +function makeIssue(opts: { + ruleId: string; + category: Category; + severity: Severity; + score?: number; +}): AnalysisIssue { + return { + violation: makeViolation(opts.ruleId), + rule: makeRule({ id: opts.ruleId, category: opts.category }), + config: makeConfig(opts.severity, opts.score ?? -5), + depth: 0, + maxDepth: 5, + calculatedScore: opts.score ?? -5, + }; +} + +function makeResult(issues: AnalysisIssue[], nodeCount = 100): AnalysisResult { + const doc: AnalysisNode = { id: "0:1", name: "Document", type: "DOCUMENT", visible: true }; + const file: AnalysisFile = { + fileKey: "test", + name: "Test", + lastModified: "", + version: "1", + document: doc, + components: {}, + styles: {}, + }; + return { file, issues, maxDepth: 5, nodeCount, analyzedAt: new Date().toISOString() }; +} + +// ─── calculateScores ────────────────────────────────────────────────────────── + +describe("calculateScores", () => { + it("returns 100% for zero issues", () => { + const scores = calculateScores(makeResult([])); + + expect(scores.overall.percentage).toBe(100); + expect(scores.overall.grade).toBe("S"); + expect(scores.summary.totalIssues).toBe(0); + }); + + it("counts issues by severity correctly", () => { + const issues = [ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "group-usage", category: "layout", severity: "risk" }), + makeIssue({ ruleId: "raw-color", category: "token", severity: "missing-info" }), + makeIssue({ ruleId: "numeric-suffix-name", category: "naming", severity: "suggestion" }), + ]; + const scores = calculateScores(makeResult(issues)); + + expect(scores.summary.blocking).toBe(1); + expect(scores.summary.risk).toBe(1); + expect(scores.summary.missingInfo).toBe(1); + expect(scores.summary.suggestion).toBe(1); + expect(scores.summary.totalIssues).toBe(4); + }); + + it("applies severity density weights (blocking=3.0 > risk=2.0 > missing-info=1.0 > suggestion=0.5)", () => { + // Single blocking issue on 100 nodes + const blocking = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + ], 100)); + + // Single suggestion issue on 100 nodes + const suggestion = calculateScores(makeResult([ + makeIssue({ ruleId: "numeric-suffix-name", category: "naming", severity: "suggestion" }), + ], 100)); + + // Blocking issue should reduce layout category score more than suggestion reduces naming + expect(blocking.byCategory.layout.densityScore).toBeLessThan( + suggestion.byCategory.naming.densityScore + ); + }); + + it("density score decreases as weighted issue count increases relative to node count", () => { + const few = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + ], 100)); + + const many = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + ], 100)); + + expect(many.byCategory.layout.densityScore).toBeLessThan( + few.byCategory.layout.densityScore + ); + }); + + it("diversity score penalizes more unique rules being triggered", () => { + // 3 issues from 1 rule (low diversity = high diversity score) + const concentrated = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "risk" }), + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "risk" }), + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "risk" }), + ], 100)); + + // 3 issues from 3 different rules (high diversity = low diversity score) + const spread = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "risk" }), + makeIssue({ ruleId: "group-usage", category: "layout", severity: "risk" }), + makeIssue({ ruleId: "deep-nesting", category: "layout", severity: "risk" }), + ], 100)); + + expect(concentrated.byCategory.layout.diversityScore).toBeGreaterThan( + spread.byCategory.layout.diversityScore + ); + }); + + it("combined score = density * 0.7 + diversity * 0.3", () => { + const issues = [ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "group-usage", category: "layout", severity: "risk" }), + ]; + const scores = calculateScores(makeResult(issues, 100)); + const layout = scores.byCategory.layout; + + const expected = Math.round(layout.densityScore * 0.7 + layout.diversityScore * 0.3); + // Floor is 5, so clamp + const clamped = Math.max(5, Math.min(100, expected)); + expect(layout.percentage).toBe(clamped); + }); + + it("score never goes below SCORE_FLOOR (5) when issues exist", () => { + // To hit the floor, we need both density→0 AND diversity→0 + // Use many issues from many different rules to maximize both penalties + const layoutRules = [ + "no-auto-layout", "group-usage", "deep-nesting", "fixed-size-in-auto-layout", + "missing-responsive-behavior", "absolute-position-in-auto-layout", + "fixed-width-in-responsive-context", "missing-min-width", "missing-max-width", + "overflow-hidden-abuse", "inconsistent-sibling-layout-direction", + ] as const; + + const issues: AnalysisIssue[] = []; + for (const ruleId of layoutRules) { + for (let i = 0; i < 50; i++) { + issues.push(makeIssue({ ruleId, category: "layout", severity: "blocking" })); + } + } + + const scores = calculateScores(makeResult(issues, 10)); + + // With density near 0 and diversity near 0, combined should clamp to floor + expect(scores.byCategory.layout.percentage).toBe(5); + }); + + it("categories without issues get 100%", () => { + // Issues only in layout category + const scores = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + ])); + + expect(scores.byCategory.token.percentage).toBe(100); + expect(scores.byCategory.component.percentage).toBe(100); + expect(scores.byCategory.naming.percentage).toBe(100); + expect(scores.byCategory["ai-readability"].percentage).toBe(100); + expect(scores.byCategory["handoff-risk"].percentage).toBe(100); + }); + + it("overall score is weighted average of all 6 categories", () => { + // With equal weights (all 1.0), overall = average of all category percentages + const scores = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + ], 100)); + + const categoryPercentages = [ + scores.byCategory.layout.percentage, + scores.byCategory.token.percentage, + scores.byCategory.component.percentage, + scores.byCategory.naming.percentage, + scores.byCategory["ai-readability"].percentage, + scores.byCategory["handoff-risk"].percentage, + ]; + const expectedOverall = Math.round( + categoryPercentages.reduce((a, b) => a + b, 0) / 6 + ); + expect(scores.overall.percentage).toBe(expectedOverall); + }); + + it("handles nodeCount = 0 gracefully (no division by zero)", () => { + const scores = calculateScores(makeResult([ + makeIssue({ ruleId: "raw-color", category: "token", severity: "missing-info" }), + ], 0)); + + // With nodeCount 0, density stays at 100 (no density penalty applied) + // But diversity still applies since there are issues + expect(scores.byCategory.token.densityScore).toBe(100); + expect(scores.byCategory.token.percentage).toBeLessThan(100); + expect(Number.isFinite(scores.overall.percentage)).toBe(true); + }); + + it("handles nodeCount = 1 without edge case issues", () => { + const scores = calculateScores(makeResult([ + makeIssue({ ruleId: "raw-color", category: "token", severity: "missing-info" }), + ], 1)); + + expect(Number.isFinite(scores.overall.percentage)).toBe(true); + expect(scores.overall.percentage).toBeGreaterThanOrEqual(0); + expect(scores.overall.percentage).toBeLessThanOrEqual(100); + }); + + it("tracks uniqueRuleCount per category", () => { + const issues = [ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "group-usage", category: "layout", severity: "risk" }), + ]; + const scores = calculateScores(makeResult(issues)); + + // 2 unique rules in layout, despite 3 issues + expect(scores.byCategory.layout.uniqueRuleCount).toBe(2); + expect(scores.byCategory.layout.issueCount).toBe(3); + }); + + it("bySeverity counts are accurate per category", () => { + const issues = [ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "group-usage", category: "layout", severity: "risk" }), + makeIssue({ ruleId: "group-usage", category: "layout", severity: "risk" }), + ]; + const scores = calculateScores(makeResult(issues)); + + expect(scores.byCategory.layout.bySeverity.blocking).toBe(1); + expect(scores.byCategory.layout.bySeverity.risk).toBe(2); + expect(scores.byCategory.layout.bySeverity["missing-info"]).toBe(0); + expect(scores.byCategory.layout.bySeverity.suggestion).toBe(0); + }); +}); + +// ─── Grade boundaries ───────────────────────────────────────────────────────── + +describe("calculateGrade (via calculateScores)", () => { + // Test grade boundaries using carefully constructed inputs + // We create issues across all categories to directly control overall percentage + + function scoreWithPercentage(targetPct: number): string { + // To get an exact overall percentage, we leverage that: + // - overall = average of 6 category percentages + // - categories without issues = 100% + // So if we make 1 category have a specific score and 5 have 100%, + // overall = (targetCatPct + 5*100) / 6 + // targetCatPct = targetPct * 6 - 500 + // But targetCatPct must be >= 5 (floor) and <= 100 + // For overall >= 85: targetCatPct = targetPct * 6 - 500 + // For overall 95: catPct = 70, overall = (70+500)/6 = 95 + // For overall 90: catPct = 40, overall = (40+500)/6 = 90 + // These won't always be reachable, so we test what we can + + // Simple approach: create result with zero issues for a clean 100% = S grade + // Then test boundaries via the grade output + const result = makeResult([], 100); + const scores = calculateScores(result); + return scores.overall.grade; + } + + it("100% -> S", () => { + const scores = calculateScores(makeResult([], 100)); + expect(scores.overall.grade).toBe("S"); + }); + + it("score < 50% -> F", () => { + // Many blocking issues in all categories to push overall below 50% + const issues: AnalysisIssue[] = []; + const categories: Category[] = ["layout", "token", "component", "naming", "ai-readability", "handoff-risk"]; + const rulesPerCat: Record = { + layout: ["no-auto-layout", "group-usage", "deep-nesting", "fixed-size-in-auto-layout", "missing-responsive-behavior"], + token: ["raw-color", "raw-font", "inconsistent-spacing", "magic-number-spacing", "raw-shadow"], + component: ["missing-component", "detached-instance", "variant-not-used", "component-property-unused", "single-use-component"], + naming: ["default-name", "non-semantic-name", "inconsistent-naming-convention", "numeric-suffix-name", "too-long-name"], + "ai-readability": ["ambiguous-structure", "z-index-dependent-layout", "missing-layout-hint", "invisible-layer", "empty-frame"], + "handoff-risk": ["hardcode-risk", "text-truncation-unhandled", "image-no-placeholder", "prototype-link-in-design", "no-dev-status"], + }; + + for (const cat of categories) { + for (const ruleId of rulesPerCat[cat]!) { + for (let i = 0; i < 20; i++) { + issues.push(makeIssue({ ruleId, category: cat, severity: "blocking" })); + } + } + } + + const scores = calculateScores(makeResult(issues, 10)); + expect(scores.overall.grade).toBe("F"); + expect(scores.overall.percentage).toBeLessThan(50); + }); + + it("gradeToClassName converts + to plus", () => { + expect(gradeToClassName("A+")).toBe("Aplus"); + expect(gradeToClassName("B+")).toBe("Bplus"); + expect(gradeToClassName("C+")).toBe("Cplus"); + expect(gradeToClassName("S")).toBe("S"); + expect(gradeToClassName("F")).toBe("F"); + }); +}); + +// ─── formatScoreSummary ─────────────────────────────────────────────────────── + +describe("formatScoreSummary", () => { + it("includes overall grade and percentage", () => { + const scores = calculateScores(makeResult([])); + const summary = formatScoreSummary(scores); + + expect(summary).toContain("Overall: S (100%)"); + }); + + it("includes all 6 categories", () => { + const scores = calculateScores(makeResult([])); + const summary = formatScoreSummary(scores); + + expect(summary).toContain("layout:"); + expect(summary).toContain("token:"); + expect(summary).toContain("component:"); + expect(summary).toContain("naming:"); + expect(summary).toContain("ai-readability:"); + expect(summary).toContain("handoff-risk:"); + }); + + it("includes severity breakdown", () => { + const scores = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + ])); + const summary = formatScoreSummary(scores); + + expect(summary).toContain("Blocking: 1"); + expect(summary).toContain("Total: 1"); + }); +}); + +// ─── getCategoryLabel / getSeverityLabel ─────────────────────────────────────── + +describe("getCategoryLabel", () => { + it("returns correct labels for all categories", () => { + expect(getCategoryLabel("layout")).toBe("Layout"); + expect(getCategoryLabel("token")).toBe("Design Token"); + expect(getCategoryLabel("component")).toBe("Component"); + expect(getCategoryLabel("naming")).toBe("Naming"); + expect(getCategoryLabel("ai-readability")).toBe("AI Readability"); + expect(getCategoryLabel("handoff-risk")).toBe("Handoff Risk"); + }); +}); + +describe("getSeverityLabel", () => { + it("returns correct labels for all severities", () => { + expect(getSeverityLabel("blocking")).toBe("Blocking"); + expect(getSeverityLabel("risk")).toBe("Risk"); + expect(getSeverityLabel("missing-info")).toBe("Missing Info"); + expect(getSeverityLabel("suggestion")).toBe("Suggestion"); + }); +}); + +// ─── buildResultJson ────────────────────────────────────────────────────────── + +describe("buildResultJson", () => { + it("includes all expected fields", () => { + const result = makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "raw-color", category: "token", severity: "missing-info" }), + ]); + const scores = calculateScores(result); + const json = buildResultJson("TestFile", result, scores); + + expect(json.fileName).toBe("TestFile"); + expect(json.nodeCount).toBe(100); + expect(json.issueCount).toBe(3); + expect(json.version).toBeDefined(); + expect(json.scores).toBeDefined(); + expect(json.summary).toBeDefined(); + expect(typeof json.summary).toBe("string"); + }); + + it("aggregates issuesByRule correctly", () => { + const result = makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "no-auto-layout", category: "layout", severity: "blocking" }), + makeIssue({ ruleId: "raw-color", category: "token", severity: "missing-info" }), + ]); + const scores = calculateScores(result); + const json = buildResultJson("TestFile", result, scores); + const issuesByRule = json.issuesByRule as Record; + + expect(issuesByRule["no-auto-layout"]).toBe(2); + expect(issuesByRule["raw-color"]).toBe(1); + }); +}); From e4fb283048eb60a61efa335a4970c888f4c98928 Mon Sep 17 00:00:00 2001 From: let-sunny Date: Wed, 25 Mar 2026 20:13:59 +0900 Subject: [PATCH 2/4] fix: address CodeRabbit review feedback - Remove dead code: unused scoreWithPercentage helper in scoring.test.ts - Replace non-null assertion (rulesPerCat[cat]!) with safe lookup + continue - Add explicit toBeDefined() assertions for depthWeight test to prevent vacuous pass; fix test tree so grandchild FRAME has children (required by no-auto-layout rule) - Inject actual error in resilience test: spy on default-name rule's check to throw, verify console.error is called, restore original Co-Authored-By: Claude Opus 4.6 (1M context) --- src/core/engine/rule-engine.test.ts | 53 +++++++++++++++++++---------- src/core/engine/scoring.test.ts | 27 ++------------- 2 files changed, 38 insertions(+), 42 deletions(-) diff --git a/src/core/engine/rule-engine.test.ts b/src/core/engine/rule-engine.test.ts index b7363b5d..f3245e43 100644 --- a/src/core/engine/rule-engine.test.ts +++ b/src/core/engine/rule-engine.test.ts @@ -2,6 +2,7 @@ import { RuleEngine, analyzeFile } from "./rule-engine.js"; import type { AnalysisFile, AnalysisNode } from "../contracts/figma-node.js"; import type { RuleConfig, RuleId } from "../contracts/rule.js"; import { RULE_CONFIGS } from "../rules/rule-config.js"; +import { ruleRegistry } from "../rules/rule-registry.js"; // Import rules to register import "../rules/index.js"; @@ -257,10 +258,10 @@ describe("RuleEngine.analyze — rule filtering", () => { describe("RuleEngine.analyze — depth weight calculation", () => { it("applies higher weight at root level (depthWeight interpolation)", () => { - // Build a tree: root FRAME (depth 0) → child FRAME (depth 1) → grandchild (depth 2) - // Rules with depthWeight should score differently at root vs leaf - const grandchild = makeNode({ id: "gc:1", name: "Frame 2", type: "FRAME" }); - const child = makeNode({ id: "c:1", name: "Frame 3", type: "FRAME", children: [grandchild] }); + // no-auto-layout requires FRAME with children to trigger, so every level needs a child + const leaf = makeNode({ id: "leaf:1", name: "Leaf", type: "RECTANGLE" }); + const grandchild = makeNode({ id: "gc:1", name: "GC Frame", type: "FRAME", children: [leaf] }); + const child = makeNode({ id: "c:1", name: "Child Frame", type: "FRAME", children: [grandchild] }); const root = makeNode({ id: "0:1", name: "Document", @@ -272,16 +273,17 @@ describe("RuleEngine.analyze — depth weight calculation", () => { // Use only no-auto-layout which has depthWeight: 1.5 and is in "layout" (supports depth weight) const result = analyzeFile(file, { enabledRules: ["no-auto-layout"] }); - // Find issues at different depths + // Find issues at different depths — assert they exist to avoid vacuous pass const issueAtChild = result.issues.find((i) => i.violation.nodeId === "c:1"); const issueAtGC = result.issues.find((i) => i.violation.nodeId === "gc:1"); - if (issueAtChild && issueAtGC) { - // Issue closer to root should have higher absolute score (more negative) - expect(Math.abs(issueAtChild.calculatedScore)).toBeGreaterThanOrEqual( - Math.abs(issueAtGC.calculatedScore) - ); - } + expect(issueAtChild).toBeDefined(); + expect(issueAtGC).toBeDefined(); + + // Issue closer to root should have higher absolute score (more negative) + expect(Math.abs(issueAtChild!.calculatedScore)).toBeGreaterThanOrEqual( + Math.abs(issueAtGC!.calculatedScore) + ); }); }); @@ -363,21 +365,36 @@ describe("RuleEngine.analyze — error resilience", () => { name: "Document", type: "DOCUMENT", children: [ - // Frame 1 is a generic name → should trigger default-name makeNode({ id: "f:1", name: "Frame 1", type: "FRAME" }), ], }); const file = makeFile({ document: doc }); - // Spy on console.error to verify error is logged + // Make an existing rule's check throw to verify error resilience + const defaultNameRule = ruleRegistry.get("default-name"); + expect(defaultNameRule).toBeDefined(); + + const originalCheck = defaultNameRule!.check; + defaultNameRule!.check = () => { throw new Error("boom"); }; + const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - // Even if some rules throw, analysis should complete - const result = analyzeFile(file); - expect(result.issues).toBeDefined(); - expect(result.nodeCount).toBe(2); + try { + // Analysis should still complete despite the throwing rule + const result = analyzeFile(file); + expect(result.issues).toBeDefined(); + expect(result.nodeCount).toBe(2); - consoleSpy.mockRestore(); + // Verify the error was logged + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining("default-name"), + expect.any(Error) + ); + } finally { + // Restore the original check function and console spy + defaultNameRule!.check = originalCheck; + consoleSpy.mockRestore(); + } }); }); diff --git a/src/core/engine/scoring.test.ts b/src/core/engine/scoring.test.ts index 52aec091..1354aa5f 100644 --- a/src/core/engine/scoring.test.ts +++ b/src/core/engine/scoring.test.ts @@ -264,29 +264,6 @@ describe("calculateScores", () => { // ─── Grade boundaries ───────────────────────────────────────────────────────── describe("calculateGrade (via calculateScores)", () => { - // Test grade boundaries using carefully constructed inputs - // We create issues across all categories to directly control overall percentage - - function scoreWithPercentage(targetPct: number): string { - // To get an exact overall percentage, we leverage that: - // - overall = average of 6 category percentages - // - categories without issues = 100% - // So if we make 1 category have a specific score and 5 have 100%, - // overall = (targetCatPct + 5*100) / 6 - // targetCatPct = targetPct * 6 - 500 - // But targetCatPct must be >= 5 (floor) and <= 100 - // For overall >= 85: targetCatPct = targetPct * 6 - 500 - // For overall 95: catPct = 70, overall = (70+500)/6 = 95 - // For overall 90: catPct = 40, overall = (40+500)/6 = 90 - // These won't always be reachable, so we test what we can - - // Simple approach: create result with zero issues for a clean 100% = S grade - // Then test boundaries via the grade output - const result = makeResult([], 100); - const scores = calculateScores(result); - return scores.overall.grade; - } - it("100% -> S", () => { const scores = calculateScores(makeResult([], 100)); expect(scores.overall.grade).toBe("S"); @@ -306,7 +283,9 @@ describe("calculateGrade (via calculateScores)", () => { }; for (const cat of categories) { - for (const ruleId of rulesPerCat[cat]!) { + const rules = rulesPerCat[cat]; + if (!rules) continue; + for (const ruleId of rules) { for (let i = 0; i < 20; i++) { issues.push(makeIssue({ ruleId, category: cat, severity: "blocking" })); } From f7a3c4e56712cfe1d6d47205bf909409c9e59e24 Mon Sep 17 00:00:00 2001 From: let-sunny Date: Wed, 25 Mar 2026 20:21:57 +0900 Subject: [PATCH 3/4] fix: strengthen filter tests to prevent vacuous passes Replace conditional assertions (if baseline > 0) with explicit baseline assertions (expect > 0) in excludeNodeTypes and disabledRules tests, ensuring the filter behavior is always validated. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/core/engine/rule-engine.test.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/core/engine/rule-engine.test.ts b/src/core/engine/rule-engine.test.ts index f3245e43..5626ad93 100644 --- a/src/core/engine/rule-engine.test.ts +++ b/src/core/engine/rule-engine.test.ts @@ -157,11 +157,9 @@ describe("RuleEngine.analyze — node exclusion", () => { (i) => i.violation.nodeId === "t:1" ); - expect(textIssuesExcluded.length).toBeLessThanOrEqual(textIssuesAll.length); - // If TEXT generated any issues, they should be filtered out - if (textIssuesAll.length > 0) { - expect(textIssuesExcluded.length).toBe(0); - } + // Baseline must have issues from TEXT node to validate the filter + expect(textIssuesAll.length).toBeGreaterThan(0); + expect(textIssuesExcluded.length).toBe(0); }); it("skips nodes matching excludeNodeNames pattern", () => { @@ -228,10 +226,9 @@ describe("RuleEngine.analyze — rule filtering", () => { (i) => i.violation.ruleId === "default-name" ); - // default-name issues should be absent when disabled - if (defaultNameAll.length > 0) { - expect(defaultNameDisabled.length).toBe(0); - } + // Baseline must have default-name issues to validate the filter + expect(defaultNameAll.length).toBeGreaterThan(0); + expect(defaultNameDisabled.length).toBe(0); }); it("skips rules disabled in config (enabled: false)", () => { From 12407d08be19515e26f81ea8828cf1b779591b54 Mon Sep 17 00:00:00 2001 From: let-sunny Date: Wed, 25 Mar 2026 20:28:50 +0900 Subject: [PATCH 4/4] fix: add baseline assertions to all filter tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - excludeNodeNames: add baseline run to prove node generates issues before verifying exclusion filters them out - enabledRules: assert issues.length > 0 before loop to prevent vacuous pass on empty result - no-dev-status: add positive control — explicitly enable the rule to prove it fires, then verify default config disables it Co-Authored-By: Claude Opus 4.6 (1M context) --- src/core/engine/rule-engine.test.ts | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/core/engine/rule-engine.test.ts b/src/core/engine/rule-engine.test.ts index 5626ad93..ce1a0527 100644 --- a/src/core/engine/rule-engine.test.ts +++ b/src/core/engine/rule-engine.test.ts @@ -173,8 +173,16 @@ describe("RuleEngine.analyze — node exclusion", () => { }); const file = makeFile({ document: doc }); - const result = analyzeFile(file, { excludeNodeNames: ["IgnoreMe"] }); + // Baseline: verify ignored node produces issues without exclusion + const baseline = analyzeFile(file); + const ignoredIssuesBaseline = baseline.issues.filter( + (i) => i.violation.nodeId === "i:1" + ); + expect(ignoredIssuesBaseline.length).toBeGreaterThan(0); + + // With exclusion: issues from that node should be absent + const result = analyzeFile(file, { excludeNodeNames: ["IgnoreMe"] }); const ignoredIssues = result.issues.filter( (i) => i.violation.nodeId === "i:1" ); @@ -199,6 +207,9 @@ describe("RuleEngine.analyze — rule filtering", () => { // Enable only default-name rule const result = analyzeFile(file, { enabledRules: ["default-name"] }); + // Must have at least one issue to avoid vacuous pass + expect(result.issues.length).toBeGreaterThan(0); + // All issues should be from default-name only for (const issue of result.issues) { expect(issue.violation.ruleId).toBe("default-name"); @@ -242,7 +253,16 @@ describe("RuleEngine.analyze — rule filtering", () => { }); const file = makeFile({ document: doc }); - // no-dev-status is disabled by default in RULE_CONFIGS + // Positive control: explicitly enable no-dev-status to prove it can fire + const enabledConfigs = { ...RULE_CONFIGS }; + enabledConfigs["no-dev-status"] = { ...enabledConfigs["no-dev-status"], enabled: true }; + const resultEnabled = analyzeFile(file, { configs: enabledConfigs }); + const devStatusEnabled = resultEnabled.issues.filter( + (i) => i.violation.ruleId === "no-dev-status" + ); + expect(devStatusEnabled.length).toBeGreaterThan(0); + + // Default config: no-dev-status is disabled → no issues const result = analyzeFile(file); const devStatusIssues = result.issues.filter( (i) => i.violation.ruleId === "no-dev-status"