Skip to content

quack: refuse an empty group universe in lower_group_avg; record lancedb remote cost - #1285

Merged
AdaWorldAPI merged 3 commits into
mainfrom
claude/fold-distillation-pr-wave-s57uj7
Sep 25, 2026
Merged

AdaWorldAPI merged 3 commits into
mainfrom
claude/fold-distillation-pr-wave-s57uj7

Conversation

@AdaWorldAPI

Copy link
Copy Markdown
Owner

Follow-up to #1282, which merged before these two commits.

lower_group_avg refuses groups == 0 (codex P2 on #1282)

Codex flagged the new Pair arm. The problem is wider: the count half of every AVG plan is GroupReduce Count, and that terminal needs a non-empty Out::I64. So a K = 0 plan could never execute for Local and Via keys either, and the zero case was accepted for all of them. The function now returns LowerError::EmptyGroupUniverse for every key kind, the same as lower_group_having.

  • New test lower_group_avg_refuses_an_empty_group_universe_for_every_key_kind. It covers Local and Pair keys, and asserts both that K = 0 is refused and that K = 1 lowers.
  • The test went red before the fix (Local(Col(1)): Ok where Err was expected). A disable run that deletes the guard turns it red again.
  • quack: 28 lib + 39 integration tests green; clippy -D warnings clean.

The remote feature's cost, recorded (stranded from #1282)

crates/lance-graph/Cargo.toml and the sweep's board entry now record the measured cost of lancedb's remote feature. Measured with cargo tree -p lance-graph --features lancedb-sdk, it adds 10 crates (630 vs 620):

  • New: arrow-flight, axum 0.7 and 0.8 (two majors), axum-core ×2, matchit ×2, tower-http, serde_path_to_error, httpdate.
  • Already present via lance: reqwest, tonic, prost, http.

This part is a comment and documentation only; no behaviour changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GXUahz73MZxtxWcfpHp9dG


Generated by Claude Code

Codex P2 on #1282 (Pair arm), and wider than reported: the count half is
GroupReduce Count for every key kind, and that terminal needs a non-empty
Out::I64, so a groups == 0 plan could never execute for Local or Via keys
either. Reject with LowerError::EmptyGroupUniverse, as lower_group_having
already does. Test covers Local and Pair, both directions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GXUahz73MZxtxWcfpHp9dG
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: d9606a72-976f-4783-beef-bd9641bf56de

📥 Commits

Reviewing files that changed from the base of the PR and between c1ed7ff and 9d4a7cc.

📒 Files selected for processing (3)
  • .claude/board/entries/2026-09-25-lance12-lancedb039-sweep.md
  • crates/lance-graph-quack/src/lib.rs
  • crates/lance-graph/Cargo.toml
 _____________________________________________________________________________
< Hear the roar of my V12 GPU engine? 120 TFLOPS of pure bug detection power. >
 -----------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

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

@cursor

cursor Bot commented Sep 25, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_a155aa7b-0e9b-4980-9c42-702eaaaafaf1)

@AdaWorldAPI
AdaWorldAPI marked this pull request as ready for review September 25, 2026 02:10
@AdaWorldAPI
AdaWorldAPI merged commit e4df70c into main Sep 25, 2026
10 of 11 checks passed
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