Allow strings instead of jsons in queries' dimensions, measures, time dimensions, and order.column#15
Merged
ZmeiGorynych merged 5 commits intomainfrom Apr 9, 2026
Conversation
…ns, and order.column in queries
Contributor
📝 WalkthroughWalkthroughThe pull request simplifies the query interface by allowing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/examples/03_auto_ingest/auto_ingest.ipynb (1)
379-379: Inconsistentorder.columnformat — should use shorthand string.The
orderentries still use the verbose object form{"column": {"name": "quantity_sum"}, "direction": "desc"}while other fields (fields,dimensions) have been updated to use shorthand strings.For consistency with the PR's shorthand syntax and other updated examples, this should be:
- "order": [{"column": {"name": "quantity_sum"}, "direction": "desc"}], + "order": [{"column": "quantity_sum", "direction": "desc"}],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/03_auto_ingest/auto_ingest.ipynb` at line 379, Replace the verbose order entry objects inside the engine.execute calls with the shorthand string form for the column (e.g., replace {"column": {"name": "quantity_sum"}, "direction": "desc"} with "quantity_sum") in both query invocations shown (the 1-hop and multi-hop examples) so the order list matches the shorthand syntax used for fields and dimensions; update the order arrays passed to engine.execute accordingly.docs/examples/06_multistage_queries/multistage_queries.ipynb (1)
173-173: Use shorthandorder.columnhere for consistency with the rest of the notebook.Line 173 and Line 193 still use nested
{"column": {"name": ...}}; switching to{"column": "..."}would match the new style shown elsewhere in this PR.Also applies to: 193-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/06_multistage_queries/multistage_queries.ipynb` at line 173, The order specification for the engine.execute query uses the verbose form {"column": {"name": "..."}}; update the query passed to engine.execute (the dict assigned to query in the Stage 1 total orders per customer block that produces inner_result) to use the shorthand form {"column": "orders.count"} (and similarly for the other occurrence in the notebook) so the order entries follow the consistent {"column": "<column_name>","direction": "desc"} style used elsewhere in this PR.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@slayer/core/query.py`:
- Around line 121-133: The coercion helpers _coerce_fields and
_coerce_dimensions currently iterate over any non-None input, causing scalar
strings or dicts to be treated like iterables (e.g., "count" → chars); update
both functions to first validate the input is a list (or tuple) and raise a
TypeError (or ValueError) if it's a scalar or other non-iterable type, then
perform the existing comprehension only on the validated sequence; specifically
change _coerce_fields and _coerce_dimensions to check isinstance(v, (list,
tuple)) before mapping and return or raise for anything else to fail fast on
invalid inputs.
---
Nitpick comments:
In `@docs/examples/03_auto_ingest/auto_ingest.ipynb`:
- Line 379: Replace the verbose order entry objects inside the engine.execute
calls with the shorthand string form for the column (e.g., replace {"column":
{"name": "quantity_sum"}, "direction": "desc"} with "quantity_sum") in both
query invocations shown (the 1-hop and multi-hop examples) so the order list
matches the shorthand syntax used for fields and dimensions; update the order
arrays passed to engine.execute accordingly.
In `@docs/examples/06_multistage_queries/multistage_queries.ipynb`:
- Line 173: The order specification for the engine.execute query uses the
verbose form {"column": {"name": "..."}}; update the query passed to
engine.execute (the dict assigned to query in the Stage 1 total orders per
customer block that produces inner_result) to use the shorthand form {"column":
"orders.count"} (and similarly for the other occurrence in the notebook) so the
order entries follow the consistent {"column": "<column_name>","direction":
"desc"} style used elsewhere in this PR.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7600f03-1f9a-40d5-aa43-4572a351ac9f
📒 Files selected for processing (23)
docs/concepts/formulas.mddocs/concepts/ingestion.mddocs/concepts/models.mddocs/concepts/queries.mddocs/examples/02_sql_vs_dsl/sql_vs_dsl.ipynbdocs/examples/03_auto_ingest/auto_ingest.ipynbdocs/examples/03_auto_ingest/auto_ingest.mddocs/examples/04_time/time.ipynbdocs/examples/04_time/time.mddocs/examples/05_joined_measures/joined_measures.ipynbdocs/examples/05_joined_measures/joined_measures.mddocs/examples/05_joins/joins.ipynbdocs/examples/06_multistage_queries/multistage_queries.ipynbdocs/examples/06_multistage_queries/multistage_queries.mddocs/getting-started.mddocs/interfaces/cli.mddocs/interfaces/mcp.mddocs/interfaces/python-client.mddocs/interfaces/rest-api.mdslayer/core/query.pyslayer/mcp/server.pytests/test_mcp_server.pytests/test_models.py
💤 Files with no reviewable changes (1)
- tests/test_mcp_server.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/examples/05_joins/joins.ipynb (2)
152-156: Add explicit ordering for reproducible example outputs.These examples display limited/grouped results but don’t define a stable
order, so printed rows can vary across runs/backends.Deterministic ordering example
result = engine.execute(query={ "source_model": "orders", "fields": ["count"], "dimensions": ["stores.name"], + "order": [{"column": "count", "direction": "desc"}, {"column": "stores.name", "direction": "asc"}], })result = engine.execute(query={ "source_model": "order_items", "fields": ["count"], "dimensions": ["orders.customers.name"], + "order": [{"column": "count", "direction": "desc"}, {"column": "orders.customers.name", "direction": "asc"}], "limit": 3, })Also applies to: 271-276
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/05_joins/joins.ipynb` around lines 152 - 156, The query passed to engine.execute (the dict with "source_model": "orders", "fields": ["count"], "dimensions": ["stores.name"]) lacks an explicit ordering which makes example outputs non-deterministic; update these example queries (including the similar block at lines referenced 271-276) to include a stable "order" clause (e.g., ordering by the grouped dimension and/or aggregated field) in the query dict so printed rows are reproducible across runs and backends (modify the example query objects used by engine.execute to add an "order" key with a deterministic sort specification).
27-34: Strip volatile notebook execution metadata to reduce diff noise.The timestamp-heavy
metadata.executionblocks will churn on every run and obscure meaningful doc changes. Consider clearing execution metadata before committing notebooks.Suggested normalization pattern
- "metadata": { - "execution": { - "iopub.execute_input": "...", - "iopub.status.busy": "...", - "iopub.status.idle": "...", - "shell.execute_reply": "..." - } - }, + "metadata": {},Also applies to: 62-69, 128-135, 167-174, 225-232, 299-306, 378-385, 424-431, 485-492
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/05_joins/joins.ipynb` around lines 27 - 34, The notebook contains volatile timestamped execution metadata under the "metadata.execution" objects (e.g., keys like "iopub.execute_input", "iopub.status.busy", "iopub.status.idle", "shell.execute_reply") which churns diffs; remove or sanitize these blocks across the notebook (and similar per-cell execution fields such as "execution_count" and per-cell "outputs" if desired) before committing. Use a normalization step (nbstripout, nbformat-based script, or CI pre-commit hook) to strip "metadata.execution" and other transient execution fields from the joins.ipynb content so that only meaningful notebook changes remain. Ensure you apply the same stripping to the other listed ranges/cells to eliminate repeated timestamped entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/examples/05_joins/joins.ipynb`:
- Around line 152-156: The query passed to engine.execute (the dict with
"source_model": "orders", "fields": ["count"], "dimensions": ["stores.name"])
lacks an explicit ordering which makes example outputs non-deterministic; update
these example queries (including the similar block at lines referenced 271-276)
to include a stable "order" clause (e.g., ordering by the grouped dimension
and/or aggregated field) in the query dict so printed rows are reproducible
across runs and backends (modify the example query objects used by
engine.execute to add an "order" key with a deterministic sort specification).
- Around line 27-34: The notebook contains volatile timestamped execution
metadata under the "metadata.execution" objects (e.g., keys like
"iopub.execute_input", "iopub.status.busy", "iopub.status.idle",
"shell.execute_reply") which churns diffs; remove or sanitize these blocks
across the notebook (and similar per-cell execution fields such as
"execution_count" and per-cell "outputs" if desired) before committing. Use a
normalization step (nbstripout, nbformat-based script, or CI pre-commit hook) to
strip "metadata.execution" and other transient execution fields from the
joins.ipynb content so that only meaningful notebook changes remain. Ensure you
apply the same stripping to the other listed ranges/cells to eliminate repeated
timestamped entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6bb3d5e5-934f-405c-8595-df60d0da9e4f
📒 Files selected for processing (6)
docs/examples/02_sql_vs_dsl/sql_vs_dsl.ipynbdocs/examples/03_auto_ingest/auto_ingest.ipynbdocs/examples/04_time/time.ipynbdocs/examples/05_joined_measures/joined_measures.ipynbdocs/examples/05_joins/joins.ipynbdocs/examples/06_multistage_queries/multistage_queries.ipynb
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/examples/03_auto_ingest/auto_ingest.ipynb
- docs/examples/04_time/time.ipynb
- docs/examples/05_joined_measures/joined_measures.ipynb
- docs/examples/06_multistage_queries/multistage_queries.ipynb
- docs/examples/02_sql_vs_dsl/sql_vs_dsl.ipynb
…ev-1221-allow-plain-strings-as-some-of-the-entries-in
…ev-1221-allow-plain-strings-as-some-of-the-entries-in
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/examples/03_auto_ingest/auto_ingest_nb.ipynb (1)
462-481: Consider using shorthand fororder.columnhere too for style consistency.This notebook now mixes shorthand and nested
{"name": ...}in adjacent examples. Since shorthand is the focus of this PR, switching these twoorder.columnentries keeps the docs uniform.Suggested consistency patch
- "order": [{"column": {"name": "quantity_sum"}, "direction": "desc"}], + "order": [{"column": "quantity_sum", "direction": "desc"}], @@ - "order": [{"column": {"name": "quantity_sum"}, "direction": "desc"}], + "order": [{"column": "quantity_sum", "direction": "desc"}],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/03_auto_ingest/auto_ingest_nb.ipynb` around lines 462 - 481, The notebook mixes shorthand and nested {"name": ...} styles in adjacent examples for the order column; update the two engine.execute calls that set "order": [{ "column": { "name": "quantity_sum" }, "direction": "desc" }] to use the shorthand form (e.g. "order": [{ "column": "quantity_sum", "direction": "desc" }]) so both the 1-hop (order_items -> products) and multi-hop (order_items -> orders -> customers) queries use the same shorthand style and keep the docs consistent; look for the engine.execute invocations and the "order" entries to apply the change.docs/examples/06_multistage_queries/multistage_queries_nb.ipynb (1)
309-315: Optional: switch remainingorder.columnobjects to shorthand for consistency.These two query blocks still use
{"column": {"name": ...}}while surrounding updates use string shorthand.Suggested consistency patch
- "order": [{"column": {"name": "count"}, "direction": "desc"}], + "order": [{"column": "count", "direction": "desc"}], @@ - "order": [{"column": {"name": "order_total_sum"}, "direction": "desc"}], + "order": [{"column": "order_total_sum", "direction": "desc"}],Also applies to: 390-393
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/06_multistage_queries/multistage_queries_nb.ipynb` around lines 309 - 315, The query objects passed to engine.execute use verbose order entries like {"column": {"name": "count"}}, which should be converted to the shorthand string form used elsewhere; update the order arrays in the inner_result query (and the later similar query) to use order entries with "column": "count" (i.e., replace the nested {"column": {"name": ...}} objects with the equivalent string shorthand) so the query objects are consistent with the surrounding examples.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/examples/03_auto_ingest/auto_ingest_nb.ipynb`:
- Around line 462-481: The notebook mixes shorthand and nested {"name": ...}
styles in adjacent examples for the order column; update the two engine.execute
calls that set "order": [{ "column": { "name": "quantity_sum" }, "direction":
"desc" }] to use the shorthand form (e.g. "order": [{ "column": "quantity_sum",
"direction": "desc" }]) so both the 1-hop (order_items -> products) and
multi-hop (order_items -> orders -> customers) queries use the same shorthand
style and keep the docs consistent; look for the engine.execute invocations and
the "order" entries to apply the change.
In `@docs/examples/06_multistage_queries/multistage_queries_nb.ipynb`:
- Around line 309-315: The query objects passed to engine.execute use verbose
order entries like {"column": {"name": "count"}}, which should be converted to
the shorthand string form used elsewhere; update the order arrays in the
inner_result query (and the later similar query) to use order entries with
"column": "count" (i.e., replace the nested {"column": {"name": ...}} objects
with the equivalent string shorthand) so the query objects are consistent with
the surrounding examples.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 51a81eee-adad-4156-a813-faef85e47ec9
📒 Files selected for processing (10)
docs/examples/02_sql_vs_dsl/sql_vs_dsl_nb.ipynbdocs/examples/03_auto_ingest/auto_ingest.mddocs/examples/03_auto_ingest/auto_ingest_nb.ipynbdocs/examples/04_time/time.mddocs/examples/04_time/time_nb.ipynbdocs/examples/05_joined_measures/joined_measures.mddocs/examples/05_joined_measures/joined_measures_nb.ipynbdocs/examples/05_joins/joins_nb.ipynbdocs/examples/06_multistage_queries/multistage_queries.mddocs/examples/06_multistage_queries/multistage_queries_nb.ipynb
✅ Files skipped from review due to trivial changes (4)
- docs/examples/06_multistage_queries/multistage_queries.md
- docs/examples/04_time/time.md
- docs/examples/05_joined_measures/joined_measures.md
- docs/examples/03_auto_ingest/auto_ingest.md
AivanF
approved these changes
Apr 9, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fields list items — plain string treated as {"formula": string}
"fields": ["count", "revenue_sum", "cumsum(revenue)"]
// equivalent to
"fields": [{"formula": "count"}, {"formula": "revenue_sum"}, {"formula": "cumsum(revenue)"}]
Mix-and-match works: ["count", {"formula": "revenue / count", "name": "aov", "label": "Avg Order Value"}]
dimensions list items — plain string treated as {"name": string}
"dimensions": ["status", "customers.regions.name"]
// equivalent to
"dimensions": [{"name": "status"}, {"name": "customers.regions.name"}]
Dotted join paths work. Use the dict form when you need label or sql.
time_dimensions[].dimension — plain string treated as {"name": string}
"time_dimensions": [{"dimension": "created_at", "granularity": "month"}]
// equivalent to
"time_dimensions": [{"dimension": {"name": "created_at"}, "granularity": "month"}]
order[].column — plain string treated as {"name": string}
"order": [{"column": "revenue_sum", "direction": "desc"}]
// equivalent to
"order": [{"column": {"name": "revenue_sum"}, "direction": "desc"}]
In all four cases:
Summary by CodeRabbit
Release Notes
New Features
fields: ["count"]instead offields: [{"formula": "count"}]anddimensions: ["status"]instead ofdimensions: [{"name": "status"}].Documentation
Tests