feat: report v1 Claude API 실구현 및 프롬프트 품질 검증 (#11)#15
Conversation
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 40 minutes and 24 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the 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 have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughLLMClient 의존성 주입, 프롬프트 템플릿 추가, v1_baseline의 LLM 호출·JSON 파싱·검증·1회 재시도 로직을 도입해 mini/branding/narrative/strengths+interview/highlights 5개 엔드포인트를 실제 생성 흐름으로 교체하고 평가 스크립트·결과·테스트 모킹을 추가했습니다. Changes리포트 v1_baseline 실구현
Sequence DiagramsequenceDiagram
participant Client
participant APIHandler
participant ReportService
participant LLMClient
Client->>APIHandler: POST /v1/reports/{endpoint} (AiReportRequest)
APIHandler->>APIHandler: get_llm_client -> llm from Request.state
APIHandler->>ReportService: run_{endpoint}(req, llm, model)
ReportService->>LLMClient: create_message(prompt)
LLMClient-->>ReportService: response (text) or raise LLMError
ReportService->>ReportService: parse/validate JSON, maybe retry once
ReportService-->>APIHandler: Ai*Response or raise ReportValidationError
APIHandler->>APIHandler: map exceptions -> HTTPException
APIHandler-->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
experiments/report/eval_v1_baseline.py (1)
1-440:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win🧹 CI가 실패한 포맷 이슈를 먼저 정리해 주세요.
현재 이 파일은
ruff format --check에서 실패 중입니다. PR 병합 전uv run ruff format experiments/report/eval_v1_baseline.py로 포맷을 맞추는 게 안전합니다.🤖 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 `@experiments/report/eval_v1_baseline.py` around lines 1 - 440, The file fails ruff formatting; run the formatter and commit the changes to satisfy CI: run `ruff format` on this module (or your repo's formatter command) to auto-fix style issues, then review the diff to ensure no semantic changes to functions like _LLMTracker, _run_endpoint, _eval_one, _print_results and main; stage and commit the formatted file so `ruff format --check` passes in CI.
🧹 Nitpick comments (2)
tests/test_report.py (1)
214-241: ⚡ Quick win재시도/실패 경로 검증 테스트를 1개 추가해 주세요 🧪
지금 변경은 정상 응답(200)만 검증해서, 이번 PR 핵심인 corrective 1회 재시도와 실패 매핑 회귀를 못 잡습니다.
make_mock_llm가 순차 응답을 지원하니 1차 malformed JSON → 2차 valid JSON 케이스(성공) 또는 2회 모두 invalid 케이스(실패 매핑) 중 최소 1개를 추가하는 게 좋아요.
As per coding guidelines,tests/**: 해피 패스뿐 아니라 경계값·실패 케이스도 커버하는지 확인.app/services/report/_compute.py (1)
24-25: ⚡ Quick win_sorted_tag_items의 top_n 경계값(≤0) 처리 보강 권장
⚠️
top_n=-1같은 경우 PythonCounter.most_common(-1)은 빈 리스트([])를 반환하지만, 현재 구현은sorted(... )[:top_n]이라[:-1]처럼 “마지막 1개 제외” 형태로 결과가 달라질 수 있습니다. 다만 현재top_detail_tags/top_detail_tag_stats호출부는top_n을 넘기지 않아 기본값(3)만 사용되고 있어(현재 경로에서는) 실질 영향은 없습니다. 그래도 시그니처가 열려 있으니 향후 오용 방지를 위해 가드 추가가 안전합니다.🔧 제안 수정
def _sorted_tag_items(counter: Counter[str], top_n: int) -> list[tuple[str, int]]: + if top_n <= 0: + return [] return sorted(counter.items(), key=lambda x: (-x[1], x[0]))[:top_n]🤖 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 `@app/services/report/_compute.py` around lines 24 - 25, _sorted_tag_items currently slices with [:top_n] which makes negative top_n behave like Python negative slicing; update _sorted_tag_items to guard against non-positive top_n by returning an empty list when top_n <= 0 (or coerce top_n to int and use max(0, top_n)); locate the function _sorted_tag_items and add the early-return/guard so callers passing top_n <= 0 get [] (alternatively use Counter.most_common(top_n) but still guard non-positive values).
🤖 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 `@app/api/v1/report.py`:
- Around line 68-73: The handler currently exposes internal exception text by
placing str(exc) into the HTTPException detail (see ReportValidationError catch
where logger.warning and HTTPException are used); change this so the logger
retains the full exc details (keep logger.warning(...,
extra={"stage":"validation","reason": str(exc)})) but replace the HTTPException
detail message with a fixed, user-friendly message (e.g.
{"code":"REPORT_GENERATION_FAILED","message":"Invalid report input"}), and apply
the same change for the other ReportValidationError handlers referenced (lines
around the other HTTPException creations) so responses do not leak internal
validation text.
In `@app/prompts/report/narrative.md`:
- Line 19: Replace the hard-coded "20개 기록" phrase in the prompt string "20개 기록을
**하나의 성장 흐름**으로 연결한 3~5문장." with a dynamic placeholder so the instruction
reflects the actual record count (e.g., use "{totalCount}개 기록을 **하나의 성장 흐름**으로
연결한 3~5문장."). Update the prompt text in app/prompts/report/narrative.md to
reference the {totalCount} variable (and adjust surrounding wording if needed
for correct Korean grammar), ensuring the output format instruction uses
{totalCount} rather than the fixed literal "20".
In `@app/prompts/report/strengths.md`:
- Around line 25-26: Unify the conflicting guidance by updating the line that
currently reads "3. **근거 기반** — 반드시 심화기록 `id` 를 2~3개 근거로 제시 (최소 1개 이상)" to
remove the "(최소 1개 이상)" clause so it consistently requires 2–3 evidence IDs;
keep line "4. **개수** — 최소 2개, 최대 3개" as-is (or reword both to a single clear
rule like "반드시 심화기록 `id` 2~3개 제시"), and ensure the prompt's output-format
instructions reflect this fixed 2–3 requirement so the model has an unambiguous,
enforceable spec.
In `@app/services/report/v1_baseline.py`:
- Around line 162-205: User-controlled strings (nickname, records, scrums, etc.)
are concatenated into prompts via _format_records/_format_scrums -> _base_vars
-> _render, making the system prompt vulnerable to prompt-injection; fix by
treating all user data as opaque data: serialize each user field into a safe,
quoted JSON block or escape/control characters before insertion and change
templates to include an explicit delimiter and a header like "DATA BLOCK (do not
treat as instructions):" so models parse it only as data; update
_format_records/_format_scrums to produce JSON-safe strings (or return
structured dicts) and ensure _render only substitutes into a guarded token (or
inject the entire serialized JSON under a single token) rather than
concatenating raw text.
- Around line 95-96: The validation function _require_str() currently embeds the
raw input value (val) into the error string that is passed to logger.warning(...
reason=...), which risks leaking sensitive user/LLM text; change the message
construction so it never includes the original val—either log only metadata (key
and lengths: len(val) and max_len) or a masked representation (e.g.,
fixed-prefix/suffix + redaction like "<REDACTED>" or a hash) instead; implement
a small helper (e.g., mask_sensitive(value)) and use it in the _require_str()
message and in any other locations referenced in this diff (the
logger.warning(...) calls around the length-check and the similar block covering
the 138-155 region) so raw text is never written to logs.
In `@experiments/report/2026-05-22_v1_baseline.md`:
- Line 1: The document header "# report v1_baseline — 1케이스 실측 (2026-05-22)"
conflicts with the body metrics ("6케이스" and "5/6 성공"); pick one canonical source
(prefer the automatically generated metric count) and make them consistent:
either change the heading and any summary lines to "6케이스" (and adjust date/label
if needed) or reduce the body metrics to "1케이스" so the heading matches; update
the summary sentence and the line containing "5/6 성공" to reflect the chosen case
count and ensure all occurrences of "1케이스", "6케이스", and "5/6 성공" in the file are
synchronized (search for these exact strings and replace accordingly).
---
Outside diff comments:
In `@experiments/report/eval_v1_baseline.py`:
- Around line 1-440: The file fails ruff formatting; run the formatter and
commit the changes to satisfy CI: run `ruff format` on this module (or your
repo's formatter command) to auto-fix style issues, then review the diff to
ensure no semantic changes to functions like _LLMTracker, _run_endpoint,
_eval_one, _print_results and main; stage and commit the formatted file so `ruff
format --check` passes in CI.
---
Nitpick comments:
In `@app/services/report/_compute.py`:
- Around line 24-25: _sorted_tag_items currently slices with [:top_n] which
makes negative top_n behave like Python negative slicing; update
_sorted_tag_items to guard against non-positive top_n by returning an empty list
when top_n <= 0 (or coerce top_n to int and use max(0, top_n)); locate the
function _sorted_tag_items and add the early-return/guard so callers passing
top_n <= 0 get [] (alternatively use Counter.most_common(top_n) but still guard
non-positive values).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fa045350-d289-497b-8499-a00476f0fde4
📒 Files selected for processing (20)
app/api/v1/report.pyapp/prompts/report/branding.mdapp/prompts/report/highlights.mdapp/prompts/report/interview.mdapp/prompts/report/mini.mdapp/prompts/report/narrative.mdapp/prompts/report/strengths.mdapp/services/report/__init__.pyapp/services/report/_compute.pyapp/services/report/exceptions.pyapp/services/report/v1_baseline.pyexperiments/report/2026-05-22_v1_baseline.mdexperiments/report/_template.mdexperiments/report/eval_v1_baseline.pyexperiments/report/fixtures/eval_set_v1.jsonlexperiments/report/results/.gitkeepexperiments/report/results/2026-05-22T182803Z_v1_baseline.jsonexperiments/report/results/2026-05-22T200323Z_v1_baseline.jsontests/conftest.pytests/test_report.py
…-interview/highlights)
af24769 to
7bc629c
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/test_report.py (1)
213-246: ⚡ Quick winv1 핵심 로직(검증 실패/재시도/실패 매핑) 테스트가 빠져 있어요 🧪
현재 Line 213-246은 해피패스만 검증합니다. 이번 변경의 핵심인 “1차 파싱 실패 → corrective 재시도”와 “재시도도 실패 시 에러 응답” 경로를 최소 1개씩 추가하는 게 안전합니다.
예시 테스트 추가안
class TestV1Reports: @@ def test_mini_returns_200(self, client: TestClient, token: str) -> None: llm = make_mock_llm(_MINI_RESP) @@ assert "topDetailTags" in data + + def test_mini_retries_once_and_succeeds(self, client: TestClient, token: str) -> None: + llm = make_mock_llm("not-json", _MINI_RESP) + _app.dependency_overrides[get_llm_client] = lambda: llm + try: + r = client.post( + "/v1/reports/mini", + json=_BASE_BODY, + headers={"X-Internal-Token": token}, + ) + finally: + _app.dependency_overrides.pop(get_llm_client, None) + assert r.status_code == 200 + assert llm.create_message.await_count == 2 + + def test_mini_retry_exhausted_returns_422(self, client: TestClient, token: str) -> None: + llm = make_mock_llm("not-json", "still-not-json") + _app.dependency_overrides[get_llm_client] = lambda: llm + try: + r = client.post( + "/v1/reports/mini", + json=_BASE_BODY, + headers={"X-Internal-Token": token}, + ) + finally: + _app.dependency_overrides.pop(get_llm_client, None) + assert r.status_code == 422As per coding guidelines
tests/**: "해피 패스뿐 아니라 경계값·실패 케이스도 커버하는지 확인".🤖 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 `@tests/test_report.py` around lines 213 - 246, Add tests alongside test_mini_returns_200 and test_strengths_and_interview_returns_200 that simulate a first LLM parse failure followed by a successful corrective retry and another test where both initial parse and retry fail; use make_mock_llm to feed responses (e.g., first response = malformed/invalid parse, second = valid _MINI_RESP or _STRENGTHS_RESP/_INTERVIEW_RESP for the retry, and for the failure case both responses invalid), override _app.dependency_overrides[get_llm_client] to return that mock llm, POST the same endpoints with _BASE_BODY and token, and assert that the successful-retry test returns 200 with expected fields while the double-failure test returns the appropriate error status/code and error mapping in the JSON body (use the existing test helpers and assertions patterns found in test_mini_returns_200 and test_strengths_and_interview_returns_200).experiments/report/eval_v1_baseline.py (1)
370-372: ⚡ Quick win템플릿 필수 메트릭 블록 검증은
assert대신 명시적 예외가 더 안전해요⚠️
assert new_block_match는python -O/PYTHONOPTIMIZE환경에선 검증이 비활성화될 수 있어, 메트릭 블록 누락 시 의도한 실패 지점이 보장되지 않습니다. (다만 현재 저장소 내에서python -O/PYTHONOPTIMIZE를 직접 쓰는 흔적은 확인되지 않았습니다.)
명시적으로RuntimeError를 던지면 어떤 실행 모드에서도 동일하게 “누락 원인”을 확실히 전달할 수 있어요.🔧 제안 수정안
new_block_match = _METRIC_BLOCK_RE.search(rendered) - assert new_block_match + if new_block_match is None: + raise RuntimeError( + "템플릿에 메트릭 블록이 없습니다: " + "<!-- AUTO:START METRICS --> ... <!-- AUTO:END METRICS -->" + ) new_block = new_block_match.group(0)🤖 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 `@experiments/report/eval_v1_baseline.py` around lines 370 - 372, The current check uses a bare assert (assert new_block_match) which can be skipped under Python optimizations; change it to an explicit runtime check that raises a clear exception (e.g., raise RuntimeError or ValueError) when _METRIC_BLOCK_RE.search(rendered) returns None; locate the lines that assign new_block_match from _METRIC_BLOCK_RE.search(rendered) and replace the assert with a conditional that raises an error describing that the metric block is missing and include context (e.g., the rendered string or a snippet) before extracting new_block via new_block_match.group(0).
🤖 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 `@app/api/v1/report.py`:
- Around line 48-57: The _handle_llm_exc function currently collapses all
non-rate-limit LLM exceptions into a 502 which misrepresents client vs upstream
errors; update _handle_llm_exc to explicitly handle LLMBadRequestError by
raising HTTP 400 (bad request) and LLMAuthError by raising HTTP 401
(authentication) so clients don’t treat those as transient upstream failures,
keep the existing LLMRateLimitedError => 503 branch, and fall back to HTTP 502
for any other unhandled Exception types.
In `@app/prompts/report/strengths.md`:
- Around line 26-27: The "근거 기반" rule requires 2–3 evidenceIds but the provided
example's second strength uses only one; update the example to conform by adding
an additional evidence id (so each strength shows 2–3 evidenceIds) and ensure
the wording still matches the output format rules; specifically edit the example
strengths in strengths.md to include two evidenceIds for the second strength
(reference symbols: "3. **근거 기반**", "evidenceIds", and the example strength
entries) so the rule and example are consistent.
In `@app/services/report/v1_baseline.py`:
- Line 225: Remove direct user identifiers from logs: replace the logger.info
calls that pass extra={"nickname": req.nickname} (the
logger.info("report.mini.done", ...) occurrences) so they no longer include
req.nickname; either omit the extra field entirely or replace it with a
non-identifying trace token (e.g., request_id or a hashed/anonymized user key)
and ensure all occurrences (the logger.info calls at the "report.mini.done"
points and the other logger.info lines referencing req.nickname) are updated
consistently.
In `@experiments/report/eval_v1_baseline.py`:
- Around line 376-387: The calls to out_path.relative_to(Path.cwd()) (seen
around usages with out_path and alt after computing rendered/updated/created
paths) can raise ValueError when the CWD isn't a parent; replace them with a
small safe helper (e.g., _safe_rel_path(p: Path) -> str) that tries
p.relative_to(Path.cwd()) in a try/except and falls back to
os.path.relpath(str(p), str(Path.cwd())) or str(p) on failure, then use that
helper for all return messages (created →, updated (인사이트 보존) →, conflict. alt 생성
→) so the script won't crash during post-eval save/output steps.
---
Nitpick comments:
In `@experiments/report/eval_v1_baseline.py`:
- Around line 370-372: The current check uses a bare assert (assert
new_block_match) which can be skipped under Python optimizations; change it to
an explicit runtime check that raises a clear exception (e.g., raise
RuntimeError or ValueError) when _METRIC_BLOCK_RE.search(rendered) returns None;
locate the lines that assign new_block_match from
_METRIC_BLOCK_RE.search(rendered) and replace the assert with a conditional that
raises an error describing that the metric block is missing and include context
(e.g., the rendered string or a snippet) before extracting new_block via
new_block_match.group(0).
In `@tests/test_report.py`:
- Around line 213-246: Add tests alongside test_mini_returns_200 and
test_strengths_and_interview_returns_200 that simulate a first LLM parse failure
followed by a successful corrective retry and another test where both initial
parse and retry fail; use make_mock_llm to feed responses (e.g., first response
= malformed/invalid parse, second = valid _MINI_RESP or
_STRENGTHS_RESP/_INTERVIEW_RESP for the retry, and for the failure case both
responses invalid), override _app.dependency_overrides[get_llm_client] to return
that mock llm, POST the same endpoints with _BASE_BODY and token, and assert
that the successful-retry test returns 200 with expected fields while the
double-failure test returns the appropriate error status/code and error mapping
in the JSON body (use the existing test helpers and assertions patterns found in
test_mini_returns_200 and test_strengths_and_interview_returns_200).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d12b1395-6130-4024-9f6f-cbf778e21979
📒 Files selected for processing (25)
app/api/v1/report.pyapp/prompts/report/branding.mdapp/prompts/report/highlights.mdapp/prompts/report/interview.mdapp/prompts/report/mini.mdapp/prompts/report/narrative.mdapp/prompts/report/strengths.mdapp/schemas/common.pyapp/services/report/__init__.pyapp/services/report/_compute.pyapp/services/report/exceptions.pyapp/services/report/v1_baseline.pyexperiments/report/2026-05-22_v1_baseline.mdexperiments/report/2026-05-23_v1_baseline.mdexperiments/report/_template.mdexperiments/report/eval_v1_baseline.pyexperiments/report/fixtures/eval_set_v1.jsonlexperiments/report/results/.gitkeepexperiments/report/results/2026-05-22T182803Z_v1_baseline.jsonexperiments/report/results/2026-05-22T200323Z_v1_baseline.jsonexperiments/report/results/2026-05-23T052223Z_v1_baseline.jsonexperiments/report/results/2026-05-23T053815Z_v1_baseline.jsonexperiments/tagging/eval_v3_tiebreak.pytests/conftest.pytests/test_report.py
✅ Files skipped from review due to trivial changes (7)
- experiments/report/_template.md
- experiments/tagging/eval_v3_tiebreak.py
- experiments/report/results/2026-05-23T053815Z_v1_baseline.json
- experiments/report/2026-05-23_v1_baseline.md
- experiments/report/2026-05-22_v1_baseline.md
- experiments/report/results/2026-05-22T182803Z_v1_baseline.json
- experiments/report/results/2026-05-22T200323Z_v1_baseline.json
요약
변경 사항
도메인 / 서비스 레이어
ReportValidationError등 report 도메인 예외 클래스 추가/v1/reports/*라우터에LLMClient의존성 주입 및 예외 매핑v1_baseline.py: mini / branding / narrative / strengths-and-interview / highlights 5개 엔드포인트 Claude API 구현_call_with_retry)max_tokenseval 실측값 기준으로 재산정_compute.py: 순수 Python 집계 헬퍼 분리topDetailTags/topDetailTagStats동점 시 가나다순 정렬로 결정적 동작 보장missing_high_tags: 가장 약한 역량 카테고리에서 직군 High 태그 중 미기록 태그 산출프롬프트 개선 (
app/prompts/report/)narrative.md: 역량 카테고리 코드값 출력 금지, 임의 태그 생성 금지strengths.md: title 명사구 규칙 추가, 15자 엄수mini.md: nextFocusPoint를 직군별 미기록 고빈도 태그 기반으로 재작성 (직군 미스매치 해소), 태그명 직접 노출 금지interview.md: 질문에서 내부 id 번호 직접 참조 금지, 파고들 포인트 명확할 때만 3개 출력실험 (
experiments/report/)eval_v1_baseline.py) 및 픽스처 추가results/2026-05-22T200323Z_v1_baseline.json)테스트
uv run ruff check . && uv run mypy apppython -m experiments.report.eval_v1_baseline→ 6/6 포맷 성공 확인스크린샷/로그 (선택)
experiments/report/2026-05-22_v1_baseline.md참고롤백/리스크
strengths title15자 제약이 간헐적으로 초과될 수 있음 (corrective retry로 대응 중이나 모니터링 필요)LLMClient미주입 상태로 되돌리면 기존 stub 동작 유지관련 이슈
Closes #11
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation