From 728e1cf755b959d09ef1662eede4c2bd2b811c74 Mon Sep 17 00:00:00 2001 From: Wada Yusuke Date: Sat, 27 Jun 2026 21:54:54 +0900 Subject: [PATCH] =?UTF-8?q?refactor(backend):=20GitHubLinkCache=20?= =?UTF-8?q?=E3=81=AE=E7=9B=B4=E3=82=AF=E3=82=A8=E3=83=AA=E3=82=92=20reposi?= =?UTF-8?q?tory=20=E3=81=B8=E9=9B=86=E7=B4=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BE_refacter の指摘（report/BE_report_20260627_2044.md）の High + Medium を適用。 - repositories/github_link.py に GitHubLinkCacheRepository を新設 （get_by_user / get_or_create）。get_or_create は IntegrityError 後の 再 SELECT が None なら RuntimeError を上げる（database.md 準拠） - 9 箇所の直クエリ（router/service/handler/context_builder）を repository 経由へ置換し layers.md パターン B 違反を解消 - routers/github_link.py を package 化し、HTTP 出力整形 to_skill_item を _responses.py へ分離 - chat_service.run_agent_chat の「呼び出し+トークン加算+ログ」を内部ヘルパへ 集約（ADR-0012 課金契約は不変） - get_or_create の競合パス単体テストを追加 make lint-backend / make test-backend(632 passed) / codegen-drift いずれも green。 Co-Authored-By: Claude Opus 4.8 --- backend/app/repositories/__init__.py | 2 + backend/app/repositories/github_link.py | 57 ++++++++++ backend/app/routers/github_link/__init__.py | 5 + backend/app/routers/github_link/_responses.py | 47 ++++++++ .../endpoints.py} | 77 ++++--------- backend/app/services/agent/chat_service.py | 28 +++-- backend/app/services/agent/context_builder.py | 4 +- .../intelligence/github_link_service.py | 20 ++-- .../services/tasks/handlers/github_link.py | 3 +- .../test_github_link_cache_repository.py | 104 ++++++++++++++++++ 10 files changed, 266 insertions(+), 81 deletions(-) create mode 100644 backend/app/repositories/github_link.py create mode 100644 backend/app/routers/github_link/__init__.py create mode 100644 backend/app/routers/github_link/_responses.py rename backend/app/routers/{github_link.py => github_link/endpoints.py} (74%) create mode 100644 backend/tests/test_github_link_cache_repository.py diff --git a/backend/app/repositories/__init__.py b/backend/app/repositories/__init__.py index f6023e8f..fb7caf1d 100644 --- a/backend/app/repositories/__init__.py +++ b/backend/app/repositories/__init__.py @@ -3,6 +3,7 @@ from .base import BaseMasterRepository, SingleUserDocumentRepository from .billing import BillingRepository from .blog import BlogAccountRepository, BlogArticleRepository +from .github_link import GitHubLinkCacheRepository from .master_data import MQualificationRepository, MTechnologyStackRepository from .resume import ResumeRepository from .skill import GitHubSkillRepository @@ -13,6 +14,7 @@ "BillingRepository", "BlogAccountRepository", "BlogArticleRepository", + "GitHubLinkCacheRepository", "GitHubSkillRepository", "MQualificationRepository", "MTechnologyStackRepository", diff --git a/backend/app/repositories/github_link.py b/backend/app/repositories/github_link.py new file mode 100644 index 00000000..1e1f3c00 --- /dev/null +++ b/backend/app/repositories/github_link.py @@ -0,0 +1,57 @@ +"""GitHub 連携キャッシュ（``GitHubLinkCache``）のデータアクセス。 + +``GitHubLinkCache`` はユーザーあたり 1 件のレコードで、``user_id`` を一意境界とする。 +取得クエリ（``filter_by(user_id=...)``）を本リポジトリへ集約し、router / service / +handler / context_builder からの直クエリ散在を防ぐ。``user_id`` スコープは +IDOR 防止の認可境界であり、1 箇所に閉じ込めることで条件追加時の漏れを防ぐ。 +""" + +from sqlalchemy import select +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + +from ..models import GitHubLinkCache + + +class GitHubLinkCacheRepository: + """ユーザーの GitHub 連携キャッシュの読み取り・作成。 + + セッションはコンストラクタで受け取る。GitHub 連携の実行経路では libSQL の + idle stream timeout 対策でフェーズごとにセッションを開閉するため、本リポジトリは + セッションを保持せず呼び出し側が渡したものをそのまま使う。 + """ + + def __init__(self, db: Session): + self.db = db + + def get_by_user(self, user_id: str) -> GitHubLinkCache | None: + """ユーザーのキャッシュを取得する。存在しなければ ``None``。""" + return self.db.scalar( + select(GitHubLinkCache).where(GitHubLinkCache.user_id == user_id) + ) + + def get_or_create(self, user_id: str) -> GitHubLinkCache: + """ユーザーのキャッシュを取得し、存在しなければ作成して flush する。 + + 並列リクエストが ``user_id`` の一意制約で衝突した場合は rollback して再取得する。 + 他セッションが先に commit していたケースを想定し、再 SELECT が ``None`` を返したら + 明示的に ``RuntimeError`` を上げて non-Optional な戻り値契約を守る + （.claude/rules/backend/database.md「IntegrityError 後の再 SELECT は None を判定する」）。 + """ + cache = self.get_by_user(user_id) + if cache is not None: + return cache + + cache = GitHubLinkCache(user_id=user_id) + self.db.add(cache) + try: + self.db.flush() + except IntegrityError: + self.db.rollback() + existing = self.get_by_user(user_id) + if existing is None: + raise RuntimeError( + f"GitHubLinkCache の作成と再取得に失敗しました (user_id={user_id})" + ) from None + return existing + return cache diff --git a/backend/app/routers/github_link/__init__.py b/backend/app/routers/github_link/__init__.py new file mode 100644 index 00000000..d8a03648 --- /dev/null +++ b/backend/app/routers/github_link/__init__.py @@ -0,0 +1,5 @@ +"""GitHub 連携 API パッケージ。""" + +from .endpoints import router + +__all__ = ["router"] diff --git a/backend/app/routers/github_link/_responses.py b/backend/app/routers/github_link/_responses.py new file mode 100644 index 00000000..9d5165cf --- /dev/null +++ b/backend/app/routers/github_link/_responses.py @@ -0,0 +1,47 @@ +"""GitHub 連携 API の ORM → レスポンススキーマ変換。 + +HTTP 出力整形（プレゼンテーション層）を endpoints から分離する +（.claude/rules/common/duplication.md の Backend ヒエラルキー「routers//_responses.py」）。 +""" + +from ...schemas.github_skill import ( + GitHubSkillItem, + SkillEvidence, + SkillProficiency, +) + + +def to_skill_item(skill) -> GitHubSkillItem: + """ORM の GitHubSkill を API スキーマへ変換する。""" + proficiency = None + if skill.proficiency is not None: + proficiency = SkillProficiency( + self_assessed_level=skill.proficiency.self_assessed_level, + narrative=skill.proficiency.narrative, + duration_months=skill.proficiency.duration_months, + scale=skill.proficiency.scale, + source=skill.proficiency.source, + reviewed=skill.proficiency.reviewed, + ) + return GitHubSkillItem( + kind=skill.kind, + canonical_name=skill.canonical_name, + # 言語は ecosystem を "" で持つので API では null に正規化する + ecosystem=skill.ecosystem or None, + parent=skill.parent, + display_name=skill.display_name, + evidence=[ + SkillEvidence( + repo_full_name=ev.repo_full_name, + repo_url=ev.repo_url, + signal_source=ev.signal_source, + confidence=ev.confidence, + language_bytes=ev.language_bytes, + dependency_kind=ev.dependency_kind, + manifest_path=ev.manifest_path, + partial_scan=ev.partial_scan, + ) + for ev in skill.evidence + ], + proficiency=proficiency, + ) diff --git a/backend/app/routers/github_link.py b/backend/app/routers/github_link/endpoints.py similarity index 74% rename from backend/app/routers/github_link.py rename to backend/app/routers/github_link/endpoints.py index d45dc423..d08fec7a 100644 --- a/backend/app/routers/github_link.py +++ b/backend/app/routers/github_link/endpoints.py @@ -11,27 +11,24 @@ from fastapi import APIRouter, BackgroundTasks, Depends, Request from sqlalchemy.orm import Session -from ..core.errors import ErrorCode, raise_app_error, resolve_async_error_code -from ..core.messages import get_error -from ..core.security.auth import get_current_user -from ..core.security.dependencies import limiter -from ..db import get_db -from ..models import GitHubLinkCache, User -from ..repositories.skill import GitHubSkillRepository -from ..schemas.github_link import ( +from ...core.errors import ErrorCode, raise_app_error, resolve_async_error_code +from ...core.messages import get_error +from ...core.security.auth import get_current_user +from ...core.security.dependencies import limiter +from ...db import get_db +from ...models import User +from ...repositories.github_link import GitHubLinkCacheRepository +from ...repositories.skill import GitHubSkillRepository +from ...schemas.github_link import ( CachedGitHubLinkResponse, GitHubLinkRequest, ProgressResponse, ) -from ..schemas.github_skill import ( - GitHubSkillItem, - GitHubSkillsResponse, - SkillEvidence, - SkillProficiency, -) -from ..schemas.shared import TaskAcceptedResponse, TaskStatusResponse -from ..services.intelligence.github_link_service import get_or_create_github_link_cache -from ..services.tasks import AsyncTaskCacheService, TaskType +from ...schemas.github_skill import GitHubSkillsResponse +from ...schemas.shared import TaskAcceptedResponse, TaskStatusResponse +from ...services.intelligence.github_link_service import get_or_create_github_link_cache +from ...services.tasks import AsyncTaskCacheService, TaskType +from ._responses import to_skill_item logger = logging.getLogger(__name__) @@ -69,7 +66,7 @@ def get_cache( db: Session = Depends(get_db), ): """保存済みの連携結果を取得する。""" - cache = db.query(GitHubLinkCache).filter_by(user_id=user.id).first() + cache = GitHubLinkCacheRepository(db).get_by_user(user.id) if not cache: return CachedGitHubLinkResponse() return CachedGitHubLinkResponse( @@ -89,48 +86,12 @@ async def get_link_progress( Redis にデータがない場合（タスク未開始・Redis 障害）は step_index=0 のデフォルトを返す。 """ - from ..services.progress_service import get_progress + from ...services.progress_service import get_progress data = await get_progress(user.id) return ProgressResponse(**data) -def _to_skill_item(skill) -> GitHubSkillItem: - """ORM の GitHubSkill を API スキーマへ変換する。""" - proficiency = None - if skill.proficiency is not None: - proficiency = SkillProficiency( - self_assessed_level=skill.proficiency.self_assessed_level, - narrative=skill.proficiency.narrative, - duration_months=skill.proficiency.duration_months, - scale=skill.proficiency.scale, - source=skill.proficiency.source, - reviewed=skill.proficiency.reviewed, - ) - return GitHubSkillItem( - kind=skill.kind, - canonical_name=skill.canonical_name, - # 言語は ecosystem を "" で持つので API では null に正規化する - ecosystem=skill.ecosystem or None, - parent=skill.parent, - display_name=skill.display_name, - evidence=[ - SkillEvidence( - repo_full_name=ev.repo_full_name, - repo_url=ev.repo_url, - signal_source=ev.signal_source, - confidence=ev.confidence, - language_bytes=ev.language_bytes, - dependency_kind=ev.dependency_kind, - manifest_path=ev.manifest_path, - partial_scan=ev.partial_scan, - ) - for ev in skill.evidence - ], - proficiency=proficiency, - ) - - @router.get("/skills", response_model=GitHubSkillsResponse) def get_skills( user: User = Depends(get_current_user), @@ -141,7 +102,7 @@ def get_skills( 連携がまだ実行されていない場合は空配列を返す。 """ skills = GitHubSkillRepository(db, user.id).list_for_user() - return GitHubSkillsResponse(skills=[_to_skill_item(s) for s in skills]) + return GitHubSkillsResponse(skills=[to_skill_item(s) for s in skills]) @router.get("/cache/status", response_model=TaskStatusResponse) @@ -150,7 +111,7 @@ def get_cache_status( db: Session = Depends(get_db), ): """連携ステータスを返す（軽量ポーリング用）。""" - cache = db.query(GitHubLinkCache).filter_by(user_id=user.id).first() + cache = GitHubLinkCacheRepository(db).get_by_user(user.id) if not cache: return TaskStatusResponse(status="completed") return TaskStatusResponse( @@ -213,7 +174,7 @@ async def retry_github_link( ``dead_letter`` 状態のキャッシュのみ再実行可能。 ``retry_count`` を 0 にリセットし、ステータスを ``pending`` に戻して再ディスパッチする。 """ - cache = db.query(GitHubLinkCache).filter_by(user_id=user.id).first() + cache = GitHubLinkCacheRepository(db).get_by_user(user.id) if not cache: raise_app_error( status_code=404, diff --git a/backend/app/services/agent/chat_service.py b/backend/app/services/agent/chat_service.py index fe6f033f..0d2af8ef 100644 --- a/backend/app/services/agent/chat_service.py +++ b/backend/app/services/agent/chat_service.py @@ -301,11 +301,24 @@ async def run_agent_chat( input_tokens = 0 output_tokens = 0 + async def _generate_and_account(call_messages: list[dict], *, label: str): + """LLM を呼び出し、合算トークンへ加算してから生応答を返す。 + + 課金漏れ防止（ADR-0012）のため、初回・リトライの両方でトークン加算経路を + この 1 箇所に集約する。``client.generate`` が失敗した場合は加算前に例外が伝播し、 + 呼び出し元で確定済みの合算使用量を載せて再 raise する（使用量の二重計上を防ぐ）。 + """ + nonlocal input_tokens, output_tokens + call_result = await client.generate( + system_prompt, call_messages, output_schema, model_id + ) + input_tokens += call_result.input_tokens + output_tokens += call_result.output_tokens + logger.debug("Agent LLM %s応答（パース前）: len=%d", label, len(call_result.text)) + return call_result + # 1 回目の呼び出し。パースまで成功すればここで確定して返す。 - result = await client.generate(system_prompt, messages, output_schema, model_id) - input_tokens += result.input_tokens - output_tokens += result.output_tokens - logger.debug("Agent LLM 生応答（パース前）: len=%d", len(result.text)) + result = await _generate_and_account(messages, label="生") try: response = _parse_response(result.text, request.scope) return AgentChatResult( @@ -331,17 +344,12 @@ async def run_agent_chat( # リトライ呼び出し（1 回のみ）。以降は失敗時も合算使用量を載せて伝播する。 try: - result = await client.generate( - system_prompt, retry_messages, output_schema, model_id - ) + result = await _generate_and_account(retry_messages, label="リトライ") except LLMError as retry_exc: # リトライ呼び出し自体が失敗。1 回目の API 原価は発生済みのため使用量を # 載せて伝播し、router 側で課金を確定させる（課金漏れを防ぐ / ADR-0012） retry_exc.usage = _make_usage(request, input_tokens, output_tokens) raise - input_tokens += result.input_tokens - output_tokens += result.output_tokens - logger.debug("Agent LLM リトライ応答（パース前）: len=%d", len(result.text)) try: response = _parse_response(result.text, request.scope) except AgentResponseParseError as retry_exc: diff --git a/backend/app/services/agent/context_builder.py b/backend/app/services/agent/context_builder.py index 11f08ce7..a2094fe1 100644 --- a/backend/app/services/agent/context_builder.py +++ b/backend/app/services/agent/context_builder.py @@ -13,8 +13,8 @@ from sqlalchemy.orm import Session -from ...models.cache import GitHubLinkCache from ...repositories.blog import BlogArticleRepository +from ...repositories.github_link import GitHubLinkCacheRepository from ...services.blog.scorer import blog_articles_to_score_dicts, calculate_blog_score logger = logging.getLogger(__name__) @@ -63,7 +63,7 @@ def build_reference_context(db: Session, user_id: str, scope: str) -> dict | Non def _build_github_context(db: Session, user_id: str) -> dict | None: """GitHubLinkCache から圧縮済み GitHub コンテキストを生成する。""" try: - cache = db.query(GitHubLinkCache).filter_by(user_id=user_id).first() + cache = GitHubLinkCacheRepository(db).get_by_user(user_id) if not cache or cache.status != "completed" or not cache.result: return None diff --git a/backend/app/services/intelligence/github_link_service.py b/backend/app/services/intelligence/github_link_service.py index 68cb150e..1457178f 100644 --- a/backend/app/services/intelligence/github_link_service.py +++ b/backend/app/services/intelligence/github_link_service.py @@ -16,6 +16,7 @@ from ...core.logging_utils import get_logger from ...core.messages import get_error from ...models import GitHubLinkCache +from ...repositories.github_link import GitHubLinkCacheRepository from ...repositories.skill import GitHubSkillRepository from ..progress_service import set_progress from ..tasks.exceptions import NonRetryableError @@ -36,13 +37,12 @@ def _now() -> datetime: def get_or_create_github_link_cache(db: Session, user_id: str) -> GitHubLinkCache: - """ユーザーの GitHubLinkCache レコードを取得し、存在しなければ作成する。""" - cache = db.query(GitHubLinkCache).filter_by(user_id=user_id).first() - if not cache: - cache = GitHubLinkCache(user_id=user_id) - db.add(cache) - db.flush() - return cache + """ユーザーの GitHubLinkCache レコードを取得し、存在しなければ作成する。 + + 取得・作成ロジックは GitHubLinkCacheRepository に集約済み。本関数は既存呼び出し元 + （router）の入口を維持するための薄いラッパ。 + """ + return GitHubLinkCacheRepository(db).get_or_create(user_id) async def run_github_link(session_factory: SessionFactory, payload: dict) -> None: @@ -64,7 +64,7 @@ async def run_github_link(session_factory: SessionFactory, payload: dict) -> Non # ── フェーズA: 検証 + processing マーク ───────────────────────────────── with session_factory() as db: - cache = db.query(GitHubLinkCache).filter_by(user_id=user_id).first() + cache = GitHubLinkCacheRepository(db).get_by_user(user_id) if not cache: message = "GitHub 連携キャッシュが見つかりません" logger.error(message, extra={"user_id": user_id}) @@ -101,7 +101,7 @@ async def _on_repo_fetched(done: int, total: int) -> None: ) except GitHubUserNotFoundError as exc: with session_factory() as db: - cache = db.query(GitHubLinkCache).filter_by(user_id=user_id).first() + cache = GitHubLinkCacheRepository(db).get_by_user(user_id) if cache: cache.status = "dead_letter" cache.error_message = ( @@ -147,7 +147,7 @@ async def _on_repo_fetched(done: int, total: int) -> None: # ステップ 4: DB 保存 await set_progress(task_id, 4, _TOTAL_STEPS, "結果を保存中...") with session_factory() as db: - cache = db.query(GitHubLinkCache).filter_by(user_id=user_id).first() + cache = GitHubLinkCacheRepository(db).get_by_user(user_id) if not cache: logger.warning( "結果書き戻し時にキャッシュが見つかりません", diff --git a/backend/app/services/tasks/handlers/github_link.py b/backend/app/services/tasks/handlers/github_link.py index 2f3772c3..2a2189fe 100644 --- a/backend/app/services/tasks/handlers/github_link.py +++ b/backend/app/services/tasks/handlers/github_link.py @@ -3,6 +3,7 @@ from sqlalchemy.orm import Session from ....models import GitHubLinkCache +from ....repositories.github_link import GitHubLinkCacheRepository from .base import SessionFactory, TaskHandler @@ -13,7 +14,7 @@ def get_record(self, db: Session, payload: dict) -> GitHubLinkCache | None: user_id = payload.get("user_id") if not user_id: return None - return db.query(GitHubLinkCache).filter_by(user_id=user_id).first() + return GitHubLinkCacheRepository(db).get_by_user(user_id) async def run(self, session_factory: SessionFactory, payload: dict) -> None: # 循環インポート回避のため遅延 import する diff --git a/backend/tests/test_github_link_cache_repository.py b/backend/tests/test_github_link_cache_repository.py new file mode 100644 index 00000000..5473939f --- /dev/null +++ b/backend/tests/test_github_link_cache_repository.py @@ -0,0 +1,104 @@ +"""GitHubLinkCacheRepository の単体テスト。 + +特に get_or_create の競合パス（IntegrityError 後の再 SELECT 分岐）を検証する。 +.claude/rules/backend/database.md「IntegrityError 後の再 SELECT は None を判定する」の +再発防止契約を守るためのテスト。DB はモックせず実 SQLite セッションを使う。 +""" + +import pytest +from app.models import GitHubLinkCache +from app.repositories import GitHubLinkCacheRepository, UserRepository +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + + +def _make_user(db: Session, username: str = "gh-user"): + return UserRepository(db).create(username, email=f"{username}@test.com") + + +def test_get_by_user_returns_none_when_absent(db_session: Session): + """キャッシュが無いユーザーでは None を返す。""" + repo = GitHubLinkCacheRepository(db_session) + assert repo.get_by_user("missing-user") is None + + +def test_get_or_create_creates_when_absent(db_session: Session): + """キャッシュが無ければ新規作成して flush する（id が採番される）。""" + user = _make_user(db_session) + repo = GitHubLinkCacheRepository(db_session) + + cache = repo.get_or_create(user.id) + + assert cache.user_id == user.id + assert cache.id is not None + # 同一ユーザーで再度呼んでも新規作成せず同じ行を返す + again = repo.get_or_create(user.id) + assert again.id == cache.id + + +def test_get_or_create_returns_existing(db_session: Session): + """既存キャッシュがあればそれを返し、重複作成しない。""" + user = _make_user(db_session) + existing = GitHubLinkCache(user_id=user.id, status="completed") + db_session.add(existing) + db_session.commit() + + repo = GitHubLinkCacheRepository(db_session) + cache = repo.get_or_create(user.id) + + assert cache.id == existing.id + assert cache.status == "completed" + + +def test_get_or_create_returns_existing_on_integrity_error( + db_session: Session, monkeypatch: pytest.MonkeyPatch +): + """並列作成で user_id 一意制約に衝突しても、rollback 後の再 SELECT で既存行を返す。 + + 他リクエストが先に commit していたレースを再現する。最初の SELECT は行を見逃し + （None）、INSERT で IntegrityError → rollback → 再 SELECT で既存行を取得する経路。 + """ + user = _make_user(db_session) + # 先行リクエストが commit 済みの行（最初の get_by_user では見逃す想定） + committed = GitHubLinkCache(user_id=user.id, status="processing") + db_session.add(committed) + db_session.commit() + + repo = GitHubLinkCacheRepository(db_session) + real_get_by_user = repo.get_by_user + calls = {"n": 0} + + def fake_get_by_user(uid: str): + # 1 回目（INSERT 前のチェック）は None を返して create 分岐へ進ませる。 + # 2 回目以降（rollback 後の再 SELECT）は実クエリで既存行を返す。 + calls["n"] += 1 + if calls["n"] == 1: + return None + return real_get_by_user(uid) + + monkeypatch.setattr(repo, "get_by_user", fake_get_by_user) + + cache = repo.get_or_create(user.id) + + assert cache.id == committed.id + assert calls["n"] == 2 + + +def test_get_or_create_raises_runtime_error_when_reselect_none( + db_session: Session, monkeypatch: pytest.MonkeyPatch +): + """IntegrityError 後の再 SELECT も None なら RuntimeError を上げる（None を握りつぶさない）。""" + user = _make_user(db_session) + repo = GitHubLinkCacheRepository(db_session) + + # get_by_user は常に None（行が見つからない異常状態） + monkeypatch.setattr(repo, "get_by_user", lambda uid: None) + + # flush は一意制約衝突を模して IntegrityError を投げる + def fake_flush(): + raise IntegrityError("INSERT", {}, Exception("UNIQUE constraint failed")) + + monkeypatch.setattr(db_session, "flush", fake_flush) + + with pytest.raises(RuntimeError, match="再取得に失敗"): + repo.get_or_create(user.id)