feat: Architecture category — component-size metrics (ARCH001/ARCH002) - #71
Merged
Merged
Conversation
…H002) Third "Svelte Doctor" code-health category, reusing the component-body scan (CLI/static). Deterministic, high-precision size metrics for bloated components: - ARCH001 component size: flags a .svelte file over 400 lines (info). - ARCH002 prop count: flags >10 props destructured from $props() (info). ComponentFacts gains loc + propCount (source line count; named-prop count from a $props() destructure, 0 when rest/non-destructured). componentRule's category widens to include 'architecture'; console reporter shows an Architecture line. Taken before the reactivity slice (those heuristics are lower-precision / partly compiler-covered, which conflicts with the no-false-positive principle). Docs (en+ja), changeset, spec, tests added. pnpm -r test 517 green; typecheck, lint, docs build (105 pages) green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a new ChangesArchitecture Category: ARCH001 / ARCH002
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/providers/source/components.ts (1)
13-17: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve
locwhen parsing fails after the file was read.Because the
trywraps bothreadFileandparseComponentFacts, any parser error dropslocback to0. ARCH001 only needs the raw source text, so a syntactically broken or parser-unsupported component will disappear from the architecture report instead of still being flagged by size.Suggested fix
return Promise.all( files.sort().map(async (rel): Promise<ComponentFacts> => { + let source: string | null = null; try { - const source = await rt.readFile(rt.join(cwd, rel)); + source = await rt.readFile(rt.join(cwd, rel)); return { file: rel, ...parseComponentFacts(source, rel) }; } catch { - return { file: rel, eachBlocks: [], effects: [], htmlTags: [], javascriptUrls: [], loc: 0, propCount: 0 }; + const loc = source === null ? 0 : source.split('\n').length; + return { file: rel, eachBlocks: [], effects: [], htmlTags: [], javascriptUrls: [], loc, propCount: 0 }; } }) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/providers/source/components.ts` around lines 13 - 17, The fallback in the source component reader is resetting metadata when parseComponentFacts fails after a successful read, so preserve the raw-file-derived loc value instead of defaulting to 0. Update the try/catch in components.ts around readFile and parseComponentFacts so that parse errors still return the file’s loc (and any other already-known raw metrics) while only the parse-specific fields fall back to empty values. Use the existing source-reading flow and parseComponentFacts helper to keep ARCH001 reporting working for broken or unsupported components.
🧹 Nitpick comments (1)
packages/cli/test/docs-links.test.ts (1)
11-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid a category allowlist here.
This still has to be updated every time a new category lands; if someone forgets, the docs-links test silently stops covering that slice. Since this test is validating rule docs coverage, prefer checking
allRulesdirectly or asserting that this set exactly matches the categories present inallRules.Simple option
-// Every rule links its findings to our own docs, so every category has reference pages. -const DOCUMENTED_CATEGORIES = new Set(['seo', 'performance', 'correctness', 'security', 'architecture']); -const documented = allRules.filter((r) => DOCUMENTED_CATEGORIES.has(r.category)); +// Every rule links its findings to our own docs, so every rule should have a reference page. +const documented = allRules;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/test/docs-links.test.ts` around lines 11 - 13, The docs-links test is using a hardcoded category allowlist, which can go stale when new rule categories are added. Update the test around DOCUMENTED_CATEGORIES/documented to derive coverage from allRules directly, or assert that the documented categories set exactly matches the categories present in allRules, so docs coverage stays in sync automatically.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/providers/source/parse.ts`:
- Around line 457-465: countProps currently keeps the last successful $props()
destructuring count, which can leave a stale nonzero result after an uncountable
shape. Update countProps in parse.ts so that when walkEstree sees any
VariableDeclarator with isPropsCall(n.init) that is not a plain ObjectPattern
without RestElement, it immediately returns 0 for the whole function. Use
countProps, walkEstree, and isPropsCall as the key places to adjust so mixed
$props() patterns no longer suppress ARCH002.
In `@packages/cli/test/parse-component-facts.test.ts`:
- Around line 100-102: `parseComponentFacts()` is overcounting LOC for
newline-terminated sources because it uses a raw split-based count. Update the
LOC calculation in `parseComponentFacts` to ignore a single trailing newline,
and add a test in `parse-component-facts.test.ts` covering a source string that
ends with a newline to verify the count stays correct.
In `@packages/core/src/rules/architecture/arch001-002.ts`:
- Around line 17-18: ARCH001 is currently applied to every component, including
ones with unknown size because source parsing falls back to loc: 0. Update the
rule in arch001-002 so the applies predicate only returns true when the
component location is known (for example, c.loc > 0), and keep the existing
MAX_LOC check in bad; this ensures unreadable components are skipped instead of
counting as passing. Use the arch001-002 rule definition and the component loc
field as the key symbols to locate the change.
---
Outside diff comments:
In `@packages/cli/src/providers/source/components.ts`:
- Around line 13-17: The fallback in the source component reader is resetting
metadata when parseComponentFacts fails after a successful read, so preserve the
raw-file-derived loc value instead of defaulting to 0. Update the try/catch in
components.ts around readFile and parseComponentFacts so that parse errors still
return the file’s loc (and any other already-known raw metrics) while only the
parse-specific fields fall back to empty values. Use the existing source-reading
flow and parseComponentFacts helper to keep ARCH001 reporting working for broken
or unsupported components.
---
Nitpick comments:
In `@packages/cli/test/docs-links.test.ts`:
- Around line 11-13: The docs-links test is using a hardcoded category
allowlist, which can go stale when new rule categories are added. Update the
test around DOCUMENTED_CATEGORIES/documented to derive coverage from allRules
directly, or assert that the documented categories set exactly matches the
categories present in allRules, so docs coverage stays in sync automatically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e1c73ed6-d2f3-4a28-9831-9d73dce06259
📒 Files selected for processing (20)
.changeset/architecture-category.mddocs/src/content/docs/ja/rules/arch001.mddocs/src/content/docs/ja/rules/arch002.mddocs/src/content/docs/rules/arch001.mddocs/src/content/docs/rules/arch002.mddocs/superpowers/specs/2026-06-30-architecture-category-design.mdpackages/cli/src/providers/source/components.tspackages/cli/src/providers/source/parse.tspackages/cli/test/docs-links.test.tspackages/cli/test/parse-component-facts.test.tspackages/core/src/component.tspackages/core/src/index.tspackages/core/src/reporter/console.tspackages/core/src/rules/architecture/arch001-002.tspackages/core/src/rules/component-rule.tspackages/core/src/rules/index.tspackages/core/src/types.tspackages/core/test/architecture-rules.test.tspackages/core/test/correctness-rules.test.tspackages/core/test/security-rules.test.ts
… mixed shapes (PR #71 review) - countLines no longer over-counts a single trailing newline (avoids a 400-line boundary false positive for newline-terminated files). - ARCH001 applies only when loc > 0, so an unanalyzable component (loc 0 = read/parse failure) is skipped rather than scored as a PASS. - countProps returns 0 if ANY $props() shape is uncountable (rest / non- destructured), so a mixed pattern can't leave a stale count that suppresses ARCH002. Tests added for each. pnpm -r test 519 green; typecheck, lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new Architecture code-health category to Svelte Vitals’ component-body (CLI/static) analysis, introducing deterministic “size smell” metrics for Svelte components and surfacing them through core rule exports, reporting, tests, and docs.
Changes:
- Add Architecture rules ARCH001 (component LOC) and ARCH002 (prop count from
$props()destructuring) asinfoseverity component-scoped rules. - Extend component parsing to compute
ComponentFacts.locandComponentFacts.propCount, and wire the new category through rule registry + console reporting. - Add tests, documentation pages (EN/JA), a design spec, and a changeset for the new category/rules.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/test/security-rules.test.ts | Updates test component fixtures to include new ComponentFacts fields (loc, propCount). |
| packages/core/test/correctness-rules.test.ts | Updates test component fixtures to include new ComponentFacts fields (loc, propCount). |
| packages/core/test/architecture-rules.test.ts | Adds unit tests for ARCH001/ARCH002 rule behavior and category/severity output. |
| packages/core/src/types.ts | Extends Category union with 'architecture'. |
| packages/core/src/rules/index.ts | Registers and re-exports the new architecture rules in the core rule registry. |
| packages/core/src/rules/component-rule.ts | Expands ComponentCategory to allow 'architecture' for component-scoped rules. |
| packages/core/src/rules/architecture/arch001-002.ts | Implements ARCH001/ARCH002 via the shared componentRule factory. |
| packages/core/src/reporter/console.ts | Adds Architecture label and ordering so console output includes the new category score line. |
| packages/core/src/index.ts | Re-exports architecture rules from the package entrypoint. |
| packages/core/src/component.ts | Extends ComponentFacts with loc and propCount fields. |
| packages/cli/test/parse-component-facts.test.ts | Adds parser tests for loc and $props()-based propCount behavior. |
| packages/cli/test/docs-links.test.ts | Updates documented-category allowlist to include 'architecture'. |
| packages/cli/src/providers/source/parse.ts | Computes loc and propCount during component parsing. |
| packages/cli/src/providers/source/components.ts | Ensures parse failures return loc: 0 and propCount: 0 in ComponentFacts. |
| docs/superpowers/specs/2026-06-30-architecture-category-design.md | Adds a design spec describing the new category, facts, and rules. |
| docs/src/content/docs/rules/arch002.md | Adds EN docs page for ARCH002. |
| docs/src/content/docs/rules/arch001.md | Adds EN docs page for ARCH001. |
| docs/src/content/docs/ja/rules/arch002.md | Adds JA docs page for ARCH002. |
| docs/src/content/docs/ja/rules/arch001.md | Adds JA docs page for ARCH001. |
| .changeset/architecture-category.md | Adds release notes / version bump metadata for the new category and rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… (PR #71 review) - countProps returns 0 when more than one $props() is found (a normal component has exactly one), so it never reports just the last destructure's size. - Update the spec's ARCH001 line to the implemented applies: (c) => c.loc > 0. Test added. Tests/typecheck/lint green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 task
This was referenced Jul 1, 2026
This was referenced Jul 26, 2026
This was referenced Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The third "Svelte Doctor" code-health category (next slice of #69), reusing the component-body scan (
ctx.components, CLI/static only). Deterministic, high-precision size metrics that flag bloated "god components" — no overlap with the compiler / svelte-check / eslint.New rules
infoinfoWhat
ComponentFactsgainsloc(source line count) andpropCount(named props destructured from$props(); 0 when unknowable — a...restelement or a non-destructuredlet p = $props()).componentRule'sComponentCategorywidens to include'architecture'; the console reporter shows an Architecture score line (html/json enumerate dynamically); the docs-link allowlist gains it.infoseverity — size/props are advisory smells, not defects. Thresholds are named constants (configurable surface deferred).Tests / docs
locline count;propCountfrom a destructured$props(); rest/non-destructured → 0) + rule behavior.pnpm -r test(517: core 236 / vite 76 / cli 196 / mcp 9),pnpm -r typecheck,pnpm lint,pnpm --filter docs build(105 pages) — all green.Next (#69)
Workflow ergonomics (
--diff/--staged), then the deferred reactivity heuristics and bundle depth.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
$props()).Documentation
Tests