[Feat] Cohere Embed API 클라이언트 1차 연동 - #227
Merged
Merged
Conversation
- Cohere 설정 프로퍼티와 RestClient 기반 embedding client 추가 - /v2/embed 요청/응답 DTO 및 search_document/search_query 호출 분리 - API key, 입력, 응답 개수 및 1024차원 검증 로직 추가 - Cohere API 오류를 기존 GeneralException 체계로 변환 - 외부 API 호출 없는 단위 테스트와 수동 검증 문서 추가
📝 WalkthroughWalkthroughCohere Embed API 연동을 위한 설정 바인딩, 요청·응답 DTO, 임베딩 클라이언트가 추가되었습니다. 클라이언트는 입력과 응답을 검증하고 HTTP 오류를 공통 예외로 변환하며, 스텁 서버 기반 테스트와 로컬 검증 문서가 포함됩니다. ChangesCohere 임베딩 연동
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant 호출자
participant CohereEmbeddingClient
participant Cohere Embed API
호출자->>CohereEmbeddingClient: embedDocuments 또는 embedQuery 호출
CohereEmbeddingClient->>CohereEmbeddingClient: API 키와 입력 검증
CohereEmbeddingClient->>Cohere Embed API: POST /v2/embed
Cohere Embed API-->>CohereEmbeddingClient: 임베딩 응답 또는 HTTP 오류
CohereEmbeddingClient-->>호출자: float[] 또는 List<float[]> 반환
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/jobdri/jobdri_api/global/cohere/CohereEmbeddingClient.java`:
- Around line 31-38: Update the requestFactory used by CohereEmbeddingClient to
use a connection-pooling HTTP client such as
HttpComponentsClientHttpRequestFactory instead of
SimpleClientHttpRequestFactory. Configure pooled connection limits and preserve
the existing embedding connect and read timeout values from CohereProperties.
- Around line 77-91: Update the Cohere request flow around the 429/5xx handler
to retry transient failures with bounded exponential backoff before converting
them to SERVICE_UNAVAILABLE. Honor the Retry-After response header when present,
while preserving the existing invalidParameter handling for 4xx client errors
and the final unavailable error after retries are exhausted.
In `@src/main/resources/application-prod.yaml`:
- Around line 174-175: Remove the unused nested api.key block under cohere from
application-prod.yaml (174-175), application-analysis-eval.yaml (96-97), and
application-dev.yaml (173-174); retain the supported cohere.api-key
configuration unchanged.
In
`@src/test/java/com/jobdri/jobdri_api/global/cohere/CohereEmbeddingClientTest.java`:
- Around line 107-178: CohereEmbeddingClient의 callCohere timeout 처리 경로를 검증하는
테스트를 추가하세요. 기존 테스트 헬퍼인 TestCohereServer를 활용해 readTimeout보다 긴 지연 후 응답하도록 구성하고,
client.embedQuery 또는 embedDocuments가 GeneralException을 발생시키며 코드가
EXTERNAL_SERVICE_TIMEOUT인지 확인하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b574183f-d610-4a22-9b2e-faf47bbe3c5b
⛔ Files ignored due to path filters (2)
evaluation/evaluation_nlg_judge_missing_keyword_provenance.csvis excluded by!**/*.csvevaluation/evaluation_nlg_judge_missing_keyword_provenance_comparison.csvis excluded by!**/*.csv
📒 Files selected for processing (10)
README.mdsrc/main/java/com/jobdri/jobdri_api/global/cohere/CohereConfig.javasrc/main/java/com/jobdri/jobdri_api/global/cohere/CohereEmbeddingClient.javasrc/main/java/com/jobdri/jobdri_api/global/cohere/CohereProperties.javasrc/main/java/com/jobdri/jobdri_api/global/cohere/dto/CohereEmbeddingRequest.javasrc/main/java/com/jobdri/jobdri_api/global/cohere/dto/CohereEmbeddingResponse.javasrc/main/resources/application-analysis-eval.yamlsrc/main/resources/application-dev.yamlsrc/main/resources/application-prod.yamlsrc/test/java/com/jobdri/jobdri_api/global/cohere/CohereEmbeddingClientTest.java
Comment on lines
+31
to
+38
| public CohereEmbeddingClient(CohereProperties properties, RestClient.Builder restClientBuilder) { | ||
| this.properties = properties; | ||
| this.restClient = restClientBuilder | ||
| .baseUrl(properties.baseUrl()) | ||
| .requestFactory(requestFactory(properties)) | ||
| .defaultHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
SimpleClientHttpRequestFactory는 커넥션 풀링을 제공하지 않습니다.
SimpleClientHttpRequestFactory는 JDK HttpURLConnection 기반으로 요청마다 새 연결을 열고 닫는 구조라 TCP/TLS 핸드셰이크 비용이 매번 발생하고, 커넥션 풀 크기 등을 세밀하게 제어할 수 없습니다. corpus 적재처럼 대량으로 embedDocuments를 호출하는 시나리오에서는 처리량 저하 요인이 될 수 있습니다. HttpComponentsClientHttpRequestFactory(Apache HttpClient) 등 풀링을 지원하는 팩토리로 교체하는 것을 권장합니다.
As per path instructions, "외부 API 호출 성능: connection pooling ... 대량 요청 시 처리량 저하 가능성"을 우선순위 높게 검토해야 합니다.
♻️ 커넥션 풀링을 지원하는 팩토리로 교체하는 예시
private static ClientHttpRequestFactory requestFactory(CohereProperties properties) {
PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager();
connectionManager.setMaxTotal(50);
connectionManager.setDefaultMaxPerRoute(20);
RequestConfig requestConfig = RequestConfig.custom()
.setConnectTimeout(Timeout.ofMilliseconds(properties.embedding().connectTimeout().toMillis()))
.setResponseTimeout(Timeout.ofMilliseconds(properties.embedding().readTimeout().toMillis()))
.build();
CloseableHttpClient httpClient = HttpClients.custom()
.setConnectionManager(connectionManager)
.setDefaultRequestConfig(requestConfig)
.build();
return new HttpComponentsClientHttpRequestFactory(httpClient);
}Also applies to: 164-169
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/jobdri/jobdri_api/global/cohere/CohereEmbeddingClient.java`
around lines 31 - 38, Update the requestFactory used by CohereEmbeddingClient to
use a connection-pooling HTTP client such as
HttpComponentsClientHttpRequestFactory instead of
SimpleClientHttpRequestFactory. Configure pooled connection limits and preserve
the existing embedding connect and read timeout values from CohereProperties.
Source: Path instructions
Comment on lines
+77
to
+91
| .onStatus( | ||
| status -> status.value() == 429 || status.is5xxServerError(), | ||
| (ignoredRequest, ignoredResponse) -> { | ||
| throw unavailable("Cohere Embed API가 일시적으로 응답할 수 없습니다."); | ||
| } | ||
| ) | ||
| .onStatus( | ||
| status -> status.value() == 400 | ||
| || status.value() == 401 | ||
| || status.value() == 403 | ||
| || status.is4xxClientError(), | ||
| (ignoredRequest, ignoredResponse) -> { | ||
| throw invalidParameter("Cohere Embed API 요청 또는 설정이 올바르지 않습니다."); | ||
| } | ||
| ) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift
429/5xx에 대한 재시도(backoff) 정책이 없습니다.
현재는 429/5xx 응답을 즉시 SERVICE_UNAVAILABLE로 변환해 실패시킵니다. Cohere는 일시적인 rate limit(429)에 대해 지수 백오프 재시도를 권장하는 사례가 흔한데, 이 구현은 상위 호출자에게 즉시 예외를 전파합니다. 1차 연동 범위상 당장 필수는 아니지만, 향후 실호출 트래픽이 늘어나면 429에서 재시도(가능하면 Retry-After 헤더 반영)를 추가하는 것을 권장합니다.
As per path instructions, "외부 API 호출 성능: ... 재시도 정책, rate limit 대응"을 우선순위 높게 검토해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/jobdri/jobdri_api/global/cohere/CohereEmbeddingClient.java`
around lines 77 - 91, Update the Cohere request flow around the 429/5xx handler
to retry transient failures with bounded exponential backoff before converting
them to SERVICE_UNAVAILABLE. Honor the Retry-After response header when present,
while preserving the existing invalidParameter handling for 4xx client errors
and the final unavailable error after retries are exhausted.
Source: Path instructions
Comment on lines
174
to
175
| api: | ||
| key: ${COHERE_API_KEY:} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
세 환경 yaml 모두에 미사용 cohere.api.key 중첩 설정이 중복 존재합니다. CohereProperties record는 apiKey(→cohere.api-key), baseUrl, embedding 필드만 가지고 있어 cohere.api.key 중첩 값은 어디에도 바인딩되지 않는 죽은 설정이며, cohere.api-key와 동일한 값(${COHERE_API_KEY:})을 중복 노출해 향후 유지보수 시 혼란을 유발할 수 있습니다.
src/main/resources/application-prod.yaml#L174-L175:api.key중첩 블록 삭제.src/main/resources/application-analysis-eval.yaml#L96-L97:api.key중첩 블록 삭제.src/main/resources/application-dev.yaml#L173-L174:api.key중첩 블록 삭제.
📍 Affects 3 files
src/main/resources/application-prod.yaml#L174-L175(this comment)src/main/resources/application-analysis-eval.yaml#L96-L97src/main/resources/application-dev.yaml#L173-L174
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/resources/application-prod.yaml` around lines 174 - 175, Remove the
unused nested api.key block under cohere from application-prod.yaml (174-175),
application-analysis-eval.yaml (96-97), and application-dev.yaml (173-174);
retain the supported cohere.api-key configuration unchanged.
Comment on lines
+107
to
+178
| @Test | ||
| @DisplayName("Cohere 429와 5xx는 서비스 일시 장애 예외로 변환한다") | ||
| void transientCohereErrors() throws Exception { | ||
| for (int status : List.of(429, 500)) { | ||
| try (TestCohereServer server = startServer(status, "{\"message\":\"temporary\"}", new AtomicReference<>())) { | ||
| CohereEmbeddingClient client = client(server.baseUrl(), "test-api-key", 3); | ||
|
|
||
| assertThatThrownBy(() -> client.embedDocuments(List.of("text"))) | ||
| .isInstanceOfSatisfying(GeneralException.class, exception -> | ||
| assertThat(exception.getCode()).isEqualTo(GeneralErrorCode.SERVICE_UNAVAILABLE)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Cohere 400, 401, 403은 요청 또는 설정 오류로 변환한다") | ||
| void requestOrConfigurationErrors() throws Exception { | ||
| for (int status : List.of(400, 401, 403)) { | ||
| try (TestCohereServer server = startServer(status, "{\"message\":\"bad request\"}", new AtomicReference<>())) { | ||
| CohereEmbeddingClient client = client(server.baseUrl(), "test-api-key", 3); | ||
|
|
||
| assertThatThrownBy(() -> client.embedDocuments(List.of("text"))) | ||
| .isInstanceOfSatisfying(GeneralException.class, exception -> | ||
| assertThat(exception.getCode()).isEqualTo(GeneralErrorCode.INVALID_PARAMETER)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("응답 embedding 개수가 요청 texts 개수와 다르면 예외 처리한다") | ||
| void mismatchedEmbeddingCount() throws Exception { | ||
| try (TestCohereServer server = startServer(200, responseJson(3, 1), new AtomicReference<>())) { | ||
| CohereEmbeddingClient client = client(server.baseUrl(), "test-api-key", 3); | ||
|
|
||
| assertThatThrownBy(() -> client.embedDocuments(List.of("first", "second"))) | ||
| .isInstanceOfSatisfying(GeneralException.class, exception -> | ||
| assertThat(exception.getCode()).isEqualTo(GeneralErrorCode.SERVICE_UNAVAILABLE)) | ||
| .hasMessageContaining("개수"); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("응답 embedding 차원이 설정 차원과 다르면 예외 처리한다") | ||
| void mismatchedEmbeddingDimension() throws Exception { | ||
| try (TestCohereServer server = startServer(200, responseJson(2, 1), new AtomicReference<>())) { | ||
| CohereEmbeddingClient client = client(server.baseUrl(), "test-api-key", 3); | ||
|
|
||
| assertThatThrownBy(() -> client.embedQuery("query")) | ||
| .isInstanceOfSatisfying(GeneralException.class, exception -> | ||
| assertThat(exception.getCode()).isEqualTo(GeneralErrorCode.SERVICE_UNAVAILABLE)) | ||
| .hasMessageContaining("차원"); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("응답 body가 없거나 embeddings가 비어 있으면 예외 처리한다") | ||
| void emptyResponseBodyOrEmbeddings() throws Exception { | ||
| try (TestCohereServer server = startServer(200, "", new AtomicReference<>())) { | ||
| CohereEmbeddingClient client = client(server.baseUrl(), "test-api-key", 3); | ||
|
|
||
| assertThatThrownBy(() -> client.embedQuery("query")) | ||
| .isInstanceOfSatisfying(GeneralException.class, exception -> | ||
| assertThat(exception.getCode()).isEqualTo(GeneralErrorCode.SERVICE_UNAVAILABLE)); | ||
| } | ||
| try (TestCohereServer server = startServer(200, "{\"embeddings\":{\"float\":[]}}", new AtomicReference<>())) { | ||
| CohereEmbeddingClient client = client(server.baseUrl(), "test-api-key", 3); | ||
|
|
||
| assertThatThrownBy(() -> client.embedQuery("query")) | ||
| .isInstanceOfSatisfying(GeneralException.class, exception -> | ||
| assertThat(exception.getCode()).isEqualTo(GeneralErrorCode.SERVICE_UNAVAILABLE)); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
timeout(ResourceAccessException → EXTERNAL_SERVICE_TIMEOUT) 시나리오 테스트가 없습니다.
429/5xx, 400/401/403, 응답 개수/차원 불일치, 빈 응답 등 에러 매핑은 잘 검증되어 있지만, CohereEmbeddingClient의 connectTimeout/readTimeout 초과 시 ResourceAccessException을 EXTERNAL_SERVICE_TIMEOUT으로 변환하는 로직(callCohere의 catch 블록)을 검증하는 테스트가 없습니다. 응답을 지연시키는 스텁 서버(예: readTimeout보다 긴 sleep 후 응답)로 이 경로를 커버하면 회귀 위험을 줄일 수 있습니다.
As per path instructions, "테스트가 동시성, 재시도, timeout, 실패 복구, 멱등성 같은 비동기 시나리오를 충분히 검증하는지 중점적으로 확인하세요. 해피패스만 있는 경우 경계 조건과 회귀 위험을 지적하세요."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/jobdri/jobdri_api/global/cohere/CohereEmbeddingClientTest.java`
around lines 107 - 178, CohereEmbeddingClient의 callCohere timeout 처리 경로를 검증하는
테스트를 추가하세요. 기존 테스트 헬퍼인 TestCohereServer를 활용해 readTimeout보다 긴 지연 후 응답하도록 구성하고,
client.embedQuery 또는 embedDocuments가 GeneralException을 발생시키며 코드가
EXTERNAL_SERVICE_TIMEOUT인지 확인하세요.
Source: Path instructions
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
1 participant
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를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [ ]
Summary by CodeRabbit
새 기능
문서
테스트