Skip to content

feat(querier): prove B2 — template-exact work tracks result, not corpus (slice 3) - #89

Merged
jensholdgaard merged 1 commit into
mainfrom
feat/querier-b2-latency
Jun 2, 2026
Merged

feat(querier): prove B2 — template-exact work tracks result, not corpus (slice 3)#89
jensholdgaard merged 1 commit into
mainfrom
feat/querier-b2-latency

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jun 2, 2026

Copy link
Copy Markdown
Owner

What

Slice 3 of the querier: proves B2 — the inverted-index-collapse claim that a template-exact query's work tracks the result size, not the corpus size. Builds on B1 (slice 2, #88).

RFC0007.2 is now a live test (tests/execution.rs::rfc0007_2_template_exact_work_scales_with_result_not_corpus), flipped from its #[ignore] red-gate stub.

How B2 is measured

Two corpora share the same target file (5 rows of template_id = 1) but differ ~8× in total size (3 vs 30 filler templates, each in its own hour ⇒ its own file ⇒ its own row group). A template_id = 1 query against each asserts:

  • rows identical (fixed result, 5 in both).
  • row_groups_scanned identical — only the target row group is read in either corpus. (the headline)
  • bytes_read identical — the bytes scanned for the fixed result do not grow with corpus (footer/metadata reads do not count toward bytes_scanned, confirmed empirically).
  • row_groups_pruned strictly larger in the big corpus, and total row groups far larger — the corpus genuinely grew, and the growth is absorbed entirely by pruning.

Methodology note (reviewers, please weigh in)

RFC 0007 §5 phrases RFC0007.2 as "measured by criterion across corpus/otel-demo-v*" (wall-clock latency). This PR proves the same claim structurally via QueryStats instead, because a deterministic scanned-work assertion is a stronger, non-flaky gate than a wall-clock median. The criterion latency bench remains supportive evidence, tracked separately (it does not gate). The acceptance-stub comment records this. I did not edit RFC 0007 §5 here (kept the PR code-only per the split-doc-from-code discipline); RFC stays specified regardless since RFC0007.3/.4 still need tests + RFC 0002.

Invariants / hazards

  • §4.6 (no DataFusion/arrow/SQL leakage): unchanged — the test asserts only on the Ourios-owned QueryStats/QueryResult. Boundary intact.
  • §3.7 (multi-tenancy): the corpus is single-tenant; tenant isolation is covered separately (RFC0007.5, live).

Verification (local)

  • cargo fmt --all --check
  • cargo clippy -p ourios-querier --all-targets --all-features -- -D warnings
  • cargo test -p ourios-querier --all-features ✅ (3 lib + 7 execution; 2 acceptance stubs .3/.4 remain #[ignore])

Closes part of #98 (B2). Epic: #82.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Updated execution status and pruning behavior documentation
  • Tests

    • Expanded test coverage for query execution and pruning operations
    • Added new test verifying that query work and I/O consumption scale with result cardinality rather than overall corpus size

…us (slice 3)

RFC0007.2 (B2, the inverted-index-collapse claim) is now a live,
deterministic test instead of a flaky wall-clock latency bench. For
a fixed-result template-exact query, the work the engine does —
`row_groups_scanned` and `bytes_read` — stays flat across an ~8×
larger corpus; the growth is absorbed entirely by row-group pruning
(`row_groups_pruned` grows, scanned does not).

`rfc0007_2_template_exact_work_scales_with_result_not_corpus` builds
two corpora sharing the same target file (5 rows of template 1) but
differing ~8× in size via filler templates, each its own row group,
and asserts scanned/bytes_read are identical while pruned grows.

The criterion latency-vs-corpus bench RFC 0007 §5 mentions is
supportive evidence, tracked separately — a structural QueryStats
assertion is the gate because it is deterministic where a wall-clock
median would be flaky. RFC 0007 stays `specified` (RFC0007.3/.4 still
need tests + RFC 0002).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jensholdgaard
jensholdgaard requested a review from Copilot June 2, 2026 10:12
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR activates the RFC0007.2 B2 work-scaling test by updating crate documentation to reflect execution slice 3, redirecting the ignored acceptance test to a new live execution test, and implementing test infrastructure and assertions to verify that row-group pruning scales efficiently with corpus growth while keeping query work flat for static template queries.

Changes

RFC0007.2 B2 Work-Scaling Test Activation

Layer / File(s) Summary
Crate status & test strategy documentation
crates/ourios-querier/src/lib.rs, crates/ourios-querier/tests/acceptance.rs
Crate-level doc advances status to "execution slice 3" and confirms B1 and B2 as live + tested; acceptance test doc removes ignored stub and adds comment redirecting RFC0007.2 criterion to live test in execution.rs.
Execution test module setup & infrastructure
crates/ourios-querier/tests/execution.rs
Module doc clarifies RFC predicate/pruning coverage; new HOUR_NS constant enables hour-partitioned row-group layout; new corpus_with_filler helper generates target template at one partition plus configurable filler templates in separate hour partitions for pruning assertions.
RFC0007.2 template-exact work-scaling test
crates/ourios-querier/tests/execution.rs
New rfc0007_2_template_exact_work_scales_with_result_not_corpus test verifies row_groups_scanned and bytes_read remain flat across small and large corpora for fixed template_id exact queries, while row_groups_pruned and total row-group count increase with larger corpus.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • jensholdgaard/ourios#88: Both PRs update execution test assertions on Querier::run QueryStats for pruning metrics, with #88 enabling B1 tenant-isolation stats and this PR adding RFC0007.2 B2 work-scaling verification.
  • jensholdgaard/ourios#87: This PR extends the execution test scaffolding introduced in #87 with a new RFC0007.2 test using the same corpus_with_filler pattern for template-exact pruning assertions.
  • jensholdgaard/ourios#86: This PR builds on the RFC0007 test scaffolding and acceptance test framework established in #86 by converting the ignored RFC0007.2 stub into a live execution test.

Poem

🐰 Slice three hops along,
B2 now runs strong—
Row groups prune tight,
Work stays light,
While corpora grow all day long!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: proving B2 (template-exact work tracks result not corpus) at slice 3, directly matching the PR's core objective.
Description check ✅ Passed The description covers all required template sections with substantial detail: clear summary of what/why, related issue/RFC links, and a complete verification checklist with all items marked completed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/querier-b2-latency

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Slice 3 of the querier flips RFC0007.2 (B2) from an ignored red-gate stub to a live integration test. It proves structurally — via QueryStats rather than wall-clock latency — that a template-exact query's scanned work (row_groups_scanned, bytes_read) stays flat as the corpus grows ~8×, with the growth fully absorbed by pruning.

Changes:

  • Added a corpus_with_filler helper plus the live rfc0007_2_template_exact_work_scales_with_result_not_corpus integration test.
  • Replaced the RFC0007.2 ignored acceptance stub with a pointer comment explaining the structural-vs-latency methodology.
  • Updated crate-level docs to reflect "execution slice 3" with B2 now live.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
crates/ourios-querier/tests/execution.rs Adds B2 corpus helper and live test asserting scanned work is flat across an ~8× larger corpus.
crates/ourios-querier/tests/acceptance.rs Removes the RFC0007.2 ignored stub; replaces with note about live structural test + supportive criterion bench.
crates/ourios-querier/src/lib.rs Bumps status to slice 3 and documents B2 as live + tested.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/ourios-querier/tests/execution.rs (1)

273-301: ⚡ Quick win

Assert the exact scanned row-group count.

These assertions only prove the scan work is equal across the two corpora. If the planner regressed to scanning one extra fixed row group in both cases, this test would still pass even though the fixture is built so only the target row group should be read. Adding an exact row_groups_scanned == 1 check would close that false-positive gap.

Suggested tightening
     assert_eq!(s.rows, 5);
     assert_eq!(l.rows, 5, "result size is fixed regardless of corpus");
+    assert_eq!(
+        s.stats.row_groups_scanned, 1,
+        "small corpus should scan only the target row group; stats={:?}",
+        s.stats,
+    );
+    assert_eq!(
+        l.stats.row_groups_scanned, 1,
+        "large corpus should scan only the target row group; stats={:?}",
+        l.stats,
+    );
 
     // The headline: the work scanned for the fixed result is FLAT
🤖 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 `@crates/ourios-querier/tests/execution.rs` around lines 273 - 301, The test
currently only asserts equality between s.stats.row_groups_scanned and
l.stats.row_groups_scanned, which allows an off-by-one regression to go
unnoticed; modify the test to assert the exact expected count (assert that
s.stats.row_groups_scanned == 1 and likewise l.stats.row_groups_scanned == 1)
using clear failure messages referencing s.stats and l.stats so the fixture
verification ensures exactly one row group is scanned rather than merely equal
counts across corpora.
🤖 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 `@crates/ourios-querier/tests/execution.rs`:
- Around line 273-301: The test currently only asserts equality between
s.stats.row_groups_scanned and l.stats.row_groups_scanned, which allows an
off-by-one regression to go unnoticed; modify the test to assert the exact
expected count (assert that s.stats.row_groups_scanned == 1 and likewise
l.stats.row_groups_scanned == 1) using clear failure messages referencing
s.stats and l.stats so the fixture verification ensures exactly one row group is
scanned rather than merely equal counts across corpora.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e3e29216-85e8-49a3-a8cf-3cf142944dc0

📥 Commits

Reviewing files that changed from the base of the PR and between 532a98d and e012155.

📒 Files selected for processing (3)
  • crates/ourios-querier/src/lib.rs
  • crates/ourios-querier/tests/acceptance.rs
  • crates/ourios-querier/tests/execution.rs

@jensholdgaard
jensholdgaard requested a review from Copilot June 2, 2026 10:19
@jensholdgaard
jensholdgaard merged commit d0bceb9 into main Jun 2, 2026
11 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants