Skip to content

refactor(wren-core): cube Phase C cleanup nits#2276

Merged
goldmedal merged 2 commits into
feat/wasm-cubefrom
feat/cube-cleanup
May 14, 2026
Merged

refactor(wren-core): cube Phase C cleanup nits#2276
goldmedal merged 2 commits into
feat/wasm-cubefrom
feat/cube-cleanup

Conversation

@goldmedal
Copy link
Copy Markdown
Collaborator

@goldmedal goldmedal commented May 14, 2026

Summary

Follow-up to PR #2265 — addresses the four CodeRabbit nits left unresolved at merge.

Nit 1+2 — tidy cube module exports (a00143f2)

  • Moved pub use cube::{cube_query_to_sql, CubeQuery} to sit directly below pub mod cube; to match the file's convention.
  • Narrowed cube to pub(crate) mod cube; so the curated mdl::{cube_query_to_sql, CubeQuery} surface is the single canonical path. Secondary DTOs (Granularity, CubeFilter, FilterValue, …) stay internal until a binding layer needs them.
  • Verified wren-core-py and wren-core-wasm still compile — neither imports the cube DTOs.

Nit 3+4 — scope measure resolution + cache regex (725e6cb8)

  • resolve_measures now walks the transitive closure of query.measures instead of every measure on the cube. A cycle among unrelated measures no longer fails an otherwise valid query.
  • Pre-compute the word-boundary regex per measure name once and reuse it for both dependency discovery and the fixpoint substitution loop.
  • Adds test_resolve_measures_ignores_unrequested_cycle covering the cycle case.

Test plan

  • RUST_MIN_STACK=8388608 cargo test --lib --tests --bins — 117 tests pass (33 cube + 84 prior)
  • cargo clippy --all-targets --all-features -- -D warnings — clean
  • cargo fmt --all -- --check — clean
  • cargo check -p wren-core-py and -p wren-core-wasm — clean
  • CI on the wren-core path-filtered workflow passes

Closes the four CodeRabbit threads on #2265.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced query planning robustness to handle measure dependency cycles more gracefully. Queries now resolve dependencies only for explicitly requested measures, preventing unrequested dependency cycles from causing query planning failures.
  • Tests

    • Added test coverage to validate proper handling of unrequested measure dependency cycles.
  • Refactor

    • Optimized internal module organization and overall code structure.

Review Change Stack

goldmedal and others added 2 commits May 14, 2026 11:39
Move the re-export below pub mod cube; to follow the file's convention
of declaring the module first, and narrow cube to pub(crate). External
consumers go through the curated mdl::{cube_query_to_sql, CubeQuery}
surface; secondary DTOs (Granularity, CubeFilter, FilterValue, …) stay
internal until a binding layer needs them. wren-core-py and
wren-core-wasm still build — neither imports the cube DTOs directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
resolve_measures now walks the transitive closure of query.measures
rather than the cube's full measure list. A cycle among measures the
query never references no longer fails an otherwise valid query.

Also pre-compute the word-boundary regex per measure name once at the
top of resolve_measures and reuse it for both dependency discovery and
fixpoint substitution, instead of rebuilding the regex on every call.

Adds test_resolve_measures_ignores_unrequested_cycle for the cycle case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e2b7f2c0-cda8-4da6-af7f-b74ea9e1e7fb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/cube-cleanup

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 and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core rust Pull requests that update rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant