[Fix] cohere 파싱 방식 변경 (#38)#100
Merged
Merged
Conversation
📝 WalkthroughWalkthroughTwo independent fixes: a single-line SQL correction in the Python corpus import script changes the selected column from ChangesClassification ID SQL Column Fix
Cohere Embedding Response Parsing Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 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.
🧹 Nitpick comments (3)
src/main/java/com/jobdri/jobdri_api/domain/corpus/service/CohereCorpusEmbeddingClient.java (2)
68-74: ⚡ Quick winRemove unused
toFloatArray()method.This method is now dead code. The new
parseEmbeddings()extracts floats directly fromJsonNode(lines 94-97) without using this helper.🧹 Proposed fix to remove dead code
- private float[] toFloatArray(List<Double> values) { - float[] array = new float[values.size()]; - for (int i = 0; i < values.size(); i++) { - array[i] = values.get(i).floatValue(); - } - return array; - } -🤖 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/domain/corpus/service/CohereCorpusEmbeddingClient.java` around lines 68 - 74, The toFloatArray() method in CohereCorpusEmbeddingClient is no longer used since the parseEmbeddings() method now extracts floats directly from JsonNode without relying on this helper. Remove the entire toFloatArray() method definition (the private method that takes a List<Double> and returns a float array) as it is dead code that is no longer called anywhere in the class.
101-103: 💤 Low valueConsider catching
IOExceptioninstead ofException.Catching generic
Exceptioncan mask programming errors (e.g.,NullPointerException,ArrayIndexOutOfBoundsException) that indicate bugs rather than malformed API responses.objectMapper.readTree()throwsJsonProcessingException(extendsIOException).♻️ Proposed fix
- } catch (Exception e) { + } catch (IOException e) { throw new IllegalStateException("Cohere 임베딩 응답 파싱에 실패했습니다.", e); }This requires adding the import:
import java.io.IOException;🤖 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/domain/corpus/service/CohereCorpusEmbeddingClient.java` around lines 101 - 103, In the CohereCorpusEmbeddingClient class, replace the generic `Exception` catch type with `IOException` in the catch block that handles the objectMapper.readTree() call. This is more specific since JsonProcessingException (thrown by objectMapper.readTree()) extends IOException, and catching the more specific exception type prevents masking programming errors like NullPointerException. Add the necessary import statement for java.io.IOException at the top of the file.scripts/import_corpus.py (1)
213-213: ⚡ Quick winConsider removing the unnecessary join for clarity and performance.
The join to
detail_classifications dcmappears redundant since:
- You only select from
ccm.detail_classification_id(line 211)- The WHERE clause filters only on
ccmcolumns (lines 214-216)- The FK constraint on
corpus_classification_mappings.detail_classification_idalready ensures referential integrityRemoving the join would simplify the query and avoid an unnecessary table scan.
♻️ Proposed simplification
row = fetch_one( cur, """ select ccm.detail_classification_id as id from corpus_classification_mappings ccm - join detail_classifications dcm on dcm.id = ccm.detail_classification_id where ccm.source_job_group_l1 = %s and ccm.source_job_family_l2 = %s and ccm.source_role_l3 = %s🤖 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 `@scripts/import_corpus.py` at line 213, Remove the redundant join to the detail_classifications table (the join clause that references dcm.id = ccm.detail_classification_id) from the SQL query in the import_corpus.py script. Since no columns from the detail_classifications table are being selected in the query and the WHERE clause only filters on ccm columns, this join is unnecessary and can be safely deleted to simplify the query and improve performance.
🤖 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.
Nitpick comments:
In `@scripts/import_corpus.py`:
- Line 213: Remove the redundant join to the detail_classifications table (the
join clause that references dcm.id = ccm.detail_classification_id) from the SQL
query in the import_corpus.py script. Since no columns from the
detail_classifications table are being selected in the query and the WHERE
clause only filters on ccm columns, this join is unnecessary and can be safely
deleted to simplify the query and improve performance.
In
`@src/main/java/com/jobdri/jobdri_api/domain/corpus/service/CohereCorpusEmbeddingClient.java`:
- Around line 68-74: The toFloatArray() method in CohereCorpusEmbeddingClient is
no longer used since the parseEmbeddings() method now extracts floats directly
from JsonNode without relying on this helper. Remove the entire toFloatArray()
method definition (the private method that takes a List<Double> and returns a
float array) as it is dead code that is no longer called anywhere in the class.
- Around line 101-103: In the CohereCorpusEmbeddingClient class, replace the
generic `Exception` catch type with `IOException` in the catch block that
handles the objectMapper.readTree() call. This is more specific since
JsonProcessingException (thrown by objectMapper.readTree()) extends IOException,
and catching the more specific exception type prevents masking programming
errors like NullPointerException. Add the necessary import statement for
java.io.IOException at the top of the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 38031f2a-b32e-4221-8494-0d95b6b9bcfd
📒 Files selected for processing (2)
scripts/import_corpus.pysrc/main/java/com/jobdri/jobdri_api/domain/corpus/service/CohereCorpusEmbeddingClient.java
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이 필요한지 작업 내용을 자세하게 설명해주세요
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [#38]
Summary by CodeRabbit