feat: AI 가정통신문 전체 분석 응답 저장 연동#64
Conversation
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 10 minutes and 37 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: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAI 뉴스레터 분석 파이프라인이 기존 ChangesAI Newsletter Analyze API Integration
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ 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
🤖 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
`@src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterAiAnalyzer.java`:
- Around line 43-47: Remove the redundant null-check for items in
NewsletterAiAnalyzer: since AiNewsletterClient.analyze(...) already calls
normalized() and guarantees AnalysisResponse.items() is non-null, replace the
conditional assignment of items with a direct use of analysisResponse.items()
(keep the variable name items if needed) and remove the ternary null fallback;
update any code that relied on the previous fallback accordingly so
NewsletterAiAnalyzer relies on the normalized contract from
AiNewsletterClient.analyze().
In
`@src/test/java/com/gachi/be/domain/newsletter/pipeline/AiNewsletterClientTest.java`:
- Around line 20-84: Add three negative tests to AiNewsletterClientTest that
exercise AiNewsletterClient.analyze's error paths: (1) simulate the AI server
returning a 500 response and assert analyze throws ExternalApiException with
ErrorCode.EXTERNAL_API_ERROR (or equivalent), (2) simulate a delayed response
that exceeds properties.connect/read timeout to assert analyze throws
ExternalApiException for timeouts, and (3) return a 200 status with malformed
JSON and assert analyze throws ExternalApiException for parse errors; reuse the
existing HttpServer setup and AiServerProperties to point to the test server,
and tear down server/executor in finally blocks as in the existing
analyzeCallsAnalyzeEndpointAndParsesTitleSummaryItems test.
In
`@src/test/java/com/gachi/be/domain/newsletter/pipeline/NewsletterAiAnalyzerTest.java`:
- Around line 81-103: Add two unit tests in NewsletterAiAnalyzerTest to cover
partial-fallback scenarios: one where aiNewsletterClient.analyze returns a
non-blank title and blank summary, and one where it returns a blank title and
non-blank summary. Use the same setup as
analyzeFallsBackWhenAiTitleSummaryAreBlank (mock newsletterRepository.findById
and aiNewsletterClient.analyze) but return AnalysisResponse with only title or
only summary populated; then call newsletterAiAnalyzer.analyze and assert
AiAnalysisResult.title()/summary() fall back to the original inputs when the AI
value is blank while accepting the AI-provided value when present, and verify
checklistRepository.saveAll is never called. Reference symbols:
NewsletterAiAnalyzerTest, newsletterAiAnalyzer.analyze,
aiNewsletterClient.analyze, AnalysisResponse, AiAnalysisResult,
checklistRepository.saveAll.
- Line 95: In NewsletterAiAnalyzerTest update the mocked AnalysisResponse
returned in the .thenReturn(...) call so title and summary use the same
empty-case for consistency (e.g., both "" or both " "), or split into two tests
verifying each empty-case separately; target the mocked return construction of
new AnalysisResponse(...) in the test to ensure the fallback/normalization
behavior of NewsletterAiAnalyzer is validated against a single, consistent
empty-value scenario (or add a second test that uses the alternate empty-value).
- Around line 69-70: Remove the local unchecked captor and the
`@SuppressWarnings`; instead declare a generic, type-safe captor as a field using
Mockito's `@Captor`: add a field like "private `@Captor`
ArgumentCaptor<List<Checklist>> captor" in NewsletterAiAnalyzerTest and ensure
mocks are initialized in the test setup (e.g.,
MockitoAnnotations.openMocks(this) or `@ExtendWith`(MockitoExtension.class)) so
the captor is injected and you avoid raw types and unchecked warnings when
capturing arguments.
- Around line 73-78: The test in NewsletterAiAnalyzerTest currently asserts only
content/detail for the saved Checklist; extend it to verify the time-related
fields from the source ExtractedItem (datetime, timezone, dateStatus,
confidence) are correctly mapped and stored. After capturing savedItems
(captor.getValue()), add assertions on savedItems.get(0) to compare its
datetime, timezone, dateStatus and confidence properties against the expected
values used when constructing the ExtractedItem in the test setup so the mapping
from ExtractedItem -> Checklist is validated.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: b8f7cfdd-7f8c-4298-b277-423ebd4133e3
📒 Files selected for processing (5)
docs/newsletter-ai-analyze-integration.mdsrc/main/java/com/gachi/be/domain/newsletter/pipeline/AiNewsletterClient.javasrc/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterAiAnalyzer.javasrc/test/java/com/gachi/be/domain/newsletter/pipeline/AiNewsletterClientTest.javasrc/test/java/com/gachi/be/domain/newsletter/pipeline/NewsletterAiAnalyzerTest.java
Hminkyung
left a comment
There was a problem hiding this comment.
AI 호출경로 변경한 부분부터 자세하게 작성한것까지 전부 확인 완료했씁니당!!
고생하셨써요~~~
📌 작업 요약
/ai/newsletters/extract-items에서/ai/newsletters/analyze로 전환title,summary,items를 BE 저장 흐름에 반영items는 기존 checklist 저장 로직에 연결title,summary가 비어 있을 경우 기존 BE fallback 로직을 사용하도록 처리/analyze연동 저장 정책 문서 추가🌿 브랜치 정보
feat/#63-newsletter-analyze-savedevelop(기본)✅ 체크리스트
feat/refac/hotfix/chore/design/bugfix)feat/fix/refactor/docs/style/chore)🧪 테스트 결과
Summary by CodeRabbit
릴리스 노트
New Features
Documentation
Tests