code-review

コードレビュー

以下の原則に照らして、差分または指定されたファイルをレビューしてください。

1. 名前は現在の実態を表す必要があります

• 変数名と関数名は、以前の状態ではなく、現在の役割を表す必要があります。
• 基盤となる仕組みが変わった場合（例：FUSE → NFS）、関連するすべての名前も更新する必要があります。
• この名前を見た新しい読者は混乱しないでしょうか？

2. 古い参照を残さない

• リファクタリング後は、古いアプローチへの参照を grep で検索してください。不要になった検出ロジック、廃止された機能フラグ、削除したコードに言及するコメントなどが対象です。
• 試してから取りやめたものは、痕跡をすべて削除してください。コードベースは、現在のアプローチが最初から計画されていたかのような状態にする必要があります。

3. 検出ロジックとガードロジックを簡素化する

• 「この機能を実行できるか」という判定では、本当に重要な1つの条件だけを確認してください。
• 1つのチェックで十分なのに、代替の検出方法を連鎖させないでください（バイナリが存在する、またはソースが存在する、またはツールチェーンが存在する、など）。

4. コメントでは「何が起きたか」ではなく「なぜそうするのか」を説明する

• 残す： 技術的に自明でない発見、プラットフォーム固有の癖、「これを削除すると、Y が原因で X が壊れる」という説明。
  • 良い例：// com.apple.provenance causes SIGKILL when spawned as child process
  • 良い例：// umount while server is alive panics the macOS NFS client
  • 良い例：// NFS client uses cookie verifier to decide if cached readdir is valid
• 削除する： 試したこと、取りやめたこと、名前を変更したことの経緯。
  • 悪い例：// We changed this from cp to cat to work around the provenance issue
  • 悪い例：// Wrapper added because FUSE-T was zero-padding (see commit abc123)
  • 悪い例：// Previously this was called fuseAvailable but we renamed it
• 削除する： 理由を加えず、コードの動作をそのまま言い換えているコメント。
• 多くの場合、コメントは必要ありません。

5. インストール時と実行時を分ける

• インストールスクリプトでは、前提条件（ツールチェーン、システム依存関係）をセットアップします。
• 実行時のコマンドでは、コンパイル、キャッシュ、バイナリ管理を行います。
• これらを混在させないでください。ランタイムでもコンパイルするバイナリをインストールスクリプトでコンパイルしているなら、どちらかが誤りです。

6. 中間生成物を残さない

• 複数のアプローチを試した後は、つなぎとしてのみ存在していたロジックがないか確認してください。
• アプローチ A 向けに回避策を追加し、その後アプローチ B に切り替えたなら、その回避策が無害でも削除してください。
• 正しい答えを最初から知っていた人がゼロから書いたかのように、コードを読める状態にしてください。

7. 変更後の保証を厳密にする

• 変更によって何かが保証されるようになった場合（例：変数が常に設定される、ファイルが必ず作成される、関数が必ず値を返す）、下流のコードが古い「そうとは限らない」状態に対するガードを残していないか確認してください。
• もはや null にならない値に対する冗長な null チェック、フォールバックのデフォルト値、if (x) ガードは誤解を招きます。存在しない可能性を示唆してしまうためです。
• 再代入が不要になったら、let より const を優先してください。
• シグナルを決して抑制せず、根本原因を修正してください。 未使用パラメーターの _ プレフィックス、// @ts-ignore、eslint-disable、as any は、いずれも本当の問題を隠してしまいます。パラメーターが未使用なら削除し、すべての呼び出し元からも連鎖的に削除してください。型が一致しないなら、型を修正してください。チェッカーを実行し、すべてのエラーを修正して、再度実行してください。多数のファイルにまたがる機械的な変更こそ、エージェントが得意とすることです。「呼び出し元が多すぎる」ということはありません。

8. 変更が絡み合っている場合は、最初からやり直す

• ファイルに相反する編集が3回以上加えられている場合は、git restore で元に戻し、必要な変更だけを再適用してください。
• 混乱した状態を外科的に修正しようとしないでください。最初からやり直す方が速く、ミスも少なくなります。

9. テストではコレクションのサイズだけでなく、実際の値を検証する

• 重複排除、正規化、冪等な操作をテストするときは、toHaveLength(1) だけでなく、保存された値をアサートします。
• 長さだけではロジックが正しく動作した証明にはなりません。正規化処理に不具合があると、入力の一方が気づかないうちに破棄されたり、異なる形式に正規化された値がたまたま一致したりする可能性があります。
• パターン：形式 A の値を追加し、形式 B でもう一度追加してから、件数が 1 であることと、保存された値が期待する正規化済み形式であることの両方をアサートします。
• 悪い例：expect(data.phones).toHaveLength(1) — 正規化に不具合があってもパスします
• 良い例：expect(data.phones).toHaveLength(1); expect(data.phones[0]).toBe('+12125551234') — 両方の形式が正しく認識されたことを証明します

10. 薄いラッパーや再エクスポートだけのモジュールを作らない

• モジュールが別のパッケージの関数や型をそのまま転送するだけなら、そのモジュールを削除し、元のパッケージから直接インポートします。
• スキャフォールドコードや未リリースのブランチ作業のために、ローカルの互換性シムを残さないでください。スキャフォールドをそのまま置き換えます。
• 再エクスポート用のバレルは、プロジェクト固有の意味を持つ実質的な公開境界を定義する場合を除き、役に立ちません。export * from ... だけを役割とするファイルは作らないでください。
• loadSkills() に対する discoverAll() のようなローカルの別名を考案するより、上流 API の名前をそのまま使うことを優先します。

11. 実際の責務の境界に沿ってのみ切り出す

• 行数を減らすためだけにファイルを分割しないでください。新しいファイルには、読み手が名前を付けられる、一貫した責務を持たせます。
• 関連するポリシー、状態遷移、リソース処理、ドメイン固有の振る舞いがまとまっている場合に、モジュールを切り出します。
• 呼び出し側で使い方を理解するために細部をすべて把握する必要があるなら、単発のヘルパー関数を新しいファイルに移さないでください。
• 適切な分割では通常、責務とともに依存関係も移動するため、元のファイルに対するインポートの負荷が軽減されます。
• 切り出しによって所有者が明確にならず、間接性だけが増える場合は、コードをその場に残します。

12. テストを実装から切り離す — システムをエンドツーエンドで動かす

最も価値のあるテストスイートとは、対象の実装から最も切り離されたものです。バックエンドをフロントエンド経由で動かし、モジュールをユーザーと同じエントリーポイントから実行するほど、切り離してください。内部を直接調べるテストは内部構造を固定してしまいますが、公開されたインターフェースを通じて動かすテストなら、その下にあるすべてを自由にリファクタリングできます。

• 高速で決定的な結果が得られる範囲で、最も外側のエントリーポイントを優先してください。 CLI バイナリ ＞ 最上位のエクスポート関数 ＞ 内部ヘルパー ＞ プライベートメソッド。ユーザーが CLI を実行するなら、CLI を対象にテストを書いてください。ユーザーが TUI を見るなら、実際のキー入力と同じ入力パイプラインを通じて操作し、中間状態ではなく描画されたフレームを検証してください。
• 構造ではなく動作をテストしてください。 終了コード、標準出力、描画された出力、ディスク上のファイル、HTTP レスポンス、永続化された行など、観測可能な結果を検証してください。どの関数がどの順番で、どのような中間データを使って呼び出されたかは検証しないでください。それらは自由に変更できるべき実装の詳細です。
• テストが成功したなら、実際のユーザーも正しい結果を得られるべきです。 本番経路が壊れていても成功するテストは、配線が間違っています。よくある問題の兆候として、テスト対象をスタブ化する、ルーター／ディスパッチャー／パーサーを迂回する、本番環境なら入力から生成される内部イベントを手作業で構築する、といったものがあります。
• ハーネスは、本番環境と同じようにシステムを配線する必要があります。 実際には常に設定されるオプション入力は、契約の一部です。同等の入力をハーネスにも渡してください（例：確定済みステータスのストリーム、モーダルの設定、同じ環境変数）。実環境で実行したときにしかバグを発見できなかったなら、ハーネスでは本番環境が設定する何かを省略しています。同じ形のリグレッションを次に検出するのがユーザーではなくテストスイートになるよう、ハーネスを修正してください。
• 密結合したテストはリファクタリングの負担になります。 内部関数の名前変更やモジュールの移動だけで、ユーザーに見える動作は変わらないのに何十ものテストが壊れるなら、テスト対象のレイヤーが間違っています。それらのテストを外側のインターフェースに対するものに書き換え、壊れやすいテストは削除してください。
• エンドツーエンドまたはシステムレベルのリグレッションテストを基本にしてください。 フィードバックを速めることで開発が大幅に効率化される、特定の複雑なビジネスロジックに限り、ユニットテストを使ってください。

13. チェックの失敗はコードの欠陥ではなく、ツールチェーンの不具合かもしれません

型を書き足す、キャストを追加する、値を固定する、またはコードを再構成してチェッカー（型チェッカー、コンパイラー、リンター、テストランナー、ビルド）を通す前に、その失敗がコードの欠陥であり、ツールチェーンや環境の問題ではないことを確認してください。壊れている、あるいはバージョンが一致しないツールを通すためにコードを修正するのは、本当の問題を覆い隠す回避策です。これは、より上位の「シグナルを抑制しない」ルールに反します。

• 失敗を報告したツールチェーンとまったく同じ環境で再現してください。 ローカル環境だけでなく、CI／デプロイで使われるバージョンと設定を使ってください。異なるバージョンで実行したなら、ローカルでチェックが成功しても何の証明にもなりません。固定されたプレリリース版、プレビュー版、または手元と CI のバージョンのずれがあると、同一のソースでもローカルでは成功し、CI では失敗する（またはその逆になる）ことがあります。
• 同じソースが実際に使われる固定済みツールで成功するなら、コードは正しいです。 修正すべきなのはコードではなくツールチェーンです（バージョンを固定する、設定を修正する）。出力が曖昧な場合は、原因を推測するのではなく、ツールに直接問い合わせて、実際に計算した値、型、エラーを出力させてください。
• ローカル環境と CI を一致させてください。 マージ／デプロイを左右するチェックで、デプロイと同じツールチェーンが使われていることを確認してください。バージョンのずれがあると、成功したチェックはすべて疑わしくなり、「ローカルでは通った」ことはデプロイも成功するという根拠ではなくなります。

14. アイデンティティは認証情報から取得し、呼び出し元からは決して取得しない

• 操作を行うアイデンティティ（組織／テナント／ワークスペース ID、ユーザー ID、アクター）は、呼び出し元が指定した引数ではなく、認証済みリクエスト（API キー、セッション、または検証済みの内部キー）から解決します。引数の actorId／orgId を参照する公開ミューテーションは、機能ではなくなりすましの抜け道です。
• 唯一の例外は、信頼された内部呼び出しです。有効な内部キーがある場合に限り、呼び出し元が指定したアクターを受け入れます。それ以外の場合は、認証情報からアクターを導出し、指定された ID は拒否します。
• 認証済みスコープ内の子要素（特定のチャンネル、共有、アプリ）を対象にする場合は、_子要素_の ID を受け取り、それが認証情報から導出した親要素に属することを検証します。リクエスト本文に別途指定された親 ID は信頼しないでください。

15. 公開 API アクションは、認証とディスパッチに徹し、ビジネスロジックを直接記述しない

• 公開ルートでは、認証、検証、アイデンティティの導出を行い、処理を担当するモデル関数または内部アクションにディスパッチします。このラッパーは薄く保ち、認証／ランタイム境界として機能させます。
• 「たった 3 行だから」と、サードパーティ SDK／分析／メールのロジックを公開境界に直接記述しないでください。ディスパッチの先に置きます。
• 特に Convex では、1 つのアクションを満たすためだけに、複数のアクションを含むファイルに 'use node' を追加してはいけません。ファイル内のすべてのアクションが Node ランタイムで実行されることになります。分割してください。認証＋ディスパッチ（デフォルトのランタイム）→ services/<thing>.ts（'use node' を指定し、SDK 呼び出しを担当）。

16. 公開エンドポイントは必要最小限の範囲で提供する

• 1 つのルートにつき、1 リクエストで 1 項目を処理します。具体的に 2 つ目の呼び出し元が必要とするまでは、先回りして /batch のバリエーション、ページネーション付き一覧、フィルターパラメーターを追加しないでください。
• スキーマはルート内に記述します。再利用する箇所がないのに、4 行のスキーマを「再利用のため」に共通の schemas.ts に切り出さないでください。リクエスト本文が実際に無制限でない限り、本文サイズの上限も設けません。

17. 既存の型を再利用し、再宣言せずに導出する

• 型がすでに上流（スキーマバリデーター、プロトコルパッケージ、生成されたクライアント、SDK）に存在する場合は、それを直接使い、形状を一部でもインラインで再宣言しないでください。再宣言した形状は既存のものから乖離します。
• バリエーションが必要な場合は、Pick／Omit／Partial／Parameters／ReturnType／インデックスアクセス／Extract／Exclude で導出します。Convex／Zod の場合は、Infer<typeof V>、FunctionArgs<typeof api.x.y>、z.input/z.output に相当します。
• 悪い例：既存の User の隣に type UserSummary = { id: string; name: string; email: string } を定義する。良い例：Pick<User, 'id' | 'name' | 'email'>。

18. インデックスでフィルタリングし、take() の後にフィルタリングしない

• リストクエリでテーブル内のサブバケット（アクティブとアーカイブ、status === X など）の行を取得する場合は、インデックスでフィルタリングする必要があります。フィルタリング前のクエリに .take(N) を適用し、取得後に JavaScript で必要なバケットを .filter() するのは避けてください。
• これが問題になる理由：.take(N) は、インデックスの並び順で先頭の N 行を返します。先頭が別のバケットの行で埋まっていると、対象のバケットに古い行が何百件あっても、取得後のフィルタリング結果は 0 件になります。take(N*2) にしても、問題の発生を遅らせるだけです。
• パターン：識別子をインデックスに含め、.withIndex(..., q => q.eq(...)) を使うか、「存在する」バケットには q.gt(field, 0) を使います（Convex では undefined は定義済みの値より前に並びます）。.first()／.unique() や、「X があるか」という確認にも同じことが当てはまります。

19. 決して .collect() を使わず、.take(N) で読み取り件数を制限する

• Convex の .collect() は、上限なしで一致するすべての行を読み取ります。時間とともにテナントごとのデータが蓄積するもの（セッション、イベント、台帳行、監査行）では、初日は問題のないクエリでも、いずれ 1 回のリアクティブな更新で何千行も取得するようになり、クライアント全体のリアクティビティを気づかないうちに低下させます。
• .collect() は、現在の作業セットを明確に上回る安全上限（100、1000）を N とした .take(N) に置き換えます。これは異常なデータ量に対する上限であり、UX のページネーションではありません。.withIndex(...).filter(...).collect() にも同じことが当てはまります。
• すべての行が本当に必要な場合（マイグレーション、管理者によるダンプ、単発のメンテナンス）は、その旨をコメントに記し、キャンセル可能なミューテーション／アクションから実行します。リアクティブな query から実行してはいけません。

20. 名前の変更やデフォルト値の設定だけを目的に行を再マッピングしない

• すべてのフィールドを引き継いで名前を変更したり、undefined → false にまとめたりするオブジェクトリテラルの .map() はノイズです。マップの更新が行われるまで、新しい列が黙って欠落します。利用側がすべてのフィールドを必要とするなら行をそのまま返し、一部だけ必要なら Pick／Omit を使うか、分割代入と rest を使います。新しい名前によって明確さが実質的に増す場合にのみ名前を変更し、後続処理が値の欠落を本当に扱えない場合にのみデフォルト値を設定します。
• ストレージ専用のフィールド（_id、_creationTime、内部 ID）は、分割代入と rest を使って一度だけ取り除きます。よい例：return rows.map(({ _id, _creationTime, ...row }) => row)。

21. 内部 API にバージョンや互換性の仕組みを持ち込まない

• 送信側と受信側の両方を自分たちが管理している場合（自社のアプリ、サービス、関数など）、protocolVersion フィールド、バージョンネゴシエーション、機能フラグ、「相手側が古い場合に備える」分岐を追加してはいけません。デプロイがバージョンです。
• 唯一の実質的な互換性の軸はバージョンではなくフィールドです。これは調整用の設定ではなく、非対称性です。リクエストは厳密に解析し（未知のフィールドを拒否）、レスポンス／イベントは寛容に解析します（未知のフィールドを無視します）。これにより、コンポーネントがそれぞれのスケジュールで更新されるという、実際に起こりうる唯一のバージョンずれに対して、受信側を同時にリリースせずに送信側がフィールドを追加できます。
• 要注意な例：誰も読み取らないワイヤースキーマのバージョン定数、自分たちでデプロイするコードだけが呼び出す if (payload.v >= 2) 分岐、レスポンスに対する厳密なパーサー（サーバー側のフィールド追加がすべて破壊的変更になってしまいます）。
• 例外：再デプロイできない利用者がいる真の外部 API では、バージョンをフィールドごとではなくルートで管理します（/v1/）。

22. 一致している必要があるものには、単一の管理元を持たせる

• 2つの宣言の整合性を保つ必要がある一方、それぞれが単独で変更されうる場合、内容にずれが生じます。共通の判断を1か所にまとめ、各箇所ではそこから組み立てます。別の場所にある一連の値を言い換えるだけの並行する列挙型、選択肢リスト、switch が典型例です（形状の導出についてはルール 17 を参照してください。ここでは、一致させる必要があるあらゆる値を対象とする、より広いルールを示します）。
• さらに難しい点として、よく似ていても正当に異なるものについては、その違いをコード内で説明してください。それ以外は同一に見えるもの同士の違いに説明がなければ、ずれによるバグと区別できません。読む人には、意図的な違いなのか見落としなのか判断できません。

23. 名前に価値がない限り、一度しか使わない値を名前付きの const に切り出さない

• モジュールスコープに切り出した const がちょうど1か所でしか参照されない場合、得るものがないまま間接性が増します。値を知るために宣言まで移動する必要があり、その名前もインライン値と短いコメントで十分に伝わる内容を言い換えているだけです。1か所だけの呼び出し箇所にインラインで記述し、WHY はコメントで説明します。
• ただし、以下のいずれかに当てはまる場合は、一度しか使わない名前付き const にも意味があります。
  • 頻繁に変更される設定値である、または運用担当者／読む人が調整することを想定している（調整項目としてまとめたタイムアウト／上限値、機能のしきい値、調整対象のリトライ予算など）。
  • 関連する const の隣に置かれ、そのまとまりによって意味が生まれる（*_TIMEOUT_MS のブロック、上限値の表、同じ列挙型のメンバーなど）。このまとまり自体が説明になります。
  • 独立して宣言することに構造上の意味がある。モジュールの公開 API の一部としてエクスポートされる、型から参照される、または値を適用するデータ／ファイルと並べて置かれているため、将来の2つ目の呼び出し元が見つけられる、といった場合です。
• それ以外はインラインにします。レビュー時の判断基準：名前が一度だけ使われるリテラルにラベルを付けるためだけに存在し、頻繁に変更されるものでも関連項目の隣にあるものでもなければ、それはノイズです。値の理由を説明するコメントを添えて、呼び出し箇所にまとめてください。
• 悪い例：const STOP_SESSION_CONNECT_TIMEOUT_MS = 5_000 を単独で宣言し、runAction({ connectTimeoutMs: STOP_SESSION_CONNECT_TIMEOUT_MS }) の1か所だけで使う。
• よい例：呼び出し箇所に connectTimeoutMs: 5_000, // 5s: a cold sandbox must not hang on the 60s default connect と記述する。

24. バッチループでは項目ごとに失敗を分離する — 1つの不良項目がほかの項目を滞らせることはありません

• 独立した作業項目を処理するループ（スイープ、cron バッチ、キューの排出、フリートの一括処理、ファンアウト）では、項目ごとに例外を捕捉して処理を続ける必要があります。失敗を結果（failed リスト）にまとめるか、ログに記録します。項目ごとの例外を捕捉しないとパス全体が中断されます。また、ランナー（cron、スケジューラー、同期処理）が次のティックで同じ順序のセットを再選択するため、決定論的に失敗する項目が恒久的なヘッドオブラインブロッカーになります。後続の項目はすべて永久にキューへ戻される一方で、メトリクスにはジョブが「再試行中」と表示されます。
• トランザクションのコンテキスト（たとえば、ページを処理してカーソルを進めるデータベースの変更）では、状況はさらに悪化します。捕捉されない例外によってカーソルや進捗の書き込みもロールバックされるため、同じページが永久に再実行されます。エラーを捕捉すればトランザクションは継続されますが、失敗した項目が例外を投げる前に行った書き込みは保持されます。そのため、項目の境界で捕捉するのは、その書き込みが冪等であるか、安全に再実行できる場合に限ります。
• 失敗時に早期終了してよいのは、共有の下流サービスに到達できない場合など、失敗がバッチ全体に影響することが確実なときだけです。項目固有のエラーでは早期終了してはいけません。呼び出し元が失敗を把握する必要がある場合は、エラーを捕捉して記録しながら処理を続け、パスの完了後に最初のエラーを再スローします。
• 失敗を分離していても、順序に関する落とし穴に注意してください。成功した項目が候補セットに残る場合（たとえば、再試行のたびにブロッカーより前のすべての項目を再スナップショットする場合）、スイープの選択対象から処理済みの作業を除外する必要もあります。
• 採用できる形：項目ごとの try/catch と、結果に含める failed。項目をキーにした Promise.allSettled。エラーを捕捉して記録しながら処理を続け、パスの完了後に再スローする方法。
• 悪い例：1時間ごとの cron 内で for (const ws of candidates) { await archive(ws); await mark(ws) } を実行すること — 到達不能な VM が1つあるだけで、その後ろのすべての候補が永久に停止します。
• 良い例：同じループで、項目ごとの処理全体を try/catch で囲み、{ workspaceId, error } を failed 配列に追加して呼び出し元へ返す方法。

25. ユーザーに見える名前を変更するなら、テストに見える名前も一掃する

• ユーザー向けの文言、アクセシブルネーム、ルート、data-* 属性は、ゲートで実行されないテスト層（ライブステージング用ハーネス、ジャーニーテストスイート、外部モニター）にとっても重要です。すべてのローカルゲートを通過する変更でも、名前を変更すれば、数時間後に別の人のセッションで気づかれないまま問題が起きることがあります。
• 変更によって表示文言の名前を変更したり、画面構成を変えたりする場合は、リポジトリ全体を対象に旧文字列や旧セレクターを grep します。テストハーネスや e2e パッケージも明示的に含め、同じ作業でアンカーをすべて更新します。名前を変更する差分で、関連箇所も一掃します。
• レビュアー向けの確認方法：差分から変更した文字列を1つ選び、リポジトリ全体で grep します。差分の外に一致箇所が残っていれば、次のライブ実行で問題が起きる可能性があります。
• 表示文言より構造を優先します。ジャーニーテストにアンカーが必要な場合は、表示テキストではなく、画面上に専用の data-* 属性を設けることを優先します。そうすれば、テストに触れずにプロダクトの文言を自由に変更できます。構造属性がすでにあるのに表示文言をアンカーにしているジャーニーテストにはフラグを立てます。

26. 見つけたときよりコードをよくする — 差分を整えるためにリポジトリを乱雑なままにしない

• 差分を絞るために、付随して行われた改善を元に戻してはいけません。 フォーマッター、リンター、またはコードモッドによって、変更箇所以外のファイルも整理された場合は、それもコミットしてください（差分が大きくなる場合は、別のコミットにしてください）。元に戻すと、レビュー時の差分の見た目は整いますが、その後の作業のたびに同じコストがかかり、また同じ変更を元に戻すことになります。差分の見た目より、リポジトリの健全性を優先してください。
• 不要なものは、見つけた作業の中で削除してください。 不要なカラム、引数、関数、エクスポート、または呼び出し元がテストしかないテストがあれば、その場で削除してください。説明を擁護するコメントもあわせて削除してください。チケットに起票してはいけません。メモに残すのは先送りにすぎず、次に読む人が不要だと判断するところからやり直すことになります。
• 見つけたコメントが誤っていたら、それは不具合です。 その場で修正してください。誤ったコメントは、それを修正するはずだった差分が消えた後も残り、積極的に読者を誤解させます。コメントがない状態よりも悪い結果です。
• 変更によって表面化した潜在的なバグは修正してください。 まれにしか実行されない経路を毎回実行するような変更をすると、順序に関するバグ、実行されないクリーンアップ、発火したことのないアサーションが明らかになることがあります。そのバグは今やあなたの責任です。あなたが到達可能にしたのですから。
• 「改善」する前に検証してください。 削除や書き換えの前に、本当に不要、誤り、または壊れていることを確認してください。利用箇所を grep で探し、テストを実行してください。重要なものを自信満々に削除するのは、直そうとしていた乱雑さよりもはるかに悪いことです。
• 制限事項：機能のコミットに無関係な 動作 の変更を紛れ込ませたり、隣接するモジュールの形が気に入らないという理由で書き換えたりしてはいけません。このルールの対象は、機械的な整理、不要なコード、誤ったコメント、およびあなたが到達可能にしたバグです。機会に乗じた再設計は対象外です。
• レビュアーが確認する点：この変更が取り込まれた後、リポジトリ内に、変更前よりも 状態が悪くなった ファイルや、誤りだと分かっている記述を含むファイルが残っていませんか。残っているなら、この作業は完了していません。

あなたのタスク

レビュー対象：$ARGUMENTS

引数が指定されていない場合は、git diff --staged または git diff（ステージされていない変更）をレビューしてください。

見つけた各問題について、ファイル名と行番号を示してください。カテゴリごとにまとめてください。最後に、問題なし／問題ありの判定を記載してください。