Skip to content

feat(mcp): add list and get tools for alerts and reports#40299

Closed
aminghadersohi wants to merge 6 commits into
masterfrom
mcp-reports-99978
Closed

feat(mcp): add list and get tools for alerts and reports#40299
aminghadersohi wants to merge 6 commits into
masterfrom
mcp-reports-99978

Conversation

@aminghadersohi
Copy link
Copy Markdown
Contributor

SUMMARY

Adds list_reports and get_report_info MCP tools for the Alerts & Reports domain, following the canonical database domain pattern.

New files:

  • superset/mcp_service/report/schemas.pyReportFilter, ReportInfo, ReportList, ListReportsRequest, GetReportInfoRequest, ReportError, serialize_report_object()
  • superset/mcp_service/report/tool/list_reports.py — lists alerts/reports with filtering, search, pagination
  • superset/mcp_service/report/tool/get_report_info.py — fetches a single schedule by numeric ID

Key design decisions:

  • ReportFilter restricts filterable columns to: name, type, active, dashboard_id, chart_id
  • GetReportInfoRequest.identifier is int only (ReportSchedule has no UUID or slug)
  • owners field is defined in ReportInfo but stripped by filter_user_directory_fields (same privacy pattern as other list tools)
  • No data_model_metadata privacy gate (reports are user-level content, not database metadata)
  • Tools registered in app.py and advertised in DEFAULT_INSTRUCTIONS

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — backend-only MCP tools

TESTING INSTRUCTIONS

# Run unit tests for the new domain
pytest tests/unit_tests/mcp_service/report/ -v

# Verify tools register and are callable via MCP client
# list_reports: call with empty request → returns paginated report list
# get_report_info: call with {"identifier": <id>} → returns schedule metadata

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration
  • Introduces new feature or API
  • Removes existing feature or API

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 59.50000% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.82%. Comparing base (73f66e4) to head (2305683).
⚠️ Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/report/tool/list_reports.py 30.00% 42 Missing ⚠️
superset/mcp_service/report/schemas.py 79.38% 19 Missing and 1 partial ⚠️
...uperset/mcp_service/report/tool/get_report_info.py 43.47% 13 Missing ⚠️
superset/mcp_service/common/schema_discovery.py 75.00% 2 Missing ⚠️
superset/mcp_service/mcp_core.py 33.33% 2 Missing ⚠️
superset/mcp_service/system/tool/get_schema.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #40299      +/-   ##
==========================================
- Coverage   64.14%   63.82%   -0.33%     
==========================================
  Files        2592     2596       +4     
  Lines      138846   140126    +1280     
  Branches    32201    32497     +296     
==========================================
+ Hits        89069    89439     +370     
- Misses      48245    49123     +878     
- Partials     1532     1564      +32     
Flag Coverage Δ
hive 39.11% <59.50%> (-0.22%) ⬇️
mysql 58.26% <59.50%> (-0.61%) ⬇️
postgres 58.33% <59.50%> (-0.61%) ⬇️
presto 40.75% <59.50%> (-0.25%) ⬇️
python 59.86% <59.50%> (-0.64%) ⬇️
sqlite 57.98% <59.50%> (-0.60%) ⬇️
unit 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

@aminghadersohi aminghadersohi force-pushed the mcp-reports-99978 branch 2 times, most recently from c126d7f to 942c608 Compare May 21, 2026 15:10
aminghadersohi and others added 5 commits May 21, 2026 19:10
Adds list_reports and get_report_info MCP tools under a new
superset/mcp_service/report/ domain, following the canonical database
domain pattern. Includes unit tests and app.py registration.
- Register report model type in get_schema (Fix #1): add _get_report_schema_core
  factory + "report" entry in _SCHEMA_CORE_FACTORIES; ModelType now includes "report"
- Add OwnedByMeMixin/CreatedByMeMixin to ListReportsRequest (Fix #2)
- DRY up list_reports.py column constants (Fix #3): import REPORT_* constants and
  get_report_columns from schema_discovery; pass created_by_me/owned_by_me to run_tool
- Extend test coverage (Fix #6): humanized timestamp fields, invalid order_column
  guard, owned_by_me/created_by_me DAO filter injection

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tools

- Add ReportListCore subclass in list_reports.py that overrides filter
  injection to use owners.id (instead of generic owner) and calls the
  DAO with filters= kwarg (instead of column_operators=) so tests can
  assert on the kwarg by name
- Extract _call_dao_list hook in ModelListCore so subclasses can change
  the DAO kwarg name without duplicating run_tool
- Add owners.id to SELF_REFERENCING_FILTER_COLUMNS so it is excluded
  from filters_applied in responses

Fixes: test_list_reports_owned_by_me_passed_to_dao,
       test_list_reports_created_by_me_passed_to_dao
created_by_fk was removed from SELF_REFERENCING_FILTER_COLUMNS so it
now appears in filters_applied, but ReportFilter.col Literal didn't
include it, causing pydantic validation error in list_reports responses.
…e ColumnOperator for filters_applied

- ReportFilter.col Literal no longer includes created_by_fk; callers
  must use the created_by_me flag instead (internal-only filter)
- ReportList.filters_applied typed as List[ColumnOperator] so that
  internally-injected ColumnOperator instances (e.g. created_by_fk from
  created_by_me=True) pass pydantic validation without coercion errors
@aminghadersohi aminghadersohi marked this pull request as ready for review May 22, 2026 02:43
@dosubot dosubot Bot added the alert-reports Namespace | Anything related to the Alert & Reports feature label May 22, 2026
@aminghadersohi aminghadersohi requested a review from eschutho May 22, 2026 02:52
@aminghadersohi
Copy link
Copy Markdown
Contributor Author

Closing in favour of #40348 (branch renamed to amin/mcp-list-reports per naming convention)

Copy link
Copy Markdown
Contributor

@bito-code-review bito-code-review Bot left a comment

Choose a reason for hiding this comment

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

Code Review Agent Run #840a5b

Actionable Suggestions - 1
  • superset/mcp_service/report/schemas.py - 1
Additional Suggestions - 2
  • superset/mcp_service/report/tool/get_report_info.py - 1
    • Missing resource access validation · Line 72-85
      Unlike get_chart_info which validates dataset accessibility (lines 327-330), this tool doesn't verify the report's associated resources are accessible. This could leak information about reports linked to inaccessible dashboards or charts.
  • superset/mcp_service/common/schema_discovery.py - 1
    • Dead code: unused constant · Line 679-679
      Remove the unused REPORT_ALL_COLUMNS constant from schema_discovery.py.
Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

  • superset/mcp_service/report/tool/list_reports.py - 1
  • superset/mcp_service/report/tool/get_report_info.py - 2
  • superset/mcp_service/common/schema_discovery.py - 1
  • superset/mcp_service/report/schemas.py - 1
Review Details
  • Files reviewed - 14 · Commit Range: 2e87c2c..2305683
    • superset/mcp_service/app.py
    • superset/mcp_service/common/schema_discovery.py
    • superset/mcp_service/constants.py
    • superset/mcp_service/mcp_core.py
    • superset/mcp_service/privacy.py
    • superset/mcp_service/report/__init__.py
    • superset/mcp_service/report/schemas.py
    • superset/mcp_service/report/tool/__init__.py
    • superset/mcp_service/report/tool/get_report_info.py
    • superset/mcp_service/report/tool/list_reports.py
    • superset/mcp_service/system/tool/get_schema.py
    • tests/unit_tests/mcp_service/report/__init__.py
    • tests/unit_tests/mcp_service/report/tool/__init__.py
    • tests/unit_tests/mcp_service/report/tool/test_report_tools.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

"""Convert a datetime to a humanized string like '2 hours ago'."""
if dt is None:
return None
now = datetime.now(dt.tzinfo) if dt.tzinfo else datetime.now()
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.

datetime.now() called without timezone

Add timezone argument to datetime.now() call on line 268. Use datetime.now(dt.tzinfo) or datetime.now(timezone.utc) for consistency.

Code suggestion
Check the AI-generated fix before applying
Suggested change
now = datetime.now(dt.tzinfo) if dt.tzinfo else datetime.now()
now = datetime.now(dt.tzinfo) if dt.tzinfo else datetime.now(timezone.utc)

Code Review Run #840a5b


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

alert-reports Namespace | Anything related to the Alert & Reports feature size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant