codeant-ai-for-open-source[bot] commented on code in PR #40343:
URL: https://github.com/apache/superset/pull/40343#discussion_r3311922449
##########
superset/mcp_service/system/tool/get_schema.py:
##########
@@ -144,6 +152,42 @@ def _get_database_schema_core() ->
ModelGetSchemaCore[ModelSchemaInfo]:
)
+def _get_css_template_schema_core() -> ModelGetSchemaCore[ModelSchemaInfo]:
+ """Create CSS template schema core with dynamically extracted columns."""
+ from superset.daos.css import CssTemplateDAO
+
+ return ModelGetSchemaCore(
+ model_type="css_template",
+ dao_class=CssTemplateDAO,
+ output_schema=ModelSchemaInfo,
+ select_columns=get_css_template_columns(),
+ sortable_columns=CSS_TEMPLATE_SORTABLE_COLUMNS,
+ default_columns=CSS_TEMPLATE_DEFAULT_COLUMNS,
+ search_columns=CSS_TEMPLATE_SEARCH_COLUMNS,
+ default_sort="changed_on",
+ default_sort_direction="desc",
+ logger=logger,
+ )
Review Comment:
**Suggestion:** The new CSS-template schema core exposes `filter_columns`
from `CssTemplateDAO` without constraining them to what `list_css_templates`
actually accepts, so `get_schema(model_type="css_template")` will advertise
filters (like `id`, `uuid`, timestamps, etc.) that `ListCssTemplatesRequest`
rejects at validation time. Restrict the schema-discovery filters to the
supported set (or widen the list request filter schema) so discovery metadata
matches the callable API contract. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ get_schema(css_template) advertises unusable filter columns.
- ⚠️ list_css_templates rejects schema-suggested filters via validation.
- ⚠️ LLM agents see inconsistent contract between discovery and listing.
- ⚠️ Clients must special-case css_template filters despite schema API.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the unified schema tool `get_schema` with
`model_type="css_template"` from an MCP
client; this hits `get_schema()` in
`superset/mcp_service/system/tool/get_schema.py:214-236`, which looks up
`_SCHEMA_CORE_FACTORIES["css_template"]` at `get_schema.py:191-202` and
instantiates
`_get_css_template_schema_core()` at `get_schema.py:155-170`.
2. `_get_css_template_schema_core()` constructs `ModelGetSchemaCore` with
`dao_class=CssTemplateDAO` and no `exclude_filter_columns` (lines 155-170),
so
`ModelGetSchemaCore._get_filter_columns()` in
`superset/mcp_service/mcp_core.py:75-102`
calls `CssTemplateDAO.get_filterable_columns_and_operators()`, which is
inherited from
`BaseDAO.get_filterable_columns_and_operators()` in
`superset/daos/base.py:11-68` and
returns filters for all model columns (including `id`, `uuid`, `changed_on`,
`created_on`,
etc.).
3. The resulting `GetSchemaResponse.schema_info.filter_columns` for
`model_type="css_template"` therefore advertises a mapping that includes
keys like `"id"`
and `"uuid"` as valid filter columns, even though the list tool only
supports filtering on
`template_name`.
4. When an MCP client then uses this schema metadata to construct a
`list_css_templates`
request, it must send filters as `ListCssTemplatesRequest.filters` (defined
in
`superset/mcp_service/css_template/schemas.py:139-149`) where each item is a
`CssTemplateFilter` (`schemas.py:47-65`) whose `col` field is constrained to
`Literal["template_name"]`; any filter using `col="id"` or `col="uuid"` from
the
advertised schema will fail Pydantic validation for `CssTemplateFilter`
before
`list_css_templates()` in
`superset/mcp_service/css_template/tool/list_css_templates.py:56-127` can
execute,
creating a concrete mismatch between `get_schema(model_type="css_template")`
and the
actual list API.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=74a53e8b16364a66bdde3db51762abcb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=74a53e8b16364a66bdde3db51762abcb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/system/tool/get_schema.py
**Line:** 155:170
**Comment:**
*Api Mismatch: The new CSS-template schema core exposes
`filter_columns` from `CssTemplateDAO` without constraining them to what
`list_css_templates` actually accepts, so
`get_schema(model_type="css_template")` will advertise filters (like `id`,
`uuid`, timestamps, etc.) that `ListCssTemplatesRequest` rejects at validation
time. Restrict the schema-discovery filters to the supported set (or widen the
list request filter schema) so discovery metadata matches the callable API
contract.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40343&comment_hash=24775c4a6a90b877a838f31157a321d5aaa9a063228c696ac0de6b9972eee6ea&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40343&comment_hash=24775c4a6a90b877a838f31157a321d5aaa9a063228c696ac0de6b9972eee6ea&reaction=dislike'>👎</a>
##########
superset/mcp_service/system/tool/get_schema.py:
##########
@@ -144,6 +152,42 @@ def _get_database_schema_core() ->
ModelGetSchemaCore[ModelSchemaInfo]:
)
+def _get_css_template_schema_core() -> ModelGetSchemaCore[ModelSchemaInfo]:
+ """Create CSS template schema core with dynamically extracted columns."""
+ from superset.daos.css import CssTemplateDAO
+
+ return ModelGetSchemaCore(
+ model_type="css_template",
+ dao_class=CssTemplateDAO,
+ output_schema=ModelSchemaInfo,
+ select_columns=get_css_template_columns(),
+ sortable_columns=CSS_TEMPLATE_SORTABLE_COLUMNS,
+ default_columns=CSS_TEMPLATE_DEFAULT_COLUMNS,
+ search_columns=CSS_TEMPLATE_SEARCH_COLUMNS,
+ default_sort="changed_on",
+ default_sort_direction="desc",
+ logger=logger,
+ )
+
+
+def _get_theme_schema_core() -> ModelGetSchemaCore[ModelSchemaInfo]:
+ """Create theme schema core with dynamically extracted columns."""
+ from superset.daos.theme import ThemeDAO
+
+ return ModelGetSchemaCore(
+ model_type="theme",
+ dao_class=ThemeDAO,
+ output_schema=ModelSchemaInfo,
+ select_columns=get_theme_columns(),
+ sortable_columns=THEME_SORTABLE_COLUMNS,
+ default_columns=THEME_DEFAULT_COLUMNS,
+ search_columns=THEME_SEARCH_COLUMNS,
+ default_sort="changed_on",
+ default_sort_direction="desc",
+ logger=logger,
+ )
Review Comment:
**Suggestion:** The new theme schema core also returns unrestricted DAO
filter metadata, which can include many columns not allowed by `ThemeFilter` in
`list_themes`; this creates a broken contract where
`get_schema(model_type="theme")` suggests valid filters that immediately fail
request validation. Make schema discovery and list filter validation consistent
by either limiting exposed filter columns here or expanding the accepted filter
columns in the theme request schema. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ get_schema(theme) lists filter columns list_themes disallows.
- ⚠️ list_themes rejects schema-guided filters at request validation.
- ⚠️ Agents relying on schema cannot reliably construct theme filters.
- ⚠️ Extra client-side logic needed to reconcile schema with list API.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the unified schema tool `get_schema` with `model_type="theme"` from
an MCP client;
this executes `get_schema()` in
`superset/mcp_service/system/tool/get_schema.py:214-236`,
which uses `_SCHEMA_CORE_FACTORIES["theme"]` at `get_schema.py:191-202` to
create a core
via `_get_theme_schema_core()` at `get_schema.py:173-188`.
2. `_get_theme_schema_core()` builds a `ModelGetSchemaCore` with
`dao_class=ThemeDAO` and
no `exclude_filter_columns` (lines 173-188), so
`ModelGetSchemaCore._get_filter_columns()`
in `superset/mcp_service/mcp_core.py:75-102` calls
`ThemeDAO.get_filterable_columns_and_operators()`, inherited from
`BaseDAO.get_filterable_columns_and_operators()` in
`superset/daos/base.py:11-68`, which
exposes all SQL columns (e.g. `id`, `uuid`, `changed_on`, `created_on`,
etc.) as
filterable.
3. The returned `GetSchemaResponse.schema_info.filter_columns` for
`model_type="theme"`
therefore lists many filterable column names beyond `theme_name`, even
though the list
tool request schema only allows filters on `theme_name`.
4. An MCP client that follows this schema and sends a `list_themes` request
with a filter
like `{"col": "id", "opr": "eq", "value": 1}` will fail validation in
`ListThemesRequest.filters`
(`superset/mcp_service/theme/schemas.py:146-156`), because
each item must be a `ThemeFilter` (`schemas.py:47-65`) whose `col` is
constrained to
`Literal["theme_name"]`; this prevents `list_themes()` in
`superset/mcp_service/theme/tool/list_themes.py:56-125` from running and
creates a broken
contract between `get_schema(model_type="theme")` and the theme listing API.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a4b85ba42636484697bc2e909aedecfd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a4b85ba42636484697bc2e909aedecfd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/system/tool/get_schema.py
**Line:** 173:188
**Comment:**
*Api Mismatch: The new theme schema core also returns unrestricted DAO
filter metadata, which can include many columns not allowed by `ThemeFilter` in
`list_themes`; this creates a broken contract where
`get_schema(model_type="theme")` suggests valid filters that immediately fail
request validation. Make schema discovery and list filter validation consistent
by either limiting exposed filter columns here or expanding the accepted filter
columns in the theme request schema.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40343&comment_hash=125162e5b94624571dabf92df4ed7b0b0ee05e090e3dae55f3a29227a873504b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40343&comment_hash=125162e5b94624571dabf92df4ed7b0b0ee05e090e3dae55f3a29227a873504b&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
