Skip to content

[REFACTOR] AI요금 사용량에 따른 모델 변경 로직 작성#80

Merged
coli-geonwoo merged 34 commits intodevelopfrom
refactor/#72
Aug 25, 2025
Merged

[REFACTOR] AI요금 사용량에 따른 모델 변경 로직 작성#80
coli-geonwoo merged 34 commits intodevelopfrom
refactor/#72

Conversation

@coli-geonwoo
Copy link
Collaborator

@coli-geonwoo coli-geonwoo commented Aug 23, 2025

🚩 연관 이슈

close #72
close #69
close #77

🔂 변경 내역

  • GPT-5, GPT-5-mini , GPT- 5-nano 순으로 각각 월 15000원을 기준으로 7500원, 12500원을 기준으로 모델을 동적으로 변경하여 프롬프팅할 수 있도록 합니다.
  • GPT 사용 요금을 추적하여 저장합니다.

Summary by CodeRabbit

  • 신기능

    • 기존 레포지토리 재연결(re-tracking) 지원 및 웹훅 자동 재등록
    • PR 분석 요청에 모델 지정 기능 추가 및 채팅 기반 AI 분석 흐름 도입
    • AI 과금 도메인(AiCharge, OpenAiModel) 및 통화 변환 유틸 추가
  • 개선

    • PR 분석 응답에 토큰 사용량 및 구조화된 분석 결과 포함
    • 모델 자동선택에 따른 월별 이용요금 누적 처리
  • 설정

    • 개발 프로필에 Secrets Manager 연동 추가 및 로컬 설정 정리 (주석 처리)
  • 테스트

    • 관련 단위/통합 테스트와 테스트 유틸(제너레이터·페이크 클라이언트) 추가

@coderabbitai
Copy link

coderabbitai bot commented Aug 23, 2025

Walkthrough

저장 시 사용자+externalId로 Optional 검사를 수행해 기존 레포는 재연결(re-tracking) 또는 ALREADY_SAVED 예외로 분기하며, PR 분석을 DTO/ChatClient 기반으로 리팩터링하고 AI 과금 도메인·리포지토리와 월별 과금 누적 흐름을 추가함.

Changes

Cohort / File(s) Summary
Repository 재연결 흐름
gss-api-app/.../RepositoryFacadeService.java, gss-domain/.../service/repository/RepositoryService.java, gss-domain/.../repository/github/repo/GithubRepoDomainRepository.java, gss-domain/.../jpa/repository/github/repo/GithubRepoDomainRepositoryImpl.java, gss-domain/.../entity/github/repo/GithubRepository.java, gss-domain/.../jpa/entity/github/repo/GithubRepositoryEntity.java, gss-mcp-app/.../WebhookFacadeService.java
사용자+externalId로 Optional 조회해 중복이면 re-tracking 또는 ALREADY_SAVED 예외로 분기하도록 save 흐름 변경. 도메인 리포지토리에 Optional finder 추가·throwing getter 분리. 엔티티 githubRepositoryId의 unique 제약 제거. 웹훅 등록 호출 추가.
PR 분석 클라이언트 전환
gss-client/gss-mcp-client/.../PrAnalysisClient.java, .../PrAnalysisClientImpl.java, .../PromptBuilder.java, .../config/AiConfig.java, .../serdes/PrAnalysisMapper.java
analyze 시그니처를 AnalyzePrRequest DTO로 변경하고 ChatClient/프롬프트 기반으로 리팩터링. PromptBuilder·AiConfig 추가, AI 응답 역직렬화용 PrAnalysisMapper 도입.
PR 분석 DTO/응답 구조화
gss-client/gss-mcp-client/.../dto/request/AnalyzePrRequest.java, .../dto/response/PrAnalysis.java, .../dto/response/AnalyzePrResponse.java
AnalyzePrRequest 레코드 추가. AnalyzePrResponse를 토큰 사용량(int...)과 구조화된 PrAnalysis로 변경(Usage→PrAnalysis 생성자 추가). PrAnalysis 타입 신설(요약/세부/질문 구조).
MCP 앱 분석 흐름 통합
gss-mcp-app/.../PrAnalysisAdapter.java, .../service/pranalysis/PrAnalysisService.java, .../event/QuestionEventListener.java, .../dto/request/AdaptedAnalyzePrResponse.java
PrAnalysisService 신설: GitHub diff 취득→월별 AiCharge 조회→모델 선택→분석 호출→소비금액 계산·누적. Adapter/Listener는 DTO 기반 호출로 전환, 응답에 토큰 정보 포함.
AI 과금 도메인·영속화
gss-common/.../CurrencyUtil.java, gss-domain/.../entity/analysis/AiCharge.java, .../entity/analysis/OpenAiModel.java, .../repository/analysis/AiChargeRepository.java, .../jpa/entity/analysis/AiChargeEntity.java, .../jpa/repository/analysis/AiChargeJpaRepository.java, .../jpa/repository/analysis/AiChargeRepositoryImpl.java
환율 유틸 추가, AiCharge 도메인·JPA 엔티티·리포지토리·구현 추가. OpenAiModel로 모델 선택 및 토큰 단가 기반 KRW 계산·월 누적 업데이트 구현.
설정·프롬프트 변경
gss-client/.../application-mcp-client.yml, gss-mcp-app/src/test/resources/application.yml, gss-client/.../etc/OpenaiPropertiesLogger.java, gss-mcp-app/src/main/resources/application-local.yml
시스템 프롬프트 추가 및 diff를 Base64로 처리 지시, 로컬 모델 설정 제거·dev 프로파일 AWS Secrets import 추가. OpenaiPropertiesLogger에서 모델 로그 제거. 로컬 DB 설정 주석 처리.
테스트·픽스처·MSW 인프라
gss-api-app/.../RepositoryFacadeServiceTest.java, gss-domain/.../OpenAiModelTest.java, gss-domain/.../testFixtures/.../GithubRepoGenerator.java, gss-api-app/.../RepositoryServiceTest.java, gss-mcp-app/.../PrAnalysisClientImplTest.java, gss-mcp-app/.../fake/FakePrAnalysisClient.java, gss-api-app/.../BaseRepositoryTest.java, gss-api-app/.../BaseServiceTest.java, gss-api-app/.../repository/analysis/AiChargeRepositoryTest.java, gss-domain/.../testFixtures/AiChargeGenerator.java
재연결/저장 테스트 추가, 모델 선택 테스트 추가, 생성기 확장 및 AiCharge 관련 테스트·베이스 클래스 추가.
기타 포맷/빌드 설정
gss-mcp-app/.../event/QuestionCreateEvent.java, gss-client/gss-mcp-client/build.gradle, gss-client/gss-mcp-client/src/test/java/.../ConcurrencyModelTest.java
포맷 변경, 모듈 종속성(gss-common) 추가, 동시성 관찰용 Disabled 테스트 추가.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User
  participant RepoFacade as RepositoryFacadeService
  participant RepoSvc as RepositoryService
  participant RepoRepo as GithubRepoDomainRepository
  participant Webhook as WebHookService
  Note over RepoFacade,RepoSvc: 저장 흐름 (중복 검사 → 재연결 또는 신규저장)

  User->>RepoFacade: saveRepository(user, repositoryInfo)
  RepoFacade->>RepoSvc: findByUserAndExternalId(user, externalId)
  alt exists (Optional present)
    RepoFacade->>RepoSvc: reTracking(user, externalId)
    RepoSvc->>RepoRepo: getByExternalIdAndUserId(externalId, user.id)
    RepoRepo-->>RepoSvc: GithubRepository
    RepoSvc->>RepoRepo: update(reTracking(repo))
    RepoFacade->>Webhook: registerWebhook(user, repo)
    RepoFacade-->>User: GithubRepository (tracking=true)
  else not exists
    RepoFacade->>RepoSvc: save(createCommand)
    RepoSvc-->>RepoFacade: GithubRepository
    RepoFacade->>Webhook: registerWebhook(user, repo)
    RepoFacade-->>User: GithubRepository
  end
Loading
sequenceDiagram
  autonumber
  participant GH as GithubAdaptor
  participant Service as PrAnalysisService
  participant Adapter as PrAnalysisAdapter
  participant Client as PrAnalysisClient
  participant ChargeRepo as AiChargeRepository
  participant Model as OpenAiModel
  Note over Service,Client: PR 분석 및 과금 누적 (요청 DTO → ChatClient)

  Service->>GH: getChanges(githubToken, request.diffUrl)
  GH-->>Service: diff
  Service->>ChargeRepo: getByYearAndMonth(currentYear, currentMonth)
  ChargeRepo-->>Service: AiCharge
  Service->>Model: getModelByUsage(AiCharge.charge)
  Service->>Adapter: analyze(title, desc, diff, modelName)
  Adapter->>Client: analyze(AnalyzePrRequest)
  Client-->>Adapter: AnalyzePrResponse (tokens + PrAnalysis)
  Adapter-->>Service: AdaptedAnalyzePrResponse
  Service->>Model: getCharge(promptTokens, completionTokens)
  Model-->>Service: consumedCharge(KRW)
  Service->>ChargeRepo: addCharge(year, month, consumedCharge)
  Service-->>Caller: AdaptedAnalyzePrResponse
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–90 minutes

Assessment against linked issues

Objective Addressed Explanation
재등록 시 tracking 기준으로 유효성 검사 변경 (레포 재등록 관련) [#77]
Spring AI 인터페이스 활용하여 raw code 리팩터링 [#69]
토큰 권한이 없을 때 에러코드 구분 [#72] 권한/토큰 권한별 에러 코드 분기 또는 매핑 변경 없음.

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
AI 과금 도메인·JPA·서비스 추가 (gss-domain/.../analysis/*, AiChargeJpaRepository, AiChargeRepositoryImpl, PrAnalysisService.java) 링크된 이슈들은 레포 재등록/AI 리팩터링/토큰 권한 분류에 초점. 신규 과금 도메인·영속화·누적 로직은 이들 이슈의 명시적 요구사항 범위를 벗어남.
설정·프롬프트 대규모 변경 (application-mcp-client.yml, application.yml 테스트 리소스) 프롬프트·설정 변경은 리팩터링에 연관될 수 있으나 #77(레포 재등록) 및 #72(토큰 권한 분기) 목적과 직접 일치하지 않음.

Possibly related PRs

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/#72

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • 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

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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: 13

🧹 Nitpick comments (50)
gss-mcp-app/src/main/resources/application-local.yml (3)

5-14: PR 범위와의 정합성 확인: 로컬 DB 비활성화가 AI 요금/모델 전환 기능 테스트에 미치는 영향

이번 PR의 핵심은 AI 요금 누적/티어 기반 모델 스위칭입니다. 해당 기능이 영속 계층(요금 누적 저장 등)에 의존한다면, 로컬에서 DB 비활성화 시 통합 테스트/수동 검증이 어려워질 수 있습니다. 로컬만 비활성화하고 dev/prod에서만 동작시키려는 의도인지 확인 부탁드립니다. 필요 시 로컬 전용 토글(예: gss.ai.billing.enabled=false)로 기능을 우회하는 방법도 고려해 주세요.


5-14: (재활성화 시) dialect 및 ddl-auto 설정 정리 제안

주석 블록을 미래에 재활성화한다면:

  • dialect는 최신 Hibernate에서 JDBC 메타데이터 기반 자동 감지가 가능하므로 명시를 생략하는 방향을 권장합니다.
  • 로컬에서 create-drop은 개발 편의성은 좋지만, 마이그레이션 도구(Flyway/Liquibase) 사용 시 충돌/혼란을 유발할 수 있습니다. 필요 시 update 또는 관리 도구로 일원화하세요.

5-14: 이전 평문 크리덴셜 처리 점검

기존에 username/password가 YAML에 있었던 것으로 보입니다(현재는 주석). 레포지토리 이력에 노출되었을 수 있으니, 실제 비밀번호 사용 이력이 있다면 회수/로테이션 여부와 시크릿 스캐닝(예: GitHub Secret Scanning) 상태를 확인해 주세요. 필요하면 제가 체크리스트/가이드 정리 도와드리겠습니다.

gss-domain/src/main/java/com/devoops/domain/entity/github/repo/GithubRepository.java (1)

27-29: 메서드 명명 개선 제안: stopTracking과의 대칭성 확보

reTracking()은 의미 파악이 즉각적이지 않습니다. stopTracking()과의 대칭을 고려하면 startTracking() 또는 resumeTracking()이 더 자연스럽습니다. 공개 API라면 도메인 용어 일관성도 중요합니다. 추후 IDE 자동완성/검색 편의성까지 감안해 리네이밍을 권장합니다.

gss-api-app/src/test/java/com/devoops/service/facade/RepositoryFacadeServiceTest.java (2)

13-13: 불필요한 import 정리

RepositoryCreateCommand는 본 테스트 클래스에서 사용되지 않습니다. 정리해 불필요한 의존성을 줄여주세요.

-import com.devoops.command.request.RepositoryCreateCommand;

85-101: 재연결 시 검증 보강: 동일 엔티티 재활성화 및 웹훅 호출 확인

재연결 흐름의 핵심은

  • 기존 레포지토리(ID 유지)의 isTracking이 true로 바뀌고,
  • 웹훅이 재등록되는지
    를 보장하는 것입니다. 아래와 같이 검증을 보강하면 회귀를 방지할 수 있습니다.
         @Test
         void 레포지토리를_재연결_할_수_있다() {
             User user = userGenerator.generate("김건우");
             GithubRepository unTrackingRepo = repoGenerator.generate(user, "연결 끊긴 레포지토리", 123L, false);
             RepositorySaveRequest request = new RepositorySaveRequest("https://github.com/octocat/Hello-World");
             mockingGithubClient();

-            GithubRepository reTrackingRepository = repositoryFacadeService.save(request, user);
+            GithubRepository reTrackingRepository = repositoryFacadeService.save(request, user);

             GithubRepository actual = githubRepoDomainRepository.findByIdAndUserId(
                     unTrackingRepo.getId(),
                     user.getId()
             );

             assertThat(actual.isTracking()).isTrue();
+            // 동일 엔티티가 재활성화되었는지 확인
+            assertThat(reTrackingRepository.getId()).isEqualTo(unTrackingRepo.getId());
+            // 재연결 시 웹훅/레포 조회 호출이 수행되는지 확인
+            Mockito.verify(gitHubClient, times(1)).createWebhook(any(), any(), any(), any());
+            Mockito.verify(gitHubClient, times(1)).getRepositoryInfo(any(), any(), any());
         }
gss-mcp-app/src/main/java/com/devoops/service/webhook/WebhookFacadeService.java (2)

28-31: 머지 여부 가드 더 엄격히: null 안전

현재 조건은 false일 때만 반환하여 null도 처리 경로를 타게 됩니다. “머지된 이벤트만 처리”가 목적이라면 아래처럼 true일 때만 진행하는 것이 명확합니다.

-        if (Boolean.FALSE.equals(request.isMerged())) {
-            return;
-        }
+        if (!Boolean.TRUE.equals(request.isMerged())) {
+            return;
+        }

51-55: 주석 수정 제안: prCount 증가 로직 반영

WebhookFacadeService 51행의 주석에 “prCount 올리기”라고 되어 있으나, 실제 prCount 증분은 본 메서드 내부가 아니라 PullRequestDomainRepositoryImpl.save() 메서드에서 호출되는
githubRepositoryEntity.plusPrCount()에서 처리됩니다. 따라서 아래와 같이 주석을 명확히 수정하거나, 필요 없으면 제거하길 권장합니다.

– 파일: gss-mcp-app/src/main/java/com/devoops/service/webhook/WebhookFacadeService.java
– 라인: 51

-        // 레포 아이디를 기반으로 찾기 -> 풀리퀘 생성 -> prCount 올리기
+        // 레포 아이디를 기반으로 조회 → 풀리퀘 생성  
+        // (prCount 증분은 PullRequestDomainRepositoryImpl.save() 내 plusPrCount 호출로 수행)
gss-domain/src/testFixtures/java/com/devoops/generator/GithubRepoGenerator.java (2)

20-30: Lombok 전필드 생성자 의존 감소

테스트 픽스처에서 Lombok이 생성하는 “전 필드 생성자”(필드 순서 민감)에 의존하면, 필드 순서 변경 시 컴파일 타임에 깨질 수 있습니다. 클래스에 이미 공개 생성자 (userId, name, url, owner, prCount, externalId, isTracking)가 있으므로 그 생성자를 사용하는 편이 안전합니다.

-        GithubRepository repository = new GithubRepository(
-                null,
-                user.getId(),
-                repoName,
-                "url",
-                "owner",
-                0,
-                externalId,
-                tracking
-        );
+        GithubRepository repository = new GithubRepository(
+                user.getId(),
+                repoName,
+                "url",
+                "owner",
+                0,
+                externalId,
+                tracking
+        );

16-18: externalId의 양수 보장

GitHub 리소스 ID는 양수라는 전제가 일반적입니다. 랜덤 값에서 음수가 나올 수 있으니 양수 범위를 명시하는 편이 안정적입니다.

-        return generate(user, repoName, ThreadLocalRandom.current().nextLong(), true);
+        return generate(user, repoName, ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE), true);
gss-api-app/src/test/java/com/devoops/service/repository/RepositoryServiceTest.java (1)

31-51: 신규 저장 테스트 LGTM + 트래킹 기본값도 함께 검증 제안

입력 → 저장 결과 필드 검증이 명확합니다. 추가로 기본 트래킹 상태까지 검증하면 의도 파악과 회귀 방지에 도움이 됩니다.

             assertAll(
                     () -> assertThat(createCommand.url()).isEqualTo(savedRepository.getUrl()),
                     () -> assertThat(createCommand.userId()).isEqualTo(savedRepository.getUserId()),
                     () -> assertThat(createCommand.externalId()).isEqualTo(savedRepository.getExternalId()),
                     () -> assertThat(createCommand.prCount()).isEqualTo(savedRepository.getPrCount()),
                     () -> assertThat(createCommand.ownerName()).isEqualTo(savedRepository.getOwner()),
-                    () -> assertThat(createCommand.repositoryName()).isEqualTo(savedRepository.getName())
+                    () -> assertThat(createCommand.repositoryName()).isEqualTo(savedRepository.getName()),
+                    () -> assertThat(savedRepository.isTracking()).isTrue()
             );
gss-domain/src/main/java/com/devoops/service/repository/RepositoryService.java (2)

28-31: Optional 기반 조회 좋습니다. readOnly 트랜잭션을 붙여 일관성을 유지해 주세요.
동일 클래스 내 다른 조회 메서드들과의 일관성과 커넥션 최적화를 위해 @Transactional(readOnly = true)를 권장합니다. 또한 파라미터명 repositoryId는 실제로는 GitHub externalId를 의미하므로, 가능하다면 externalId로의 리네이밍도 고려해 주세요.

-    public Optional<GithubRepository> findByUserAndExternalId(User user, long repositoryId) {
+    @Transactional(readOnly = true)
+    public Optional<GithubRepository> findByUserAndExternalId(User user, long repositoryId) {
         return repoRepository.findByExternalIdAndUserId(repositoryId, user.getId());
     }

32-36: 재연결(reTracking) 읽기-수정-저장 동작에 트랜잭션 경계 추가 제안
동시에 stopTracking 같은 갱신이 들어올 수 있는 점을 고려하면, get→도메인 변환→update를 하나의 트랜잭션 경계로 묶는 편이 안전합니다. 현재 update는 트랜잭션이지만, 조회와의 사이에 경합 여지가 있습니다. 또한 one-liner로 간결화 가능합니다.

-    public GithubRepository reTracking(User user, long repositoryId) {
-        GithubRepository repo = repoRepository.getByExternalIdAndUserId(repositoryId, user.getId());
-        GithubRepository reTrackingRepo = repo.reTracking();
-        return repoRepository.update(reTrackingRepo);
-    }
+    @Transactional
+    public GithubRepository reTracking(User user, long repositoryId) {
+        GithubRepository repo = repoRepository.getByExternalIdAndUserId(repositoryId, user.getId());
+        return repoRepository.update(repo.reTracking());
+    }
gss-domain/src/main/java/com/devoops/jpa/repository/github/repo/GithubRepoDomainRepositoryImpl.java (2)

69-72: readOnly 트랜잭션 누락
다른 조회성 메서드들과 동일하게 @Transactional(readOnly = true)를 붙여 주세요. JPA 플러시 방지 및 성능/일관성 측면에서 이점이 있습니다.

-    public Optional<GithubRepository> findByExternalIdAndUserId(long externalId, long userId) {
+    @Transactional(readOnly = true)
+    public Optional<GithubRepository> findByExternalIdAndUserId(long externalId, long userId) {
         return repoJpaRepository.findByGithubRepositoryIdAndUserId(externalId, userId)
                 .map(GithubRepositoryEntity::toDomainEntity);
     }

76-80: 중복 로직 제거: get는 find를 재사용해 예외만 부여하는 형태로 통일
단일 코드 경로로 유지하면 유지보수성이 좋아지고 매핑 로직의 이중화가 사라집니다.

-    public GithubRepository getByExternalIdAndUserId(long externalId, long userId) {
-        return repoJpaRepository.findByGithubRepositoryIdAndUserId(externalId, userId)
-                .orElseThrow(() -> new GssException(ErrorCode.GITHUB_REPOSITORY_NOT_FOUND))
-                .toDomainEntity();
-    }
+    public GithubRepository getByExternalIdAndUserId(long externalId, long userId) {
+        return findByExternalIdAndUserId(externalId, userId)
+                .orElseThrow(() -> new GssException(ErrorCode.GITHUB_REPOSITORY_NOT_FOUND));
+    }
gss-domain/src/main/java/com/devoops/domain/repository/github/repo/GithubRepoDomainRepository.java (1)

22-25: find/get 책임 분리가 명확합니다
find*는 Optional, get*은 예외를 던지는 규약이 인터페이스 차원에서 명확해졌습니다. 도메인 전반에 같은 네이밍 규칙을 적용해 일관성을 유지해 주세요.

gss-common/src/main/java/com/devoops/util/CurrencyUtil.java (3)

9-11: 환율 하드코딩(1397.84)과 ‘25-08-21 기준’ 주석은 빠르게 부패합니다 — 구성/프로바이더로 분리 권장

실시간/일별 변동성이 큰 환율을 상수로 고정하면 모델 선택 임계값(7,500/12,500/15,000 KRW) 근처에서 잘못된 판단이 발생할 수 있습니다. 상수는 fallback으로만 두고, 호출자가 환율을 전달할 수 있는 오버로드를 추가하거나(권장), 별도의 ExchangeRateProvider(캐시 포함)로 분리해 주세요.

권장 최소 변경안(오버로드 추가):

+import java.math.BigDecimal;
+import java.math.RoundingMode;

-    private static final double CURRENCY_RATE = 1397.84;
+    private static final BigDecimal CURRENCY_RATE = new BigDecimal("1397.84");

+    /**
+     * 외부에서 주입받은 환율로 변환
+     */
+    public static BigDecimal usdToKrw(BigDecimal usd, BigDecimal rate) {
+        return usd.multiply(rate).setScale(0, RoundingMode.HALF_UP);
+    }

6-7: 유틸리티 클래스 관례 정리(미세 개선)

상속을 막기 위해 class를 final로 두는 것을 권장합니다. 또는 Lombok @UtilityClass로 대체 가능합니다.

-public class CurrencyUtil {
+public final class CurrencyUtil {

17-19: 경계 회귀 테스트 추가 제안

임계값(7,500/12,500/15,000 KRW) 근처에서 오판이 없는지 회귀 테스트를 제안합니다. 특히 BigDecimal 전환 전후 결과 비교가 유용합니다.

원하시면 테스트 케이스 초안(파라미터라이즈드 테스트)도 제공하겠습니다.

gss-domain/src/main/java/com/devoops/domain/entity/analysis/AiCharge.java (1)

6-13: 기간 표현 간소화(선택)

year+month 조합 대신 java.time.YearMonth를 사용하면 유효성·가독성이 좋아집니다. 캐시/Map 키로도 직관적입니다.

gss-client/gss-mcp-client/src/main/java/com/devoops/client/PromptBuilder.java (1)

17-22: 입력 null/빈값 방어 및 대용량 diff 처리 가드

title/description/diff가 null이면 NPE가 발생합니다. 또한 매우 큰 diff는 Base64로 33% 팽창하므로 토큰 한도를 초과할 수 있습니다. null 방어와 간단한 길이 가드를 권장합니다.

+import java.util.Objects;
 ...
     public String buildUserPrompt(String title, String description, String diff) {
-        return promptTemplate
-                .replace("{title}", title)
-                .replace("{description}", description)
-                .replace("{diff}", encodeDiff(diff));
+        String safeTitle = Objects.requireNonNullElse(title, "");
+        String safeDesc  = Objects.requireNonNullElse(description, "");
+        String safeDiff  = Objects.requireNonNullElse(diff, "");
+        // 필요 시 임계값(예: 200_000 chars) 초과 시 요약/자르기 로직 연동
+        return promptTemplate
+                .replace("{title}", safeTitle)
+                .replace("{description}", safeDesc)
+                .replace("{diff}", encodeDiff(safeDiff));
     }

원하시면 diff 요약/청크 분할 전략(파일 수/라인 수 기준)도 제안드리겠습니다.

gss-client/gss-mcp-client/src/main/java/com/devoops/dto/request/AnalyzePrRequest.java (1)

3-10: 입력 유효성 보강(Bean Validation)과 모델 타입 안정성

DTO 단계에서 유효성을 보장하면 하위 레이어 방어 코드가 줄어듭니다. 또한 model은 문자열보다 enum으로 제한하는 편이 안전합니다(허용값: GPT-5/GPT-5-mini/GPT-5-nano 등).

Bean Validation 예시:

 package com.devoops.dto.request;

+import jakarta.validation.constraints.NotBlank;
+
 public record AnalyzePrRequest(
-        String title,
-        String description,
-        String codeDifference,
-        String model
+        @NotBlank String title,
+        @NotBlank String description,
+        @NotBlank String codeDifference,
+        @NotBlank String model
 ) {
 }

모델 안정성 옵션:

  • client-common에 Model enum 신설 후 사용
  • 또는 문자열 유지 시, 허용값 화이트리스트 검증 추가
gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java (1)

23-27: 동시성(중복 생성) 경쟁 조건 가능성: (year, month) 신규 레코드 생성 시 레이스

두 스레드가 같은 (year, month)에 대해 동시에 최초 조회-생성을 수행하면, 유니크 제약이 없다면 중복이 생기고, 유니크 제약이 있다면 DataIntegrityViolationException이 발생할 수 있습니다. 월별 과금 집계 특성상 충분히 발생 가능한 시나리오입니다.

권장:

  • DB에 (year, month) 유니크 인덱스 추가.
  • getByMonth에서 미존재 시 save 실패(무결성 위반)하면 재조회로 복구하는 패턴 도입.

예시(요지):

return repo.findByYearAndMonth(year, month)
  .map(Entity::toDomain)
  .orElseGet(() -> {
    try {
      return repo.save(Entity.from(new AiCharge(year, month, BigDecimal.ZERO))).toDomain();
    } catch (DataIntegrityViolationException e) {
      return repo.findByYearAndMonth(year, month).orElseThrow(e::getMostSpecificCause).toDomain();
    }
  });

검증 제안:

  • (year, month) 유니크 인덱스 존재 여부 확인.
  • 부하 테스트에서 동시 최초 접근 시 예외/중복 없는지 확인.
gss-domain/src/main/java/com/devoops/domain/repository/analysis/AiChargeRepository.java (1)

7-10: 인터페이스 시그니처의 의미 명확화와 금액 타입 개선 제안

  • getByMonth(int month): “현재(Asia/Seoul) 기준 연도”를 내포한다는 점이 메서드명/시그니처만으로는 드러나지 않습니다. 주석 또는 네이밍으로 의도를 명확히 하세요(예: getByCurrentYearAndMonth(int month) 또는 Javadoc에 TZ/연도 규칙 명시).
  • addCharge(int month, double charge): 금액 타입에 double은 비권장이며, 의미도 누적(add)인지 설정(set)인지 모호합니다. BigDecimal deltaCharge로 교체하고 메서드명을 increaseCharge 등으로 바꾸는 것을 권장합니다.

Javadoc 예시(요지):

 public interface AiChargeRepository {
+    /**
+     * Asia/Seoul 타임존의 '현재 연도'와 전달된 {@code month}(1..12)를 키로
+     * 월별 과금 엔티티를 조회합니다. 없으면 0원으로 초기화하여 반환합니다.
+     * @throws IllegalArgumentException month가 1..12 범위를 벗어나면
+     */
     AiCharge getByMonth(int month);

-    void addCharge(int month, double charge);
+    /**
+     * Asia/Seoul 기준 '현재 연도'와 {@code month}(1..12)에 대해
+     * 금액을 누적 증가시킵니다.
+     */
+    void addCharge(int month, BigDecimal deltaCharge);
 }
gss-domain/src/test/java/com/devoops/domain/entity/analysis/OpenAiModelTest.java (1)

14-45: 경계값 바로 바깥값과 범위 겹침(inclusive/exclusive) 검증 케이스 보강 필요

현 테스트는 각 모델의 하한/상한 “정확히 그 값”만 검증합니다. 인접 구간 경계에서 ±1(또는 ±ε) 값, 전체 범위 밖의 값(음수/매우 큰 값), 경계 포함 규칙(상한 포함/미포함)에 대한 회귀가 없습니다. 과금에 따른 모델 전환에서는 경계 처리 오류가 치명적입니다.

보강 예시(추가 테스트 코드):

@ParameterizedTest
@CsvSource({
  // lowerBound-1는 더 낮은 티어여야 함
  "GPT_5_MINI",
  "GPT_5_NANO"
})
void 경계_하단_바깥값에서_더_낮은_티어를_선택한다(String modelName) {
    OpenAiModel m = OpenAiModel.valueOf(modelName);
    int candidate = m.getMoneyUnderCriteria() - 1;
    assertThat(OpenAiModel.getModelByUsage(candidate).ordinal())
        .isGreaterThanOrEqualTo(OpenAiModel.valueOf(modelName).ordinal()); // 요지: 더 낮은 티어
}

@ParameterizedTest
@CsvSource({
  // upperBound+1는 더 높은 티어여야 함
  "GPT_5_NANO",
  "GPT_5_MINI"
})
void 경계_상단_바깥값에서_더_높은_티어를_선택한다(String modelName) {
    OpenAiModel m = OpenAiModel.valueOf(modelName);
    int candidate = m.getMoneyUpperCriteria() + 1;
    assertThat(OpenAiModel.getModelByUsage(candidate).ordinal())
        .isLessThanOrEqualTo(OpenAiModel.valueOf(modelName).ordinal() - 1);
}

확인 요청:

  • getModelByUsage의 구간 정의가 [lower, upper] 모두 포함인지, 상한 미포함([lower, upper))인지 명확히 하고, 인접 구간과 겹치지 않도록 구현/테스트 합의가 필요합니다.
gss-client/gss-mcp-client/src/main/java/com/devoops/config/AiConfig.java (1)

16-23: JSON Schema 기반 구조화 출력에 비해 temperature=1.0은 변동성이 큼

구조화(JSON Schema) 출력을 기대하는 작업에서는 낮은 온도(예: 0.0~0.3)가 일반적으로 더 안정적입니다. 현재 1.0은 응답 다양성을 높여 스키마 불일치/파싱 실패를 유발할 수 있습니다.

권장 수정:

-                .reasoningEffort("medium")
-                .temperature(1.0);
+                .reasoningEffort("medium")
+                .temperature(0.2);
gss-mcp-app/src/test/resources/application.yml (1)

38-63: YAML trailing space 제거 필요

YAML 린트 에러가 보고되었습니다(예: Line 39, 40, 53, 56, 61). 불필요한 공백을 제거해 주세요.

수정 예:

   github-pr-analysis:
     system: |
-      당신은 10년차 시니어 개발자로서 아래 PR의 변경 내용을 분석하고, 기술 면접에서 사용할 수 있는 질문을 생성해.    
+      당신은 10년차 시니어 개발자로서 아래 PR의 변경 내용을 분석하고, 기술 면접에서 사용할 수 있는 질문을 생성해.
-      
+
       조건:
       - "summary"는 이 PR이 어떤 핵심 작업을 했는지 1문장으로 요약한 것
       - "summaryDetail"은 변경 내용을 항목별로 요약한 제목(title) + 설명(description) 쌍으로 구성해.
       - "category"는 기술적인 관점에서 PR 코드 변경 내용을 반영하여 선택해 (예: 성능, 보안, 확장성, 유지보수성, 테스트 등)
       - "question"은 각 category에 대해 기술 면접에서 사용할 수 있는 질문이어야 해.
       - 각 질문들은 반드시 PR 코드 변경 내용("diff")을 인용해서 생성해.
       - "diff"를 굉장히 자세하게 분석하고 몇몇 질문에는 코드를 반영해서 만들어줘
       - 질문 수는 카테고리마다 3개 이상 만들어.
       - 질문은 꼭 존댓말로 "?"로 끝나는 의문문이어야 해.
 
     prompt: |
-      당신은 숙련된 코드 리뷰어입니다.
-      
+      당신은 숙련된 코드 리뷰어입니다.
+
       PR 제목: {title}
-      PR 내용: {description}
-      
+      PR 내용: {description}
+
       PR 코드 변경 내용:
       --- BEGIN DIFF ---
       {diff}
       --- END DIFF ---
-      
+
       diff를 Base64에서 디코딩한 후 분석하고 PR 요약과 질문을 만들어 주세요.
gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClientImpl.java (2)

42-51: callChatResponse 시그니처 단순화 제안(가독성·변경 용이성)

메서드 인자를 AnalyzePrRequest 하나로 받으면 파라미터 확장 시 변경 범위를 줄일 수 있습니다. 현재 3개 문자열 + 옵션 인자보다 추적이 수월합니다.

예시:

-    private ChatResponse callChatResponse(String title, String description, String codeDifference,
-                                          ChatOptions options) {
-        String userPrompt = promptBuilder.buildUserPrompt(title, description, codeDifference);
+    private ChatResponse callChatResponse(AnalyzePrRequest request, ChatOptions options) {
+        String userPrompt = promptBuilder.buildUserPrompt(request.title(), request.description(), request.codeDifference());
         String systemPrompt = promptBuilder.buildSystemPrompt();
         return chatClient.prompt()
                 .options(options)
                 .system(systemPrompt)
                 .user(userPrompt)
                 .call()
                 .chatResponse();
     }

그리고 호출부:

- ChatResponse chatResponse = callChatResponse(
-     request.title(), request.description(), request.codeDifference(), openAiChatOptions);
+ ChatResponse chatResponse = callChatResponse(request, openAiChatOptions);

30-39: 제안: camelCase 네이밍 적용 및 ChatResponse 방어 코드 추가 권장

LLM 호출 결과가 null이거나 내부 필드가 비어 있을 경우 NPE 가능성이 있어, 변수명 수정과 함께 방어 코드를 더해 안정성을 확보하는 것을 권장드립니다.

  • 대상 파일

    • gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClientImpl.java (30–39행)
  • 제안 diff

-        ChatResponse chatresponse = callChatResponse(
+        ChatResponse chatResponse = callChatResponse(
                 request.title(),
                 request.description(),
                 request.codeDifference(),
                 openAiChatOptions
         );

-        Usage usage = chatresponse.getMetadata().getUsage();
-        String analysisResult = chatresponse.getResult().getOutput().getText();
-        return new AnalyzePrResponse(usage, analysisResult);
+        if (chatResponse == null
+                || chatResponse.getResult() == null
+                || chatResponse.getResult().getOutput() == null) {
+            throw new IllegalStateException("Empty ChatResponse from AI provider");
+        }
+        Usage usage = chatResponse.getMetadata() != null
+                ? chatResponse.getMetadata().getUsage()
+                : null;
+        String analysisResult = chatResponse.getResult().getOutput().getText();
+        return new AnalyzePrResponse(usage, analysisResult);
gss-mcp-app/src/main/java/com/devoops/event/QuestionEventListener.java (2)

49-57: 오탈자: pullRequestId 네이밍 수정

파라미터명이 pulLRequestId로 잘못 표기되어 있습니다. 컨벤션과 오타 모두 수정하세요.

-    private List<Question> createQuestionListFromCategorizedQuestions(
-            List<PrAnalysis.CategorizedQuestion> questions,
-            Long pulLRequestId
-    ) {
+    private List<Question> createQuestionListFromCategorizedQuestions(
+            List<PrAnalysis.CategorizedQuestion> questions,
+            Long pullRequestId
+    ) {
         return questions.stream()
                 .flatMap(taggedQuestion -> taggedQuestion.question()
                         .stream()
-                        .map(question -> new QuestionCreateCommand(pulLRequestId, taggedQuestion.category(), question))
+                        .map(question -> new QuestionCreateCommand(pullRequestId, taggedQuestion.category(), question))
                 )
                 .map(QuestionCreateCommand::toDomainEntity)
                 .toList();
     }

27-47: @async 이벤트 핸들러의 예외 전파·로깅 보강

비동기 이벤트에서 예외가 터지면 호출자 문맥에서 사라지고, 후속 저장 로직이 스킵될 수 있습니다. 최소한 로깅 및 실패 시그널링(예: 상태 갱신) 처리를 고려하세요.

 @Async
 @EventListener(QuestionCreateEvent.class)
 public void createQuestion(QuestionCreateEvent questionCreateEvent) {
-    AppWebhookEventRequest request = questionCreateEvent.getRequest();
-    GithubToken githubToken = questionCreateEvent.getToken();
-    PullRequest readyPullRequest = questionCreateEvent.getInitializedPullRequest();
-
-    AdaptedAnalyzePrResponse adaptedAnalyzePrResponse = prAnalysisService.analyzePullRequest(request, githubToken);
-
-    PullRequest updatedPullRequest = pullRequestService.updateAnalyzeResult(
-            readyPullRequest.getId(),
-            adaptedAnalyzePrResponse.summary(),
-            adaptedAnalyzePrResponse.detailSummary()
-    );
-
-    List<Question> createdQuestions = createQuestionListFromCategorizedQuestions(
-            adaptedAnalyzePrResponse.questions(), updatedPullRequest.getId()
-    );
-    questionService.saveAll(createdQuestions, updatedPullRequest.getId());
+    try {
+        AppWebhookEventRequest request = questionCreateEvent.getRequest();
+        GithubToken githubToken = questionCreateEvent.getToken();
+        PullRequest readyPullRequest = questionCreateEvent.getInitializedPullRequest();
+
+        AdaptedAnalyzePrResponse adaptedAnalyzePrResponse = prAnalysisService.analyzePullRequest(request, githubToken);
+
+        PullRequest updatedPullRequest = pullRequestService.updateAnalyzeResult(
+                readyPullRequest.getId(),
+                adaptedAnalyzePrResponse.summary(),
+                adaptedAnalyzePrResponse.detailSummary()
+        );
+
+        List<Question> createdQuestions = createQuestionListFromCategorizedQuestions(
+                adaptedAnalyzePrResponse.questions(), updatedPullRequest.getId()
+        );
+        if (!createdQuestions.isEmpty()) {
+            questionService.saveAll(createdQuestions, updatedPullRequest.getId());
+        }
+    } catch (Exception e) {
+        // TODO: 에러 처리 정책에 맞춰 재시도/상태 업데이트/경고 이벤트 발행 등 고려
+        // 로그 프레임워크 주입(@Slf4j) 후 사용 권장
+        e.printStackTrace();
+    }
 }

참고: 로그 사용을 위해 클래스에 @Slf4j 추가를 권장합니다.

gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java (2)

4-6: 불필요한 import 정리

사용되지 않는 Answer, AnswerEntity, LocalDate*, ZoneId import가 있습니다. 유지 시 혼동을 유발합니다.

-import com.devoops.domain.entity.github.answer.Answer;
-import com.devoops.jpa.entity.github.answer.AnswerEntity;
...
-import java.time.LocalDate;
-import java.time.LocalDateTime;
-import java.time.ZoneId;

Also applies to: 12-15


31-38: NOT NULL 제약 명시

DB 스키마 수준에서 누락을 방지하기 위해 nullable = false를 권장합니다.

-    @Column(name = "charge_year")
+    @Column(name = "charge_year", nullable = false)
     private int year;

-    @Column(name = "charge_month")
+    @Column(name = "charge_month", nullable = false)
     private int month;

-    private double charge;
+    @Column(nullable = false)
+    private double charge;
gss-client/gss-mcp-client/src/main/java/com/devoops/dto/response/PrAnalysis.java (2)

7-11: LLM JSON 변화 내성 강화: 알 수 없는 필드 무시

응답 스키마가 진화할 수 있으므로, 역직렬화 시 알 수 없는 필드를 무시하면 안정성이 높아집니다.

-public record PrAnalysis(
+@JsonIgnoreProperties(ignoreUnknown = true)
+public record PrAnalysis(

필요 import:

+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;

18-21: 복수형 키와의 호환성 확보(@JsonAlias)

외부/프롬프트에서 questions로 내려올 가능성에 대비해 alias를 추가하면 내성이 좋아집니다. 기존 필드명은 유지합니다.

-    public record CategorizedQuestion(
-            @JsonProperty(required = true) String category,
-            @JsonProperty(required = true) List<String> question
-    ) {
+    public record CategorizedQuestion(
+            @JsonProperty(required = true) String category,
+            @JsonProperty(required = true) @JsonAlias({"questions"}) List<String> question
+    ) {

필요 import:

+import com.fasterxml.jackson.annotation.JsonAlias;
gss-mcp-app/src/test/java/com/devoops/fake/FakePrAnalysisClient.java (1)

14-42: MOCK_RESPONSE는 불변으로 두는 편이 안전합니다

테스트 더블의 공유 상태가 변경되지 않도록 public static final로 선언하는 것을 권장합니다. 또한 내부 리스트는 List.of(...)를 사용하면 의도치 않은 변경을 방지할 수 있습니다.

적용 예:

-    public static PrAnalysis MOCK_RESPONSE = new PrAnalysis(
+    public static final PrAnalysis MOCK_RESPONSE = new PrAnalysis(
@@
-                            Arrays.asList(
+                            List.of(
@@
-                            Arrays.asList(
+                            List.of(
gss-mcp-app/src/main/java/com/devoops/adaptor/PrAnalysisAdapter.java (1)

16-20: model 파라미터의 유효성 검증 추가 제안

외부로 전달되는 모델명이 null/blank면 하류 호출 실패로 이어질 수 있습니다. 최소한의 선행 검증을 추가하는 것을 권장합니다.

예시:

@@
-    public AdaptedAnalyzePrResponse analyze(String title, String description, String diff, String model) {
-        AnalyzePrRequest analyzePrRequest = new AnalyzePrRequest(title, description, diff, model);
+    public AdaptedAnalyzePrResponse analyze(String title, String description, String diff, String model) {
+        if (model == null || model.isBlank()) {
+            throw new IllegalArgumentException("model must not be null/blank");
+        }
+        AnalyzePrRequest analyzePrRequest = new AnalyzePrRequest(title, description, diff, model);
         AnalyzePrResponse analyzePrResponse = prAnalysisClient.analyze(analyzePrRequest);
         return new AdaptedAnalyzePrResponse(analyzePrResponse);
     }
gss-mcp-app/src/main/java/com/devoops/service/pranalysis/PrAnalysisService.java (3)

24-41: 트랜잭션 경계 재검토 권장(외부 호출 포함 여부 주의)

AI 호출(네트워크 I/O)과 DB 갱신을 한 트랜잭션에 묶지 말고, 과금 갱신 구간만 트랜잭션으로 감싸는 편이 안전합니다. 재시도 시 중복 과금 방지(멱등성) 전략도 함께 고려해 주세요.

예시(서비스 계층):

  • analyzePullRequest(...)는 외부 호출 수행
  • 과금 반영은 @Transactional 메서드 applyCharge(year, month, consumedCharge)로 분리

24-41: 초기 월 과금 레코드 미존재 시 처리 누락 가능성

getByMonth가 레코드 미존재 시 예외/Optional을 어떻게 반환하는지에 따라 NPE 또는 갱신 누락이 발생할 수 있습니다. 월 최초 호출 시 레코드 생성(업서트) 로직이 필요합니다.

원하시면 업서트 형태의 도메인 레포지토리 구현/테스트 코드까지 같이 제안드릴게요.


38-39: 금액 계산의 반올림/단위 보장

getChargedouble을 반환하여 누적 시 미세한 부동소수 오차가 축적될 수 있습니다. 최소한 KRW 단위로 반올림(예: 원단위 반올림) 후 정수형으로 저장하는 것을 권장합니다.

gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeJpaRepository.java (2)

17-22: and ai_charge.charge >= 0 조건의 의도 확인

현재 금액이 음수인 레코드는 갱신되지 않습니다. 초기값(-1 등)을 사용하는 경우 첫 갱신이 영원히 적용되지 않을 수 있습니다. 의도된 방어 로직인지 확인해 주세요. 필요 없다면 조건 제거를 권장합니다.

가능 변경:

-            where ai_charge.year = :year
-                        and ai_charge.month = :month
-                        and ai_charge.charge >= 0
+            where ai_charge.year = :year
+              and ai_charge.month = :month

13-14: 파라미터 명은 의미 기반으로 단순화 권장

메서드 파라미터명 todayYear, todayMonth는 구현 세부(현재 시점)를 노출합니다. year, month로 단순화하는 편이 가독성에 유리합니다.

gss-domain/src/main/java/com/devoops/domain/entity/analysis/OpenAiModel.java (3)

23-29: 모델 선택의 결정성 보장 및 경계 외 값 처리 보강

  • findAny() 대신 findFirst()를 사용해 결정성을 높이세요(스트림 순서 기반).
  • 음수 사용량은 입력 오류로 보고 최상위 품질(GPT_5)을 기본값으로 반환하는 처리를 권장합니다. (15,000원 초과 시 GPT_5_NANO 유지 정책은 그대로)

예시:

     public static OpenAiModel getModelByUsage(double currentUsageWon) {
-        return Stream.of(values())
+        if (currentUsageWon < 0) {
+            return GPT_5;
+        }
+        return Stream.of(values())
                 .filter(model -> model.moneyUnderCriteria <= currentUsageWon
                         && model.moneyUpperCriteria >= currentUsageWon)
-                .findAny()
+                .findFirst()
                 .orElse(GPT_5_NANO);
     }

31-35: 금액 계산 시 반올림/정밀도 관리

KRW는 분(cent) 단위가 없으므로 원 단위 반올림(또는 올림) 후 정수 저장을 권장합니다. 현재 double 합산은 부동소수 오차가 누적될 수 있습니다. 도메인 리턴 타입을 long(원)으로 올리는 것도 고려해 주세요.


17-22: 필드 네이밍 가독성 개선 제안

moneyUnderCriteria/moneyUpperCriteria 대신 lowerBoundInclusive/upperBoundInclusive 같은 의미 중심 이름이 가독성에 더 좋습니다. 국제화 주석(//원, //달러)은 유지하되 상수/필드 명만 개선을 검토해 주세요.

gss-client/gss-mcp-client/src/main/resources/application-mcp-client.yml (2)

3-15: system 프롬프트 행 끝 공백 제거 및 문구 정리

YAMLlint가 지적한 트레일링 스페이스가 존재합니다(라인 4, 5). 또한 문장 끝 공백은 VCS diff를 불필요하게 키울 수 있습니다. 아래처럼 정리해 주세요.

-    system: |
-      당신은 10년차 시니어 개발자로서 아래 PR의 변경 내용을 분석하고, 기술 면접에서 사용할 수 있는 질문을 생성해.    
-      
+    system: |
+      당신은 10년차 시니어 개발자로서 아래 PR의 변경 내용을 분석하고, 기술 면접에서 사용할 수 있는 질문을 생성해.
+
       조건:
       - "summary"는 이 PR이 어떤 핵심 작업을 했는지 1문장으로 요약한 것
       - "summaryDetail"은 변경 내용을 항목별로 요약한 제목(title) + 설명(description) 쌍으로 구성해.
       - "category"는 기술적인 관점에서 PR 코드 변경 내용을 반영하여 선택해 (예: 성능, 보안, 확장성, 유지보수성, 테스트 등)
       - "question"은 각 category에 대해 기술 면접에서 사용할 수 있는 질문이어야 해.
       - 각 질문들은 반드시 PR 코드 변경 내용("diff")을 인용해서 생성해.
       - "diff"를 굉장히 자세하게 분석하고 몇몇 질문에는 코드를 반영해서 만들어줘
       - 질문 수는 카테고리마다 3개 이상 만들어.
       - 질문은 꼭 존댓말로 "?"로 끝나는 의문문이어야 해.

31-38: 로컬 프로필의 OpenAI API Key 하드코딩

spring.ai.openai.api-key: testKey는 하드코딩되어 있으나, 테스트 편의 외에는 환경 변수/프로퍼티로 주입하는 구성이 더 안전합니다. CI 환경에서의 실수 방지를 위해 로컬 전용 예시로 유지하되, 샘플 값은 .example 파일로 안내하는 방식을 고려해 주세요.

gss-mcp-app/src/test/java/com/devoops/PrAnalysisClientImplTest.java (3)

10-12: 통합 테스트 안정성: 프로파일/키 부재 시 실패 가능

@SpringBootTest만 지정되어 있어 테스트 환경에서 OpenAI 키가 주입되지 않으면 실패할 수 있습니다. 통합 테스트임을 명시하고 선택적으로만 실행되도록 하는 것이 안전합니다.

예시:

-@SpringBootTest
+@SpringBootTest
+@Tag("integration")
+@Disabled("LLM 의존 통합 테스트 - 필요 시에만 실행하세요")

또는 환경 변수를 이용해 조건부 실행:

Assumptions.assumeTrue("1".equals(System.getenv("RUN_LLM_TESTS")), "LLM 테스트 비활성화");

36-44: 모델 선택 하드코딩은 PR 목적과 상충 — 동적 모델 선택 검증 테스트 추가 제안

PR 목적(월 과금 한도/구간에 따른 GPT-5/mini/nano 동적 선택)을 고려하면, 여기에서 모델을 "gpt-5-nano"로 직접 지정하는 대신, 모델 선택 서비스(또는 전략)가 기대대로 동작하는지 경계값(7,500/12,500/15,000 KRW) 주변에서 선택 모델을 검증하는 테스트가 필요합니다.

원하시면, 비용 산정 리포지토리를 목킹하여 아래 3가지 케이스(<=7,500, <=12,500, >12,500~15,000, 초과 시 강등 등)를 커버하는 테스트 템플릿을 드리겠습니다.


53-942: 초장문 diff 픽스처는 리소스로 외부화하여 테스트 가독성/성능 개선

테스트 코드에 수백 라인 문자열 리터럴을 직접 포함하면 가독성과 유지보수성이 저하됩니다. src/test/resources/fixtures/pr_diff_example.txt 등으로 옮기고 파일로부터 로드하세요.

예시(추가 코드):

private String example() {
    try {
        return java.nio.file.Files.readString(
            java.nio.file.Path.of("src/test/resources/fixtures/pr_diff_example.txt"),
            java.nio.charset.StandardCharsets.UTF_8
        );
    } catch (IOException e) {
        throw new UncheckedIOException(e);
    }
}

또한 프롬프트에서 Base64 디코딩을 요구하지 않도록 정리했다면, 평문 diff를 그대로 사용하세요. Base64를 유지한다면 여기서 인코딩/디코딩 책임을 명확히 하셔야 합니다.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f4604e3 and b7265e0.

📒 Files selected for processing (36)
  • gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java (3 hunks)
  • gss-api-app/src/test/java/com/devoops/service/facade/RepositoryFacadeServiceTest.java (2 hunks)
  • gss-api-app/src/test/java/com/devoops/service/repository/RepositoryServiceTest.java (3 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClient.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClientImpl.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/client/PromptBuilder.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/config/AiConfig.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/dto/request/AnalyzePrRequest.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/dto/response/AnalyzePrResponse.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/dto/response/PrAnalysis.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/etc/OpenaiPropertiesLogger.java (0 hunks)
  • gss-client/gss-mcp-client/src/main/resources/application-mcp-client.yml (1 hunks)
  • gss-common/src/main/java/com/devoops/util/CurrencyUtil.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/domain/entity/analysis/AiCharge.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/domain/entity/analysis/OpenAiModel.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/domain/entity/github/repo/GithubRepository.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/domain/repository/analysis/AiChargeRepository.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/domain/repository/github/repo/GithubRepoDomainRepository.java (2 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/entity/github/repo/GithubRepositoryEntity.java (0 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeJpaRepository.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/repository/github/repo/GithubRepoDomainRepositoryImpl.java (2 hunks)
  • gss-domain/src/main/java/com/devoops/service/repository/RepositoryService.java (2 hunks)
  • gss-domain/src/test/java/com/devoops/domain/entity/analysis/OpenAiModelTest.java (1 hunks)
  • gss-domain/src/testFixtures/java/com/devoops/generator/GithubRepoGenerator.java (1 hunks)
  • gss-mcp-app/src/main/java/com/devoops/adaptor/PrAnalysisAdapter.java (2 hunks)
  • gss-mcp-app/src/main/java/com/devoops/dto/request/AdaptedAnalyzePrResponse.java (1 hunks)
  • gss-mcp-app/src/main/java/com/devoops/event/QuestionCreateEvent.java (0 hunks)
  • gss-mcp-app/src/main/java/com/devoops/event/QuestionEventListener.java (4 hunks)
  • gss-mcp-app/src/main/java/com/devoops/service/pranalysis/PrAnalysisService.java (1 hunks)
  • gss-mcp-app/src/main/java/com/devoops/service/webhook/WebhookFacadeService.java (2 hunks)
  • gss-mcp-app/src/main/resources/application-local.yml (1 hunks)
  • gss-mcp-app/src/test/java/com/devoops/PrAnalysisClientImplTest.java (2 hunks)
  • gss-mcp-app/src/test/java/com/devoops/fake/FakePrAnalysisClient.java (2 hunks)
  • gss-mcp-app/src/test/resources/application.yml (1 hunks)
💤 Files with no reviewable changes (3)
  • gss-mcp-app/src/main/java/com/devoops/event/QuestionCreateEvent.java
  • gss-domain/src/main/java/com/devoops/jpa/entity/github/repo/GithubRepositoryEntity.java
  • gss-client/gss-mcp-client/src/main/java/com/devoops/etc/OpenaiPropertiesLogger.java
🧰 Additional context used
🧬 Code graph analysis (3)
gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java (1)
gss-domain/src/main/java/com/devoops/domain/entity/analysis/AiCharge.java (1)
  • Getter (6-13)
gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClientImpl.java (1)
gss-mcp-app/src/main/java/com/devoops/adaptor/PrAnalysisAdapter.java (1)
  • Component (10-21)
gss-mcp-app/src/main/java/com/devoops/dto/request/AdaptedAnalyzePrResponse.java (1)
gss-common/src/main/java/com/devoops/util/SummaryFormatter.java (1)
  • SummaryFormatter (6-18)
🪛 YAMLlint (1.37.1)
gss-mcp-app/src/test/resources/application.yml

[error] 39-39: trailing spaces

(trailing-spaces)


[error] 40-40: trailing spaces

(trailing-spaces)


[error] 53-53: trailing spaces

(trailing-spaces)


[error] 56-56: trailing spaces

(trailing-spaces)


[error] 61-61: trailing spaces

(trailing-spaces)

gss-client/gss-mcp-client/src/main/resources/application-mcp-client.yml

[error] 4-4: trailing spaces

(trailing-spaces)


[error] 5-5: trailing spaces

(trailing-spaces)


[error] 18-18: trailing spaces

(trailing-spaces)


[error] 21-21: trailing spaces

(trailing-spaces)


[error] 26-26: trailing spaces

(trailing-spaces)

🔇 Additional comments (23)
gss-mcp-app/src/main/resources/application-local.yml (1)

5-14: DB/JPA 자동설정 미적용 확인 – Local 프로필 주석 처리 영향 없음

gss-mcp-app 모듈에 JPA/JDBC 관련 의존성이 포함되어 있지 않아, Local 프로필에서 주석 처리된 datasource/jpa 설정은 실제로 자동설정에 영향을 주지 않습니다.

  • gss-mcp-app/build.gradle에서 spring-boot-starter-data-jpa 또는 spring-jdbc 의존성을 선언하지 않음
  • 포함된 의존성은 spring-boot-starter-web, spring-boot-starter-data-redis 등으로, DataSourceAutoConfiguration 조건(EmbeddedDatabaseType 클래스 등)을 만족하지 않음
  • 따라서 application-local.yml에 남아 있는 주석 처리된 DB·JPA 설정은 적용되지 않으며, 기동 실패 우려가 없습니다

→ 주석 처리된 설정은 유지하거나, 코드 청결을 위해 필요 시 제거해도 무방합니다.

Likely an incorrect or invalid review comment.

gss-mcp-app/src/main/java/com/devoops/service/webhook/WebhookFacadeService.java (2)

35-36: 예외 일관성 개선 LGTM

findBy...에서 getByExternalIdAndUserId로 전환해 “없음” 상황을 예외로 일관 처리하는 방향 좋습니다. 호출부에서 NPE/Optional 누락 리스크가 줄어듭니다.


52-52: 조회 흐름 통일 LGTM

savePullRequest에서도 동일하게 getByExternalIdAndUserId 사용으로 에러 처리 일관성 확보된 점 좋습니다.

gss-api-app/src/test/java/com/devoops/service/repository/RepositoryServiceTest.java (2)

8-12: 필요한 의존성 import 적절

서비스 레벨 저장 테스트 도입에 따라 RepositoryCreateCommand, GithubRepository import 추가 적절합니다.


146-147: 포매팅 분리만 수행됨 — OK

의미 변화 없이 가독성 향상을 위한 줄바꿈만 발생했습니다.

gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java (1)

50-56: 중복 저장 사전 차단 흐름 LGTM
Optional 기반 사전 조회 후 재연결 여부를 분기하는 접근이 명확합니다. 이 로직 덕분에 서비스 레이어의 단순 저장을 유지하면서도 중복 등록을 방지할 수 있습니다.

gss-domain/src/main/java/com/devoops/domain/repository/github/repo/GithubRepoDomainRepository.java (1)

6-6: 검증 완료: 도메인 레이어에 더 이상 제거된 메서드 호출 없음

스크립트 검증 결과, 도메인(repository) 인터페이스 변경에 따른

  1. 과거 findByExternalId(…) 호출
  2. non-Optional 시그니처(getByExternalIdAndUserId(…))
    에 대한 잔존 사용처는 도메인 레이어에는 존재하지 않습니다.
  • Optional 반환 메서드(findByExternalIdAndUserId)와 예외 기반 접근 메서드(getByExternalIdAndUserId)는 모두 도메인 인터페이스에 정의된 대로 사용 중이며, 호출부(RepositoryService, WebhookFacadeService 등)도 정상 동작합니다.
  • 스크립트에서 감지된 findByExternalId 호출은 웹훅용 레지스트리 카운트 모듈(gss-domain/.../GithubRepoRegistryCountJpaRepositoryGithubRepoRegistryCountRepositoryImpl)의 JPA 리포지토리이며, 이번 인터페이스 변경 대상이 아닙니다.

따라서 도메인 레이어에 남아있는 불필요한 메서드 호출은 없으므로, 이 부분은 그대로 승인합니다.

gss-domain/src/main/java/com/devoops/domain/entity/analysis/AiCharge.java (1)

6-13: 용도 확인 요청: equals/hashCode 필요 여부

동일 월 레코드를 Set/Map 키로 사용할 계획이면 equals/hashCode 정의를 고려해 주세요(현재는 Lombok 기본 미생성).

gss-client/gss-mcp-client/src/main/java/com/devoops/client/PromptBuilder.java (2)

28-30: Base64 인코딩 구현은 단순·안전 — LGTM


11-16: 설정 파일에 프로퍼티 미정의 확인 요망
설정 파일(application*.yml/.properties)에서 dev-oops.github-pr-analysis.promptdev-oops.github-pr-analysis.system 프로퍼티가 정의되어 있지 않아, 런타임 buildUserPrompt 호출 시 NullPointerException 발생 위험이 있습니다.

아래 중 한 가지 방안을 적용해 주세요:

  • 옵션 A: 기본값 지정
     @Value("${dev-oops.github-pr-analysis.prompt}")
     private String promptTemplate;
    - // ↓ 기본값 빈 문자열 추가
    + @Value("${dev-oops.github-pr-analysis.prompt:}")
     @Value("${dev-oops.github-pr-analysis.system}")
     private String systemPrompt;
    + @Value("${dev-oops.github-pr-analysis.system:}")
  • 옵션 B: 애플리케이션 시작 시 검증
    import jakarta.annotation.PostConstruct;
    import org.springframework.util.Assert;
    …
    @PostConstruct
    void validate() {
        Assert.hasText(promptTemplate, "'dev-oops.github-pr-analysis.prompt' must not be empty");
        Assert.hasText(systemPrompt,  "'dev-oops.github-pr-analysis.system' must not be empty");
    }

• 다시 한 번 설정 파일 내 해당 키 존재 여부를 확인해 주세요.

gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClient.java (1)

8-9: B/C 변경: PrAnalysisClient 인터페이스 시그니처 변경에 따른 호출부 전수 검증 필요

인터페이스를

AnalyzePrResponse analyze(AnalyzePrRequest request);

로 변경하면서, 기존에 사용하던

analyze(String title, String description, String diff)

호출부가 모두 갱신되었는지 스크립트 점검을 통해 확인했으나, 실제 코드베이스 전반을 100% 보장하지 못할 수 있습니다. 아래 항목을 수동으로 검증해주세요.

  • PrAnalysisClientImpl(실제 구현체)와 FakePrAnalysisClient(테스트용 구현체)는 정상적으로 업데이트됨을 확인
  • 그 외 서비스/컨트롤러/유틸 등에서 analyze(title, description, diff) 형태의 호출이 남아있는지 전수 점검
  • 호출부가 많아 일괄 수정이 부담된다면, default 메서드 추가를 통해 하위 호환(Back-Compatibility) 완화 가능

optional refactor 예시:

 public interface PrAnalysisClient {
     AnalyzePrResponse analyze(AnalyzePrRequest request);
+
+    default AnalyzePrResponse analyze(String title, String description, String diff) {
+        return analyze(new AnalyzePrRequest(title, description, diff, null));
+    }
 }

빠른 점검 커맨드(다시 한번 실행 권장):

# 구현체 리스트
rg -nP '(class|record)\s+\w+\s+implements\s+PrAnalysisClient' -g '!**/target/**' -C2

# 호출부(3-arg 형식) 검색
rg -nP '\banalyze\s*\([^)]*,[^)]*,[^)]*\)' -g '!**/target/**'
gss-client/gss-mcp-client/src/main/java/com/devoops/config/AiConfig.java (2)

18-23: ResponseFormat(JSON_SCHEMA) 사용 방식 호환성 확인 필요

Spring AI/OpenAI의 최신 스펙에서는 json_schema 타입에 name 필드를 요구하는 경우가 있습니다. 현재 코드 시그니처(new ResponseFormat(Type.JSON_SCHEMA, outputJsonSchema()))가 런타임에서 적법한 페이로드를 생성하는지 확인이 필요합니다. 모델별(특히 경량/나노) 지원 여부도 다를 수 있습니다.

확인 항목:

  • 실제 전송되는 요청 바디에 response_format: { type: 'json_schema', json_schema: { name: '...', schema: { ... }}} 형태가 포함되는지.
  • 사용 모델군(GPT-5/mini/nano)에서 reasoningEffortjson_schema가 모두 지원되는지.

30-33: openAiChatBuilder 빈의 소비처 확인

OpenAiChatOptions.Builder 빈이 실제로 주입되어 옵션이 적용되는지 확인이 필요합니다. 현재 ChatClientOpenAiChatModel로부터 생성되는데, 기본 옵션/모델은 프로퍼티나 호출 시점에 지정해야 반영됩니다.

확인 항목:

  • PR 내에서 openAiChatBuilder()가 주입되어 model/responseFormat/temperature가 실제 요청에 적용되는지.
  • 테스트 프로파일에서 기본 모델 미지정 시, 동적으로 모델을 매번 지정하는지(또는 기본값이 설정되어 있는지).
gss-mcp-app/src/main/java/com/devoops/dto/request/AdaptedAnalyzePrResponse.java (1)

20-29: DTO 변환 흐름 명확하고 일관적 — 토큰/분석 결과 매핑 적절

AnalyzePrResponse로부터 토큰/요약/세부 요약/질문을 일관되게 추출하고 있어 어댑터 역할을 잘 수행합니다.

gss-mcp-app/src/test/java/com/devoops/fake/FakePrAnalysisClient.java (2)

3-7: 새 요청/응답 타입으로의 전환은 일관적입니다

AnalyzePrRequest 기반 시그니처로의 전환과 PrAnalysis 사용으로의 정렬이 테스트 더블에도 잘 반영되었습니다. 상호 작용의 의도(요약/질문 카테고리)도 유지되었습니다.


45-52: AnalyzePrResponse 생성자 파라미터 순서 확인 완료

테스트 더블에서 사용된

new AnalyzePrResponse(100, 100, 200, MOCK_RESPONSE)

의 각 숫자는 record 선언부

public record AnalyzePrResponse(
    int promptTokens,
    int completionTokens,
    int totalTokens,
    PrAnalysis prAnalysis
) { … }

와 정확히 일치합니다.
따라서 추가 수정 없이 그대로 사용하셔도 무방합니다.

gss-mcp-app/src/main/java/com/devoops/adaptor/PrAnalysisAdapter.java (1)

16-20: DTO 위임으로의 단순화는 적절합니다

어댑터가 포맷팅 로직을 제거하고 DTO 생성/위임만 담당하도록 정리된 점 좋습니다. 테스트 및 유지보수성이 향상됩니다.

gss-mcp-app/src/main/java/com/devoops/service/pranalysis/PrAnalysisService.java (1)

24-41: 설계 의도에 따른 추상화 적용 — 서비스 레이어 변경 불필요

  • AiChargeRepository 인터페이스에는 getByMonth(int month)addCharge(int month, double)만 선언되어 있으며, 구현체(AiChargeRepositoryImpl)에서 내부적으로 서울 시간대의 현재 연도를 구해
    • JPA 리포지토리의 findByYearAndMonth(year, month)
    • updateChargeById(year, month, charge)
      를 호출하도록 처리하고 있습니다.
  • 따라서 서비스 레이어(PrAnalysisService)에서 연도를 직접 관리하거나 메서드 시그니처를 변경할 필요 없이, 도메인 레포지토리의 추상화를 존중해 기존 코드를 그대로 유지하시면 됩니다.

Likely an incorrect or invalid review comment.

gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeJpaRepository.java (1)

13-27: 메서드명 및 반환값 수정, 트랜잭션 적용 검증 필요

아래 사항을 반영하고, 호출 경로(Service 계층 등)에 @Transactional이 적용돼 있는지 수동으로 확인해주세요.

  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeJpaRepository.java
    • 메서드명 updateChargeByIdupdateChargeByYearAndMonth로 변경
    • 반환타입 voidint (갱신된 행 수)로 변경
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java
    chargeJpaRepository.updateChargeById(...) 호출부가 트랜잭션 내에서 실행되는지 확인 (해당 클래스/메서드에는 @Transactional이 없음)

권장 변경 예시:

-    @Modifying(clearAutomatically = true, flushAutomatically = true)
-    @Query("""
-            update AiChargeEntity ai_charge
-            set ai_charge.charge = ai_charge.charge + :charge
-            where ai_charge.year = :year
-                and ai_charge.month = :month
-                and ai_charge.charge >= 0
-            """)
-    void updateChargeById(
+    @Modifying(clearAutomatically = true, flushAutomatically = true)
+    @Query("""
+            update AiChargeEntity ai_charge
+            set ai_charge.charge = ai_charge.charge + :charge
+            where ai_charge.year = :year
+                and ai_charge.month = :month
+                and ai_charge.charge >= 0
+            """)
+    int updateChargeByYearAndMonth(
         @Param("year") int year,
         @Param("month") int month,
         @Param("charge") double charge
     );

위 변경 후에도 호출부(Service/도메인)에서 반드시 @Transactional 어노테이션으로 감싸져야 합니다.

gss-domain/src/main/java/com/devoops/domain/entity/analysis/OpenAiModel.java (1)

12-15: 요금 구간 배치는 PR 목표와 부합합니다

사용량 증가에 따라 GPT_5 → GPT_5_MINI → GPT_5_NANO로 단계적으로 다운그레이드하는 구간 설정(07,500 / 7,50112,500 / 12,501~15,000원)이 목적과 일치합니다.

gss-client/gss-mcp-client/src/main/java/com/devoops/dto/response/AnalyzePrResponse.java (1)

14-21: Usage null 안전성 및 박싱(NPE) 방지 확인 요청

아래 사항을 반드시 검토해 주세요:

  • PrAnalysisClientImpl.java 39행의 new AnalyzePrResponse(usage, analysisResult) 호출부

    • usagechatresponse.getMetadata().getUsage()로 받아오며 이 값 자체는 기본 구현(EmptyUsage)으로 null이 되지 않지만,
    • 생성자 내부에서 usage.getPromptTokens(), usage.getCompletionTokens(), usage.getTotalTokens() 호출 시 반환값(Integer 혹은 Long)이 boxed 타입이므로 null일 경우 언박싱 시 NullPointerException이 발생할 수 있습니다.
  • Spring AI 의존성 버전 확인 및 문서 참조

    • pom.xml(또는 build.gradle) 에서 사용 중인 Spring AI 버전을 확인하세요.
    • 해당 버전의 Usage 인터페이스 메서드(getPromptTokens(), getCompletionTokens() 등) 반환 타입과 null 가능성을 공식 문서(예: Spring AI Reference)에서 검토해야 합니다.
  • 대응 방안 (필요 시)

    • Usage 반환값이 null인 경우 0으로 default 처리(additional null check).
    • 또는 AnalyzePrResponse 필드 타입을 boxed 타입(Integer/Long)으로 변경해 null 허용 여부를 명시적으로 관리.
gss-client/gss-mcp-client/src/main/resources/application-mcp-client.yml (2)

16-28: 입력 포맷 불일치: Base64 디코딩 지시 vs 실제 테스트 입력은 일반 텍스트

프롬프트는 “diff를 Base64에서 디코딩한 후 분석”을 요구하지만, PrAnalysisClientImplTest에서는 평문 diff 문자열을 그대로 전달합니다. 모델에게 Base64 디코딩을 시키는 접근은 실패/왜곡 가능성이 높아 권장되지 않습니다. 서버(클라이언트) 측에서 확실히 디코딩하여 평문을 모델에 전달하거나, 프롬프트 문구에서 Base64 관련 지시를 제거해 주세요.

다음 두 가지 중 하나로 정합성을 맞추는 것을 권장합니다.

  • 서버/클라이언트에서 미리 Base64 디코딩 수행 → 프롬프트에서 Base64 언급 제거:
-      diff를 Base64에서 디코딩한 후 분석하고 PR 요약과 질문을 만들어 주세요.
+      위 diff를 분석하여 PR 요약과 질문을 만들어 주세요.
  • 또는 테스트에서 diff를 Base64로 인코딩해 전달(권장하지 않음. 모델에 디코딩을 의존하지 마세요).

41-46: dev 프로필에서 AWS Secrets import 사용 시 부트 순서/오류 전파 확인

spring.config.import: 'aws-secretsmanager:dev/devoops/secrets' 사용 시 해당 시크릿이 없거나 권한이 부족하면 부트가 실패합니다. dev 환경에서만 활성화되는지, 필요한 키들(예: spring.ai.openai.api-key)이 실제로 주입되는지 확인 부탁드립니다. 필요하면 optional: 접두를 붙여 부팅 실패를 방지할 수 있습니다.

-    import: 'aws-secretsmanager:dev/devoops/secrets'
+    import: 'optional:aws-secretsmanager:dev/devoops/secrets'

Comment on lines 17 to 19
public static double usdToKrw(double usd) {
return usd * CURRENCY_RATE;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

double 사용과 반올림 정책 부재로 임계값 경계에서 모델 선택 오판 가능

현재 double 연산은 부동소수 오차로 인해 12,500 KRW 경계 등에서 12,499.9999/12,500.0001로 튈 수 있어, 상위/하위 모델이 잘못 선택될 수 있습니다. 금액 연산은 BigDecimal과 명시적 반올림(원화: 소수 0자리, HALF_UP)을 사용해 주세요. 기존 시그니처는 유지하되 deprecated 처리로 점진 이관을 권장합니다.

제안 수정안:

-import lombok.AccessLevel;
-import lombok.NoArgsConstructor;
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+import java.math.BigDecimal;
+import java.math.RoundingMode;

 @NoArgsConstructor(access = AccessLevel.PRIVATE)
-public class CurrencyUtil {
+public final class CurrencyUtil {

-    // 25-08-21 기준
-    private static final double CURRENCY_RATE = 1397.84;
+    // 2025-08-21 기준 (fallback). 운영에서는 외부 주입/프로바이더 사용 권장.
+    private static final BigDecimal CURRENCY_RATE = new BigDecimal("1397.84");

     /**
      * 달러를 원으로 변환
      * @param usd 달러 금액
      * @return KRW 금액
      */
-    public static double usdToKrw(double usd) {
-        return usd * CURRENCY_RATE;
-    }
+    @Deprecated(since = "0.1.0", forRemoval = true)
+    public static double usdToKrw(double usd) {
+        return usdToKrw(BigDecimal.valueOf(usd)).doubleValue();
+    }
+
+    /**
+     * 달러를 원으로 변환 (BigDecimal, KRW 반올림 적용)
+     */
+    public static BigDecimal usdToKrw(BigDecimal usd) {
+        return usd.multiply(CURRENCY_RATE).setScale(0, RoundingMode.HALF_UP);
+    }

추가로, 경계값 비교는 BigDecimal.compareTo로 수행해 주세요.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static double usdToKrw(double usd) {
return usd * CURRENCY_RATE;
}
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
import java.math.BigDecimal;
import java.math.RoundingMode;
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public final class CurrencyUtil {
- // 25-08-21 기준
// 2025-08-21 기준 (fallback). 운영에서는 외부 주입/프로바이더 사용 권장.
private static final BigDecimal CURRENCY_RATE = new BigDecimal("1397.84");
/**
* 달러를 원으로 변환
* @param usd 달러 금액
* @return KRW 금액
*/
- public static double usdToKrw(double usd) {
- return usd * CURRENCY_RATE;
@Deprecated(since = "0.1.0", forRemoval = true)
public static double usdToKrw(double usd) {
return usdToKrw(BigDecimal.valueOf(usd)).doubleValue();
}
/**
* 달러를 원으로 변환 (BigDecimal, KRW 반올림 적용)
*/
public static BigDecimal usdToKrw(BigDecimal usd) {
return usd.multiply(CURRENCY_RATE)
.setScale(0, RoundingMode.HALF_UP);
}
}

Comment on lines +31 to +35
private static String resolveDetailSummary(List<PrAnalysis.SummaryDetails> summaryDetails) {
return summaryDetails.stream()
.map(sd -> Map.entry(sd.title(), sd.description()))
.collect(Collectors.collectingAndThen(Collectors.toList(), SummaryFormatter::formatWithNumbering));
}
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

summaryDetails null/빈 컬렉션 방어 로직 추가

LLM/파서 단계에서 세부 요약이 누락될 경우 NPE가 발생할 수 있습니다. 방어 로직을 추가하세요.

-    private static String resolveDetailSummary(List<PrAnalysis.SummaryDetails> summaryDetails) {
-        return summaryDetails.stream()
+    private static String resolveDetailSummary(List<PrAnalysis.SummaryDetails> summaryDetails) {
+        if (summaryDetails == null || summaryDetails.isEmpty()) {
+            return "";
+        }
+        return summaryDetails.stream()
                 .map(sd -> Map.entry(sd.title(), sd.description()))
                 .collect(Collectors.collectingAndThen(Collectors.toList(), SummaryFormatter::formatWithNumbering));
     }

요약 번호 매기기 기대치 확인을 위해, 실제 출력에 번호가 포함되는지 점검해 주세요. 유틸 SummaryFormatter.formatWithNumbering(...) 구현이 번호를 붙이지 않는다면(현재 구현 스니펫 기준), 유틸 수정 또는 여기에서 인덱싱하여 접두어를 추가하는 방향이 필요합니다.


SummaryFormatter.formatWithNumbering의 실제 구현에서 번호 매기기가 적용되는지 확인하기 위해 아래 스크립트를 실행해 주세요.


🏁 Script executed:

#!/bin/bash
# SummaryFormatter 클래스 파일 위치 찾기
formatter_file=$(fd --full-path SummaryFormatter.java | head -n1)
echo "Found SummaryFormatter at: $formatter_file"

# formatWithNumbering 메서드 구현 확인
rg -nP 'formatWithNumbering' -C5 "$formatter_file"

Length of output: 864


resolveDetailSummary에 null/빈 컬렉션 방어 로직 추가 및 번호 매기기 구현 확인/수정 필요

검토 결과,

  • resolveDetailSummarysummaryDetailsnull이거나 비어 있을 때 NPE 혹은 빈 출력 문제가 발생할 수 있습니다.
  • SummaryFormatter.formatWithNumbering(...)를 확인한 결과, 현재 구현에는 번호(prefix) 로직이 없어 “1. …”, “2. …” 형태의 번호 매기기가 적용되지 않고 있습니다.

따라서 다음 두 가지를 반드시 반영해주세요:

  • resolveDetailSummary에 null/빈 컬렉션 방어 로직 추가
  • 번호 매기기 로직이 필요하다면
    • SummaryFormatter.formatWithNumbering에 직접 번호 매기기 구현 추가 또는
    • resolveDetailSummary 내부에서 인덱스를 붙여 호출

아래는 null/빈 체크를 추가하는 예시입니다.

-    private static String resolveDetailSummary(List<PrAnalysis.SummaryDetails> summaryDetails) {
-        return summaryDetails.stream()
+    private static String resolveDetailSummary(List<PrAnalysis.SummaryDetails> summaryDetails) {
+        if (summaryDetails == null || summaryDetails.isEmpty()) {
+            return \"\";
+        }
+        // TODO: 필요 시 스트림에서 인덱스를 활용해 번호를 붙이거나,
+        //       SummaryFormatter.formatWithNumbering에 번호 로직을 구현하세요.
+        return summaryDetails.stream()
             .map(sd -> Map.entry(sd.title(), sd.description()))
             .collect(Collectors.collectingAndThen(Collectors.toList(),
                                                 SummaryFormatter::formatWithNumbering));
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static String resolveDetailSummary(List<PrAnalysis.SummaryDetails> summaryDetails) {
return summaryDetails.stream()
.map(sd -> Map.entry(sd.title(), sd.description()))
.collect(Collectors.collectingAndThen(Collectors.toList(), SummaryFormatter::formatWithNumbering));
}
private static String resolveDetailSummary(List<PrAnalysis.SummaryDetails> summaryDetails) {
if (summaryDetails == null || summaryDetails.isEmpty()) {
return "";
}
// TODO: 필요 시 스트림에서 인덱스를 활용해 번호를 붙이거나,
// SummaryFormatter.formatWithNumbering에 번호 로직을 구현하세요.
return summaryDetails.stream()
.map(sd -> Map.entry(sd.title(), sd.description()))
.collect(Collectors.collectingAndThen(Collectors.toList(),
SummaryFormatter::formatWithNumbering));
}
🤖 Prompt for AI Agents
In
gss-mcp-app/src/main/java/com/devoops/dto/request/AdaptedAnalyzePrResponse.java
around lines 31-35, add defensive handling so that when summaryDetails is null
or empty the method returns an empty string (or suitable default) to avoid
NPE/empty-output; also implement numbering: either (preferred) modify this
method to enumerate the list (use IntStream.range or an indexed loop) and map
each PrAnalysis.SummaryDetails to a numbered entry like "1. title - description"
before passing to SummaryFormatter.formatWithNumbering (or directly join/return
the numbered strings), or alternatively add numbering logic inside
SummaryFormatter.formatWithNumbering; ensure no assumptions about non-null
inputs and include tests/usage that cover null and empty lists.

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: 0

🧹 Nitpick comments (9)
gss-api-app/src/test/java/com/devoops/BaseServiceTest.java (1)

44-45: 테스트에서도 생성자 주입을 고려해 주세요.

필드 주입은 테스트에서는 흔히 쓰지만, 생성자 주입으로 전환하면 불변성·가시성이 좋아지고 IDE 리팩터링/누락 의존성 감지도 수월합니다. 우선순위는 낮습니다.

gss-api-app/src/test/java/com/devoops/BaseRepositoryTest.java (2)

7-8: 사용되지 않는 import 제거

GithubRepoJpaRepository, PullRequestJpaRepository는 본 클래스에서 사용되지 않습니다. 정리해 주세요.

적용 diff:

-import com.devoops.jpa.repository.github.repo.GithubRepoJpaRepository;
-import com.devoops.jpa.repository.github.pr.PullRequestJpaRepository;

15-19: @import 대상 최소화: 인터페이스(AiChargeRepository)는 불필요합니다.

구현체 또는 설정 클래스가 아니라 인터페이스를 @Import 하는 것은 의미가 없습니다. 아래처럼 인터페이스 항목만 제거해 주세요. (참고: AiChargeJpaRepository가 Spring Data JPA 리포지토리 인터페이스라면 @DataJpaTest가 자동 구성하므로 이것도 제거 가능하지만, 이는 선택입니다.)

적용 diff(최소 변경):

 @Import({
-        AiChargeRepository.class,
         AiChargeRepositoryImpl.class,
         AiChargeJpaRepository.class,
         AiChargeGenerator.class,
 })
gss-domain/src/testFixtures/java/com/devoops/generator/AiChargeGenerator.java (2)

12-13: 생성자 주입으로 전환해 테스트 픽스처의 의존성을 명확히 하세요.

필드 주입 대신 생성자 주입을 사용하면 가독성과 불변성이 개선됩니다.

적용 diff:

-    @Autowired
-    private AiChargeJpaRepository repository;
+    private final AiChargeJpaRepository repository;
+
+    @Autowired
+    public AiChargeGenerator(AiChargeJpaRepository repository) {
+        this.repository = repository;
+    }

15-19: 월 인자 유효성 검사 추가 제안

테스트 픽스처라도 잘못된 month(예: 0, 13)가 들어오면 즉시 실패하도록 방어 로직을 두면 디버깅이 쉬워집니다.

적용 diff:

     public AiCharge generate(int year, int month, double charge) {
+        if (month < 1 || month > 12) {
+            throw new IllegalArgumentException("month must be 1..12");
+        }
         AiCharge aiCharge = new AiCharge(year, month, charge);
         return repository.save(AiChargeEntity.from(aiCharge))
                 .toDomainEntity();
     }
gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java (4)

5-6: Repository 슬라이스 베이스 클래스로 교체하여 테스트 속도 최적화

단순 리포지토리 테스트라면 BaseServiceTest(전체 컨텍스트) 대신 BaseRepositoryTest(JPA 슬라이스)를 상속하는 편이 가볍고 빠릅니다.

적용 diff:

-import com.devoops.BaseServiceTest;
+import com.devoops.BaseRepositoryTest;
@@
-class AiChargeRepositoryTest extends BaseServiceTest {
+class AiChargeRepositoryTest extends BaseRepositoryTest {

Also applies to: 13-13


24-30: 시간 의존성 완화: YearMonth 사용으로 의도 명확화

LocalDate.now()도 문제는 없지만, 테스트 의도상 연·월만 필요합니다. YearMonth를 쓰면 가독성이 좋아집니다.

적용 diff:

-import java.time.LocalDate;
+import java.time.LocalDate;
+import java.time.YearMonth;
@@
-            LocalDate localDate = LocalDate.now();
-            aiChargeGenerator.generate(localDate.getYear(), localDate.getMonthValue(), charge);
+            LocalDate localDate = LocalDate.now(); // 필요 시 다른 곳에서 사용
+            YearMonth ym = YearMonth.now();
+            aiChargeGenerator.generate(ym.getYear(), ym.getMonthValue(), charge);
@@
-            AiCharge actual = chargeRepository.getByMonth(localDate.getMonthValue());
+            AiCharge actual = chargeRepository.getByMonth(ym.getMonthValue());

29-30: 실수 비교 시 허용 오차 사용 고려

현재 값들이 정수 형태라 실패 가능성은 낮지만, 과금 누적 로직이 변하면 부동소수 오차에 민감해질 수 있습니다. isCloseTo(x, within(1e-6)) 같은 형태를 고려해 주세요. 필요 시 import static org.assertj.core.api.Assertions.within; 추가.

Also applies to: 38-39, 54-55


53-55: 변수명 카멜케이스 정정

updatedchargeupdatedCharge

적용 diff:

-            AiCharge updatedcharge = chargeRepository.getByMonth(localDate.getMonthValue());
-            assertThat(updatedcharge.getCharge()).isEqualTo(charge * 2);
+            AiCharge updatedCharge = chargeRepository.getByMonth(localDate.getMonthValue());
+            assertThat(updatedCharge.getCharge()).isEqualTo(charge * 2);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b7265e0 and 82415d0.

📒 Files selected for processing (8)
  • gss-api-app/src/test/java/com/devoops/BaseRepositoryTest.java (1 hunks)
  • gss-api-app/src/test/java/com/devoops/BaseServiceTest.java (2 hunks)
  • gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeJpaRepository.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java (1 hunks)
  • gss-domain/src/testFixtures/java/com/devoops/BaseRepositoryTest.java (0 hunks)
  • gss-domain/src/testFixtures/java/com/devoops/generator/AiChargeGenerator.java (1 hunks)
💤 Files with no reviewable changes (1)
  • gss-domain/src/testFixtures/java/com/devoops/BaseRepositoryTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
  • gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeJpaRepository.java
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java
🔇 Additional comments (5)
gss-api-app/src/test/java/com/devoops/BaseServiceTest.java (1)

44-45: AiChargeGenerator 주입 추가, 테스트 데이터 시딩 용도로 적절합니다.

@SpringBootTest 전체 컨텍스트에서 테스트 픽스처 컴포넌트가 자동 스캔되어 문제없이 주입될 것입니다.

gss-api-app/src/test/java/com/devoops/BaseRepositoryTest.java (1)

1-26: JPA 슬라이스 전용 베이스 클래스 구성 괜찮습니다.

@DataJpaTest + @Import로 필요한 커스텀 리포지토리/픽스처만 주입해 테스트 부팅 시간을 절약하는 방향이 좋아 보입니다.

gss-domain/src/testFixtures/java/com/devoops/generator/AiChargeGenerator.java (2)

9-20: 테스트 픽스처 구성 깔끔합니다.

도메인 → JPA 엔티티 변환 후 즉시 save, 다시 도메인으로 환원하는 패턴이 명확합니다.


15-19: 금액 타입에 대한 장기적 고려

과금 누적/임계값 비교(7,500·12,500·15,000 KRW)에 부동소수 오차가 관여할 수 있습니다. 도메인이 double을 채택한 이유가 명확하다면 유지하되, 장기적으로 BigDecimal(스케일 0, KRW) 전환을 검토해 주세요.

gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java (1)

27-33: AiChargeRepository 시그니처 확인 완료 – 테스트가 계약에 부합합니다

검사 결과, AiChargeRepository 인터페이스와 구현체 모두 다음과 같이 month만 인자로 받도록 정의되어 있습니다.
따라서 현재 테스트(27–33, 36–39, 51–55행)는 리포지토리 계약을 정확히 따르고 있습니다.

  • gss-domain/src/main/java/com/devoops/domain/repository/analysis/AiChargeRepository.java

    • AiCharge getByMonth(int month);
    • void addCharge(int month, double charge);
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java

    • public AiCharge getByMonth(int month) { … }
    • public void addCharge(int month, double charge) { … }

만약 도메인 엔티티(AiCharge)가 연·월 복합키로 설계되어야 한다면, 리포지토리 메서드 시그니처부터 변경해야 합니다.
현 시점에서는 테스트를 수정할 필요 없이, 데이터 모델 설계 의도에 따라 계약을 재검토해 주세요.

@github-actions
Copy link

github-actions bot commented Aug 24, 2025

Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 73afb58.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Aug 24, 2025

📝 Test Coverage Report

Overall Project NaN% NaN% 🍏

There is no coverage information present for the Files changed

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: 13

♻️ Duplicate comments (6)
gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java (1)

25-29: 금액 타입 double → BigDecimal 전환 및 메서드 이름 명확화 필요

현재 도메인·엔티티·리포지토리 계층 전반에서 금액을 double로 표현하고 있어 누적 시 반올림 오차가 발생할 수 있으며, 메서드명(addCharge)만으로 “누적”인지 “덮어쓰기”인지 불분명합니다. 다음 항목들을 반드시 고쳐주세요:

• gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java
– 36행: private double charge;private BigDecimal charge;

• gss-domain/src/main/java/com/devoops/domain/entity/analysis/AiCharge.java
– 12행: private final double charge;private final BigDecimal charge;

• gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeJpaRepository.java
– 메서드 시그니처: void updateChargeById(int year, int month, double charge);int increaseChargeById(int year, int month, BigDecimal deltaCharge);
– JPQL 쿼리 파라미터도 :charge:deltaCharge 로 변경

• gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java
– 27–29행:
```diff

  • public void addCharge(int year, int month, double charge) {
  •   chargeJpaRepository.updateChargeById(year, month, charge);
    
  • }
  • public void increaseCharge(int year, int month, BigDecimal deltaCharge) {
  •   chargeJpaRepository.increaseChargeById(year, month, deltaCharge);
    
  • }
    
    

위와 같이 double 대신 BigDecimal을 사용해 반올림 오차를 방지하고, 메서드명을 “누적(increase)” 의미가 명확하도록 변경해야 합니다.

gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java (2)

18-20: 연-월 유니크 제약 추가는 적절합니다

월별 누적 과금 모델 특성상 중복 레코드 방지에 필수입니다. 현재 설정은 방향성이 좋습니다.


35-36: 화폐 금액에 double 사용 + precision/scale 불일치 — BigDecimal로 전환 필요

@Column(precision=..., scale=...)double에 의미가 거의 없고, 누적 시 반올림 오차가 발생합니다. 엔티티 필드를 BigDecimal로 전환하고, 컬럼 스펙을 명시하세요. 도메인 AiCharge도 BigDecimal로 정렬되어야 합니다.

-    @Column(precision = 10, scale = 2)
-    private double charge;
+    @Column(precision = 19, scale = 4, nullable = false)
+    private java.math.BigDecimal charge;

도메인 매핑도 함께 조정 필요(아래 from/toDomainEntity 패치 참고).

gss-common/src/main/java/com/devoops/util/CurrencyUtil.java (1)

11-26: 임계값 근처 오판 가능: double 반환 + KRW 반올림 스케일 2 사용으로 모델 전환 로직에 영향

  • KRW는 소수 자릿수가 없으므로 setScale(0, HALF_UP)이 안전합니다.
  • double로 반환하면 이후 비교/합산에서 부동소수 오차가 재도입됩니다. BigDecimal 오버로드를 기본으로 제공하고 double 메서드는 폐지(deprecate)하세요.
  • 환율 상수는 BigDecimal.valueOf(…)보다 문자열 생성자가 안전합니다.

아래와 같이 수정 제안합니다.

 @NoArgsConstructor(access = AccessLevel.PRIVATE)
-public class CurrencyUtil {
+public final class CurrencyUtil {
 
-    // 25-08-21 기준
-    private static final BigDecimal CURRENCY_RATE = BigDecimal.valueOf(1397.84);
-    private static final int CONCURRENCY_ROUNDING_SCALE = 2;
+    // 2025-08-21 기준 (기본값). 운영에서는 외부 주입/프로바이더 사용 권장.
+    private static final BigDecimal CURRENCY_RATE = new BigDecimal("1397.84");
+    private static final int CURRENCY_ROUNDING_SCALE = 0;
@@
-    public static double usdToKrw(double usd) {
-        BigDecimal usdDecimal = BigDecimal.valueOf(usd);
-        return usdDecimal.multiply(CURRENCY_RATE)
-                .setScale(CONCURRENCY_ROUNDING_SCALE, RoundingMode.HALF_UP)
-                .doubleValue();
-    }
+    @Deprecated(since = "0.1.0", forRemoval = false)
+    public static double usdToKrw(double usd) {
+        return usdToKrw(BigDecimal.valueOf(usd)).doubleValue();
+    }
+
+    /**
+     * 달러를 원으로 변환 (BigDecimal, KRW 반올림 적용)
+     */
+    public static BigDecimal usdToKrw(BigDecimal usd) {
+        if (usd == null) throw new IllegalArgumentException("usd must not be null");
+        return usd.multiply(CURRENCY_RATE)
+                .setScale(CURRENCY_ROUNDING_SCALE, RoundingMode.HALF_UP);
+    }
gss-mcp-app/src/test/java/com/devoops/PrAnalysisClientImplTest.java (1)

42-47: 콘솔 출력/벤치마크 코드 제거 및 단언 추가 필요

테스트는 부작용 없는 단언 중심이어야 합니다. 시간 측정/System.out.println은 신뢰도를 해치고, 앞선 리뷰에서도 동일 지적이 있었습니다. 기본 단언을 추가하고 프린트를 제거하세요. 네트워크 의존 시 통합 테스트로 태깅하거나 Fake/Mock 사용을 권장합니다.

-        long startTime = System.currentTimeMillis();
         AnalyzePrRequest request = new AnalyzePrRequest(title, desc, diff, "gpt-5-nano");
         PrAnalysis result = prAnalysisClient.analyze(request).prAnalysis();
-        long endTime = System.currentTimeMillis();
-
-        System.out.println(endTime - startTime+ "ms");
+        // then
+        org.junit.jupiter.api.Assertions.assertNotNull(result, "prAnalysis 결과가 null 입니다");
+        org.junit.jupiter.api.Assertions.assertNotNull(result.summary(), "summary가 null 입니다");
+        org.junit.jupiter.api.Assertions.assertFalse(result.summary().isBlank(), "summary가 비어 있습니다");
+        org.junit.jupiter.api.Assertions.assertNotNull(result.summaryDetails(), "summaryDetails가 null 입니다");
+        org.junit.jupiter.api.Assertions.assertFalse(result.summaryDetails().isEmpty(), "summaryDetails가 비어 있습니다");
+        org.junit.jupiter.api.Assertions.assertNotNull(result.questions(), "questions가 null 입니다");
+        org.junit.jupiter.api.Assertions.assertFalse(result.questions().isEmpty(), "questions가 비어 있습니다");

추가로, 정적 임포트를 선호하시면 별도로 아래를 상단에 추가하세요:

import static org.junit.jupiter.api.Assertions.*;
gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java (1)

68-73: 이전 코멘트 해결됨: 웹훅 이중 등록 제거 확인

과거 지적된 reTrackingOrThrowException 내부의 registerWebhook(...) 호출이 제거되어, 상위 한 곳에서만 등록되도록 일원화되었습니다. 👍

🧹 Nitpick comments (24)
gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java (2)

6-8: DataIntegrityViolationException import 누락

위 수정안을 적용하려면 DataIntegrityViolationException 임포트가 필요합니다.

 import lombok.RequiredArgsConstructor;
 import org.springframework.stereotype.Repository;
 import org.springframework.transaction.annotation.Transactional;
+import org.springframework.dao.DataIntegrityViolationException;

16-23: 입력 검증(월 범위) 추가 권장

월은 1..12 범위로 고정됩니다. 잘못된 입력 조기 차단을 위해 검증을 추가하는 편이 안전합니다.

 @Override
 @Transactional
 public AiCharge getByYearAndMonth(int year, int month) {
+    if (month < 1 || month > 12) {
+        throw new IllegalArgumentException("month must be in range 1..12");
+    }
gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java (2)

3-15: BigDecimal 사용을 위한 import 추가

BigDecimal 전환 시 import를 추가하세요. (또는 위 diff처럼 FQCN 사용)

 import com.devoops.domain.entity.analysis.AiCharge;
+import java.math.BigDecimal;

29-36: NOT NULL 제약 명시 권장

연/월/금액은 비즈니스 상 필수 값입니다. nullable=false로 스키마 제약을 강화하세요.

-    @Column(name = "charge_year")
+    @Column(name = "charge_year", nullable = false)
     private int year;

-    @Column(name = "charge_month")
+    @Column(name = "charge_month", nullable = false)
     private int month;

-    @Column(precision = 10, scale = 2)
+    @Column(precision = 19, scale = 4, nullable = false)
     private BigDecimal charge;
gss-client/gss-mcp-client/src/test/java/com/devoops/client/ConcurrencyModelTest.java (3)

23-29: 동시 시작 보장을 위한 배리어 추가로 경합 재현성 향상

현재는 작업 제출 타이밍이 엇갈려 경합이 약합니다. CountDownLatch로 동시에 시작해 데이터 레이스를 재현하거나, 반대로 “섞이지 않아야 한다” 테스트의 신뢰도를 높이세요.

@@
-        for (int i = 0; i < 10; i++) {
+        final java.util.concurrent.CountDownLatch start = new java.util.concurrent.CountDownLatch(1);
+        for (int i = 0; i < 10; i++) {
             final int id = i;
             results.add(executor.submit(() -> {
-                return "id = " + id + " model = " +
+                start.await(); // 동시에 시작
+                return "id = " + id + " model = " +
                         OpenAiChatOptions.builder().model(buildModel(id)).build().getModel();
             }));
         }
+        start.countDown();

필요 시 @RepeatedTest(50) 등을 사용해 재현 확률을 높일 수 있습니다(단, CI 플래키 방지를 위해 적절한 태깅 필요).


13-14: 클래스 전체 @disabled로 인해 CI에서 영구 비활성 — 목적에 맞는 태깅/분리 제안

“관찰용/재현용” 테스트라면 @tag("manual"|"concurrency")로 마킹하고 기본 빌드에서 제외하는 Gradle/Surefire 설정을 권장합니다. 반대로 “보장 테스트”라면 본 테스트는 비활성로 유지하되, 별도의 긍정 케이스(작업별 빌더 생성 시 섞이지 않음을 단언) 테스트를 추가하세요.

원하시면 build 스크립트의 태그 기반 include/exclude 설정 패치를 함께 드리겠습니다.


36-41: 모델 문자열 하드코딩 — 도메인 상수/enum과의 일관성 및 커버리지 확장

  • 문자열 오타·변경에 취약합니다. gss-domain의 OpenAiModel(enum) 또는 중앙 상수를 참조하세요. 모듈 의존성 제약이 있다면 아래와 같이 클래스 내부 상수로라도 통일해 주세요.
  • 본 PR의 모델 전환 로직은 gpt-5, gpt-5-mini, gpt-5-nano 3종을 다룹니다. 테스트도 nano 케이스를 포함하면 더 현실적인 커버리지가 됩니다.

예시(클래스 상단에 추가):

private static final String MODEL_GPT5 = "gpt-5";
private static final String MODEL_GPT5_MINI = "gpt-5-mini";
private static final String MODEL_GPT5_NANO = "gpt-5-nano";

그리고 선택 로직:

private String buildModel(int id) {
    int mod = id % 3;
    if (mod == 0) return MODEL_GPT5_NANO;
    if (mod == 1) return MODEL_GPT5_MINI;
    return MODEL_GPT5;
}
gss-mcp-app/src/main/java/com/devoops/service/pranalysis/PrAnalysisService.java (5)

24-26: 외부 호출 예외 처리 및 입력 유효성 검증을 보강하세요.

request.diffUrl() 또는 githubToken.getToken()null/blank일 경우 NPE/잘못된 호출이 발생할 수 있고, GithubAdaptor 호출 실패 시 상위로 원인 맥락 없이 예외가 전파됩니다. 최소한의 파라미터 검증과 예외 매핑을 추가해 주세요.

적용 예시(핵심 부분만 교체):

-        String diff = githubAdaptor.getCodeChangeHistory(request.diffUrl(), githubToken.getToken());
+        final String diffUrl = request.diffUrl();
+        final String token = (githubToken != null) ? githubToken.getToken() : null;
+        if (diffUrl == null || diffUrl.isBlank()) {
+            throw new IllegalArgumentException("diffUrl must not be null/blank");
+        }
+        if (token == null || token.isBlank()) {
+            throw new IllegalArgumentException("github token must not be null/blank");
+        }
+        final String diff;
+        try {
+            diff = githubAdaptor.getCodeChangeHistory(diffUrl, token);
+        } catch (Exception e) {
+            // TODO: 도메인 예외로 매핑(예: ExternalIntegrationException)하여 상위 계층에 원인과 컨텍스트를 전달
+            throw new IllegalStateException("Failed to fetch code change history from GitHub: " + diffUrl, e);
+        }

26-29: 월 과금 인덱싱은 YearMonth를 사용하고, TZ 하드코딩은 최소화하세요.

월별 누적과금만 필요하므로 LocalDate 대신 YearMonth가 더 적합합니다. 또한 "Asia/Seoul" 문자열 하드코딩은 설정 주입으로 대체하는 것을 권장합니다. 우선 간단히 YearMonth로 정리해도 이득이 큽니다.

예시:

+import java.time.YearMonth;
-        ZoneId seoulZoneId = ZoneId.of("Asia/Seoul");
-        LocalDate today = LocalDate.now(seoulZoneId);
-        AiCharge aiCharge = chargeRepository.getByYearAndMonth(today.getYear(), today.getMonthValue());
+        YearMonth currentYm = YearMonth.now(ZoneId.of("Asia/Seoul"));
+        AiCharge aiCharge = chargeRepository.getByYearAndMonth(currentYm.getYear(), currentYm.getMonthValue());

이후 addCharge 호출부도 today 대신 currentYm을 사용하도록 맞추세요.


16-22: 관측 가능성(Observability) 보강을 위한 최소 로그 추가 제안.

선택된 모델, 토큰 사용량, 과금액, 대상 연/월 정도를 DEBUG/INFO로 남기면 운영 중 문제 추적이 쉬워집니다. 민감 데이터(토큰 문자열, diff 내용, GitHub 토큰)는 절대 로깅하지 마세요.

예시:

+import lombok.extern.slf4j.Slf4j;
 @Service
 @RequiredArgsConstructor
+@Slf4j
 public class PrAnalysisService {
   ...
-        chargeRepository.addCharge(today.getYear(), today.getMonthValue(), consumedCharge);
+        chargeRepository.addCharge(today.getYear(), today.getMonthValue(), consumedCharge);
+        log.debug("PR analysis charged: model={}, promptTokens={}, completionTokens={}, consumed={} KRW, year={}, month={}",
+                aiModel.getName(), result.promptTokens(), result.completionTokens(), consumedCharge,
+                today.getYear(), today.getMonthValue());

28-35: 모델 임계값(7,500/12,500/15,000 KRW) 경계 테스트 추가를 권장합니다.

경계값에서의 모델 선택은 회귀에 취약합니다. 해당 세 그레일 포인트(7,499/7,500/7,501, 12,499/12,500/12,501, 14,999/15,000/15,001 KRW)에 대한 단위/통합 테스트를 추가해 주세요. 월경계(TZ=Asia/Seoul, 23:59:59→00:00:00) 시 로직도 포함되면 좋습니다.

원하시면 테스트 스켈레톤 생성 도와드리겠습니다.


31-36: 모델 타입을 문자열(String) 대신 강한 타입(Enum/Value Object)으로 전달하세요

현재 PrAnalysisService에서 aiModel.getName()으로 문자열을 넘기고, PrAnalysisAdapter.analyze(...)String model 파라미터를 받도록 선언되어 있습니다. 이 방식은 모델 식별자 계약이 문자열에 의존하기 때문에, 오타나 변경에 취약하며 타입 안정성을 보장하지 못합니다.

검토 결과:

  • PrAnalysisService.java (31행)
    AdaptedAnalyzePrResponse result = prAnalysisAdapter.analyze(
            request.title(),
            request.description(),
            diff,
            aiModel.getName()   // ← 문자열 전달
    );
  • PrAnalysisAdapter.java (16행)
    public AdaptedAnalyzePrResponse analyze(String title, String description, String diff, String model) {
        AnalyzePrRequest analyzePrRequest = new AnalyzePrRequest(title, description, diff, model);
        …
    }

제안하는 리팩터링:

  • analyze(...) 시그니처를 String model 대신 강한 타입(예: OpenAiModel Enum 또는 별도 ModelId Value Object)으로 변경
  • PrAnalysisService 호출부에서 aiModel 객체를 그대로 전달하도록 수정
  • 어댑터 내부에서 필요 시 해당 타입을 문자열로 매핑하고, 매핑 실패 시명확한 예외 처리

예시 Diff:

--- a/gss-mcp-app/src/main/java/com/devoops/adaptor/PrAnalysisAdapter.java
+++ b/gss-mcp-app/src/main/java/com/devoops/adaptor/PrAnalysisAdapter.java
@@ -13,7 +13,7 @@ public class PrAnalysisAdapter {
-    public AdaptedAnalyzePrResponse analyze(String title, String description, String diff, String model) {
+    public AdaptedAnalyzePrResponse analyze(String title, String description, String diff, OpenAiModel model) {
         // 모델 매핑 로직 (예: model.name())
         AnalyzePrRequest analyzePrRequest = new AnalyzePrRequest(
             title, description, diff, model.name()
         );
--- a/gss-mcp-app/src/main/java/com/devoops/service/pranalysis/PrAnalysisService.java
+++ b/gss-mcp-app/src/main/java/com/devoops/service/pranalysis/PrAnalysisService.java
@@ -28,7 +28,7 @@ public class PrAnalysisService {
-        AdaptedAnalyzePrResponse result = prAnalysisAdapter.analyze(
+        AdaptedAnalyzePrResponse result = prAnalysisAdapter.analyze(
                 request.title(),
                 request.description(),
                 diff,
-                aiModel.getName()
+                aiModel
         );

이렇게 변경하면 컴파일 시점에 모델 식별자의 유효성을 보장하고, 문자열 계약에 따른 예기치 않은 런타임 에러를 방지할 수 있습니다.

gss-common/src/main/java/com/devoops/util/CurrencyUtil.java (3)

13-13: 오타: CONCURRENCY_ROUNDING_SCALE → CURRENCY_ROUNDING_SCALE

의미상 통화(Currency)로 보입니다. 상수명을 바로잡아 가독성과 검색성을 높여 주세요. 위 제안 diff에 포함되어 있습니다.


8-9: 유틸 클래스는 final 권장

상속 목적이 없는 순수 유틸리티이므로 final 지정이妥當합니다. 위 제안 diff에 포함했습니다.


11-12: 하드코딩 환율의 구성 가능성 확보

운영 환경에서는 환율이 수시로 변합니다. 프로퍼티/환경변수 또는 Provider(예: CurrencyRateProvider)로 외부 주입을 고려하세요. 기본값은 본 클래스에 두되, 주입값이 존재하면 우선 사용하도록 설계하면 안전합니다.

gss-common/src/main/java/com/devoops/exception/errorcode/ErrorCode.java (1)

41-43: 에러 메시지 가독성 개선 제안

메시지를 간결하게 다듬으면 운영 모니터링에서 유용합니다. 상태코드(500)는 유지하되 문구만 손봅니다.

-    GITHUB_CLIENT_ERROR(500, "깃허브 클라이언트 소통과정에 문제가 발생했습니다"),
-    AI_RESPONSE_PARSING_ERROR(500, "AI로부터 온 질문 생성을 파싱하는 과정에 오류가 발생했습니다"),
+    GITHUB_CLIENT_ERROR(500, "깃허브 클라이언트 통신 중 오류가 발생했습니다"),
+    AI_RESPONSE_PARSING_ERROR(500, "AI 응답(JSON) 파싱 중 오류가 발생했습니다"),
gss-client/gss-mcp-client/src/main/java/com/devoops/serdes/PrAnalysisMapper.java (1)

17-20: ObjectMapper는 빈 주입으로 통합 관리 권장

여러 곳에서 Jackson 설정을 공유해야 한다면, @Configuration에서 ObjectMapper 빈을 통일해 주입받는 편이 유지보수에 유리합니다. 현재 static 초기화는 테스트/프로파일 간 설정 일관성을 해칠 수 있습니다.

gss-domain/src/main/java/com/devoops/service/repository/RepositoryService.java (1)

30-32: 파라미터 명 혼동 및 읽기 전용 트랜잭션 권장

  • 현재 메서드명은 findByUserAndExternalId인데 파라미터명이 repositoryId라 혼동 소지가 큽니다. 외부 ID임을 명시해 주세요.
  • 조회용 메서드는 @Transactional(readOnly = true)로 표시하는 것을 권장합니다.

적용 예시:

-    public Optional<GithubRepository> findByUserAndExternalId(User user, long repositoryId) {
-        return repoRepository.findByExternalIdAndUserId(repositoryId, user.getId());
+    @Transactional(readOnly = true)
+    public Optional<GithubRepository> findByUserAndExternalId(User user, long externalId) {
+        return repoRepository.findByExternalIdAndUserId(externalId, user.getId());
     }
gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java (1)

50-56: 재등록(리트래킹) 사전 탐지 플로우 적절 — 네이밍과 예외 흐름 명확성 제안

  • externalId로 사전 탐지 후 분기하는 흐름 적절합니다.
  • 변수명 alreadyRegisteredRepo는 “이미 등록된 레포(추적 여부 무관)” 의미를 잘 전달합니다. 유지 추천.
  • 예외 흐름은 아래 reTrackingOrThrowException에서 일원화되어 깔끔합니다. 유지 추천.

작은 제안:

  • findByUserAndExternalId(...) 호출부 주변에 “externalId 기준 중복 검사” 주석을 추가하면 가독성이 더 좋아집니다.
gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClientImpl.java (4)

31-34: 모델/옵션 설정 안정성: reasoningEffort 지원 여부와 온도 조정 검토

  • reasoningEffort("medium")는 사용하는 Spring AI/OpenAI 클라이언트 버전에 따라 미지원일 수 있습니다. 미지원 시 런타임 예외 가능성이 있어 확인이 필요합니다.
  • JSON Schema 기반 구조화 출력에는 낮은 온도(예: 0.2~0.3)가 안정적으로 동작하는 경우가 많습니다.

검증/개선 예시:

-        OpenAiChatOptions openAiChatOptions = openAiChatBuilder()
-                .model(request.model())
-                .build();
+        OpenAiChatOptions openAiChatOptions = openAiChatBuilder()
+                .model(request.model())
+                .temperature(0.2)
+                .build();

라이브러리 호환성 확인 요청:

  • 현재 Spring AI/OpenAI 클라이언트 버전에서 OpenAiChatOptions#reasoningEffort 지원 여부 확인 후, 필요 시 대응되는 API(예: reasoning(...) 혹은 vendor 확장 속성)로 교체해 주세요.

35-46: 변수 네이밍 및 널 안정성 소폭 개선

  • chatresponsechatResponse로 카멜 케이스 정렬.
  • 드물게 Usage가 누락되는 공급자도 있어 NPE 방지를 위해 널 가드를 권장합니다. (토큰 0 디폴트 등)

예시:

-        ChatResponse chatresponse = callChatResponse(
+        ChatResponse chatResponse = callChatResponse(
                 request.title(),
                 request.description(),
                 request.codeDifference(),
                 openAiChatOptions
         );
-
-        Usage usage = chatresponse.getMetadata().getUsage();
-        String analysisResult = chatresponse.getResult().getOutput().getText();
+        Usage usage = chatResponse.getMetadata() != null ? chatResponse.getMetadata().getUsage() : null;
+        String analysisResult = chatResponse.getResult().getOutput().getText();

48-53: JSON Schema 응답 포맷 강화(이름/strict 옵션)

OpenAI의 json_schema 포맷은 스키마와 함께 name/strict 플래그를 요구/권장하는 버전이 있습니다. Spring AI의 ResponseFormat가 이를 지원한다면 이름(예: "PrAnalysis")과 strict=true를 지정해 구조화 출력의 일관성을 높여 주세요.

예시(라이브러리 지원 시):

// 가령 ResponseFormat.jsonSchema("PrAnalysis", outputJsonSchema(), true) 형태 지원 시 해당 팩토리를 사용

61-71: 프롬프트 빌드 경량화 및 추적 안전성

현재 체인은 명확합니다. 추가로:

  • .user(...)(지원 시) 설정으로 호출자 식별자를 넣으면 벤더 측 레이트리미트/감사 추적에 도움이 됩니다.
  • 필요 시 maxTokens 또는 seed(지원 시) 설정으로 응답 길이와 결정성을 관리할 수 있습니다.
gss-client/gss-mcp-client/src/main/java/com/devoops/dto/response/AnalyzePrResponse.java (1)

3-5: 불필요한 import 정리

JsonProcessingException, ObjectMapper는 본 파일에서 사용되지 않습니다. 정리해 주세요.

-import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.ObjectMapper;
+// unused imports removed
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cc9c21f and 9f8c1f2.

📒 Files selected for processing (17)
  • gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java (3 hunks)
  • gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java (1 hunks)
  • gss-api-app/src/test/java/com/devoops/service/facade/RepositoryFacadeServiceTest.java (1 hunks)
  • gss-client/gss-mcp-client/build.gradle (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClientImpl.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/config/AiConfig.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/dto/response/AnalyzePrResponse.java (1 hunks)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/serdes/PrAnalysisMapper.java (1 hunks)
  • gss-client/gss-mcp-client/src/test/java/com/devoops/client/ConcurrencyModelTest.java (1 hunks)
  • gss-common/src/main/java/com/devoops/exception/errorcode/ErrorCode.java (1 hunks)
  • gss-common/src/main/java/com/devoops/util/CurrencyUtil.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/domain/repository/analysis/AiChargeRepository.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/service/repository/RepositoryService.java (2 hunks)
  • gss-mcp-app/src/main/java/com/devoops/service/pranalysis/PrAnalysisService.java (1 hunks)
  • gss-mcp-app/src/test/java/com/devoops/PrAnalysisClientImplTest.java (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • gss-client/gss-mcp-client/src/main/java/com/devoops/config/AiConfig.java
  • gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java
  • gss-domain/src/main/java/com/devoops/domain/repository/analysis/AiChargeRepository.java
  • gss-api-app/src/test/java/com/devoops/service/facade/RepositoryFacadeServiceTest.java
🧰 Additional context used
🧬 Code graph analysis (3)
gss-client/gss-mcp-client/src/main/java/com/devoops/serdes/PrAnalysisMapper.java (1)
gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClientImpl.java (1)
  • Component (19-72)
gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClientImpl.java (2)
gss-client/gss-mcp-client/src/main/java/com/devoops/serdes/PrAnalysisMapper.java (1)
  • Slf4j (11-29)
gss-mcp-app/src/main/java/com/devoops/adaptor/PrAnalysisAdapter.java (1)
  • Component (10-21)
gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java (1)
gss-domain/src/main/java/com/devoops/domain/entity/analysis/AiCharge.java (1)
  • Getter (6-13)
🔇 Additional comments (4)
gss-client/gss-mcp-client/src/test/java/com/devoops/client/ConcurrencyModelTest.java (1)

16-16: 확인: 프로덕션 코드에 공유된 OpenAiChatOptions.Builder 필드가 없습니다

  • PrAnalysisClientImpl#openAiChatBuilder() 메서드는 호출 시마다 OpenAiChatOptions.builder()로 새 인스턴스를 생성합니다.
  • analyze 메서드 내에서 openAiChatBuilder().model(...).build() 형태로 사용되어, 요청 간 옵션이 섞일 위험이 없습니다.
  • 테스트 코드(ConcurrencyModelTest)의 공유 빌더 패턴은 오직 테스트에만 적용되며, 실제 서비스 코드에는 영향을 주지 않습니다.

별도 조치 없이 현 구조를 유지하셔도 괜찮습니다. 공통 기본값 캡슐화 또한 이미 헬퍼 메서드(openAiChatBuilder)로 구현되어 있어 권장 사항을 잘 따르고 있습니다.

gss-mcp-app/src/main/java/com/devoops/service/pranalysis/PrAnalysisService.java (2)

31-40: AdaptedAnalyzePrResponse의 토큰 필드는 primitive int로 null 불가능 — 추가 null 검사는 불필요합니다.
promptTokens()completionTokens()가 wrapper가 아닌 primitive int로 선언되어 있어 NPE 위험이 없습니다.

Likely an incorrect or invalid review comment.


28-30: 잘못된 null 반환 우려 — 확인 결과 NPE 불가능
AiChargeRepositoryImpl#getByYearAndMonth(int, int)는 내부적으로

chargeJpaRepository.findByYearAndMonth(year, month)
    .orElseGet(() -> /* charge=0인 AiCharge 생성 */)

를 사용해 레코드가 없을 경우에도 기본값(charge=0)의 AiCharge 객체를 반환하므로 null이 절대 반환되지 않습니다.

  • 구현 위치:
    • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java 라인 17–19
  • 관련 테스트:
    • gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java 라인 34–38 (기본값 0원 검증)

따라서 별도의 Optional 처리나 서비스 레이어 방어 로직은 불필요합니다. 이 리뷰 코멘트는 무시해 주시기 바랍니다.

Likely an incorrect or invalid review comment.

gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java (1)

53-56: 리트래킹 분기 반환 경로 단순화 좋음 — 후속 호출 단일화 확인

saveRepository가 리트래킹 결과를 그대로 반환하고, 상위 save()에서만 웹훅 등록을 1회 호출하는 구조로 정리된 점 좋습니다. 재등록 경로에서도 중복 호출이 발생하지 않는지 테스트에서 verify(webHookService, times(1)).registerWebhook(...);로 보장해 주세요.

테스트 보강 제안:

  • “이미 존재하나 isTracking=false” 케이스에서 registerWebhook 1회 호출, “isTracking=true” 케이스에서 예외 발생을 각각 검증.

}

dependencies {
implementation project(":gss-common")
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

implementation vs api 재검토 — gss-common 타입이 공개 API에 노출된다면 api가 맞습니다

java-library를 사용하는 경우, gss-common의 타입(GssException, ErrorCode, DTO 등)이 이 모듈의 public 메서드 시그니처/예외에 등장하면 api project(":gss-common")로 노출해야 합니다. 그렇지 않으면 소비자 모듈에서 클래스패스 누락 문제가 날 수 있습니다.

다음 스크립트로 공개 API 노출 여부를 확인해 주세요(리포지토리 루트에서 실행).


🏁 Script executed:

#!/bin/bash
# gss-mcp-client의 public 시그니처에 gss-common 타입이 보이는지 점검
rg -n --type=java -C2 'public (class|interface|enum)|public [^=]*\(' gss-client/gss-mcp-client/src/main/java \
  | rg -n 'GssException|ErrorCode|com\.devoops\.(exception|util|dto)\.'

Length of output: 441


gss-mcp-client의 public 메서드 시그니처에 AnalyzePrResponse가 사용되는지 확인하기 위해 파일 상단을 출력합니다.


🏁 Script executed:

#!/bin/bash
# PrAnalysisClient.java 상위 200줄 출력
sed -n '1,200p' gss-client/gss-mcp-client/src/main/java/com/devoops/client/PrAnalysisClient.java

Length of output: 323


gss-mcp-client: gss-common 의존성 implementationapi로 변경 필요

PrAnalysisClient 인터페이스가 AnalyzePrRequest/AnalyzePrResponse (com.devoops.dto.*) 타입을 public API 시그니처에 노출하고 있으므로, gss-common 모듈을 api로 선언해야 소비자 모듈에서 클래스패스 누락 없이 정상 컴파일/실행이 가능합니다.

– 파일: gss-client/gss-mcp-client/build.gradle
– 변경 위치: 6행

제안하는 수정:

-dependencies {
-    implementation project(":gss-common")
+dependencies {
+    api project(":gss-common")
     // …
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
implementation project(":gss-common")
// gss-client/gss-mcp-client/build.gradle
dependencies {
api project(":gss-common")
//
}
🤖 Prompt for AI Agents
In gss-client/gss-mcp-client/build.gradle around line 6, the dependency on
gss-common is declared as implementation which hides its types from consumers;
change the declaration to use api project(":gss-common") so the PrAnalysisClient
public API types (AnalyzePrRequest/AnalyzePrResponse) are exposed on the
consumer classpath and downstream modules can compile and run without missing
classes.

Comment on lines +14 to +20
public AnalyzePrResponse(Usage usage, PrAnalysis prAnalysis) {
this(
usage.getPromptTokens(),
usage.getCompletionTokens(),
usage.getTotalTokens(),
prAnalysis
);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Usage 누락 시 NPE 위험 — 방어적 생성자 적용 제안

일부 제공자/설정에서 Usage가 null일 수 있어 현재 생성자는 NPE 위험이 있습니다. 토큰 값을 0으로 보정하도록 방어 로직을 추가해 주세요.

-    public AnalyzePrResponse(Usage usage, PrAnalysis prAnalysis) {
-        this(
-                usage.getPromptTokens(),
-                usage.getCompletionTokens(),
-                usage.getTotalTokens(),
-                prAnalysis
-        );
-    }
+    public AnalyzePrResponse(Usage usage, PrAnalysis prAnalysis) {
+        int prompt = (usage != null) ? usage.getPromptTokens() : 0;
+        int completion = (usage != null) ? usage.getCompletionTokens() : 0;
+        int total = (usage != null) ? usage.getTotalTokens() : (prompt + completion);
+        this(prompt, completion, total, prAnalysis);
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public AnalyzePrResponse(Usage usage, PrAnalysis prAnalysis) {
this(
usage.getPromptTokens(),
usage.getCompletionTokens(),
usage.getTotalTokens(),
prAnalysis
);
// … other members …
public AnalyzePrResponse(Usage usage, PrAnalysis prAnalysis) {
int prompt = (usage != null) ? usage.getPromptTokens() : 0;
int completion = (usage != null) ? usage.getCompletionTokens() : 0;
int total = (usage != null) ? usage.getTotalTokens() : (prompt + completion);
this(prompt, completion, total, prAnalysis);
}
// … other members …
🤖 Prompt for AI Agents
In
gss-client/gss-mcp-client/src/main/java/com/devoops/dto/response/AnalyzePrResponse.java
around lines 14 to 20, the constructor dereferences Usage directly causing an
NPE if Usage is null; change the constructor to defensively handle a null Usage
by substituting 0 for promptTokens, completionTokens, and totalTokens (e.g.,
check if usage == null and pass 0s to the delegated constructor, otherwise pass
usage.getXxx()).

Comment on lines +21 to +28
public PrAnalysis mapToPrAnalysis(String analysisResult) {
try {
return MAPPER.readValue(analysisResult, PrAnalysis.class);
} catch (JsonProcessingException e) {
log.error("PR 질문 생성 파싱 오류 : {}", analysisResult.substring(0, MAX_LOGGING_LENGTH));
throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

NULL/빈 문자열 방어 로직 및 예외 컨텍스트 보강

  • analysisResult가 null/blank일 수 있으므로 조기 검증이 필요합니다.
  • 로깅 시 스택트레이스를 함께 남기세요.
  • 가능하면 GssException에 cause를 전달하는 생성자를 제공/사용해 원인 추적성을 높이세요.

아래와 같이 방어 코드를 추가하세요.

-    public PrAnalysis mapToPrAnalysis(String analysisResult) {
-        try {
-            return MAPPER.readValue(analysisResult, PrAnalysis.class);
-        } catch (JsonProcessingException e) {
-            log.error("PR 질문 생성 파싱 오류 : {}", analysisResult.substring(0, MAX_LOGGING_LENGTH));
-            throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
-        }
-    }
+    public PrAnalysis mapToPrAnalysis(String analysisResult) {
+        if (analysisResult == null || analysisResult.isBlank()) {
+            log.warn("PR 질문 생성 파싱 오류 : 빈 응답(null/blank) 수신");
+            throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
+        }
+        try {
+            return MAPPER.readValue(analysisResult, PrAnalysis.class);
+        } catch (JsonProcessingException e) {
+            final String preview = analysisResult.substring(0, Math.min(analysisResult.length(), MAX_LOGGING_LENGTH));
+            log.error("PR 질문 생성 파싱 오류 : {}", preview, e);
+            // TODO: GssException이 cause를 받는 생성자가 있다면 그걸 사용하세요.
+            throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
+        }
+    }

원하시면 단위 테스트 케이스(정상 JSON, 빈 문자열, 잘못된 JSON, 매우 긴 JSON)도 함께 작성해 드립니다.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public PrAnalysis mapToPrAnalysis(String analysisResult) {
try {
return MAPPER.readValue(analysisResult, PrAnalysis.class);
} catch (JsonProcessingException e) {
log.error("PR 질문 생성 파싱 오류 : {}", analysisResult.substring(0, MAX_LOGGING_LENGTH));
throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
}
}
public PrAnalysis mapToPrAnalysis(String analysisResult) {
if (analysisResult == null || analysisResult.isBlank()) {
log.warn("PR 질문 생성 파싱 오류 : 빈 응답(null/blank) 수신");
throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
}
try {
return MAPPER.readValue(analysisResult, PrAnalysis.class);
} catch (JsonProcessingException e) {
final String preview = analysisResult.substring(0, Math.min(analysisResult.length(), MAX_LOGGING_LENGTH));
log.error("PR 질문 생성 파싱 오류 : {}", preview, e);
// TODO: GssException이 cause를 받는 생성자가 있다면 그걸 사용하세요.
throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
}
}
🤖 Prompt for AI Agents
In
gss-client/gss-mcp-client/src/main/java/com/devoops/serdes/PrAnalysisMapper.java
around lines 21 to 28, add defensive checks and richer exception context: first
validate analysisResult for null or blank and throw a
GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR) (or IllegalArgumentException
mapped to that error) immediately if empty; when catching
JsonProcessingException, log the (possibly truncated) analysisResult plus the
exception stacktrace (use log.error with the exception parameter) and rethrow a
GssException constructed with the ErrorCode and the caught exception as the
cause (add/use a GssException constructor that accepts a cause if necessary) so
the original parsing error is preserved for debugging.

Comment on lines +25 to +26
log.error("PR 질문 생성 파싱 오류 : {}", analysisResult.substring(0, MAX_LOGGING_LENGTH));
throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

substring 사용 시 길이 검사 누락으로 SIOOBE 발생 가능

analysisResult 길이가 255보다 짧으면 StringIndexOutOfBoundsException이 발생합니다. 안전한 절단을 적용하세요.

-            log.error("PR 질문 생성 파싱 오류 : {}", analysisResult.substring(0, MAX_LOGGING_LENGTH));
+            final String preview = analysisResult == null
+                    ? "null"
+                    : analysisResult.substring(0, Math.min(analysisResult.length(), MAX_LOGGING_LENGTH));
+            log.error("PR 질문 생성 파싱 오류 : {}", preview, e);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
log.error("PR 질문 생성 파싱 오류 : {}", analysisResult.substring(0, MAX_LOGGING_LENGTH));
throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
final String preview = analysisResult == null
? "null"
: analysisResult.substring(0, Math.min(analysisResult.length(), MAX_LOGGING_LENGTH));
log.error("PR 질문 생성 파싱 오류 : {}", preview, e);
throw new GssException(ErrorCode.AI_RESPONSE_PARSING_ERROR);
🤖 Prompt for AI Agents
In
gss-client/gss-mcp-client/src/main/java/com/devoops/serdes/PrAnalysisMapper.java
around lines 25 to 26, the call to analysisResult.substring(0,
MAX_LOGGING_LENGTH) can throw StringIndexOutOfBoundsException when
analysisResult is shorter than MAX_LOGGING_LENGTH (or null); guard against this
by computing an end index with Math.min(analysisResult.length(),
MAX_LOGGING_LENGTH) (and null-checking analysisResult first) and use
substring(0, endIndex) (or use StringUtils.left(analysisResult,
MAX_LOGGING_LENGTH)) so the logged slice is always safe.

@Disabled
class ConcurrencyModelTest {

private final OpenAiChatOptions.Builder sharedBuilder = OpenAiChatOptions.builder();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

공유된 OpenAiChatOptions.Builder는 스레드-세이프하지 않아 옵션이 섞일 수 있습니다. 실서비스 코드에도 같은 패턴이 없는지 점검 필요

동일 빌더 인스턴스를 여러 스레드가 mutate(model 설정) 후 build하면 레이스가 터집니다. 테스트 의도가 “섞임 관측”이라면 현재 구조가 맞지만, 검증 가능한 테스트(불변성 보장)를 원한다면 각 작업에서 새 빌더를 생성하거나 ThreadLocal을 사용하세요. 아래는 “작업별 로컬 빌더”로 바꾸는 최소 수정案입니다.

-    private final OpenAiChatOptions.Builder sharedBuilder = OpenAiChatOptions.builder();
+    // 테스트 및 실서비스 모두에서 빌더 공유를 피합니다(각 작업에서 새 빌더 생성).

@@
-                sharedBuilder.model(buildModel(id));
-                return "id = " + id + " model = " + sharedBuilder.build().getModel();
+                return "id = " + id + " model = " +
+                        OpenAiChatOptions.builder().model(buildModel(id)).build().getModel();

원하시면 ThreadLocal 활용 버전도 제안드릴 수 있습니다(테스트 목적 유지 + 데이터 경합 완화).

Also applies to: 26-27

Comment on lines +47 to +53
public AiCharge toDomainEntity() {
return new AiCharge(
year,
month,
charge
);
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

toDomainEntity(): 도메인 타입(BigDecimal)으로 일치

정밀도 손실 방지를 위해 도메인도 BigDecimal을 사용하도록 맞추세요.

-    public AiCharge toDomainEntity() {
-        return new AiCharge(
-                year,
-                month,
-                charge
-        );
-    }
+    public AiCharge toDomainEntity() {
+        return new AiCharge(
+                year,
+                month,
+                charge // BigDecimal
+        );
+    }

도메인 클래스 변경(파일 외부 참고):

// gss-domain/src/main/java/com/devoops/domain/entity/analysis/AiCharge.java
@Getter
@RequiredArgsConstructor
public class AiCharge {
    private final int year;
    private final int month;
    private final BigDecimal charge;
}

DB 마이그레이션 예(Flyway):

ALTER TABLE ai_charge
    MODIFY COLUMN charge DECIMAL(19,4) NOT NULL;

Comment on lines 16 to 23
@Override
public AiCharge getByYearAndMonth(int year, int month) {
return chargeJpaRepository.findByYearAndMonth(year, month)
.orElseGet(() -> {
AiCharge initializeCharge = new AiCharge(year, month, 0);
return chargeJpaRepository.save(AiChargeEntity.from(initializeCharge));
}).toDomainEntity();
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

엔트리 자동 초기화 시 경쟁 조건(race) 가능 — 트랜잭션 + 재조회(retry) 패턴 적용 권장

동일 연-월에 대해 동시 호출 시, 두 쓰레드가 모두 save를 시도하면 유니크 제약에 의해 한쪽이 예외로 끝날 수 있습니다. 메서드를 트랜잭션으로 감싸고, 저장 충돌 시 재조회하는 패턴으로 안정화하세요. 또한 Optional.map(...::toDomainEntity) 사용이 더 명료합니다.

-    @Override
-    public AiCharge getByYearAndMonth(int year, int month) {
-        return chargeJpaRepository.findByYearAndMonth(year, month)
-                .orElseGet(() -> {
-                    AiCharge initializeCharge = new AiCharge(year, month, 0);
-                    return chargeJpaRepository.save(AiChargeEntity.from(initializeCharge));
-                }).toDomainEntity();
-    }
+    @Override
+    @Transactional
+    public AiCharge getByYearAndMonth(int year, int month) {
+        return chargeJpaRepository.findByYearAndMonth(year, month)
+                .map(AiChargeEntity::toDomainEntity)
+                .orElseGet(() -> {
+                    try {
+                        return chargeJpaRepository
+                                .save(AiChargeEntity.from(new AiCharge(year, month, 0)))
+                                .toDomainEntity();
+                    } catch (org.springframework.dao.DataIntegrityViolationException e) {
+                        // 동시 초기화 충돌 시 재조회
+                        return chargeJpaRepository.findByYearAndMonth(year, month)
+                                .orElseThrow(() -> e)
+                                .toDomainEntity();
+                    }
+                });
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Override
public AiCharge getByYearAndMonth(int year, int month) {
return chargeJpaRepository.findByYearAndMonth(year, month)
.orElseGet(() -> {
AiCharge initializeCharge = new AiCharge(year, month, 0);
return chargeJpaRepository.save(AiChargeEntity.from(initializeCharge));
}).toDomainEntity();
}
@Override
@Transactional
public AiCharge getByYearAndMonth(int year, int month) {
return chargeJpaRepository.findByYearAndMonth(year, month)
.map(AiChargeEntity::toDomainEntity)
.orElseGet(() -> {
try {
return chargeJpaRepository
.save(AiChargeEntity.from(new AiCharge(year, month, 0)))
.toDomainEntity();
} catch (org.springframework.dao.DataIntegrityViolationException e) {
// 동시 초기화 충돌 시 재조회
return chargeJpaRepository.findByYearAndMonth(year, month)
.orElseThrow(() -> e)
.toDomainEntity();
}
});
}

Comment on lines +34 to +38
public GithubRepository reTracking(User user, long repositoryId) {
GithubRepository repo = repoRepository.getByExternalIdAndUserId(repositoryId, user.getId());
GithubRepository reTrackingRepo = repo.reTracking();
return repoRepository.update(reTrackingRepo);
}
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

reTracking 파라미터의 의미 불일치 및 트랜잭션 경계 보강

  • 시그니처의 repositoryId는 실제로 externalId로 사용됩니다. 오해 방지를 위해 명확히 이름을 바꿔 주세요.
  • 상태 전이를 포함하는 도메인 업데이트(repo.reTracking()update)는 트랜잭션 경계 내에서 처리하는 것이 안전합니다.

적용 예시:

-    public GithubRepository reTracking(User user, long repositoryId) {
-        GithubRepository repo = repoRepository.getByExternalIdAndUserId(repositoryId, user.getId());
+    @Transactional
+    public GithubRepository reTracking(User user, long externalId) {
+        GithubRepository repo = repoRepository.getByExternalIdAndUserId(externalId, user.getId());
         GithubRepository reTrackingRepo = repo.reTracking();
         return repoRepository.update(reTrackingRepo);
     }

검증 제안:

  • 서비스 외부에서 save()가 직접 호출될 수 있다면, (userId, externalId) 유니크 제약이 반드시 존재해야 합니다. 아래 스크립트로 엔티티/DDL에 유니크 제약이 있는지 확인해 주세요.

🏁 Script executed:

#!/bin/bash
# (1) 엔티티에 유니크 제약/인덱스 선언 확인
rg -nP -C3 '(?i)@Table\(.*uniqueConstraints|@Column\(.*unique\s*=\s*true' --type=java
rg -nP -C3 '(?i)class\s+GithubRepository\b|externalId\b' --type=java

# (2) 마이그레이션/DDL에서 유니크 인덱스 확인
fd -t f -i -g '*sql*' | xargs -I{} rg -nP -C2 '(?i)unique|unique index|unique key|constraint' {}

Length of output: 56977


reTracking 메소드 파라미터명 변경 및 트랜잭션 추가, 복합 유니크 제약 조건 누락

스크립트 실행 결과, GithubRepository 엔티티와 DDL에서 (user_id, external_id) 복합 유니크 제약이 설정되어 있지 않습니다. 데이터 중복 방지와 도메인 일관성 확보를 위해 아래 사항을 반영해 주세요.

  • 서비스 계층 (gss-domain/src/main/java/com/devoops/service/repository/RepositoryService.java)
    • 메소드 시그니처에서 repositoryIdexternalId로 파라미터명 변경
    • 상태 전이와 DB 업데이트를 같은 트랜잭션에서 처리하도록 @Transactional 어노테이션 추가

  • 도메인 엔티티 (gss-domain/src/main/java/com/devoops/jpa/entity/github/repo/GithubRepositoryEntity.java)
    • 클래스 레벨에 복합 유니크 제약조건 추가

    @Entity
    @Table(
      name = "github_repository",
      uniqueConstraints = @UniqueConstraint(
        name = "uq_github_repository_user_external",
        columnNames = {"user_id", "github_repository_id"}
      )
    )
    public class GithubRepositoryEntity { … }
  • 마이그레이션 DDL

    ALTER TABLE github_repository
      ADD CONSTRAINT uq_github_repository_user_external
      UNIQUE (user_id, github_repository_id);
  • 서비스 메소드 예시(diff)

    - public GithubRepository reTracking(User user, long repositoryId) {
    -     GithubRepository repo = repoRepository.getByExternalIdAndUserId(repositoryId, user.getId());
    + @Transactional
    + public GithubRepository reTracking(User user, long externalId) {
    +     GithubRepository repo = repoRepository.getByExternalIdAndUserId(externalId, user.getId());
          GithubRepository reTrackingRepo = repo.reTracking();
          return repoRepository.update(reTrackingRepo);
      }
🤖 Prompt for AI Agents
In
gss-domain/src/main/java/com/devoops/service/repository/RepositoryService.java
around lines 34-38, change the method parameter name repositoryId to externalId
and annotate the method with @Transactional so the state transition
(repo.reTracking()) and the subsequent update happen in the same transaction; in
addition, add a DB-level unique constraint to the GithubRepository entity by
updating
gss-domain/src/main/java/com/devoops/jpa/entity/github/repo/GithubRepositoryEntity.java
to include a @Table uniqueConstraints entry (name =
"uq_github_repository_user_external", columnNames = {"user_id",
"github_repository_id"}), and add a migration DDL that alters the
github_repository table to add the same unique constraint (ALTER TABLE
github_repository ADD CONSTRAINT uq_github_repository_user_external UNIQUE
(user_id, github_repository_id)); ensure the method uses the new parameter name
consistently and import javax.transaction.Transactional (or
org.springframework.transaction.annotation.Transactional) as appropriate.

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: 0

🧹 Nitpick comments (9)
gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java (9)

24-29: 시간 의존성 제거: LocalDate.now() 대신 고정된 (연,월) 사용 + BigDecimal로 금액 비교

동일 월에 다른 테스트가 데이터를 심는 경우/병렬 실행/타임존 차이로 플래키해질 수 있습니다. 테스트별로 서로 다른 고정 월을 사용하고 금액 비교는 doubleValue() 대신 BigDecimal 비교를 권장합니다.

아래처럼 변경을 제안합니다(이 테스트는 2030-01을 사용):

-            LocalDate localDate = LocalDate.now();
-            aiChargeGenerator.generate(localDate.getYear(), localDate.getMonthValue(), charge);
+            int year = 2030;
+            int month = 1;
+            aiChargeGenerator.generate(year, month, charge);
 
-            AiCharge actual = chargeRepository.getByYearAndMonth(localDate.getYear(), localDate.getMonthValue());
+            AiCharge actual = chargeRepository.getByYearAndMonth(year, month);
 
-            assertThat(actual.getCharge().doubleValue()).isEqualTo(charge);
+            assertThat(actual.getCharge()).isEqualByComparingTo(java.math.BigDecimal.valueOf(charge));

34-39: “데이터 없음” 케이스도 고정 월 사용 + BigDecimal 비교

현재 월에 다른 테스트가 데이터를 심으면 본 테스트가 실패할 수 있습니다. 독립적인 월을 사용하고 0 비교도 BigDecimal로 수행하세요.

-            LocalDate localDate = LocalDate.now();
-
-            AiCharge actual = chargeRepository.getByYearAndMonth(localDate.getYear(), localDate.getMonthValue());
-
-            assertThat(actual.getCharge().doubleValue()).isEqualTo(0.0);
+            int year = 2030;
+            int month = 2;
+            AiCharge actual = chargeRepository.getByYearAndMonth(year, month);
+            assertThat(actual.getCharge()).isEqualByComparingTo(java.math.BigDecimal.ZERO);

47-55: 업데이트 테스트도 고정 월 + 변수명 타이포 수정 + BigDecimal 비교

  • 시간 의존성 제거: 2030-03 등 고정 월 사용
  • 변수명 오타: updatedchargeupdatedCharge
  • 금액 비교는 BigDecimal로 수행
-            double charge = 1500.0;
-            LocalDate localDate = LocalDate.now();
-            aiChargeGenerator.generate(localDate.getYear(), localDate.getMonthValue(), charge);
-
-            chargeRepository.addCharge(localDate.getYear(), localDate.getMonthValue(), charge);
-
-            AiCharge updatedcharge = chargeRepository.getByYearAndMonth(localDate.getYear(), localDate.getMonthValue());
-            assertThat(updatedcharge.getCharge().doubleValue()).isEqualTo(charge * 2);
+            double charge = 1500.0;
+            int year = 2030;
+            int month = 3;
+            aiChargeGenerator.generate(year, month, charge);
+
+            chargeRepository.addCharge(year, month, charge);
+
+            AiCharge updatedCharge = chargeRepository.getByYearAndMonth(year, month);
+            assertThat(updatedCharge.getCharge())
+                .isEqualByComparingTo(java.math.BigDecimal.valueOf(charge).multiply(java.math.BigDecimal.valueOf(2)));

18-20: 네이밍 정밀화: GetByMonth → GetByYearAndMonth

테스트 그룹명이 실제 API 시그니처(getByYearAndMonth)를 반영하면 가독성이 좋아집니다.

-    class GetByMonth {
+    class GetByYearAndMonth {

21-31: 금액 타입 일관성: 테스트에서도 BigDecimal 사용 고려

도메인이 통화/요금 누적을 다루므로 double 대신 BigDecimal을 사용하는 편이 반올림 이슈를 줄입니다. 위의 비교 변경만으로도 안전하나, 테스트 입력 역시 BigDecimal을 사용하면 더 명확합니다.

예시:

-            double charge = 1500.0;
+            double charge = 1500.0; // 유지 가능
+            // 또는:
+            // BigDecimal charge = BigDecimal.valueOf(1500);
+            // aiChargeGenerator.generate(year, month, charge.doubleValue());

42-56: 추가 케이스 제안: 존재하지 않는 (연,월)에 addCharge 호출 시 동작 검증

현재는 seed 후 add만 검증합니다. “레코드 없음 → addCharge 호출” 경로가 생성+누적을 보장하는지 확인하는 테스트를 한 케이스 추가하면 회귀를 막는 데 유용합니다.

예시(동일 클래스 내 Update 섹션에 추가):

@Test
void 존재하지_않는_월에_addCharge를_호출하면_생성되고_누적된다() {
    int year = 2030, month = 5;
    chargeRepository.addCharge(year, month, 700.0);
    AiCharge after = chargeRepository.getByYearAndMonth(year, month);
    assertThat(after.getCharge()).isEqualByComparingTo(BigDecimal.valueOf(700.0));
}

원하시면 제가 PR에 바로 반영 가능한 커밋용 패치를 만들어 드리겠습니다.


3-11: AssertJ 유틸/정확 비교를 위한 import 보강

위 제안( BigDecimal 비교 등)을 적용하려면 BigDecimal import 가 필요합니다.

 import static org.assertj.core.api.Assertions.assertThat;
 
 import com.devoops.BaseServiceTest;
 import com.devoops.domain.entity.analysis.AiCharge;
 import com.devoops.domain.repository.analysis.AiChargeRepository;
+import java.math.BigDecimal;
 import java.time.LocalDate;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;

13-16: ⚡ Repository 레이어 테스트 범위 축소 제안: BaseServiceTestBaseRepositoryTest

AiChargeGeneratorBaseRepositoryTest에서도 제공되는 것을 확인했습니다. 따라서 불필요한 빈 로딩 없이, 레포지토리 단위 테스트만 수행하도록 테스트 스코프를 축소할 수 있습니다.

  • 대상 파일
    • gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java
  • 변경 내용
    • 클래스 선언부 상속을 BaseServiceTest에서 BaseRepositoryTest로 수정
- class AiChargeRepositoryTest extends BaseServiceTest {
+ class AiChargeRepositoryTest extends BaseRepositoryTest {

32-39: “초기화”가 실제로 DB에 저장되는지 테스트에 명시적으로 검증합시다

현재 테스트는 반환된 charge 값이 0.0인지 확인만 하고 있지만, AiChargeRepositoryImpl#getByYearAndMonth 구현상 초기화 시 chargeJpaRepository.save(...) 를 호출해 DB에 엔티티를 저장합니다. 따라서 아래와 같은 추가 검증을 통해

  • 동일 연·월에 재호출했을 때 같은 엔티티를 조회하는지
  • addCharge 호출 후 누적된 값이 반환되는지

명확히 확인하는 것을 권장합니다.

대상 테스트:

  • 파일: gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java
  • 클래스: GetByMonth 내부

추천 보강 예시:

@Test
void 초기화_후_동일_연월_재호출시_엔티티_재사용을_검증한다() {
    int year = 2030, month = 4;

    // 첫 호출 시 초기화 및 저장
    AiCharge first = chargeRepository.getByYearAndMonth(year, month);
    assertThat(first.getCharge()).isEqualByComparingTo(BigDecimal.ZERO);

    // 두 번째 호출 시 DB에 저장된 동일 엔티티 조회
    AiCharge second = chargeRepository.getByYearAndMonth(year, month);
    assertThat(second).isEqualTo(first); // equals/hashCode가 도메인 엔티티 동일성을 보장해야 함
}

@Test
void 초기화_후_addCharge_호출시_누적된_요금을_반환한다() {
    int year = 2030, month = 4;

    // 초기화
    chargeRepository.getByYearAndMonth(year, month);

    // 누적
    chargeRepository.addCharge(year, month, 500.0);
    AiCharge updated = chargeRepository.getByYearAndMonth(year, month);
    assertThat(updated.getCharge()).isEqualByComparingTo(BigDecimal.valueOf(500.0));
}
  • equals()/hashCode() 구현 여부에 따라 첫 번째 테스트는 first.getId() 비교나 charge 값만 비교하는 식으로 조정 가능합니다.
  • 이미 존재하는 aiChargeGenerator를 활용해 DB 초기 상태를 깔끔하게 관리하세요.

이 검증을 통해 “초기화”가 메모리 반환이 아닌 영속화된 엔티티 생성을 의도대로 수행함을 확실히 검증할 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9f8c1f2 and 73afb58.

📒 Files selected for processing (7)
  • gss-api-app/src/test/java/com/devoops/repository/analysis/AiChargeRepositoryTest.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/domain/entity/analysis/AiCharge.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/domain/entity/analysis/OpenAiModel.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java (1 hunks)
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java (1 hunks)
  • gss-domain/src/test/java/com/devoops/domain/entity/analysis/OpenAiModelTest.java (1 hunks)
  • gss-domain/src/testFixtures/java/com/devoops/generator/AiChargeGenerator.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • gss-domain/src/testFixtures/java/com/devoops/generator/AiChargeGenerator.java
  • gss-domain/src/main/java/com/devoops/jpa/repository/analysis/AiChargeRepositoryImpl.java
  • gss-domain/src/main/java/com/devoops/domain/entity/analysis/AiCharge.java
  • gss-domain/src/main/java/com/devoops/jpa/entity/analysis/AiChargeEntity.java
  • gss-domain/src/main/java/com/devoops/domain/entity/analysis/OpenAiModel.java
  • gss-domain/src/test/java/com/devoops/domain/entity/analysis/OpenAiModelTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-and-push

@coli-geonwoo coli-geonwoo merged commit d551e8b into develop Aug 25, 2025
5 checks passed
@coli-geonwoo coli-geonwoo deleted the refactor/#72 branch August 25, 2025 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

1 participant