Repository navigation
feat: Correctness category — component-body analysis (CORRECT001/002), step 1 toward Svelte Doctor - #68
Merged
Conversation
|
Warning Review limit reached
Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a ChangesCorrectness Category
Sequence Diagram(s)sequenceDiagram
participant CLI as analyzeProject (CLI)
participant collectComponentFacts
participant parseComponentFacts
participant svelteCompiler as svelte/compiler
participant runRules
CLI->>collectComponentFacts: cwd
CLI->>collectComponentFacts: Promise.all with collectProjectFacts
collectComponentFacts->>collectComponentFacts: glob src/**/*.svelte
loop each .svelte file
collectComponentFacts->>parseComponentFacts: source, filename
parseComponentFacts->>svelteCompiler: parse(source)
svelteCompiler-->>parseComponentFacts: template AST + script ESTree
parseComponentFacts-->>collectComponentFacts: ComponentFacts {eachBlocks, effects}
end
collectComponentFacts-->>CLI: ComponentFacts[]
CLI->>runRules: ctx { project, components }
runRules->>runRules: correct001EachKey, correct002EffectDerived
runRules-->>CLI: RuleResult[]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
1 task
…ECT001/002)
First step toward a "Svelte Doctor": analyze component bodies (not just <head>)
with a new `correctness` category.
- New `correctness` Category + `scope: 'component'` (scoring is data-driven, so the
category flows into per-category/combined Health with no scoring change).
- New CLI component scan: collectComponentFacts globs src/**/*.svelte (incl. $lib)
and parses each into a ComponentFacts channel (ctx.components); CLI/static only,
so rendered mode no-ops. Findings score per source file.
- CORRECT001 keyed-each: flags an `{#each}` with no key.
- CORRECT002 effect-derived: flags an `$effect` whose body only assigns `$state`
(the useEffect-to-$effect anti-pattern) — use `$derived`.
Fact extraction: template walk for EachBlock keys; instance-script ESTree walk for
$state declarations and $effect calls (conservative assign-only detection). Docs
(en+ja), changeset, spec, and tests (parser facts + rules + integration) added.
pnpm -r test 488 green; typecheck, lint, docs build (97 pages) green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
oekazuma
force-pushed
the
feat/correctness-category
branch
from
June 29, 2026 07:59
eee4641 to
4dc7773
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/cli/test/parse-component-facts.test.ts (2)
35-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the collector’s error-tolerant fallback too.
collectComponentFacts()is intentionally resilient: it catches read/parse failures and returns empty facts for that file. This test only locks in the happy path, so a future change could break that guarantee without any signal.Suggested test shape
describe('collectComponentFacts (memory runtime)', () => { it('scans every .svelte under src, including $lib', async () => { const rt = createMemoryRuntime({ 'src/routes/+page.svelte': '{`#each` xs as x}<i>{x}</i>{/each}', 'src/lib/Card.svelte': '<script>let n = $state(0); let d = $state(0); $effect(() => { d = n + 1; });</script>', 'src/app.html': '<html></html>' // not .svelte → ignored }); const facts = await collectComponentFacts(rt, ''); const byFile = new Map(facts.map((f) => [f.file, f])); expect(byFile.get('src/routes/+page.svelte')!.eachBlocks).toEqual([{ hasKey: false, line: 1 }]); expect(byFile.get('src/lib/Card.svelte')!.effects[0]!.assignsOnlyState).toBe(true); expect(byFile.has('src/app.html')).toBe(false); }); + + it('returns empty facts for unreadable or unparsable components', async () => { + const rt = createMemoryRuntime({ + 'src/lib/Broken.svelte': '<script>let x = </script>' + }); + await expect(collectComponentFacts(rt, '')).resolves.toContainEqual({ + file: 'src/lib/Broken.svelte', + eachBlocks: [], + effects: [] + }); + }); });🤖 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/parse-component-facts.test.ts` around lines 35 - 47, The current test only covers the happy path for collectComponentFacts and does not verify its error-tolerant fallback. Update the existing collectComponentFacts (memory runtime) test in parse-component-facts.test.ts to also cover a read/parse failure case by including a file that will fail during collection and asserting collectComponentFacts returns empty facts for that file rather than throwing, using the same collectComponentFacts, createMemoryRuntime, and byFile lookup patterns to keep the check anchored to the collector behavior.
15-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit
$effect.preparser case.The design spec calls out
$effect.pre, but this suite only exercises bare$effect. Since rune matching is a separate branch in the parser, a regression there would currently slip through.Suggested test
it('does not flag assignment to a non-$state variable', () => { const e = facts('let count = $state(0); let plain = 0; $effect(() => { plain = count; });'); expect(e[0]!.assignsOnlyState).toBe(false); }); + it('treats $effect.pre the same way', () => { + const e = facts('let count = $state(0); let double = $state(0); $effect.pre(() => { double = count * 2; });'); + expect(e).toEqual([{ line: 1, assignsOnlyState: true }]); + }); it('reports no effects when there are none', () => { expect(facts('let count = $state(0);')).toEqual([]); });🤖 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/parse-component-facts.test.ts` around lines 15 - 33, Add an explicit parser test coverage for $effect.pre in parseComponentFacts, since the current parseComponentFacts — $effect suite only validates bare $effect and can miss regressions in the separate rune branch. Extend the existing facts helper and add a focused case that parses a script containing $effect.pre, then assert the returned effects entry is reported correctly using the same assignsOnlyState behavior as the $effect cases.
🤖 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/index.ts`:
- Around line 105-108: The `runRules()` call in `index.ts` still evaluates all
collected components even when `opts.route` is set, so route-filtered runs can
emit CORRECT001/CORRECT002 findings from unrelated components. Update the
`collectComponentFacts`/`runRules` flow to respect `RunOptions.route` by
excluding or gating component-scope correctness rules during route-scoped
execution until component-to-route attribution is available. Use the existing
`opts.route` check and the `runRules` invocation in `index.ts` as the main
points to adjust.
In `@packages/cli/src/providers/source/parse.ts`:
- Around line 363-367: The state detection in isRuneCall is too broad for $state
because it also treats member calls like $state.snapshot(...) as declarations,
which causes stateNames to include non-state variables. Update isRuneCall so the
$state path only matches actual declaration forms (while preserving the broader
MemberExpression handling for $effect.pre), and keep the rest of the CORRECT002
scanning logic in parse.ts aligned with that stricter check.
---
Nitpick comments:
In `@packages/cli/test/parse-component-facts.test.ts`:
- Around line 35-47: The current test only covers the happy path for
collectComponentFacts and does not verify its error-tolerant fallback. Update
the existing collectComponentFacts (memory runtime) test in
parse-component-facts.test.ts to also cover a read/parse failure case by
including a file that will fail during collection and asserting
collectComponentFacts returns empty facts for that file rather than throwing,
using the same collectComponentFacts, createMemoryRuntime, and byFile lookup
patterns to keep the check anchored to the collector behavior.
- Around line 15-33: Add an explicit parser test coverage for $effect.pre in
parseComponentFacts, since the current parseComponentFacts — $effect suite only
validates bare $effect and can miss regressions in the separate rune branch.
Extend the existing facts helper and add a focused case that parses a script
containing $effect.pre, then assert the returned effects entry is reported
correctly using the same assignsOnlyState behavior as the $effect cases.
🪄 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: 5ebcf611-5aaa-4437-8ef1-9bab2c4014c0
📒 Files selected for processing (18)
.changeset/correctness-category.mddocs/src/content/docs/ja/rules/correct001.mddocs/src/content/docs/ja/rules/correct002.mddocs/src/content/docs/rules/correct001.mddocs/src/content/docs/rules/correct002.mddocs/superpowers/specs/2026-06-29-correctness-category-design.mdpackages/cli/src/index.tspackages/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/rule.tspackages/core/src/rules/correctness/correct001-002.tspackages/core/src/rules/index.tspackages/core/src/types.tspackages/core/test/correctness-rules.test.ts
…h (PR #68 review) - A --route-filtered run now skips the component (Correctness) scan instead of reporting findings from unrelated/shared components — component→route attribution doesn't exist yet, so route scoping can't apply to them. - $state-name detection only matches declaration forms ($state / $state.raw / $state.frozen), no longer $state.snapshot, so CORRECT002 doesn't pick up snapshot reads as reactive state. Tests added for both. pnpm -r test 490 green; typecheck, lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- CORRECT002: only treat a plain `=` assignment as a derive candidate;
compound assignments (`+=`, `*=`, `??=`, …) accumulate and can't become a
self-referential `$derived`, so flagging them was a false positive.
- CORRECT002: narrow effect detection to `$effect` / `$effect.pre` only,
excluding the non-effect readers `$effect.tracking()` / `$effect.root()`
that otherwise seeded spurious pass units.
- CORRECT001: ignore constant inline array literals (`{#each [1,2,3] as n}`)
— fixed length, never reorders, so a key can't help; spreads stay dynamic.
- Tests + en/ja docs note for the constant-list exclusion.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces the first component-body, static (CLI/source) analysis pipeline to extend svelte-vitals beyond <svelte:head> checks, adding a new Correctness category with initial high-precision reactivity/lifecycle heuristics.
Changes:
- Added
correctnesscategory +componentscope, plus two new rules: CORRECT001 (unkeyed{#each}) and CORRECT002 (assign-only$effect→ prefer$derived). - Implemented CLI component scanning (
src/**/*.svelte) to collectComponentFactsand thread them through the rule engine viactx.components. - Added rule docs (en/ja), changeset, and tests covering parsing, rule behavior, and component-facts collection.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/test/correctness-rules.test.ts | Adds unit tests for CORRECT001/002 rule behavior and rendered-mode no-op behavior. |
| packages/core/src/types.ts | Extends shared types with Scope: 'component' and Category: 'correctness'. |
| packages/core/src/rules/index.ts | Registers correctness rules in allRules and re-exports them. |
| packages/core/src/rules/correctness/correct001-002.ts | Implements the correctness rule factory and CORRECT001/002 rules (component-scoped). |
| packages/core/src/rule.ts | Adds components?: ComponentFacts[] to RuleContext. |
| packages/core/src/index.ts | Re-exports component fact types and new correctness rules from the public API. |
| packages/core/src/component.ts | Defines the ComponentFacts schema used for component-body analysis. |
| packages/cli/test/parse-component-facts.test.ts | Adds tests for parsing {#each} keys, $effect assign-only detection, and integration scan over src/**/*.svelte. |
| packages/cli/test/docs-links.test.ts | Updates docs-link integrity test to include correctness rules. |
| packages/cli/src/providers/source/parse.ts | Adds parseComponentFacts and AST walks for EachBlock keys and $state/$effect detection. |
| packages/cli/src/providers/source/components.ts | Implements collectComponentFacts scanning src/**/*.svelte with parse-failure tolerance. |
| packages/cli/src/index.ts | Threads component facts into rule execution (skips component scan for route-filtered runs). |
| docs/superpowers/specs/2026-06-29-correctness-category-design.md | Adds design spec documenting architecture, facts, rules, and test plan for the new category. |
| docs/src/content/docs/rules/correct002.md | Adds English rule docs page for CORRECT002. |
| docs/src/content/docs/rules/correct001.md | Adds English rule docs page for CORRECT001. |
| docs/src/content/docs/ja/rules/correct002.md | Adds Japanese rule docs page for CORRECT002. |
| docs/src/content/docs/ja/rules/correct001.md | Adds Japanese rule docs page for CORRECT001. |
| .changeset/correctness-category.md | Declares minor version bumps and release notes for the new correctness category and rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…68 review) Adding the `correctness` Category fed the combined Health score but the console reporter's CATEGORY_ORDER/CATEGORY_LABEL still listed only seo/performance, so the per-category score lines omitted Correctness. Add it (html/json reporters already enumerate categories dynamically). Test added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 29, 2026
This was referenced Jul 22, 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 first step toward a "Svelte Doctor" — broadening svelte-vitals from SEO/Performance
<head>checks into component-body code-health analysis, starting with a new Correctness category.Why
The goal is a deterministic, agent-native scanner that catches "the bad code an AI agent writes" across correctness, security, and architecture — not only SEO/Performance. Until now svelte-vitals only inspected
<svelte:head>/<img>/ headings. This PR adds the first analysis of component bodies and the first non-SEO/Perf category, focusing on high-precision checks the Svelte compiler /svelte-check/eslint-plugin-sveltedon't enforce as health signals (no duplication of official tooling).What
correctnesscategory +scope: 'component'. Scoring is data-driven (scoresByCategorygroups bycategory), so it flows into per-category and combined Health with no scoring-code change.collectComponentFactsglobssrc/**/*.svelte(including$lib, not just routes) and parses each into aComponentFactschannel (ctx.components). CLI/static only — the rendered provider can't see reactivity, so the rules no-op there. Findings are scored per source file (whole-codebase health).{#each}with no key (reordering an unkeyed list destroys/recreates DOM, losing element state).$effectwhose body only assigns to$state— use$derivedinstead.Fact extraction: a template walk for
EachBlockkeys, and an instance-script ESTree walk for$statedeclarations +$effectcalls (conservative assign-only detection for high precision).Tests / docs
$effectassign-only vs mixed,$statecollection), rule behavior, and acollectComponentFactsintegration oversrc/**/*.svelte.pnpm -r test(488: core 224 / vite 76 / cli 179 / mcp 9),pnpm -r typecheck,pnpm lint,pnpm --filter docs build(97 pages) — all green.Next (separate slices)
Security (
{@html}XSS,target=_blankrel), Architecture metrics, workflow ergonomics (--diff/--staged), and more reactivity heuristics — tracked in #69.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
{#each}blocks and$effectusage that should be replaced with$derived.Documentation
Tests