Skip to content

refactor: コードベース負債の修正とルール整備#325

Merged
yusuke0610 merged 10 commits into
mainfrom
claude/codebase-refactoring-plan-rakv8f
Jun 12, 2026
Merged

refactor: コードベース負債の修正とルール整備#325
yusuke0610 merged 10 commits into
mainfrom
claude/codebase-refactoring-plan-rakv8f

Conversation

@yusuke0610

@yusuke0610 yusuke0610 commented Jun 12, 2026

Copy link
Copy Markdown
Owner

変更概要

リファクタリング計画書(.report/refactoring-plan-20260612.md)に基づき、BE_refacter / FE_refacter で検知した技術的負債を修正し、同じ負債の再発を防ぐためのルール・設定を整備した。


Backend 修正(3件)

パターン C — ORM model への表示ロジック混入

  • backend/app/models/resume.py から sort_utils の import を削除
  • Resume.experiences / Resume.qualifications / ResumeClient.projects@property がソート済みリストを返していたが、repository の sort_order と relationship の order_by で既に順序保証されていたため二重ソートを解消

パターン A — router から collector を直接 import

  • routers/blog/accounts.pynormalize_username / verify_user_exists を直接呼び、外部 API 例外を router 内で処理していた
  • BlogAccountService.add_account() を新設し router を service への委譲のみに変更
  • 影響テストのモック先を app.routers.blog.accounts.*app.services.blog.account_service.* に修正

パターン B — router 内 DB クエリ直書き

  • routers/github_link.py_get_or_create_cache()github_link_service.pyget_or_create_github_link_cache() として移設
  • router からは service 経由で呼ぶように変更

Backend テスト: 505 件 pass


Frontend 修正(1件)

モーダル useState が 3 個以上(component-design.md 違反)

  • CareerResumeForm.tsx(534 行)の showDeleteConfirm / showSaveConfirm / editingField + handleDelete / handleConfirmSaveuseCareerFormModals フックに集約
  • テスト 5 件追加(useCareerFormModals.test.ts

Frontend テスト: 329 件 pass


ルール・設定整備

再発防止のため以下を追加・修正:

  • .claude/rules/backend/layers.md — 層ごとの禁止事項と Bad/Good 例(パターン A/B/C)
  • .claude/rules/frontend/component-design.md — コンポーネント設計ルール
  • .claude/CLAUDE.md — 「失敗から学んだ知見」に 3 項目追記
  • CONTRIBUTING.md — ブランチ戦略修正・ADR 一覧更新(ADR-0007〜0010 追加)
  • .github/PULL_REQUEST_TEMPLATE.md — PR チェックリスト
  • .claude/settings.json — Stop hook(セッション終了時に make dupe-check をバックグラウンド実行)

セルフレビューチェックリスト

  • make ci 相当が pass している(backend: 505 tests、frontend: 329 tests)
  • コメント・ドキュメントは日本語で記述
  • 新規環境変数なし
  • 破壊的変更なし(API 契約・DB スキーマに変更なし)

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT

claude added 5 commits June 12, 2026 09:28
backend / frontend / infra / CI / docs の全領域を静的調査し、
技術的負債をフェーズ分けで解消するための計画を .report/ に作成。

- Phase 0: 計測基盤・ベースライン確定(jscpd / カバレッジ)
- Phase 1: クイックウィン(FIELD_ENCRYPTION_KEY TODO・テスト統合・ADR 整理)
- Phase 2: Backend 責務分離(PDF generator / blog collector / router ロジック漏れ)
- Phase 3: Frontend 責務分離(CareerResumeForm 分割・props drilling 解消)
- Phase 4: テスト補強(巨大テスト分割・FE API 層 / Redux テスト追加)
- Phase 5: CI/インフラ最適化(デプロイジョブ統合・WeasyPrint 共通化)
- Phase 6: 仕上げ・再発防止(jscpd 閾値引き上げ・ADR 起票)

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT
router/service/repository/model 各層の禁止事項を .claude/rules/backend/layers.md に
新規作成(Bad/Good 例付き)。今回の調査で発見した3パターンの負債を記録:
- router への外部 API 例外処理の漏れ
- router 内への DB クエリ直書き
- ORM model への表示ロジック混入(sort_utils import)

CLAUDE.md「失敗から学んだ知見」に同3パターンを再発防止ルールとして追記し、
layers.md / component-design.md(次 PR)への参照を追加。

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT
コンポーネント肥大化・props drilling の再発防止ルールを
.claude/rules/frontend/component-design.md に新規作成。

- 行数目安(300行超:分割検討、500行超:必ず分割、フック150行超)
- props drilling の定義と Context 導入の判断基準(3層以上素通し)
- モーダル管理パターン(Bad/Good 例付き)
- useProjectModalState 等の既存良パターンへの参照

CLAUDE.md 1コミット前の追記から参照される。

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT
ブランチ戦略:
- develop ブランチを廃止済みとして表から削除
- PR 作成先を develop → main に修正
- 廃止経緯の Note を追記

ADR 一覧:
- ADR-0007(codegen-drift)/ 0009x2(toast/textlint)/ 0010(Agent)を追加
- ADR-0008 のステータスを Accepted → Superseded by ADR-0010 に修正

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT
.github/PULL_REQUEST_TEMPLATE.md:
  PR ごとの確認漏れを防ぐ最小限チェックリストを新規作成。
  必須確認(make ci・日本語コメント)と条件付き確認
  (codegen-types・E2E・env_keys 4箇所同期・messages リテラル禁止)を分離。

.claude/settings.json:
  common/duplication.md で参照されていた Stop hook を実装。
  セッション終了時に make dupe-check をバックグラウンド実行し、
  次セッション開始時に jscpd-report.json を参照できる状態にする。

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@yusuke0610, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 35 minutes and 15 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fbe5854d-94a8-4d7a-aea7-6013a2850b66

📥 Commits

Reviewing files that changed from the base of the PR and between dfd1b14 and 9a53ea9.

📒 Files selected for processing (7)
  • .claude/settings.json
  • .report/refactoring-plan-20260612.md
  • CONTRIBUTING.md
  • docs/adr/0011-frontend-textlint-proofread.md
  • frontend/src/components/forms/CareerResumeForm.tsx
  • frontend/src/hooks/career/useCareerFormModals.test.ts
  • frontend/src/hooks/career/useCareerFormModals.ts
📝 Walkthrough

Walkthrough

This PR adds architecture and component design guidelines, a six-phase refactoring roadmap, branch/PR workflow updates, and moves several backend behaviors into service helpers with matching test adjustments and small model ordering changes.

Changes

Architecture & Process Governance

Layer / File(s) Summary
Backend layer responsibility separation
.claude/CLAUDE.md, .claude/rules/backend/layers.md
Backend layers are explicitly separated by responsibility: routers handle HTTP and exception translation; services coordinate business logic; repositories abstract storage; models hold contracts. Three antipattern examples (external API exception leaks, router-level DB queries, model display logic) show the correct layer boundary. Exception handling rules specify that external API exceptions are caught and converted at the router boundary only, never passed through services.
Frontend component design patterns
.claude/rules/frontend/component-design.md
Component responsibility is measured by line count: .tsx files beyond ~200 lines and custom hooks beyond ~150 lines signal extraction. Props drilling beyond 3 layers requires Context introduction (differentiated by whether props are used or merely forwarded). Modal state management is extracted to hooks when a parent reaches 3+ modal switches. Examples point to existing reusable patterns in the codebase.
Development workflow and branch strategy
.claude/settings.json, .github/PULL_REQUEST_TEMPLATE.md, CONTRIBUTING.md
Develop branch is eliminated; all feature/fix/docs/refactor/infra branches merge directly to main. GitHub PR template enforces change summary, self-review checklist, breaking-change flags, and ADR references in Japanese. Claude development hook runs make dupe-check silently on save. ADR list expands with ADR-0007 through ADR-0010 entries, including ADR-0008 superseded by ADR-0010.
Six-phase refactoring roadmap
.report/refactoring-plan-20260612.md
A comprehensive refactoring plan spans 6 phases. Phase 0 establishes metrics baseline (duplication, coverage, line counts); Phase 1 delivers quick wins; Phase 2 recovers backend layer separation (router/service/repository/model boundaries); Phase 3 extracts oversized frontend components and eliminates props drilling; Phase 4 strengthens regression testing; Phase 5 unifies CI job execution and caches; Phase 6 raises automated checks and measures outcomes. Common operational rules (PR grain, make ci runs, dupe-check gates, skill-flow integration) apply across all phases. Specified directories are explicitly out of scope.
Backend runtime refactors and tests
backend/app/services/blog/account_service.py, backend/app/routers/blog/accounts.py, backend/app/services/intelligence/github_link_service.py, backend/app/routers/github_link.py, backend/app/models/resume.py, backend/tests/**
Adds BlogAccountService.add_account(...) and BlogAccountAlreadyRegisteredError, shifts add_account router logic to call the service and maps domain errors to HTTP responses; adds get_or_create_github_link_cache helper and replaces router-local cache helper; resume model stops using runtime sort helpers and relies on relationship order; tests update mock patch targets to the service-level verify function.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit reads the roadmap, hops in stride,
Rules for layers keep responsibilities tied,
Services take the logic, routers translate the cries,
Frontend trims its props and modals harmonize,
To main we send our branches—clean and wide. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.59% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor: コードベース負債の修正とルール整備' accurately summarizes the main changes: fixing technical debt in the codebase and establishing new architectural rules for backend/frontend layers.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/codebase-refactoring-plan-rakv8f

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/settings.json:
- Line 9: Update the "command" entry that currently contains "make dupe-check
2>/dev/null &" so the duplication check runs synchronously and errors are
visible: remove the background ampersand and the stderr redirection so the value
becomes a plain "make dupe-check" (update the "command" key in
.claude/settings.json referencing the existing "make dupe-check" invocation).

In @.github/PULL_REQUEST_TEMPLATE.md:
- Around line 18-20: Update the PR title example to include the required
"<type>" placeholder so it matches the later convention; specifically edit the
PR title format line near the "### 破壊的変更" section to use the pattern "type: <内容>
— short description" (or include "<type>" where it was omitted) so contributors
are guided to produce well-formed titles consistent with the template’s listed
convention.

In @.report/refactoring-plan-20260612.md:
- Around line 23-27: The fenced diagram block is unlabeled and triggers
markdownlint MD040; update the code fence that contains the phase diagram (the
triple-backtick block showing "Phase 0 ─→ Phase 1 ─→ Phase 2...") by adding a
language label such as text after the opening backticks (i.e., change ``` to
```text) so the fence is explicitly labeled and markdownlint-cli2 no longer
flags it.

In `@CONTRIBUTING.md`:
- Around line 78-82: Rename the duplicate ADR entry labelled "ADR-0009" to the
next unused ADR number (e.g., change the second "ADR-0009" row to "ADR-0011"),
and update its Markdown link and document filename reference accordingly (the
table cell containing "ADR-0009" and
"docs/adr/0009-frontend-textlint-proofread.md" should be updated to the new
identifier and matching docs/adr/00XX-frontend-textlint-proofread.md); also
search for and update any cross-references to the old ADR-0009 identifier
elsewhere in the repo (including README or other ADR docs) to the new ADR
number.
- Around line 18-20: In CONTRIBUTING.md update the PR title format example to
show the full placeholder by replacing the incomplete "`: <内容>`" with "`<type>:
<内容>`" and ensure the example line still reads like the rest of the doc (e.g.,
keep the example "`feat: GitHub 連携スコア計算の追加`") so the PR title format is
unambiguous across the workflow docs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2404a139-6958-4a26-9abb-176ff582e62b

📥 Commits

Reviewing files that changed from the base of the PR and between 28ca612 and 1e61c5d.

📒 Files selected for processing (7)
  • .claude/CLAUDE.md
  • .claude/rules/backend/layers.md
  • .claude/rules/frontend/component-design.md
  • .claude/settings.json
  • .github/PULL_REQUEST_TEMPLATE.md
  • .report/refactoring-plan-20260612.md
  • CONTRIBUTING.md

Comment thread .claude/settings.json Outdated
Comment on lines +18 to +20

### 破壊的変更

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the PR title format text.

Line 19 omits the required <type> placeholder, even though the template later lists the correct type: <内容> convention. As written, this will steer contributors toward malformed titles.

Suggested fix
-- PR タイトルは `: <内容>` の形式(例: `feat: GitHub 連携スコア計算の追加`)
+- PR タイトルは `<type>: <内容>` の形式(例: `feat: GitHub 連携スコア計算の追加`)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
### 破壊的変更
- PR タイトルは `<type>: <内容>` の形式(例: `feat: GitHub 連携スコア計算の追加`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/PULL_REQUEST_TEMPLATE.md around lines 18 - 20, Update the PR title
example to include the required "<type>" placeholder so it matches the later
convention; specifically edit the PR title format line near the "### 破壊的変更"
section to use the pattern "type: <内容> — short description" (or include "<type>"
where it was omitted) so contributors are guided to produce well-formed titles
consistent with the template’s listed convention.

Comment thread .report/refactoring-plan-20260612.md Outdated
Comment thread CONTRIBUTING.md
Comment thread CONTRIBUTING.md
claude added 2 commits June 12, 2026 15:34
- models/resume.py: sort_utils(services 層)の import を除去し、@Property を
  list(self.<xxx>_rows) に簡略化。save 時に sort_order で順序確定済みのため動作不変。
- routers/blog/accounts.py + account_service.py: add_account のロジックを
  BlogAccountService へ移設し、router から collector を直接 import しないようにした。
  BlogAccountAlreadyRegisteredError 例外を新規追加。
- routers/github_link.py: _get_or_create_cache(DB クエリ直書き)を
  github_link_service.get_or_create_github_link_cache へ移設。
- 影響するテストのモックパスを router → service に更新
  (test_accounts.py, test_sync.py, test_idor.py, test_mass_assignment.py)

全テスト 505 passed。

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT
CareerResumeForm(534行)に存在した3つのモーダル用useState
(showDeleteConfirm / showSaveConfirm / editingField)と関連ハンドラ
(handleDelete / handleConfirmSave)をuseCareerFormModalsフックに集約。

component-design.mdの「3個以上のモーダルuseStateは専用フックに切り出す」
ルールに対応。コンポーネントの見通しを改善しハンドラを単独でテスト可能にする。

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
backend/app/services/blog/account_service.py (1)

32-48: 💤 Low value

Consider more descriptive exception message for debugging.

Line 40 passes only platform to BlogAccountAlreadyRegisteredError, resulting in an exception message like "zenn". While the router converts this to a localized user-facing message, a more descriptive message would aid debugging and logging.

💡 Optional improvement
-        raise BlogAccountAlreadyRegisteredError(platform)
+        raise BlogAccountAlreadyRegisteredError(f"Account already registered for platform: {platform}")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/services/blog/account_service.py` around lines 32 - 48, The
current add_account raises BlogAccountAlreadyRegisteredError(platform) which
yields an unhelpful message; update the raise to include a descriptive context
string (e.g. "account already registered: {platform}/{normalized_username}")
using the normalized_username variable so logs/debugging show both platform and
username; change the raise in add_account to pass the constructed message to
BlogAccountAlreadyRegisteredError instead of just platform.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@backend/app/services/blog/account_service.py`:
- Around line 32-48: The current add_account raises
BlogAccountAlreadyRegisteredError(platform) which yields an unhelpful message;
update the raise to include a descriptive context string (e.g. "account already
registered: {platform}/{normalized_username}") using the normalized_username
variable so logs/debugging show both platform and username; change the raise in
add_account to pass the constructed message to BlogAccountAlreadyRegisteredError
instead of just platform.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 51261494-72d0-44b5-82fe-1015a755350f

📥 Commits

Reviewing files that changed from the base of the PR and between 1e61c5d and dfd1b14.

📒 Files selected for processing (9)
  • backend/app/models/resume.py
  • backend/app/routers/blog/accounts.py
  • backend/app/routers/github_link.py
  • backend/app/services/blog/account_service.py
  • backend/app/services/intelligence/github_link_service.py
  • backend/tests/blog/test_accounts.py
  • backend/tests/blog/test_sync.py
  • backend/tests/security/test_idor.py
  • backend/tests/security/test_mass_assignment.py

@yusuke0610 yusuke0610 changed the title docs: 技術的負債の再蓄積防止のためのルール・CLAUDE.md・CONTRIBUTING.md 整備 refactor: コードベース負債の修正とルール整備 Jun 12, 2026
claude added 3 commits June 12, 2026 15:56
…n> に修正してビルドエラーを解消

useDocumentForm の save は Promise<boolean> を返すが、useCareerFormModals が
() => Promise<void> と宣言していたため tsc -b で TS2322 が発生していた。
(...args: never[]) => Promise<unknown> に変更することで両方の型を受け入れられるようにした。

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT
- settings.json: Stop hookのdupe-checkを同期実行に変更(エラーが見えるよう2>/dev/null &を削除)
- CONTRIBUTING.md: PRタイトル形式の表記を「: <内容>」→「<type>: <内容>」に修正
- CONTRIBUTING.md: 重複ADR-0009エントリをADR-0011に修正
- docs/adr: 0009-frontend-textlint-proofread.md を 0011-frontend-textlint-proofread.md にリネーム
- .report/refactoring-plan-20260612.md: 未ラベルのコードフェンスにtextを追加

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT
リモート実行環境にはnixが存在せず、同期実行に変更したStop hookが
`nix: not found` でエラー終了していた。
`command -v nix` でnixの存在を確認してから実行するよう変更。
nixがある環境では同期実行・エラー可視のまま動作する。

https://claude.ai/code/session_01GBcTxFo79GGmT7FcCuREVT
@yusuke0610
yusuke0610 merged commit c1866f5 into main Jun 12, 2026
18 checks passed
@yusuke0610
yusuke0610 deleted the claude/codebase-refactoring-plan-rakv8f branch June 20, 2026 06:53
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.

2 participants