[Phase0] Vitest設定(unit/component/dbの3 environment分離) + package.jsonスクリプト - #18
Conversation
test/unit(node)・test/component(jsdom)・test/db(node)をvitestのprojects 機能で分離し、yarn testはunit/componentのみ、yarn test:dbはdbのみを 対象にした。test/dbが空でもpassWithNoTestsで成立する。 typecheck/test/test:db/gen:typesスクリプトをpackage.jsonに登録。 Closes #3
There was a problem hiding this comment.
🟢 Ready to approve
変更内容がIssueの完了条件と整合しており、設定・スクリプト追加として破壊的影響が小さいためです。
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Issue #3(Phase0の足場)に対応し、Vitest のテスト実行を unit/component/db の3プロジェクトに分離して、ローカルSupabase依存の db テストを通常の yarn test から切り離せるようにするPRです。あわせて、開発で必要な typecheck/test/test:db/gen:types のスクリプトを package.json に追加し、依存関係(vitest/vite/jsdom)を導入しています。
Changes:
vitest.config.tsを追加し、test.projectsでunit(node) / component(jsdom) / db(node)を分離package.jsonにtypecheck,test,test:db,gen:typesスクリプトを追加vitest/vite/jsdom追加に伴うyarn.lock更新
File summaries
| File | Description |
|---|---|
| yarn.lock | vitest/vite/jsdom 追加に伴う依存解決結果の反映 |
| vitest.config.ts | Vitest projects による unit/component/db の環境分離設定を追加 |
| package.json | typecheck/test/test:db/gen:types のスクリプトと関連devDependenciesを追加 |
Review details
- Files reviewed: 2/3 changed files
- Comments generated: 0
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
レビュー結果: 承認寄り(ブロッキングな指摘なし)概要
docs/ とCLAUDE.mdへの適合確認
補足(ブロッキングではない観察)
全体として、ドキュメントで定義された方針(特に |
* docs: レビューフローの見直し(Draft先行 + CodeRabbit導入) PR #18〜#32の実績分析(Claude/Copilotの指摘重複率、Copilotのクレジット 消費)を踏まえた運用変更。 - PRはまずDraftで作成し、Claude/Codex/CodeRabbit(無料)と反復してから Ready化してCopilot(有料)の最終レビューを受ける運用をCLAUDE.mdに明記 (Copilotのrulesetは既にreview_draft_pull_requests: falseのため ruleset自体の変更は不要だった) - .coderabbit.yamlを追加。デフォルトでdraftをスキップする設定を drafts: trueで上書きし、CLAUDE.mdの絶対ルール・RLS・test/dbの 観点をpath_instructionsで明示 - docs/roadmap.mdの「保留: 外部アカウント待ち」にCodeRabbit導入を追記 - docs/permissions.mdに、INSERT/UPDATE/DELETEのRETURNINGがSELECTポリシー で空になるだけでは「弾かれた」証明にならない、という#26で見つかった 罠を追記 * fix: CodeRabbitの初回レビュー指摘に対応 - auto_pause_after_reviewed_commits: 0 を追加(既定5コミットで自動レビューが 止まる設定は反復push運用と相性が悪い) - CLAUDE.mdの記述を.coderabbit.yaml(drafts: true)と一致させる。 「Copilot(導入後はCodeRabbitも)の最終レビュー」という誤った表現を修正し、 CodeRabbitもDraft中から反復レビューする旨を明記 - CodeRabbit Freeプランの実際のレート制限(GitHub連携PRレビューは1回/時/開発者) をCLAUDE.md・docs/roadmap.md・.coderabbit.yamlに反映。前回調査した 「200ファイル/時・4レビュー/時」は誤りだった(CodeRabbit自身のレビュー コメントがPostgreSQL公式ドキュメントとCodeRabbit公式docsをその場で クロールして訂正) - docs/permissions.mdのRETURNING説明をPostgreSQL公式ドキュメントに基づき 修正。UPDATEはSELECTポリシー不一致だと実際はエラーになり(空配列ではない)、 DELETEはSELECTポリシーを満たさない行がそもそも削除候補から除外される、 というのが正確な挙動 - markdownlint MD040(コードフェンスの言語識別子)を修正 - .coderabbit.yamlのpath_instructionsを強化(security definerの安全条件、 操作別のRLS否定側テスト観点) * fix: CodeRabbit 2回目レビュー指摘を反映 - CLAUDE.md: レビュー実行条件をツールごとに正確化(Codexはシークレット設定時、CodeRabbitはFreeプランのレート制限対象) - docs/permissions.md: select()のコード例をdata/errorの分解に修正、UPDATE/DELETEのSELECTポリシー要求条件を限定 * fix: CodeRabbit 4回目レビュー指摘を反映(Claudeレビューのトークン条件を明記) CLAUDE_CODE_OAUTH_TOKEN未設定時はClaudeレビューがスキップされる旨が 記述から抜けており、実際のワークフロー(claude-review.yml)の挙動と 矛盾していた。Codex/CodeRabbitと同様に条件を明記する。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: Claude 3回目レビュー指摘を反映(未検証事項の明記) roadmap.mdがCopilotのdraft/ready挙動を「確認済み」と断定していたが、 このPR自体のTest planは未検証のままだった。CodeRabbitのDraft動作の 確認と、Copilotの挙動(CLAUDE.mdの既存記載を踏襲・未検証)を書き分けた。 .coderabbit.yamlのauto_pause_after_reviewed_commits: 0も、5コミットを 超えた継続の実地確認がまだであることをコメントに追記した。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: CodeRabbit 5回目レビュー指摘を反映(Planの食い違い・INSERT否定テスト観点) ## CodeRabbit 5回目レビューの指摘分類 (commit 4ef0ec8) - CodeRabbitのレビュー実行結果が毎回`Plan: Pro Plus`と表示されており、 CLAUDE.md/roadmap.mdが前提としていた「Freeプラン・1回/時のレート制限」 と矛盾していた。GitHub App導入直後のトライアル期間中の可能性が高いため、 本物の修正として注記を追加した(トライアル終了後にFreeプランの制限が 実際に効き始めたら更新する)。 - .coderabbit.yamlのtest/db/**向けpath_instructionsで、INSERTの否定側 テストが「対象行を見られる側の視点で」新規行が作られていないことを 確認する、という条件が抜けていた(UPDATE/DELETEは明記済み)。妥当な nitpickとして、docs/permissions.mdの既存記述と揃える形で修正した。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: Claude 4回目レビュー指摘を反映(RETURNING/USING句の自己矛盾を修正) ## Claude 4回目レビューの指摘分類 (commit c68ebb4) - docs/permissions.md:88-99が自己矛盾していた。「UPDATE...RETURNINGは SELECTポリシー不一致でエラーになる」と書きながら、直後で同じ状況を 「エラーも出ない」と説明し、コード例もtry/catchなしでdata=[]を期待して いた。実際のexpenses_update_ownポリシー(using (user_id = auth.uid())) で検証すると、他人の行を更新しようとするケースはUSING句で静かに除外 される(エラーなし・0件)。「更新後の行がSELECTポリシーを満たさない とエラー」は別の仕組みで、UPDATE/SELECTポリシーの条件が食い違う場合 にのみ起こる、より狭いケース。本物の修正として2つの仕組みを分けて 説明し直した - docs/roadmap.mdの「Copilotのdraft/ready挙動はCLAUDE.mdの既存記載を 踏襲」という記述が誤り(base branch時点のCLAUDE.mdにCopilotへの言及は なく、このPRで新規に持ち込んだ主張だった)。妥当な指摘として、 Rulesetの設定自体は既知の事実である一方、挙動の実地確認はまだして いない、という位置づけに書き直した Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: Copilot初回レビュー指摘を反映 + auto_pauseの実地検証結果を反映 ## Copilotレビューの指摘分類 (commit 2ec236b) - docs/permissions.md:112 | 妥当なnitpick | コード例の`元の値`が未定義識別子で コピペ不可だった。事前に本人視点で値を控える`before`変数を追加し、それと 比較する形に修正した - docs/permissions.md:84/94/108 | 妥当なnitpick | `.select()`の戻り値`data`は nullになり得るのに`toHaveLength`を直接呼んでいた。`data ?? []`で安全にした - docs/permissions.md:98 | 妥当なnitpick | UPDATE...RETURNINGのエラーを 「USING句とは別の仕組み」「WITH CHECKとの関係」まで断定していたのは 実装の内部機構に踏み込みすぎだった。PostgreSQL公式ドキュメントの一次情報の 引用に留める形にシンプル化した - .coderabbit.yaml:18 | 見送り(実地検証で解決) | 「0が無制限として機能するか 未確認」という記述に対し、Copilotは公式ドキュメント通り断定すべきと指摘。 実際にPR #35で7コミット目push後に`@coderabbitai review`を手動実行したところ 「Review rate limited」という応答で、「pauseされている」旨の案内では なかった。つまりauto_pause自体は意図通り無効化されており、レビューが 止まった原因はレート制限だと実地で確認できた。この結果をコメントと docs/roadmap.mdの注記に反映した Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: Claude 5回目レビュー指摘を反映(Copilot挙動の実地確認結果を記載) ## Claude 5回目レビューの指摘分類 (commit 509c11f) - docs/roadmap.mdの「Copilotのdraft/ready挙動は未検証」という記述が、 このPR自身のレビュー履歴(Ready化後にcopilot-pull-request-reviewerが 3回動いた記録)と食い違っていた。本物の修正として、Draft中は一度も Copilotのレビューが付かず、Ready化後に動いたことを実地確認済みとして 明記した - CLAUDE.mdに、CodeRabbitのレート制限は詳しく書かれているのに Copilotのquota上限による失敗モードがどこにも記載されていなかった。 本物の修正として、quota上限で失敗した場合の再リクエスト手順を追記した Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: Copilot 2回目レビュー指摘を反映(dataのnullフォールバックがエラーを隠す) ## Copilot 2回目レビューの指摘分類 (commit 40bf55d) - docs/permissions.md:81/85/108,112 | 本物の修正 | 前回追加した`data ?? []` というnull安全化が、実はerrorが発生してdata:nullになったケースを 「0件」として静かに握りつぶしてしまう、というCopilotの指摘は的確だった。 「否定側テストが何も検証していない」ことへの警戒を説くドキュメント自身が 同じ落とし穴を持っていたことになる。`?? []`を外し、`error`がnullである ことを明示的に検証する形に修正した Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: Copilot 3回目・Claude 6回目レビュー指摘を反映(error検証の一貫性) ## レビュー指摘分類 (commit 9447f05) - docs/permissions.md:110/119 | 本物の修正(Copilot) | 前回`error`検証を 追加したUPDATE否定側テスト例で、同じブロック内の`before`/`unchanged` 取得(`.single()`)にだけerror検証が抜けていた。一貫性のため全クエリに error検証を追加した - docs/roadmap.md:89 | 妥当なnitpick(Claude) | 「プレミアムリクエスト13回 相当」という数値に出典がなかった。公式の固定値ではなく実測値である旨を 明記した Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
PR #68 のClaude Reviewの指摘に対応。指摘は正しかった。 誤り: 「300行を超えた5本」は事実と異なり、素の差分では9本だった (#31 307 / #59 365 / #60 380 / #30 441 / #29 617 / #18 679 / #41 698 / #32 1063 / #16 3528)。同じ本文が書いていた「300行以下が65%(17/26)」は 超過9本を含意しており、記述が自己矛盾していた。 原因: 母集団の統計(中央値・65%)は素の差分で数え、外れ値の説明だけに 除外規則を適用していた。数え方を混在させたうえ、超過リストを上位5本で 打ち切って全件確認しなかった。 訂正: 26本すべてを git show --numstat で数え直し、除外の段階ごとに 表で示す。素の差分(中央値135行 / 65% / 超過9本)、パス名で機械的に 判定できる除外まで(中央値128行 / 85% / 超過4本)、除外規則を最後まで 適用(超過2本 = #59・#60)。落ちる7本の内訳も明記した。 あわせて、3段階目の中央値を出さない理由を書いた。supabase/config.toml は #29 では supabase init の出力(416行)、#56 では根拠コメント付きで手で 直した6行で、同じパスでも扱いが逆になる。パス名では決まらないことが、 この節が機械的ゲートになり得ない理由そのものなので、「lintではない」の 段落の根拠もこの実測に差し替えた。 Refs #44
* docs: Issueの粒度とPR差分サイズの目安をCLAUDE.mdに明文化 判断ポイントは1 Issueに3個まで(5個超で分割)、PR差分は300行を目安とする。 記事の実測値をそのまま採らず、main にマージ済みのPR 26本(中央値135行、 300行以下65%)で裏を取ってから採用した。行数の数え方から生成物・ロック ファイル・権限マトリクスを写したテスト表を除外する根拠も、超過した PR #16 / #32 の実態から示した。 機械的ゲートにしない旨と、3回ルール(モデルを上げる) / PO確認(判断を 下せる層に上げる) / 粒度超過(Issueを分ける)の対処の違いを表で整理。 docs/roadmap.md はポインタ1行に留め、根拠は1箇所にだけ置く。 Refs #44 * docs: 判断ポイント数の境界を一本化し、裏取り済みの数値と外部実測を書き分ける PR #68 のClaude Reviewの指摘2件に対応。 指摘1: 「3個まで、5個を超えるなら分割」で4個の扱いが未定義だった。 閾値を「3個まで。4個目が出てきたら分ける」に一本化する。機械的ゲートに しない方針である以上、「検討」と「必ず」の二段構えは実効性のない false precisionになるため、緩衝域を作らず単一の線にした。 指摘2: 裏取り済みの300行と、外部実測のままの3個が同じ文脈に並んでいた。 「2つの数字は裏付けの強さが違う」として段落を分け、300行はこのリポジトリの 実測(PR 26本、中央値135行/300行以下65%)で検証済み、3個は外部実測のみを 根拠とする未検証のヒューリスティックであると明示した。過去Issueの判断数は 記録がなく後から数え直せないため、このリポジトリでの裏取りが今はできない 理由も併記。採用の根拠はコストの非対称性に置いた。 あわせて、外部実測に対応値のない4個/5個を推定して線を引いていないことと、 実績が溜まったら見直す旨を記載した。 Refs #44 * docs: PR実測の集計を数え直し、超過本数の誤りを訂正する PR #68 のClaude Reviewの指摘に対応。指摘は正しかった。 誤り: 「300行を超えた5本」は事実と異なり、素の差分では9本だった (#31 307 / #59 365 / #60 380 / #30 441 / #29 617 / #18 679 / #41 698 / #32 1063 / #16 3528)。同じ本文が書いていた「300行以下が65%(17/26)」は 超過9本を含意しており、記述が自己矛盾していた。 原因: 母集団の統計(中央値・65%)は素の差分で数え、外れ値の説明だけに 除外規則を適用していた。数え方を混在させたうえ、超過リストを上位5本で 打ち切って全件確認しなかった。 訂正: 26本すべてを git show --numstat で数え直し、除外の段階ごとに 表で示す。素の差分(中央値135行 / 65% / 超過9本)、パス名で機械的に 判定できる除外まで(中央値128行 / 85% / 超過4本)、除外規則を最後まで 適用(超過2本 = #59・#60)。落ちる7本の内訳も明記した。 あわせて、3段階目の中央値を出さない理由を書いた。supabase/config.toml は #29 では supabase init の出力(416行)、#56 では根拠コメント付きで手で 直した6行で、同じパスでも扱いが逆になる。パス名では決まらないことが、 この節が機械的ゲートになり得ない理由そのものなので、「lintではない」の 段落の根拠もこの実測に差し替えた。 Refs #44 * docs: PR #29/#32の除外理由が2段階なのに1段階しか書いていなかった記述漏れを修正 Claude Reviewの指摘どおり、#29(617→22)はsupabase/config.toml(416行) だけでなくsupabase/types.ts(179行)も、#32(1063→79)はテスト表(924行) だけでなくyarn.lock(60行)も除外して初めて数字が再現できる。 片方しか書いていなかったため、追試すると数値が合わなかった。 Refs #44
概要
Issue #3 の対応。Vitestを
test.projectsで3つに分離し、package.jsonにスクリプトを登録した。test/unit/— environment: nodetest/component/— environment: jsdomtest/db/— environment: node(Supabaseローカル起動が必要。中身は空のまま)変更内容
vitest.config.tsを新規追加。projectsオプションで3 environmentを分離passWithNoTests: true(中身が空のprojectでも成立させるため)package.jsonにスクリプトを追加:typechecktesttest:dbgen:typesyarn test→vitest run --project unit --project componentyarn test:db→vitest run --project dbvitestvite(vitest 4のpeer dep)jsdomを追加確認したこと
yarn test/yarn test:dbがSupabase起動なしで通る(中身が空でも設定として成立)test/unitとtest/dbにわざと失敗するテストを一時的に置き、yarn testが unit の失敗を検出し、db を実行しないことyarn test:dbが db の失敗を検出することを確認してから削除した(赤くなることの確認 → 復元)
yarn lint/yarn typecheck/yarn test/yarn test:dbが全て通る補足
vitest.config.tsは.mtsにするとeslint-config-next側のパーサー割り当てが.mtsに対して@typescript-eslint/parserに戻らず、型情報が必要なルールが壊れるため.tsのまま。Vite native config loaderのESM/CJS警告は出るが実害はない(exit 0)Test plan
yarn lintyarn typecheckyarn testyarn test:db