Conversation
2 tasks
📝 WalkthroughWalkthroughProovy-ai 서버의 새로운 SSE 프로토콜(v2)에 맞춰 스트리밍 처리 로직을 마이그레이션합니다. ConversationController에서는 SSE 이벤트 구성을 표준 event 필드 기반으로 변경하고, ChatServiceImpl에서는 /stream 대신 /stream/v2 엔드포인트를 호출하며 ServerSentEvent 객체 기반 파싱과 토큰 누적 로직을 업데이트합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client as Client (Frontend)
participant Controller as ConversationController
participant Service as ChatServiceImpl
participant ProovyAi as Proovy-ai Server
Client->>Controller: POST /conversation/stream
Controller->>Service: streamResponse() 호출
Service->>ProovyAi: GET /stream/v2 (WebClient)
activate ProovyAi
loop SSE 이벤트 스트림
ProovyAi-->>Service: ServerSentEvent<String>
Note over Service: sse.event() & sse.data() 파싱
alt llm.token.delta 이벤트
Service->>Service: "delta" 키에서 토큰 추출
Service-->>Controller: ProovyAiStreamEvent (token_delta)
else run.completed 이벤트
Service->>Service: 스트림 완료 감지
Service-->>Controller: ProovyAiStreamEvent (run_completed)
else run.failed 이벤트
Service->>Service: BusinessException 발생
Service-->>Controller: 에러 이벤트
else 기타 이벤트
Service-->>Controller: ProovyAiStreamEvent (적절한 타입)
end
Controller->>Controller: ServerSentEvent.builder().event(...) 구성
Controller-->>Client: SSE 이벤트 발송
end
deactivate ProovyAi
Service-->>Controller: 스트림 종료
Controller-->>Client: 연결 종료
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/proovy/domain/conversation/controller/ConversationController.java (1)
151-162:⚠️ Potential issue | 🟠 Major
new ObjectMapper()매 이벤트마다 생성 – 핫패스 성능 이슈
convertToJson()은 모든 SSE 이벤트(토큰 수신마다)에서 호출되는데, 매번new ObjectMapper()를 생성합니다.ObjectMapper는 생성 비용이 크며 스레드 안전(thread-safe)하므로 싱글톤 빈으로 주입받아 공유해야 합니다. 또한 이 방식은ChatServiceImpl에 주입된ObjectMapper빈의 커스텀 설정(날짜 포맷, 모듈 등)을 무시합니다.🐛 수정 제안
ConversationController에ObjectMapper를 주입하고:`@RequiredArgsConstructor` public class ConversationController { private final ChatService chatService; private final ConversationQueryService conversationQueryService; + private final ObjectMapper objectMapper;
convertToJson에서 주입된 인스턴스를 사용:private String convertToJson(Object data) { try { if (data == null) { return "{}"; } - return new com.fasterxml.jackson.databind.ObjectMapper().writeValueAsString(data); + return objectMapper.writeValueAsString(data); } catch (Exception e) { log.warn("Failed to convert to JSON", e); return "{}"; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/proovy/domain/conversation/controller/ConversationController.java` around lines 151 - 162, convertToJson currently constructs a new com.fasterxml.jackson.databind.ObjectMapper on every call; change ConversationController to accept an ObjectMapper (the shared/Spring-injected bean used elsewhere, e.g., the one used in ChatServiceImpl) via constructor or field injection and update convertToJson to use that injected instance instead of new ObjectMapper(), avoiding per-event allocation and preserving global custom settings.
🧹 Nitpick comments (4)
src/main/java/com/proovy/domain/conversation/service/ChatServiceImpl.java (3)
263-266: 매직 스트링 상수화 권장
"llm.token.delta","delta","run.completed","run.failed"등 SSE v2 프로토콜에서 정의된 이벤트명/필드명들이 코드 내 여러 곳에 인라인으로 분산되어 있습니다. v2 명세 변경 시 누락될 위험이 있으므로 상수로 추출하는 것을 권장합니다.♻️ 상수 추출 제안
+ // SSE v2 이벤트 타입 상수 + private static final String EVENT_LLM_TOKEN_DELTA = "llm.token.delta"; + private static final String EVENT_RUN_COMPLETED = "run.completed"; + private static final String EVENT_RUN_FAILED = "run.failed"; + private static final String EVENT_ERROR = "error"; + // SSE v2 페이로드 필드 상수 + private static final String FIELD_TOKEN_DELTA = "delta"; + private static final String FIELD_THREAD_ID = "thread_id";이후 인라인 문자열 리터럴을 이 상수로 교체합니다 (예:
"llm.token.delta"→EVENT_LLM_TOKEN_DELTA등).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/proovy/domain/conversation/service/ChatServiceImpl.java` around lines 263 - 266, Extract all SSE v2 protocol literals used in ChatServiceImpl (e.g., "llm.token.delta", "delta", "run.completed", "run.failed") into well-named static final constants (suggest names like EVENT_LLM_TOKEN_DELTA, FIELD_DELTA, EVENT_RUN_COMPLETED, EVENT_RUN_FAILED) and replace every inline occurrence such as the checks against eventType and data.get("delta") with those constants; update the class-level declarations so other methods in ChatServiceImpl reference the constants and run a project-wide search to replace any identical literals in related classes to keep naming consistent.
224-233:doOnNext에서의 예외 발생 – 비관용적 패턴
doOnNext는 사이드이펙트 전용 연산자이며, 예외를 던져 흐름을 제어하는 것은 Reactor의 관용적 패턴이 아닙니다. RuntimeException이 전파되기는 하지만,.handle()연산자를 사용하면 이벤트를 선택적으로 에러 신호로 변환하는 의도를 더 명확히 표현할 수 있습니다.또한 현재 구조에서
run.failed가 도착하면doOnNext에서 예외가 발생하므로, 이 이벤트는 클라이언트에게 SSE 이벤트로 전달되지 않고 스트림이 에러로 종료됩니다. 클라이언트에서 세분화된 에러 처리가 필요한 경우 고려가 필요합니다.♻️ `.handle()` 연산자를 사용한 리팩터 제안
- .doOnNext(event -> { - Map<String, Object> data = event.getData(); - String eventType = event.getEvent(); - - // 에러 이벤트 처리 - if ("run.failed".equals(eventType) || "error".equals(eventType)) { - String errMsg = "Stream Error"; - if (data != null) { - errMsg = String.valueOf(data.getOrDefault("message", "Unknown error")); - } - throw new BusinessException(ErrorCode.CONV5002, "스트리밍 중 에러 발생: " + errMsg); - } - // ... thread_id, token 처리 - }) + .handle((event, sink) -> { + Map<String, Object> data = event.getData(); + String eventType = event.getEvent(); + + if ("run.failed".equals(eventType) || "error".equals(eventType)) { + String errMsg = data != null + ? String.valueOf(data.getOrDefault("message", "Unknown error")) + : "Stream Error"; + sink.error(new BusinessException(ErrorCode.CONV5002, "스트리밍 중 에러 발생: " + errMsg)); + return; + } + // ... thread_id, token 처리 + sink.next(event); + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/proovy/domain/conversation/service/ChatServiceImpl.java` around lines 224 - 233, The current doOnNext block throws a BusinessException when eventType is "run.failed" or "error", which is non-idiomatic for Reactor; replace this pattern by switching from doOnNext to using .handle((event, sink) -> { ... }) where you inspect event.getEvent() (eventType) and if it's an error case call sink.error(new BusinessException(ErrorCode.CONV5002, "스트리밍 중 에러 발생: " + errMsg)) and otherwise emit the event via sink.next(event); remove the throw from ChatServiceImpl so error signaling is expressed via the reactive sink and normal events continue to be emitted to clients.
453-459: 파싱 실패 시 AI 메시지 삭제가 발생하는 cascade 위험catch 블록이
event="error"인ProovyAiStreamEvent를 생성합니다. 이 이벤트는streamConversation의doOnNext(Line 227)에서BusinessException으로 전환되고, 이에 따라doOnError가 AI placeholder 메시지를 삭제합니다. 즉, JSON 파싱 실패 한 건(예: heartbeat의 예상치 못한 형식, 일시적 인코딩 오류)이 전체 스트림을 CONV5002 에러로 종료시키고 AI 메시지도 삭제합니다.파싱 실패를
Mono.empty()(이벤트 스킵)로 처리하거나, 치명적인 에러와 일시적 파싱 실패를 구분하는 것을 고려하세요.♻️ 파싱 실패를 스킵으로 처리하는 제안
} catch (Exception e) { - log.warn("[Chat] SSE 이벤트 파싱 실패: {}", sse, e); - return ProovyAiStreamEvent.builder() - .event("error") - .data(Map.of("error", "Failed to parse event", "raw", String.valueOf(sse.data()))) - .build(); + log.warn("[Chat] SSE 이벤트 파싱 실패 (스킵): sse={}", sse, e); + return null; // flatMap에서 null 반환 시 Mono.empty() 처리 필요 }또는
Mono.fromCallable대신Mono.defer를 사용하고 catch 블록에서Mono.empty()를 반환하도록 구조를 변경합니다.- return Mono.fromCallable(() -> { + return Mono.defer(() -> { try { // ... 파싱 로직 + return Mono.just(ProovyAiStreamEvent.builder()...build()); } catch (Exception e) { log.warn("[Chat] SSE 이벤트 파싱 실패 (스킵): {}", sse, e); - return ProovyAiStreamEvent.builder() - .event("error") - ... + return Mono.empty(); // 파싱 실패는 스킵 } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/proovy/domain/conversation/service/ChatServiceImpl.java` around lines 453 - 459, The current catch builds an error ProovyAiStreamEvent which later in streamConversation's doOnNext -> doOnError path triggers BusinessException and causes AI placeholder deletion; instead change the parsing error handling to skip non-fatal parse failures by returning an empty signal rather than an error event: modify the parsing block that currently returns ProovyAiStreamEvent.builder().event("error")... to return Mono.empty() (or refactor the surrounding Mono.fromCallable into Mono.defer so you can return Mono.empty() on parse failures), and ensure streamConversation/doOnNext still processes only real events while transient parse issues are ignored.src/main/java/com/proovy/domain/conversation/controller/ConversationController.java (1)
144-145: 스트림 에러 시 클라이언트에 에러 SSE 이벤트 미전송
doOnError는 로깅만 수행하며, 서비스 레이어에서BusinessException(예:run.failed, CONV5002)이 발생하면 SSE 연결이 에러 이벤트 없이 그냥 닫힙니다. 클라이언트는 왜 스트림이 종료됐는지 알 수 없습니다.
onErrorResume을 추가하여 에러 SSE 이벤트를 전송한 후 정상 완료 신호를 보내는 것을 권장합니다.♻️ 클라이언트에 에러 SSE 이벤트 전송 제안
.doOnError(error -> log.error("SSE stream error for userId: {}", userId, error)) + .onErrorResume(error -> { + String message = (error instanceof BusinessException be) + ? be.getMessage() + : "스트리밍 중 오류가 발생했습니다."; + String errorJson = "{\"message\":\"" + message.replace("\"", "\\\"") + "\"}"; + return Flux.just(ServerSentEvent.<String>builder() + .event("error") + .data(errorJson) + .build()); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/proovy/domain/conversation/controller/ConversationController.java` around lines 144 - 145, The SSE stream currently only logs errors via doOnError; update the reactive chain in ConversationController (the SSE-producing Flux where doOnError is used) to add onErrorResume that catches BusinessException (e.g., run.failed / CONV5002) and other errors, maps them to a Server-Sent-Event error payload (include an error code/message or minimal info safe for clients), emits that single SSE event, and then completes the stream normally; keep doOnError logging but ensure onErrorResume returns a Flux that emits the SSE error event and then completes so clients receive an explicit error SSE before the connection closes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/proovy/domain/conversation/service/ChatServiceImpl.java`:
- Around line 358-363: Remove the redundant/old Javadoc block that precedes the
callProovyAiStream method: delete the earlier comment /** Proovy-ai /stream 호출
*/ so only the new Javadoc /** Proovy-ai /stream/v2 호출 (SSE v2) */ remains
immediately above the callProovyAiStream(...) declaration; ensure there are no
duplicate JavaDoc blocks left for that method.
- Around line 422-427: Remove the stray/duplicate Javadoc comment above the
parseSseEvent method in ChatServiceImpl: delete the old short Javadoc block "/**
SSE 이벤트 파싱 */" so only the new Javadoc "SSE 이벤트 파싱 (v2: ServerSentEvent ->
ProovyAiStreamEvent)" remains; apply the same cleanup pattern as was done for
callProovyAiStream to avoid duplicated Javadoc comments.
---
Outside diff comments:
In
`@src/main/java/com/proovy/domain/conversation/controller/ConversationController.java`:
- Around line 151-162: convertToJson currently constructs a new
com.fasterxml.jackson.databind.ObjectMapper on every call; change
ConversationController to accept an ObjectMapper (the shared/Spring-injected
bean used elsewhere, e.g., the one used in ChatServiceImpl) via constructor or
field injection and update convertToJson to use that injected instance instead
of new ObjectMapper(), avoiding per-event allocation and preserving global
custom settings.
---
Nitpick comments:
In
`@src/main/java/com/proovy/domain/conversation/controller/ConversationController.java`:
- Around line 144-145: The SSE stream currently only logs errors via doOnError;
update the reactive chain in ConversationController (the SSE-producing Flux
where doOnError is used) to add onErrorResume that catches BusinessException
(e.g., run.failed / CONV5002) and other errors, maps them to a Server-Sent-Event
error payload (include an error code/message or minimal info safe for clients),
emits that single SSE event, and then completes the stream normally; keep
doOnError logging but ensure onErrorResume returns a Flux that emits the SSE
error event and then completes so clients receive an explicit error SSE before
the connection closes.
In `@src/main/java/com/proovy/domain/conversation/service/ChatServiceImpl.java`:
- Around line 263-266: Extract all SSE v2 protocol literals used in
ChatServiceImpl (e.g., "llm.token.delta", "delta", "run.completed",
"run.failed") into well-named static final constants (suggest names like
EVENT_LLM_TOKEN_DELTA, FIELD_DELTA, EVENT_RUN_COMPLETED, EVENT_RUN_FAILED) and
replace every inline occurrence such as the checks against eventType and
data.get("delta") with those constants; update the class-level declarations so
other methods in ChatServiceImpl reference the constants and run a project-wide
search to replace any identical literals in related classes to keep naming
consistent.
- Around line 224-233: The current doOnNext block throws a BusinessException
when eventType is "run.failed" or "error", which is non-idiomatic for Reactor;
replace this pattern by switching from doOnNext to using .handle((event, sink)
-> { ... }) where you inspect event.getEvent() (eventType) and if it's an error
case call sink.error(new BusinessException(ErrorCode.CONV5002, "스트리밍 중 에러 발생: "
+ errMsg)) and otherwise emit the event via sink.next(event); remove the throw
from ChatServiceImpl so error signaling is expressed via the reactive sink and
normal events continue to be emitted to clients.
- Around line 453-459: The current catch builds an error ProovyAiStreamEvent
which later in streamConversation's doOnNext -> doOnError path triggers
BusinessException and causes AI placeholder deletion; instead change the parsing
error handling to skip non-fatal parse failures by returning an empty signal
rather than an error event: modify the parsing block that currently returns
ProovyAiStreamEvent.builder().event("error")... to return Mono.empty() (or
refactor the surrounding Mono.fromCallable into Mono.defer so you can return
Mono.empty() on parse failures), and ensure streamConversation/doOnNext still
processes only real events while transient parse issues are ignored.
Comment on lines
358
to
+363
| /** | ||
| * Proovy-ai /stream 호출 | ||
| */ | ||
| /** | ||
| * Proovy-ai /stream/v2 호출 (SSE v2) | ||
| */ |
There was a problem hiding this comment.
이전 Javadoc 제거 누락
callProovyAiStream 메서드 앞에 이전 Javadoc (/** Proovy-ai /stream 호출 */, Lines 358-360)과 새 Javadoc (Lines 361-363)이 중복으로 존재합니다. 이전 Javadoc을 삭제해야 합니다.
🐛 수정 제안
- /**
- * Proovy-ai /stream 호출
- */
/**
* Proovy-ai /stream/v2 호출 (SSE v2)
*/
private Flux<ProovyAiStreamEvent> callProovyAiStream(ProovyAiRequest request) {📝 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
| /** | |
| * Proovy-ai /stream 호출 | |
| */ | |
| /** | |
| * Proovy-ai /stream/v2 호출 (SSE v2) | |
| */ | |
| /** | |
| * Proovy-ai /stream/v2 호출 (SSE v2) | |
| */ | |
| private Flux<ProovyAiStreamEvent> callProovyAiStream(ProovyAiRequest request) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/proovy/domain/conversation/service/ChatServiceImpl.java`
around lines 358 - 363, Remove the redundant/old Javadoc block that precedes the
callProovyAiStream method: delete the earlier comment /** Proovy-ai /stream 호출
*/ so only the new Javadoc /** Proovy-ai /stream/v2 호출 (SSE v2) */ remains
immediately above the callProovyAiStream(...) declaration; ensure there are no
duplicate JavaDoc blocks left for that method.
Comment on lines
422
to
+427
| /** | ||
| * SSE 이벤트 파싱 | ||
| */ | ||
| private Mono<ProovyAiStreamEvent> parseSseEvent(String rawEvent) { | ||
| /** | ||
| * SSE 이벤트 파싱 (v2: ServerSentEvent -> ProovyAiStreamEvent) | ||
| */ |
There was a problem hiding this comment.
parseSseEvent의 이전 Javadoc 제거 누락
Lines 422-424의 이전 Javadoc(/** SSE 이벤트 파싱 */)이 새 Javadoc(Lines 425-427)과 함께 중복으로 남아 있습니다. callProovyAiStream과 동일한 문제이며 이전 Javadoc을 삭제해야 합니다.
🐛 수정 제안
- /**
- * SSE 이벤트 파싱
- */
/**
* SSE 이벤트 파싱 (v2: ServerSentEvent -> ProovyAiStreamEvent)
*/
private Mono<ProovyAiStreamEvent> parseSseEvent(ServerSentEvent<String> sse) {📝 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
| /** | |
| * SSE 이벤트 파싱 | |
| */ | |
| private Mono<ProovyAiStreamEvent> parseSseEvent(String rawEvent) { | |
| /** | |
| * SSE 이벤트 파싱 (v2: ServerSentEvent -> ProovyAiStreamEvent) | |
| */ | |
| /** | |
| * SSE 이벤트 파싱 (v2: ServerSentEvent -> ProovyAiStreamEvent) | |
| */ | |
| private Mono<ProovyAiStreamEvent> parseSseEvent(ServerSentEvent<String> sse) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/proovy/domain/conversation/service/ChatServiceImpl.java`
around lines 422 - 427, Remove the stray/duplicate Javadoc comment above the
parseSseEvent method in ChatServiceImpl: delete the old short Javadoc block "/**
SSE 이벤트 파싱 */" so only the new Javadoc "SSE 이벤트 파싱 (v2: ServerSentEvent ->
ProovyAiStreamEvent)" remains; apply the same cleanup pattern as was done for
callProovyAiStream to avoid duplicated Javadoc comments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 관련 이슈
🏷️ PR 타입
📝 작업 내용
SOLUTION을 포함한 다양한 기능 코드가SUPPORTED_FEATURES에 추가되어 400 에러 수정ProovyAiStreamEvent이벤트 처리에서 발생한 버그 수정:SSE이벤트 파싱을 위한ServerSentEvent를 적절하게 처리하도록 수정type필드가 없는 경우 기본값 "message"로 설정[DONE]이벤트를 제대로 처리하도록 수정ProovyAiStreamEvent의data및event처리 개선📸 스크린샷
✅ 체크리스트
📎 기타 참고사항
Summary by CodeRabbit
릴리스 노트