Repository navigation
fix(measures): 有効性評価の範囲を EffectivenessScale へ寄せ、写経の取り残しを塞ぐ - #182
Conversation
9685a03 で優先度の段階・ラベル・配色を MeasurePriorityScale へ集約したが、隣の 有効性評価には EffectivenessScale を見ていない経路が 2 つ残っていた。 1. IncidentMeasuresController.RateMeasure が範囲を 1〜5 と直書きしていた。 詳細画面のラジオは EffectivenessScale.All から生成され、並行経路の PreventiveMeasuresController.Review は ReviewViewModel の [Range(EffectivenessScale.Min, EffectivenessScale.Max)] で検証しているため、 同じ項目の 3 か所のうちここだけが尺度を見ていない状態だった。 段階を 5 → 7 に増やすと(EffectivenessScale の doc が想定している変更)、 詳細画面はラジオ 6・7 を描くのにこの経路だけが弾き、評価が黙って捨てられる。 同じ値が /PreventiveMeasures/Review からは保存されるという経路間の食い違いになる。 2. PreventiveMeasure.EffectivenessRating の [Range(1, 5)] と [Display(Name = "有効性評価(1〜5)")] も直書きのままだった。すぐ下の Priority は 既に尺度から引いているので、隣り合う 2 つで方針が割れていた。 あわせて、この取り残しを検出できなかったテスト側の穴も塞いだ。範囲外テストは 下限(0)しか無く、成功経路も評価値を 5 と直書きしていたため、上限を広げても 両方とも緑のまま通っていた。範囲外を下限・上限の Theory にし、成功経路は EffectivenessScale.Max で評価して保存されることまで確認する。 検証: 尺度の Max を一時的に 7 へ変えて mutation テストを実施。 - Max=7 + 修正前コントローラ(1〜5 直書き)→ 1 failed(検出できる) - Max=7 + 修正後コントローラ(尺度参照) → 7 passed - Max=5 へ戻して全体 → 462 passed / 0 failed(ビルド 0 warning) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LXb7LwvtQ9p5CfCpKbnuM7
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
セルフレビューの指摘対応。前のコミットは受け付ける側の境界を上限しか押さえておらず、 下限を 1 つ内側へずらす取り違え(< を <= にする等)がどのテストにも観測されないまま 通っていた。実際に変異させて 462 件すべて緑のまま通ることを確認している。 そのとき捨てられるのは ★1「効果なし」=対策が効かなかったことを示す評価で、再発検知の KPI に直接効く値なので、無言で拒否されるのは実害が大きい。範囲外テストは弾かれる側 (Min-1)しか見ないため、この経路は成功側でしか押さえられない。 成功経路を下限・上限の Theory にし、両端が受け付けられて保存されることを確認する。 あわせてレビューで見つかった、同じ「写経の取り残し」に当たるコメント 2 件を直した。 - PreventiveMeasuresController.Delete の Include の直上に、部署スコープの認可判定に 必要だと述べる古いコメントが残っていた。前コミットで 8 行下に正しい説明を足したため、 上から読むと先に誤った主張に当たる状態になっていた。2 つを 1 つにまとめた。 - Views/Incidents/Details.cshtml のラジオ生成の直上に「1〜5の5段階(デフォルトは中央の3)」 と段階数と既定値を直書きしたコメントが残っていた。次の行が既に 「EffectivenessScale から引く」と正しく述べており、尺度を変えると描画だけが追随して この説明文が取り残される。本 PR が塞いでいる失敗の仕方そのものなので数値を落とした。 検証: 下限変異(< → <=)・上限変異(> → >=)のどちらでも 1 件落ちることを確認。 変異なしで 463 passed / 0 failed、ビルド 0 warning。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LXb7LwvtQ9p5CfCpKbnuM7
CI 状況の報告: Vercel の 2 件は本 PR の失敗ではありません失敗しているチェック
本 PR のリポジトリ CI は全て緑
本 PR の変更が原因ではないと判断した根拠同じ 2 件が、本 PR と無関係な過去の PR でも同一に失敗しており、その PR はそのままマージされています。
本 PR の初回コミット また本リポジトリは ASP.NET Core 8 / EF Core の .NET アプリケーションで、Node のビルド成果物を持ちません。Vercel はこの構成をビルドできないため、この 2 つの Vercel プロジェクト連携はコミット内容によらず常に失敗する状態にあります。本 PR の差分(C# の定数参照化・コメント・xUnit のテスト)はデプロイ構成に一切触れていません。 対応方針修正は行いません。 塞ぐには Vercel 側のプロジェクト連携を解除するか、リポジトリに Vercel 用のビルド設定を追加する必要があり、いずれもリポジトリ/Vercel アカウントの構成変更であってコードの変更ではありません。本 PR の目的(有効性評価の尺度の一元化)と無関係な変更を混ぜないため( 再実行しても構成が変わらない以上同じ結果になるため、再実行も行っていません。
Generated by Claude Code |
PR #184 の 2 巡目レビュー指摘 3 件を反映する。 1. EnumLabels.AuditEntityJa が導出できない 4 つ目の写しだった AllowedEntityNames を宣言から導出するようにしたぶん、そのすぐ隣で使う日本語ラベルの 変換表だけが手書きの写しとして残った。JapaneseAuditEntity は辞書に無いキーを元の値の まま返すフォールバックを持つため、監査対象を足してラベルを書き忘れても例外にならず、 監査ログ画面の 3 箇所(ドロップダウン / 一覧の各行 / 詳細)に CLR の型名が英語のまま出る。 ビルドも全テストも緑のまま通る。 フォールバック自体は残す(監査対象から外したエンティティの過去行を表示するときに、 例外で画面を落とすより元の値を出す方が安全)。代わりに AuditEntityLabelCoverageTests が 「ラベル表は監査対象を全網羅する」ことを機械的に固定する。 2. fluent の HasMaxLength() が裸の数値の抜け道になっていた 前コミットで上限の充足判定を EF のモデル(GetMaxLength())へ寄せたため、fluent で 設定した上限も「上限あり」として通るようになった。ところが裸の数値を検出する FieldLengthsTests は CLR の [MaxLength] 属性しか見ていないので、「上限はある(緑)/ その値は FieldLengths 由来ではない(誰も見ていない)」という状態が作れてしまう。 実際 ApplicationDbContext の 20 / 50 が既にその状態だった。 ——エスケープハッチを足したぶん既存の検出網が黙って狭くなるという、この PR 自身が 塞いだのとまったく同じ形。属性側と同じ許容集合でモデル側も見る検査を足し、 20 / 50 を FieldLengths.EnumCode / EnumCodeJapanese として名前付き定数にした (値は変えていないのでスキーマは不変。マイグレーション不要)。 3. PartitionStringColumns のコメントが実際の挙動を過大に述べていた 「呼び出し側が同じ走査を 2 回しない」と書いていたが、2 つの薄い射影はそれぞれ独立に 呼ぶので両方使えば 2 回走る。実際の挙動と、メモ化せず素直に再計算する判断の理由へ直した。 なお同レビューが挙げた EffectivenessScale の文言と EfCorePackageAlignmentTests の 2 件は 本 PR の差分ではなくマージ済みコミット(#182 / #181)由来のため、ここでは扱わない。 変異 2 通り(ラベル表から 1 件落とす / fluent の上限を裸の 48 へ戻す)で、それぞれ対応する 検査だけが赤になることを実測済み。 dotnet restore --locked-mode / build (警告 0) / test 484 件 / npm run typecheck すべて緑。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AAEwHDpPsNzmEnxHPmzKf5
課題
9685a03(#173)で優先度の段階・ラベル・配色をMeasurePriorityScaleへ集約しましたが、隣の「有効性評価」にはEffectivenessScaleを見ていない経路が 2 つ残っていました。EffectivenessScaleの doc は自ら「同じ語彙・同じ段階数が次の 5 箇所へ写経されていた」と列挙し、段階を 5 → 7 に増やす変更を想定例として挙げています。しかし下記 2 か所はその一覧から漏れていました。1.
IncidentMeasuresController.RateMeasureが範囲を直書きsrc/IncidentInsight.Web/Controllers/IncidentMeasuresController.cs:208同じ項目を扱う 3 か所のうち、ここだけが尺度を見ていません:
Views/Incidents/Details.cshtml:625)EffectivenessScale.All✅PreventiveMeasuresController.Review(ReviewViewModel)[Range(EffectivenessScale.Min, EffectivenessScale.Max)]✅IncidentMeasuresController.RateMeasure1/5を直書き ❌具体的な壊れ方:
Maxを 7 に上げると、詳細画面のモーダルはラジオ 6・7 を描画するのに、送信するとRateMeasureが「有効性評価は1〜5の値を指定してください。」で弾き、評価が黙って捨てられます。一方/PreventiveMeasures/Review/{id}から同じ値を送ると保存される、という経路間の食い違いになります。2. エンティティ側の属性も直書き
src/IncidentInsight.Web/Models/PreventiveMeasure.cs:96同じファイルのすぐ下の
Priorityは #173 でMeasurePriorityScale.Min/Max/DisplayNameへ寄せ済みで、隣り合う 2 つで方針が割れていました。3. これを検出できなかったテスト側の穴
上記がビルドも 461 件のテストも緑のまま通っていた理由:
0)しか回していなかった — 上限を広げても0は弾かれ続けるので緑RateMeasure_NoRecurrence_SetsSuccessは評価値を5と直書き — 上限を 7 にしても5は有効なままなので緑つまり「受け付ける側の写経漏れ」を見る検証が 1 つも無い状態でした。
変更内容
RateMeasureの範囲判定・警告文言をEffectivenessScale.Min/.Maxから引く(文言は補間で組み立て、既存の言い回しは維持)PreventiveMeasure.EffectivenessRatingの[Range]/[Display]を尺度から引く(Priorityと同じ形に揃える)[Theory]にし、期待する範囲も尺度から組み立てるEffectivenessScale.Maxで評価し、その値が実際に保存されることまで確認する[Range]/[Display]は DataAnnotations のみでスキーマに影響しないため、マイグレーションは不要です。検証(mutation テスト)
「実行日に依存しない」だけでなく「バグを実際に検出できる」ことを機械的に裏付けました。尺度の
Maxを一時的に 7 へ変えて:Max=7+ 修正前コントローラ(1〜5直書き)Max=7+ 修正後コントローラ(尺度参照)Max=5へ戻して全体dotnet buildは 0 Warning / 0 Error。(修正前のベースラインは 461 passed で、追加した Theory ケース 1 件ぶん増えています。)対象外とした関連事項
レビュー中に、
IncidentsController.Delete/PreventiveMeasuresController.Deleteのコメントが「部署スコープも考慮」「部署一致/管理者系」と書いているのに対し、Policies.CanDeleteIncidentはRequireRole(Admin, RiskManager)のみでSameDepartmentRequirementを持たないことを確認しました(Program.cs:242)。現状は Admin/RiskManager しか到達できないため実害はありませんが、コメントを信じて将来 Staff へ広げると他部署インシデントの削除が素通りします。ポリシー自体の変更は認可の振る舞いを変えるため見送り、コメントを実態(ロール判定のみ/リソースを渡すのは将来ポリシーへ部署要件を足したとき自動追随させるため)へ訂正するに留めました。
Generated by Claude Code