Skip to content

docs: accept RFC 0031/0034/0036 + correct §9.28 row-cap finding - #593

Merged
jensholdgaard merged 1 commit into
mainfrom
rfc-finalize-accepts
Jul 22, 2026
Merged

docs: accept RFC 0031/0034/0036 + correct §9.28 row-cap finding#593
jensholdgaard merged 1 commit into
mainfrom
rfc-finalize-accepts

Conversation

@jensholdgaard

Copy link
Copy Markdown
Owner

Finalizes the RFC 0036 arc.

Accepted flips (maintainer-authorized, 2026-07-22)

  • RFC 0031 validated → accepted — all 11 frozen comparative gates pass authoritatively (§9.24).
  • RFC 0034 specified → accepted — the D1 re-scope; a meta/re-scope RFC with no thesis-gate of its own, so specified → accepted per its own status note.
  • RFC 0036 validated → accepted — five §5 scenarios green + §9.26 baseline no-regression + §9.27 in-repo before/after + §9.28 threshold sweep.
  • Roadmap §3 ladder synced.

Correction (honesty)

The §9.28 Finding 3 I merged in #592 mis-recommended raising max_row_group_size "so the byte threshold bites." That is backwards: at this corpus's ~30 encoded B/row, arrow's default 1,048,576-row cap fills a group at ~30 MiB — which trips before the 32/64 MiB byte flush. Raising the cap would let 32/64 MiB coarsen to ~2 groups, the wrong direction for pruning. The row cap is a granularity floor that is currently helping; the real pruning lever is a smaller byte threshold (16 MiB → half the window materialization), the deferred §7 sweep question. Corrected in benchmarks.md §9.28 and the RFC §7 box.

Documented follow-ups (not blockers)

  • Comparative harness builds single-file partitions → RFC 0036's sort isn't exercised there (a multi-file compacting harness is future work).
  • The byte-threshold value tuning (16 MiB candidate) awaits the authoritative baseline sweep.

Verification

mdbook build clean. Doc-only.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

Maintainer-authorised terminal flips (2026-07-22):
- RFC 0031 validated→accepted (all 11 frozen gates pass, §9.24).
- RFC 0034 specified→accepted (re-scope RFC, no thesis-gate of its own).
- RFC 0036 validated→accepted (five §5 green + §9.26/§9.27/§9.28).
Roadmap ladder synced.

Also corrects §9.28 Finding 3 (and the §7 box): a prior draft called the
arrow 1,048,576-row cap a "gap" and recommended raising max_row_group_size
"so the byte threshold bites". That is backwards — at ~30 B/row the cap
fills a group at ~30 MiB, tripping before the 32/64 MiB byte flush, so
raising it would COARSEN 32/64 (~2 groups), the wrong way for pruning. The
row cap is a granularity floor that is helping; the real pruning lever is a
SMALLER byte threshold (16 MiB), the §7 sweep question.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
@jensholdgaard
jensholdgaard requested a review from Copilot July 22, 2026 13:06
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 30 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: 976ed7cf-251c-49e3-98c4-3d668bfc6159

📥 Commits

Reviewing files that changed from the base of the PR and between 09cdbb7 and eea83b1.

📒 Files selected for processing (5)
  • docs/benchmarks.md
  • docs/rfcs/0031-comparative-evaluation-loki.md
  • docs/rfcs/0034-d1-rescope.md
  • docs/rfcs/0036-write-side-layout.md
  • docs/roadmap.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc-finalize-accepts

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

This PR finalizes the RFC 0036 documentation arc by flipping RFC statuses to accepted, syncing the roadmap, and correcting the previously inverted interpretation of the Parquet row-group row-cap interaction in the §9.28 sweep write-up.

Changes:

  • Mark RFC 0031 / 0034 / 0036 as accepted and update their status notes accordingly.
  • Sync docs/roadmap.md to reflect the maintainer-authorized acceptance flips.
  • Correct docs/benchmarks.md §9.28 Finding 3 to clarify that the default row cap is currently helping pruning granularity, and that the pruning lever is a smaller byte threshold (not raising the row cap).

Reviewed changes

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

Show a summary per file
File Description
docs/roadmap.md Updates the RFC ladder entries for 0031/0034/0036 to accepted with dates and brief rationale.
docs/rfcs/0036-write-side-layout.md Flips RFC 0036 status to accepted and updates the §9.28 status-note wording to correct the row-cap interpretation.
docs/rfcs/0034-d1-rescope.md Flips RFC 0034 status to accepted and updates the status note to reflect terminal state for the re-scope RFC.
docs/rfcs/0031-comparative-evaluation-loki.md Flips RFC 0031 status to accepted and updates the status note to reflect maintainer sign-off.
docs/benchmarks.md Corrects §9.28 Finding 3 to remove the prior recommendation to raise the row cap and explains the correct pruning implication.

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

@jensholdgaard
jensholdgaard merged commit d3b4324 into main Jul 22, 2026
26 checks passed
@jensholdgaard
jensholdgaard deleted the rfc-finalize-accepts branch July 22, 2026 13:13
jensholdgaard added a commit that referenced this pull request Jul 26, 2026
* docs(roadmap): refresh §3 through RFC 0041 (closes #642)

The RFC 0036 bullet under "What's actually open" described it as
`specified` with design review gating `red`, months after #593 flipped
it to `accepted` and #592/#594/#595 landed the implementation. The
roadmap is what a contributor reads to pick up work, so the entry
actively misdirected — it already did, on #640.

Checking the neighbours for the same drift found the file had gone
stale wholesale against its own §6 cadence rule (refresh whenever a
merged PR materially changes §3):

- ladder stopped at RFC 0036; adds 0037 (`green`), 0038/0039/0040
  (`green`, the self-observability arc) and 0041 (`drafted`)
- "all ten product crates" predates `-config`, `-df-otel` and
  `-testgen`; now twelve product crates plus one dev-only
- the `-telemetry` bullet described a metrics-only export surface,
  which stopped being true at RFC 0038/0039
- §5's Perses row still read "prerequisite is clear, left for after
  RFC 0031" with no pointer to RFC 0041, which now works that
  question up for both hosts

Also records the unreleased breaking change (#641) sitting on `main`
behind v0.5.0, so whoever cuts the next tag doesn't call it a patch.

Prior banner entries are left as written — they are point-in-time
records; the 2026-07-21 entry gains a one-line note that its arc closed.

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

* docs(roadmap): reconcile §5's telemetry row with the §3 refresh

Review caught the exact failure this PR exists to fix: §3 now records
RFC 0038/0039/0040 as green while §5's telemetry row still read "Traces
deliberately deferred", leaving two contradictory statuses in one file.

Also tightens the ourios-df-otel dependency claim — datafusion +
opentelemetry are its *runtime* deps; it does carry dev-dependencies
(opentelemetry_sdk, chrono, criterion). The load-bearing property is
that no ourios-* crate is among them, so say that instead.

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

---------

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
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