-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor : course #116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor : course #116
Conversation
검토 요약WalkthroughCourseGenerationAiService에 씰 스팟 캐싱, 경량 하이브리드 워크플로우(라이트모드), 시드 기반 재시도 카운터, 위치 필터링 강화, 다단계 후보 조립(씰 우선, k-NN 보강), 동적 max_tokens 계산 및 오류/파싱 개선이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as CourseGenerationAiService
participant Cache as SealSpotsCache
participant Repo as SpotRepository
participant OpenAI as OpenAI API
participant Parser as ResponseParser
Client->>Service: generateAiCourse(request)
rect rgb(230,245,250)
Note over Service: 위치 정규화 및 필터링\n(finalLocations 계산, 군위 제외)
Service->>Service: effectiveLocations 결정
end
rect rgb(240,250,240)
Note over Service,Cache: 씰 스팟 취득(우선)
Service->>Cache: getSealSpots(effectiveLocations)
alt cache hit
Cache-->>Service: cached seal spots
else cache miss
Service->>Repo: fetch seal spots
Repo-->>Service: seal spots
Service->>Cache: cache seal spots
end
end
rect rgb(250,240,245)
Note over Service,Repo: 일반 스팟 및 k-NN 보강
Service->>Repo: fetch regular spots per location
Repo-->>Service: spots
Service->>Service: topKNearest 보강 (위치별)
end
rect rgb(250,245,230)
Note over Service: 후보 조립 및 프롬프트 생성
Service->>Service: 합치기(씰 우선, 중복제거, 시드/리롤 적용)
Service->>Service: SpotForAi -> 콤팩트 JSON, dynamic max_tokens 계산
end
rect rgb(245,230,250)
Note over Service,OpenAI: OpenAI 호출
Service->>OpenAI: send prompt (dynamic max_tokens)
OpenAI-->>Service: response
end
rect rgb(230,250,245)
Note over Service,Parser: 응답 파싱 및 검증
Service->>Parser: parse + 주소/메타데이터 필터 + null 검사
Parser-->>Service: normalized course
end
Service-->>Client: return course
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 분
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test Results9 tests 0 ✅ 0s ⏱️ Results for commit ba76e82. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (6)
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java (6)
55-55:LIGHT_MODE를 구성 가능하게 만드세요.
LIGHT_MODE가true로 하드코딩되어 있어 런타임에 조정할 수 없습니다. 이를 애플리케이션 프로퍼티나 요청 파라미터로 외부화하면 프로덕션에서 더 유연하게 운영할 수 있습니다.
97-111: Java record를 사용하여 간결성을 향상시키세요.
SpotForAi는 간단한 데이터 전달 객체입니다. Java 14 이상을 사용하는 경우 record로 변환하면 보일러플레이트를 줄이고 불변성을 보장할 수 있습니다.private record SpotForAi( Long spotId, String name, Double latitude, Double longitude, Boolean isSealSpot ) {}
211-236: 병렬 검색의 예외 처리를 강화하세요.라인 235에서
CompletableFuture.join()을 호출하지만 예외 처리가 없습니다. 특정 지역의 검색이 실패하면 전체 코스 생성이 실패합니다. 부분 실패를 허용하거나 최소한 상세한 로깅을 추가하는 것이 좋습니다.parallelDocuments.addAll( futures.stream() .map(f -> { try { return f.join(); } catch (Exception e) { log.error("Location search failed", e); return List.<Document>of(); } }) .flatMap(List::stream) .toList());
500-520: 씰 스팟 우선순위 로직을 명확히 하세요.라인 517-520의 씰 스팟 우선순위 로직에서 기존 스팟이 씰이 아닐 때만 덮어쓰는데, 동일한 spotId에 대해 씰/비씰 정보가 다를 수 있다는 가정이 명확하지 않습니다. 이 상황이 발생하는 이유와 의도를 주석으로 설명하면 유지보수에 도움이 됩니다.
// 3. 씰 스팟 우선 덮어쓰기 + // 동일한 contentId가 일반 스팟과 씰 스팟 모두로 등록될 수 있어, 씰 정보를 우선 적용 if (existingSpot == null || (isSealSpot && !Boolean.TRUE.equals(existingSpot.getIsSealSpot()))) { uniq.put(id, newSpot); }
648-678: k-NN 알고리즘이 효율적으로 구현되었습니다.우선순위 큐를 사용한 O(n log k) 시간 복잡도의 k-최근접 이웃 구현이 좋습니다. 전체 정렬 대신 힙을 사용하여 성능을 최적화한 점이 훌륭합니다.
라인 664-665에서 worst를 peek한 후 다시 거리를 계산하는 대신, 우선순위 큐에 거리 정보를 함께 저장하면 중복 계산을 피할 수 있습니다. 다만 현재 구현도 충분히 효율적입니다.
639-645: 좌표 0 검증 로직을 재검토하세요.라인 640에서 위도나 경도가 0일 때
Double.MAX_VALUE를 반환하는데, 이는 적도나 본초자오선 근처의 유효한 장소를 제외할 수 있습니다. 경북 지역에서는 문제없겠지만, null 체크만으로 충분할 수 있습니다.private double fastDist2(double lat1, double lon1, double lat2, double lon2) { - if (lat1 == 0 || lon1 == 0 || lat2 == 0 || lon2 == 0) return Double.MAX_VALUE; double latRad = Math.toRadians((lat1 + lat2) * 0.5); double x = Math.toRadians(lon2 - lon1) * Math.cos(latRad); double y = Math.toRadians(lat2 - lat1); return x * x + y * y; }null 체크는 호출자(line 659, 663, 665)에서 이미 처리되므로 이 메서드에서는 불필요합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java (1)
src/main/java/com/yfive/gbjs/domain/course/dto/response/CourseResponse.java (1)
CourseResponse(12-179)
🔇 Additional comments (8)
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java (8)
76-95: 시드 생성 로직이 잘 구현되었습니다.FNV-1a 스타일 해시와 LRU 카운터를 조합하여 재생성 시 다양한 결과를 생성하는 접근 방식이 적절합니다. 결정론적 시드가 동일한 입력에 대해 일관된 기본 결과를 보장하면서도, 재호출 시 카운터를 통해 다양성을 제공합니다.
306-310: Null-safe Boolean 체크가 올바르게 수정되었습니다.
Boolean.TRUE.equals(s.getIsSealSpot())를 사용하여 null 안전성을 확보한 것은 좋은 수정입니다. 이는 NPE를 방지하면서도 명확한 의도를 표현합니다.
388-408: 프롬프트 구조가 명확하고 제약사항을 잘 전달합니다.핵심 규칙을 강조하고 씰 스팟 필수 포함을 명시한 프롬프트 구조가 좋습니다. 특히 "핵심 규칙 0"에서 전체 여행 기간을 커버해야 한다는 점을 명확히 한 것이 효과적입니다.
429-429: 오류 로깅 개선이 좋습니다.프롬프트 크기를 포함하여 디버깅에 유용한 정보를 제공하는 것이 좋은 개선입니다. AI 생성 실패 시 원인 파악에 도움이 됩니다.
452-480: 위치 필터링 로직이 강화되었습니다.주소(addr1)와 메타데이터(location) 두 가지 방법으로 위치를 검증하는 다단계 접근 방식이 좋습니다. 대소문자 무시 및 교차 검사를 통해 다국어 지원도 개선되었습니다.
547-612:postFix메서드의 검증 로직이 견고합니다.null 체크, 최소 스팟 수(3개) 검증, visitOrder 정규화 등이 잘 구현되어 있습니다. AI 응답의 불완전한 데이터를 안전하게 처리합니다.
614-637: 방문 순서 정규화 로직이 올바릅니다.null과 잘못된 visitOrder 값을 처리하고 순차적으로 재할당하는 로직이 견고합니다.
186-210: 씰 스팟 캐시 초기화 시 topK=200 값 검증 필요라인 194의
topK(200)은 하드코딩된 값으로, "경북 전체 씰 스팟을 모두 가져오기 위한 충분한 값"이라는 주석이 있으나 실제 데이터 기반의 근거가 없습니다.경북 지역 씰 스팟이 200개를 초과하는 경우 벡터 검색 결과에서 일부가 누락될 수 있습니다. 다음을 확인하세요:
- SealSpot 엔티티의 실제 개수 (지역별 또는 전체)
- 벡터 스토어에 인덱싱된 씰 스팟의 총 개수
- 필요시 topK 값을 동적으로 계산하거나 충분한 여유값으로 조정
| private final ChatClient chatClient; | ||
| private final ObjectMapper objectMapper; | ||
|
|
||
| private List<Document> sealSpotsCache; // 씰 스팟 캐시 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sealSpotsCache를 volatile로 선언해야 합니다.
sealSpotsCache는 라인 188-204에서 double-checked locking 패턴으로 초기화되고 있습니다. Java 메모리 모델에서 double-checked locking이 올바르게 동작하려면 필드가 volatile로 선언되어야 합니다. 그렇지 않으면 다른 스레드가 부분적으로 초기화된 객체를 볼 수 있습니다.
다음 diff를 적용하세요:
- private List<Document> sealSpotsCache; // 씰 스팟 캐시
+ private volatile List<Document> sealSpotsCache; // 씰 스팟 캐시📝 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.
| private List<Document> sealSpotsCache; // 씰 스팟 캐시 | |
| private volatile List<Document> sealSpotsCache; // 씰 스팟 캐시 |
🤖 Prompt for AI Agents
In
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java
around line 53, the field sealSpotsCache is used with a double-checked locking
initialization (see lines 188-204) but is not declared volatile; update the
field declaration to mark it volatile (i.e., change to private volatile
List<Document> sealSpotsCache) so the Java memory model guarantees visibility
and prevents other threads from seeing a partially constructed object.
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java
Outdated
Show resolved
Hide resolved
| private static final ExecutorService EXEC = | ||
| Executors.newFixedThreadPool(Math.max(4, Runtime.getRuntime().availableProcessors())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ExecutorService가 종료되지 않아 리소스 누수가 발생합니다.
정적 ExecutorService EXEC가 생성되지만 애플리케이션 종료 시 shutdown()이나 shutdownNow()가 호출되지 않습니다. 이로 인해 스레드가 계속 실행되어 애플리케이션이 정상적으로 종료되지 않을 수 있습니다.
@PreDestroy 메서드를 추가하여 서비스가 파괴될 때 executor를 종료하세요:
@PreDestroy
public void cleanup() {
EXEC.shutdown();
try {
if (!EXEC.awaitTermination(5, TimeUnit.SECONDS)) {
EXEC.shutdownNow();
}
} catch (InterruptedException e) {
EXEC.shutdownNow();
Thread.currentThread().interrupt();
}
}🤖 Prompt for AI Agents
In
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java
around lines 64-65, the static ExecutorService EXEC is never shut down which can
leak threads and prevent JVM termination; add a @PreDestroy cleanup() method in
this service that calls EXEC.shutdown(), waits for termination with
EXEC.awaitTermination(5, TimeUnit.SECONDS), calls EXEC.shutdownNow() if it times
out, and on InterruptedException calls EXEC.shutdownNow() and re-sets the
interrupt flag (Thread.currentThread().interrupt()); ensure you import
javax.annotation.PreDestroy and java.util.concurrent.TimeUnit.
| List<String> locations = request.getLocations(); // 원본 요청 지역 | ||
| if (expectedDays <= 0) { | ||
| throw new IllegalArgumentException("endDate must be on/after startDate"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
request.getLocations()의 null 안전성을 확인하세요.
라인 140에서 request.getLocations()를 호출하지만 null 체크가 없습니다. 만약 locations가 null이면 라인 148에서 NullPointerException이 발생합니다.
다음과 같이 null 체크를 추가하세요:
- List<String> locations = request.getLocations(); // 원본 요청 지역
if (expectedDays <= 0) {
throw new IllegalArgumentException("endDate must be on/after startDate");
}
+ List<String> locations = request.getLocations();
+ if (locations == null || locations.isEmpty()) {
+ throw new IllegalArgumentException("locations must not be null or empty");
+ }📝 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.
| List<String> locations = request.getLocations(); // 원본 요청 지역 | |
| if (expectedDays <= 0) { | |
| throw new IllegalArgumentException("endDate must be on/after startDate"); | |
| } | |
| if (expectedDays <= 0) { | |
| throw new IllegalArgumentException("endDate must be on/after startDate"); | |
| } | |
| List<String> locations = request.getLocations(); | |
| if (locations == null || locations.isEmpty()) { | |
| throw new IllegalArgumentException("locations must not be null or empty"); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java
around lines 140 to 143, request.getLocations() is used without null-safety
which can lead to a NullPointerException later; ensure you null-check the result
and replace it with a safe default (e.g., Collections.emptyList() or new
ArrayList<>()) or throw a clear IllegalArgumentException if locations are
required, then use that non-null variable for the rest of the method.
| // ========================= | ||
| // [GWUNWI] 군위군 스팟 부족 시 제외할 지역 계산 | ||
| // ========================= | ||
| final int MIN_SPOTS_FOR_LOCATION = 2; | ||
| List<String> effectiveLocations = new ArrayList<>(finalLocations); | ||
| List<String> excludedLocations = new ArrayList<>(); | ||
| for (String loc : finalLocations) { | ||
| List<CourseResponse.SimpleSpotDTO> list = spotsByLocation.getOrDefault(loc, List.of()); | ||
| int total = (list == null) ? 0 : list.size(); | ||
| boolean isGwunwi = loc.contains("군위"); // "군위군", "군위" 등 포괄 | ||
| if (isGwunwi && total < MIN_SPOTS_FOR_LOCATION) { | ||
| excludedLocations.add(loc); | ||
| } | ||
| } | ||
| if (!excludedLocations.isEmpty() && effectiveLocations.size() - excludedLocations.size() >= 1) { | ||
| effectiveLocations.removeAll(excludedLocations); | ||
| log.info( | ||
| "[GWUNWI] excludedLocations={}, effectiveLocations={}", | ||
| excludedLocations, | ||
| effectiveLocations); | ||
| } else { | ||
| excludedLocations.clear(); // 제외 보류(유일 지역 등이면) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모든 지역에 대해 최소 스팟 검증을 적용하세요.
현재 군위군만 스팟 부족 시 제외되는데, 다른 지역도 스팟이 부족할 수 있습니다. 라인 273의 isGwunwi 체크를 제거하고 모든 지역에 대해 동일한 로직을 적용하는 것이 일관성 있습니다.
for (String loc : finalLocations) {
List<CourseResponse.SimpleSpotDTO> list = spotsByLocation.getOrDefault(loc, List.of());
int total = (list == null) ? 0 : list.size();
- boolean isGwunwi = loc.contains("군위"); // "군위군", "군위" 등 포괄
- if (isGwunwi && total < MIN_SPOTS_FOR_LOCATION) {
+ if (total < MIN_SPOTS_FOR_LOCATION) {
excludedLocations.add(loc);
}
}🤖 Prompt for AI Agents
In
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java
around lines 264 to 286, the current loop only excludes locations with too few
spots when the name matches "군위" via the isGwunwi check; remove that isGwunwi
condition so the same MIN_SPOTS_FOR_LOCATION validation runs for every location
(compute total from spotsByLocation, add loc to excludedLocations when total <
MIN_SPOTS_FOR_LOCATION), keep the guard that prevents removing all locations
(the existing if that checks effectiveLocations.size() -
excludedLocations.size() >= 1), and update the log message tag if desired
(remove or generalize the [GWUNWI] tag) so it reflects that exclusions can apply
to any location.
| log.info( | ||
| "[LIGHT] Total spots for AI: {} ({} seals, {} regulars)", | ||
| spotsForOpenAI.size(), | ||
| spotsForOpenAI.size() - regularSpotCandidates.size(), // 이 계산은 정확하지 않을 수 있음 | ||
| regularSpotCandidates.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로그 메시지의 계산 오류를 수정하세요.
라인 360의 로그에서 씰 스팟 개수를 계산하는 로직이 부정확합니다. spotsForOpenAI.size() - regularSpotCandidates.size()는 실제 씰 스팟 개수가 아니라 잘못된 값을 출력합니다.
log.info(
"[LIGHT] Total spots for AI: {} ({} seals, {} regulars)",
spotsForOpenAI.size(),
- spotsForOpenAI.size() - regularSpotCandidates.size(), // 이 계산은 정확하지 않을 수 있음
+ sealSpotsMap.size(),
- regularSpotCandidates.size());
+ spotsForOpenAI.size() - sealSpotsMap.size());Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java
around lines 357 to 361, the log computes "seal" spot count incorrectly by
subtracting regularSpotCandidates.size() from spotsForOpenAI.size(); instead
compute the actual number of seal spots (e.g., count elements in spotsForOpenAI
that are not in regularSpotCandidates or maintain a dedicated sealSpotCandidates
collection) and use that value in the log message; replace the subtraction with
the proper count (compute via stream/filter/count or a loop, or track seals when
building lists) and then log spotsForOpenAI.size(), sealCount, and
regularSpotCandidates.size().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java (5)
53-53:sealSpotsCache를volatile로 선언해야 합니다.이전 리뷰에서 지적된 critical 이슈가 여전히 해결되지 않았습니다. 라인 187-204의 double-checked locking 패턴이 올바르게 동작하려면 라인 53의
sealSpotsCache필드가volatile로 선언되어야 합니다.- private List<Document> sealSpotsCache; // 씰 스팟 캐시 + private volatile List<Document> sealSpotsCache; // 씰 스팟 캐시Also applies to: 187-204
64-65:ExecutorService를 종료하는 로직이 누락되었습니다.이전 리뷰에서 지적된 major 이슈가 여전히 해결되지 않았습니다. 정적
ExecutorService EXEC가 애플리케이션 종료 시 shutdown되지 않아 스레드 리소스 누수가 발생합니다.
@PreDestroy메서드를 추가하여 executor를 정리하세요.
140-143:request.getLocations()의 null 체크가 누락되었습니다.이전 리뷰에서 지적된 major 이슈가 여전히 해결되지 않았습니다. 라인 140에서
request.getLocations()를 호출하지만 null 체크가 없어서, 라인 148에서NullPointerException이 발생할 수 있습니다.
264-286: 모든 지역에 대해 최소 스팟 검증을 적용하세요.이전 리뷰에서 지적된 major 이슈가 여전히 해결되지 않았습니다. 현재 라인 273의
isGwunwi체크로 인해 군위군만 스팟 부족 시 제외되는데, 다른 지역도 동일한 문제가 있을 수 있습니다. 일관성을 위해 모든 지역에 동일한 로직을 적용해야 합니다.
357-361: 로그 메시지의 씰 스팟 개수 계산 오류를 수정하세요.이전 리뷰에서 지적된 minor 이슈가 여전히 해결되지 않았습니다. 라인 360의 계산식
spotsForOpenAI.size() - regularSpotCandidates.size()는 씰 스팟 개수를 정확히 반영하지 않습니다. 라인 301의sealSpotsMap.size()를 사용해야 합니다.log.info( "[LIGHT] Total spots for AI: {} ({} seals, {} regulars)", spotsForOpenAI.size(), - spotsForOpenAI.size() - regularSpotCandidates.size(), + sealSpotsMap.size(), - regularSpotCandidates.size()); + spotsForOpenAI.size() - sealSpotsMap.size());
🧹 Nitpick comments (2)
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java (2)
67-74:REROLL_LRU의 동시성 안전성을 검토하세요.
synchronizedMap을 사용하고 있지만, 라인 89의computeIfAbsent와 라인 90의getAndIncrement가 조합된 작업은 완전한 원자성을 보장하지 못할 수 있습니다. 높은 동시성 환경에서 같은 키에 대해 여러 스레드가 경합할 경우 드물게 카운터가 부정확해질 수 있습니다.재생성 카운터의 정확성이 중요하지 않다면 현재 구현도 충분하지만, 완벽한 원자성이 필요하다면
ConcurrentHashMap의computeIfAbsent를 사용하는 것이 더 안전합니다.Also applies to: 89-90
648-678: k-NN 알고리즘이 올바르게 구현되었습니다.
topKNearest메서드는 max-heap을 사용하여 효율적으로 k개의 가장 가까운 스팟을 선택합니다. 알고리즘의 시간 복잡도는 O(n log k)로 k가 n보다 훨씬 작을 때 전체 정렬(O(n log n))보다 효율적입니다.사소한 개선 제안: 라인 656의 Comparator와 라인 663-665에서 거리를 두 번 계산하고 있습니다. 성능이 중요한 경우, 거리 계산 결과를 캐싱하는 것을 고려할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java (1)
src/main/java/com/yfive/gbjs/domain/course/dto/response/CourseResponse.java (1)
CourseResponse(12-179)
🔇 Additional comments (6)
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java (6)
56-56: 상수와 프롬프트가 일치합니다.이전 리뷰에서 지적된
LLM_PLACES_PER_DAY와 프롬프트 간의 불일치가 해결되었습니다. 라인 56의 상수값(4)과 라인 395의 프롬프트 요구사항("반드시 4개")이 정확히 일치합니다.Also applies to: 395-395
306-310: Null-safe Boolean 체크가 올바르게 적용되었습니다.
Boolean.TRUE.equals(s.getIsSealSpot())패턴을 사용하여 null-safe하게 Boolean 필드를 체크하고 있습니다. 이는isSealSpot이 null일 때NullPointerException을 방지하는 좋은 방법입니다.Also applies to: 322-322
445-526: 문서 파싱 로직이 크게 개선되었습니다.
parseAndAddDocuments메서드에 여러 개선사항이 추가되었습니다:
- 이중 위치 필터링 (라인 457-480):
addr1과locationMeta두 가지 방법으로 위치를 검증하여 누락을 방지- 대소문자 무관 매칭 (라인 469): i18n 지원을 위한 case-insensitive 비교
- Null 안전성 (라인 457, 507): addr1이 null일 수 있음을 고려한 처리
- 씰 스팟 우선순위 (라인 517-520): 일반 스팟을 씰 스팟으로 덮어써서 정확도 향상
이러한 개선으로 위치 필터링의 정확성과 견고성이 크게 향상되었습니다.
387-408: 프롬프트 구성이 명확하고 제약사항이 잘 강조되었습니다.AI 프롬프트에 다음과 같은 개선사항이 적용되었습니다:
- 핵심 규칙 강조 (라인 392, 394): ★ 마커로 가장 중요한 제약사항 강조
- 전체 기간 커버리지 (라인 392): 여행 기간 전체를 포함하도록 명시
- 씰 스팟 우선순위 (라인 394): 씰 스팟 1-2개 필수 포함 규칙
- effectiveLocations 반영 (라인 406): 군위군 제외 로직이 적용된 실제 지역 목록 사용
이러한 개선으로 LLM이 요구사항을 더 정확히 이해하고 따를 수 있게 되었습니다.
411-417: 동적 max tokens 계산이 합리적으로 설정되었습니다.여행 일수에 비례하여 응답 토큰 상한을 조절하는 로직(라인 411)이 추가되어, 긴 여행 일정의 JSON 응답이 잘리는 문제를 방지합니다.
- 일당 500 토큰 할당 (합리적인 추정치)
- 최소 2048 토큰 보장 (짧은 여행의 경우)
- 라이트 모드에서 동적 조절 활성화
이 접근법은 응답 품질과 비용 효율성의 균형을 잘 맞추고 있습니다.
622-627:visitOrdernull 처리가 견고하게 구현되었습니다.정렬 로직에서
visitOrder가 null이거나 0 이하인 경우를Integer.MAX_VALUE로 처리하여, AI가 순서를 지정하지 않은 스팟들을 끝으로 밀어내는 방어적 프로그래밍이 잘 적용되었습니다. 이후 라인 633-635에서 모든 스팟에 순차적으로 번호를 다시 부여하여 일관성을 보장합니다.
| private long resolveSeed(LocalDate start, LocalDate end, List<String> locations) { | ||
| String key = start + "|" + end + "|" + String.join(", ", locations); | ||
| AtomicInteger counter = REROLL_LRU.computeIfAbsent(key, k -> new AtomicInteger(0)); | ||
| int reroll = counter.getAndIncrement(); // 첫 호출 0, 이후 1,2,... | ||
| long base = baseSeed(start, end, locations); | ||
| long mixed = (base * 1099511628211L) ^ (reroll & 0xffffffffL); | ||
| log.info("AI Course seed resolved. key='{}', reroll={}, seed={}", key, reroll, mixed); | ||
| return mixed; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
재생성 카운터가 의도치 않게 두 번 증가합니다.
resolveSeed 메서드가 라인 156(지역 축소 시)과 라인 294(일반 스팟 섞기)에서 각각 호출되는데, 라인 90의 getAndIncrement()가 매번 카운터를 증가시킵니다. 이로 인해 generateAiCourse를 한 번 호출할 때마다 재생성 카운터가 2씩 증가하여, 의도한 재생성 다양화 로직이 올바르게 동작하지 않습니다.
해결 방법:
- 라인 156에서는
baseSeed를 직접 사용하거나, 별도의 상수 offset을 추가 - 또는
resolveSeed를 한 번만 호출하고 결과를 재사용
예시 수정:
- Random locationShuffleRand = new Random(resolveSeed(start, end, locations));
+ Random locationShuffleRand = new Random(baseSeed(start, end, locations) + 1);Also applies to: 156-156, 294-294
🤖 Prompt for AI Agents
In
src/main/java/com/yfive/gbjs/domain/course/service/CourseGenerationAiService.java
around lines 87-95 (also affecting caller sites at 156 and 294): resolveSeed
currently uses counter.getAndIncrement(), which causes the reroll counter to
advance each time the method is called and results in the counter being
increased twice per generation because resolveSeed is invoked from two places;
fix by preventing double increments — either (A) change resolveSeed to not
increment the counter (use get()) and provide a separate incrementReroll()
method, or (B) add a boolean parameter to resolveSeed like resolveSeed(...,
boolean increment) and only pass true from the single site that should advance
the counter, or (C) compute the seed once in generateAiCourse and reuse that
value for both places; update the two caller sites (lines ~156 and ~294) to use
the non-incrementing variant or reuse the precomputed seed so the reroll counter
advances exactly once per generation.
Summary by CodeRabbit