codeant-ai-for-open-source[bot] commented on code in PR #40773:
URL: https://github.com/apache/superset/pull/40773#discussion_r3368064156
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -200,31 +208,44 @@ def generate_explore_link(dataset_id: int | str,
form_data: Dict[str, Any]) -> s
"datasource": f"{numeric_dataset_id}__table",
}
- # Try to create form_data in cache using MCP-specific
CreateFormDataCommand
+ # Try durable permalink first (DB-backed key-value store, does not
expire).
+ # CreateExplorePermalinkCommand wraps its internal failures
(encode/create/
+ # SQLAlchemy errors) into ExplorePermalinkCreateFailedError, so catch
only
+ # those expected modes here — letting programming errors (TypeError,
etc.)
+ # surface instead of being silently masked by the form_data_key
fallback.
+ try:
+ state = {"formData": form_data_with_datasource}
+ permalink_key = CreateExplorePermalinkCommand(state=state).run()
+ return f"{base_url}/explore/p/{permalink_key}/"
Review Comment:
**Suggestion:** Returning a permalink URL by default breaks existing callers
that still rely on a `form_data_key` URL contract. `update_chart_preview`
immediately parses the generated URL with `extract_form_data_key_from_url` and
now fails with `"missing form_data_key"` for the normal success path, and
`generate_chart` preview mode similarly loses its unsaved-state key. Keep
permalink generation for the explore tool path, but preserve/optionally force
`form_data_key` generation for shared callers that require it (or update those
callers to consume permalink keys end-to-end before changing this shared helper
default). [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ update_chart_preview tool returns PreviewError for valid preview updates.
- ⚠️ generate_chart preview responses no longer expose form_data_key.
- ⚠️ LLM workflows cannot reuse cached preview state keys.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Invoke the MCP `generate_chart` tool in preview mode by calling
`generate_chart(request={... save_chart: false ...})` as documented in
`superset/mcp_service/app.py:102-114` and implemented in
`superset/mcp_service/chart/tool/generate_chart.py:81-161`.
2. In the preview-only branch of `generate_chart` at
`superset/mcp_service/chart/tool/generate_chart.py:527-535`, observe that it
imports
`generate_explore_link` and calls `explore_url =
generate_explore_link(request.dataset_id,
form_data)` (line 530), then immediately calls `form_data_key =
extract_form_data_key_from_url(explore_url)` (lines 533-534) to populate the
response's
`form_data_key`.
3. The shared helper `generate_explore_link` used above is implemented in
`superset/mcp_service/chart/chart_utils.py:160-116`; on the normal success
path it now
executes `state = {"formData": form_data_with_datasource}` and
`permalink_key =
CreateExplorePermalinkCommand(state=state).run()` at
`chart_utils.py:217-218`, then
returns `f"{base_url}/explore/p/{permalink_key}/"` at `chart_utils.py:219` —
a permalink
URL with **no** `form_data_key` query parameter, so
`extract_form_data_key_from_url` in
`superset/mcp_service/chart/chart_helpers.py:19-28` returns `None`, causing
`generate_chart` preview responses to lose the previously-populated unsaved
preview
`form_data_key`.
4. Independently, call the MCP `update_chart_preview` tool defined at
`superset/mcp_service/chart/tool/update_chart_preview.py:115-33` with a
valid `dataset_id`
and `config`. After validation, the function generates a new explore URL via
`explore_url
= generate_explore_link(request.dataset_id, new_form_data)` at
`update_chart_preview.py:244`, then immediately runs `new_form_data_key =
extract_form_data_key_from_url(explore_url)` at
`update_chart_preview.py:247` and checks
`if not new_form_data_key:` returning an error payload with `error_type:
"PreviewError"`
and message `"Failed to generate preview: missing form_data_key"` at
`update_chart_preview.py:248-156`. Because the shared
`generate_explore_link` now returns
a `/explore/p/<permalink_key>/` URL without a `form_data_key` under normal
conditions,
this error path is taken even when permalink generation succeeds, breaking
the normal
success flow for `update_chart_preview` and any clients that follow the
preview-first
workflow described in `superset/mcp_service/app.py:102-114`.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=add0f73a3ad44952a9bb3555b7e2947f&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=add0f73a3ad44952a9bb3555b7e2947f&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/chart/chart_utils.py
**Line:** 217:219
**Comment:**
*Api Mismatch: Returning a permalink URL by default breaks existing
callers that still rely on a `form_data_key` URL contract.
`update_chart_preview` immediately parses the generated URL with
`extract_form_data_key_from_url` and now fails with `"missing form_data_key"`
for the normal success path, and `generate_chart` preview mode similarly loses
its unsaved-state key. Keep permalink generation for the explore tool path, but
preserve/optionally force `form_data_key` generation for shared callers that
require it (or update those callers to consume permalink keys end-to-end before
changing this shared helper default).
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%2F40773&comment_hash=61206cc347f9156a1074bc49963d03498c2a38dac28ce4fac2af9010bb537652&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40773&comment_hash=61206cc347f9156a1074bc49963d03498c2a38dac28ce4fac2af9010bb537652&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]
