GitHub連携データから経歴書ドラフトPDFを生成する機能を追加#463
Merged
Merged
Conversation
構造（プロジェクト・技術スタック・期間）はルールベースで決定論的に組み立て、 自然文（職務要約・自己PR・プロジェクト説明）のみLLMで生成するハイブリッド方式。 生成物はPDFのみでDBには保存しない（ADR-0018）。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
github-actions
Bot
added
documentation
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds ADR-0018 resume draft generation from GitHub-linked data, including backend draft source/mapping/LLM pipeline, a new authenticated PDF endpoint with billing hooks, schema and cache-response extensions, frontend preview generation, and related tests and docs. ChangesResume Draft Generation Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
web/src/api/download.ts (2)
38-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
getBlobUrlhardcodes a generic fallback message for all callers.
FALLBACK_MESSAGES.PREVIEW_FETCHis baked intogetBlobUrlitself, so every current and future consumer (now including resume-draft PDF generation) shows the same generic fallback wording when the backend response has no JSON error body. Consider accepting the fallback message as a parameter, mirroring howtoApiErroralready does.♻️ Suggested change
-export async function getBlobUrl(url: string, options?: RequestInit): Promise<string> { +export async function getBlobUrl( + url: string, + options?: RequestInit, + fallbackMessage: string = FALLBACK_MESSAGES.PREVIEW_FETCH, +): Promise<string> { const response = await fetch(`${API_BASE_URL}${url}`, { ...options, headers: buildHeaders(options), credentials: "include", }); if (!response.ok) { // AppErrorResponse の code / message / action を保持して呼び出し元の分岐・表示に使う - throw await toApiError(response, FALLBACK_MESSAGES.PREVIEW_FETCH); + throw await toApiError(response, fallbackMessage); } const blob = await response.blob(); return URL.createObjectURL(blob); }🤖 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 `@web/src/api/download.ts` around lines 38 - 50, getBlobUrl currently hardcodes FALLBACK_MESSAGES.PREVIEW_FETCH, so all callers share the same generic fallback text even when they need different wording. Update getBlobUrl to accept a fallback message parameter, pass it through to toApiError, and adjust existing callers to provide their own context-specific fallback message (including the resume-draft PDF path) so error handling remains caller-specific.
2-14: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExtract the shared CSRF header builder
download.tsduplicates the POST/PUT/DELETE/PATCH CSRF rule fromclient.ts. Share that logic instead of re-implementing it here so both fetch paths stay aligned.🤖 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 `@web/src/api/download.ts` around lines 2 - 14, Move the CSRF header construction logic out of download.ts and reuse the shared implementation from client.ts instead of duplicating the POST/PUT/DELETE/PATCH check in buildHeaders. Update buildHeaders in download.ts to call the shared helper used by request() so both fetch paths stay aligned and any future CSRF rule changes live in one place.web/src/hooks/useResumeDraftPdf.ts (1)
17-46: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBlob URL can leak on unmount or repeated
generate()calls.Two related gaps in this hook's Blob URL lifecycle:
- If the component unmounts while
previewUrlis set (e.g., user navigates away with the preview modal open), the object URL is never revoked — no cleanup effect exists.- If
generate()is called again while a previouspreviewUrlis still set (beforeclosePreview()), the old URL is overwritten bysetPreviewUrlwithout revoking it, leaking the previous Blob.Currently the dashboard only exposes the generate button, which is naturally blocked by the full-screen preview overlay while a preview is open, so case 2 is not reachable through existing UI. But case 1 (navigation away mid-preview) is reachable and leaks memory until reload.
♻️ Suggested cleanup
import { useState } from "react"; +import { useEffect, useRef } from "react"; ... export function useResumeDraftPdf(model: AgentModelAlias) { const [generating, setGenerating] = useState(false); const [previewUrl, setPreviewUrl] = useState<string | null>(null); const [error, setError] = useState<AppErrorState | null>(null); + const previewUrlRef = useRef<string | null>(null); + previewUrlRef.current = previewUrl; + + // アンマウント時に残っている Blob URL を解放する + useEffect(() => { + return () => { + if (previewUrlRef.current) { + URL.revokeObjectURL(previewUrlRef.current); + } + }; + }, []);🤖 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 `@web/src/hooks/useResumeDraftPdf.ts` around lines 17 - 46, The Blob URL lifecycle in useResumeDraftPdf is incomplete: previewUrl is revoked only in closePreview, so it can leak on unmount or when generate overwrites an existing preview. Add cleanup in the hook itself (for example with an effect tied to previewUrl/unmount) to revoke any current object URL when the component goes away, and in generate revoke the existing previewUrl before calling setPreviewUrl with the new value. Keep the existing closePreview behavior consistent with this cleanup.backend/app/services/agent/resume_draft/draft_service.py (1)
202-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRetry control flow is correct but relies on implicit fallthrough.
retry_messagesis only assigned inside theexcept AgentResponseParseErrorblock, and is safely readable afterward only because the success path returns unconditionally inside the firsttry. This works today (and is well covered by tests), but it's a fragile pattern for future edits — restructuring into a nested try makes the dependency between the exception and the retry code explicit.♻️ Suggested restructure for clarity
result = await _generate_and_account(messages, label="生") try: output = _parse_draft(result.text, set(allowed_names)) - return ResumeDraftResult( - payload=_merge_output(skeleton, selected, output), usage=_usage() - ) except AgentResponseParseError as exc: # 出力契約違反は 1 回だけリトライ（違反内容をフィードバックして再生成 / ADR-0010） logger.warning("ドラフト LLM 応答が出力契約に違反したためリトライ: %s", type(exc).__name__) retry_messages = [ *messages, {"role": "assistant", "content": result.text}, { "role": "user", "content": ( "直前の応答は出力契約に違反しています。" f"違反内容: {str(exc)[:_MAX_RETRY_ERROR_LENGTH]}\n" "契約に従って同じ依頼への応答を再生成してください。" ), }, ] - - try: - result = await _generate_and_account(retry_messages, label="リトライ") - except LLMError as retry_exc: - # 1 回目の API 原価は発生済み。使用量を載せて router 側で課金を確定させる（ADR-0012） - retry_exc.usage = _usage() - raise - try: - output = _parse_draft(result.text, set(allowed_names)) - except AgentResponseParseError as retry_exc: - # 2 回目も失敗。合算使用量を載せて伝播する（課金漏れ防止 / ADR-0012） - raise AgentResponseParseError(str(retry_exc), usage=_usage()) from retry_exc - return ResumeDraftResult(payload=_merge_output(skeleton, selected, output), usage=_usage()) + try: + result = await _generate_and_account(retry_messages, label="リトライ") + except LLMError as retry_exc: + retry_exc.usage = _usage() + raise + try: + output = _parse_draft(result.text, set(allowed_names)) + except AgentResponseParseError as retry_exc: + raise AgentResponseParseError(str(retry_exc), usage=_usage()) from retry_exc + + return ResumeDraftResult(payload=_merge_output(skeleton, selected, output), usage=_usage())🤖 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/agent/resume_draft/draft_service.py` around lines 202 - 235, The retry path in draft_service’s resume draft flow depends on implicit fallthrough from the first _generate_and_account/_parse_draft try block, which makes retry_messages look conditionally defined. Restructure this logic so the retry generation and second _parse_draft are clearly nested inside the AgentResponseParseError handler in draft_service, keeping the control flow explicit around _generate_and_account, _parse_draft, and ResumeDraftResult.
🤖 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 `@backend/app/routers/agent.py`:
- Around line 188-191: The usage charge is committed before PDF generation in
the agent resume flow, so a PDF failure can bill the user without delivering a
file. Update the logic around _record_usage_after_llm, build_resume_pdf, and
stream_pdf so the PDF bytes are generated first, and only if
build_resume_pdf(result.payload) succeeds should _record_usage_after_llm be
called and the response streamed. Keep the usage description and billing
behavior unchanged, but ensure the credit record happens after the bytes exist.
In `@backend/app/services/agent/resume_draft/context.py`:
- Around line 80-95: The GitHub link validation in the resume draft context is
treating an empty repos list as an error state, which can misclassify a
legitimate completed link as “旧形式.” Update the logic in the context lookup path
around GitHubLinkResponse.model_validate and the subsequent result.repos check
to only reject truly missing legacy data, not an intentionally empty list
persisted by run_github_link(). Keep the completed-state check, but remove or
narrow the empty-list fallback so a valid completed cache with zero repos is
accepted.
In `@docs/adr/0018-github-resume-draft-generation.md`:
- Around line 45-46: `collect_repos()` can return a valid empty list, so using
`not result.repos` to detect legacy cache incorrectly treats zero-repo users as
stale. Update the GitHub link response/cached result handling around
`GitHubLinkResponse` and `github_link_cache.result` to distinguish legacy JSON
from a legitimate empty repo snapshot using an explicit version/marker field or
equivalent metadata, and only return 409 for truly old cached formats.
---
Nitpick comments:
In `@backend/app/services/agent/resume_draft/draft_service.py`:
- Around line 202-235: The retry path in draft_service’s resume draft flow
depends on implicit fallthrough from the first
_generate_and_account/_parse_draft try block, which makes retry_messages look
conditionally defined. Restructure this logic so the retry generation and second
_parse_draft are clearly nested inside the AgentResponseParseError handler in
draft_service, keeping the control flow explicit around _generate_and_account,
_parse_draft, and ResumeDraftResult.
In `@web/src/api/download.ts`:
- Around line 38-50: getBlobUrl currently hardcodes
FALLBACK_MESSAGES.PREVIEW_FETCH, so all callers share the same generic fallback
text even when they need different wording. Update getBlobUrl to accept a
fallback message parameter, pass it through to toApiError, and adjust existing
callers to provide their own context-specific fallback message (including the
resume-draft PDF path) so error handling remains caller-specific.
- Around line 2-14: Move the CSRF header construction logic out of download.ts
and reuse the shared implementation from client.ts instead of duplicating the
POST/PUT/DELETE/PATCH check in buildHeaders. Update buildHeaders in download.ts
to call the shared helper used by request() so both fetch paths stay aligned and
any future CSRF rule changes live in one place.
In `@web/src/hooks/useResumeDraftPdf.ts`:
- Around line 17-46: The Blob URL lifecycle in useResumeDraftPdf is incomplete:
previewUrl is revoked only in closePreview, so it can leak on unmount or when
generate overwrites an existing preview. Add cleanup in the hook itself (for
example with an effect tied to previewUrl/unmount) to revoke any current object
URL when the component goes away, and in generate revoke the existing previewUrl
before calling setPreviewUrl with the new value. Keep the existing closePreview
behavior consistent with this cleanup.
🪄 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: 255eba1e-de5a-48bb-a068-4619d6efcd21
📒 Files selected for processing (33)
.claude/rules/backend/agent.md.claude/rules/backend/architecture.mdbackend/app/core/security/auth.pybackend/app/messages.jsonbackend/app/prompts/agent_resume_draft.mdbackend/app/routers/agent.pybackend/app/routers/github_link/endpoints.pybackend/app/schemas/agent.pybackend/app/schemas/github_link.pybackend/app/services/agent/resume_draft/__init__.pybackend/app/services/agent/resume_draft/context.pybackend/app/services/agent/resume_draft/draft_service.pybackend/app/services/agent/resume_draft/mapper.pybackend/app/services/agent/resume_draft/output_schema.pybackend/app/services/billing/credit_service.pybackend/app/services/intelligence/github_link_service.pybackend/app/services/intelligence/response_mapper.pybackend/tests/test_resume_draft_api.pybackend/tests/test_resume_draft_mapper.pybackend/tests/test_resume_draft_service.pydocs/adr/0018-github-resume-draft-generation.mddocs/adr/README.mddocs/design-principles.mdweb/src/api/agent.tsweb/src/api/client.tsweb/src/api/download.tsweb/src/api/generated.tsweb/src/api/paths.tsweb/src/components/github-link/GitHubLinkDashboard.tsxweb/src/constants/messages.tsweb/src/hooks/useResumeDraftPdf.test.tsweb/src/hooks/useResumeDraftPdf.tsweb/src/test/renderWithProviders.tsx
- PDF 生成成功後に課金する順序へ変更（生成失敗時にユーザー課金しない） - 旧形式キャッシュ判定を repos キー有無に変更し、0件ユーザーを別導線（別メッセージ）に - useResumeDraftPdf の Blob URL をアンマウント時・再生成時に解放（リーク防止） - CSRF ヘッダ付与を client.ts の共有ヘルパー applyCsrfHeader に集約 - getBlobUrl の fallback メッセージを呼び出し元指定にパラメータ化 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TestClient がデフォルトで生の例外を再 raise し、CI では Starlette の TaskGroup が RuntimeError を ExceptionGroup にラップするため res.status_code の検証に到達せず落ちていた。pytest.raises に ExceptionGroup を直接渡すとグループ検査モードになり素直にマッチ しないため、BaseException で受けて例外型に依存しない検証にする。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/tests/test_resume_draft_api.py (1)
197-202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBroad
pytest.raises(BaseException)weakens the assertion, but is a reasonable environment-compatibility trade-off.This will pass for essentially any exception raised during the request, not just the intended PDF-generation
RuntimeError(wrapped or not). The comment explains this is intentional to dodge inconsistentExceptionGroupwrapping across environments, and the follow-upAgentUsageLogcount check still verifies the core no-billing behavior, so this is acceptable as-is.If you want a bit more assurance without reintroducing environment coupling, you could capture the exception and check the message substring across both plain and grouped cases:
♻️ Optional: tighten assertion while staying exception-type agnostic
- with pytest.raises(BaseException): # noqa: B017, PT011 - client.post("/api/agent/resume-draft/pdf", json={"model": "haiku"}, headers=headers) + with pytest.raises(BaseException) as exc_info: # noqa: B017, PT011 + client.post("/api/agent/resume-draft/pdf", json={"model": "haiku"}, headers=headers) + assert "PDF 生成失敗" in repr(exc_info.value)🤖 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/tests/test_resume_draft_api.py` around lines 197 - 202, The test in test_resume_draft_api currently uses a broad BaseException match, which is too permissive even with the ExceptionGroup compatibility concern. Update the resume-draft PDF request assertion to capture the raised exception from client.post and verify it matches the expected PDF-generation failure by checking the exception message (and, if needed, unwrapping an ExceptionGroup) while keeping the existing AgentUsageLog verification. Keep the assertion in the same test function and preserve the environment-agnostic handling around pytest.raises and the /api/agent/resume-draft/pdf call.
🤖 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/tests/test_resume_draft_api.py`:
- Around line 197-202: The test in test_resume_draft_api currently uses a broad
BaseException match, which is too permissive even with the ExceptionGroup
compatibility concern. Update the resume-draft PDF request assertion to capture
the raised exception from client.post and verify it matches the expected
PDF-generation failure by checking the exception message (and, if needed,
unwrapping an ExceptionGroup) while keeping the existing AgentUsageLog
verification. Keep the assertion in the same test function and preserve the
environment-agnostic handling around pytest.raises and the
/api/agent/resume-draft/pdf call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a6440641-19e9-441c-94dc-272611fe96b9
📒 Files selected for processing (1)
backend/tests/test_resume_draft_api.py
BaseException 捕捉のみだと無関係な例外でも pass しうるため、捕捉した
例外を平坦化（ExceptionGroup を再帰展開）し、_fail_pdf が投げた
RuntimeError("PDF 生成失敗") であることまで検証する。環境非依存の
ExceptionGroup ラップ吸収は維持。
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
resumesテーブルの 1 ユーザー1件制約と衝突しない設計）変更内容
services/agent/resume_draft/（context / mapper / output_schema / draft_service）+POST /api/agent/resume-draft/pdf（rate limit 5/min、課金配線は既存 chat と同一契約）github_link_cache.result.reposにリポジトリ単位サマリ（description/created_at/pushed_at）を永続化（Optional・後方互換、旧形式は 409 で再連携を促す）useResumeDraftPdfフック + GitHub 連携ダッシュボードに生成ボタン・PDF プレビューを追加Test plan
make ci（lint + typecheck + test + build-web）greenmake lint-adr-indexOK🤖 Generated with Claude Code
Summary by CodeRabbit