Fix: Pro機能Epicリリース前レビュー指摘の解消(Critical+Major)

[ippei-shimizu]

概要

issue #445（Pro機能Epicリリース前レビュー指摘の解消）のうち、back側のCritical/Major項目に対応する。

対応内容

• goal更新APIでmetric_key/comparison_type/practice_menu_idを変更不可にする修正（指標差し替えで既存target_valueが無意味になる不整合を防止）
• シーズン粒度推移(granularity=season)でseason_id絞り込みと併用すると1シーズンに縮退する不具合を修正（batting_trend / era_trend共通処理として抽出）
• /pro/syncをRevenueCat REST APIとの実同期に置き換え（RevenueCat::SubscriberClient / RevenueCat::SubscriberSyncを新規追加。グレース期間・タイムアウト等のエッジケースはセルフレビューで追加修正済み）
  • 補足: 現時点でこのエンドポイントを叩くのは購入完了時・購入復元時の自動呼び出しのみ。ユーザーが能動的に押す「同期更新」ボタンはmobile側に無く、検討の結果追加しないことにした（RevenueCatのWebhook自体が再送されるため、頻度に対して手動ボタンの保守コストが見合わないと判断）
• 素振りカウンターのバックグラウンド継続実行entitlement追加（shadow_swing_background。実装はmobile側）
• カレンダー俯瞰(GET /api/v2/plans/calendar)にPro/無料の閲覧範囲出し分けを追加（schedule_calendar_full_history entitlement、無料は今日の前後3ヶ月にクランプ）
• 防御率推移(EraTrendChart)にシーズン粒度・自己ベスト強調を追加（era_trend_serviceを月/シーズン対応に書き換え、レスポンス形状を{trend:[{month,era}]}から{granularity,points:[{key,label,era}]}へ変更。batting_trendと統一）
• 達成バッジ(goal_badges)を閲覧するAPIを追加（GET /api/v2/goal_badges）

セルフレビュー（自動実施）で追加修正した項目

• グレース期間中にexpires_atへ生の期限を保存しpro_active?を壊していた不具合（SubscriberSync）
• タイムアウト・接続エラーが502ハンドリングを迂回する不具合（SubscriberClient）

外部レビュー対応で追加した項目

• SubscriberSyncがPlanCatalog未登録のproduct_id/storeを無視してplan_type/platformにnilを静かに保存していた不具合を修正（Webhook側のunknown_product?と同じガード・Sentry警告を追加）
• NETWORK_ERRORSにNet::WriteTimeoutを追加（現状write_timeout未設定のため実害なし、将来設定時の考慮漏れ防止）
• secret_keyのENV.fetchにデフォルト値を設けていない意図（設定ミスは502で揉み消さずKeyErrorで気付けるようにする）をコメントで明記

スコープ外（別issueへ切り出し）

以下2件はバックエンドの新規ジョブ・push基盤など大規模な追加実装が必要なため、別issueで対応する:

• Feature: 草機能の12週ロック表示とF-05/06/11(月別ナビ・日付詳細・年ビュー) buzzbase#446 草機能の12週ロック表示とF-05/06/11/13
• Fix: Streak途切れ通知をサーバー側の日次動的判定+push方式に是正 buzzbase#447 Streak途切れ通知のサーバー側是正

またissue #445のMinor項目（32件）は今回のスコープから除外し、別PRで自動対応予定。

スコープから外した項目

development環境のadminユーザーに強制Proモードを実装する対応は、不要と判断し取り下げ（git revert）。

手動テストで見つかった不具合の修正

カレンダー俯瞰の閲覧範囲を無料は前後15日→前後3ヶ月に変更（狭すぎるとの判断）。

「練習と成績のつながり」（相関インサイト）で、練習を一度もしていない（試合記録のみの）ユーザーにも
実在しない「傾向」が断定表示される不具合を修正。以下2つの独立した問題が原因だった:

1. 素振り本数のような入力値が全週0（変動が無い）場合でも上位群/下位群に分割し、意味の無い差分を
   傾向として表示していた。入力値に変動が無いペアは非断定カードにする
2. 「練習した日数」の判定に草・Streak向けのintensity_levelを使っていたため、試合しか記録して
   いない日も「練習した日」としてカウントされていた。practice_menu_countまたは
   total_swing_countが実際に正の値を持つ日だけを練習日として数えるようにする

テスト

• docker compose exec back bundle exec rspec — 1623 examples, 0 failures
• docker compose exec back bundle exec rubocop — 593 files, no offenses

関連

• Fix: Pro機能Epic(back #216/mobile #80)リリース前レビュー指摘の解消 buzzbase#445

[ippei-shimizu]

@claude
コードレビューをお願いします。

[claude]

Claude finished @ippei-shimizu's task in 4m 55s —— View job

---

コードレビュー

TODOリスト

• PRの差分を取得・分析（git diff origin/release/pro-202605...HEAD、24ファイル / +635 -40）
• 各変更ファイルをレビュー（goal更新API / stats trend / RevenueCat同期 / calendar entitlement / goal_badges / correlation_builder）
• テスト内容の確認
• レビュー結果をコメントにまとめる

---

全体評価

Critical/Major項目（goal指標変更不可・シーズン粒度のseason_id縮退バグ・RevenueCat実同期・カレンダーPro出し分け・ERA推移拡張・goal_badges閲覧API）はいずれも実装・テストとも妥当です。セルフレビューでグレース期間中のexpires_at破壊とタイムアウト時の502迂回を追加で潰しているのも良い動きで、Critical/Major級のブロッカーは見つかりませんでした。以下はMinorな指摘のみです。

Minor指摘

1. RevenueCat::SubscriberSyncが未知のproduct_id/storeをガードしていない（app/services/revenue_cat/subscriber_sync.rb:30-59）
   Webhook側のRevenueCat::Handlers::BaseHandler#unknown_product?はPlanCatalogに未登録のproduct_id/storeが来たら更新をスキップしてSentryへ警告を送りますが、SubscriberSync#attributes_forはPlanCatalog.plan_type_from / platform_fromがnilを返しても素通りでsubscription.update!してしまいます（plan_type / platformカラムはnull許容なのでエラーにはならず、静かにnilが保存されます）。RevenueCat側に新SKUを追加してからPlanCatalogの追従が漏れた場合、Webhook経由では検知できてもsync経由では気づけない非対称が生まれます。
   Fix this →

2. ENV.fetch('REVENUECAT_SECRET_API_KEY')のデフォルト無し（app/services/revenue_cat/subscriber_client.rb:47-49）
   同じRevenueCat連携でもWebhook側はENV.fetch('REVENUECAT_WEBHOOK_SECRET', nil)とデフォルト値ありなのに対し、こちらはデフォルト無しでKeyErrorを送出します。環境変数未設定時はRequestFailedError（→502）ではなく素の500になるため、意図的な「設定ミスは502で揉み消さず気付けるようにする」判断であれば問題ありませんが、確認のうえ意図通りか一言コメントしておくと親切です（重要度は低いです）。

3. NETWORK_ERRORSにNet::WriteTimeoutが入っていない（app/services/revenue_cat/subscriber_client.rb:16-19）
   現状write_timeoutオプションを渡していないので実害はありませんが、将来write_timeoutを追加する際に見落とされやすいので気になりました（Nit）。

良い点

• Api::V2::GoalsController#update_paramsでmetric系フィールドを弾く対応は既存のperiod_type変更不可パターンに揃っていて自然です（app/controllers/api/v2/goals_controller.rb:80-85）。
• season_aware_paramsをcontrollerの共通privateメソッドとして抽出し、batting_trend / era_trend両方から使う設計は.claude/rules/controller-design.mdのCRUD/共通化方針に沿っています。
• Insights::CorrelationBuilderの2つの独立したバグ修正（入力値の分散チェック、practice_daysの判定をintensity_levelからpractice_menu_count/total_swing_countベースに変更）はそれぞれ専用のテストで再現→修正確認されており説得力があります。
• SubscriberSyncのグレース期間中expires_at保存ロジックはコメントで意図（pro_active?/in_grace_period?が矛盾しないように）が明記されていて読みやすいです。
• GoalBadgesControllerは読み取り専用でcurrent_api_v1_user.goal_badgesにスコープされており、他ユーザーのバッジ漏洩の心配はありません。

テストについて

• spec/services/revenue_cat/subscriber_sync_spec.rbはグレース期間・billing_issue・cancelled・trialの分岐を一通りカバーしており十分です。
• spec/requests/api/v2/stats_batting_trend_season_spec.rb / stats_spec.rbのseason_id併用テストはリグレッション防止として的確です。
• spec/requests/api/v2/plans_spec.rbのカレンダー範囲クランプは未来方向のみテストされていますが、実装は過去方向も対称なロジックなので問題ないと判断しました。
• CI環境の制約でこちらではdocker compose exec back bundle exec rspec / rubocopを実行できていません（Dockerが利用できないサンドボックスのため）。PR記載の「1621 examples, 0 failures」「593 files, no offenses」は静的レビューの範囲では矛盾は見当たりませんでした。