Skip to content

Comments

[SRLT-126] PDF AI 리포트 관련 에러 수정 및 구조 리팩토링#82

Open
2ghrms wants to merge 13 commits intodevelopfrom
SRLT-126-pdf-ai-리포트-고도화-및-이벤트-처리

Hidden character warning

The head ref may contain hidden characters: "SRLT-126-pdf-ai-\ub9ac\ud3ec\ud2b8-\uace0\ub3c4\ud654-\ubc0f-\uc774\ubca4\ud2b8-\ucc98\ub9ac"
Open

[SRLT-126] PDF AI 리포트 관련 에러 수정 및 구조 리팩토링#82
2ghrms wants to merge 13 commits intodevelopfrom
SRLT-126-pdf-ai-리포트-고도화-및-이벤트-처리

Conversation

@2ghrms
Copy link
Member

@2ghrms 2ghrms commented Feb 17, 2026

🚀 Why - 해결하려는 문제가 무엇인가요?

  • AI 리포트 생성 에러: 리포트 생성 시 JSON 응답 파싱 실패가 발생하고, 컨텍스트가 없을 때 정상 응답이 나오지 않는 문제

  • 전문가 신청 시 PDF 전송: PDF로 사업계획서를 만든 경우, 전문가 신청 시 해당 PDF를 이메일 등으로 전송해야 하는데 기존에는 업로드 파일만 사용하던 문제

  • 인프라 의존성: PDF/이미지 관련 공통 로직이 특정 어댑터에만 있어 재사용·유지보수가 어려운 문제

  • 테스트 구조: 테스트 코드 위치가 메인 소스 구조와 맞지 않아 일관성이 떨어지던 문제

  • 유틸리티 클래스 의존성 문제: AiReportResponseParser / BusinessPlanContentExtractor 등

-> AI 리포트 고도화 및 이벤트 처리 개선 작업 과정에서 위 이슈들이 함께 정리·개선 필요.

✅ What - 무엇이 변경됐나요?

  • AI 리포트: JSON 응답 파싱 보완 및 재처리 로직 추가, 컨텍스트가 없어도 동작하도록 커스텀 프롬프트 템플릿 적용

  • 전문가 신청: businessPlan에 pdfUrl이 있으면 해당 URL에서 PDF를 다운로드해 전송하고, 없을 때만 업로드된 파일을 사용하도록 로직 수정 ExpertApplicationCommandService.java

  • 인프라: PDF·이미지 공통 로직을 어댑터에서 분리하고, 사용 방법을 정리한 개발 가이드 작성

  • 테스트: 메인 코드 패키지 구조에 맞게 테스트 코드 위치 및 구조 정리

  • 유틸리티 클래스 의존성 문제: AiReportResponseParser의 단순 컨버트 로직은 AiReportResult record에 정적 메소드로 위임, 유틸리티 클래스의 계층 이동

🛠️ How - 어떻게 해결했나요?

AI 리포트:
프롬프트 출력 스펙의 배열이 문자열로 되어있던 것을 배열 JSON 형태로 수정
JSON 파싱 실패 시 재시도/재처리 로직으로 안정성 확보
컨텍스트 부재 시에도 응답이 나오도록 Spring AI의 QuestionAnswerAdvisor의 커스텀 프롬프트 템플릿 사용

전문가 신청 PDF:
BusinessPlan.isPdfBased()(즉 pdfUrl 존재 여부)로 분기
pdfUrl 있음 → PdfDownloadPort로 URL에서 다운로드한 바이트 사용
pdfUrl 없음 → 기존처럼 MultipartFile 검증 후 사용

인프라 분리:
PDF/이미지 처리 포트·클라이언트를 공통 레이어(adapter/shared/infrastructure/)로 분리하고, 어댑터는 해당 포트만 사용하도록 변경
분리된 구조와 사용 규칙을 개발 가이드에 문서화

테스트:
메인 소스의 패키지/디렉터리 구조에 맞춰 테스트 패키지 및 클래스 위치 재정렬

유틸리티 클래스:
AiReportResponseParser의 단순 컨버트 로직은 AiReportResult record에 정적 메소드로 위임
AiReportResponseParser를 applciation/aireport/util -> adapter/aireport/report/parser (adapter 계층에서 사용하기 때문)
BusinessPlanContentExtractor를 application/businessplan/util -> applciation/aireport/util (aireport 에서만 사용하기 때문)

🖼️ Attachment

  • 화면 이미지, 결과 캡처 등 첨부

💬 기타 코멘트

  • 리뷰어에게 전하고 싶은 말, 테스트 방법, 주의할 점 등

Summary by CodeRabbit

  • 신규 기능

    • 비즈니스플랜 이미지 업로드 엔드포인트(/v1/business-plans/images/...) 및 관련 API 문서 추가
    • PDF URL 전용 검증 어노테이션(@ValidPdfUrl) 도입
  • 개선 사항

    • AI 섹션 채점에 재시도(지수 백오프 포함) 로직 추가로 안정성 향상
    • PDF 다운로드/처리 흐름 개선 및 더 안전한 폴백 처리
    • 이미지 파일명 검증 적용
  • 문서

    • 어댑터 공유 규칙(개발 가이드) 업데이트

@2ghrms 2ghrms self-assigned this Feb 17, 2026
@2ghrms 2ghrms added the 🧵 REFACTOR 코드 리팩토링 label Feb 17, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 17, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'tools'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
📝 Walkthrough

Walkthrough

📝 Walkthrough

여러 도메인별 포트 분리(새 PdfDownloadPort/PresignedUrlProviderPort 추가 및 재배치), PDF 다운로드 에러 타입/예외 추가, PDF URL/이미지 파일명 검증기 도입, BusinessPlan 이미지 API로의 리네임/경로 변경, OCR 및 PDF 다운로드 호출부 교체, AI 섹션 채점 에이전트에 재시도(백오프) 로직 추가 등 구조·인터페이스와 검증/오류 처리를 중심으로 한 광범위한 리팩터링 및 기능 보강이 적용되었습니다.

Changes

Cohort / File(s) Summary
PDF download port & client
src/main/java/starlight/application/aireport/required/PdfDownloadPort.java, src/main/java/starlight/application/expertApplication/required/PdfDownloadPort.java, src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java, src/test/java/.../PdfDownloadClientTest.java, src/test/java/.../PdfDownloadClientIntegrationTest.java
새로운 PdfDownloadPort 인터페이스(aireport/expertApplication) 추가 및 PdfDownloadClient가 해당 포트를 구현하도록 변경. 메서드명 downloadPdfFromUrldownloadFromUrl, 예외 타입(Ocr→PdfDownload) 및 테스트 업데이트 포함.
OCR adapter wiring
src/main/java/starlight/adapter/aireport/infrastructure/ocr/ClovaOcrProvider.java, src/test/java/.../ClovaOcrProviderTest.java
ClovaOcrProvider가 PdfDownloadClient 대신 PdfDownloadPort 주입·사용으로 교체(메서드 호출도 downloadFromUrl로 변경). 테스트 목/검증 경로도 동일하게 갱신.
AI grading agent (retry flow)
src/main/java/starlight/adapter/aireport/report/agent/impl/SpringAiSectionGradeAgent.java
채점 호출을 최대 재시도(MAX_RETRIES)하도록 변경. 프롬프트·채팅 클라이언트·어드바이저 초기화는 루프 외부로 이동, 재시도마다 지수적 백오프와 실패 메시지 누적 로직 추가.
Advisor prompt template
src/main/java/starlight/adapter/aireport/report/provider/SpringAiAdvisorProvider.java
QA 프롬프트 템플릿을 설정으로 주입하도록 추가(스태틱 템플릿 렌더러와 PromptTemplate 생성 및 Advisor 빌드 시 사용).
Presigned URL ports & providers
src/main/java/starlight/application/businessplan/required/PresignedUrlProviderPort.java, src/main/java/starlight/application/backoffice/image/required/PresignedUrlProviderPort.java, src/main/java/starlight/adapter/shared/infrastructure/storage/NcpPresignedUrlProvider.java, src/main/java/starlight/adapter/backoffice/image/webapi/BackofficeImageController.java, src/test/.../BusinessPlanImageControllerIntegrationTest.java, src/test/.../NcpPresignedUrlProviderUnitTest.java
PresignedUrlProviderPort 패키지 재배치(aireport → businessplan/backoffice) 및 NcpPresignedUrlProvider가 추가 포트(backoffice.image) 구현. 컨트롤러 및 테스트의 의존성/패키지 경로 갱신.
BusinessPlan 이미지 API 리네임
src/main/java/starlight/adapter/businessplan/webapi/BusinessPlanImageController.java, src/main/java/starlight/adapter/businessplan/webapi/swagger/BusinessPlanImageApiDoc.java, src/test/.../BusinessPlanImageControllerIntegrationTest.java
이미지 API가 BusinessPlan 전용으로 리네임 및 경로 변경(/v1/images → /v1/business-plans/images). 컨트롤러 클래스/인터페이스명·매핑·테스트 경로 일괄 변경.
검증 어노테이션 - 이미지/ PDF URL
src/main/java/starlight/adapter/shared/webapi/validation/ValidImageFileName.java, src/main/java/starlight/adapter/shared/webapi/validation/ValidImageFileNameValidator.java, src/main/java/starlight/adapter/shared/webapi/validation/ValidPdfUrl.java, src/main/java/starlight/adapter/shared/webapi/validation/ValidPdfUrlValidator.java, src/main/java/starlight/adapter/aireport/webapi/dto/AiReportCreateWithPdfRequest.java, src/main/java/starlight/adapter/businessplan/webapi/dto/BusinessPlanCreateWithPdfRequest.java, src/main/java/starlight/adapter/backoffice/image/webapi/swagger/BackofficeImageApiDoc.java
이미지 파일명 검증기와 PDF URL 검증기(어노테이션+밸리데이터) 추가/공유 패키지로 이동. DTO 및 컨트롤러 파라미터에 새 어노테이션 적용.
에러 타입 / 예외 추가
src/main/java/starlight/adapter/shared/infrastructure/pdf/exception/PdfDownloadErrorType.java, src/main/java/starlight/adapter/shared/infrastructure/pdf/exception/PdfDownloadException.java, src/main/java/starlight/domain/businessplan/exception/BusinessPlanErrorType.java
PDF 다운로드 전용 ErrorType enum 및 PdfDownloadException 추가. BusinessPlanErrorType에 INVALID_PDF_URL 항목 추가.
AiReport parsing robustness
src/main/java/starlight/application/aireport/util/AiReportResponseParser.java
section gradingListScores 처리 강화: 노드가 배열일 경우 직렬화, 비어있거나 잘못된 JSON에 대한 안전한 폴백("[]") 로직 추가.
Expert application: PDF-based flow
src/main/java/starlight/application/expertApplication/ExpertApplicationCommandService.java
plan이 PDF 기반일 경우 PdfDownloadPort로 파일 바이트를 받아 처리하도록 분기 추가. 이벤트 발행 로직을 파일바이트와 파일명으로 변경(IO 예외 처리 위치 변경).
기타 문서·워크플로우·테스트 패키지 이동
개발가이드.md, .github/workflows/test.yaml, 여러 테스트 패키지 파일들...`
개발 가이드에 어댑터 공유 규칙 추가, CI에 SPRING_PROFILES_ACTIVE=test 추가, 테스트 패키지/네임스페이스 조정 등 부수 변경.

Sequence Diagram(s)

emermaid
mermaid
sequenceDiagram
participant Caller as Client
participant Agent as SpringAiSectionGradeAgent
participant Chat as ChatClient
participant Advisor as QuestionAnswerAdvisor
participant CB as CircuitBreaker

Caller->>Agent: gradeSection(request)
Agent->>Agent: prepare prompt, Chat, Advisor (once)
loop attempts (1..MAX_RETRIES)
    Agent->>Chat: sendPrompt(prompt, advisors)
    Chat-->>Agent: response / error
    alt success
        Agent->>CB: recordSuccess()
        Agent-->>Caller: parsedResult
        break
    else failure
        Agent->>Agent: log, wait exponentialBackoff
    end
end
Agent->>CB: recordFailure()
Agent-->>Caller: failureResult(lastFailureMessage)

(Note: diagram shows high-level retry flow between Client, Agent, ChatClient, Advisor, and CircuitBreaker.)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

✨ FEAT, ✅ TEST, :memo: DOCS

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목은 변경의 핵심을 명확히 전달하며, PDF AI 리포트 에러 수정 및 구조 리팩토링이라는 주요 변경사항을 정확히 반영하고 있습니다.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch SRLT-126-pdf-ai-리포트-고도화-및-이벤트-처리

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.

❤️ Share

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

@2ghrms 2ghrms changed the title [SRLT-126] PDF 관련 AI 리포트 고도화 [SRLT-126] PDF 관련 AI 리포트 고도화 (미완) Feb 17, 2026
@github-actions
Copy link

github-actions bot commented Feb 17, 2026

Test Results

246 tests  ±0   246 ✅ ±0   9s ⏱️ -1s
 45 suites ±0     0 💤 ±0 
 45 files   ±0     0 ❌ ±0 

Results for commit 9d43a80. ± Comparison against base commit 31848a8.

♻️ This comment has been updated with latest results.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/starlight/application/expertApplication/ExpertApplicationCommandService.java (1)

50-84: ⚠️ Potential issue | 🟠 Major

PDF 다운로드 시 예외 처리가 try-catch 블록 바깥에 있습니다.

plan.isPdfBased() 분기(Line 56-58)에서 pdfDownloadPort.downloadFromUrl()이 실패하면 OcrException이 그대로 전파됩니다. 이는 Line 70-83의 try-catch 블록 바깥이므로 ExpertApplicationException으로 래핑되지 않습니다.

반면, 비 PDF 경로(Line 61-66)에서는 IOExceptionExpertApplicationException으로 래핑하고 있어 예외 처리가 비대칭적입니다. 또한 OcrExceptionaireport 도메인의 예외 타입이므로, expertApplication 도메인 외부로 누수되는 문제도 있습니다.

🐛 PDF 다운로드 예외를 도메인 예외로 래핑하는 수정 제안
         if (plan.isPdfBased()) {
-            fileBytes = pdfDownloadPort.downloadFromUrl(plan.getPdfUrl());
+            try {
+                fileBytes = pdfDownloadPort.downloadFromUrl(plan.getPdfUrl());
+            } catch (Exception e) {
+                log.error("Failed to download PDF. planId={}, expertId={}", planId, expertId, e);
+                throw new ExpertApplicationException(ExpertApplicationErrorType.FILE_READ_ERROR);
+            }
             filename = generateFilenameForPdfPlan(plan, menteeName);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/application/expertApplication/ExpertApplicationCommandService.java`
around lines 50 - 84, The PDF download path in requestFeedback leaks
OcrException because pdfDownloadPort.downloadFromUrl(...) is called outside the
try-catch that wraps other failures; wrap the PDF download call in the same
exception handling and convert domain-specific exceptions into the
expertApplication domain by catching OcrException (and any checked IO
exceptions) around pdfDownloadPort.downloadFromUrl(...) and rethrowing an
ExpertApplicationException (e.g., ExpertApplicationErrorType.FILE_READ_ERROR or
a new type like PDF_DOWNLOAD_ERROR); ensure plan.isPdfBased() branch mirrors the
non-PDF branch's error handling so all exceptions from pdfDownloadPort and file
reading are caught and translated before escaping requestFeedback.
🧹 Nitpick comments (8)
src/main/java/starlight/application/aireport/util/AiReportResponseParser.java (2)

126-161: parse() 메서드에서 AiReportException이 catch 블록에 의해 다시 래핑됩니다.

Line 146, 155에서 던진 AiReportException이 Line 159의 catch (Exception e) 에 잡혀서 동일한 에러 타입으로 다시 래핑됩니다. 기존 코드이므로 이번 PR 범위는 아니지만, 향후 디버깅 시 원인 추적이 어려울 수 있습니다. AiReportException을 먼저 catch하여 그대로 re-throw하는 것을 고려해 보세요.

♻️ 선택적 개선안
-        } catch (Exception e) {
+        } catch (AiReportException e) {
+            throw e;
+        } catch (Exception e) {
+            log.error("Failed to parse LLM response", e);
             throw new AiReportException(AiReportErrorType.AI_RESPONSE_PARSING_FAILED);
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/application/aireport/util/AiReportResponseParser.java`
around lines 126 - 161, The parse() method in AiReportResponseParser currently
catches all Exceptions and re-wraps even AiReportException instances, obscuring
original causes; update the try/catch so you first catch AiReportException and
re-throw it unchanged, then catch generic Exception to wrap into a new
AiReportException(AiReportErrorType.AI_RESPONSE_PARSING_FAILED); ensure this
change surrounds the logic that calls cleanJsonResponse(),
objectMapper.readTree(), parseFromJsonNode(), and the isDefaultResponse() check
so original AiReportException stack traces are preserved.

460-473: 검증 조건 강화도 적절합니다. 다만, startsWith("[") 전에 trim() 호출 위치를 확인해 주세요.

Line 463에서 gradingListScores.trim().startsWith("[") 로 trim을 하고 있지만, 이후 objectMapper.readTree(gradingListScores)(Line 469)에는 원본(trim 안 된) 문자열을 전달합니다. readTree는 내부적으로 공백을 처리하므로 현재 동작에 문제는 없지만, 일관성을 위해 검증 전에 gradingListScores = gradingListScores.trim()을 한번 해두면 더 명확합니다.

♻️ 선택적 개선안
                    // gradingListScores가 유효한 JSON 문자열인지 검증
                    if (!gradingListScores.equals("[]") && !gradingListScores.isEmpty()) {
+                       gradingListScores = gradingListScores.trim();
                        try {
                            // JSON 배열 형식인지 확인
-                           if (!gradingListScores.trim().startsWith("[")) {
+                           if (!gradingListScores.startsWith("[")) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/application/aireport/util/AiReportResponseParser.java`
around lines 460 - 473, In AiReportResponseParser, ensure gradingListScores is
trimmed once before validation: assign gradingListScores =
gradingListScores.trim() prior to the startsWith check and then use that trimmed
variable for objectMapper.readTree(...) and subsequent logic (preserve the
existing fallback to "[]"); update the block around the startsWith check in the
method where gradingListScores is validated so both the check and the JSON parse
use the same trimmed string.
src/main/java/starlight/adapter/aireport/report/agent/impl/SpringAiSectionGradeAgent.java (1)

60-68: 동일한 temperature/topP 설정으로 재시도 시 동일한 응답을 받을 가능성이 높습니다.

temperature(0.0), topP(0.1) 설정은 거의 결정적(deterministic)인 응답을 유도합니다. 파싱 실패가 LLM 응답 포맷 문제에서 기인한 경우, 동일한 파라미터로 재시도하면 같은 응답을 받아 반복 실패할 가능성이 높습니다.

재시도 시 약간의 temperature 증가(예: 0.0 → 0.2 → 0.4)를 적용하거나, 프롬프트에 재시도 힌트를 추가하는 것을 고려해보세요. 물론 API 호출 자체의 일시적 오류(네트워크, 타임아웃 등)에 대한 재시도에는 현재 방식이 유효합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/aireport/report/agent/impl/SpringAiSectionGradeAgent.java`
around lines 60 - 68, The current call in SpringAiSectionGradeAgent that builds
the LLM request via
chatClient.prompt(...).options(ChatOptions.builder().temperature(0.0).topP(0.1).build()).advisors(...).call()
uses fixed, near-deterministic sampling and will likely return the same
unparseable output on retries; modify the retry logic around this call to vary
sampling parameters across attempts (e.g., iterate temperatures like 0.0 → 0.2 →
0.4 and/or adjust topP) or append a retry hint to the prompt for subsequent
attempts, ensuring each retry invokes chatClient.prompt with the updated
ChatOptions so transient parsing issues can produce different outputs.
src/main/java/starlight/adapter/aireport/report/provider/SpringAiAdvisorProvider.java (1)

22-24: @RequiredArgsConstructor@Value 필드 주입 혼용에 대한 참고

vectorStorefinal로 생성자 주입되고, qaAdvisorTemplate@Value로 필드 주입됩니다. 동작에는 문제 없지만, 테스트 시 ReflectionTestUtils가 필요할 수 있습니다. 일관성을 위해 생성자 주입으로 통일하는 것도 고려해보세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/aireport/report/provider/SpringAiAdvisorProvider.java`
around lines 22 - 24, The code mixes `@RequiredArgsConstructor` constructor
injection (vectorStore) with `@Value` field injection (qaAdvisorTemplate); make
qaAdvisorTemplate constructor-injected to be consistent: change the
qaAdvisorTemplate field to a private final String and remove the `@Value` on the
field, then add an explicit constructor or a constructor parameter annotated
with `@Value`("${prompt.report.qa-advisor.template}") so Spring injects it via the
constructor (update SpringAiAdvisorProvider, qaAdvisorTemplate, vectorStore
references accordingly).
src/main/java/starlight/application/expertApplication/ExpertApplicationCommandService.java (1)

101-108: registerApplicationRecordpublic으로 노출되어 있습니다.

이 메서드는 requestFeedback 내부에서만 호출되는 것으로 보입니다. private로 변경하여 불필요한 API 노출을 줄이는 것을 권장합니다.

♻️ 접근 제어자 변경 제안
-    public void registerApplicationRecord(Long expertId, Long planId) {
+    private void registerApplicationRecord(Long expertId, Long planId) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/application/expertApplication/ExpertApplicationCommandService.java`
around lines 101 - 108, The registerApplicationRecord method is unnecessarily
public; change its access modifier to private so it's only callable from within
this class (it is only used by requestFeedback). Locate the method named
registerApplicationRecord(Long expertId, Long planId) and update its declaration
from public to private, keeping the existing logic that checks
applicationQueryPort.existsByExpertIdAndBusinessPlanId(...), throws
ExpertApplicationException on duplicates, creates ExpertApplication.create(...),
and calls applicationQueryPort.save(...).
src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java (1)

22-24: 생성자 접근 제어자가 package-private입니다 — 의도된 것인지 확인해주세요.

@Component로 등록되어 Spring DI로 주입되므로 동작에는 문제 없지만, 다른 패키지의 테스트에서 직접 생성이 불가합니다. 현재 테스트들이 같은 패키지에 있으므로 괜찮지만, 의도적인 설계인지 확인 부탁드립니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java`
around lines 22 - 24, The PdfDownloadClient constructor is package-private which
prevents direct instantiation from other packages (e.g., external tests); if
this was unintended make the constructor public: change the
PdfDownloadClient(`@Qualifier`("pdfDownloadRestClient") RestClient downloadClient)
constructor to public so other packages can construct the class directly while
still allowing Spring to inject it; if package-private was intentional, add a
short comment on the constructor documenting that it is intentionally non-public
for encapsulation/testing reasons.
src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientIntegrationTest.java (1)

146-160: 대용량 파일 테스트에서 new String(largeBytes) 변환의 인코딩 주의점.

10MB 영(zero) 바이트 배열을 new String(largeBytes)로 변환 후 setBody에 전달하고 있습니다. 바이트→문자열→바이트 라운드트립은 문자 인코딩에 따라 크기가 달라질 수 있습니다. 현재는 hasSize 검증만 하고 있어 실패하진 않겠지만, okhttp3.mockwebserver.MockResponsesetBody(okio.Buffer)를 사용하면 바이너리 데이터를 직접 설정할 수 있어 더 정확한 테스트가 됩니다.

♻️ 바이너리 데이터 직접 설정 제안
         byte[] largeBytes = new byte[10 * 1024 * 1024]; // 10MB
+        okio.Buffer buffer = new okio.Buffer();
+        buffer.write(largeBytes);
         mockWebServer.enqueue(new MockResponse()
                 .setResponseCode(200)
-                .setBody(new String(largeBytes)));
+                .setBody(buffer));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientIntegrationTest.java`
around lines 146 - 160, The test downloadFromUrl_LargeFile_Success in
PdfDownloadClientIntegrationTest currently converts a zero-filled byte[] to
String then sets it as the MockResponse body which can corrupt binary content
via charset roundtrip; instead, set the MockResponse body using an okio.Buffer
containing the raw byte[] (use mockWebServer.enqueue(new
MockResponse().setResponseCode(200).setBody(bufferWithBytes))) so the
pdfDownloadClient.downloadFromUrl call receives exact binary data and the
hasSize assertion remains valid; update references to MockResponse.setBody to
use okio.Buffer and ensure the buffer is written with largeBytes before
enqueueing.
src/test/java/starlight/adapter/businessplan/webapi/BusinessPlanImageControllerIntegrationTest.java (1)

58-83: 주석 처리된 테스트 블록이 남아 있습니다.

getPresignedUrl_Success 테스트가 주석 처리되어 있습니다. 삭제하거나 @Disabled 어노테이션으로 전환하여 의도를 명확히 하는 것을 권장합니다. 주석 처리된 테스트는 코드 히스토리에서 추적이 어렵습니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/starlight/adapter/businessplan/webapi/BusinessPlanImageControllerIntegrationTest.java`
around lines 58 - 83, The commented-out test method getPresignedUrl_Success
should not remain as a large block comment; either delete the entire commented
block or restore the test as a real method and mark it explicitly with `@Disabled`
to indicate intentional skip; if choosing `@Disabled`, uncomment the method, add
the org.junit.jupiter.api.Disabled import and the `@Disabled` annotation above
getPresignedUrl_Success, keeping references to presignedUrlProvider,
PreSignedUrlResponse, mockMvc and createMockAuthDetails intact so the test
compiles but is skipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/main/java/starlight/adapter/aireport/report/agent/impl/SpringAiSectionGradeAgent.java`:
- Around line 58-85: The retry loop in SpringAiSectionGradeAgent (inside the
method that calls chatClient.prompt) currently retries instantly and catches all
Exceptions; update the loop to add a backoff between attempts (e.g., fixed or
exponential backoff with jitter) using MAX_RETRIES and attempt to compute delay,
and ensure you sleep/delay (Thread.sleep or a scheduler) before the next attempt
when a retryable failure occurs; additionally narrow the catch to only retry on
transient exceptions (network/timeouts/rate-limit errors from chatClient) and
rethrow or break on non-retryable exceptions (e.g., configuration/auth errors)
so that only appropriate errors cause retries.
- Around line 86-92: The final error message selection can return a stale
exception because lastException and lastFailureResult may come from different
attempts; change logic to capture the actual last-attempt failure message during
the retry loop (e.g., maintain a single String lastFailureMessage that you
update at the end of each attempt with either the exception message or
lastFailureResult.errorMessage()), and then in the failure path use that
lastFailureMessage when calling SectionGradingResult.failure(getSectionType(),
...), while keeping the existing circuitBreaker.recordFailure(getSectionType()),
log.error("[{}] 채점 최종 실패 ({}회 시도)", getSectionType(), MAX_RETRIES) and
MAX_RETRIES semantics.

In
`@src/main/java/starlight/adapter/aireport/report/provider/SpringAiAdvisorProvider.java`:
- Around line 35-38: The injected qaAdvisorTemplate must use the custom {var}
delimiters because PromptTemplate is built with StTemplateRenderer configured
with startDelimiterToken('{') and endDelimiterToken('}'); locate the
PromptTemplate construction in SpringAiAdvisorProvider and either (A) validate
the qaAdvisorTemplate at startup by attempting a small render/check that it
contains/works with {…} syntax and fail-fast with a clear error, or (B) change
the renderer to the StringTemplate4 defaults if the templates use <var> syntax;
reference the PromptTemplate, StTemplateRenderer and qaAdvisorTemplate symbols
when implementing the validation or renderer change.

In
`@src/main/java/starlight/adapter/businessplan/webapi/BusinessPlanBusinessPlanImageController.java`:
- Line 16: Rename the class BusinessPlanBusinessPlanImageController to
BusinessPlanImageController (update the class declaration and the public class
name) and update all references/usages (constructors, imports, bean/component
scans, tests, and any XML/annotation configuration) to the new identifier so
compilation and dependency injection still work; ensure the filename and any
Spring controller/component annotations match the new class name.
- Line 8: BusinessPlanBusinessPlanImageController imports the aireport
PresignedUrlProviderPort but should use a businessplan-specific port; create a
new interface PresignedUrlProviderPort under the businessplan domain (preferably
package businessplan.image.required.PresignedUrlProviderPort or
businessplan.required.PresignedUrlProviderPort), move or duplicate the contract
there, update BusinessPlanBusinessPlanImageController to import that new
interface, and update any implementation classes and dependency
injection/bindings to implement and provide the new businessplan
PresignedUrlProviderPort (or explicitly document/justify shared usage if you
intend to keep referencing aireport's port).

In
`@src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java`:
- Around line 8-9: PdfDownloadClient currently imports and throws
aireport-specific exceptions (OcrException, OcrErrorType); create a new
shared-layer exception (e.g., PdfDownloadException and optional
PdfDownloadErrorType) in starlight.adapter.shared.infrastructure.pdf.exception,
replace imports of OcrException/OcrErrorType in PdfDownloadClient with the new
shared exceptions, update the method signatures in PdfDownloadClient to throw
PdfDownloadException, and convert or map any caught OcrException instances into
PdfDownloadException before rethrowing so the shared adapter no longer depends
on adapter/aireport types.

In
`@src/main/java/starlight/application/expertApplication/ExpertApplicationCommandService.java`:
- Around line 56-57: Guard against a null PDF URL before attempting to download:
in ExpertApplicationCommandService where you check plan.isPdfBased(), also
verify plan.getPdfUrl() != null (or use Optional) and handle the null case
(e.g., log and throw a meaningful exception or mark the plan invalid) instead of
calling pdfDownloadPort.downloadFromUrl(plan.getPdfUrl()) with a null. Ensure
the error path provides clear context (include plan id or type) and keep
pdfDownloadPort.downloadFromUrl(...) only for non-null URLs.

In
`@src/test/java/starlight/adapter/businessplan/webapi/BusinessPlanImageControllerIntegrationTest.java`:
- Line 29: Rename the duplicated controller class name
BusinessPlanBusinessPlanImageController to BusinessPlanImageController: change
the class declaration in BusinessPlanBusinessPlanImageController.java to public
class BusinessPlanImageController and update all references/imports/usages (for
example the test annotation using controllers =
BusinessPlanBusinessPlanImageController.class) to use
BusinessPlanImageController.class; ensure constructors, annotations and any file
name consistency are updated accordingly so the class and its references match
the new identifier.

---

Outside diff comments:
In
`@src/main/java/starlight/application/expertApplication/ExpertApplicationCommandService.java`:
- Around line 50-84: The PDF download path in requestFeedback leaks OcrException
because pdfDownloadPort.downloadFromUrl(...) is called outside the try-catch
that wraps other failures; wrap the PDF download call in the same exception
handling and convert domain-specific exceptions into the expertApplication
domain by catching OcrException (and any checked IO exceptions) around
pdfDownloadPort.downloadFromUrl(...) and rethrowing an
ExpertApplicationException (e.g., ExpertApplicationErrorType.FILE_READ_ERROR or
a new type like PDF_DOWNLOAD_ERROR); ensure plan.isPdfBased() branch mirrors the
non-PDF branch's error handling so all exceptions from pdfDownloadPort and file
reading are caught and translated before escaping requestFeedback.

---

Nitpick comments:
In
`@src/main/java/starlight/adapter/aireport/report/agent/impl/SpringAiSectionGradeAgent.java`:
- Around line 60-68: The current call in SpringAiSectionGradeAgent that builds
the LLM request via
chatClient.prompt(...).options(ChatOptions.builder().temperature(0.0).topP(0.1).build()).advisors(...).call()
uses fixed, near-deterministic sampling and will likely return the same
unparseable output on retries; modify the retry logic around this call to vary
sampling parameters across attempts (e.g., iterate temperatures like 0.0 → 0.2 →
0.4 and/or adjust topP) or append a retry hint to the prompt for subsequent
attempts, ensuring each retry invokes chatClient.prompt with the updated
ChatOptions so transient parsing issues can produce different outputs.

In
`@src/main/java/starlight/adapter/aireport/report/provider/SpringAiAdvisorProvider.java`:
- Around line 22-24: The code mixes `@RequiredArgsConstructor` constructor
injection (vectorStore) with `@Value` field injection (qaAdvisorTemplate); make
qaAdvisorTemplate constructor-injected to be consistent: change the
qaAdvisorTemplate field to a private final String and remove the `@Value` on the
field, then add an explicit constructor or a constructor parameter annotated
with `@Value`("${prompt.report.qa-advisor.template}") so Spring injects it via the
constructor (update SpringAiAdvisorProvider, qaAdvisorTemplate, vectorStore
references accordingly).

In
`@src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java`:
- Around line 22-24: The PdfDownloadClient constructor is package-private which
prevents direct instantiation from other packages (e.g., external tests); if
this was unintended make the constructor public: change the
PdfDownloadClient(`@Qualifier`("pdfDownloadRestClient") RestClient downloadClient)
constructor to public so other packages can construct the class directly while
still allowing Spring to inject it; if package-private was intentional, add a
short comment on the constructor documenting that it is intentionally non-public
for encapsulation/testing reasons.

In
`@src/main/java/starlight/application/aireport/util/AiReportResponseParser.java`:
- Around line 126-161: The parse() method in AiReportResponseParser currently
catches all Exceptions and re-wraps even AiReportException instances, obscuring
original causes; update the try/catch so you first catch AiReportException and
re-throw it unchanged, then catch generic Exception to wrap into a new
AiReportException(AiReportErrorType.AI_RESPONSE_PARSING_FAILED); ensure this
change surrounds the logic that calls cleanJsonResponse(),
objectMapper.readTree(), parseFromJsonNode(), and the isDefaultResponse() check
so original AiReportException stack traces are preserved.
- Around line 460-473: In AiReportResponseParser, ensure gradingListScores is
trimmed once before validation: assign gradingListScores =
gradingListScores.trim() prior to the startsWith check and then use that trimmed
variable for objectMapper.readTree(...) and subsequent logic (preserve the
existing fallback to "[]"); update the block around the startsWith check in the
method where gradingListScores is validated so both the check and the JSON parse
use the same trimmed string.

In
`@src/main/java/starlight/application/expertApplication/ExpertApplicationCommandService.java`:
- Around line 101-108: The registerApplicationRecord method is unnecessarily
public; change its access modifier to private so it's only callable from within
this class (it is only used by requestFeedback). Locate the method named
registerApplicationRecord(Long expertId, Long planId) and update its declaration
from public to private, keeping the existing logic that checks
applicationQueryPort.existsByExpertIdAndBusinessPlanId(...), throws
ExpertApplicationException on duplicates, creates ExpertApplication.create(...),
and calls applicationQueryPort.save(...).

In
`@src/test/java/starlight/adapter/businessplan/webapi/BusinessPlanImageControllerIntegrationTest.java`:
- Around line 58-83: The commented-out test method getPresignedUrl_Success
should not remain as a large block comment; either delete the entire commented
block or restore the test as a real method and mark it explicitly with `@Disabled`
to indicate intentional skip; if choosing `@Disabled`, uncomment the method, add
the org.junit.jupiter.api.Disabled import and the `@Disabled` annotation above
getPresignedUrl_Success, keeping references to presignedUrlProvider,
PreSignedUrlResponse, mockMvc and createMockAuthDetails intact so the test
compiles but is skipped.

In
`@src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientIntegrationTest.java`:
- Around line 146-160: The test downloadFromUrl_LargeFile_Success in
PdfDownloadClientIntegrationTest currently converts a zero-filled byte[] to
String then sets it as the MockResponse body which can corrupt binary content
via charset roundtrip; instead, set the MockResponse body using an okio.Buffer
containing the raw byte[] (use mockWebServer.enqueue(new
MockResponse().setResponseCode(200).setBody(bufferWithBytes))) so the
pdfDownloadClient.downloadFromUrl call receives exact binary data and the
hasSize assertion remains valid; update references to MockResponse.setBody to
use okio.Buffer and ensure the buffer is written with largeBytes before
enqueueing.

- PDF: 공유 계층 예외 도입 (PdfDownloadException, PdfDownloadErrorType)
  - PdfDownloadClient가 aireport OcrException 의존 제거
- PDF URL 저장 시 검증
  - 커스텀 Validation @ValidPdfUrl 추가 (ValidPdfUrlValidator)
  - BusinessPlan/AiReport 요청 DTO에 @ValidPdfUrl 적용
  - 저장 전 downloadFromUrl로 접근 가능 여부 검증 (INVALID_PDF_URL)
- 전문가 신청: businessPlan에 pdfUrl 있으면 해당 URL에서 PDF 다운로드 후 전송
- AI 리포트: SpringAiSectionGradeAgent 재시도 지수 백오프, 마지막 시도 실패 메시지로 통일
- 이미지/PDF 인프라: PresignedUrlProviderPort 분리, NcpPresignedUrlProvider 정리
- ValidImageFileName/Validator를 adapter.shared.webapi.validation으로 이동
- BusinessPlanErrorType에 INVALID_PDF_URL 추가
- BusinessPlanImageController 등 관련 테스트·API Doc 반영
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/main/java/starlight/adapter/businessplan/webapi/BusinessPlanImageController.java (1)

29-32: ⚠️ Potential issue | 🟠 Major

finalizePublicobjectUrl 파라미터에 검증이 없습니다.

BackofficeImageController에서는 @Valid @RequestBody BackofficeImagePublicRequest를 사용하여 요청 본문을 검증하고 있는데, 여기서는 @RequestParam으로 raw URL을 직접 받고 있습니다. 검증 없이 외부 입력 URL을 S3 ACL 변경 작업에 전달하면 보안 위험(예: 의도치 않은 객체 공개)이 있습니다.

백오피스와 동일하게 DTO + @Valid를 사용하거나, 최소한 URL 형식 및 도메인 소속 여부를 검증하는 것을 권장합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/businessplan/webapi/BusinessPlanImageController.java`
around lines 29 - 32, finalizePublic currently accepts a raw `@RequestParam`
String objectUrl and calls presignedUrlReader.makePublic without validation;
change this to accept a validated DTO (e.g., BusinessPlanImagePublicRequest)
annotated with `@Valid` and `@RequestBody` (or perform explicit validation) and
validate the URL format and allowed domain/host before calling
presignedUrlReader.makePublic to prevent arbitrary object exposure; update the
controller method signature (finalizePublic) and add/reuse a DTO with validation
annotations (e.g., `@NotBlank`, `@URL` or custom validator) or inline checks that
ensure the URL belongs to your S3 bucket/expected domain prior to invoking
presignedUrlReader.makePublic.
src/main/java/starlight/adapter/shared/infrastructure/storage/NcpPresignedUrlProvider.java (1)

75-94: ⚠️ Potential issue | 🟠 Major

makePublic 메서드에 버킷 소유권 검증 로직이 부족합니다.

extractKeyFromUrl은 URL 형식(스킴, 경로 존재 여부)을 검증하지만, 전달된 objectUrl이 서비스의 설정된 버킷에 속하는지 검증하지 않습니다. 악의적인 사용자가 유효한 URL 형식을 갖춘 다른 버킷의 객체 URL을 전달할 경우, 메서드는 해당 객체의 ACL을 변경하려 시도할 수 있습니다.

권장사항: URL에서 버킷 이름을 추출하여 설정된 버킷(${cloud.ncp.object-storage.bucket-name})과 일치하는지 검증하는 로직을 추가하세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/shared/infrastructure/storage/NcpPresignedUrlProvider.java`
around lines 75 - 94, The makePublic method currently only extracts the object
key via extractKeyFromUrl and does not verify the URL's bucket, allowing ACL
changes on objects in other buckets; update makePublic to parse the bucket name
from objectUrl (alongside the key), compare it to the configured bucket field
(cloud.ncp.object-storage.bucket-name / this.bucket) and throw or return an
error (e.g., AiReportException with AiReportErrorType.OBJECT_ACL_UPDATE_FAILED)
if they do not match before calling ncpS3Client.putObjectAcl; keep existing
S3Exception handling intact and log a clear mismatch error including the
provided URL and expected bucket.
🧹 Nitpick comments (5)
src/main/java/starlight/adapter/aireport/webapi/dto/AiReportCreateWithPdfRequest.java (1)

11-15: @URL(protocol = "https")@ValidPdfUrl의 검증 범위 중복

@URL(protocol = "https")는 이미 URL 형식 + https 프로토콜을 검증하므로, 현재 @ValidPdfUrl(단순 URI 파싱)이 추가적으로 걸러주는 케이스가 없습니다. 두 어노테이션의 역할이 중복됩니다.

ValidPdfUrlValidator를 강화하여 PDF 특화 검증(예: .pdf 확장자, https 스킴)을 담당하게 한 뒤, @URL을 제거하고 @ValidPdfUrl 하나로 통합하거나, 반대로 @ValidPdfUrl을 제거하고 @URL만 유지하는 것이 깔끔합니다. BusinessPlanCreateWithPdfRequest와의 일관성도 함께 고려해 주세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/aireport/webapi/dto/AiReportCreateWithPdfRequest.java`
around lines 11 - 15, The `@URL`(protocol = "https") and `@ValidPdfUrl` annotations
on AiReportCreateWithPdfRequest#pdfUrl overlap; remove the redundant `@URL` and
consolidate PDF-specific checks into the ValidPdfUrl validator
(ValidPdfUrlValidator) so it enforces https scheme, .pdf extension and any
additional PDF-host policy checks, or alternatively remove ValidPdfUrl and rely
on `@URL` only — pick the approach consistent with
BusinessPlanCreateWithPdfRequest; if you choose to keep ValidPdfUrl, update
ValidPdfUrlValidator to perform URI parsing, check scheme == "https", ensure
path endsWith(".pdf"), and update the annotation usage on
AiReportCreateWithPdfRequest to only `@NotBlank` and `@ValidPdfUrl`.
src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java (1)

41-47: URI.create(url) 호출 시 잘못된 URL에 대한 방어 처리 확인

URI.create(url)은 URL이 유효하지 않을 경우 IllegalArgumentException을 던집니다. 현재 generic catch (Exception e) 블록에서 잡히긴 하지만, 이 경우 PDF_DOWNLOAD_ERROR(502 BAD_GATEWAY)로 매핑됩니다. 클라이언트가 잘못된 URL을 전달한 것이라면 4xx 계열 응답이 더 적절할 수 있습니다.

PR에서 ValidPdfUrl 검증이 추가되었으므로, 상위에서 이미 URL 형식이 검증된다면 현재 구조로 충분합니다. 다만 이 메서드가 검증 없이 직접 호출되는 경로가 있는지 확인해주세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java`
around lines 41 - 47, The downloadFromUrl method uses URI.create(url) which
throws IllegalArgumentException for invalid URLs but is currently lumped into a
generic catch that maps all failures to PDF_DOWNLOAD_ERROR (502); update
downloadFromUrl to explicitly validate or catch IllegalArgumentException from
URI.create and map it to a 4xx client error (e.g., return or throw a
BAD_REQUEST/invalid-url response) while keeping other failures mapped to
PDF_DOWNLOAD_ERROR; reference the downloadFromUrl method, the URI.create call,
and the PDF_DOWNLOAD_ERROR mapping so you locate and adjust the specific
exception handling or add a pre-validation guard (or rely on ValidPdfUrl at call
sites) accordingly.
src/main/java/starlight/adapter/businessplan/webapi/BusinessPlanImageController.java (1)

19-19: 필드명 presignedUrlReader가 실제 용도와 불일치합니다.

getPreSignedUrl(읽기)과 makePublic(쓰기) 양쪽 모두에 사용되므로, 포트 인터페이스 이름과 일치하는 presignedUrlProvider가 더 적합합니다.

♻️ 필드명 변경 제안
-    private final PresignedUrlProviderPort presignedUrlReader;
+    private final PresignedUrlProviderPort presignedUrlProvider;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/businessplan/webapi/BusinessPlanImageController.java`
at line 19, The field name presignedUrlReader in BusinessPlanImageController is
misleading because the PresignedUrlProviderPort is used for both reading and
writing (getPreSignedUrl and makePublic); rename the field to
presignedUrlProvider and update its constructor parameter and all usages inside
BusinessPlanImageController (references in getPreSignedUrl and makePublic) to
match the port name so the identifier aligns with the interface purpose.
src/test/java/starlight/adapter/businessplan/webapi/BusinessPlanImageControllerIntegrationTest.java (1)

58-83: 주석 처리된 테스트 코드를 정리해 주세요.

getPresignedUrl_Success 테스트가 전체 주석 처리되어 있습니다. 성공 케이스 테스트가 없으면 @ValidImageFileName 검증이 정상적으로 통과하는 경로와 Presigned URL 응답 형식을 검증할 수 없습니다.

테스트를 복원하거나, 현재 동작하지 않는 사유가 있다면 TODO 주석과 함께 이슈로 추적하는 것을 권장합니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/starlight/adapter/businessplan/webapi/BusinessPlanImageControllerIntegrationTest.java`
around lines 58 - 83, Uncomment and restore the getPresignedUrl_Success test so
the successful path for `@ValidImageFileName` and the Presigned URL response shape
are verified: re-enable the `@Test` and `@DisplayName` annotations, ensure the
mocked presignedUrlProvider is stubbed with
given(presignedUrlProvider.getPreSignedUrl(userId,
fileName)).willReturn(response), and confirm createMockAuthDetails(userId) is
available for the user(...) principal; keep the existing assertions against
PreSignedUrlResponse.of(preSignedUrl, objectUrl) and the
verify(presignedUrlProvider).getPreSignedUrl(userId, fileName) call. If the test
cannot be re-enabled now, add a TODO comment above the commented block
referencing a new tracking issue (create an issue ID) that explains why it is
disabled and what must be fixed to re-enable getPresignedUrl_Success.
src/main/java/starlight/adapter/shared/infrastructure/storage/NcpPresignedUrlProvider.java (1)

16-17: 공유 어댑터에서 AiReportException/AiReportErrorType을 직접 참조하고 있습니다.

NcpPresignedUrlProvidershared.infrastructure에 위치하며 businessplanbackoffice 두 도메인의 포트를 구현하고 있는데, makePublic (Line 90)에서 여전히 starlight.domain.aireport.exception.AiReportException을 던지고 있습니다. 공유 어댑터가 특정 도메인(aireport)의 예외에 의존하면 포트 분리의 취지가 희석됩니다.

shared 패키지 내 공통 예외(예: StorageException)를 정의하거나, 각 도메인 포트가 자체 예외를 정의하고 어댑터에서 매핑하는 방식을 고려해 주세요.

♻️ 예시: 공통 스토리지 예외 도입
-import starlight.domain.aireport.exception.AiReportErrorType;
-import starlight.domain.aireport.exception.AiReportException;
+import starlight.shared.exception.StorageException;
+import starlight.shared.exception.StorageErrorType;

makePublic 메서드 내:

-            throw new AiReportException(AiReportErrorType.OBJECT_ACL_UPDATE_FAILED, e);
+            throw new StorageException(StorageErrorType.OBJECT_ACL_UPDATE_FAILED, e);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/shared/infrastructure/storage/NcpPresignedUrlProvider.java`
around lines 16 - 17, NcpPresignedUrlProvider currently references
domain-specific AiReportException/AiReportErrorType (imported at top) and throws
AiReportException from makePublic; change this to use a shared/common storage
exception or map errors to the port-specific exceptions at the adapter boundary:
remove imports of starlight.domain.aireport.exception.*, throw a new shared
StorageException (or return a domain-neutral error type) from
NcpPresignedUrlProvider.makePublic, and if callers in aireport need
AiReportException, perform the mapping in the aireport adapter/port layer (or
have the port translate StorageException -> AiReportException using
AiReportErrorType). Ensure all references to AiReportException/AiReportErrorType
are removed from shared.infrastructure code and replace with the shared
exception type or mapping calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/main/java/starlight/adapter/businessplan/webapi/dto/BusinessPlanCreateWithPdfRequest.java`:
- Around line 10-12: The DTO BusinessPlanCreateWithPdfRequest currently
annotates the pdfUrl field with `@ValidPdfUrl` only, which is inconsistent with
AiReportCreateWithPdfRequest that uses `@URL`(protocol = "https"); align the
behavior by either adding `@URL`(protocol = "https") to the pdfUrl field in
BusinessPlanCreateWithPdfRequest or by updating the ValidPdfUrlValidator to
enforce an https scheme (and keep `@ValidPdfUrl`), and ensure the field name
pdfUrl and the validator ValidPdfUrlValidator are the targets of the change so
both DTOs enforce the same https policy.

In
`@src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java`:
- Around line 57-62: The catch block in PdfDownloadClient currently logs only
e.getMessage() and throws a new PdfDownloadException without preserving the
original cause; update the generic Exception catch to log the full throwable (e)
via log.error("PDF 다운로드 실패", e) and throw new
PdfDownloadException(PdfDownloadErrorType.PDF_DOWNLOAD_ERROR, e) so the original
exception is passed as the cause; reference PdfDownloadClient,
PdfDownloadException, PdfDownloadErrorType and the existing log.error call when
making the change.

In
`@src/main/java/starlight/adapter/shared/webapi/validation/ValidPdfUrlValidator.java`:
- Around line 12-21: ValidPdfUrlValidator.isValid currently treats blank strings
as invalid and uses URI.create which is too permissive; change it to return true
for null/blank (let `@NotBlank` handle that) and validate actual PDF HTTPS URLs by
constructing a java.net.URL (new URL(value)) or parsing the URI and then
enforcing that the scheme equals "https" and that host is non-empty, catching
MalformedURLException/IllegalArgumentException and returning false on parsing
failures; ensure error handling is limited to those exceptions and preserve the
method signature.

---

Outside diff comments:
In
`@src/main/java/starlight/adapter/businessplan/webapi/BusinessPlanImageController.java`:
- Around line 29-32: finalizePublic currently accepts a raw `@RequestParam` String
objectUrl and calls presignedUrlReader.makePublic without validation; change
this to accept a validated DTO (e.g., BusinessPlanImagePublicRequest) annotated
with `@Valid` and `@RequestBody` (or perform explicit validation) and validate the
URL format and allowed domain/host before calling presignedUrlReader.makePublic
to prevent arbitrary object exposure; update the controller method signature
(finalizePublic) and add/reuse a DTO with validation annotations (e.g.,
`@NotBlank`, `@URL` or custom validator) or inline checks that ensure the URL
belongs to your S3 bucket/expected domain prior to invoking
presignedUrlReader.makePublic.

In
`@src/main/java/starlight/adapter/shared/infrastructure/storage/NcpPresignedUrlProvider.java`:
- Around line 75-94: The makePublic method currently only extracts the object
key via extractKeyFromUrl and does not verify the URL's bucket, allowing ACL
changes on objects in other buckets; update makePublic to parse the bucket name
from objectUrl (alongside the key), compare it to the configured bucket field
(cloud.ncp.object-storage.bucket-name / this.bucket) and throw or return an
error (e.g., AiReportException with AiReportErrorType.OBJECT_ACL_UPDATE_FAILED)
if they do not match before calling ncpS3Client.putObjectAcl; keep existing
S3Exception handling intact and log a clear mismatch error including the
provided URL and expected bucket.

---

Duplicate comments:
In
`@src/test/java/starlight/adapter/businessplan/webapi/BusinessPlanImageControllerIntegrationTest.java`:
- Around line 28-29: The duplicate BusinessPlan class name issue has been
resolved and the test correctly references the controller; ensure the WebMvcTest
annotation remains as controllers = BusinessPlanImageController.class and remove
any remaining duplicate BusinessPlan class or imports (search for BusinessPlan
duplicates and unused imports) so the test compiles cleanly and there are no
naming collisions.

---

Nitpick comments:
In
`@src/main/java/starlight/adapter/aireport/webapi/dto/AiReportCreateWithPdfRequest.java`:
- Around line 11-15: The `@URL`(protocol = "https") and `@ValidPdfUrl` annotations
on AiReportCreateWithPdfRequest#pdfUrl overlap; remove the redundant `@URL` and
consolidate PDF-specific checks into the ValidPdfUrl validator
(ValidPdfUrlValidator) so it enforces https scheme, .pdf extension and any
additional PDF-host policy checks, or alternatively remove ValidPdfUrl and rely
on `@URL` only — pick the approach consistent with
BusinessPlanCreateWithPdfRequest; if you choose to keep ValidPdfUrl, update
ValidPdfUrlValidator to perform URI parsing, check scheme == "https", ensure
path endsWith(".pdf"), and update the annotation usage on
AiReportCreateWithPdfRequest to only `@NotBlank` and `@ValidPdfUrl`.

In
`@src/main/java/starlight/adapter/businessplan/webapi/BusinessPlanImageController.java`:
- Line 19: The field name presignedUrlReader in BusinessPlanImageController is
misleading because the PresignedUrlProviderPort is used for both reading and
writing (getPreSignedUrl and makePublic); rename the field to
presignedUrlProvider and update its constructor parameter and all usages inside
BusinessPlanImageController (references in getPreSignedUrl and makePublic) to
match the port name so the identifier aligns with the interface purpose.

In
`@src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java`:
- Around line 41-47: The downloadFromUrl method uses URI.create(url) which
throws IllegalArgumentException for invalid URLs but is currently lumped into a
generic catch that maps all failures to PDF_DOWNLOAD_ERROR (502); update
downloadFromUrl to explicitly validate or catch IllegalArgumentException from
URI.create and map it to a 4xx client error (e.g., return or throw a
BAD_REQUEST/invalid-url response) while keeping other failures mapped to
PDF_DOWNLOAD_ERROR; reference the downloadFromUrl method, the URI.create call,
and the PDF_DOWNLOAD_ERROR mapping so you locate and adjust the specific
exception handling or add a pre-validation guard (or rely on ValidPdfUrl at call
sites) accordingly.

In
`@src/main/java/starlight/adapter/shared/infrastructure/storage/NcpPresignedUrlProvider.java`:
- Around line 16-17: NcpPresignedUrlProvider currently references
domain-specific AiReportException/AiReportErrorType (imported at top) and throws
AiReportException from makePublic; change this to use a shared/common storage
exception or map errors to the port-specific exceptions at the adapter boundary:
remove imports of starlight.domain.aireport.exception.*, throw a new shared
StorageException (or return a domain-neutral error type) from
NcpPresignedUrlProvider.makePublic, and if callers in aireport need
AiReportException, perform the mapping in the aireport adapter/port layer (or
have the port translate StorageException -> AiReportException using
AiReportErrorType). Ensure all references to AiReportException/AiReportErrorType
are removed from shared.infrastructure code and replace with the shared
exception type or mapping calls.

In
`@src/test/java/starlight/adapter/businessplan/webapi/BusinessPlanImageControllerIntegrationTest.java`:
- Around line 58-83: Uncomment and restore the getPresignedUrl_Success test so
the successful path for `@ValidImageFileName` and the Presigned URL response shape
are verified: re-enable the `@Test` and `@DisplayName` annotations, ensure the
mocked presignedUrlProvider is stubbed with
given(presignedUrlProvider.getPreSignedUrl(userId,
fileName)).willReturn(response), and confirm createMockAuthDetails(userId) is
available for the user(...) principal; keep the existing assertions against
PreSignedUrlResponse.of(preSignedUrl, objectUrl) and the
verify(presignedUrlProvider).getPreSignedUrl(userId, fileName) call. If the test
cannot be re-enabled now, add a TODO comment above the commented block
referencing a new tracking issue (create an issue ID) that explains why it is
disabled and what must be fixed to re-enable getPresignedUrl_Success.

- 기존의 @url 방식의 검증을 ValidPdfUrlValidator에 통일
  - @notblank와 중첩되지 않도록 함
- PdfDownloadClient의 에러 메시지를 넘기도록 수정
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java (2)

49-56: 응답 Content-Type 미검증 — 비(非)PDF 바이트가 하류 OCR/이메일 처리까지 전달될 수 있음

현재는 바이트가 비어있거나 크기를 초과하는 경우만 검증합니다. Pre-signed URL 만료, 리다이렉트 오류, 인증 오류 등으로 서버가 text/html이나 application/json을 반환해도 통과됩니다. 이 경우 ClovaOcrProvider나 이메일 첨부 단계에서 의미 없는 오류가 발생합니다.

🛡️ Content-Type 검증 추가 제안
         byte[] data = entity.getBody();
         if (data == null || data.length == 0) {
             throw new PdfDownloadException(PdfDownloadErrorType.PDF_EMPTY_RESPONSE);
         }
+        MediaType contentType = entity.getHeaders().getContentType();
+        if (contentType == null || !contentType.isCompatibleWith(MediaType.APPLICATION_PDF)) {
+            log.warn("PDF가 아닌 Content-Type 응답 수신: contentType={}, url={}", contentType, url);
+            throw new PdfDownloadException(PdfDownloadErrorType.PDF_DOWNLOAD_ERROR);
+        }
         if (data.length > MAX_PDF_BYTES) {
             throw new PdfDownloadException(PdfDownloadErrorType.PDF_TOO_LARGE);
         }

필요 시 PdfDownloadErrorTypePDF_INVALID_CONTENT_TYPE을 별도 타입으로 추가하면 호출자 측 오류 분기 처리가 더 명확해집니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java`
around lines 49 - 56, The code in PdfDownloadClient currently only validates
byte length but not the HTTP response Content-Type, so non-PDF responses
(HTML/JSON) can flow into ClovaOcrProvider; update the download logic in
PdfDownloadClient to read the response Content-Type header before returning data
and reject any type that is not a PDF (e.g., does not start with
"application/pdf" or an allowed binary PDF MIME), throwing PdfDownloadException;
add a new PdfDownloadErrorType value (PDF_INVALID_CONTENT_TYPE) for clearer
error handling and ensure missing or null Content-Type is treated as invalid so
callers can handle it explicitly.

16-17: 동일한 계약을 가진 포트 인터페이스 중복 정의 — 통합 검토 권장

aireport.required.PdfDownloadPortexpertApplication.required.PdfDownloadPort는 완전히 동일한 메서드 시그니처를 정의합니다:

  • byte[] downloadFromUrl(String url)

두 인터페이스의 계약이 동일하기 때문에 Java 특성상 implements 절에 FQN이 강제되는 것입니다. 이는 인터페이스 중복 설계의 증상이며, 다음과 같은 개선을 고려해 주세요:

  1. 공유 포트로 통합: starlight.application.shared.required.PdfDownloadPort 같은 공유 포트로 통합하고, 양쪽 bounded context에서 이를 참조
  2. 단일 정의 위임: 두 bounded context 중 하나에서만 포트를 정의하고, 다른 한쪽이 이를 의존하는 방향 고려

헥사고날 아키텍처에서 각 bounded context가 고유한 포트를 정의하는 것이 일반적이지만, 계약이 동일하면 결국 단일 구현체가 양쪽을 만족시켜야 하므로 DRY 원칙 위반이 됩니다. 향후 두 포트의 요구사항이 달라질 경우 유지보수가 복잡해질 위험도 있습니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java`
around lines 16 - 17, PdfDownloadClient currently implements two identical
interfaces (starlight.application.aireport.required.PdfDownloadPort and
starlight.application.expertApplication.required.PdfDownloadPort); consolidate
the duplicated contract by creating a single shared port (e.g.,
starlight.application.shared.required.PdfDownloadPort) or pick one existing port
to be the canonical definition, update PdfDownloadClient to implement that
single interface (replace both FQNs in the implements clause with the
shared/canonical interface), and update all consumers/imports in the aireport
and expertApplication bounded contexts to reference the new/shared interface so
the single implementation satisfies both contexts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@src/main/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClient.java`:
- Around line 49-56: The code in PdfDownloadClient currently only validates byte
length but not the HTTP response Content-Type, so non-PDF responses (HTML/JSON)
can flow into ClovaOcrProvider; update the download logic in PdfDownloadClient
to read the response Content-Type header before returning data and reject any
type that is not a PDF (e.g., does not start with "application/pdf" or an
allowed binary PDF MIME), throwing PdfDownloadException; add a new
PdfDownloadErrorType value (PDF_INVALID_CONTENT_TYPE) for clearer error handling
and ensure missing or null Content-Type is treated as invalid so callers can
handle it explicitly.
- Around line 16-17: PdfDownloadClient currently implements two identical
interfaces (starlight.application.aireport.required.PdfDownloadPort and
starlight.application.expertApplication.required.PdfDownloadPort); consolidate
the duplicated contract by creating a single shared port (e.g.,
starlight.application.shared.required.PdfDownloadPort) or pick one existing port
to be the canonical definition, update PdfDownloadClient to implement that
single interface (replace both FQNs in the implements clause with the
shared/canonical interface), and update all consumers/imports in the aireport
and expertApplication bounded contexts to reference the new/shared interface so
the single implementation satisfies both contexts.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientIntegrationTest.java (2)

54-58: 바이너리 데이터 전달 시 setBody(okio.Buffer) 사용 권장

setBody(String)은 내부적으로 UTF-8로 인코딩하여 바디를 설정합니다. "PDF content".getBytes()는 JVM 기본 인코딩을 사용하므로, 환경에 따라 UTF-8이 아닐 경우 바이트 불일치가 발생할 수 있습니다. 실제 바이너리 PDF 데이터를 다루는 테스트에서는 String 변환 우회로 바이트 무결성을 보장하는 것이 좋습니다.

♻️ 제안: new Buffer().write(bytes) 사용
-byte[] expectedBytes = "PDF content".getBytes();
+byte[] expectedBytes = "PDF content".getBytes(StandardCharsets.UTF_8);
 mockWebServer.enqueue(new MockResponse()
         .setResponseCode(200)
-        .setBody(new String(expectedBytes))
+        .setBody(new okio.Buffer().write(expectedBytes))
         .addHeader("Content-Type", "application/pdf"));

Line 126-130도 동일하게 적용:

-byte[] expectedBytes = "PDF with params".getBytes();
+byte[] expectedBytes = "PDF with params".getBytes(StandardCharsets.UTF_8);
 mockWebServer.enqueue(new MockResponse()
         .setResponseCode(200)
-        .setBody(new String(expectedBytes)));
+        .setBody(new okio.Buffer().write(expectedBytes)));

Also applies to: 126-130

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientIntegrationTest.java`
around lines 54 - 58, In PdfDownloadClientIntegrationTest replace uses of
MockResponse.setBody(String) when enqueuing binary PDF fixtures so the raw bytes
are preserved: create an okio.Buffer (new Buffer()) and write the byte[]
(expectedBytes) into it, then call setBody(buffer) on the MockResponse instead
of setBody(new String(...)); do the same fix for the second occurrence around
the other MockResponse setup; ensure okio.Buffer is imported and the
buffer.write(bytes) is used to avoid JVM default charset conversions.

147-160: 대용량 파일 테스트: new String(largeBytes) 패턴이 메모리 낭비 및 검증 약화 초래

두 가지 문제가 있습니다:

  1. new String(new byte[10 * 1024 * 1024])는 내부적으로 10MB byte[] + 약 20MB의 UTF-16 char[]를 동시에 힙에 올립니다. 바이너리 바디 설정 시에는 new Buffer(); buf.write(body); setBody(buf) 패턴을 사용하면 불필요한 String 변환을 피할 수 있습니다.

  2. Line 159의 assertThat(result).hasSize(10 * 1024 * 1024) 검증은 사이즈만 확인하므로, 전송 과정에서 바이트가 변조되더라도 테스트가 통과합니다.

♻️ 제안
-byte[] largeBytes = new byte[10 * 1024 * 1024]; // 10MB
+byte[] largeBytes = new byte[10 * 1024]; // 10KB — 크기 경계 테스트 목적만이라면 충분
 mockWebServer.enqueue(new MockResponse()
         .setResponseCode(200)
-        .setBody(new String(largeBytes)));
+        .setBody(new okio.Buffer().write(largeBytes)));
 ...
-assertThat(result).hasSize(10 * 1024 * 1024);
+assertThat(result).isEqualTo(largeBytes);

참고: 30MB 초과 시 PDF_TOO_LARGE 예외가 발생하는 경계 조건(unit test에서만 검증 중)을 integration test에서도 보완하는 것을 고려해 보세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientIntegrationTest.java`
around lines 147 - 160, The test uses new String(largeBytes) which creates an
unnecessary UTF-16 char[] and wastes memory; replace the MockResponse body setup
to write raw bytes using a Buffer/okio buffer (e.g., create a new Buffer and
write the largeBytes into it, then pass that to MockResponse.setBody) in the
mockWebServer.enqueue call, and strengthen the assertion after
pdfDownloadClient.downloadFromUrl(url) to verify byte-for-byte equality (e.g.,
assertThat(result).isEqualTo(largeBytes)) instead of only checking size to
ensure no content corruption.
src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientTest.java (1)

166-183: 프리사인드 URL 테스트에서 URI 전달 여부 검증 누락

downloadFromUrl_Success_WithEncodedUrl 테스트(Line 163)와 달리, 이 테스트에는 실제 URI가 클라이언트에 그대로 전달되는지 확인하는 verify 호출이 없습니다. 프리사인드 URL은 서명 파라미터가 변조되면 인증 오류가 발생하므로, URL이 수정 없이 그대로 전달된다는 것을 명시적으로 검증하는 것이 테스트 의도에 부합합니다.

♻️ 제안
     // then
     assertThat(result).isEqualTo(testPdfBytes);
+    verify(requestHeadersUriSpec).uri(URI.create(presignedUrl));
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientTest.java`
around lines 166 - 183, The test downloadFromUrl_Success_WithPresignedUrl is
missing a verification that the exact presignedUrl is passed to the WebClient
flow; after calling pdfDownloadClientInstance.downloadFromUrl(presignedUrl) add
a verify that requestHeadersUriSpec.uri(...) was invoked with a URI built from
the original presignedUrl (e.g.,
verify(requestHeadersUriSpec).uri(eq(URI.create(presignedUrl)))). This ensures
the presigned query parameters are not altered before retrieval.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientIntegrationTest.java`:
- Around line 54-58: In PdfDownloadClientIntegrationTest replace uses of
MockResponse.setBody(String) when enqueuing binary PDF fixtures so the raw bytes
are preserved: create an okio.Buffer (new Buffer()) and write the byte[]
(expectedBytes) into it, then call setBody(buffer) on the MockResponse instead
of setBody(new String(...)); do the same fix for the second occurrence around
the other MockResponse setup; ensure okio.Buffer is imported and the
buffer.write(bytes) is used to avoid JVM default charset conversions.
- Around line 147-160: The test uses new String(largeBytes) which creates an
unnecessary UTF-16 char[] and wastes memory; replace the MockResponse body setup
to write raw bytes using a Buffer/okio buffer (e.g., create a new Buffer and
write the largeBytes into it, then pass that to MockResponse.setBody) in the
mockWebServer.enqueue call, and strengthen the assertion after
pdfDownloadClient.downloadFromUrl(url) to verify byte-for-byte equality (e.g.,
assertThat(result).isEqualTo(largeBytes)) instead of only checking size to
ensure no content corruption.

In
`@src/test/java/starlight/adapter/shared/infrastructure/pdf/PdfDownloadClientTest.java`:
- Around line 166-183: The test downloadFromUrl_Success_WithPresignedUrl is
missing a verification that the exact presignedUrl is passed to the WebClient
flow; after calling pdfDownloadClientInstance.downloadFromUrl(presignedUrl) add
a verify that requestHeadersUriSpec.uri(...) was invoked with a URI built from
the original presignedUrl (e.g.,
verify(requestHeadersUriSpec).uri(eq(URI.create(presignedUrl)))). This ensures
the presigned query parameters are not altered before retrieval.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/test.yaml (1)

33-36: Gradle 의존성 캐싱 미적용으로 CI 빌드 시간이 증가할 수 있습니다.

현재 워크플로우에는 Gradle 의존성 캐시 설정이 없습니다. 이번 PR에서 테스트가 대폭 확장되었고 외부 의존성도 늘어난 만큼, gradle/actions/setup-gradle을 활용한 캐싱을 적용하면 반복 빌드 시간을 단축할 수 있습니다.

⚡ Gradle 캐싱 적용 예시
      - name: Set up JDK 21
        uses: actions/setup-java@v4
        with:
          java-version: '21'
          distribution: temurin

+     - name: Setup Gradle
+       uses: gradle/actions/setup-gradle@v3

      - name: Grant execute permission for gradlew
        run: chmod +x gradlew
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/test.yaml around lines 33 - 36, The CI workflow currently
runs the "Test with Gradle" step without caching, causing longer repeat builds;
update the job to install and configure Gradle cache by adding the Gradle setup
action (gradle/actions/setup-gradle) and an actions/cache step to persist
~/.gradle/caches and ~/.gradle/wrapper (or use the setup-gradle built-in caching
options), ensuring the "Test with Gradle" step (the run: ./gradlew --info test)
uses the cached directories and set a versioned cache key (e.g., including
gradle-wrapper.properties or build.gradle checksum) so dependencies are restored
between runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/test.yaml:
- Around line 35-36: The CI sets SPRING_PROFILES_ACTIVE: test but there is no
test profile config, so either add a test profile or remove the env var: create
src/test/resources/application-test.yml (or application-test.properties)
containing test-specific configuration (e.g., in-memory H2 datasource, mock
endpoints or properties for services used by beans) so Spring will load it for
tests, ensuring beans configured in ObjectMapperConfig, RedisConfig,
SecurityConfig and external ports like PdfDownloadPort and
PresignedUrlProviderPort are wired to test/mocked values; alternatively remove
the SPRING_PROFILES_ACTIVE: test entry from the workflow if you don't want to
use a separate test profile.

---

Nitpick comments:
In @.github/workflows/test.yaml:
- Around line 33-36: The CI workflow currently runs the "Test with Gradle" step
without caching, causing longer repeat builds; update the job to install and
configure Gradle cache by adding the Gradle setup action
(gradle/actions/setup-gradle) and an actions/cache step to persist
~/.gradle/caches and ~/.gradle/wrapper (or use the setup-gradle built-in caching
options), ensuring the "Test with Gradle" step (the run: ./gradlew --info test)
uses the cached directories and set a versioned cache key (e.g., including
gradle-wrapper.properties or build.gradle checksum) so dependencies are restored
between runs.

- 기존의 AiReportResponseParser의 DTO/Entity 변환 로직은 AiReportResult로 로직 이동
- AiReportResponseParser를 adapter/aireport/report/parser로 패키지 이동
- BusinessPlanContentExtractor를 application/aireport/util로 이동
- 기존의 AiReportResponseParser의 DTO/Entity 변환 로직은 AiReportResult로 로직 이동
@2ghrms 2ghrms changed the title [SRLT-126] PDF 관련 AI 리포트 고도화 (미완) [SRLT-126] PDF AI 리포트 관련 에러 수정 및 구조 리팩토링 Feb 19, 2026
@SeongHo5356 SeongHo5356 self-requested a review February 21, 2026 07:22
Copy link
Member

@SeongHo5356 SeongHo5356 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생했서요~!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🧵 REFACTOR 코드 리팩토링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants