[EDMT-406] 단계별 진행상황 전송 #56
Conversation
WalkthroughAI 작업 상태 DB 컬럼과 관련 enum을 제거하고 taskId 타입을 숫자에서 문자열로 변경했으며, SSE 전송을 위한 DTO(SSEMessage, AIProgressMessage)와 SSEChannelManager/A IEventListener 연동(진행/응답 메시지 표준화) 및 SQS idempotencyKey 도입을 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API
participant AIEventListener
participant AIService
participant SSEChannelManager
participant Redis
participant SQS
rect rgb(250,250,255)
Client->>API: 스트리밍 채널(또는 생성 요청)
API->>SSEChannelManager: register/createChannel(taskId)
SSEChannelManager->>Redis: (옵션) GET taskStatusKey
SSEChannelManager-->>Client: SSE "ai-message" (type=PROGRESS) [if status exists]
end
rect rgb(255,250,250)
API->>AIEventListener: 요청 트리거(taskId as String)
AIEventListener->>AIService: start generation
AIEventListener->>SSEChannelManager: sendProgressMessage(taskId, generationStarted)
AIEventListener->>SQS: sendMessage(event, idempotencyKey)
end
loop stream responses
AIService-->>AIEventListener: AI chunk(finalContent, version)
AIEventListener->>SSEChannelManager: sendMessage(taskId, AIResponseMessage)
SSEChannelManager-->>Client: SSE "ai-message" (type=RESPONSE, data={finalContent,version})
SSEChannelManager->>Redis: INCR responseCount / set TTL
end
rect rgb(250,255,250)
AIEventListener->>SSEChannelManager: sendProgressMessage(taskId, generationCompleted)
SSEChannelManager-->>Client: SSE "ai-message" (type=PROGRESS)
SSEChannelManager->>Redis: SET taskStatusKey = PHASE1_COMPLETED (TTL)
end
note over SSEChannelManager,SQS: IOException/전송 실패 시 채널 제거 및 상태 정리
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Comment |
…ge and related classes
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java (1)
27-33: AIEventListener에 SSEChannelManager 조건 누락
AIEventListener(@ConditionalOnBean {AIService, SqsService})에서 SSEChannelManager는 RedisStoreService 조건으로만 생성되어, 로컬/테스트 프로필 등 RedisStoreService 빈이 없을 경우 애플리케이션 부팅 시 DI 실패가 발생합니다.
다음 중 하나를 적용하세요.옵션 A(권장): SSEChannelManager를 조건에 추가
- @ConditionalOnBean({AIService.class, SqsService.class}) + @ConditionalOnBean({AIService.class, SqsService.class, SSEChannelManager.class})옵션 B: Optional 주입 + 사용부 가드
- private final SSEChannelManager sseChannelManager; + private final Optional<SSEChannelManager> sseChannelManager;사용부에서
sseChannelManager.ifPresent(m -> m.sendProgressMessage(...));로 감싸기.
🧹 Nitpick comments (8)
edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/AIProgressMessage.java (2)
11-13: 널 입력 방지 가드 추가 권장생성 팩토리에 간단한
null가드를 넣어 직렬화/전달 시 NPE를 예방하세요.import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.Objects; @@ - public static AIProgressMessage of(final Long taskId, final String message) { - return new AIProgressMessage(taskId, message); - } + public static AIProgressMessage of(final Long taskId, final String message) { + return new AIProgressMessage( + Objects.requireNonNull(taskId, "taskId"), + Objects.requireNonNull(message, "message") + ); + }
15-21: 하드코딩된 “3가지 버전” 제거(가변 버전 수 대응)버전 수가 변경될 경우 메시지가 실제 동작과 어긋납니다. 오버로드를 추가해 확장성/일관성을 확보하세요. 기존 시그니처는 3을 기본값으로 위임하면 하위 호환됩니다.
- public static AIProgressMessage generationStarted(final Long taskId) { - return new AIProgressMessage(taskId, "3가지 버전 생성 중"); - } + public static AIProgressMessage generationStarted(final Long taskId) { + return generationStarted(taskId, 3); + } + public static AIProgressMessage generationStarted(final Long taskId, final int variantCount) { + return new AIProgressMessage(taskId, variantCount + "가지 버전 생성 중"); + } @@ - public static AIProgressMessage generationCompleted(final Long taskId) { - return new AIProgressMessage(taskId, "3가지 버전 생성 완료"); - } + public static AIProgressMessage generationCompleted(final Long taskId) { + return generationCompleted(taskId, 3); + } + public static AIProgressMessage generationCompleted(final Long taskId, final int variantCount) { + return new AIProgressMessage(taskId, variantCount + "가지 버전 생성 완료"); + }edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/SSEMessage.java (1)
3-14: 문자열 기반 type → enum으로 강타입화매직스트링을 제거해 오타/변경에 견고하게 만듭니다. 외부 사용처 변경 영향도 낮습니다(팩토리 메서드 유지).
-public record SSEMessage( - Long taskId, - String type, - Object data -) { +public record SSEMessage( + Long taskId, + Type type, + Object data +) { + public enum Type { PROGRESS, RESPONSE } @@ - public static SSEMessage progress(final Long taskId, final String message) { - return new SSEMessage(taskId, "PROGRESS", new ProgressData(message)); + public static SSEMessage progress(final Long taskId, final String message) { + return new SSEMessage(taskId, Type.PROGRESS, new ProgressData(message)); @@ - public static SSEMessage response(final Long taskId, final String finalContent, final Integer version) { - return new SSEMessage(taskId, "RESPONSE", new ResponseData(finalContent, version)); + public static SSEMessage response(final Long taskId, final String finalContent, final Integer version) { + return new SSEMessage(taskId, Type.RESPONSE, new ResponseData(finalContent, version));edukit-core/src/main/java/com/edukit/core/studentrecord/db/entity/StudentRecordAITask.java (1)
56-62: 시간 획득 방식 개선(테스트 용이성/타임존 일관성)
LocalDateTime.now()는 테스트 고정과 타임존 일관성이 떨어집니다. 서비스 레이어에서Clock을 주입해 시각을 인자로 전달하거나Instant기반으로 기록 후 뷰단에서 변환하는 방식을 권장합니다.예: 서비스에서 호출
task.start(clock.instant()); task.complete(clock.instant());엔티티
public void start(Instant now) { this.startedAt = LocalDateTime.ofInstant(now, ZoneOffset.UTC); } public void complete(Instant now) { this.completedAt = LocalDateTime.ofInstant(now, ZoneOffset.UTC); }edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java (2)
44-46: 하드코딩된 “3가지 버전” 표현 제거 및 키 재사용주석의 “3가지”는 구현 변경 시 부정확해집니다. 또한
String.valueOf(taskId)반복을 피하면 좋습니다.- // SSE로 3가지 버전 생성 시작 알림 - sseChannelManager.sendProgressMessage(String.valueOf(taskId), AIProgressMessage.generationStarted(taskId)); + // SSE로 버전 생성 시작 알림 + final String taskKey = Long.toString(taskId); + sseChannelManager.sendProgressMessage(taskKey, AIProgressMessage.generationStarted(taskId));
97-99: 에러 경로에도 진행상황(실패) 알림을 전송하세요현재 완료 알림만 있어 실패 시 클라이언트가 영원히 “진행 중”으로 보일 수 있습니다. 에러 콜백에서도 진행상황(실패) 메시지를 전송하는 것을 권장합니다.
원하시면
AIProgressMessage에generationFailed(...)팩토리를 추가하는 변경과 함께 적용 diff를 준비해드리겠습니다.edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java (2)
55-60: 로그에 버전 정보 포함응답 전송 로그에 버전 번호를 함께 남기면 트러블슈팅이 수월합니다.
- log.info("Sent response message to SSE channel for taskId: {}", taskId); + log.info("Sent response message to SSE channel for taskId: {}, version: {}", taskId, message.version());또한
message.taskId()/SSEMessage.response(...)의 taskId 타입(String/Long) 일치 여부를 확인해주세요. 필요 시String.valueOf(message.taskId())로 안전하게 변환하세요.
74-88: 중복된 SSE 전송 로직 DRY화
sendMessage와sendProgressMessage가 동일 패턴을 반복합니다. 공통 헬퍼로 추출해 유지보수성을 높이세요.- public void sendProgressMessage(final String taskId, final AIProgressMessage message) { - SseEmitter emitter = activeChannels.get(taskId); - if (emitter != null) { - try { - SSEMessage sseMessage = SSEMessage.progress(message.taskId(), message.message()); - emitter.send(SseEmitter.event() - .name(SSE_EVENT_NAME) - .data(sseMessage)); - log.info("Sent progress message to SSE channel for taskId: {}, message: {}", taskId, message.message()); - } catch (IOException e) { - log.error("Failed to send progress message to SSE channel for taskId: {}", taskId, e); - removeChannel(taskId); - } - } - } + public void sendProgressMessage(final String taskId, final AIProgressMessage message) { + sendSse(taskId, SSEMessage.progress(message.taskId(), message.message()), + () -> log.info("Sent progress message to SSE channel for taskId: {}, message: {}", taskId, message.message())); + }추가(파일 내 임의 위치, 클래스 private 메서드):
private void sendSse(final String taskId, final SSEMessage sseMessage, final Runnable onSuccess) { SseEmitter emitter = activeChannels.get(taskId); if (emitter != null) { try { emitter.send(SseEmitter.event().name(SSE_EVENT_NAME).data(sseMessage)); onSuccess.run(); } catch (IOException e) { log.error("Failed to send SSE to channel for taskId: {}", taskId, e); removeChannel(taskId); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
edukit-api/src/main/resources/db/migration/V8__Remove_student_record_ai_task_columns.sql(1 hunks)edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java(4 hunks)edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/AIProgressMessage.java(1 hunks)edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/SSEMessage.java(1 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/db/entity/StudentRecordAITask.java(1 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/db/enums/AITaskStatus.java(0 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java(4 hunks)
💤 Files with no reviewable changes (1)
- edukit-core/src/main/java/com/edukit/core/studentrecord/db/enums/AITaskStatus.java
🔇 Additional comments (3)
edukit-api/src/main/resources/db/migration/V8__Remove_student_record_ai_task_columns.sql (1)
3-4: 방어적 컬럼 제거:IF EXISTS추가ALTER TABLE student_record_ai_task DROP COLUMN IF EXISTS status;rg 검색 결과 해당 컬럼/열거형 참조가 발견되지 않았으나, 운영 DB 호환성 강화를 위해 방어적으로
IF EXISTS를 사용하고, 인덱스·뷰·트리거 의존성이 없는지 프로덕션 환경에서 수동 검토하세요.edukit-core/src/main/java/com/edukit/core/studentrecord/db/entity/StudentRecordAITask.java (1)
42-54: status 필드 제거 검증 완료
StudentRecordAITask 엔티티 및 관련 JPQL/네이티브 쿼리에서 status 참조가 전혀 남아있지 않음을 확인했습니다.edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java (1)
32-32: SSE 이벤트 이름 호환성 확인
이 레포지토리에는SSE_EVENT_NAME = "ai-message"를 사용하는 발신 측(SSEChannelManager)만 존재하며, 수신 측 구현이 없으므로 프론트엔드/클라이언트가 해당 이벤트명을 수신하도록 업데이트되었는지 반드시 확인하세요.
| public record SSEMessage( | ||
| Long taskId, | ||
| String type, | ||
| Object data | ||
| ) { |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
JSON 필드 네이밍 일관성 확인 필요(snake_case vs camelCase)
AIProgressMessage는 task_id를 사용하지만 여기서는 taskId로 직렬화될 가능성이 있습니다. 클라이언트 계약이 snake_case라면 명시적 어노테이션을 추가하세요. 프로젝트 전역 NamingStrategy 설정 여부도 확인 부탁드립니다.
package com.edukit.core.common.event.ai.dto;
+import com.fasterxml.jackson.annotation.JsonProperty;
public record SSEMessage(
- Long taskId,
+ @JsonProperty("task_id")
+ Long taskId,
String type,
Object data
) {
@@
- public record ResponseData(
- String finalContent,
+ public record ResponseData(
+ @JsonProperty("final_content")
+ String finalContent,
Integer version
) {
}전역 설정을 쓰는 경우 아래로 확인 부탁드립니다.
Also applies to: 16-24
🏁 Script executed:
#!/bin/bash
# Jackson 전역 네이밍 전략 사용 여부 탐색
rg -nP 'PropertyNamingStrategy|PropertyNamingStrategies|SNAKE_CASE' --type=java --type=yml -C2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Jackson 전역 네이밍 전략 설정 탐색(확장)
rg -nP '(PropertyNamingStrategy|PropertyNamingStrategies|property-naming-strategy|SNAKE_CASE)' -g '*.java' -g '*.yml' -g '*.yaml' -C2Length of output: 0
Jackson 전역 NamingStrategy 미설정: JSON 필드에 snake_case 어노테이션 추가 필요
프로젝트 내 PropertyNamingStrategy/SNAKE_CASE 설정을 찾을 수 없으므로, 클라이언트 계약이 snake_case일 경우 아래 필드에 명시적 @JsonProperty 어노테이션을 추가하세요:
- SSEMessage.taskId →
@JsonProperty("task_id") - ResponseData.finalContent →
@JsonProperty("final_content")
🤖 Prompt for AI Agents
In edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/SSEMessage.java
around lines 3 to 7, the record components lack explicit JSON property names for
a snake_case client contract; annotate the taskId component with
@JsonProperty("task_id") on the record header and also locate the ResponseData
class and annotate its finalContent field or accessor with
@JsonProperty("final_content") so serialized JSON uses snake_case; ensure
imports for com.fasterxml.jackson.annotation.JsonProperty are added and that
annotations are applied to the record component and the ResponseData
field/getter accordingly.
…ent in AIProgressMessage and SSEChannelManager
…inal content details
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java (1)
71-93: 중대: SSE 채널이 없으면 응답 카운트가 증가하지 않아 작업이 영원히 완료되지 않음현재는 emitter 존재 시에만 응답 카운트 증가 및 완료 처리가 수행됩니다. 클라이언트가 스트림을 열지 않거나 끊긴 경우, DB 상 작업 완료가 누락될 수 있습니다.
아래와 같이 카운팅/완료 로직을 채널 존재 여부와 분리해 항상 수행해 주세요.
- public void sendMessage(final String taskId, final AIResponseMessage message) { - SseEmitter emitter = activeChannels.get(taskId); - if (emitter != null) { - try { - SSEMessage sseMessage = SSEMessage.response(message.taskId(), message.reviewedContent(), - message.version()); - emitter.send(SseEmitter.event() - .name(SSE_EVENT_NAME) - .data(sseMessage)); - log.info("Sent response message to SSE channel for taskId: {}", taskId); - - Long responseCount = redisStoreService.increment(responseCountKey(taskId), RESPONSE_COUNT_TTL); - log.info("Response count for taskId {}: {}", taskId, responseCount); - - if (responseCount >= MAX_RESPONSE_COUNT) { - completeTask(taskId); - } - } catch (IOException e) { - log.error("Failed to send message to SSE channel for taskId: {}", taskId, e); - removeChannel(taskId); - } - } - } + public void sendMessage(final String taskId, final AIResponseMessage message) { + // 1) 응답 카운팅 및 완료 판정은 항상 수행 + Long responseCount = redisStoreService.increment(responseCountKey(taskId), RESPONSE_COUNT_TTL); + log.info("Response count for taskId {}: {}", taskId, responseCount); + + // 2) SSE 채널이 있으면 전송 시도 + SseEmitter emitter = activeChannels.get(taskId); + if (emitter != null) { + try { + SSEMessage sseMessage = SSEMessage.response(message.taskId(), message.reviewedContent(), message.version()); + emitter.send(SseEmitter.event().name(SSE_EVENT_NAME).data(sseMessage)); + log.info("Sent response message to SSE channel for taskId: {}", taskId); + } catch (IOException e) { + log.error("Failed to send message to SSE channel for taskId: {}", taskId, e); + removeChannel(taskId); + } + } else { + log.info("No active SSE channel for taskId: {}, response stored/processed only", taskId); + } + + // 3) 최대 응답 수 도달 시 작업 완료 + if (responseCount != null && responseCount >= MAX_RESPONSE_COUNT) { + completeTask(taskId); + } + }
🧹 Nitpick comments (7)
edukit-core/src/main/java/com/edukit/core/studentrecord/service/enums/AITaskStatus.java (1)
17-23: 알 수 없는 상태 처리 시 null 리턴은 클라이언트 혼란을 유발할 수 있음
getMessageByStatus가 미매칭 시null을 리턴합니다. 호출부(SSE 전송)에서 그대로 송신되면data.message: null이 나갈 수 있습니다. 최소한 호출부에서 null 가드 또는 여기서 Optional로 감싸는 편이 안전합니다.다음처럼 Optional을 리턴하도록 변경을 검토해 주세요(호출부 수정 필요):
- public static String getMessageByStatus(final String status) { - return Arrays.stream(values()) - .filter(aiTaskStatus -> aiTaskStatus.getStatus().equals(status)) - .map(AITaskStatus::getMessage) - .findFirst() - .orElse(null); - } + public static java.util.Optional<String> getMessageByStatus(final String status) { + return Arrays.stream(values()) + .filter(aiTaskStatus -> aiTaskStatus.getStatus().equals(status)) + .map(AITaskStatus::getMessage) + .findFirst(); + }edukit-api/src/main/java/com/edukit/studentrecord/controller/StudentRecordAIApi.java (1)
142-143: 고정된 “3회 전송 시 종료” 문구는 상수 변경 시 문서 불일치 위험
MAX_RESPONSE_COUNT가 바뀌면 문서와 실제 동작이 어긋납니다. “최대 3회(현재 기준)”처럼 표현을 완화하거나, 운영 가이드에만 명시하는 방식을 고려해 주세요.edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/AIProgressMessage.java (1)
1-20: 진행 이벤트 도입 LGTM팩토리 메서드 분리로 사용성이 좋습니다. 추후 실패 케이스를 전파하려면 enum과 함께
generationFailed(...)확장 여지 있습니다.edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java (1)
44-46: SSE 진행 알림 타이밍 적절 — 에러 케이스도 고려해 주세요시작/완료 알림 포인트 배치가 합리적입니다. 다만 오류 발생 시 진행상태가 “완료”로 마무리되지 않으므로, 실패 상태(예: PHASE1_FAILED)를 추가해 전파하는 것을 권장합니다.
가능한 변경 예:
AITaskStatus에PHASE1_FAILED추가error콜백에서sendProgressMessage(taskId, AIProgressMessage.generationFailed(taskId))전송Also applies to: 97-100
edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java (3)
67-69: 메서드명 오타(naming): hasActivateChannel → hasActiveChannel 권장공용 API라면 오타는 누적 혼란을 줍니다. 리네임 + 사용처 변경을 권장합니다.
- public boolean hasActivateChannel(final String taskId) { + public boolean hasActiveChannel(final String taskId) { return activeChannels.containsKey(taskId); }
120-131: 채널 제거 시 응답 카운트 키도 정리하는 것이 안전현재
response-count:키는 TTL(5분) 만료를 기다립니다. 즉시 청소하면 재사용/오탐을 더 줄일 수 있습니다.} finally { redisStoreService.delete(sseChannelKey(taskId)); redisStoreService.delete(taskStatusKey(taskId)); + redisStoreService.delete(responseCountKey(taskId)); log.info("Removed SSE channel for taskId: {}", taskId); }
135-146: 현재 모든 taskId는 숫자 전용이므로 변환 실패 위험 없음
registerTaskChannel,AIEventListener,RedisStreamConsumer등에서 모두String.valueOf(long)으로 전달되어Long.valueOf(taskId)변환이 항상 성공합니다. 향후 UUID 등 비숫자 ID 체계 도입 시에는 메소드 시그니처를Long으로 변경하거나 Javadoc으로 숫자 전용임을 명시하는 리팩터링을 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
edukit-api/src/main/java/com/edukit/studentrecord/controller/StudentRecordAIApi.java(2 hunks)edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java(3 hunks)edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/AIProgressMessage.java(1 hunks)edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/AIResponseMessage.java(1 hunks)edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/DraftGenerationEvent.java(1 hunks)edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/SSEMessage.java(1 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java(6 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/service/enums/AITaskStatus.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/SSEMessage.java
🔇 Additional comments (3)
edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/AIResponseMessage.java (1)
7-7: 하위 호환성 점검 완료
AIResponseMessage 생성·소비 지점(edukit-core/src/main/java/com/edukit/core/studentrecord/service/RedisStreamConsumer.java, SSEChannelManager.java) 모두 String taskId로 일관되게 적용되었습니다.edukit-core/src/main/java/com/edukit/core/common/event/ai/dto/DraftGenerationEvent.java (1)
4-4: 이벤트 퍼블리셔/컨슈머 검증 완료
모든 DraftGenerationEvent.of 호출이 String 파라미터 기준(taskId, requestPrompt, byteCount, version, draftContent, traceId)으로 일치하며 추가 조치 불필요합니다.edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java (1)
64-74: (스크립트 실행 대기 중...)
edukit-api/src/main/java/com/edukit/studentrecord/controller/StudentRecordAIApi.java
Show resolved
Hide resolved
edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java
Show resolved
Hide resolved
edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java
Show resolved
Hide resolved
…ndling in AITaskStatus
…ce, StudentRecordAIController, and StudentRecordAIFacade
…i for consistency
… for improved consistency
…roved message handling
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
edukit-api/src/main/java/com/edukit/studentrecord/controller/StudentRecordAIApi.java (1)
176-184: 오류 예시의 코드/메시지 불일치 수정 필요"작업 미존재" 케이스는
SR-40406 / "AI 작업을 찾을 수 없습니다."가 맞습니다. 현재 예시는 학생 기록 미존재 코드/메시지를 사용합니다.적용 diff:
- { - "code": "SR-40401", - "message": "해당 학생 기록이 존재하지 않습니다." - } + { + "code": "SR-40406", + "message": "AI 작업을 찾을 수 없습니다." + }edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java (1)
119-131: Redis 키 정리 보장
activeChannels에 없으면 키가 삭제되지 않습니다. 키 삭제는 emitter 유무와 무관하게 보장하세요.적용 diff:
- public void removeChannel(final String taskId) { - SseEmitter emitter = activeChannels.remove(taskId); - if (emitter != null) { - try { - emitter.complete(); - } catch (Exception e) { - log.warn("SSE complete failed for taskId: {}", taskId, e); - } finally { - redisStoreService.delete(sseChannelKey(taskId)); - redisStoreService.delete(taskStatusKey(taskId)); - log.info("Removed SSE channel for taskId: {}", taskId); - } - } - } + public void removeChannel(final String taskId) { + SseEmitter emitter = activeChannels.remove(taskId); + try { + if (emitter != null) { + emitter.complete(); + } + } catch (Exception e) { + log.warn("SSE complete failed for taskId: {}", taskId, e); + } finally { + redisStoreService.delete(sseChannelKey(taskId)); + redisStoreService.delete(taskStatusKey(taskId)); + log.info("Removed SSE channel for taskId: {}", taskId); + } + }
♻️ Duplicate comments (2)
edukit-api/src/main/java/com/edukit/studentrecord/controller/StudentRecordAIApi.java (1)
221-223: PathVariable를 String으로 통일한 점 확인이전 리뷰 지적 사항이 반영되었습니다.
레포 전반에 남아 있을 수 있는
long|Long taskId사용처를 확인해 주세요:#!/bin/bash rg -nP --type=java -C2 '(?:@PathVariable|@RequestParam|\\b)\\s*(?:long|Long)\\s+taskId\\b'edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java (1)
39-45: Emitter 콜백 등록으로 누수 방지클라이언트 종료/타임아웃 시 자원 정리를 보장하세요. (Controller에도 있으나, Manager 레벨에서도 방어적으로 두는 것을 권장)
적용 diff:
public void registerTaskChannel(final String taskId, final SseEmitter emitter) { String serverId = serverInstanceManager.getServerId(); redisStoreService.store(sseChannelKey(taskId), serverId, Duration.ofHours(1)); activeChannels.put(taskId, emitter); log.info("Registered SSE channel for taskId: {} on server: {}", taskId, serverId); + + emitter.onCompletion(() -> removeChannel(taskId)); + emitter.onTimeout(() -> { + log.warn("SSE timeout for taskId: {}", taskId); + removeChannel(taskId); + }); + emitter.onError(ex -> { + log.warn("SSE error for taskId: {}", taskId, ex); + removeChannel(taskId); + });
🧹 Nitpick comments (5)
edukit-core/src/main/java/com/edukit/core/studentrecord/exception/StudentRecordErrorCode.java (1)
18-20: 마지막 enum 상수 뒤 불필요한 콤마 제거 권장컴파일엔 문제 없으나 스타일상 제거가 깔끔합니다.
적용 diff:
- NUMBER_FORMAT_EXCEPTION("SR-40010", "숫자 형식이 올바르지 않습니다."),; + NUMBER_FORMAT_EXCEPTION("SR-40010", "숫자 형식이 올바르지 않습니다.");edukit-core/src/main/java/com/edukit/core/studentrecord/service/AITaskService.java (2)
37-39: 읽기 전용 트랜잭션 옵션 제안조회 전용이므로
@Transactional(readOnly = true)적용을 권장합니다.적용 diff:
- public void validateUserTask(final long memberId, final String taskId) { + @Transactional(readOnly = true) + public void validateUserTask(final long memberId, final String taskId) {
44-50: 예외 래핑 시 원인(cause) 포함원인 스택 추적을 보존하도록
cause를 전달하세요.적용 diff:
- } catch (NumberFormatException e) { - throw new StudentRecordException(StudentRecordErrorCode.NUMBER_FORMAT_EXCEPTION); + } catch (NumberFormatException e) { + throw new StudentRecordException(StudentRecordErrorCode.NUMBER_FORMAT_EXCEPTION, e);edukit-api/src/main/java/com/edukit/studentrecord/facade/StudentRecordAIFacade.java (1)
43-49: SSE 타임아웃 상수화/설정값 노출 고려
new SseEmitter(10 * 60 * 1000L)은 하드코딩되어 있어 설정으로 분리하면 운영 조정이 수월합니다.edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java (1)
66-68: 메서드명 타이포
hasActivateChannel→hasActiveChannel로 수정 권장.적용 diff:
- public boolean hasActivateChannel(final String taskId) { + public boolean hasActiveChannel(final String taskId) { return activeChannels.containsKey(taskId); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
edukit-api/src/main/java/com/edukit/studentrecord/controller/StudentRecordAIApi.java(4 hunks)edukit-api/src/main/java/com/edukit/studentrecord/controller/StudentRecordAIController.java(1 hunks)edukit-api/src/main/java/com/edukit/studentrecord/facade/StudentRecordAIFacade.java(1 hunks)edukit-api/src/main/java/com/edukit/studentrecord/facade/response/StudentRecordTaskResponse.java(1 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/exception/StudentRecordErrorCode.java(1 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/service/AITaskService.java(1 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java(6 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/service/enums/AITaskStatus.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
edukit-core/src/main/java/com/edukit/core/studentrecord/service/enums/AITaskStatus.java (1)
edukit-core/src/main/java/com/edukit/core/studentrecord/exception/StudentRecordException.java (1)
StudentRecordException(5-14)
🔇 Additional comments (10)
edukit-api/src/main/java/com/edukit/studentrecord/facade/response/StudentRecordTaskResponse.java (2)
7-7: taskId 타입을 String으로 전환한 결정에 동의API 전반의 문자열 기반 taskId 표준화와 일관됩니다. Swagger 예시도 문자열로 맞습니다.
9-11: 팩토리 메서드 시그니처 일관화 OK
of(final String taskId)로의 변경이 상위 레이어와 정합합니다.edukit-api/src/main/java/com/edukit/studentrecord/controller/StudentRecordAIController.java (1)
39-41: 스트리밍 엔드포인트의 taskId를 String으로 변경한 점 확인Facade/Service 계층의 변경과 일치하며, 경로 변수 파싱 책임이 서비스로 위임되어 좋습니다.
edukit-api/src/main/java/com/edukit/studentrecord/facade/StudentRecordAIFacade.java (2)
40-41: 문자열 taskId 응답 매핑 적절
String.valueOf(task.getId())로 공개 DTO에 맞춘 점 좋습니다.
51-53: 채널 정리 위임 방식 OKController에서의 콜백과 Manager의 정리가 호환됩니다.
edukit-api/src/main/java/com/edukit/studentrecord/controller/StudentRecordAIApi.java (2)
142-143: 스트리밍 설명 갱신 적절진행/응답 메시지 및 자동 종료 정책이 구현과 일치합니다.
154-168: SSE 이벤트 예시가 구현과 일치이벤트명(
ai-message)과 페이로드 형태가 Manager 상수/DTO와 일관됩니다.edukit-core/src/main/java/com/edukit/core/studentrecord/service/enums/AITaskStatus.java (1)
12-24: 진행 상태 → 사용자 메시지 매핑 명확간결하고 의도가 분명합니다. 알 수 없는 상태에 예외를 던지는 정책도 타당합니다.
edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java (2)
34-38: 상수 일관성 OK
SSE_EVENT_NAME = "ai-message"가 API 문서와 일치합니다.
70-80: 검증 완료: SSE 메시지 응답의finalContentJSON 필드가 문서 예시와 일치합니다.
| // SSE 채널 등록 시 현재 진행 상태가 있다면 전송 | ||
| String currentStatus = redisStoreService.get(taskStatusKey(taskId)); | ||
| if (currentStatus != null) { | ||
| try { | ||
| String message = AITaskStatus.getMessageByStatus(currentStatus); | ||
| SSEMessage sseMessage = SSEMessage.progress(taskId, message); | ||
| emitter.send(SseEmitter.event() | ||
| .name(SSE_EVENT_NAME) | ||
| .data(sseMessage)); | ||
| log.info("Sent stored progress message to SSE channel for taskId: {}, message: {}", taskId, message); | ||
| } catch (IOException e) { | ||
| log.error("Failed to send stored progress message to SSE channel for taskId: {}", taskId, e); | ||
| removeChannel(taskId); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
저장된 진행 상태 전송 시 알 수 없는 상태 예외 처리 누락
AITaskStatus.getMessageByStatus가 예외를 던지면 채널 등록이 실패할 수 있습니다. 안전하게 무시/경고 로그로 처리하세요.
적용 diff:
String currentStatus = redisStoreService.get(taskStatusKey(taskId));
if (currentStatus != null) {
try {
String message = AITaskStatus.getMessageByStatus(currentStatus);
SSEMessage sseMessage = SSEMessage.progress(taskId, message);
- emitter.send(SseEmitter.event()
- .name(SSE_EVENT_NAME)
- .data(sseMessage));
+ emitter.send(SseEmitter.event().name(SSE_EVENT_NAME).data(sseMessage));
log.info("Sent stored progress message to SSE channel for taskId: {}, message: {}", taskId, message);
+ } catch (StudentRecordException sre) {
+ log.warn("Unknown stored status for taskId: {}, status: {}", taskId, currentStatus);
} catch (IOException e) {
log.error("Failed to send stored progress message to SSE channel for taskId: {}", taskId, e);
removeChannel(taskId);
}
}📝 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.
| // SSE 채널 등록 시 현재 진행 상태가 있다면 전송 | |
| String currentStatus = redisStoreService.get(taskStatusKey(taskId)); | |
| if (currentStatus != null) { | |
| try { | |
| String message = AITaskStatus.getMessageByStatus(currentStatus); | |
| SSEMessage sseMessage = SSEMessage.progress(taskId, message); | |
| emitter.send(SseEmitter.event() | |
| .name(SSE_EVENT_NAME) | |
| .data(sseMessage)); | |
| log.info("Sent stored progress message to SSE channel for taskId: {}, message: {}", taskId, message); | |
| } catch (IOException e) { | |
| log.error("Failed to send stored progress message to SSE channel for taskId: {}", taskId, e); | |
| removeChannel(taskId); | |
| } | |
| } | |
| // SSE 채널 등록 시 현재 진행 상태가 있다면 전송 | |
| String currentStatus = redisStoreService.get(taskStatusKey(taskId)); | |
| if (currentStatus != null) { | |
| try { | |
| String message = AITaskStatus.getMessageByStatus(currentStatus); | |
| SSEMessage sseMessage = SSEMessage.progress(taskId, message); | |
| emitter.send(SseEmitter.event().name(SSE_EVENT_NAME).data(sseMessage)); | |
| log.info("Sent stored progress message to SSE channel for taskId: {}, message: {}", taskId, message); | |
| } catch (StudentRecordException sre) { | |
| log.warn("Unknown stored status for taskId: {}, status: {}", taskId, currentStatus); | |
| } catch (IOException e) { | |
| log.error("Failed to send stored progress message to SSE channel for taskId: {}", taskId, e); | |
| removeChannel(taskId); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
edukit-core/src/main/java/com/edukit/core/studentrecord/service/SSEChannelManager.java
around lines 45 to 59, AITaskStatus.getMessageByStatus may throw an exception
which currently would abort channel registration; wrap the call that obtains the
message in a separate try/catch (catch RuntimeException or the specific
unchecked exception thrown) so that if it fails you log a warning and skip
sending the stored progress message without removing the channel or failing
registration, while preserving the existing IOException handling for
emitter.send.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
edukit-core/src/main/java/com/edukit/core/common/service/SqsService.java (1)
5-5: 기능 호환성 보장을 위한 디폴트 메서드 오버로드 및 계약 명시File: edukit-core/src/main/java/com/edukit/core/common/service/SqsService.java
외부 구현체와의 바이너리/소스 호환성을 유지하려면, 다음과 같이 기본sendMessage(Object message)디폴트 메서드를 추가하고
idempotencyKey의 null·공백 허용 여부를 Javadoc으로 명시하세요.제안 diff:
public interface SqsService { + /** + * Sends a message without an idempotency key. + * @param message 전송할 메시지, null 불가 + */ + default void sendMessage(Object message) { + sendMessage(message, null); + } void sendMessage(Object message, String idempotencyKey); }edukit-external/src/main/java/com/edukit/external/aws/sqs/SqsServiceImpl.java (4)
57-62: 메시지 크기 검증이 Attribute 크기를 반영하지 않습니다SQS는 Body+Attributes 모두 합산 256KB 제한입니다. 현재는 Body만 검사합니다. Attribute(특히
idempotencyKey)가 추가되면 경계값에서 실패 위험이 있습니다. Attribute를 포함한 총량 검증으로 이관을 권장합니다.권장 diff(개념 예):
- private void validateMessageSize(final int messageSizeBytes) { - if (messageSizeBytes > SQS_MAX_MESSAGE_SIZE) { + private void validateMessageSize(final int totalSizeBytes) { + if (totalSizeBytes > SQS_MAX_MESSAGE_SIZE) { log.error("메시지 크기가 SQS 제한(256KB)을 초과했습니다 - 메시지 크기 초과로 전송 실패: {} bytes", messageSizeBytes); throw new SQSException(SQSErrorCode.MESSAGE_SIZE_EXCEEDED); } }그리고
sendMessage(...)에서 Body만이 아니라sendMessageInternal(...)에서 Attribute 구성 후 아래처럼 총량을 계산해 호출:- validateMessageSize(messageSizeBytes); + // ... sendMessageInternal에서 계산- private void sendMessageInternal(final String messageBody, final String idempotencyKey) { + private void sendMessageInternal(final String messageBody, final String idempotencyKey) { Map<String, MessageAttributeValue> messageAttributes = new HashMap<>(); ... + int attrsBytes = messageAttributes.entrySet().stream() + .mapToInt(e -> e.getKey().getBytes(StandardCharsets.UTF_8).length + + (e.getValue().stringValue() != null ? e.getValue().stringValue().getBytes(StandardCharsets.UTF_8).length : 0)) + .sum(); + int totalSize = messageBody.getBytes(StandardCharsets.UTF_8).length + attrsBytes; + validateMessageSize(totalSize);
83-88:idempotencyKey공백 문자열 처리널만 체크하면 공백 키가 Attribute로 붙을 수 있습니다.
StringUtils.hasText(...)로 보강을 권장합니다.권장 diff:
+import org.springframework.util.StringUtils; ... - if (idempotencyKey != null) { + if (StringUtils.hasText(idempotencyKey)) { messageAttributes.put("idempotencyKey", MessageAttributeValue.builder() .dataType("String") .stringValue(idempotencyKey) .build()); }
90-96: SQS 멱등 보장 목적이라면 FIFO 설정/필드가 필요합니다멱등키를 Attribute로만 전송하면 “소비자 측 중복 제거”에만 활용됩니다. 전송 레벨에서의 멱등/순서 보장을 의도했다면 FIFO 큐 +
messageGroupId/messageDeduplicationId가 필요합니다. 현재는 두 필드가 미설정입니다(표준 큐라면 그대로 OK).
- 실제 큐가 FIFO(.fifo)인지, 표준 큐인지 확인 부탁드립니다.
- FIFO라면 아래처럼 조건부 설정을 권장합니다.
권장 diff(큐 URL로 단순 판별 예):
+import java.util.UUID; +import org.springframework.util.StringUtils; ... - final SendMessageRequest request = sqsClient.sendMessage(request); + boolean isFifo = sqsProperties.queueUrl() != null && sqsProperties.queueUrl().endsWith(".fifo"); + + SendMessageRequest.Builder builder = SendMessageRequest.builder() + .queueUrl(sqsProperties.queueUrl()) + .messageBody(messageBody) + .messageAttributes(messageAttributes); + + if (isFifo) { + builder = builder + .messageGroupId("ai-draft-" + (StringUtils.hasText(userId) ? userId : "default")) + .messageDeduplicationId(StringUtils.hasText(idempotencyKey) ? idempotencyKey : UUID.randomUUID().toString()); + } + + final SendMessageRequest request = builder.build(); + final SendMessageResponse response = sqsClient.sendMessage(request);FIFO가 아니라면(표준 큐) 현재 구현대로 Attribute로 내려보내고, 컨슈머에서 멱등 처리하는 전략이면 충분합니다. 해당 전략을 README/컨슈머 코드에 명시해 주세요.
Also applies to: 83-88
97-99: 로그: 민감도 및 가시성 밸런스 OK, 단 null 표시를 명시적으로
idempotencyKey가 null일 수 있으니 로그에 기본값을 넣어 노이즈를 줄이는 것도 방법입니다(선택).권장 diff:
- log.info("SQS 메시지 전송 완료 - MessageId: {}, TraceId: {}, IdempotencyKey: {}", - response.messageId(), traceId, idempotencyKey); + log.info("SQS 메시지 전송 완료 - MessageId: {}, TraceId: {}, IdempotencyKey: {}", + response.messageId(), traceId, (idempotencyKey != null ? idempotencyKey : "N/A"));edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java (2)
44-46: SSE 전송 실패가 전체 플로우를 막지 않도록 캡슐화SSE 채널이 끊긴 경우 예외가 전파되면 작업 흐름에 영향이 있을 수 있습니다. try-catch로 흡수하고 로그만 남기는 것을 권장합니다.
권장 diff:
- // SSE로 3가지 버전 생성 시작 알림 - sseChannelManager.sendProgressMessage(taskId, AIProgressMessage.generationStarted(taskId)); + // SSE로 3가지 버전 생성 시작 알림 + try { + sseChannelManager.sendProgressMessage(taskId, AIProgressMessage.generationStarted(taskId)); + } catch (Exception sseEx) { + log.warn("SSE 시작 알림 전송 실패 - taskId: {}, reason: {}", taskId, sseEx.getMessage()); + }
99-101: 완료 알림도 보호적 전송 권장완료 시점 SSE도 동일하게 보호해 주세요.
권장 diff:
- // SSE로 3가지 버전 생성 완료 알림 - sseChannelManager.sendProgressMessage(taskId, - AIProgressMessage.generationCompleted(taskId)); + // SSE로 3가지 버전 생성 완료 알림 + try { + sseChannelManager.sendProgressMessage(taskId, AIProgressMessage.generationCompleted(taskId)); + } catch (Exception sseEx) { + log.warn("SSE 완료 알림 전송 실패 - taskId: {}, reason: {}", taskId, sseEx.getMessage()); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java(4 hunks)edukit-core/src/main/java/com/edukit/core/common/service/SqsService.java(1 hunks)edukit-external/src/main/java/com/edukit/external/aws/sqs/SqsServiceImpl.java(2 hunks)
🔇 Additional comments (2)
edukit-external/src/main/java/com/edukit/external/aws/sqs/SqsServiceImpl.java (1)
34-55: 직렬화/전송 예외 처리 흐름은 적절합니다메시지 직렬화/전송 예외를 구분해 래핑하는 전략은 👍 입니다.
edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java (1)
74-76: 멱등키 구성은 명확하고 단순해서 👍, 다만 큐 타입에 따라 의미가 달라집니다
taskId-version포맷은 재시도 시 중복 전송 방지에 유용합니다. 다만 표준 큐면 컨슈머 쪽 중복 제거와 함께, FIFO 큐면messageDeduplicationId로 활용되도록 송신 측 설정이 필요합니다(별도 코멘트 참조).큐 타입(FIFO/Standard) 확인 부탁드립니다.
| private final SSEChannelManager sseChannelManager; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
조건부 빈 주입 조건에 SSEChannelManager 포함 권장
새 의존성을 생성자에 추가했지만 @ConditionalOnBean에는 포함되지 않아 특정 프로필/테스트 컨텍스트에서 빈 미존재 시 구동 실패 위험이 있습니다.
권장 diff:
-@ConditionalOnBean({AIService.class, SqsService.class})
+@ConditionalOnBean({AIService.class, SqsService.class, SSEChannelManager.class})📝 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 final SSEChannelManager sseChannelManager; | |
| @@ | |
| @ConditionalOnBean({AIService.class, SqsService.class, SSEChannelManager.class}) | |
| public class AIEventListener { | |
| private final SSEChannelManager sseChannelManager; | |
| // … | |
| } |
🤖 Prompt for AI Agents
In
edukit-core/src/main/java/com/edukit/core/common/event/ai/AIEventListener.java
around lines 33-34, you added a new constructor dependency private final
SSEChannelManager sseChannelManager but did not include SSEChannelManager in the
class's @ConditionalOnBean check; update the class-level conditional to include
SSEChannelManager.class (or alternatively make the dependency optional via
@Autowired(required=false) or ObjectProvider<SSEChannelManager>) so the bean
won't fail to instantiate in profiles/tests where SSEChannelManager is absent.
📣 Jira Ticket
EDMT-406
👩💻 작업 내용
OpenAI를 통한 생기부 응답 생성 시, 단계별 진행 상황을 전송하는 기능을 추가하였습니다.
📸 스크린 샷
Summary by CodeRabbit