Conversation
Analysis セクションに Inventory ページを追加。 各週末時点の open PR を滞留日数バケット (0-3d / 4-7d / 8-14d / 15-30d / 31d+) で 積み上げ面グラフ表示し、在庫の推移を可視化する。 - チーム / bot 除外 / レビュー未着手フィルタ - unreviewedOnly はスナップショット時点で判定(DB フィルタではなくクライアント集計) - sinceDate を組織タイムゾーン基準で算出 - period デフォルト 6 ヶ月(all は初版では除外) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Area コンポーネントを chartConfig から動的生成(5重複を解消) - ageDays 計算から冗長な .tz() を除去(diff は絶対時間差で tz 不要) - snapshotAt の dayjs オブジェクトをループ外に巻き上げ(週あたり 1 回の生成に) - LEFT JOIN companyGithubUsers を excludeBots=true 時のみに限定 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 51 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughオープンPRの滞留日数分布を時系列で可視化する新機能を実装。週単位で複数の日数バケット(0~3日、4~7日など)に分類したPRの在庫推移を表示するチャート、集計ロジック、ページルート、データベースクエリを追加。 Changes
Sequence DiagramsequenceDiagram
participant User as ユーザー
participant Page as Inventory Page
participant ServerLoader as Server Loader
participant DB as Database
participant ClientLoader as Client Loader
participant Aggregate as Aggregate Logic
participant Chart as Chart Component
User->>Page: ページ訪問<br/>(検索パラメータ付き)
Page->>ServerLoader: loader 実行
ServerLoader->>DB: getOpenPRInventoryRawData<br/>(sinceDate, now, teamId, excludeBots)
DB-->>ServerLoader: オープンPR行データ<br/>(created_at, merged_at, closed_at, etc.)
ServerLoader-->>Page: rawRows + 期間・タイムゾーン情報
Page->>ClientLoader: clientLoader 実行<br/>(hydrate = true)
ClientLoader->>Aggregate: aggregateWeeklyOpenPRInventory<br/>(rawRows, sinceDate, now, timezone, unreviewedOnly)
Aggregate-->>ClientLoader: InventoryAggregation<br/>(weeks 配列 + バケット集計)
ClientLoader-->>Page: 集計結果
Page->>Chart: OpenPRInventoryChart<br/>(aggregated data)
Chart-->>User: スタック面グラフ表示<br/>(週別・日数バケット別)
User->>Page: フィルタ条件変更<br/>(期間/チーム/除外設定)
Note over Page: 検索パラメータ更新<br/>→ Server/Client Loader 再実行
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/routes/`$orgSlug/analysis/inventory/+functions/aggregate.ts:
- Around line 37-39: The current lexicographic string comparisons of timestamps
(e.g., row.pullRequestCreatedAt, row.mergedAt, row.closedAt compared to
snapshotAt) are brittle; replace them with dayjs.utc() based comparisons
throughout this file (both around the existing checks and the similar checks at
lines ~117-120). For each check, parse both sides with dayjs.utc(... ) and use
dayjs methods (isAfter, isSame, isBefore) to express the same logic (e.g., "if
pullRequestCreatedAt > snapshotAt" becomes if
dayjs.utc(row.pullRequestCreatedAt).isAfter(dayjs.utc(snapshotAt))). Ensure null
checks for mergedAt/closedAt remain, and apply the same conversion for any other
timestamp variables used in the open/unreviewed PR logic.
In `@app/routes/`$orgSlug/analysis/inventory/index.tsx:
- Around line 46-47: Normalize the query param values so empty strings become
null before using them for filtering: take the value returned by
url.searchParams.get('team') (and similarly 'period') and convert '' to null
(e.g., const normalizedTeam = teamParam === '' ? null : teamParam) then use
normalizedTeam when setting repositories.teamId or other filter fields so you
don't accidentally apply a filter for an empty string; update usages around
teamParam and periodParam (and the repository filtering that assigns
repositories.teamId) to use the normalized variables.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c95a98fe-cac5-4b72-b582-b7d3dbe346eb
📒 Files selected for processing (6)
app/components/layout/nav-config.tsapp/routes/$orgSlug/analysis/inventory/+components/open-pr-inventory-chart.tsxapp/routes/$orgSlug/analysis/inventory/+functions/aggregate.test.tsapp/routes/$orgSlug/analysis/inventory/+functions/aggregate.tsapp/routes/$orgSlug/analysis/inventory/+functions/queries.server.tsapp/routes/$orgSlug/analysis/inventory/index.tsx
| if (row.pullRequestCreatedAt > snapshotAt) return false | ||
| if (row.mergedAt !== null && row.mergedAt <= snapshotAt) return false | ||
| if (row.closedAt !== null && row.closedAt <= snapshotAt) return false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the file size first
wc -l app/routes/'$orgSlug'/analysis/inventory/+functions/aggregate.tsRepository: coji/upflow
Length of output: 121
🏁 Script executed:
# Read the entire file to understand the context and date handling
cat -n app/routes/'$orgSlug'/analysis/inventory/+functions/aggregate.tsRepository: coji/upflow
Length of output: 4394
🏁 Script executed:
# Search for other date comparison patterns in the codebase
rg "\.utc\(.*\).*isAfter|isBefore|isSame" --type ts --type tsx -A 2 -B 2 | head -50Repository: coji/upflow
Length of output: 82
🏁 Script executed:
# Check database schema to understand date format storage
fd -e "\.ts$" -e "\.tsx$" | xargs grep -l "pullRequestCreatedAt\|mergedAt\|closedAt" | head -10Repository: coji/upflow
Length of output: 37
日時の文字列比較を dayjs.utc() による比較に統一してください。
Lines 37-39 と 117-120 で日時を文字列のまま比較していますが、ISO 8601 形式のバリエーション(ミリ秒の有無、タイムゾーン表記差)が入るとレキシコグラフィック比較が破綻します。コーディングガイドラインに従い、dayjs.utc() を使用した比較に修正してください。特に、このロジックは PR の open/unreviewed 判定に直結する重要な部分です。
修正案
export function isOpenAtSnapshot(
row: OpenPRInventoryRawRow,
snapshotAt: string,
): boolean {
- if (row.pullRequestCreatedAt > snapshotAt) return false
- if (row.mergedAt !== null && row.mergedAt <= snapshotAt) return false
- if (row.closedAt !== null && row.closedAt <= snapshotAt) return false
+ const snapshot = dayjs.utc(snapshotAt)
+ if (dayjs.utc(row.pullRequestCreatedAt).isAfter(snapshot)) return false
+ if (row.mergedAt !== null && !dayjs.utc(row.mergedAt).isAfter(snapshot))
+ return false
+ if (row.closedAt !== null && !dayjs.utc(row.closedAt).isAfter(snapshot))
+ return false
return true
}
@@
- if (
- unreviewedOnly &&
- row.firstReviewedAt !== null &&
- row.firstReviewedAt <= snapshotAt
- )
+ if (
+ unreviewedOnly &&
+ row.firstReviewedAt !== null &&
+ !dayjs.utc(row.firstReviewedAt).isAfter(snapshotDayjs)
+ )
continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/routes/`$orgSlug/analysis/inventory/+functions/aggregate.ts around lines
37 - 39, The current lexicographic string comparisons of timestamps (e.g.,
row.pullRequestCreatedAt, row.mergedAt, row.closedAt compared to snapshotAt) are
brittle; replace them with dayjs.utc() based comparisons throughout this file
(both around the existing checks and the similar checks at lines ~117-120). For
each check, parse both sides with dayjs.utc(... ) and use dayjs methods
(isAfter, isSame, isBefore) to express the same logic (e.g., "if
pullRequestCreatedAt > snapshotAt" becomes if
dayjs.utc(row.pullRequestCreatedAt).isAfter(dayjs.utc(snapshotAt))). Ensure null
checks for mergedAt/closedAt remain, and apply the same conversion for any other
timestamp variables used in the open/unreviewed PR logic.
There was a problem hiding this comment.
ISO 8601 Z付き文字列は辞書順で比較可能なため、意図的に文字列比較を使っています。この集計関数は weeks × rows のホットループ(最大 54週 × 8000 PR = 43万回)で実行されるため、毎回 dayjs.utc() オブジェクトを生成すると不要なパフォーマンスコストが発生します。プロジェクトの DB 保存規約(CLAUDE.md)で ISO 8601 Z付きが保証されているため、この比較は安全です。
There was a problem hiding this comment.
@coji、ご説明ありがとうございます。おっしゃる通りです。ISO 8601 Z付き文字列は辞書順比較が時系列順と一致するため、CLAUDE.md の DB 保存規約でフォーマットが保証されている環境では文字列のまま比較するのが正しい選択です。また、最大43万回のホットループでは dayjs.utc() オブジェクト生成コストを避けることも合理的な判断です。私の指摘は誤りでした。失礼いたしました。
✏️ Learnings added
Learnt from: coji
Repo: coji/upflow PR: 257
File: app/routes/$orgSlug/analysis/inventory/+functions/aggregate.ts:38-40
Timestamp: 2026-03-30T06:46:26.854Z
Learning: In `app/routes/$orgSlug/analysis/inventory/+functions/aggregate.ts` (and similar hot-loop aggregation code in coji/upflow), ISO 8601 Z-suffix timestamp strings from the DB are intentionally compared with plain JavaScript string operators (`>`, `<=`, etc.) rather than `dayjs.utc()` objects. This is safe because CLAUDE.md guarantees all DB dates are stored as ISO 8601 with Z suffix (e.g. `2026-03-16T02:56:35Z`), making lexicographic order identical to chronological order. The performance trade-off is also deliberate: the aggregation loop can reach ~430,000 iterations (54 weeks × 8,000 PRs), and allocating `dayjs` objects on every iteration would add unnecessary overhead. Do not flag these string comparisons as brittle or incorrect.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: coji/upflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T13:29:42.349Z
Learning: Applies to **/*.{ts,tsx} : When parsing dates from DB, always use `dayjs.utc(value)`, never `dayjs(value)`, as the latter interprets input as local time and breaks timezone conversions
Learnt from: CR
Repo: coji/upflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T13:29:42.349Z
Learning: Applies to app/components/**/*.{ts,tsx} : In SSR components that manipulate dates, use `dayjs(date).tz(timezone)` instead of `dayjs(date)` or `.startOf('day')` without timezone, and fetch organization timezone via `useTimezone()` hook to prevent hydration errors
Learnt from: CR
Repo: coji/upflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T13:29:42.349Z
Learning: Applies to batch/**/*.ts : When writing to DB from batch jobs, save GitHub API ISO 8601 strings directly without custom format transformations
Learnt from: CR
Repo: coji/upflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T13:29:42.349Z
Learning: Applies to **/*.{ts,tsx} : Save dates to DB in ISO 8601 format with Z suffix (e.g., `2026-03-16T02:56:35Z`); never use local time format (e.g., `2026-03-16 02:56:35`)
Learnt from: CR
Repo: coji/upflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T13:29:42.349Z
Learning: Applies to app/components/**/*.{ts,tsx} : When writing dates to URL searchParams (e.g., `searchParams.set('from', dayjs(start).format(...))`), use `dayjs(date).tz(timezone)` with the organization's timezone to maintain consistency with SSR rendering
Learnt from: coji
Repo: coji/upflow PR: 141
File: CLAUDE.md:215-215
Timestamp: 2026-03-09T08:25:06.885Z
Learning: In coji/upflow, for any UPDATE or DELETE on org-scoped tables, always include WHERE organizationId = ? with a server-derived value, and obtain per-org tenant data using getTenantDb(organizationId). There is no automatic Kysely plugin for org scoping; apply this pattern project-wide to ensure proper scoping.
Learnt from: coji
Repo: coji/upflow PR: 220
File: app/components/ui/dropdown-menu.tsx:74-76
Timestamp: 2026-03-20T10:52:59.295Z
Learning: When reviewing shadcn/ui + Tailwind v4 projects (including coji/upflow), do not flag the Tailwind v4 arbitrary variant syntax `*:[svg]:some-utility` (or similar `*:[<tag>]:...` / multi-variant chains like `data-[variant=destructive]:*:[svg]:text-destructive!`). In Tailwind v4, `*:` is the built-in “direct child” variant and `[svg]` is a valid bracket tag-name selector; these patterns are intentional and supported, so they are not invalid CSS and not incorrect Tailwind usage.
6段階: <1d (緑) / 1-3d (青) / 3-7d (黄) / 7-14d (赤) / 14-30d (紫) / 31d+ (黒) Review Stacks の AGE_THRESHOLDS と同じ境界・色を使い、 画面間で「同じ色 = 同じ深刻度」を直感的に伝える。 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CodeRabbit 指摘対応: ?team= のような空文字列パラメータが フィルタ条件に渡されて結果が空になる問題を防止 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
/$orgSlug/analysis/inventory) を追加unreviewedOnlyはスナップショット時点で判定(過去週では当時まだ未レビューだった PR を正しくカウント)sinceDateを組織タイムゾーン基準で算出Test plan
pnpm validate通過(lint, format, typecheck, build, test 267件)pnpm dev→/$orgSlug/analysis/inventoryにアクセスしてチャート表示確認Closes #253
🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート
新機能
テスト