Skip to content

fix(mcp): warn about dbt selector grammar in select param descriptions#1462

Open
wcchang1115 wants to merge 1 commit into
mainfrom
feature/drc-3834-recce-agent-comma-separated-model-list-in-select-is-a-dbt
Open

fix(mcp): warn about dbt selector grammar in select param descriptions#1462
wcchang1115 wants to merge 1 commit into
mainfrom
feature/drc-3834-recce-agent-comma-separated-model-list-in-select-is-a-dbt

Conversation

@wcchang1115

@wcchang1115 wcchang1115 commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

PR checklist

  • Ensure you have added or ran the appropriate tests for your PR.
  • DCO signed

What type of PR is this?

fix

What this PR does / why we need it:

The MCP select string feeds dbt's node-selection grammar, where a comma means intersection. Agents comma-join distinct model names (e.g. a,b), so dbt intersects them → zero nodes → an empty diff indistinguishable from "no changes" → silent under-reporting (seen in production). Since select is passed to dbt verbatim, the fix is in the description contract: warn on every select param that a comma is intersection, a space is union, and bare names are exact.

Which issue(s) this PR fixes:

DRC-3834

Special notes for your reviewer:

  • Grammar verified against dbt docs (Core 1.11): space=union, comma=intersection, bare name=exact, wildcard needs *.

Does this PR introduce a user-facing change?:

NONE

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
recce/mcp_server.py 90.46% <100.00%> (+<0.01%) ⬆️
tests/test_mcp_server.py 99.87% <100.00%> (+<0.01%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wcchang1115 wcchang1115 force-pushed the feature/drc-3834-recce-agent-comma-separated-model-list-in-select-is-a-dbt branch 3 times, most recently from 628cbee to 35d19bc Compare July 10, 2026 11:08
Agents comma-join distinct model names in `select` (e.g. "a,b"), but dbt
reads a comma as intersection, so it silently resolves to an empty diff
that looks like "no changes". Add a shared SELECTOR_SYNTAX_NOTE to all five
select descriptions: space=union, comma=intersection over all selector
arguments, bare name=exact, wildcard needs `*`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Wei-Chun, Chang <wcchang@infuseai.io>
@wcchang1115 wcchang1115 force-pushed the feature/drc-3834-recce-agent-comma-separated-model-list-in-select-is-a-dbt branch from 35d19bc to e6a1d75 Compare July 10, 2026 11:11
@wcchang1115 wcchang1115 requested a review from iamcxa July 10, 2026 11:14
@wcchang1115 wcchang1115 marked this pull request as ready for review July 10, 2026 11:15

@iamcxa iamcxa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed together with the agent-side sibling DataRecce/recce-cloud-infra#1522 (same DRC-3834 fix; the MCP-description ⇄ prompt contract). Grammar guidance is consistent across both.

Verified correct ✅

  • SELECTOR_SYNTAX_NOTE is appended to all 5 tool-schema select params (lines 730 / 765 / 948 / 1023 / 1380) — complete coverage, not a partial rollout.
  • The new test enforces it well: the per-tool "intersection (AND)" in select["description"] assertion means every tool exposing a select param must carry the note (not merely ≥5), so a future tool that adds select without it will fail the test. Good regression guard.
  • The note's grammar matches the agent-side prompt in #1522 (space = union, comma = intersection, bare = exact, * for prefix).

Minor (NIT) 💡

  • test_mcp_server.py L3163: the diff also collapses an unrelated with patch.object(...) from multi-line onto one line (looks like a ruff format sweep caught in the same commit). Harmless — just unrelated to the selector fix.

Approving — clean, well-tested description fix. The NIT is optional.

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.

2 participants