Skip to content

fix(core): give PERF009 findings a real line number - #200

Merged
oekazuma merged 3 commits into
mainfrom
advisor/028-perf009-line-tracking
Jul 13, 2026
Merged

oekazuma merged 3 commits into
mainfrom
advisor/028-perf009-line-tracking

Conversation

@oekazuma

@oekazuma oekazuma commented Jul 13, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • PERF009 always reported line: 0, silently defeating svelte-vitals-disable-next-line PERF009 suppression directives.
  • Adds ComponentFacts.importSpans (source + line per import) alongside the existing imports: string[], and switches PERF009 to use it.

Test plan

  • pnpm --filter @svelte-vitals/core test — 401/401 passing, including new line-number and suppression regression tests.
  • pnpm --filter svelte-vitals test — 563/563 passing (fixtures updated for the new required field).
  • Changeset added (@svelte-vitals/core minor).

Generated via an /improve advisor session, plan plans/028-perf009-line-tracking.md.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • PERF009 findings now report the correct source line for heavy imports.
    • // svelte-vitals-disable-next-line PERF009 suppression directives now work reliably.
    • Duplicate heavy-import findings are consolidated while preserving the first relevant line.
  • Enhancements
    • Import analysis now includes source-line information for more precise diagnostics.

PERF009 (heavy dependency import) always reported line: 0 because it
sourced its data from `imports: string[]`, which never tracked line
numbers. Since component-rule's suppression check only looks up a
`svelte-vitals-disable-next-line` directive when a finding's line > 0,
PERF009 findings could never be suppressed.

ComponentFacts gains `importSpans: { source: string; line: number }[]`
(alongside the untouched `imports` field), collected the same way
PERF010's `namespaceImports` already is. PERF009 now reports the real
import line, so suppression directives work like every other rule.
@coderabbitai

coderabbitai Bot commented Jul 13, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@oekazuma, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5f242e79-2588-44d0-ab59-ede09f362390

📥 Commits

Reviewing files that changed from the base of the PR and between bf6932d and c31e622.

⛔ Files ignored due to path filters (1)
  • packages/action/dist/index.js is excluded by !**/dist/**
📒 Files selected for processing (1)
  • packages/core/src/rules/perf/perf009-heavy-import.ts
📝 Walkthrough

Walkthrough

Adds line-aware importSpans to component facts, populates it during parsing, and updates PERF009 to report real import lines and support line-based suppression. Tests and empty-facts fixtures are updated for the new field.

Changes

PERF009 line tracking

Layer / File(s) Summary
Import span contract and parsing
packages/core/src/component.ts, packages/core/src/component-parse.ts, packages/core/src/component-collect.ts
ComponentFacts now exposes line-aware importSpans; parsing collects spans from module and instance imports while retaining imports. Empty facts include the new field.
PERF009 diagnostics and suppression
packages/core/src/rules/perf/perf009-heavy-import.ts, packages/core/test/bundle-rules.test.ts, .changeset/perf009-line-tracking.md
PERF009 reports actual import lines, preserves deduplication, and tests suppression against matching and non-matching lines.
Component facts shape updates
packages/*/test/*
Test factories and malformed or empty component expectations include importSpans: [].

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: PERF009 findings now report their actual source line instead of line 0.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

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.

Pull request overview

This PR fixes a long-standing issue where PERF009 findings always reported line: 0, which prevented svelte-vitals-disable-next-line PERF009 suppression directives from matching. It extends component import facts to include per-import line numbers and updates the PERF009 rule and tests accordingly.

Changes:

  • Add ComponentFacts.importSpans (module specifier + source line) and derive imports from it during parsing.
  • Update PERF009 to iterate importSpans and emit findings with real line numbers (including dedupe “first-seen line” behavior).
  • Update unit/E2E tests and generated action dist output to include the new required importSpans field and add suppression regression coverage.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/core/src/component.ts Extends ComponentFacts with importSpans to carry import line info.
packages/core/src/component-parse.ts Collects import spans (specifier + line) and derives imports from them.
packages/core/src/component-collect.ts Ensures emptyComponentFacts initializes importSpans.
packages/core/src/rules/perf/perf009-heavy-import.ts Switches PERF009 to use import spans so findings have real line numbers.
packages/core/test/bundle-rules.test.ts Adds PERF009 line-number + suppression regression tests; adapts fixtures to importSpans.
packages/core/test/component-collect.test.ts Updates empty facts expectations to include importSpans.
packages/core/test/component-rule.test.ts Updates ComponentFacts test helper defaults to include importSpans.
packages/core/test/architecture-rules.test.ts Updates ComponentFacts test helper defaults to include importSpans.
packages/core/test/correctness-rules.test.ts Updates ComponentFacts test helper defaults to include importSpans.
packages/core/test/security-rules.test.ts Updates ComponentFacts test helper defaults to include importSpans.
packages/cli/test/suppression-e2e.test.ts Updates ComponentFacts test helper defaults to include importSpans.
packages/cli/test/malformed-svelte.test.ts Updates malformed-file fixtures to include importSpans.
packages/action/dist/index.js Regenerates the action bundle to reflect core’s import span changes.
.changeset/perf009-line-tracking.md Adds a changeset documenting the PERF009 line-number fix and new importSpans.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 21 to 31
applies: (c) => c.importSpans.length > 0,
bad: (c) => {
// `Object.hasOwn` (not `in`) so inherited keys like `toString` never match;
// dedupe so the same package imported in both scripts isn't double-penalized.
const seen = new Set<string>();
const out: { line: number; message: string }[] = [];
for (const src of c.imports) {
for (const { source: src, line } of c.importSpans) {
if (!Object.hasOwn(HEAVY_PACKAGES, src) || seen.has(src)) continue;
seen.add(src);
out.push({ line: 0, message: `Heavy import "${src}" — ${HEAVY_PACKAGES[src]}` });
out.push({ line, message: `Heavy import "${src}" — ${HEAVY_PACKAGES[src]}` });
}
@oekazuma
oekazuma merged commit bfc2a2a into main Jul 13, 2026
7 checks passed
@oekazuma
oekazuma deleted the advisor/028-perf009-line-tracking branch July 13, 2026 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants