Skip to content

Infra Refactor PR Report / Backend Refactor PR Report / Frontend Refactor PR Report#283

Merged
yusuke0610 merged 3 commits into
devfrom
refactor/backend/deadcode
May 29, 2026
Merged

Infra Refactor PR Report / Backend Refactor PR Report / Frontend Refactor PR Report#283
yusuke0610 merged 3 commits into
devfrom
refactor/backend/deadcode

Conversation

@yusuke0610

Copy link
Copy Markdown
Owner

Summary

  • versions.tf(3 環境完全一致)を shared/versions.tf に統合し symlink 化(PR1)。
  • environment を tfvars 変数化し、provider + module "devforge_stack" 呼び出しを含む main.tfshared/main.tf に統合して 3 環境を symlink 化(PR2)。これで env 別 main.tf の唯一の差分(environment リテラル)が解消。
  • HCL コード重複率 3.04% → 0%(残る infra clone は ENV_CHECKLIST.md の md のみ)。
  • PR3(ENV_CHECKLIST.md)は調査の結果、stg/prod が env 固有値で大半が異なるため symlink 化せず allowed duplication として記録(下記 Skipped)。
  • 全変更は state 影響なし(resource アドレス module.devforge_stack.* 不変、渡る値も同一文字列)。

Applied Changes

Medium

  • [infra/environments/{dev,stg,prod}/main.tf] provider 3 ブロック + devforge_stack 呼び出しのコピペを解消。environment = "<env>" リテラルを var.environment 化し、実体を shared/main.tf に集約 → 3 環境を ../shared/main.tf への symlink に置換。元レポート "Medium / Variable 抽出候補 / PR2"。

Low

  • [infra/environments/{dev,stg,prod}/versions.tf] 3 環境完全一致を shared/versions.tf に統合し symlink 化。元レポート "Low / PR1"。

Modules / Variable Changes

Modules 化

  • なし(既存構造で完了済み。新規 module 作成・resource 置換なし)。

Variable 抽出

  • [infra/environments/shared/variables.tf] variable "environment" を追加。validation で dev/stg/prod を制約。description は日本語。
  • terraform.tfvarsenvironment = "dev"|"stg"|"prod" を 1 行追記(tfvars は Git 管理対象で、placeholder 値を含むテンプレート運用)。

Module 統合

  • なし。

Structure Changes

  • env 別実体ファイルは backend.tf(GCS bucket 差分・変数化不可)と terraform.tfvars のみに縮小。main.tf / versions.tf / variables.tf / moved.tf / outputs.tf はすべて shared/ の symlink に統一。
infra/environments/
  shared/
    main.tf        # 新規(provider + devforge_stack 呼び出し、var.environment)
    versions.tf    # 新規
    variables.tf   # 既存(environment 変数を追加)
    moved.tf       # 既存
    outputs.tf     # 既存
  dev/  stg/  prod/
    backend.tf         # 実体(env 別 GCS bucket)
    terraform.tfvars   # 実体(env 別値、environment 追記)
    main.tf      -> ../shared/main.tf      (新規 symlink)
    versions.tf  -> ../shared/versions.tf  (新規 symlink)
    variables.tf -> ../shared/variables.tf (既存 symlink)
    moved.tf     -> ../shared/moved.tf     (既存 symlink)
    outputs.tf   -> ../shared/outputs.tf   (既存 symlink)

State Migration

  • state 移行は不要
  • 理由: module 呼び出しのアドレスは module.devforge_stack.* のまま変わらず、environment に渡る文字列も従来のリテラルと完全同一("dev"/"stg"/"prod")。リソースの再作成・rename は発生しない。
  • tofu plan は state 認証が必要なため本 PR では未実行(下記 Validation / Follow-ups)。apply 担当者は念のため各環境で tofu plan を回し、差分ゼロ(No changes)を確認してから apply すること。

Skipped

  • PR3: ENV_CHECKLIST.md の symlink 統合を見送り
    • 理由: diff stg/ENV_CHECKLIST.md prod/ENV_CHECKLIST.md の結果、GCS バケット名(devforge-stg-dbdevforge-prod-db)、CORS、OAuth 登録手順、GCP プロジェクト名、GitHub Secret 名(GCP_SA_KEY_STG_PROD)など大半が env 固有値。jscpd が検出した 9 行 clone は表ヘッダ等の共通テンプレ行のみ。
    • 結論: terraform.tfvars と同種の「構造は同じ・値が env 別」= allowed duplication。markdown には変数展開が無く、symlink 化すると env 固有値が失われるため統合は不適切。現状維持。
    • 参考: dev には ENV_CHECKLIST.md が存在しない(追加するなら別タスク)。

Validation

  • make infra-fmt-check: pass(fmt 差分なし)
  • make infra-validate: pass(dev / stg / prod すべて "Success! The configuration is valid.")
  • tofu plan (各環境): 未実行(GCS backend / state 認証が必要なため。state 影響なしの根拠は State Migration 参照)
  • make dupe-check: HCL 3.04% → 0.00%(46/1514 行 → 0/1411 行)。残る infra clone は ENV_CHECKLIST.md(stg↔prod, 9 行 ×2、allowed)のみ。

Follow-ups

  • infra 専用ブランチの作成: 現在 refactor/backend/deadcode(無関係な backend 作業ブランチ)上で変更している。コミット前に origin/dev 起点で infra 専用ブランチを切り、本変更だけを載せること(プロジェクト規約)。
  • apply 前の plan 確認: 認証環境で tofu -chdir=infra/environments/{dev,stg,prod} plan を実行し、No changes を確認(state 影響なしの最終確認)。
  • ENV_CHECKLIST.md: dev 版が欠落。必要なら dev 用を追加するか、共通の前提手順だけ docs/ に正本化する別 PR を検討(本 PR スコープ外)。

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 53d65072-58a8-4edc-b57e-d2569a87b4f7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/backend/deadcode

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yusuke0610

Copy link
Copy Markdown
Owner Author

Summary

  • worker.py の終端処理(completed / dead_letter / retrying の 3 分岐)で重複していた「新規セッション開閉 + 通知ガード」を _run_in_new_session / _finalize_* ヘルパーへ抽出し、挙動を変えずに本質的重複を解消。
  • blog repository の upsert_many / sync_many で重複していた merge ループ(apply/add/commit)を _merge(..., delete_missing=) に統合。key を (account_id, external_id) に一本化。
  • github_link.py の 2 エンドポイントで完全一致していた dispatch 失敗時の 500 送出を _raise_dispatch_failed() に集約。
  • テストの完全一致 Resume payload を conftest.make_resume_payload() ファクトリへ集約(test_delete_documents.py)。
  • make lint-backend / make test-backend ともに pass(443 passed)。

Applied Changes

High

  • [app/services/tasks/worker.py] Findings High / Duplication High。execute_task 内の 3 つの db = SessionLocal() → try/finally _safe_close ブロックと isinstance(user_id, str) and user_id != "unknown" 通知ガードを撤去し、以下のヘルパーへ集約:
    • _run_in_new_session(work): 新規セッションの開閉のみを担う(libSQL Hrana 失効対策のコメントを 1 箇所に集約)
    • _notify_if_real_user(db, task_type, user_id, status): 実ユーザー判定 + 通知
    • _finalize_completed / _finalize_dead_letter / _finalize_retrying: 各終端遷移
    • 挙動は不変(completed は実ユーザー時のみセッションを開く、dead_letter はユーザー有無に関わらず必ずマーク、retrying は通知しない)。SessionLocal / _mark_* / _create_notification は module-level のまま残したのでテストの patch ポイントも維持。

Medium

  • [app/repositories/blog.py:89-] upsert_many(複数アカウント・削除なし)と sync_many(単一アカウント・全置換)の merge ループを _merge(normalized_articles, existing_articles, *, delete_missing) に統合。
    • key を (account_id, external_id) に統一。sync_many は新規エンティティ生成・突合のため各記事へ account_id{**article, "account_id": account_id} で確実に付与(呼び出し側 sync_service.py が既に注入しているが、リポジトリ単体でも成立するよう防御的に付与)。
    • upsert_many の空入力早期 return(空 IN 句クエリ回避)と、sync_many の空入力→全削除セマンティクスを両方保持。

Low

  • [app/routers/github_link.py] start_github_link / retry_github_linkexcept Exception: raise_app_error(500, INTERNAL_ERROR, task.dispatch_failed, ...)_raise_dispatch_failed() に抽出。

Test Changes

Removed

  • [tests/test_delete_documents.py] モジュールローカルの完全コピー _RESUME_PAYLOAD(35 行)を削除し、make_resume_payload() の呼び出しに置換。jscpd で検出されていた test_delete_documents.py:8-27 ↔ test_endpoints.py:119-138 の構造的クローンのうち、完全一致側を解消。

Added

  • [tests/conftest.py] make_resume_payload(**overrides) ファクトリを追加。取引先・プロジェクト・体制まで含む完全な resume payload を返す共通フィクスチャ。呼ぶたびに新規 dict を返すため共有副作用なし。今後の統合テストの正準 payload として再利用可能。
  • 機能テストの追加は無し(レポート Add は「明確な不足なし」と判定。retry 409 / dispatch 失敗→dead_letter は既存テストでカバー済みを確認)。

Duplication Resolved

  • worker.py 終端セッションブロック(Duplication High)→ _run_in_new_session + _finalize_* に集約。
  • blog repository upsert/sync の merge ループ(Duplication Medium)→ _merge に統合。元レポートは「2 箇所どまりで保留」としていたが、(a) key 戦略・(b) 新規 account_id・(c) delete_missing の 3 差分が delete_missing フラグ 1 個+account_id 注入でコールバック無しに吸収でき、コア loop が完全一致だったため、過剰抽象化にならない範囲で統合した。
  • テスト Resume payload 完全一致(Duplication High)→ conftest ファクトリへ。

Structure Changes

  • ディレクトリ移動・ファイル分割なし(元レポート Structure: 提案なし)。

Skipped

元レポート自身が「現状維持/保留推奨」とした項目。CLAUDE.md「過剰な抽象化を避ける」「Rule of Three」に従い、機械的 DRY 化を見送り記録のみ:

  • collector.py の fetch_zenn/note/qiita ページング共通化(Medium): 元レポートが「現状維持を推奨」。終端条件(next_page / isLastPage / len<per_page)・sleep・User-Agent・記事マッピングがプラットフォームごとに独立した変更理由を持つ偶発的重複。共通化すると逆に複雑化するため未着手。
  • resume / intelligence の PDF↔Markdown ジェネレータの意味的重複(Medium): 走査構造は同型だが出力フォーマットが本質的に異なり、正規化は既に shared/resume_format.py に集約済み。2 箇所どまりで visitor 抽出の価値が薄く保留。
  • worker.py _safe_rollback の削除検討(Low): tests/test_worker/test_execute_task.py の 2 ケースが直接テストしており、休眠 API 温存方針にも合致するため削除しない。
  • github_link.py の github: ログイン必須ガードの共通化: 2 エンドポイントで重複するが元レポートのスコープ外。3 箇所目が出たら _require_github_username(user) 抽出を検討(Follow-ups)。
  • schemas/resume.py の validate_dates 系(Allowed): エラーメッセージは get_error 経由で SSoT 維持済み。共通化するとフィールド名分岐が増えるため現状維持。
  • test_endpoints.py の intent 固有 payload / markdown・schema・csrf の変種 payload: AAA の可読性のため残す(duplication ポリシーで許容)。

Validation

  • make lint-backend: pass(All checks passed!)
  • make test-backend: pass(443 passed, 2 warnings, 29.66s)
  • worker.py 抽出後の挙動回帰は既存 test_worker/test_execute_task.py(completed 通知 / dead_letter 遷移)と test_retry_flow.py(retrying)でカバー済みであることを確認。

Follow-ups

  • collector のページング・ジェネレータの意味的重複・blog 以外の upsert は「3 箇所目の出現時に再評価」。
  • github_link.py のログイン必須ガード共通化(_require_github_username)は次に同種エンドポイントが増えた時点で。
  • make dupe-check の再実行で worker.py / blog.py のクローンが消えたかを次セッションで確認するとよい(本 PR では lint+test のみ実行)。

@yusuke0610

Copy link
Copy Markdown
Owner Author

Summary

  • ユーザー向け「成功」文言のハードコードを constants/messages.ts に集約し、useBlogAccountManager / GitHubLinkDashboard のリテラルを定数・関数参照へ置換。
  • これらが CI を擦り抜けていた根本原因(lint パターンの穴)を scripts/lint-frontend-messages.shsetSuccess/setInfo/setMessagetoAppError(…, "…") の検知を追加して塞いだ(逆検証で検知動作を確認)。
  • blog テストの literal assert を同じ定数 import に統一し、文言 SSoT を 1 本化。
  • 463 行の CareerExperienceEditor.tsx から取引先ブロックを ClientEditor.tsx に抽出(出力同値・DOM 不変)。

Applied Changes

High

  • なし(レポートに High 指摘なし)

Medium

  • [frontend/src/hooks/blog/useBlogAccountManager.ts] 成功文言のハードコード解消
    • constants/messages.tsSUCCESS_MESSAGESBLOG_LINKED / BLOG_UNLINKED / BLOG_USERNAME_UPDATED)と動的関数 blogLinkedSyncSuccessMessage / blogSyncSuccessMessage / blogUsernameUpdatedSyncSuccessMessage を追加し、4 箇所のリテラル(連携・同期・解除・username 更新)を置換。
  • [frontend/src/components/github-link/GitHubLinkDashboard.tsx:60] toAppError fallback リテラル解消
    • FALLBACK_MESSAGES.GITHUB_LINK("連携に失敗しました")を追加し、setError(toAppError(e, FALLBACK_MESSAGES.GITHUB_LINK)) へ置換。

Low

  • [scripts/lint-frontend-messages.sh] 検知パターン拡張(再発防止)
    • setter パターンを set\w*(Error|Success|Info|Message)\w* に拡張、toAppError\([^,)]+,\s*"…" の補助パターンを -e で追加。エラーメッセージ文言とヘッダコメントも更新。
    • 逆検証: setSuccess("保存しました")setError(toAppError(e, "連携に失敗しました")) を含む使い捨てファイルで exit 1(検知)を確認、ファイルは削除済み。
  • [frontend/src/components/forms/CareerFormEditors/ClientEditor.tsx 新規] CareerExperienceEditor 分割
    • per-client ブロック(取引先ヘッダ + 休暇分岐 + プロジェクト一覧)を ClientEditor へ抽出。CareerExperienceEditor.tsx は 463 → 283 行、ClientEditor.tsx は 236 行。state は持たず props のハンドラを呼ぶ純粋な描画分割で DOM 出力は不変。
    • ※レポートでは「過剰抽象化リスクのため現状維持推奨」としていたが、ユーザーが「全部」を選択したため実施。取引先という明確なドメイン境界での分割に留め、これ以上の細分化(VacationEditor 等)はしていない。

Test Changes

Removed

  • なし

Added / Updated

  • [frontend/src/hooks/blog/useBlogAccountManager.test.ts] 成功文言を assert する 4 箇所(解除 / 連携 / 連携後同期件数 / username 更新)を SUCCESS_MESSAGES.* および blogLinkedSyncSuccessMessage(3, 5) の定数・関数参照へ置換。守る挙動は不変(成功メッセージの内容と表示タイミング)だが、文言の二重管理を解消。

Duplication Resolved

  • レポートの Duplication Findings は High/Medium ともゼロ(jscpd frontend clone は全て許容重複)。本 PR で新規の重複統合は行っていない。
  • 副次的に、成功文言の「ハードコード文字列 × 実装/テストの二重管理」を SSoT へ統合した。

Structure Changes

frontend/src/components/forms/CareerFormEditors/
  CareerExperienceEditor.tsx   # 463 → 283 行(経歴レベルの shell に集中)
  ClientEditor.tsx             # 新規 236 行(取引先1件の描画)

Skipped

  • なし(採用スコープ「全部」を完了)。

Validation

  • make lint-frontend: pass(eslint src/ エラーなし)
  • make lint-frontend-messages: pass(exit 0)
  • make test-frontend: pass(node:test 4 / vitest 189、全 23 test files green)
  • make build-frontend: pass(gen-redirects + tsc -b + vite build、224 modules)
  • E2E (npm run test:e2e): pass(21 passed)。career-dirty-indicator / github-link を含む。ログ中の ECONNREFUSED はモック未対象ルートへの Vite dev proxy 由来で、テスト結果に影響なし。

Follow-ups

  • なし。仕様判断が必要な保留項目もなし。
  • 将来 success/info トーストを増やす際は SUCCESS_MESSAGES(静的)または動的関数(件数埋め込み)に追加すること。拡張済み lint が setSuccess リテラルを CI で検知する。

@yusuke0610
yusuke0610 merged commit db7a74d into dev May 29, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant