fix(buzz-db): return an empty page from get_channel_window when limit is 0 - #3536
fix(buzz-db): return an empty page from get_channel_window when limit is 0#3536hasky00 wants to merge 2 commits into
Conversation
13c6811 to
b0ec12a
Compare
… is 0 get_channel_window derived `has_more` from the pre-truncation `limit + 1` probe but read `next_cursor` from the truncated row set, so a `limit == 0` call returned `has_more = true` with `next_cursor = None` — breaking the documented `Some` iff `has_more` invariant and panicking or infinitely looping any caller that trusts it. Guard `limit == 0` and return an empty, exhausted page, which also skips a needless database round-trip. Adds a Postgres-gated regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Hasky <239224029+hasky00@users.noreply.github.com>
b0ec12a to
031424a
Compare
Chessing234
left a comment
There was a problem hiding this comment.
Clear contract fix. The early limit == 0 return restores the documented next_cursor.is_some() iff has_more invariant that the limit + 1 probe otherwise breaks (probe row → has_more=true → truncate to empty → next_cursor=None). The ignored Postgres test is the right shape for this crate. One nit: consider whether any caller currently depends on the broken behavior as a "sentinel" — if not, this is merge-ready.
Chessing234
left a comment
There was a problem hiding this comment.
Approving the limit==0 pagination contract fix.
|
i checked and no caller can currently depend on the limit 'limit ==0' behaviour as sentinel : let limit = filter
.limit
.map(|l| (l as u32).min(BRIDGE_WINDOW_MAX_LIMIT))
.unwrap_or(BRIDGE_WINDOW_DEFAULT_LIMIT)
.max(1);So
So the fix only changes behavior for direct library consumers calling with |
Summary
get_channel_windowinbuzz-dbcan violate its own documented paginationcontract when called with
limit == 0.ChannelWindow::next_cursoris documented asSomeiffhas_more(
crates/buzz-db/src/thread.rs). The function deriveshas_morefrom alimit + 1probe row before truncation, but derivesnext_cursorfrom thetruncated row set:
With
limit == 0and at least one matching row, this returnshas_more = trueandnext_cursor = None, breaking the invariant. Acaller that trusts the contract (
window.next_cursor.expect("has_more implies next_cursor")— exactly what the module's own pagination test does) panics,and a keyset-pagination loop would spin forever returning empty pages.
The sole current production caller clamps the limit with
.max(1), so thisisn't reachable from the HTTP surface today — but it's a latent contract
violation in a public
DatabaseAPI that any future caller can trip.Fix
Guard
limit == 0at the top ofget_channel_windowand return an empty,exhausted page (
rows: [],has_more: false,next_cursor: None). Thishonors the invariant and also skips a pointless database round-trip for a
zero-row request.
Test
Adds
channel_window_zero_limit_reports_empty_exhausted_page, mirroring theexisting
channel_window_*Postgres tests: it inserts rows, requests azero-limit window, and asserts the page is empty,
has_moreis false, andnext_cursorisNone. (Marked#[ignore = "requires Postgres"]like itssiblings.)
Verification
cargo fmt -p buzz-db -- --check— cleancargo check -p buzz-db --tests— cleancargo clippy -p buzz-db --tests— cleanThe new test requires Postgres and follows the repo's existing
#[ignore = "requires Postgres"]convention.🤖 Generated with Claude Code