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

   ### SUMMARY
   
   `call_tool → list_metrics` with `{}` — the natural first call an LLM makes — 
returned `Error: Response too large: ~52,527 tokens (limit: 25,000)` on a 
workspace with a single 24-metric semantic view, so the client had to retry 
with a smaller page (observed live as well). `ListMetricsRequest` defaulted to 
`include_compatible_dimensions=True` and `page_size=50`, and every external 
metric embedded its full compatible-dimension list as `DimensionInfo` objects 
(~50 tokens each).
   
   Two fixes on the same code path:
   
   1. **Grain-variant duplicates (sc-120963).** 
`SemanticView.get_compatible_dimensions` returned one name per `Dimension` 
object, and a semantic view exposes one object per (name, grain) for time 
dimensions — so `get_compatible_dimensions` listed every time dimension once 
per grain (51 entries for 21 distinct names on the Jaffle Shop view), and 
`list_metrics`, which calls it per metric, inflated its embedded lists ~2.4×. 
It returns each name once, sorted (the implementation's sets are unordered, so 
the previous order varied run to run). `_unique_dimensions` already collapsed 
grain variants for `columns`; this brings the compatibility path in line.
   2. **Defaults and a cap (sc-120960).** `include_compatible_dimensions` 
defaults to `False` and `page_size` to 25; when dimensions are embedded, 
`page_size` is capped at 8 with a validation error that states the cap and 
points at `get_compatible_dimensions`. The cap was measured with the response 
guard's own estimator (`estimate_response_tokens`): 20 metrics × 40 dimensions 
is 42,336 tokens even after the dedupe; 10 × 40 is 21,186; 8 × 40 is 16,956 
(18,284 with the fallback estimator) against the 25,000 limit. No silent 
clamping.
   
   The `list_metrics` docstring workflow now reads list_metrics → 
get_compatible_dimensions → get_table.
   
   **Verified on a live `superset mcp run` against the Jaffle Shop view** (same 
fastmcp `call_tool` shape the chatbot uses):
   
   | call | before | after |
   |---|---|---|
   | `list_metrics {}` | Response too large (~52.5k tokens) | 23 metrics, 7,281 
chars (~1.8k tokens), `page_size` 25, no embedded dims |
   | `list_metrics {view_id, page_size: 8, include_compatible_dimensions: 
true}` | — | 8 metrics, ~5.5k tokens, per-metric dimension names unique |
   | same with `page_size: 9` | — | validation error naming the cap and 
`get_compatible_dimensions` |
   | `get_compatible_dimensions {view_id: 1, selected_metrics: ["revenue"]}` | 
51 entries / 21 distinct | 21 entries / 21 distinct, sorted |
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A (MCP tool responses; table above).
   
   ### TESTING INSTRUCTIONS
   
   ```
   pytest tests/unit_tests/mcp_service/semantic_layer/ 
tests/unit_tests/semantic_layers/models_test.py
   ```
   New tests: the default request on a synthetic 60-metric × 40-dimension × 
6-grain view stays under half the token limit by the guard's estimator; a 
dimensions-on page of 8 stays under the limit with 40 unique names per metric; 
page 9 with dimensions is rejected; three same-named grain variants collapse to 
one deterministic name. Red-first controls were run: with the old defaults the 
default request exceeds the limit; without the dedupe the per-metric list is 
not 40.
   
   ### ADDITIONAL INFORMATION
   
   - [x] Has associated issue: sc-120960, sc-120963
   - [ ] 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
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   
   https://claude.ai/code/session_01267VBWbvWTNZUg9GvXKgkC
   


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