Skip to content

DEV-1372: MCP query tool accepts inline source_model#109

Merged
ZmeiGorynych merged 2 commits into
mainfrom
egor/dev-1372-slayer-mcp-query-tool-accept-inline-slayerquery-json-in
May 9, 2026
Merged

DEV-1372: MCP query tool accepts inline source_model#109
ZmeiGorynych merged 2 commits into
mainfrom
egor/dev-1372-slayer-mcp-query-tool-accept-inline-slayerquery-json-in

Conversation

@ZmeiGorynych
Copy link
Copy Markdown
Member

@ZmeiGorynych ZmeiGorynych commented May 8, 2026

Summary

  • MCP query tool's source_model parameter now typed str | ModelExtension | SlayerModel, matching SlayerQuery.source_model's native polymorphism. Previously typed str, which forced agents to JSON-encode inline dicts — SLayer then validated the JSON blob as a name and rejected it with Invalid model name '{...}': must not contain '/'. Wire-level evidence (Haiku 4.5, households_1): 15 / 88 query-tool calls (17 %) on a single 15-task run failed this way.
  • Run-by-name shortcut (engine.execute(str)) preserved — it layers model.query_variables → stage → runtime precedence that the regular SlayerQuery path doesn't reach. Gated on isinstance(source_model, str) so inline values fall through to SlayerQuery.model_validate. The variable-precedence asymmetry that forces this gate is filed separately as DEV-1373.
  • Docs updated in docs/reference/mcp.md and docs/interfaces/mcp.md.

Test plan

  • tests/test_mcp_server.py::TestQueryAcceptsInlineSourceModel — three new tests covering string regression, inline ModelExtension dict, inline SlayerModel dict
  • poetry run pytest -m "not integration" — 2173 passed
  • poetry run ruff check slayer/ tests/ — clean

Closes DEV-1372.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • The query tool now accepts inline model definitions and extensions for the source_model parameter, in addition to saved model names.
  • Documentation

    • Updated tool docs with type descriptions and examples covering the expanded source_model options.
  • Tests

    • Added tests verifying query accepts string, inline extension, and inline model payloads.

…nsion | SlayerModel)

Previously typed `str`, which forced agents to JSON-encode dicts; SLayer
then validated the JSON blob as a name and rejected with `Invalid model
name '{...}'`. The signature now matches `SlayerQuery.source_model`'s
native polymorphism, so FastMCP exposes both shapes in the tool schema.

The run-by-name shortcut stays — it applies `model.query_variables` as
the lowest precedence layer, which the regular `SlayerQuery` path
doesn't (see DEV-1373). Gated on `isinstance(source_model, str)` so
inline values fall through to `SlayerQuery.model_validate`.

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

coderabbitai Bot commented May 8, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2d67fcba-fb8c-432f-bd4b-a6926e017bf2

📥 Commits

Reviewing files that changed from the base of the PR and between 6934fde and 6362b8b.

📒 Files selected for processing (1)
  • slayer/mcp/server.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • slayer/mcp/server.py

📝 Walkthrough

Walkthrough

This PR expands the MCP query tool's source_model parameter to accept three forms: a stored model name (string), an inline ModelExtension object, or an inline SlayerModel object. Changes include imports, type signature updates, dispatch logic refinement, documentation updates, and test coverage.

Changes

Polymorphic source_model Support

Layer / File(s) Summary
Type Contract & Imports
slayer/mcp/server.py
ModelExtension imported; query tool parameter type updated from str to str | ModelExtension | SlayerModel.
Query Tool Implementation
slayer/mcp/server.py
Help text expanded to describe three source_model forms; run-by-name shortcut reworked to dispatch correctly based on parameter type and use local model_name when executing stored models.
API Documentation
docs/interfaces/mcp.md, docs/reference/mcp.md
Parameter documentation updated to show all three supported source_model shapes with example structures.
Test Coverage
tests/test_mcp_server.py
New test class TestQueryAcceptsInlineSourceModel verifies string model names, inline extensions, and inline model definitions are all accepted and produce generated SQL.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • MotleyAI/slayer#67: Directly related prior work on expanding the same MCP query tool to accept inline model objects and adjust dispatch behavior.
  • MotleyAI/slayer#10: Related implementation changes to the MCP query tool parameters and request handling.

Suggested reviewers

  • whimo

Poem

🐰 A bunny's burrow of models deep,
Now stores in forms both short and steep,
Strings and objects, side by side,
The query tool opens gates so wide,
Three flavors blend in SQL delight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: the MCP query tool now accepts inline source_model objects, not just string references.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch egor/dev-1372-slayer-mcp-query-tool-accept-inline-slayerquery-json-in

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
slayer/mcp/server.py (1)

1055-1056: ⚡ Quick win

Use query= keyword argument in the engine.execute() call.

Line 1055–1056 passes model_name as a positional argument, violating the repository guideline for multi-parameter functions. The execute() method signature defines the first parameter as query, and all other call sites in this file consistently use query= explicitly.

Change:

result = await engine.execute(
    model_name,

To:

result = await engine.execute(
    query=model_name,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/mcp/server.py` around lines 1055 - 1056, The call to engine.execute is
passing model_name positionally; change it to use the query= keyword so it
matches the execute(query, ...) signature and the project's convention. Locate
the engine.execute(...) invocation around the result = await engine.execute(
model_name, and replace the positional model_name argument with
query=model_name, leaving other arguments unchanged to preserve call semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@slayer/mcp/server.py`:
- Around line 1055-1056: The call to engine.execute is passing model_name
positionally; change it to use the query= keyword so it matches the
execute(query, ...) signature and the project's convention. Locate the
engine.execute(...) invocation around the result = await engine.execute(
model_name, and replace the positional model_name argument with
query=model_name, leaving other arguments unchanged to preserve call semantics.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b767181d-0102-48d1-82bd-749c1d0ad3ba

📥 Commits

Reviewing files that changed from the base of the PR and between 20bb323 and 6934fde.

📒 Files selected for processing (4)
  • docs/interfaces/mcp.md
  • docs/reference/mcp.md
  • slayer/mcp/server.py
  • tests/test_mcp_server.py

…xecute

Use keyword argument on the run-by-name shortcut's engine.execute call to
match every other call site in this file and the user's global rule for
multi-parameter functions.

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

sonarqubecloud Bot commented May 8, 2026

@ZmeiGorynych ZmeiGorynych merged commit 598aced into main May 9, 2026
4 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.

1 participant