feat: align reviews drill-down UI and use corrected sizes#152
Merged
Conversation
- Use SizeBadge component for colored size labels (was grey bg-muted pill) - Display risk areas as outline badges (was plain text) - Show SizeBadge in sheet description subtitle - Format PR count with locale comma separators - Unify SelectTrigger height to default across period and team filters Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- LEFT JOIN pull_request_feedbacks in reviews queries (WIP cycle, PR size) - Update getPRComplexity to prefer correctedComplexity over LLM complexity - Add correctedComplexity to WipRawRow and PRSizeRawRow types - Format PR count with locale comma separator in drill-down description Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis change introduces support for corrected PR size classification, enabling users to provide feedback that overrides automated complexity categorization. The feature adds a correctedComplexity field throughout the data pipeline, from database queries through classification logic to UI display with new badge components and improved formatting. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/routes/$orgSlug/reviews/+functions/classify.ts (1)
49-67:⚠️ Potential issue | 🟡 MinorValidate the corrected label before overriding the original one.
Right now any non-null
correctedComplexitywins, even if it is not one of the supported size labels. Because this value is stored as free text, a bad/stale correction would downgrade an otherwise validcomplexitytoUnclassifiedhere and incomplexitySortingFn.🐛 Suggested fix
+function toValidSizeLabel( + value: string | null | undefined, +): PRSizeLabel | null { + return value != null && (PR_SIZE_LABELS as readonly string[]).includes(value) + ? (value as PRSizeLabel) + : null +} + /** 補正済みサイズ → LLM分類 → Unclassified の優先順で返す */ export function getPRComplexity(pr: { complexity: string | null correctedComplexity?: string | null }): PRSizeLabel { - const value = pr.correctedComplexity ?? pr.complexity - if (value != null && (PR_SIZE_LABELS as readonly string[]).includes(value)) { - return value as PRSizeLabel - } - return 'Unclassified' + return ( + toValidSizeLabel(pr.correctedComplexity) ?? + toValidSizeLabel(pr.complexity) ?? + 'Unclassified' + ) } /** correctedComplexity ?? complexity でソートする関数 */ export function complexitySortingFn< T extends { correctedComplexity: string | null; complexity: string | null }, >(a: { original: T }, b: { original: T }): number { - const aVal = a.original.correctedComplexity ?? a.original.complexity ?? '' - const bVal = b.original.correctedComplexity ?? b.original.complexity ?? '' + const aVal = + toValidSizeLabel(a.original.correctedComplexity) ?? + toValidSizeLabel(a.original.complexity) ?? + 'Unclassified' + const bVal = + toValidSizeLabel(b.original.correctedComplexity) ?? + toValidSizeLabel(b.original.complexity) ?? + 'Unclassified' return (PR_SIZE_RANK[aVal] ?? 5) - (PR_SIZE_RANK[bVal] ?? 5) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/reviews/+functions/classify.ts around lines 49 - 67, getPRComplexity and complexitySortingFn currently unconditionally prefer correctedComplexity even if it contains free-text that isn't in PR_SIZE_LABELS; update both to validate correctedComplexity against PR_SIZE_LABELS (and coerce to PRSizeLabel) before using it, falling back to complexity if correctedComplexity is invalid or null, and ensure complexitySortingFn computes aVal/bVal from the validated label values so PR_SIZE_RANK lookups only receive known keys (use PR_SIZE_LABELS and PR_SIZE_RANK in the validation).
🧹 Nitpick comments (3)
app/routes/$orgSlug/reviews/+components/pr-size-chart.tsx (1)
30-30: Use the app alias for this new cross-folder import.Please switch this to the
~/form instead of../../...so the new import follows the repo rule and stays stable if this folder moves.♻️ Suggested change
-import { SizeBadge } from '../../+components/size-badge' +import { SizeBadge } from '~/app/routes/$orgSlug/+components/size-badge'As per coding guidelines, "Use path aliases with
~/prefix for imports from theapp/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/reviews/+components/pr-size-chart.tsx at line 30, The import for SizeBadge in pr-size-chart.tsx uses a relative path; change the import to use the app alias (~/) so it reads from the app root instead of '../../+components/size-badge' — update the import that references SizeBadge to use '~/routes/$orgSlug/reviews/+components/size-badge' (or the appropriate path under the app alias) so the module resolution stays stable if the file is moved.app/routes/$orgSlug/reviews/+components/pr-drill-down-sheet.tsx (1)
11-11: Use the app alias for this new import as well.This new cross-folder import should use the
~/form instead of../../...to stay aligned with the repo convention.♻️ Suggested change
-import { SizeBadge } from '../../+components/size-badge' +import { SizeBadge } from '~/app/routes/$orgSlug/+components/size-badge'As per coding guidelines, "Use path aliases with
~/prefix for imports from theapp/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/reviews/+components/pr-drill-down-sheet.tsx at line 11, The import in pr-drill-down-sheet.tsx uses a relative path for SizeBadge; update the import to use the app alias (~/) instead of '../../+components/size-badge' so it follows the repo convention; locate the import statement for SizeBadge in the file and change it to the `~/` prefixed path referencing the same +components/size-badge module.app/routes/$orgSlug/reviews/+functions/aggregate.test.ts (1)
63-78: Add a regression test for corrected-size precedence.Now that
sizeRowcarriescorrectedComplexity, please add oneaggregatePRSizecase wherecomplexityandcorrectedComplexitydisagree and assert that the corrected bucket wins. That is the main behavior change in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/`$orgSlug/reviews/+functions/aggregate.test.ts around lines 63 - 78, Add a regression test in app/routes/$orgSlug/reviews/+functions/aggregate.test.ts that ensures correctedComplexity takes precedence: create an aggregatePRSize test case where the input sizeRow (constructed by the existing helper used in the file) has complexity set to one bucket and correctedComplexity set to a different bucket, call aggregatePRSize with that row, and assert the returned size bucket equals the correctedComplexity value (not complexity); reference the aggregatePRSize function and the correctedComplexity and complexity properties on the sizeRow when adding the case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/pr-size-classification-guide.md`:
- Around line 27-37: The fenced code blocks containing the ASCII diagram (the
block starting with "┌─────────────┐ ┌──────────────┐ ┌─────────────────┐"
and the other similar block later in the file) are missing a language tag and
trigger MD040; add a language specifier of text to each fenced block (change ```
to ```text) for both the diagram shown and the other fenced block around lines
64-78 so the markdown linter stops warning.
---
Outside diff comments:
In `@app/routes/`$orgSlug/reviews/+functions/classify.ts:
- Around line 49-67: getPRComplexity and complexitySortingFn currently
unconditionally prefer correctedComplexity even if it contains free-text that
isn't in PR_SIZE_LABELS; update both to validate correctedComplexity against
PR_SIZE_LABELS (and coerce to PRSizeLabel) before using it, falling back to
complexity if correctedComplexity is invalid or null, and ensure
complexitySortingFn computes aVal/bVal from the validated label values so
PR_SIZE_RANK lookups only receive known keys (use PR_SIZE_LABELS and
PR_SIZE_RANK in the validation).
---
Nitpick comments:
In `@app/routes/`$orgSlug/reviews/+components/pr-drill-down-sheet.tsx:
- Line 11: The import in pr-drill-down-sheet.tsx uses a relative path for
SizeBadge; update the import to use the app alias (~/) instead of
'../../+components/size-badge' so it follows the repo convention; locate the
import statement for SizeBadge in the file and change it to the `~/` prefixed
path referencing the same +components/size-badge module.
In `@app/routes/`$orgSlug/reviews/+components/pr-size-chart.tsx:
- Line 30: The import for SizeBadge in pr-size-chart.tsx uses a relative path;
change the import to use the app alias (~/) so it reads from the app root
instead of '../../+components/size-badge' — update the import that references
SizeBadge to use '~/routes/$orgSlug/reviews/+components/size-badge' (or the
appropriate path under the app alias) so the module resolution stays stable if
the file is moved.
In `@app/routes/`$orgSlug/reviews/+functions/aggregate.test.ts:
- Around line 63-78: Add a regression test in
app/routes/$orgSlug/reviews/+functions/aggregate.test.ts that ensures
correctedComplexity takes precedence: create an aggregatePRSize test case where
the input sizeRow (constructed by the existing helper used in the file) has
complexity set to one bucket and correctedComplexity set to a different bucket,
call aggregatePRSize with that row, and assert the returned size bucket equals
the correctedComplexity value (not complexity); reference the aggregatePRSize
function and the correctedComplexity and complexity properties on the sizeRow
when adding the case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0159322f-6119-4fef-9bc6-b36a9329c0b6
📒 Files selected for processing (8)
app/components/team-filter.tsxapp/routes/$orgSlug/reviews/+components/pr-drill-down-sheet.tsxapp/routes/$orgSlug/reviews/+components/pr-size-chart.tsxapp/routes/$orgSlug/reviews/+functions/aggregate.test.tsapp/routes/$orgSlug/reviews/+functions/aggregate.tsapp/routes/$orgSlug/reviews/+functions/classify.tsapp/routes/$orgSlug/reviews/+functions/queries.server.tsdocs/pr-size-classification-guide.md
Comment on lines
+27
to
+37
| ``` | ||
| ┌─────────────┐ ┌──────────────┐ ┌─────────────────┐ | ||
| │ LLM が分類 │ → │ 人間が補正 │ → │ 補正データ蓄積 │ | ||
| │ (自動) │ │ (ダッシュボード) │ │ (フィードバック) │ | ||
| └─────────────┘ └──────────────┘ └────────┬────────┘ | ||
| │ | ||
| ┌──────────────┐ │ | ||
| │ 分類精度向上 │ ←────────────┘ | ||
| │ (プロンプト改善) │ | ||
| └──────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add a language to both fenced blocks.
These examples currently trigger MD040. Using text for both blocks is enough to clear the warning and keep the docs lint-clean.
Also applies to: 64-78
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/pr-size-classification-guide.md` around lines 27 - 37, The fenced code
blocks containing the ASCII diagram (the block starting with "┌─────────────┐
┌──────────────┐ ┌─────────────────┐" and the other similar block later in
the file) are missing a language tag and trigger MD040; add a language specifier
of text to each fenced block (change ``` to ```text) for both the diagram shown
and the other fenced block around lines 64-78 so the markdown linter stops
warning.
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
1 participant
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.
Summary
Changes
pr-drill-down-sheet.tsx: グレーの pill →SizeBadgeコンポーネント、riskAreasをバッジ表示pr-size-chart.tsx: ドリルダウンのサブタイトルにSizeBadge追加、件数にtoLocaleString()classify.ts:getPRComplexityがcorrectedComplexityを優先するよう変更queries.server.ts:getWipCycleRawData/getPRSizeDistributionにpullRequestFeedbacksLEFT JOIN 追加team-filter.tsx: SelectTrigger の高さをデフォルト（h-9）に統一Test plan
pnpm validatepasses🤖 Generated with Claude Code