fix(query): preserve subquery limit after offset skips - #730
fix(query): preserve subquery limit after offset skips#730QuantumExplorer wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughProtocol version v3 now enables ChangesSubquery Offset-Consumed-Empty Limit Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #730 +/- ##
===========================================
+ Coverage 91.42% 91.43% +0.01%
===========================================
Files 236 236
Lines 67053 67116 +63
===========================================
+ Hits 61305 61370 +65
+ Misses 5748 5746 -2
🚀 New features to boost your workflow:
|
789d5ff to
5687dce
Compare
* feat(version): add GROVE_V4, behaviourally identical to V3 GROVE_V3 is live, so a fix that changes an accepted/rejected outcome, a committed root hash, or a tracked cost cannot be applied unconditionally — nodes carrying it would diverge from nodes that do not. There is currently nowhere for such a fix to land, which has left several of them stuck: - #776: overwriting an indexed tree with a bare Reference skips the per-axis secondary cleanup. Closing it costs an extra stored-element read on EVERY reference overwrite (+1 seek, +79 storage_loaded_bytes, measured by the refresh-reference cost tests), and references over plain trees are shipped functionality. - Batch DeleteTree treats the caller-declared tree type as authoritative when selecting cleanup namespaces. Reading the stored element instead fixes both an indexed type-confusion and a live CommitmentTree wrong-emptiness-path bug, but adds a read to a released path. - Per project notes, five audit-fix PRs (#726, #730, #732, #734, #739) are gated on v3 and need re-gating before they can merge. This adds the version and nothing else. Every method-version slot is copied from V3 unchanged, so activating protocol version 4 today is a no-op; each gate is a deliberate, separately-reviewable slot bump. Verified rather than assumed: registering V4 changes what `GroveVersion::latest()` resolves to, and the whole test suite defaults to latest. The full workspace suite passes with V4 as latest (2459 grovedb + 705 merk + the rest), and the only two failures were the version registry's own self-describing tests — `grove_version_latest_returns_v3` and `grove_versions_count` — which are updated here. That is the evidence the change is inert. Adds `grove_v4_is_behaviourally_identical_to_v3_until_a_gate_is_added`, which compares every slot and fails the moment one is bumped. That failure is the intended prompt to document the gate rather than let V4 accrete behaviour silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(version): drop the V3/V4 slot-identity assertion It pinned V4's initial state as inert, which was worth verifying once but becomes churn the moment a gate is added — and the DeleteTree read and #776 are both queued to gate on V4 next, so it would fail immediately and be deleted anyway. The evidence it provided is preserved where it belongs: the PR description records that the full workspace suite passed with V4 as latest and that only the registry's own self-describing tests changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
Fixes #690.
Verification
Summary by CodeRabbit
Bug Fixes
Tests