Conversation
📝 WalkthroughWalkthroughDocumentation updates reflecting a major API change: measures are now defined as row-level SQL expressions without aggregation types, with aggregation specified at query time using colon syntax (e.g., Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/getting-started/mcp.md`:
- Line 54: Change the example call to use a dict/JSON-style payload instead of
keyword args: replace the call shown as query(source_model="orders",
fields=["*:count"], dimensions=["status"]) with a single dict argument (e.g.,
query({"source_model":"orders","fields":["*:count"],"dimensions":["status"]}))
so the example follows the docs' guideline to use JSON/dict syntax for MCP query
objects.
In `@docs/getting-started/python.md`:
- Around line 57-61: The example uses the Python constructor SlayerQuery(...)
when calling engine.execute(query=SlayerQuery(...)); replace the
constructor-style query object with a plain dict/JSON literal (e.g., query =
{"source_model": "...", "fields": [...], "dimensions": [...]}) and pass that
dict to engine.execute(query=...); apply the same change to the other
occurrences mentioned (around the blocks formerly at lines 90-94 and 102-106) so
all docs use JSON/dict query objects instead of SlayerQuery constructors.
🪄 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: 3ae4dcc1-c233-47ec-8df9-2102d0bc74e9
📒 Files selected for processing (7)
.claude/skills/slayer-models.md.claude/skills/slayer-query.mddocs/getting-started.mddocs/getting-started/cli.mddocs/getting-started/mcp.mddocs/getting-started/python.mddocs/getting-started/rest-api.md
💤 Files with no reviewable changes (1)
- docs/getting-started.md
| > **You:** How many orders per status? | ||
| > | ||
| > **Agent:** *calls `query(source_model="orders", fields=[{"formula": "count"}], dimensions=["status"])`* | ||
| > **Agent:** *calls `query(source_model="orders", fields=["*:count"], dimensions=["status"])`* |
There was a problem hiding this comment.
Keep MCP query object in dict form for docs consistency.
Prefer a dict payload shape in this example (e.g., query({"source_model": "...", "fields": [...], "dimensions": [...]})) instead of keyword-argument constructor style.
As per coding guidelines: “Use JSON/dict syntax for all query objects in docs and examples — not Python class constructors.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/getting-started/mcp.md` at line 54, Change the example call to use a
dict/JSON-style payload instead of keyword args: replace the call shown as
query(source_model="orders", fields=["*:count"], dimensions=["status"]) with a
single dict argument (e.g.,
query({"source_model":"orders","fields":["*:count"],"dimensions":["status"]}))
so the example follows the docs' guideline to use JSON/dict syntax for MCP query
objects.
There was a problem hiding this comment.
But this is AI agent sample output, not an actual code snippet...
There was a problem hiding this comment.
@AivanF, you're absolutely right — that line is descriptive prose illustrating what an AI agent does in a conversation, not a code snippet that users would actually run. My comment was incorrect to apply the JSON/dict syntax guideline here. Apologies for the noise!
I'll update the long-term memory to clarify this distinction so it doesn't get flagged again.
✏️ Learnings added
Learnt from: AivanF
Repo: MotleyAI/slayer PR: 25
File: docs/getting-started/mcp.md:54-54
Timestamp: 2026-04-10T12:15:07.864Z
Learning: In MotleyAI/slayer docs (e.g., docs/getting-started/mcp.md), illustrative/conversational agent output (e.g., lines like `> **Agent:** *calls \`query(...)\`*` inside blockquotes) is descriptive prose, not actual code. The JSON/dict syntax guideline only applies to real code examples/snippets, not to these narrative descriptions of what an agent does. Do not flag keyword-arg style in such prose as a violation of the JSON/dict syntax rule.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: AivanF
Repo: MotleyAI/slayer PR: 25
File: docs/getting-started/python.md:57-61
Timestamp: 2026-04-10T12:12:08.571Z
Learning: In the MotleyAI/slayer repo docs, use language-agnostic “JSON/dict syntax” when showing query objects on non-Python pages (e.g., REST API, CLI, MCP). When the page is specifically demonstrating the Python SDK directly—such as `docs/getting-started/python.md`—it is acceptable (and preferred) to show Python constructor syntax like `SlayerQuery(...)` instead of JSON/dict syntax. Avoid mixing constructors into language-agnostic examples.
| result = engine.execute(query=SlayerQuery( | ||
| source_model="orders", | ||
| fields=[{"formula": "count"}, {"formula": "revenue_sum"}], | ||
| dimensions=[{"name": "status"}], | ||
| fields=["*:count", "revenue:sum"], | ||
| dimensions=["status"], | ||
| )) |
There was a problem hiding this comment.
Use JSON/dict query objects in examples (not SlayerQuery(...) constructors).
These sections still present query objects via Python constructor style, which conflicts with the docs portability standard.
Suggested docs fix
-result = engine.execute(query=SlayerQuery(
- source_model="orders",
- fields=["*:count", "revenue:sum"],
- dimensions=["status"],
-))
+result = engine.execute(query={
+ "source_model": "orders",
+ "fields": ["*:count", "revenue:sum"],
+ "dimensions": ["status"],
+})-result = client.query(SlayerQuery(
- source_model="orders",
- fields=["*:count"],
- dimensions=["status"],
-))
+result = client.query({
+ "source_model": "orders",
+ "fields": ["*:count"],
+ "dimensions": ["status"],
+})-df = client.query_df(SlayerQuery(
- source_model="orders",
- fields=["*:count", "revenue:sum"],
- dimensions=["status"],
-))
+df = client.query_df({
+ "source_model": "orders",
+ "fields": ["*:count", "revenue:sum"],
+ "dimensions": ["status"],
+})As per coding guidelines: “Use JSON/dict syntax for all query objects in docs and examples — not Python class constructors.”
Also applies to: 90-94, 102-106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/getting-started/python.md` around lines 57 - 61, The example uses the
Python constructor SlayerQuery(...) when calling
engine.execute(query=SlayerQuery(...)); replace the constructor-style query
object with a plain dict/JSON literal (e.g., query = {"source_model": "...",
"fields": [...], "dimensions": [...]}) and pass that dict to
engine.execute(query=...); apply the same change to the other occurrences
mentioned (around the blocks formerly at lines 90-94 and 102-106) so all docs
use JSON/dict query objects instead of SlayerQuery constructors.
There was a problem hiding this comment.
Where do you see these "As per coding guidelines"? Note that this is docs/getting-started/python.md file specifically for Python.
There was a problem hiding this comment.
@AivanF, you're right — that guideline came from a previously stored learning in my knowledge base:
"Applies to
docs/**/*.{md,rst,markdown}: Use JSON/dict syntax for all query objects in docs and examples — not Python class constructors. This keeps examples portable across Python, REST API, and MCP interfaces."
However, your point is well-taken. This file is specifically the Python SDK guide, so showing SlayerQuery(...) constructors is actually the most natural and helpful form for Python users. The dict-portability rationale applies to language-agnostic docs (REST API, CLI, MCP pages), not to a page whose whole purpose is to demonstrate the Python API.
I'll refine the learning to reflect this distinction.
Note: I've also flagged the existing broad learning (ID
af85caa4) for removal, as it is now superseded by the more precise one above. You can also delete it manually via the CodeRabbit UI learnings page if needed.
✏️ Learnings added
Learnt from: AivanF
Repo: MotleyAI/slayer PR: 25
File: docs/getting-started/python.md:57-61
Timestamp: 2026-04-10T12:12:08.571Z
Learning: In the MotleyAI/slayer repo, the rule "Use JSON/dict syntax for query objects in docs — not Python class constructors" applies to language-agnostic documentation (e.g., REST API, CLI, MCP pages). The Python-specific guide `docs/getting-started/python.md` may (and should) use `SlayerQuery(...)` constructor syntax, as it is demonstrating the Python SDK directly.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-04-08T11:48:40.209Z
Learning: Applies to docs/**/*.{md,rst,markdown} : Use JSON/dict syntax for all query objects in docs and examples — not Python class constructors. Write `{"name": "stores.name"}` not `ColumnRef(name="stores.name")`, `{"formula": "count"}` not `Field(formula="count")`. This keeps examples portable across Python, REST API, and MCP interfaces.
🧠 Learnings used
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T11:46:34.327Z
Learning: Applies to **/slayer/**/*generator*.py : Result column keys use `model_name.column_name` format (e.g., `"orders.count"`); multi-hop joined dimensions include full path (e.g., `"orders.customers.regions.name"`)
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T11:46:34.327Z
Learning: Applies to **/slayer/**/*generator*.py : Result column naming: `revenue:sum` → `orders.revenue_sum` (colon becomes underscore), `*:count` → `orders.count`
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T11:46:34.327Z
Learning: Applies to **/slayer/**/formula*.py : Use `*:count` for COUNT(*) — `*` means "all rows", not a magic keyword
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T11:46:34.327Z
Learning: Applies to **/slayer/**/models*.py : SlayerModel has optional `default_time_dimension` field for time-dependent formula resolution
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T11:46:34.327Z
Learning: Applies to **/*.{py,yaml,yml,json} : Use bare column names for dimension/measure SQL (e.g., `"amount"`); use `model_name.column_name` for complex expressions (e.g., `"orders.amount * orders.quantity"`)
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T11:46:34.327Z
Learning: Applies to **/slayer/**/query*.py : `SlayerQuery.source_model` accepts a model name, inline `SlayerModel`, or `ModelExtension`; `create_model_from_query()` saves a query as a permanent model
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T11:46:34.327Z
Learning: Applies to **/slayer/**/formula*.py : Available formula transforms: cumsum, time_shift, change, change_pct, rank, last, lag, lead
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-04-08T11:48:40.209Z
Learning: Applies to docs/**/*.{md,rst,markdown} : Use JSON/dict syntax for all query objects in docs and examples — not Python class constructors. Write `{"name": "stores.name"}` not `ColumnRef(name="stores.name")`, `{"formula": "count"}` not `Field(formula="count")`. This keeps examples portable across Python, REST API, and MCP interfaces.
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T11:46:34.327Z
Learning: Applies to **/slayer/**/*generator*.py : Use `__`-delimited path aliases in SQL for joined tables to disambiguate diamond joins (e.g., `customers__regions`); in queries use dots (`customers.regions.name`)
Learnt from: CR
Repo: MotleyAI/slayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T11:46:34.327Z
Learning: Applies to **/slayer/**/*generator*.py : Use sqlglot AST building for SQL generation, not string concatenation
There was a problem hiding this comment.
CLAUDE.md. Probably should add an exception there for docs/examples that are purely Python -specific; Though these take raw dicts now as well btw
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
2 participants
Summary by CodeRabbit
revenue:sum,*:count).