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

   ## Summary
   
   This PR fixes identifier ambiguity and validation issues in MCP tools that 
were causing LLMs to make incorrect API calls.
   
   ### Issues Fixed
   
   1. **Identifier Ambiguity**: LLMs were sometimes using 
`'public.datasetName'` format instead of numeric IDs or UUIDs for tools like 
`get_dataset_info`, `get_chart_info`, and `update_chart`
   
   2. **chartType Validation Errors**: When generating XY charts, LLMs would 
get `ValidationError: Unable to extract tag using discriminator 'chart_type'` 
because `chart_type` was marked as optional (had a default value), so LLMs 
would omit it
   
   ### Changes Made
   
   1. **Made `chart_type` required** in `XYChartConfig` and `TableChartConfig` 
schemas
      - Changed from `Field("xy", description="...")` to `Field(..., 
description="REQUIRED: must be 'xy'")`
      - Changed from `Field("table", description="...")` to `Field(..., 
description="REQUIRED: must be 'table'")`
      - This forces LLMs to explicitly specify the chart type, preventing 
discriminator errors
   
   2. **Added JSON examples to tool docstrings** for better LLM guidance:
      - `get_dataset_info`: Shows correct ID/UUID usage with examples
      - `get_chart_info`: Shows correct ID/UUID usage with examples
      - `update_chart`: Shows complete request structure including required 
`chart_type`
      - `generate_chart`: Shows both XY chart and Table chart examples
      - `generate_explore_link`: Shows chart_type requirement
   
   3. **Clarified identifier parameter usage** across all affected tools:
      - Explicitly state to use numeric ID or UUID string
      - Warn against using `schema.table_name` format (e.g., 
`"public.customers"`)
      - Direct users to `list_datasets`/`list_charts` tools to find correct IDs
   
   ### Impact
   
   These changes help LLM clients understand:
   - `chart_type` is always required (not optional)
   - Identifiers must be numeric IDs or UUIDs (not names or schema.table 
patterns)
   - Proper request structure through concrete JSON examples
   
   This resolves the following LLM behavior issues:
   - Omitting `chart_type`, causing discriminator validation errors
   - Using `"public.datasetName"` instead of numeric IDs
   - Struggling with parameter format without concrete examples
   
   ## Testing Instructions
   
   1. Test with an LLM client (e.g., Claude Desktop with MCP):
      - Ask to create an XY chart - verify LLM includes `chart_type: "xy"`
      - Ask to create a Table chart - verify LLM includes `chart_type: "table"`
      - Ask to get dataset info - verify LLM uses numeric ID or UUID (not 
schema.table format)
      - Verify JSON examples appear in tool schema introspection
   
   2. Verify existing functionality:
      - MCP tools should work as before
      - Schema validation should properly reject missing `chart_type`
      - Identifier lookup should work with both numeric IDs and UUIDs
   
   ## Additional Information
   
   - No breaking changes to API contracts
   - Only improves LLM client usability through better documentation and 
stricter validation
   - Aligns with FastMCP best practices for tool documentation


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