[rebenitez1802]

SUMMARY

Functional fixes for the Canvas draft lifecycle and its MCP/REST contract, targeting msyavuz/feat/canvas.

Draft lifecycle (3 bugs, each reproduced before the fix)

• Canceled new widgets. If a draft added a new widget and then removed it, the staged entity was pruned mid-batch. So the commit failed with 422 "add widgets that cannot be placed" (separate calls, one batch, parent removal, surviving sibling). A re-added widget also lost its stored content. Drafts now keep new widget entities until the final replay (keep_unplaced). The commit then drops new widgets that end up unplaced, so no orphan widget rows are written. Edits to existing widgets are never pruned.
• Explicit-null resets. set_scope.scope: null and set_settings.value: null were removed from the replay log (exclude_none), so replay failed with Field required. The nulls are now logged explicitly. An omitted field is still a validation error, never a reset.
• Empty close revision. Closing an empty draft returned the fork's base_revision, even after another publish. CommitCanvasDraftCommand now returns a typed CommitResult with the canvas revision actually written or observed. Both callers (REST and MCP) are updated.

MCP contract

• apply_canvas_draft_ops.ops and create_canvas.ops advertise the canonical Operation union in their input schemas (BeforeValidator(json_schema_input_type=...)). There is no second, handwritten grammar.
  • The arguments are unchanged: still flat, camelCase.
  • Omitted, null and default values behave as before. create_canvas still accepts ops omitted, null or [].
  • Indexed field-path errors (/ops/1/index) are preserved.
• Results add error_type (ValidationError, DraftUnavailable, DraftRevisionConflict, CanvasRevisionConflict, WidgetRevisionConflict), draft_revision and current_canvas_revision. Legacy keys are unchanged.
• An unavailable or foreign token discloses no revisions.
• Tool descriptions for commit_canvas_draft and apply_canvas_draft_ops now lead with publish intent and conflict recovery. That guidance survives the 300-character tool-search truncation.

REST

• 409 bodies add error_type, draftRevision and currentCanvasRevision next to the existing revision and canvasChanged.
• 404 for an unavailable draft adds error_type: DraftUnavailable.
• Status codes and existing keys are unchanged.

Behavior worth knowing (pinned by tests, documented in mcp-server.mdx): a draft whose operations cancel out, e.g. add a new widget and then remove it, commits as a real write. That is the same result as publishing those operations without a draft. If the canvas changed meanwhile, the commit returns 409 CanvasRevisionConflict. Treating such a draft as an empty close would be a new behavior and is left for a follow-up.

The query translator and chart-data path were already correct. The new real-SQL tests confirm this, and no query code changed.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A (backend and MCP only).

TESTING INSTRUCTIONS

pytest tests/unit_tests/canvas tests/unit_tests/mcp_service/canvas \
  tests/unit_tests/mcp_service/test_tool_search_transform.py \
  tests/unit_tests/mcp_service/test_mcp_server.py \
  tests/unit_tests/mcp_service/test_native_tool_surface.py \
  tests/unit_tests/mcp_service/test_tool_inventory.py

• Focused suite: 750 pass.
• Run against the old code: the new lifecycle tests fail 8 times (5 canceled-widget cases, 1 empty-close revision, 2 null resets).
• Mutation testing: reverting any single production fix turns tests red.
• Wider sweep: tests/unit_tests/mcp_service plus tests/unit_tests/canvas gives 13,665 passed and 0 failed (base: 13,502 passed).
• Checks: OpenAPI regenerated with marshmallow 4.3.1 shows no drift. Pre-commit, MyPy and Pylint (10.00/10) pass on the changed files.

Test additions:

• tests/unit_tests/canvas/widget_entities_test.py, draft_test.py, api_test.py: lifecycle regressions, conflict precedence and provenance, REST payloads.
• tests/unit_tests/canvas/placement_data_test.py: real SQLite aggregate queries (top and bottom 5, unsorted limit, query-text visibility) through an unmocked ChartDataCommand.
• tests/unit_tests/mcp_service/canvas/tool/test_canvas_contract.py (new): schema exposure, and native vs. search_tools/call_tool proxy parity with protected wrappers and RBAC enabled.

native_tool_inventory.json and the apply_canvas_draft_ops/create_canvas budgets in test_tool_inventory.py were regenerated or raised. The inlined operation grammar grows each entry to about 5.3 KB. Only Canvas entries and catalog totals changed.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags: CANVAS
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • 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

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	fef6e4a
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6ae8bcc4002034808357a710
😎 Deploy Preview	https://deploy-preview-45826--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codeant-ai-for-open-source]

Suggestion: A concurrent widget edit during a normal MCP apply raises DraftConflictError, which _apply_canvas_ops_impl does not catch, so the tool fails instead of returning the conflict payload.

Assessment: 🟠 Major · 🔁 Occurrence: Rarely · 🏷️ Api mismatch

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset/commands/canvas/apply_ops.py
**Line:** 274:279
**Comment:**
	*Api Mismatch: A concurrent widget edit during a normal MCP apply raises `DraftConflictError`, which `_apply_canvas_ops_impl` does not catch, so the tool fails instead of returning the conflict payload.

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

👍 | 👎

[codeant-ai-for-open-source]

Suggestion: ops runs after the canvas is committed in a separate transaction; if it fails, a concurrent successful edit can be erased by the cleanup delete.

Assessment: 🔴 Critical · 🔁 Occurrence: Rarely · 🏷️ Race condition

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset/mcp_service/canvas/tool/create_canvas.py
**Line:** 95:95
**Comment:**
	*Race Condition: `ops` runs after the canvas is committed in a separate transaction; if it fails, a concurrent successful edit can be erased by the cleanup delete.

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

👍 | 👎

[codeant-ai-for-open-source]

  CodeAnt PR Risk

🔴  High Risk

• The PR should not merge yet: if initial canvas operations fail after a concurrent edit succeeds, cleanup deletes the canvas and erases that edit.
• A concurrent widget edit during a normal MCP apply raises an uncaught conflict, so callers do not receive the documented conflict result.

Assessed commit: fef6e4aba8da

[codecov]

Codecov Report

❌ Patch coverage is 92.18750% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.41%. Comparing base (dc9f8c3) to head (360221f).
⚠️ Report is 179 commits behind head on msyavuz/feat/canvas.

Files with missing lines	Patch %	Lines
superset/mcp_service/canvas/tool/update_canvas.py	0.00%	2 Missing ⚠️
superset/commands/canvas/exceptions.py	92.30%	0 Missing and 1 partial ⚠️
...set/mcp_service/canvas/tool/canvas_draft_common.py	75.00%	0 Missing and 1 partial ⚠️
superset/mcp_service/canvas/tool/create_canvas.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@                   Coverage Diff                   @@
##           msyavuz/feat/canvas   #45826      +/-   ##
=======================================================
+ Coverage                84.37%   84.41%   +0.03%     
=======================================================
  Files                     3142     3142              
  Lines                   206454   206493      +39     
  Branches                 47922    47925       +3     
=======================================================
+ Hits                    174196   174306     +110     
+ Misses                   28929    28863      -66     
+ Partials                  3329     3324       -5     

Flag	Coverage Δ
hive	35.22% <37.50%> (+<0.01%)	⬆️
mysql	52.71% <37.50%> (-0.01%)	⬇️
postgres	52.72% <37.50%> (+<0.01%)	⬆️
presto	36.90% <37.50%> (+<0.01%)	⬆️
python	87.61% <92.18%> (+0.06%)	⬆️
sqlite	52.48% <37.50%> (-0.01%)	⬇️
unit	82.31% <92.18%> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[msyavuz]

Since the input schema now carries the full Operation union, can this list shrink to the rules the schema can't express (1-based grid, scope: null resets, widgetId vs widgetType)? Otherwise agents get the grammar twice.

[rebenitez1802]

Good call, done in 360221f. Every listing that shows the full docstring also serves the input schema, and search results cut the description at 300 chars before the list starts, so the list was duplication wherever an agent could see it. The docstring now keeps only what the schema can't express: widgetType+props vs widgetId on add (never both), set_props as a complete replacement read from get_canvas_draft's widgets, the required-nullable scope/value resets, and the 1-based layout grid plus overlap rules. The native description drops from 2,637 to 1,970 chars; the regenerated inventory also refreshes commit_canvas_draft's stale entry.