aminghadersohi opened a new pull request, #42655:
URL: https://github.com/apache/superset/pull/42655

   ### SUMMARY
   
   Adds safe patch semantics for updating existing table charts through MCP and 
fixes stale-filter handling during previews.
   
   - Adds `add_columns` to `update_chart`, allowing callers to append table 
dimensions or metrics without reconstructing or replacing the saved column list.
   - Honors an explicit `filters: []` by removing saved ad hoc filters from 
preview form data.
   - Treats Superset's inert `No filter` temporal placeholder as non-filtering 
during dataset-column validation.
   - Clarifies validation guidance so it describes the accepted filter-clearing 
request exactly.
   - Adds regression coverage for additive updates, parenthesized column names, 
explicit filter clearing, and inert stale filters.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Not applicable. This changes the MCP API and validation behavior without UI 
changes.
   
   ### TESTING INSTRUCTIONS
   
   ```bash
   ruff check superset/mcp_service/chart/schemas.py 
superset/mcp_service/chart/tool/update_chart.py 
superset/mcp_service/chart/compile.py 
tests/unit_tests/mcp_service/chart/tool/test_update_chart.py 
tests/unit_tests/mcp_service/chart/test_compile.py
   python -m pytest -q 
tests/unit_tests/mcp_service/chart/tool/test_update_chart.py 
tests/unit_tests/mcp_service/chart/test_compile.py
   pre-commit run
   ```
   
   The focused MCP suite passes 82 tests. The staged-file pre-commit gate 
passes. The all-files gate was also run; its failures are pre-existing 
repository-wide mypy errors and missing frontend dependencies, unrelated to 
these Python-only changes.
   
   **Eval evidence:** Four focused regression scenarios are included in the 82 
passing tests: additive metric preservation, parenthesized column names, 
explicit empty-filter replacement, and inert stale-filter validation.
   
   **Cost & latency delta:** Adds 0 LLM calls and 0 external requests. The 
update path adds only in-memory list/dictionary merging and a linear scan of 
saved filters; no runtime benchmark was needed for this local operation.
   
   ### ADDITIONAL INFORMATION
   
   **Blast radius:** Limited to MCP `update_chart` table updates and MCP chart 
filter validation. Existing full-config replacement behavior remains available.
   
   **Risk and rollback:** Low. The new request field is additive, and reverting 
this commit restores the prior behavior. Non-table charts reject `add_columns` 
with an actionable validation error.
   
   **Review guidance:** Focus on preservation of existing 
`groupby`/`metrics`/`all_columns`, and the distinction between omitted filters 
(preserve) and explicit `filters: []` (clear).
   
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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]

Reply via email to