diff --git a/src/cli/commands/analyze.ts b/src/cli/commands/analyze.ts index 4f9fcf96..bee57333 100644 --- a/src/cli/commands/analyze.ts +++ b/src/cli/commands/analyze.ts @@ -154,8 +154,8 @@ export function registerAnalyze(cli: CAC): void { const result = analyzeFile(file, analyzeOptions); log(`Nodes: ${result.nodeCount} (max depth: ${result.maxDepth})`); - // Calculate scores - const scores = calculateScores(result); + // Calculate scores using the same preset-adjusted configs + const scores = calculateScores(result, configs as Record); // JSON output mode — only JSON goes to stdout; exit code still applies if (options.json) { diff --git a/src/core/engine/scoring.test.ts b/src/core/engine/scoring.test.ts index cee9eea2..8b4a792e 100644 --- a/src/core/engine/scoring.test.ts +++ b/src/core/engine/scoring.test.ts @@ -87,33 +87,41 @@ describe("calculateScores", () => { }); it("uses calculatedScore for density: higher score = more density impact", () => { - const heavy = calculateScores(makeResult([ - makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "blocking", score: -10 }), - ], 100)); + // Create issue where calculatedScore differs from config.score + const heavyIssue = makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "blocking", score: -10 }); + heavyIssue.calculatedScore = -15; // Simulate depthWeight effect - const light = calculateScores(makeResult([ - makeIssue({ ruleId: "unnecessary-node", category: "structure", severity: "suggestion", score: -2 }), - ], 100)); + const lightIssue = makeIssue({ ruleId: "unnecessary-node", category: "structure", severity: "suggestion", score: -2 }); + lightIssue.calculatedScore = -2; // No depthWeight + const heavy = calculateScores(makeResult([heavyIssue], 100)); + const light = calculateScores(makeResult([lightIssue], 100)); + + // Density should use calculatedScore (-15 vs -2), not config.score + expect(heavy.byCategory.structure.weightedIssueCount).toBe(15); + expect(light.byCategory.structure.weightedIssueCount).toBe(2); expect(heavy.byCategory.structure.densityScore).toBeLessThan( light.byCategory.structure.densityScore ); }); it("differentiates rules within the same severity by score", () => { - const highScore = calculateScores(makeResult([ - makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "blocking", score: -10 }), - ], 100)); + // Create issues where calculatedScore differs from config.score + const highScoreIssue = makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "blocking", score: -10 }); + highScoreIssue.calculatedScore = -15; // Simulate depthWeight effect - const lowScore = calculateScores(makeResult([ - makeIssue({ ruleId: "absolute-position-in-auto-layout", category: "structure", severity: "blocking", score: -3 }), - ], 100)); + const lowScoreIssue = makeIssue({ ruleId: "absolute-position-in-auto-layout", category: "structure", severity: "blocking", score: -3 }); + lowScoreIssue.calculatedScore = -5; // Simulate different depthWeight + + const highScore = calculateScores(makeResult([highScoreIssue], 100)); + const lowScore = calculateScores(makeResult([lowScoreIssue], 100)); + // weightedIssueCount should use calculatedScore, not config.score expect(highScore.byCategory.structure.densityScore).toBeLessThan( lowScore.byCategory.structure.densityScore ); - expect(highScore.byCategory.structure.weightedIssueCount).toBe(10); - expect(lowScore.byCategory.structure.weightedIssueCount).toBe(3); + expect(highScore.byCategory.structure.weightedIssueCount).toBe(15); + expect(lowScore.byCategory.structure.weightedIssueCount).toBe(5); }); it("density score decreases as weighted issue count increases relative to node count", () => { @@ -136,15 +144,15 @@ describe("calculateScores", () => { it("diversity score penalizes more unique rules being triggered", () => { const concentrated = calculateScores(makeResult([ - makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "risk" }), - makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "risk" }), - makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "risk" }), + makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "risk", score: -5 }), + makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "risk", score: -5 }), + makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "risk", score: -5 }), ], 100)); const spread = calculateScores(makeResult([ - makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "risk" }), - makeIssue({ ruleId: "group-usage", category: "structure", severity: "risk" }), - makeIssue({ ruleId: "deep-nesting", category: "structure", severity: "risk" }), + makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "risk", score: -5 }), + makeIssue({ ruleId: "group-usage", category: "structure", severity: "risk", score: -5 }), + makeIssue({ ruleId: "deep-nesting", category: "structure", severity: "risk", score: -5 }), ], 100)); expect(concentrated.byCategory.structure.diversityScore).toBeGreaterThan( @@ -152,6 +160,39 @@ describe("calculateScores", () => { ); }); + it("diversity weights triggered rules by score severity", () => { + // One high-severity rule triggered + const heavyRule = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "blocking", score: -10 }), + ], 100)); + + // One low-severity rule triggered + const lightRule = calculateScores(makeResult([ + makeIssue({ ruleId: "unnecessary-node", category: "structure", severity: "suggestion", score: -2 }), + ], 100)); + + expect(heavyRule.byCategory.structure.diversityScore).toBeLessThan( + lightRule.byCategory.structure.diversityScore + ); + }); + + it("low-severity rules have minimal diversity impact (intentional)", () => { + // 1 suggestion rule (score -2) vs 1 blocking rule (score -10). + // Weighted ratio for the suggestion is much smaller, so diversity stays high. + // Low-severity rules correctly penalize diversity less. + const lowSeverity = calculateScores(makeResult([ + makeIssue({ ruleId: "unnecessary-node", category: "structure", severity: "suggestion", score: -2 }), + ], 100)); + + const highSeverity = calculateScores(makeResult([ + makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "blocking", score: -10 }), + ], 100)); + + // Both trigger exactly 1 rule, but severity-weighted diversity differs significantly + expect(lowSeverity.byCategory.structure.diversityScore).toBeGreaterThan(90); + expect(highSeverity.byCategory.structure.diversityScore).toBeLessThan(80); + }); + it("combined score = density * 0.7 + diversity * 0.3", () => { const issues = [ makeIssue({ ruleId: "no-auto-layout", category: "structure", severity: "blocking" }), @@ -424,4 +465,4 @@ describe("buildResultJson", () => { const withoutKey = buildResultJson("TestFile", result, scores); expect(withoutKey.fileKey).toBeUndefined(); }); -}); +}); \ No newline at end of file diff --git a/src/core/engine/scoring.ts b/src/core/engine/scoring.ts index c2c47542..6f8e5bb0 100644 --- a/src/core/engine/scoring.ts +++ b/src/core/engine/scoring.ts @@ -1,7 +1,9 @@ import type { Category } from "../contracts/category.js"; import { CATEGORIES } from "../contracts/category.js"; +import type { RuleId, RuleConfig } from "../contracts/rule.js"; import type { Severity } from "../contracts/severity.js"; import type { AnalysisResult } from "./rule-engine.js"; +import { RULE_CONFIGS, RULE_ID_CATEGORY } from "../rules/rule-config.js"; import { version as VERSION } from "../../../package.json"; /** @@ -60,16 +62,27 @@ export type Grade = "S" | "A+" | "A" | "B+" | "B" | "C+" | "C" | "D" | "F"; */ /** - * Total rules per category — used as denominator for diversity scoring. - * Must be updated when rules are added/removed from a category. + * Compute sum of |score| for all rules in each category from a given config map. + * Used as denominator for severity-weighted diversity scoring. + * Must use the same preset-adjusted config map that produced the analysis issues, + * otherwise diversity ratios will be incorrect. */ -const TOTAL_RULES_PER_CATEGORY: Record = { - structure: 9, - token: 7, - component: 4, - naming: 5, - behavior: 4, -}; +function computeTotalScorePerCategory( + configs: Record +): Record { + const totals = Object.fromEntries( + CATEGORIES.map(c => [c, 0]) + ) as Record; + + for (const [id, config] of Object.entries(configs)) { + const category = RULE_ID_CATEGORY[id as RuleId]; + if (category && config.enabled) { + totals[category] += Math.abs(config.score); + } + } + + return totals; +} /** * Category weights for overall score. @@ -151,20 +164,35 @@ function clamp(value: number, min: number, max: number): number { * Calculate scores from analysis result using density + diversity scoring * * Density Score = 100 - (weighted issue count / node count) * 100 - * Diversity Score = (1 - unique rules / total category rules) * 100 + * Diversity Score = (1 - weighted triggered rule scores / total category scores) * 100 * Final Score = density * 0.7 + diversity * 0.3 + * + * @param result Analysis result with issues + * @param configs Optional preset-adjusted config map used to produce the issues. + * If not provided, diversity totals are reconstructed from issue.config values. */ -export function calculateScores(result: AnalysisResult): ScoreReport { +export function calculateScores( + result: AnalysisResult, + configs?: Record +): ScoreReport { const categoryScores = initializeCategoryScores(); const nodeCount = result.nodeCount; - // Track unique rules per category + // Track unique rules and their base |score| per category const uniqueRulesPerCategory = new Map>(); + const ruleScorePerCategory = new Map>(); for (const category of CATEGORIES) { uniqueRulesPerCategory.set(category, new Set()); + ruleScorePerCategory.set(category, new Map()); } - // Count issues by severity per category and track unique rules + // Compute totals from the config map. + // If configs provided: use preset-adjusted totals (recommended when using presets). + // If not: fall back to static RULE_CONFIGS — only correct when issues were + // produced with default RULE_CONFIGS, otherwise diversity ratios will be skewed. + const totalScorePerCategory = computeTotalScorePerCategory(configs ?? RULE_CONFIGS); + + // Count issues by severity per category and track unique rules with scores for (const issue of result.issues) { const category = issue.rule.definition.category; const severity = issue.config.severity; @@ -174,13 +202,13 @@ export function calculateScores(result: AnalysisResult): ScoreReport { categoryScores[category].bySeverity[severity]++; categoryScores[category].weightedIssueCount += Math.abs(issue.calculatedScore); uniqueRulesPerCategory.get(category)!.add(ruleId); + ruleScorePerCategory.get(category)!.set(ruleId, Math.abs(issue.config.score)); } // Calculate percentage for each category based on density + diversity for (const category of CATEGORIES) { const catScore = categoryScores[category]; const uniqueRules = uniqueRulesPerCategory.get(category)!; - const totalRules = TOTAL_RULES_PER_CATEGORY[category]; catScore.uniqueRuleCount = uniqueRules.size; @@ -192,11 +220,17 @@ export function calculateScores(result: AnalysisResult): ScoreReport { } catScore.densityScore = densityScore; - // Diversity score: fewer unique rules = higher score (issues are concentrated) - // If no issues, diversity score is 100 (perfect) + // Diversity score: weighted by base rule |score| (config.score, not calculatedScore). + // Uses base score intentionally — diversity measures "what types of problems exist", + // not "where they occur". depthWeight affects density (volume penalty) but not diversity + // (breadth penalty). A blocking rule (score -10) penalizes diversity more than a + // suggestion (score -1), so low-severity-only designs correctly get high diversity scores. let diversityScore = 100; if (catScore.issueCount > 0) { - const diversityRatio = uniqueRules.size / totalRules; + const ruleScores = ruleScorePerCategory.get(category)!; + const weightedTriggered = Array.from(ruleScores.values()).reduce((sum, s) => sum + s, 0); + const weightedTotal = totalScorePerCategory[category]; + const diversityRatio = weightedTotal > 0 ? weightedTriggered / weightedTotal : 0; diversityScore = clamp(Math.round((1 - diversityRatio) * 100), 0, 100); } catScore.diversityScore = diversityScore; @@ -392,4 +426,4 @@ export function buildResultJson( } return json; -} +} \ No newline at end of file diff --git a/src/core/rules/rule-config.ts b/src/core/rules/rule-config.ts index a60aaa27..0018ef7a 100644 --- a/src/core/rules/rule-config.ts +++ b/src/core/rules/rule-config.ts @@ -1,5 +1,42 @@ +import type { Category } from "../contracts/category.js"; import type { RuleConfig, RuleId } from "../contracts/rule.js"; +/** + * Maps each rule ID to its category. Kept alongside RULE_CONFIGS so both + * are updated together when rules are added or removed. + */ +export const RULE_ID_CATEGORY: Record = { + "no-auto-layout": "structure", + "absolute-position-in-auto-layout": "structure", + "fixed-size-in-auto-layout": "structure", + "missing-size-constraint": "structure", + "missing-responsive-behavior": "structure", + "group-usage": "structure", + "deep-nesting": "structure", + "z-index-dependent-layout": "structure", + "unnecessary-node": "structure", + "raw-color": "token", + "raw-font": "token", + "inconsistent-spacing": "token", + "magic-number-spacing": "token", + "raw-shadow": "token", + "raw-opacity": "token", + "multiple-fill-colors": "token", + "missing-component": "component", + "detached-instance": "component", + "missing-component-description": "component", + "variant-structure-mismatch": "component", + "default-name": "naming", + "non-semantic-name": "naming", + "inconsistent-naming-convention": "naming", + "numeric-suffix-name": "naming", + "too-long-name": "naming", + "text-truncation-unhandled": "behavior", + "prototype-link-in-design": "behavior", + "overflow-behavior-unknown": "behavior", + "wrap-behavior-unknown": "behavior", +}; + /** * Central configuration for all rules * Edit scores/severity here without touching rule logic diff --git a/src/mcp/server.ts b/src/mcp/server.ts index bb42b053..e19dc16d 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -123,7 +123,7 @@ IMPORTANT — Before calling this tool, check which data source is available: ...(effectiveNodeId ? { targetNodeId: effectiveNodeId } : {}), }); - const scores = calculateScores(result); + const scores = calculateScores(result, configs as Record); // Generate HTML report (with Figma token for comment buttons) const figmaToken = token ?? process.env["FIGMA_TOKEN"];