aminghadersohi commented on code in PR #45021:
URL: https://github.com/apache/superset/pull/45021#discussion_r4197074149
##########
superset/mcp_service/dashboard/tool/manage_native_filters.py:
##########
@@ -298,50 +324,68 @@ def _merge_filter_update(
existing: dict[str, Any],
dashboard_chart_ids: list[int],
) -> dict[str, Any]:
- """Merge a partial update into an existing filter config.
+ """Merge a partial update into an existing filter (or divider) config.
Returns a FULL filter config (the backend command substitutes whole
entries, it does not merge deltas).
"""
merged = copy.deepcopy(existing)
- _validate_update_type_compat(spec, merged.get("filterType"))
+ is_divider = merged.get("type") == "DIVIDER"
+ _validate_update_type_compat(spec, merged.get("filterType"),
is_divider=is_divider)
if spec.name is not None:
- merged["name"] = spec.name
+ # Dividers store their display text under "title", not "name";
+ # writing "name" here would silently fail to update what the
+ # filter bar actually renders.
+ if is_divider:
+ merged["title"] = spec.name
+ else:
+ merged["name"] = spec.name
if spec.description is not None:
merged["description"] = spec.description
if spec.scope_chart_ids is not None:
Review Comment:
Fixed in e4454d3e54: divider updates reject scope_chart_ids with a clear
validation error before the write command is constructed. Regression tests
cover nonempty and empty scope lists, no writes, and successful scope updates
for regular filters.
##########
tests/unit_tests/mcp_service/test_tool_inventory.py:
##########
@@ -123,7 +123,10 @@ def budgeted_bytes(text: str) -> int:
"manage_dashboard_certification": 1_900,
"manage_dashboard_owners": 2_200,
"manage_dashboard_roles": 1_900,
- "manage_native_filters": 9_100,
+ # Includes the divider filter type (title/description-only visual
+ # separator) added alongside filter_select/time/range/timegrain:
+ # 9,649 bytes, rounded up plus the standard 100-byte headroom.
+ "manage_native_filters": 9_800,
Review Comment:
Coordination checked alongside e4454d3e54: #44985 is still open and its
merge is absent from the freshly fetched origin/master, so this commit leaves
the budget and default-selection code unchanged. The combined-code change
remains pending #44985 merging. When it lands, merge origin/master (no rebase),
keep _merge_select_default inside the non-divider branch, and re-measure the
combined tool budget using the existing convention.
--
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]
