Conversation
…ulas The reverse mapping in extract_filter_transforms only joined agg_args, dropping agg_kwargs entirely. Filters like "change(price:weighted_avg(weight=quantity)) > 0" would reconstruct as "change(price:weighted_avg()) > 0", losing keyword arguments.
📝 WalkthroughWalkthroughDocumentation updates across README, landing page, examples, and site navigation to document query-time aggregation features where aggregations are selected per query using colon syntax, support for custom SQL-templated aggregations, and a new aggregations tutorial section. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
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)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Comment |
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/index.md`:
- Line 62: The fenced code block in the docs architecture diagram is missing a
language tag causing MD040 lint failures; update the fence that currently starts
with ``` to include a language identifier (e.g., ```text) so the block
containing the flow lines for Agent, MCP / REST API / Python SDK, SlayerQuery,
SlayerQueryEngine, EnrichedQuery, SQLGenerator, SlayerSQLClient, and
SlayerResponse is annotated as plain text.
In `@README.md`:
- Line 27: Update the README wording for consistency: replace "parametrized"
with "parameterized" in the Aggregation-at-query-time paragraph and convert the
trailing fragment into a full sentence—e.g., end the paragraph with "Supports
per-measure `allowed_aggregations` whitelists."—so the phrasing is clearer and
consistent across the docs.
🪄 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: e11aa5f5-b179-4727-845e-d8a358a0ab3d
📒 Files selected for processing (4)
README.mddocs/examples/01_dynamic/dynamic.mddocs/index.mdmkdocs.yml
|
|
||
| ## Under the hood | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced block.
The architecture block fence is missing a language identifier (MD040), which can fail markdown linting.
Suggested fix
-```
+```text
Agent --> MCP / REST API / Python SDK
|
SlayerQuery (model, fields, dimensions, filters)
|
SlayerQueryEngine (resolves model definitions from storage)
|
EnrichedQuery (resolved SQL expressions, model metadata)
|
SQLGenerator (sqlglot AST --> dialect-aware SQL)
|
SlayerSQLClient (SQLAlchemy --> database)
|
SlayerResponse (data, columns, sql)</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/index.md` at line 62, The fenced code block in the docs architecture
diagram is missing a language tag causing MD040 lint failures; update the fence
that currently starts with ``` to include a language identifier (e.g., ```text)
so the block containing the flow lines for Agent, MCP / REST API / Python SDK,
SlayerQuery, SlayerQueryEngine, EnrichedQuery, SQLGenerator, SlayerSQLClient,
and SlayerResponse is annotated as plain text.
Summary by CodeRabbit