Repository navigation
Fix/#47 tts error - #48
Conversation
kimwuju
left a comment
kimwuju
left a comment
There was a problem hiding this comment.
리뷰 남깁니다. 핵심 수정 방향은 맞고 CI도 통과했습니다. ElevenLabs 전용 WebClient에만 수신 한도를 두고, 한도 초과를 재시도 불가 TtsClientException으로 바꾸고, chunked/non-chunked 회귀 테스트까지 넣은 점이 좋습니다.
머지를 막을 이슈는 없습니다. 인라인 코멘트는 모두 우선순위가 낮은 제안이에요.
- TTS가 꺼져 있어도 한도 검증 때문에 서버 기동이 실패할 수 있음
ConnectionProviderdispose 누락 (기존 코드인데 새 테스트에서도 반복됨)DataSize타입, 에러 코드 구분, BootWebClient.Builder재사용 제안- 문서 범위, 기본값 중복, 생성자 정리 (nit)
| @Bean("elevenLabsWebClient") | ||
| public WebClient elevenLabsWebClient(TtsProperties properties) { | ||
| TtsProperties.ElevenLabs config = properties.getElevenlabs(); | ||
| Assert.isTrue(config.getMaxInMemorySize() > 0, |
There was a problem hiding this comment.
[낮음] TTS가 꺼져 있어도 이 검증 때문에 서버가 안 뜰 수 있어요
elevenLabsWebClient 빈은 tts.enabled와 관계없이 항상 생성됩니다. 그래서 TTS_ENABLED=false인 환경에서도 ELEVENLABS_MAX_IN_MEMORY_SIZE가 0 이하로 들어가면 Chat 서버 전체가 기동에 실패합니다.
잘못된 설정을 빨리 드러내려는 의도라면 그대로 두셔도 됩니다. TTS 설정 하나 때문에 면접 기능 전체가 막히는 게 부담이라면 properties.isEnabled()일 때만 검사하는 방법도 있어요.
| TtsProperties.ElevenLabs config = properties.getElevenlabs(); | ||
| Assert.isTrue(config.getMaxInMemorySize() > 0, | ||
| "tts.elevenlabs.max-in-memory-size는 양수여야 합니다"); | ||
| ConnectionProvider provider = ConnectionProvider.builder("elevenlabs") |
There was a problem hiding this comment.
[낮음] ConnectionProvider가 dispose되지 않아요 (기존 코드 + 새 테스트)
ConnectionProvider를 지역 변수로 만들기 때문에 애플리케이션이 종료될 때 정리되지 않습니다. AnthropicWebClientConfig의 anthropicConnectionProvider처럼 @Bean(destroyMethod = "dispose")로 분리하면 종료 시 커넥션 풀도 함께 정리됩니다.
새 ElevenLabsTtsClientTest도 client()를 부를 때마다 elevenlabs 풀을 새로 만들고 해제하지 않아서, 테스트 수만큼 풀이 JVM에 남습니다. 기존 코드라 이 PR 범위 밖이라면 이슈로 따로 빼도 괜찮습니다.
| private String outputFormat = "mp3_44100_128"; | ||
| private Duration connectTimeout = Duration.ofSeconds(3); | ||
| private Duration responseTimeout = Duration.ofSeconds(20); | ||
| private int maxInMemorySize = 8 * 1024 * 1024; |
There was a problem hiding this comment.
[제안] int 대신 DataSize는 어떨까요?
같은 클래스의 시간 설정은 Duration이라 20s처럼 쓸 수 있는데, 크기 설정만 바이트 정수라서 8MB라고 쓰면 변환 오류로 기동이 실패합니다. Spring Boot의 spring.codec.max-in-memory-size도 DataSize를 씁니다.
private DataSize maxInMemorySize = DataSize.ofMegabytes(8);WebClient 쪽에는 Math.toIntExact(config.getMaxInMemorySize().toBytes())로 넘기면 됩니다. 단위가 없는 8388608 같은 기존 값도 바이트로 그대로 바인딩됩니다.
| )); | ||
| } | ||
| return response.bodyToMono(byte[].class) | ||
| .onErrorMap(DataBufferLimitException.class, error -> new TtsClientException( |
There was a problem hiding this comment.
[제안] 한도 초과를 다른 실패와 구분할 수 있으면 좋겠어요
재시도하지 않게 바꾼 건 좋습니다. 이전에는 TtsClientException이 아니어서 스케줄러가 같은 합성을 2번 더 요청했죠.
다만 InterviewTtsScheduler.completeFailed에서는 이 경우도 TTS_GENERATION_FAILED로 저장됩니다. 그래서 상태나 메트릭만으로는 ElevenLabs 4xx, 빈 오디오와 구분되지 않고, 한도를 낮게 잡아 FAILED가 반복되면 WARN 로그 메시지를 직접 봐야 원인을 알 수 있어요. 예외에 에러 코드(예: TTS_AUDIO_TOO_LARGE)를 실어 스케줄러가 그대로 쓰게 하면 운영에서 바로 보입니다.
| @@ -28,6 +31,7 @@ public WebClient elevenLabsWebClient(TtsProperties properties) { | |||
| return WebClient.builder() | |||
There was a problem hiding this comment.
[제안] Boot의 WebClient.Builder를 쓰면 설정이 한곳으로 모여요
여기는 정적 WebClient.builder()를 써서 Boot가 구성한 빌더(코덱 설정, HTTP 클라이언트 관측·메트릭 커스터마이저)를 거치지 않습니다. 256KiB 기본값이 그대로 적용됐던 이유이기도 하고요.
AnthropicWebClientConfig처럼 WebClient.Builder를 주입받아 clone()한 뒤 .codecs(...)로 이 클라이언트의 한도만 올리면, 다른 WebClient에는 영향 없이 메트릭도 함께 받을 수 있습니다. 이 PR에서 꼭 바꿀 필요는 없습니다.
| 배포 뒤 새로 준비한 면접의 긴 질문에서 오디오 READY와 재생을 확인한다. | ||
| 이미 FAILED로 저장된 오디오는 이 변경만으로 자동 재생성되지 않는다. | ||
|
|
||
| ## 녹음 업로드와 다음 질문의 구분 |
There was a problem hiding this comment.
[제안] 이 섹션은 TTS 수신 한도와 다른 내용이라 분리하면 좋겠어요
녹음 업로드, 답변 제출, Web STT 동작 설명은 #47을 조사하다 나온 맥락으로 보입니다. 그런데 tts-audio-buffer.md라는 파일명으로는 찾기 어렵습니다. 특히 "Web은 음성 인식 텍스트가 비어 있으면 … 제출하지 않는다"는 이 저장소 코드로 보장되지 않는 내용이라, Web 쪽이 바뀌면 문서가 조용히 틀리게 됩니다.
이슈 #47 코멘트나 PR 본문으로 옮기거나 별도 문서로 빼는 걸 제안합니다.
| ANTHROPIC_MODEL: ${ANTHROPIC_MODEL} | ||
| TTS_ENABLED: ${TTS_ENABLED:-false} | ||
| ELEVENLABS_API_KEY: ${ELEVENLABS_API_KEY:-} | ||
| ELEVENLABS_MAX_IN_MEMORY_SIZE: ${ELEVENLABS_MAX_IN_MEMORY_SIZE:-8388608} |
There was a problem hiding this comment.
[nit] 기본값 8388608이 세 군데에 있어요
TtsProperties(8 * 1024 * 1024), application.yml(${ELEVENLABS_MAX_IN_MEMORY_SIZE:8388608}), 여기 compose 기본값에 같은 값이 각각 하드코딩돼 있습니다. compose는 빈 문자열이 넘어가지 않도록 기본값이 필요하니 의도된 거라면 그대로 두셔도 됩니다. 기본값을 바꿀 때 세 곳을 함께 바꿔야 한다는 점만 문서에 적어 두면 좋겠어요.
| return retryable; | ||
| } | ||
|
|
||
| public TtsClientException(String message, boolean retryable, Throwable cause) { |
There was a problem hiding this comment.
[nit] 새 생성자가 isRetryable() 아래에 있어서, 생성자끼리 묶어 두면 읽기 편할 것 같아요. 기존 2인자 생성자를 this(message, retryable, null)로 위임하면 필드 초기화도 한곳에 모입니다.
danjee2009
left a comment
danjee2009
left a comment
There was a problem hiding this comment.
변경된 ElevenLabs 응답 크기 제한과 설정 검증, 초과 응답의 비재시도 처리 및 관련 테스트를 검토했습니다. 확인된 실질적 문제는 없습니다. GitHub의 test 검사는 통과했습니다.
47