Skip to content

chore(ci): exclude Storybook stories from PR tiering - #2989

Open
brandon-pereira wants to merge 1 commit into
mainfrom
claude/ignore-story-files-in-tiering
Open

chore(ci): exclude Storybook stories from PR tiering#2989
brandon-pereira wants to merge 1 commit into
mainfrom
claude/ignore-story-files-in-tiering

Conversation

@brandon-pereira

Copy link
Copy Markdown
Member

Why

The PR triage classifier (.github/scripts/pr-triage-classify.js) currently treats Storybook story files (*.stories.tsx/*.stories.ts) as regular production code. Their churn counts toward prodLines — which drives the tier — and flips on the touchesFrontend cross-layer signal.

Stories are dev-only fixtures: they're never included in the shipped app bundle and carry no production risk. They belong with tests and docs in the "excluded from tiering" bucket, not with reviewable production code.

What

  • Add a \.stories\.[jt]sx?$ pattern to TEST_FILE_PATTERNS so stories are excluded from line-counting exactly like .test.*/.spec.* files already are.
  • Add a matching isTestFile unit test.

No behavior change for any non-story file. All 113 classifier tests pass.

Story files are dev-only fixtures that never ship in the app bundle, so
their churn should not count toward production line totals or the
cross-layer signal in the PR triage classifier. Add a .stories.[jt]sx?
pattern to TEST_FILE_PATTERNS so they're excluded like tests.
@vercel

vercel Bot commented Aug 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
hyperdx-oss Ignored Ignored Aug 24, 2026 9:52pm
hyperdx-storybook Ignored Ignored Aug 24, 2026 9:52pm

Request Review

@changeset-bot

changeset-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: db6e468

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

Copy link
Copy Markdown
Contributor

🟢 Tier 1 — Trivial

Docs, images, lock files, a dependency bump, or an automated release. No functional code changes detected.

Why this tier:

  • All files are docs / images / lock files

Review process: Auto-merge once CI passes. No human review required.
SLA: Resolves automatically.

Stats
  • Production files changed: 0
  • Production lines changed: 0 (+ 5 in test files, excluded from tier calculation)
  • Branch: claude/ignore-story-files-in-tiering
  • Author: brandon-pereira

To override this classification, remove the review/tier-1 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions github-actions Bot added the review/tier-1 Trivial — auto-merge candidate once CI passes label Aug 24, 2026
@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates PR triage classification so JavaScript and TypeScript Storybook stories are treated like other dev-only test fixtures.

  • Adds the .stories.[jt]s(x) suffix to the excluded-file patterns.
  • Adds unit coverage for TypeScript and TSX story filenames.

Confidence Score: 5/5

The PR appears safe to merge because the new exclusion matches the repository’s dev-only Storybook file convention without affecting non-story files.

Story files are consumed by separate Storybook tooling rather than the production application build, and the suffix-specific pattern correctly applies the intended tiering exclusion.

Important Files Changed

Filename Overview
.github/scripts/pr-triage-classify.js Adds a narrowly scoped story-file pattern to the existing test-file exclusions; no actionable classification defect was identified.
.github/scripts/tests/pr-triage-classify.test.js Adds focused positive coverage for .stories.ts and .stories.tsx matching.

Reviews (1): Last reviewed commit: "chore(ci): exclude Storybook stories fro..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

review:

Deep Review

✅ No critical issues found.

The change routes *.stories.{js,jsx,ts,tsx} through TEST_FILE_PATTERNS, which cleanly excludes them from prodLines, criticalCandidates, and the touchesFrontend/cross-layer signals — mirroring how .test.*/.spec.* and the E2E ClickHouse fixture are already handled. The regex is $-anchored and correctly matches only true story files; helper modules inside stories/ directories (e.g. packages/app/src/stories/AssetCard.tsx) lack the .stories. infix and stay counted as production, which is the conservative outcome. No production-impacting behavior change.

🟡 P2 -- recommended

  • .github/scripts/__tests__/pr-triage-classify.test.js:53 -- the new exclusion is asserted only at the isTestFile classification level, unlike every sibling exclusion (test files, init-db-e2e.sh, docs) which pairs an end-to-end computeSignals/determineTier assertion proving the exclusion actually changes the tier.
    • Fix: Add a computeSignals/determineTier case asserting a PR of a single large .stories.tsx file yields prodLines === 0 with touchesFrontend === false and does not escalate on size.
🔵 P3 nitpicks (1)
  • .github/scripts/__tests__/pr-triage-classify.test.js:54 -- the added test asserts only positive matches, leaving no guard that non-story modules living in a stories/ directory are excluded from the pattern.
    • Fix: Add a negative assertion that packages/app/src/stories/AssetCard.tsx returns false from isTestFile, locking in the .stories. anchoring against future regex loosening.

Reviewers (3): correctness, testing, maintainability.

Testing gaps: No tier-level assertion that a .stories.* file's churn is excluded from prodLines and suppresses touchesFrontend. The exclusion is unconditional on path, so a future .stories.* file added under a critical path (packages/api/**, packages/common-utils/src/clickhouse/**, packages/otel-collector/**) would be silently dropped from critical detection — latent only; all current story files reside under packages/app/src/.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 306 passed • 1 skipped • 1223s

Status Count
✅ Passed 306
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-1 Trivial — auto-merge candidate once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants