feat(evaluation) 3/15: indexing throughput aggregation - #814
Conversation
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an indexing metrics module that aggregates file samples into throughput, duration, percentile, and extension statistics, exports it through the evaluation package, and tests successful and failed indexing scenarios. ChangesIndexing metrics
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
40c5277 to
18bd516
Compare
44c9830 to
7e09c04
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@openrag/core/evaluation/metrics.py`:
- Around line 44-46: Update the metric aggregation around succeeded, durations,
and total_bytes so bytes_total includes size_bytes from all samples, while
retaining a separate successful-byte total for MB/s calculations and excluding
failed samples from rates and percentiles. Extend the failure-accounting test
with a failed sample having nonzero size and assert that its bytes contribute to
bytes_total.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 71ffb8fb-b6cb-468e-9026-eab16c5f30e5
📒 Files selected for processing (3)
openrag/core/evaluation/__init__.pyopenrag/core/evaluation/metrics.pytests/unit/core/evaluation/test_metrics.py
7e09c04 to
8430b48
Compare
18bd516 to
cfa53f9
Compare
8430b48 to
d08f158
Compare
cfa53f9 to
fe67a25
Compare
d08f158 to
f6f4569
Compare
`indexing_metrics` folds the per-file timings the worker collects into throughput figures: files/min, MB/s, p50/p95 and a per-extension breakdown. Two definitions worth stating, both covered by tests: - Throughput divides by the measured wall clock of the indexing phase, not by the sum of the per-file durations. Summing would overstate speed the moment files are indexed concurrently. - Percentiles are nearest-rank (`ceil`). `statistics.quantiles` needs two points and interpolates, so it cannot describe a one-file run; and `round` breaks ties to even, which selects the wrong observation whenever `fraction * n` lands on an odd integer. Files that failed to index are counted, then excluded from every rate and percentile — a failure is not a fast file.
fe67a25 to
8e008f2
Compare
f6f4569 to
e573a8a
Compare
hedhoud
left a comment
There was a problem hiding this comment.
Looks good to me. The metrics are calculated consistently, failed files are handled correctly, and the focused tests pass locally.
Part 3 of 15 of the split of #811. Targets
eval/02-testset-parsing(#813).What
indexing_metrics— folds the per-file timings the worker collects (part 9) into throughput figures: files/min, MB/s, p50 / p95, and a per-extension breakdown. Pure; the worker measures, this decides what the measurements mean.Notable
Two definitions worth calling out, both pinned by tests:
ceil).statistics.quantilesneeds two points and interpolates, so it cannot describe a one-file run at all; androundbreaks ties to even, which picks the wrong observation wheneverfraction * nlands on an odd integer —p50of two files returned the slower one.test_p50_of_an_even_sample_takes_the_lower_middleis the regression test.Files that failed to index are counted in
files_total/files_failedand then excluded from every rate and percentile — a failure is not a fast file.Review follow-up
bytes_totalnow counts every file the run attempted (CodeRabbit). It previously summed only the successful samples, which made it disagree withfiles_totalsitting right beside it — and the run detail page renders this field as "Corpus size", so a run where a large PDF failed to parse would have reported a corpus smaller than the one the admin uploaded, with nothing explaining the gap.Split into the two questions it was conflating:
total_bytes(all samples →bytes_total) andindexed_bytes(successes only →megabytes_per_second). Rates and percentiles were already computed off the successes and are unchanged. The failure-accounting test now uses distinct non-zero sizes and asserts both directions, so it fails against the old code.Testing
7 unit tests: wall-clock throughput, failure accounting, single-file and even-sample percentiles, zero-division, extension bucketing.
ruff, format check and the layer-import guard pass.The rest of
metrics.py— folding promptfoo's output into retrieval and answer quality — lands in part 4.Summary by CodeRabbit
New Features
Bug Fixes
Tests