[Perf] Chunk Size·Overlap 검색 품질 및 비용 비교 Benchmark 추가 - #149
Merged
Conversation
📝 WalkthroughWalkthrough실제 ChangesChunk 품질·비용 Benchmark
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ChunkQualityPerformanceBenchmark
participant FixedSizeChunker
participant BGE-M3EmbeddingServer
participant ExactCosineRanking
participant JsonReport
ChunkQualityPerformanceBenchmark->>FixedSizeChunker: Profile별 문서 청킹
ChunkQualityPerformanceBenchmark->>BGE-M3EmbeddingServer: Chunk와 Query Batch 임베딩 요청
BGE-M3EmbeddingServer-->>ChunkQualityPerformanceBenchmark: Vector 응답
ChunkQualityPerformanceBenchmark->>ExactCosineRanking: Vector 기반 Exact Cosine 검색
ExactCosineRanking-->>ChunkQualityPerformanceBenchmark: 품질 지표와 검색 시간 반환
ChunkQualityPerformanceBenchmark->>JsonReport: Profile별 결과 저장
Possibly related PRs
🚥 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: 1
🧹 Nitpick comments (8)
docs/test-results/gimin-#147-chunk-size-overlap-quality-benchmark.md (1)
120-126: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueJWT 값을 자리표시자로 바꾸는 편이 안전합니다.
Line 124가
JWT_SECRET값을 평문으로 기록합니다. 이 값은 로컬 회귀 테스트용 더미로 보이며 운영 비밀이 아닙니다. 코딩 가이드라인의 비밀 하드코딩 금지 규칙도**/*.{java,yml,yaml}를 대상으로 하므로 이 Markdown 문서에는 직접 적용되지 않습니다.다만 문서에 남은 리터럴은 다른 환경으로 복사되기 쉽습니다. 자리표시자로 바꾸면 그 위험이 사라집니다.
-JWT_SECRET=docgrid-test-secret-key-for-local-regression-2026 \ +JWT_SECRET=<로컬 테스트용 임의 문자열> \Swagger UI 수동 테스트 결과를 기록하지 않은 판단은 적절합니다. 이 PR은 제품 API를 변경하지 않고 Benchmark만 추가합니다.
🤖 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 `@docs/test-results/gimin-`#147-chunk-size-overlap-quality-benchmark.md around lines 120 - 126, Replace the literal JWT_SECRET value in the documented ./gradlew test command with a clearly named placeholder, while preserving DB_SSLMODE and the command’s purpose. Do not alter the Swagger UI testing note or other benchmark results.Source: Learnings
src/test/java/com/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupport.java (3)
249-263: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win입력 검증을 정렬보다 먼저 실행하세요.
Line 253에서
sorted()를 먼저 실행하고, Line 254에서 null·비유한·음수를 검증합니다. 순서가 뒤바뀌어 있습니다.
samplesMillis에 null 원소가 있으면sorted()가Double.compareTo를 호출하면서NullPointerException을 먼저 던집니다. 그 결과 Line 254의value == null분기는 절대 실행되지 않습니다. 의도한IllegalArgumentException대신NullPointerException이 호출자에게 전달됩니다.현재 호출부는 primitive
double을 boxing한 List만 전달하므로 실제 실행 경로에서는 발생하지 않습니다. 다만 계약을 명확히 하려면 검증을 앞으로 옮기세요.♻️ 검증 순서 교정
- List<Double> sorted = samplesMillis.stream().sorted().toList(); - if (sorted.stream().anyMatch(value -> value == null || !Double.isFinite(value) || value < 0.0)) { + if (samplesMillis.stream().anyMatch(value -> value == null || !Double.isFinite(value) || value < 0.0)) { throw new IllegalArgumentException("Timing 표본은 0 이상의 유한값이어야 합니다."); } + List<Double> sorted = samplesMillis.stream().sorted().toList();🤖 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/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupport.java` around lines 249 - 263, Update summarizeTimings to validate every samplesMillis element for null, finiteness, and non-negativity before calling sorted(). Preserve the existing IllegalArgumentException messages and percentile/max calculation after validation.
232-244: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
cosineSimilarity가 Vector 검증과 Norm 계산을 반복합니다.
rank()는 후보마다cosineSimilarity(queryVector, ...)를 호출합니다. 그때마다 Query Vector의 1024개 원소를 다시 검증하고 Norm을 다시 계산합니다. 후보 수만큼 같은 계산이 반복됩니다.현재 규모(최대 84 후보)에서는 실측
searchMillis가 16ms 이하이므로 실제 병목은 아닙니다. 다만searchMillis지표에 중복 검증 비용이 포함됩니다. Query Norm을rank()진입 시 한 번만 계산하면 지표가 순수 Ranking 비용에 더 가까워집니다.🤖 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/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupport.java` around lines 232 - 244, Update the ranking flow around rank() and cosineSimilarity() so the query vector is validated and its norm is computed once at rank() entry, then reused for every candidate comparison. Adjust cosineSimilarity() to accept or use the precomputed query norm while retaining validation and norm calculation for candidate vectors.
426-436: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win중복 ID를 조용히 덮어쓰지 않도록 검증을 추가하세요.
ids에 중복 값이 있으면LinkedHashMap.put이 앞선 Vector를 덮어씁니다. 예외는 발생하지 않습니다. 결과 Map의 크기가ids.size()보다 작아집니다. 그 뒤requiredVector도 다른 ID의 Vector를 반환하므로 실패를 감지할 수 없습니다.현재 호출부는
documentId + ":" + chunkIndex형태의 고유 ID를 전달하므로 중복은 발생하지 않습니다. 다만 설계 문서 8절은 불변식 위반 시 부분 결과를 정상 수치로 기록하지 않는다고 규정합니다. 크기 검증을 추가하면 이 원칙이 코드로 보장됩니다.♻️ 중복 ID 검증 추가
Map<String, float[]> result = new LinkedHashMap<>(); for (int index = 0; index < ids.size(); index++) { validateVector(vectors.get(index)); - result.put(ids.get(index), vectors.get(index).clone()); + if (result.put(ids.get(index), vectors.get(index).clone()) != null) { + throw new IllegalArgumentException("중복된 Vector ID입니다: " + ids.get(index)); + } }🤖 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/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupport.java` around lines 426 - 436, Update vectorMap to verify that the constructed result contains exactly ids.size() entries after inserting all IDs, and throw an IllegalArgumentException when duplicates caused overwrites. Preserve the existing size validation, vector validation, cloning, and Map.copyOf behavior.src/test/java/com/opensource/docgrid/domain/document/benchmark/ChunkQualityPerformanceBenchmark.java (3)
297-308: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
validate()를 compact constructor로 옮기면 검증이 항상 보장됩니다.현재
validate()는fromSystemProperties()에서만 호출됩니다.BenchmarkConfiguration을 다른 경로에서 직접 생성하면 검증이 생략됩니다. compact constructor로 옮기면 모든 생성 경로가 검증을 거칩니다.URL 검증(scheme 제한, userInfo 금지)은 SSRF와 자격증명 노출을 막는 방어입니다. 이 방어가 생성 경로에 묶이는 편이 안전합니다.
🤖 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/opensource/docgrid/domain/document/benchmark/ChunkQualityPerformanceBenchmark.java` around lines 297 - 308, Move the validation currently invoked by fromSystemProperties() into BenchmarkConfiguration’s compact constructor so every construction path enforces the same checks. Preserve the existing URL scheme and userInfo restrictions and remove the redundant explicit validate() call from the factory path.
153-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
failureCount는 구조상 항상 0입니다. 지표 의미를 명확히 하세요.Line 160은
failureCount에 상수0을 전달합니다. 실제 실패 경로를 확인하면 다음과 같습니다.
embed()는 Model명이 다르면IllegalStateException을 던집니다.validateVector는 불변식 위반 시IllegalArgumentException을 던집니다.RestClient는 HTTP 오류 시 예외를 던집니다.세 경우 모두 Benchmark가 중단되고 JSON은 생성되지 않습니다. 따라서 기록된
failureCount는 언제나 0입니다.이 fail-fast 동작 자체는 설계 문서 8절과 일치합니다. 다만 설계 문서 10절 1항은 "Failure Count가 0인 Profile만 비교한다"를 판정 기준으로 제시합니다. 항상 0인 값은 판정 기준으로서 정보를 주지 않습니다.
두 가지 해석이 가능합니다. 선택이 필요합니다.
- fail-fast를 유지한다면
failureCount필드와 문서 10절 1항을 제거하고, 대신 "JSON이 생성되면 모든 검증을 통과한 것"임을 문서에 명시합니다.- 판정 기준을 유지한다면 재시도 가능한 실패를 집계하고 계속 진행하는 경로를 구현합니다.
🤖 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/opensource/docgrid/domain/document/benchmark/ChunkQualityPerformanceBenchmark.java` around lines 153 - 161, Resolve the mismatch between the always-zero failureCount in the benchmark result construction and the documented comparison criteria: either remove failureCount and the corresponding “failure count is zero” criterion while documenting that generated JSON indicates all validations passed, or implement retryable-failure aggregation with continued execution so failureCount becomes meaningful. Apply the chosen behavior consistently in the RoundMeasurement creation near chunkEmbeddings.requestCount() and the related design documentation.
213-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win참조 동등성 비교 의도를 주석으로 남기세요.
other != candidate는 참조 동등성을 사용합니다.ProfileResult는 record이므로equals는 값 기반입니다. 두 Profile의 모든 지표가 같으면equals가true를 반환합니다. 그 경우!other.equals(candidate)를 쓰면 자기 자신이 아닌 Profile까지 비교에서 빠집니다.즉 현재 참조 비교가 올바른 선택입니다. 다만 의도가 코드에 드러나지 않습니다. 이후 리팩터링에서
equals로 바뀌면 Pareto 판정이 조용히 틀어집니다.♻️ 의도 주석 추가
private boolean isParetoCandidate(ProfileResult candidate, List<ProfileResult> results) { + // record의 값 기반 equals는 지표가 같은 다른 Profile까지 제외하므로 참조 동등성으로 자기 자신만 뺀다. return results.stream().noneMatch(other -> other != candidate && dominates(other, candidate)); }🤖 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/opensource/docgrid/domain/document/benchmark/ChunkQualityPerformanceBenchmark.java` around lines 213 - 215, Add a concise comment in isParetoCandidate explaining that other != candidate intentionally uses reference identity because ProfileResult record equality is value-based, and equal-valued distinct profiles must still be compared. Leave the existing comparison logic unchanged.src/test/java/com/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupportTest.java (1)
143-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRelevant 판정 로직이 테스트에 중복 구현되어 있습니다.
Line 144-148은
ChunkQualityBenchmarkSupport.isRelevant와 같은 세 가지 조건을 다시 작성합니다. 두 곳의 로직이 갈라지면 이 테스트는 실제 Ground Truth 계약을 더 이상 검증하지 못합니다. 그래도 테스트는 통과합니다.
isRelevant는 현재private입니다. 테스트가 같은 패키지에 있으므로 package-private으로 바꾸면 중복 없이 실제 판정 경로를 검증할 수 있습니다.♻️ 실제 판정 메서드 재사용
ChunkQualityBenchmarkSupport.java에서 접근 제한자를 변경합니다.- private static boolean isRelevant(QueryCase queryCase, ChunkCandidate candidate) { + static boolean isRelevant(QueryCase queryCase, ChunkCandidate candidate) {테스트에서 재사용합니다.
- boolean covered = chunked.candidates().stream().anyMatch(candidate -> - candidate.documentId().equals(queryCase.documentId()) - && candidate.charStart() <= queryCase.evidenceStart() - && candidate.charEnd() >= queryCase.evidenceEnd() - ); + boolean covered = chunked.candidates().stream() + .anyMatch(candidate -> ChunkQualityBenchmarkSupport.isRelevant(queryCase, candidate));🤖 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/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupportTest.java` around lines 143 - 150, Change ChunkQualityBenchmarkSupport.isRelevant from private to package-private, then update the covered assertion in ChunkQualityBenchmarkSupportTest to reuse that method for each candidate/queryCase pair instead of duplicating the document ID and character-boundary conditions. Preserve the existing expected-result assertions while validating the actual ground-truth implementation.Source: Path instructions
🤖 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 `@docs/test-results/gimin-`#147-chunk-size-overlap-quality-benchmark.md:
- Line 6: Update the JSON link in the benchmark document to URL-encode the #
character as %23, preserving the existing relative link target so it resolves to
the committed file rather than being interpreted as a fragment.
---
Nitpick comments:
In `@docs/test-results/gimin-`#147-chunk-size-overlap-quality-benchmark.md:
- Around line 120-126: Replace the literal JWT_SECRET value in the documented
./gradlew test command with a clearly named placeholder, while preserving
DB_SSLMODE and the command’s purpose. Do not alter the Swagger UI testing note
or other benchmark results.
In
`@src/test/java/com/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupport.java`:
- Around line 249-263: Update summarizeTimings to validate every samplesMillis
element for null, finiteness, and non-negativity before calling sorted().
Preserve the existing IllegalArgumentException messages and percentile/max
calculation after validation.
- Around line 232-244: Update the ranking flow around rank() and
cosineSimilarity() so the query vector is validated and its norm is computed
once at rank() entry, then reused for every candidate comparison. Adjust
cosineSimilarity() to accept or use the precomputed query norm while retaining
validation and norm calculation for candidate vectors.
- Around line 426-436: Update vectorMap to verify that the constructed result
contains exactly ids.size() entries after inserting all IDs, and throw an
IllegalArgumentException when duplicates caused overwrites. Preserve the
existing size validation, vector validation, cloning, and Map.copyOf behavior.
In
`@src/test/java/com/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupportTest.java`:
- Around line 143-150: Change ChunkQualityBenchmarkSupport.isRelevant from
private to package-private, then update the covered assertion in
ChunkQualityBenchmarkSupportTest to reuse that method for each
candidate/queryCase pair instead of duplicating the document ID and
character-boundary conditions. Preserve the existing expected-result assertions
while validating the actual ground-truth implementation.
In
`@src/test/java/com/opensource/docgrid/domain/document/benchmark/ChunkQualityPerformanceBenchmark.java`:
- Around line 297-308: Move the validation currently invoked by
fromSystemProperties() into BenchmarkConfiguration’s compact constructor so
every construction path enforces the same checks. Preserve the existing URL
scheme and userInfo restrictions and remove the redundant explicit validate()
call from the factory path.
- Around line 153-161: Resolve the mismatch between the always-zero failureCount
in the benchmark result construction and the documented comparison criteria:
either remove failureCount and the corresponding “failure count is zero”
criterion while documenting that generated JSON indicates all validations
passed, or implement retryable-failure aggregation with continued execution so
failureCount becomes meaningful. Apply the chosen behavior consistently in the
RoundMeasurement creation near chunkEmbeddings.requestCount() and the related
design documentation.
- Around line 213-215: Add a concise comment in isParetoCandidate explaining
that other != candidate intentionally uses reference identity because
ProfileResult record equality is value-based, and equal-valued distinct profiles
must still be compared. Leave the existing comparison logic unchanged.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: de191f95-0755-4ee4-9183-71fe41c45935
⛔ Files ignored due to path filters (1)
docs/test-results/assets/chunk-quality/chunk-quality-tradeoff.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
build.gradledocs/design/gimin-#147-chunk-size-overlap-quality-benchmark.mddocs/test-results/gimin-#147-chunk-size-overlap-quality-benchmark-data.jsondocs/test-results/gimin-#147-chunk-size-overlap-quality-benchmark.mdsrc/test/java/com/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupport.javasrc/test/java/com/opensource/docgrid/domain/document/benchmark/ChunkQualityBenchmarkSupportTest.javasrc/test/java/com/opensource/docgrid/domain/document/benchmark/ChunkQualityPerformanceBenchmark.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.
변경 내용
FixedSizeChunker로 8개 Chunk Size·Overlap Profile을 같은 Corpus에서 비교합니다.BAAI/bge-m3Batch API와 메모리 내 Exact Cosine Ranking으로 Answer Coverage, Hit@1, Hit@3, MRR@10을 측정합니다.chunkQualityPerformanceTestGradle Task를 추가했습니다.실측 결과
400/0400/80800/0800/1601000/01000/200현재 기본1600/01600/32020% Overlap은 모든 Profile의 완전한 근거 Coverage를 100%로 복구했지만 중복량이 14.5~21.8% 증가했습니다. 합성 경계 Corpus에서
800/160의 순위 품질이 가장 높았습니다.이 결과만으로 제품 기본값을 바꾸지 않습니다. 실제 사용자 문서 평가셋을 포함한 별도 의사결정이 필요합니다.
측정 경계
검증
./gradlew test --tests '*ChunkQualityBenchmarkSupportTest'✅DB_SSLMODE=disable JWT_SECRET=<test-secret> ./gradlew test✅ 734개./gradlew chunkQualityPerformanceTest✅ 9분 52초문서
docs/design/gimin-#147-chunk-size-overlap-quality-benchmark.mddocs/test-results/gimin-#147-chunk-size-overlap-quality-benchmark.mddocs/test-results/gimin-#147-chunk-size-overlap-quality-benchmark-data.jsondocs/test-results/assets/chunk-quality/chunk-quality-tradeoff.svgCloses #147