Skip to content

test(rfc0036): red — all five §5 stubs land, status specified→red - #586

Merged
jensholdgaard merged 2 commits into
mainfrom
rfc0036-red
Jul 21, 2026
Merged

test(rfc0036): red — all five §5 stubs land, status specified→red#586
jensholdgaard merged 2 commits into
mainfrom
rfc0036-red

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Summary

RFC 0036 (write-side layout — compacted-partition clustering and row-group sizing) moves specifiedred: the five §5 acceptance scenarios land as #[ignore]d stubs, each failing on todo!() when force-run, per the docs/rfcs/README.md lifecycle ("test stubs exist and fail") and the RFC 0033 red precedent (b49758e / #508).

Placement (per RFC 0036 §6):

  • crates/ourios-parquet/tests/it/rfc0036_write_side_layout.rs — RFC0036.1 (compacted layout: clustering + sizing + sorting_columns), RFC0036.3 (D2/D3/memory properties preserved), RFC0036.4 (rebuild byte-identity), RFC0036.5 (no read-path/schema regression). These gate compaction.rs/writer.rs machinery, so they live in the parquet single-binary harness (RFC 0028 layout).
  • crates/ourios-querier/tests/it/rfc0036_window_materialization.rs — RFC0036.2's in-repo slice (the synthetic-hour scanned-count bound via the RFC 0016 scanned/pruned counts), co-located with the existing counter assertions in execution.rs. The full comparative arm (L6-shape pair on the v8 corpus, before/after §9 bytes diagnostic) is a harness/measurement concern through the ourios-bench RFC 0031 dispatch — noted in the stub's module doc, the same treatment RFC 0033's red gave its .6.

Each #[ignore] literal names the green slice that discharges it (sorted-compaction, pruning, compaction-properties, determinism, compat); each todo!() paraphrases its scenario's Given/When/Then with the §5 anchor in the doc comment.

The RFC frontmatter flips status: specifiedstatus: red, and the status note now records the stub placement, the design-review confirmation (maintainer go, 2026-07-21), and the §7 at-red decisions (compacted-threshold sweep, run format, fan-in cap F, D2 band) deferred to the green implementation, where they are measured rather than guessed.

Invariants (§3) / hazards (§4)

This PR itself changes no behavior — stubs plus a docs status flip — but the RFC it advances lives squarely in hazard #4 (small-file problem): RFC 0036 §3.3 deliberately amends H4's row-group band, dropping compacted row groups below the 128 MB – 1 GB target (proposal ~32 MiB) while leaving the 256 MiB – 2 GiB file band — the band that actually governs LIST/footer/cold-cache economics — untouched. The one-line docs/hazards.md H4 rewording (row-group band scoped to ingest-side files; compacted threshold = pruning-granularity knob) ships with the green implementation PR, per RFC 0036 §7. The RFC also pins CLAUDE.md §3.5 (no Parquet schema change; sorting_columns is pure footer metadata, RFC0036.5 gates no-migration reads) and §3.6 (sort-run spill to local scratch is cache, not truth). No invariant or hazard surface is touched by this red PR's diff.

Verification

  • cargo fmt --all --check — clean
  • cargo clippy --all-targets --all-features -- -D warnings — clean (48.5 s)
  • cargo test --all-features — green: 1201 passed, 0 failed, 42 ignored across 54 suites (the five new stubs skipped by default)
  • Force-run red proof: cargo test -p ourios-parquet --test it rfc0036 -- --ignored → 4 failed on todo!() (RFC0036.1/.3/.4/.5); cargo test -p ourios-querier --test it rfc0036 -- --ignored → 1 failed on todo!() (RFC0036.2)
  • cargo doc --workspace --no-deps --all-features — clean (the CI invocation; the featureless run has a pre-existing, unrelated ourios-telemetry intra-doc-link gap behind the testing feature)
  • mdbook build — builds; RFC 0036 status note renders

Related

RFC: docs/rfcs/0036-write-side-layout.md. Precedent: b49758e (#508, RFC 0033 red). Discharges the write-side-layout line from the #498 scoreboard / docs/benchmarks.md §9.13/§9.24.

Checklist

  • cargo fmt clean
  • cargo clippy clean (no new warnings)
  • Tests added/updated
  • Docs / CHANGELOG.md updated (RFC status flip + status note; no user-facing change)
  • RFC linked

🤖 Generated with Claude Code

https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

Summary by CodeRabbit

  • Tests

    • Added integration-test coverage placeholders for RFC 0036 write-side layout and window-query materialization scenarios.
    • Tests are currently disabled by default while implementation work is completed.
  • Documentation

    • Updated RFC 0036 status to reflect the current red-phase implementation state and documented deferred decisions.

Five #[ignore]d stubs, one per §5 acceptance scenario (RFC0036.1–.5);
default runs stay green (1201 passed, 0 failed, 42 ignored across the
workspace; stubs skipped) and force-running the ignored stubs fails
all five on todo!(). Each doc comment carries the §5 anchor, each
todo!() paraphrases its scenario's Given/When/Then, and each #[ignore]
literal names the green slice that discharges it: sorted-compaction
(.1), pruning (.2), compaction-properties (.3), determinism (.4),
compat (.5).

Placement follows §6's mapping: RFC0036.1/.3/.4/.5 in
crates/ourios-parquet/tests/it/rfc0036_write_side_layout.rs — the
machinery they gate (the §3.2 sort-run merge, the §3.3 compacted
threshold, the §3.4 sorting_columns declaration) is
compaction.rs/writer.rs code — and RFC0036.2's in-repo slice (the
synthetic-hour scanned-count bound) in
crates/ourios-querier/tests/it/rfc0036_window_materialization.rs,
beside the existing RFC 0016 scanned/pruned counter assertions
(execution.rs). The .2 comparative arm is a harness/measurement
concern through the ourios-bench RFC 0031 dispatch, not a CI stub —
the same treatment RFC 0033's red gave its .6 (b49758e).

RFC status flips specified→red with a status note recording the stub
placement, the design-review go (2026-07-21), and the §7 at-red
decisions (threshold sweep, run format, fan-in cap F, D2 band)
deferred to the green implementation where they are measured.

fmt clean; workspace clippy -D warnings clean; cargo test
--all-features green; cargo doc --workspace --no-deps --all-features
clean; mdbook builds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jensholdgaard, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a8caa40-a2ef-49b6-9d45-9d291b8ee3b3

📥 Commits

Reviewing files that changed from the base of the PR and between 7f48f3b and 38aaa77.

📒 Files selected for processing (1)
  • crates/ourios-parquet/tests/it/rfc0036_write_side_layout.rs
📝 Walkthrough

Walkthrough

RFC0036 integration-test stubs are added for parquet write-side layout and querier window materialization, wired into their test harnesses, and documented as part of the RFC’s new red status.

Changes

RFC0036 test scaffolding

Layer / File(s) Summary
Add and wire RFC0036 test stubs
crates/ourios-parquet/tests/it/*, crates/ourios-querier/tests/it/*
Five ignored todo!() integration tests are added for RFC0036 scenarios and registered in the consolidated test harnesses.
Record RFC red status
docs/rfcs/0036-write-side-layout.md
The RFC status changes to red, documenting the ignored stubs and deferred implementation decisions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding RFC 0036 red stubs and flipping status from specified to red.
Description check ✅ Passed The description follows the template with Summary, Related, and Checklist sections and includes the RFC link and verification details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc0036-red

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.

❤️ Share

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

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

Moves RFC 0036 (write-side layout) from specified → red by landing the five §5 acceptance-scenario test stubs (ignored by default, failing via todo!() when force-run) and updating the RFC status note accordingly. This fits the repo’s RFC maturity ladder by making the acceptance criteria executable placeholders before implementation begins.

Changes:

  • Flip RFC 0036 frontmatter status: specifiedstatus: red and update the status note with stub placement/details.
  • Add RFC0036 integration test stubs in ourios-parquet (compaction-side scenarios) and ourios-querier (window materialization slice), all #[ignore] + todo!().
  • Wire the new test modules into each crate’s consolidated tests/it harness.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
docs/rfcs/0036-write-side-layout.md Updates RFC 0036 status and status note to reflect the red gate and stub locations.
crates/ourios-querier/tests/it/rfc0036_window_materialization.rs Adds ignored red-phase stub for RFC0036.2 (querier-side window materialization bound).
crates/ourios-querier/tests/it/main.rs Registers the new RFC0036 querier test module in the tests/it harness.
crates/ourios-parquet/tests/it/rfc0036_write_side_layout.rs Adds four ignored red-phase stubs for RFC0036.1/.3/.4/.5 (parquet/compaction-side).
crates/ourios-parquet/tests/it/main.rs Registers the new RFC0036 parquet test module in the tests/it harness.

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

Comment thread crates/ourios-parquet/tests/it/rfc0036_write_side_layout.rs Outdated
Comment thread crates/ourios-parquet/tests/it/rfc0036_write_side_layout.rs Outdated

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 5 out of 5 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