Skip to content

履歴書生成器のリファクタリングとエラーメッセージの一元化#270

Merged
yusuke0610 merged 2 commits into
mainfrom
dev
May 26, 2026
Merged

履歴書生成器のリファクタリングとエラーメッセージの一元化#270
yusuke0610 merged 2 commits into
mainfrom
dev

Conversation

@yusuke0610

@yusuke0610 yusuke0610 commented May 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • デッドコード除去 PR の正本ドキュメント追従として .claude/rules/backend/architecture.md を現状木に同期(削除済みモジュール参照を一掃、routers/blog パッケージ・単一 TaskTypeshared/resume_format.py・休眠 LLM スタックを反映)。
  • dispatch_service の docstring を単一タスク前提へ修正し、汎用抽象を「拡張ポイントとして意図的に保持」と明記。
  • github_collector の未使用 re-export facade(__all__ の private 別名・デッド _detect_from_root_files・未使用 DEPENDENCY_TO_FRAMEWORK import)をトリムし、公開 API を実体(RepoData/collect_repos/GitHubUserNotFoundError)だけに縮小。
  • エラーメッセージ SSoT を messages.json へ寄せ、internal.py{exc} 補間(info leak)を停止。
  • markdown/pdf 両 resume_generator のプレゼンテーション非依存な入力正規化を shared/resume_format.py へ抽出(jscpd の resume clone 0 件化)。
  • github-link /run/retry の 409/404 競合パスと、run_github_link 成功時のフェーズC書き戻し内容を assert するテストを追加。test_execute_task の内部 mock 結合を DB state assert へ置換。

Applied Changes

High

  • [.claude/rules/backend/architecture.md] 正本ドキュメントの現状木同期(元レポート Findings/High)。
    • 削除済み参照を除去: routers/blog.pyrouters/career_analysis.pymodels|schemas|repositories/career_analysis.pyservices/career_analysis/ 一式、services/intelligence/{llm_summarizer,llm_advice_service,position_scorer}.py / position_weights.jsontasks/handlers/{blog_summarize,career_analysis}.py
    • 新設・現状を反映: core/env_keys.pyrouters/blog/(accounts/score/sync)パッケージ、services/shared/resume_format.py、単一 TaskType(GITHUB_LINK のみ)、markdown/pdf の generators/templates/utils 構成。
    • 「主要モジュールのポイント」も routers/blog のパッケージ化、tasks の単一タスク+拡張ポイント方針、LLM スタックが休眠インフラである旨(メモリ方針:温存・削除禁止)に更新。

Medium

  • [backend/app/services/tasks/dispatch_service.py:1-14] docstring を「3 タスク前提」から「現状 GITHUB_LINK 1 種のみ。AsyncTaskCacheService/_AsyncTaskRecord/TaskType 引数は新規タスク追加の拡張ポイントとして意図的に汎用化。インライン化しない」へ修正(元レポート Findings/Medium)。
  • [backend/app/services/intelligence/github_collector.py] 後方互換 facade トリム(元レポート Findings/Medium)。
    • __all__ を 13 → 3 件(RepoData/collect_repos/GitHubUserNotFoundError)に縮小。再エクスポート理由をコメント明記。
    • デッド _detect_from_root_filescollect_repos から未呼び出し)を削除。
    • 未使用 DEPENDENCY_TO_FRAMEWORK import を除去(skill_extractorrepo_analyzer から直接 import しており、本モジュール経由参照は 0 件を grep 確認)。
    • 事前確認: from ...github_collector import * の利用者 0 件、private 別名の外部参照 0 件。collect_repos が実際に使う関数のみ import に残置。
  • [backend/app/routers/internal.py:62,102 / github_link.py:178,190 / main.py:89] エラーメッセージ SSoT 統一 + info leak 修正(元レポート Findings/Medium)。
    • messages.jsonerror.task.unknown_task_type / error.task.execution_failed / error.github_link.not_retryable / error.validation.invalid_input を追加。
    • 各生 f-string / リテラルを get_error(...) 参照に置換。
    • internal.py:102detail=f"...: {exc}"{exc} 補間なしの定型文へ変更(例外詳細は直前の logger.exception のみに残す)。これに伴い未使用化した except Exception as exc:except Exception: へ。

Low

  • 対応なし(worker のテスト patch 用シム・response_mapper passthrough は元レポートで「現状維持で可」と判断済み)。

Test Changes

Removed

  • なし(強い削除推奨は元レポートでも無し)。

Added

  • [backend/tests/test_retry_flow.py] TestRetryEndpoints に追加:
    • test_retry_returns_404_when_no_cache — キャッシュ未作成で 404。
    • test_retry_returns_409_when_not_dead_letter(parametrize: completed / processing / pending / retrying)— dead_letter 以外は 409、かつ状態が変化しないことを assert(ガード1: is_retryable_terminal() False)。
    • test_retry_returns_409_on_concurrent_reset_raceis_retryable_terminal は通過するが try_reset_to_pending が並列競合で False を返す TOCTOU ガード(ガード2)で 409。
  • [backend/tests/test_worker/test_github_link.py] test_completed_persists_mapped_result_content — フェーズC で map_pipeline_result の出力が cache.result へそのまま永続化され、status="completed"・前回の error/warning がクリアされることを内容まで assert(既存 test_status_transitions_to_completedresult is not None 止まりだった)。

Changed (内部 mock → DB state 寄せ)

  • [backend/tests/test_worker/test_execute_task.py] test_execute_task_marks_dead_letter_on_error を、_mark_dead_letter の patch + call_args assert(実装詳細結合)から、実 DB セッションでの結果 state assert(cache.status == "dead_letter"error_message に例外文字列、completed_at 設定)へ置換(元レポート Test Review)。
  • [backend/tests/test_worker/_helpers.py] 上記のため keep_open_session(worker の db.close() を no-op 化するプロキシ)をパッケージ共有ヘルパとして追加。

Duplication Resolved

  • [backend/app/services/shared/resume_format.py] markdown/pdf 両 resume_generatorプレゼンテーション非依存な入力正規化を以下の 3 関数として抽出(元レポート Duplication/Medium):
    • normalize_clients(experience) — 旧スキーマ後方互換(clients 不在時に projects を無名取引先 1 件へ畳む)。md:旧61-63 / pdf:旧191-193 の 7 行 clone を解消。
    • normalize_team(project) — 旧スキーマ後方互換(team 不在時に scale から {"total":..., "members":[]})。md/pdf 両方の重複を解消。
    • group_stacks_by_category(stacks)technology_stacks の category 別グルーピング(挿入順保持)。
    • 表示形式に依存するラベル付け・HTML/Markdown 組み立て・format_period・エスケープは各ジェネレータに残置(元レポートの「意図的に共通化しない」判断を尊重)。
  • 確認: make dupe-checkresume_generator 関連 clone は 0 件(実行前後で resume clone が消えたことを jscpd レポートで確認)。

Structure Changes

  • ディレクトリ移動なし(元レポート Structure Review で「現構成は妥当・提案なし」)。shared/resume_format.py への関数追加のみ。

Skipped

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

Validation

  • make lint-backend: pass(All checks passed!)。github_collector / resume 系の import 並びは ruff --fix でプロジェクト規約(per-symbol import)に整形済み。
  • make test-backend: pass(420 passed, 2 warnings / 28.6s)。warning 2 件は既存・本変更と無関係(reportlab の ast.NameConstant DeprecationWarning、test_github_link.py:346 の event loop warning)。
  • make dupe-check: 全 44 clones(resume clone は 0 件化。残りは元レポートで「Allowed Duplication」分類済みの SQLAlchemy boilerplate / テストスキャフォールド / schema field 列)。
  • sandbox は ~/.cache/nix/fetcher-locks/*.lock 書き込み拒否で落ちるため、nix 実行は dangerouslyDisableSandbox: true で実施(CLAUDE.md の既知例外)。

Follow-ups

  • markdown/pdf の resume_generator 自体の単体テストが未整備(カバレッジ 6% / 12%)。normalize_clients / normalize_team / group_stacks_by_category の旧スキーマ後方互換フォールバック分岐は今回も未カバーのまま(抽出前から存在するギャップ)。次回どちらかのジェネレータを触る際に、shared 化した 3 関数のユニットテスト(新旧スキーマ両入力)を追加するのが望ましい。
  • エラーメッセージの action 文字列は既存実装に倣い inline リテラルのまま(message のみ SSoT 化)。action も SSoT へ寄せるかは別途方針判断。

Summary by CodeRabbit

  • Bug Fixes

    • GitHub linking retry operations now return standardized error messages for non-retryable states.
    • Task execution errors no longer expose internal exception details; standardized error messages are returned instead.
    • Unknown task types are now handled with clear, standardized error responses.
  • Documentation

    • Backend architecture documentation updated to reflect current module organization and service structure.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR consolidates error message handling through externalized localization, refactors resume data normalization into reusable helpers, tightens module boundaries, and expands test coverage to validate actual database state transitions during task execution and retry flows.

Changes

Backend Refactoring: Error Handling, Resume Normalization, and Tests

Layer / File(s) Summary
Architecture Documentation Update
.claude/rules/backend/architecture.md
Backend architecture rules updated to reflect env_keys.py addition, reorganized router/model/schema layout (removing career_analysis references), reshaped services section emphasizing GitHub→skill pipeline and single GITHUB_LINK task handler registration.
Error Message Externalization and Standardization
backend/app/messages.json, backend/app/main.py, backend/app/routers/github_link.py, backend/app/routers/internal.py
Validation, GitHub link retry, and task execution errors moved from inline hardcoded strings to localized messages.json entries accessed via get_error(). Validation handler, retry guard, and task type/failure handlers updated to use standardized error lookups instead of f-strings.
Resume Format Normalization Helpers
backend/app/services/shared/resume_format.py, backend/app/services/markdown/generators/resume_generator.py, backend/app/services/pdf/generators/resume_generator.py
Three new shared normalization functions extract backward-compatible data shaping: normalize_clients() handles legacy clients/projects fallback, normalize_team() converts team/scale fields, group_stacks_by_category() groups technology stacks by category. Markdown and PDF resume generators refactored to delegate data transformations to these helpers instead of repeating local logic.
GitHub Collector API Surface Restriction
backend/app/services/intelligence/github_collector.py
Module exports restricted to core public API (RepoData, collect_repos, GitHubUserNotFoundError); previously re-exported internal helpers, constants, and detect/parse symbols removed from `_all_`. Backward-compatibility note removed from docstring.
Task Dispatch Intent Documentation
backend/app/services/tasks/dispatch_service.py
Docstring clarified: only TaskType.GITHUB_LINK currently implemented; AsyncTaskCacheService structure intentionally generalized via _AsyncTaskRecord and TaskType argument for future extensibility.
Retry Endpoint Test Coverage
backend/tests/test_retry_flow.py
Added three new test cases validating retry endpoint responses: cache-miss returns 404, non-dead_letter states return 409 with unchanged cache state, concurrent reset race returns 409 when try_reset_to_pending fails.
Worker Execution Test Improvements
backend/tests/test_worker/_helpers.py, backend/tests/test_worker/test_execute_task.py, backend/tests/test_worker/test_github_link.py
New keep_open_session() helper allows tests to reuse SQLAlchemy sessions across worker finally-block close() calls. test_execute_task_marks_dead_letter_on_error rewritten to assert real GitHubLinkCache state (status, error_message, completed_at) instead of mocking internal _mark_dead_letter. New test_completed_persists_mapped_result_content validates successful worker execution persists mapped result, clears prior error/warning fields, and sets completion status/timestamp.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 Errors now speak in many tongues,
Resume data shared and reused,
Tests assert the state of things,
No more mocks of what's inside—
Reality checked, and all is true.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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.
Title check ✅ Passed The title accurately summarizes the main changes: resume generator refactoring and centralization of error messages, which aligns with the core objectives of extracting shared normalization logic and migrating to centralized error handling.

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

✨ Finishing Touches
📝 Generate docstrings
  • ✅ Generated successfully - (🔄 Check to regenerate)
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

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

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

Note

Docstrings generation - SKIPPED
Skipped regeneration as there are no new commits. Docstrings already generated for this pull request at #271.

coderabbitai Bot added a commit that referenced this pull request May 26, 2026
Docstrings generation was requested by @yusuke0610.

* #270 (comment)

The following files were modified:

* `backend/app/main.py`
* `backend/app/routers/github_link.py`
* `backend/app/routers/internal.py`
* `backend/app/services/markdown/generators/resume_generator.py`
* `backend/app/services/pdf/generators/resume_generator.py`
* `backend/app/services/shared/resume_format.py`
* `backend/tests/test_worker/_helpers.py`
* `backend/tests/test_worker/test_execute_task.py`
* `backend/tests/test_worker/test_github_link.py`
@yusuke0610 yusuke0610 changed the title Backend Refactor PR Report 履歴書生成器のリファクタリングとエラーメッセージの一元化 May 26, 2026
@yusuke0610
yusuke0610 merged commit 6889675 into main May 26, 2026
31 of 33 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