feat(querier): add severity_text predicate for the B1 level filter - #114
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds optional severity-level filtering to the querier by introducing a ChangesSeverity Text Filter Addition
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
Adds an optional severity_text equality predicate to the RFC 0007 “throwaway” query surface so B1’s ... AND level='ERROR' shape can be expressed and measured (including Parquet-statistics pruning).
Changes:
- Extend
QueryRequestwithseverity_text: Option<String>and push it down to DataFusion ascol(severity_text) = lit(X). - Add an execution test asserting
severity_text='ERROR'both counts correctly and triggers row-group pruning via Parquet min/max stats. - Update all existing
QueryRequestconstruction sites (tests + b2 bench) to setseverity_text: None.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-querier/src/lib.rs | Adds severity_text to QueryRequest and applies it as a DataFusion filter; updates crate docs. |
| crates/ourios-querier/tests/execution.rs | Adds rec_sev helper and new B1 test covering severity filtering + pruning. |
| crates/ourios-querier/tests/manifest.rs | Updates request helper to include severity_text: None. |
| crates/ourios-querier/tests/forward_compat.rs | Updates request helper to include severity_text: None. |
| crates/ourios-querier/tests/boundary.rs | Updates request construction to include severity_text: None. |
| crates/ourios-bench/benches/b2.rs | Updates benchmark request construction to include severity_text: None. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0f696dd to
5c105e8
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
The B1 thesis gate's query shape is `count WHERE tenant AND ts BETWEEN … AND level='ERROR'` (docs/benchmarks.md), but the throwaway QueryRequest only carried tenant + time + template_id, so the `level='ERROR'` arm couldn't be expressed — blocking any B1 measurement. Add an optional `severity_text` equality filter to the (explicitly mutable, pre-DSL) query surface. It's the structured counterpart to the B1 reference's `grep ERROR`: Ourios filters the RFC 0005 `severity_text` column, and the column's Parquet min/max statistics prune row groups that can't match — so an INFO-only file is skipped rather than scanned. The new `rfc0007_1_severity_filter_counts_and_prunes` test confirms both the count and that pruning fires (Utf8 stats pruning works for the equality predicate). The remaining construction sites (the other querier tests + the b2 bench) gain `severity_text: None`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5c105e8 to
a308c30
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
What
First step of the B1/B2 measurement plan: give the throwaway query surface the predicate B1 needs.
B1's thesis-gate query shape (docs/benchmarks.md) is:
…but
QueryRequestonly carriedtenant+time_range+template_id. Thelevel='ERROR'arm couldn't be expressed, which blocks any B1 run. This adds it.How
QueryRequestgains an optionalseverity_text: Option<String>equality filter →col(severity_text) = lit(X).grep ERROR: Ourios filters the RFC 0005severity_textcolumn, and that column's Parquet min/max statistics let DataFusion prune row groups that can't match (an INFO-only file is skipped, not scanned) — which is exactly the pillar-docs: add verification process spec #1 "skip via footer statistics" win B1 measures.Tests
rfc0007_1_severity_filter_counts_and_prunes: a mixed ERROR/INFO file plus an INFO-only file in another hour; aseverity_text='ERROR'query counts exactly the ERROR rows and prunes the INFO-only row group (row_groups_pruned >= 1). Confirms Utf8 min/max stats pruning fires for the equality predicate.QueryRequestconstruction sites gainseverity_text: None.Local:
cargo fmt --all --check,cargo clippy --workspace --all-targets --all-features -D warnings,cargo test --workspaceall green.Part of epic #81 (thesis-gate benchmark validation). Follow-up PR adds the
b1.rsbench + thezstdcat|grepreference comparator.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes