Skip to content

refactor(tools): default structured_output to True, require title#774

Merged
DevonFulcher merged 3 commits into
mainfrom
refactor/tool-definitions-defaults
May 13, 2026
Merged

refactor(tools): default structured_output to True, require title#774
DevonFulcher merged 3 commits into
mainfrom
refactor/tool-definitions-defaults

Conversation

@DevonFulcher
Copy link
Copy Markdown
Collaborator

Summary

  • GenericToolDefinition.structured_output now defaults to bool = True (was bool | None = False), so new tools get structured output without explicit opt-in.
  • generic_dbt_mcp_tool now requires title and accepts only keyword arguments, keeping call sites explicit.

Test plan

  • task test:unit — 692 passed
  • task check — lint, ruff, mypy clean

🤖 Generated with AI assistance

Copilot AI review requested due to automatic review settings May 13, 2026 13:57
@DevonFulcher DevonFulcher requested review from a team, b-per, jairus-m and jasnonaz as code owners May 13, 2026 13:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the tool-definition API so tools default to structured output and the generic_dbt_mcp_tool decorator is more explicit by requiring a title and enforcing keyword-only usage.

Changes:

  • Default GenericToolDefinition.structured_output to True (and make it non-optional).
  • Make generic_dbt_mcp_tool keyword-only and require title.
  • Add an “Under the Hood” changelog entry describing the refactor.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/dbt_mcp/tools/definitions.py Changes defaults for structured_output and tightens the decorator signature (title required, keyword-only).
.changes/unreleased/Under the Hood-20260513-085632.yaml Adds a changelog note for the tooling refactor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/dbt_mcp/tools/definitions.py
Copilot AI review requested due to automatic review settings May 13, 2026 14:26
@DevonFulcher DevonFulcher force-pushed the refactor/tool-definitions-defaults branch from 1e76b38 to 24cdf49 Compare May 13, 2026 14:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Flip `GenericToolDefinition.structured_output` from `bool | None = False`
to `bool = True` so newly-added tools don't have to opt in. Make `title`
required in `generic_dbt_mcp_tool` and force keyword-only args so call
sites stay explicit.

Co-Authored-By: AI Assistant <noreply@ai>
@DevonFulcher DevonFulcher force-pushed the refactor/tool-definitions-defaults branch from 24cdf49 to 85352b6 Compare May 13, 2026 14:31
Copy link
Copy Markdown
Collaborator

@jairus-m jairus-m left a comment

Choose a reason for hiding this comment

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

Looks great! With one small non-blocking comment.

Comment thread src/dbt_mcp/tools/definitions.py Outdated
Copilot AI review requested due to automatic review settings May 13, 2026 22:04
@DevonFulcher DevonFulcher enabled auto-merge (squash) May 13, 2026 22:04
@DevonFulcher DevonFulcher merged commit 9b833ab into main May 13, 2026
5 of 6 checks passed
@DevonFulcher DevonFulcher deleted the refactor/tool-definitions-defaults branch May 13, 2026 22:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment on lines 432 to 548
@@ -439,6 +440,7 @@ def get_node_details_dev(
),
ToolDefinition(
fn=compile,
title="dbt compile",
description=get_prompt("dbt_cli/compile"),
annotations=create_tool_annotations(
title="dbt compile",
@@ -449,6 +451,7 @@ def get_node_details_dev(
),
ToolDefinition(
fn=docs,
title="dbt docs",
description=get_prompt("dbt_cli/docs"),
annotations=create_tool_annotations(
title="dbt docs",
@@ -460,6 +463,7 @@ def get_node_details_dev(
ToolDefinition(
name="list",
fn=ls,
title="dbt list",
description=get_prompt("dbt_cli/list"),
annotations=create_tool_annotations(
title="dbt list",
@@ -470,6 +474,7 @@ def get_node_details_dev(
),
ToolDefinition(
fn=parse,
title="dbt parse",
description=get_prompt("dbt_cli/parse"),
annotations=create_tool_annotations(
title="dbt parse",
@@ -480,6 +485,7 @@ def get_node_details_dev(
),
ToolDefinition(
fn=run,
title="dbt run",
description=get_prompt("dbt_cli/run"),
annotations=create_tool_annotations(
title="dbt run",
@@ -490,6 +496,7 @@ def get_node_details_dev(
),
ToolDefinition(
fn=test,
title="dbt test",
description=get_prompt("dbt_cli/test"),
annotations=create_tool_annotations(
title="dbt test",
@@ -500,6 +507,7 @@ def get_node_details_dev(
),
ToolDefinition(
fn=show,
title="dbt show",
description=get_prompt("dbt_cli/show"),
annotations=create_tool_annotations(
title="dbt show",
@@ -510,6 +518,7 @@ def get_node_details_dev(
),
ToolDefinition(
fn=clone,
title="dbt clone",
description=get_prompt("dbt_cli/clone"),
annotations=create_tool_annotations(
title="dbt clone",
@@ -521,6 +530,7 @@ def get_node_details_dev(
ToolDefinition(
name="get_lineage_dev",
fn=get_lineage_dev,
title="Get Model Lineage (Dev)",
description=get_prompt("dbt_cli/get_lineage_dev"),
annotations=create_tool_annotations(
title="Get Model Lineage (Dev)",
@@ -532,6 +542,7 @@ def get_node_details_dev(
ToolDefinition(
name="get_node_details_dev",
fn=get_node_details_dev,
title="Get Node Details (Dev)",
description=get_prompt("dbt_cli/get_node_details_dev"),
annotations=create_tool_annotations(
title="Get Node Details (Dev)",
jairus-m added a commit that referenced this pull request May 14, 2026
…st (#778)

## Summary
Failure surfaced in #777 's integration test -
https://github.com/dbt-labs/dbt-mcp/actions/runs/25841708919/job/75928628173
- tho unrelated to PR.

4 failures total from that above integration test:
- 3 due to staging env error (unrelated to code and resolved after
retry)
- 1 due to need for unpacking tuple result after #774 (this PR fixes
this one only)
jairus-m added a commit that referenced this pull request May 19, 2026
…t no structured output returned' (#781)

## Summary

When a tool raised an exception, `DbtMCP.call_tool()` caught it and
returned `[TextContent(...)]` which is unstructured. After #774 changed
`structured_output` to default to `True`, all tools gained an
`outputSchema`. Because of this, the MCP server rejects the unstructured
error response with "Output validation error: outputSchema defined but
no structured output returned". The actual error is not returned and
hidden to the user/LLM:

<img width="703" height="71" alt="Screenshot 2026-05-15 at 8 46 38 AM"
src="https://github.com/user-attachments/assets/8dc8e1c8-175e-41b7-89d9-d4a0b64f0f6e"
/>

To address this, ~~we instead return `CallToolResult(isError=True)` on
exception so the MCP server "short circuits"~~ raise an error which gets
caught by FastMCP's handler _before_ the `outputSchema` check which then
properly outputs the failure:

<img width="1229" height="73" alt="Screenshot 2026-05-15 at 8 46 16 AM"
src="https://github.com/user-attachments/assets/42a3a231-641a-4cb5-99ab-146073f1f92f"
/>

## Checklist
- [ ] I have performed a self-review of my code
- [ ] I have made corresponding changes to the documentation (in
https://github.com/dbt-labs/docs.getdbt.com) if required -- Mention it
here
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes

## Additional Notes
<!-- Any additional information that would be helpful for reviewers -->
~~One thing Im unsure about about this implementation here is that we
have to override the return type established by the parent
`FastMCP.call_tool` method and add `CallToolResult` to
`DbtMCP.call_tool`'s annotations. Because of this, mypy gets a bit mad
and we have to tell it to "shhh". No behavioral changes with regards to
the [LSP](https://en.wikipedia.org/wiki/Liskov_substitution_principle),
just type annotations! Hope this is fine.~~

^^ addressed in comments
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.

3 participants