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
4 changes: 2 additions & 2 deletions src/cli/commands/analyze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RuleId, RuleConfig>);

// JSON output mode — only JSON goes to stdout; exit code still applies
if (options.json) {
Expand Down
83 changes: 62 additions & 21 deletions src/core/engine/scoring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -136,22 +144,55 @@ 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(
spread.byCategory.structure.diversityScore
);
});

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" }),
Expand Down Expand Up @@ -424,4 +465,4 @@ describe("buildResultJson", () => {
const withoutKey = buildResultJson("TestFile", result, scores);
expect(withoutKey.fileKey).toBeUndefined();
});
});
});
70 changes: 52 additions & 18 deletions src/core/engine/scoring.ts
Original file line number Diff line number Diff line change
@@ -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";

/**
Expand Down Expand Up @@ -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<Category, number> = {
structure: 9,
token: 7,
component: 4,
naming: 5,
behavior: 4,
};
function computeTotalScorePerCategory(
configs: Record<RuleId, RuleConfig>
): Record<Category, number> {
const totals = Object.fromEntries(
CATEGORIES.map(c => [c, 0])
) as Record<Category, number>;

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;
Comment on lines +70 to +84
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Include custom rules in the diversity denominator.

RULE_ID_CATEGORY only covers built-in IDs, but both updated call sites now merge custom rule configs before invoking calculateScores. Any enabled custom rule is skipped here, while Line 205 still counts it once triggered, so weightedTotal and weightedTriggered stop describing the same rule set. That over-penalizes custom rules and can even leave diversity at 100 when a category only has custom rules enabled. Build the denominator from the active rule definitions/category map, not just the built-in lookup.

Also applies to: 205-205

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/engine/scoring.ts` around lines 70 - 84,
computeTotalScorePerCategory currently uses the built-in RULE_ID_CATEGORY lookup
so any custom rules merged into configs are ignored; update
computeTotalScorePerCategory to derive the category per rule id from the active
rule definitions/category map used by calculateScores (e.g.,
RULE_DEFINITIONS[id].category or an activeRuleCategoryMap passed in) instead of
RULE_ID_CATEGORY, and include enabled custom rules' Math.abs(config.score) in
the totals so weightedTotal/weightedTriggered operate over the same rule set.

}

/**
* Category weights for overall score.
Expand Down Expand Up @@ -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<RuleId, RuleConfig>
): 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<Category, Set<string>>();
const ruleScorePerCategory = new Map<Category, Map<string, number>>();
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;
Expand All @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -392,4 +426,4 @@ export function buildResultJson(
}

return json;
}
}
37 changes: 37 additions & 0 deletions src/core/rules/rule-config.ts
Original file line number Diff line number Diff line change
@@ -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<RuleId, Category> = {
"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
Expand Down
2 changes: 1 addition & 1 deletion src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RuleId, RuleConfig>);

// Generate HTML report (with Figma token for comment buttons)
const figmaToken = token ?? process.env["FIGMA_TOKEN"];
Expand Down
Loading