diff --git a/test_mcp/test_per_source_type_layer_tools.py b/test_mcp/test_per_source_type_layer_tools.py index 616e853..7ae7559 100644 --- a/test_mcp/test_per_source_type_layer_tools.py +++ b/test_mcp/test_per_source_type_layer_tools.py @@ -1422,6 +1422,113 @@ def test_advanced_dicts_accepted_as_json_strings(self): } +class TestPopupOptionsOuterKeyNormalization: + """Server-side LLM-tolerance for the popup_options outer-key shape. + + Debug session 2026-05-21 (turn 1): gemini-flash emitted + ``popup_options.aliases = {"0": {"comid": "River ID"}}`` on an ESRI + Image Service layer. The "0" came from the prompt's + params.LAYERDEFS = "0:rivercountry = 'China'" — the LLM conflated the + sublayer index with the outer key. The React popup-render path + (`Map.js:641-650`) keys alias maps by layer NAME, so the alias + silently never fired at click time. These tests pin the server-side + auto-normalize that catches the wrong outer key and rewrites it to + the layer's `name` arg. + """ + + def test_single_entry_mismatched_outer_key_rewritten_to_name(self): + """The motivating debug case: outer key '0' rewritten to 'China Flowlines'.""" + result = add_esri_image_layer( + map_uuid=MAP_UUID, + name="China Flowlines", + url="https://example.com/MapServer", + popup_options={"aliases": {"0": {"comid": "River ID"}}}, + ) + layer = result["layer_update"]["layer"] + assert layer["attributeAliases"] == { + "China Flowlines": {"comid": "River ID"} + }, "outer key should have been rewritten from '0' to the name arg" + + def test_single_entry_matching_outer_key_preserved(self): + """When the LLM gets it right, no rewrite happens.""" + result = add_esri_image_layer( + map_uuid=MAP_UUID, + name="China Flowlines", + url="https://example.com/MapServer", + popup_options={ + "aliases": {"China Flowlines": {"comid": "River ID"}} + }, + ) + layer = result["layer_update"]["layer"] + assert layer["attributeAliases"] == { + "China Flowlines": {"comid": "River ID"} + } + + def test_literal_placeholder_outer_key_rewritten(self): + """LLM copies the literal 'layer_name' placeholder from the description.""" + result = add_esri_image_layer( + map_uuid=MAP_UUID, + name="China Flowlines", + url="https://example.com/MapServer", + popup_options={ + "aliases": {"layer_name": {"comid": "River ID"}} + }, + ) + layer = result["layer_update"]["layer"] + assert layer["attributeAliases"] == { + "China Flowlines": {"comid": "River ID"} + } + + def test_omit_single_entry_outer_key_also_normalized(self): + """popup_options.omit gets the same normalization as aliases.""" + result = add_esri_image_layer( + map_uuid=MAP_UUID, + name="China Flowlines", + url="https://example.com/MapServer", + popup_options={"omit": {"0": ["sensitive_field"]}}, + ) + layer = result["layer_update"]["layer"] + assert layer["omittedPopupAttributes"] == { + "China Flowlines": ["sensitive_field"] + } + + def test_multi_entry_outer_keys_preserved(self): + """Multi-entry case is unusual for add_*_layer but preserved verbatim. + + Caller's apparent intent of authoring popups across multiple layers + in one call is respected — no rewrite. + """ + result = add_esri_image_layer( + map_uuid=MAP_UUID, + name="China Flowlines", + url="https://example.com/MapServer", + popup_options={ + "aliases": { + "Layer A": {"a": "Alpha"}, + "Layer B": {"b": "Beta"}, + } + }, + ) + layer = result["layer_update"]["layer"] + assert layer["attributeAliases"] == { + "Layer A": {"a": "Alpha"}, + "Layer B": {"b": "Beta"}, + } + + def test_empty_popup_options_aliases_preserved(self): + """Empty sub-dict is a no-op.""" + result = add_esri_image_layer( + map_uuid=MAP_UUID, + name="China Flowlines", + url="https://example.com/MapServer", + popup_options={"aliases": {}}, + ) + # Empty aliases doesn't produce an attributeAliases entry; layer + # still ends up with the queryable default block. + layer = result["layer_update"]["layer"] + assert layer.get("attributeAliases", {}) == {} + + # --------------------------------------------------------------------------- # source_props per-source-type allowlist # --------------------------------------------------------------------------- diff --git a/test_mcp/test_popup_options_description_drift.py b/test_mcp/test_popup_options_description_drift.py new file mode 100644 index 0000000..a8ee100 --- /dev/null +++ b/test_mcp/test_popup_options_description_drift.py @@ -0,0 +1,123 @@ +"""Drift guard for popup_options field descriptions across add_*_layer tools. + +Debug session 2026-05-21: gemini-flash misrouted a "alias the comid +attribute to River ID" prompt to `attribute_variables` instead of +`popup_options.aliases` on `add_esri_image_layer`. Root cause was +asymmetric description weight — `attribute_variables` had a 35-word +description with "attribute" repeated 3 times; `popup_options` had a +3-word stub ("Click-popup options") with no mention of aliases or +omit. The LLM picked the field whose description carried the relevant +domain idiom. + +`add_wms_layer` had the full description ("aliases: {layer_name: {field: +alias}}", "omit: {layer_name: [field, ...]}"); the 10 sibling layer-add +tools all got the stub. This test pins the wording so future edits +can't silently drop it again. + +Tools that need the clause: every add_*_layer tool that accepts a +`popup_options` parameter. (add_dynamic_map_layer does not — it uses +the plugin's own popup mechanism — so it is intentionally excluded.) +""" + +import asyncio + +import pytest + +from tethysdash_mcp.mcp_server import mcp + + +# Every add_*_layer tool that accepts a popup_options parameter. The +# description on each must name both the `aliases` and `omit` keys so +# the LLM can route "alias the X attribute to Y" or "hide field X" prompts +# to popup_options instead of an adjacent field like attribute_variables. +TOOLS_WITH_POPUP_OPTIONS = ( + "add_wms_layer", + "add_esri_image_layer", + "add_esri_feature_layer", + "add_geojson_layer", + "add_kml_layer", + "add_image_tile_layer", + "add_vector_tile_layer", + "add_pmtiles_vector_layer", + "add_pmtiles_raster_layer", + "add_geotiff_layer", + "add_static_image_layer", +) + + +def _run(coro): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + +@pytest.fixture(scope="module") +def tool_input_schemas(): + """Map of tool name -> JSON schema for tool inputs (from MCP catalog).""" + async def go(): + return await mcp._local_provider.list_tools() + + tools = _run(go()) + return {t.name: t.parameters for t in tools} + + +@pytest.mark.parametrize("tool_name", TOOLS_WITH_POPUP_OPTIONS) +def test_popup_options_description_names_aliases_keyword( + tool_input_schemas: dict, tool_name: str +): + """popup_options description must name `aliases` so LLMs route table-column + renames here, not to attribute_variables or another adjacent field. + """ + schema = tool_input_schemas.get(tool_name) + assert schema is not None, f"tool {tool_name!r} not in catalog" + props = schema.get("properties", {}) + assert "popup_options" in props, ( + f"tool {tool_name!r} missing popup_options field — update " + f"TOOLS_WITH_POPUP_OPTIONS if popup_options was intentionally dropped" + ) + description = props["popup_options"].get("description", "") + assert "aliases" in description, ( + f"tool {tool_name!r} popup_options description doesn't name 'aliases'. " + f"Current: {description!r}. Add the aliases keyword so LLMs (especially " + f"small models like gemini-flash) can route 'alias attribute X to Y' " + f"prompts here instead of misrouting to attribute_variables." + ) + + +@pytest.mark.parametrize("tool_name", TOOLS_WITH_POPUP_OPTIONS) +def test_popup_options_description_names_omit_keyword( + tool_input_schemas: dict, tool_name: str +): + """popup_options description must also name `omit` for symmetric routing.""" + schema = tool_input_schemas[tool_name] + description = schema["properties"]["popup_options"].get("description", "") + assert "omit" in description, ( + f"tool {tool_name!r} popup_options description doesn't name 'omit'. " + f"Current: {description!r}." + ) + + +@pytest.mark.parametrize("tool_name", TOOLS_WITH_POPUP_OPTIONS) +def test_popup_options_description_anchors_outer_key_to_name_arg( + tool_input_schemas: dict, tool_name: str +): + """The outer-key/`name`-arg binding must be explicit. + + Debug session 2026-05-21 turn 2: gemini-flash emitted + ``popup_options.aliases = {"0": {"comid": "River ID"}}`` on an + ESRI Image layer call, because it conflated "layer ID" (the + sublayer index from params.LAYERDEFS) with the outer-key shape's + ``layer_name`` slot. The React popup-render path keys by layer + name, so the alias silently never fired. This test pins the + anchoring text that tells the LLM to use the `name` arg. + """ + schema = tool_input_schemas[tool_name] + description = schema["properties"]["popup_options"].get("description", "") + assert "`name` arg" in description, ( + f"tool {tool_name!r} popup_options description doesn't anchor the " + f"outer layer_name key to the `name` arg. Current: {description!r}. " + f"Add language naming the binding so LLMs don't fall back to " + f"sublayer IDs." + ) diff --git a/tethysdash_mcp/mcp_server.py b/tethysdash_mcp/mcp_server.py index 088c235..89544d7 100644 --- a/tethysdash_mcp/mcp_server.py +++ b/tethysdash_mcp/mcp_server.py @@ -80,13 +80,11 @@ # + The contract test # `test_prompt_target_tool_is_visible_in_default_list_tools` # trivially passes since every tool is visible at the server - # - Plan 2026-05-07-007 (T3) found that compound prompts like - # "create a map with a WMS layer" could let the 11 add_*_layer - # tools dominate the embedding score, crowding out - # create_map_visualization. If the Phase 3c smoke surfaces this - # regression, fall back: re-introduce BM25SearchTransform with a - # curated `always_visible` set or use Option C from the plan - # (pin 3-4 most-common layer types only). + # - Compound prompts like "create a map with a WMS layer" can let the + # 11 add_*_layer tools dominate the embedding score, crowding out + # create_map_visualization. If that regression appears, fall back + # to re-introducing BM25SearchTransform with a curated + # `always_visible` set or pinning 3-4 most-common layer types only. middleware=[ # Order: observability is OUTERMOST so it observes the final # envelope produced by InputValidationEnvelopeMiddleware @@ -132,8 +130,8 @@ def _backend_not_configured_envelope() -> Dict[str, Any]: def _load_runtime_plugin_registry_http() -> List[Dict[str, Any]]: """Read the runtime plugin registry from tethysdash over HTTP. - Replaces the filesystem-coupled reader from the prior architecture - (plan 2026-05-11-006). The tethysdash side exposes the registry at + Replaces the filesystem-coupled reader from the prior architecture. + The tethysdash side exposes the registry at ``${TETHYSDASH_BASE_URL}/runtime-plugins/list/`` -- a sibling of the auth-gated ``runtime-plugins/sync/`` endpoint that powers the browser-side registration UI. @@ -573,10 +571,9 @@ def create_plotly_chart( Returns a visualization spec that the chatbox dispatches as a grid item. The chart renders using TethysDash's native BasePlot component. """ - # LLM-syntax-leak recovery (Plan 2026-05-18-002 Unit 5 follow-up): - # nemotron-3, qwen-3.5, deepseek-pro-4 all observed emitting the - # string "None" / "null" for Optional[Dict] args. We accept these - # as Union[Dict, str] then coerce back to actual None here. + # LLM-syntax-leak recovery: some models emit the literal string + # "None" / "null" for Optional[Dict] args. Accept as Union[Dict, str] + # then coerce back to actual None here. layout = _coerce_none_string(layout) config = _coerce_none_string(config) # JSON-string acceptance for layout/config (matches the existing @@ -593,12 +590,11 @@ def create_plotly_chart( except json.JSONDecodeError as e: return {"error": f"invalid_args: `config` is not valid JSON: {e}"} - # Plan 2026-05-18-002 Unit 5 — validate the exactly-one-of contract - # between `data` (inline) and `data_uri` (cache URI). In the mediated - # path, chatbox-core resolves `data_uri` into `data` BEFORE dispatch - # and drops the `data_uri` arg, so this server-side validator should - # see exactly one of the two set. Defense-in-depth for unmediated - # clients (Claude Desktop, mcp-cli) that don't run chatbox-core's + # Validate the exactly-one-of contract between `data` (inline) and + # `data_uri` (cache URI). In the mediated path, chatbox-core resolves + # `data_uri` into `data` BEFORE dispatch and drops the `data_uri` arg, + # so this validator should see exactly one of the two set. + # Defense-in-depth for unmediated clients that don't run chatbox-core's # substitution layer. err = ensure_exactly_one_set(data, "data", data_uri, "data_uri") if err: @@ -615,9 +611,8 @@ def create_plotly_chart( ), "fix_hint": ( "If you're using chatbox-core, ensure `enableResultCache={true}` " - "is set on the mount. If you're a different MCP " - "client (Claude Desktop, mcp-cli, custom), retry with `data` " - "populated inline as the structured array." + "is set on the mount. Other MCP clients must retry " + "with `data` populated inline as the structured array." ), } @@ -771,8 +766,7 @@ def create_data_table( Returns a visualization spec that renders using TethysDash's native DataTable component. """ - # Plan 2026-05-18-002 Unit 5 — see create_plotly_chart for the same - # exactly-one-of contract validation. + # See create_plotly_chart for the same exactly-one-of contract. err = ensure_exactly_one_set(data, "data", data_uri, "data_uri") if err: return {"error": err} @@ -967,10 +961,10 @@ def create_card( strings raise and produce an ``{"error": ...}`` envelope rather than silently becoming a scalar label. """ - # Plan 2026-05-18-002 Unit 5 — exactly-one-of contract between `data` - # (inline) and `data_uri` (cache URI). For `create_card` the inline - # form accepts None (empty placeholder is valid) so the validator - # only fires when BOTH are set or when `data_uri` arrived unresolved. + # Exactly-one-of contract between `data` (inline) and `data_uri` (cache + # URI). For `create_card` the inline form accepts None (empty placeholder + # is valid) so the validator only fires when BOTH are set or when + # `data_uri` arrived unresolved. if data is not None and data_uri is not None: return { "error": ( @@ -1404,8 +1398,8 @@ def _env_int(name: str, default: int) -> int: "minZoom": "set_min_zoom", "maxZoom": "set_max_zoom", "minZoomQuery": "set_min_zoom_query", - # Plan 2026-05-07-004 Unit C — paired with LAYER_PROPERTIES_ALLOWLIST - # in plugin_helpers.py. Drift test enforces parity. + # Paired with LAYER_PROPERTIES_ALLOWLIST in plugin_helpers.py. + # Drift test enforces parity. "visible": "set_layer_visibility", } @@ -1415,13 +1409,12 @@ def _validate_uuid_arg( ) -> Optional[str]: """Validate that ``value`` is a well-formed UUID string. - Plan 2026-05-07-002 Unit A. Some LLMs (observed: gemma4:31b) emit - Mustache-style template placeholders like ``{{last_map_uuid}}`` for - chained tool args, expecting the host framework to substitute the - prior tool's return value. MCP doesn't do this. Without validation - the literal string passes through and the layer/patch is silently - dropped downstream when the dispatcher can't find a matching grid - item. + Some LLMs (observed: gemma4:31b) emit Mustache-style template + placeholders like ``{{last_map_uuid}}`` for chained tool args, + expecting the host framework to substitute the prior tool's return + value. MCP doesn't do this. Without validation the literal string + passes through and the layer/patch is silently dropped downstream + when the dispatcher can't find a matching grid item. Returns ``None`` on success or a structured error string the caller wraps in ``{"error": ...}``. The string is always prefixed @@ -1457,8 +1450,7 @@ def _validate_uuid_arg( def _validate_source_params( source_type: str, params: Optional[Dict[str, Any]] ) -> Optional[str]: - """Plan 2026-05-07-004 Unit A: reject `params` for source types that - don't consume it. + """Reject `params` for source types that don't consume it. The producer's per-source-type ``_flat_source_props`` build (lines ~1148-1231 of this file) only handles ``params`` for WMS, ESRI Image @@ -1621,14 +1613,12 @@ def _validate_advanced_layer_dicts( "Static Image", ] -# Plan 2026-05-07-004 Unit A: source types that don't consume `params` -# server-side. Pre-fix, calls supplying `params` for these types were -# silently dropped — the renderer never saw the keys and the LLM had no -# indication. Now `_validate_source_params` rejects with a structured -# `invalid_source_params:` envelope so the LLM can correct on the next -# round. If a future feature adds per-type `params` semantics for one of -# these (e.g., GeoJSON style overrides, KML extractStyles), remove it -# from this set in the same change that wires the consumption path. +# Source types that don't consume `params` server-side. Calls supplying +# `params` for these types are rejected via `_validate_source_params` with +# a structured `invalid_source_params:` envelope so the LLM can correct +# on the next round. If a future feature adds per-type `params` semantics +# (e.g., GeoJSON style overrides, KML extractStyles), remove it from this +# set in the same change that wires the consumption path. _TYPES_REJECTING_PARAMS = frozenset({ "GeoJSON", "KML", @@ -1686,22 +1676,17 @@ def _resolve_esri_layer_name(url: str, layer_id: Optional[str]) -> Optional[str] # --------------------------------------------------------------------------- -# Plan 2026-05-07-007 (T3): per-source-type map-layer tools. +# Per-source-type map-layer tools. # # These 11 tools replace the umbrella `add_map_service_layer`. Each has a # flat narrow signature whose required + optional fields match exactly that # source type's entry in `available_source_properties` (plugin_helpers.py). -# The conditional schema the LLM had to reason about under the umbrella is -# gone — the MCP catalog itself now is the per-type contract. +# The MCP catalog itself is the per-type contract. # -# Implementation note vs plan K7: the plan estimated the shared post-routing -# block at ~47 lines and prescribed inlining it across 11 tools. The actual -# block is ~87 lines (see umbrella history). Inlining 87 lines × 11 tools -# would be ~957 lines of duplicated code with high copy-paste-error risk. -# We deviate to a module-private helper `_apply_common_layer_options` that -# preserves K7's spirit (no public abstraction, no new test file, helper -# exercised end-to-end through every tool's per-tool oracle tests) without -# paying the duplication cost. +# Shared post-routing is factored into `_apply_common_layer_options` rather +# than inlined per-tool to avoid ~957 lines of duplicated code with high +# copy-paste-error risk. The helper is exercised end-to-end through every +# tool's per-tool oracle tests. # --------------------------------------------------------------------------- @@ -1775,8 +1760,12 @@ def _apply_common_layer_options( if attribute_variables: for key, variable in attribute_variables.items(): builder.add_attribute_variable(key, variable, attr_key) - _popup_aliases = (popup_options or {}).get("aliases") or {} - _popup_omit = (popup_options or {}).get("omit") or {} + _popup_aliases = _normalize_popup_outer_keys( + (popup_options or {}).get("aliases") or {}, attr_key, "aliases" + ) + _popup_omit = _normalize_popup_outer_keys( + (popup_options or {}).get("omit") or {}, attr_key, "omit" + ) for layer_name, alias_map in _popup_aliases.items(): if not isinstance(alias_map, dict): raise ValueError( @@ -1795,6 +1784,49 @@ def _apply_common_layer_options( builder.omit_popup_attribute(field, layer_name) +def _normalize_popup_outer_keys( + sub_dict: Dict[str, Any], layer_name: str, kind: str +) -> Dict[str, Any]: + """Rewrite a single-entry popup sub-dict's outer key to match ``layer_name``. + + LLM behavior tolerance: when ``add_*_layer`` adds a single layer named + ``layer_name``, but ``popup_options.aliases`` or ``popup_options.omit`` + is keyed by something other than the layer's name (e.g., ``"0"`` — + a sublayer ID from ``params.LAYERDEFS`` — or the literal placeholder + ``"layer_name"`` from a description-shape copy), rewrite the key. + + The React popup-render path (`reactapp/components/visualizations/Map.js`) + looks up alias maps by **layer name** (the value passed via the tool's + ``name`` arg). A mismatched outer key means the alias silently never + fires at click time. + + Rules: + * Empty dict -> return as-is. + * Single-entry dict with key != ``layer_name`` -> rewrite to + ``{layer_name: }``. Tag the call for log/debug visibility. + * Multi-entry dict OR single-entry dict whose key already matches + ``layer_name`` -> return as-is. Multi-entry case preserves the + caller's apparent intent of authoring popups across multiple + layers in one call, though that path is unusual for add_*_layer. + + ``kind`` is a label used in the debug log line (``"aliases"`` / + ``"omit"``) so the rewrite is grep-able if it ever surprises a caller. + """ + if not sub_dict: + return sub_dict + if len(sub_dict) != 1: + return sub_dict + only_key = next(iter(sub_dict)) + if only_key == layer_name: + return sub_dict + LOGGER.debug( + "popup_options.%s outer key %r != layer name %r; rewriting " + "to use the layer name (LLM tolerance).", + kind, only_key, layer_name, + ) + return {layer_name: sub_dict[only_key]} + + # --------------------------------------------------------------------------- # WMS # --------------------------------------------------------------------------- @@ -1856,7 +1888,12 @@ def add_wms_layer( ))] = None, popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " - "and {'omit': {layer_name: [field, ...]}} sub-dicts." + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." ))] = None, ) -> Dict[str, Any]: """Add a WMS service layer to an existing map.""" @@ -1963,7 +2000,15 @@ def add_esri_image_layer( style: Annotated[Optional[Union[str, Dict[str, Any]]], Field(description="Layer style")] = None, source_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced source.props")] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add an ESRI Image and Map Service layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2083,7 +2128,15 @@ def add_esri_feature_layer( style: Annotated[Optional[Union[str, Dict[str, Any]]], Field(description="Layer style")] = None, source_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced source.props")] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add an ESRI Feature Service layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2187,7 +2240,15 @@ def add_geojson_layer( style: Annotated[Optional[Union[str, Dict[str, Any]]], Field(description="Layer style")] = None, source_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced source.props")] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add a GeoJSON layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2314,7 +2375,15 @@ def add_kml_layer( style: Annotated[Optional[Union[str, Dict[str, Any]]], Field(description="Layer style")] = None, source_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced source.props")] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add a KML layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2390,7 +2459,15 @@ def add_image_tile_layer( style: Annotated[Optional[Union[str, Dict[str, Any]]], Field(description="Layer style")] = None, source_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced source.props")] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add an Image Tile layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2471,7 +2548,15 @@ def add_vector_tile_layer( style: Annotated[Optional[Union[str, Dict[str, Any]]], Field(description="Layer style")] = None, source_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced source.props")] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add a Vector Tile layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2548,7 +2633,15 @@ def add_pmtiles_vector_layer( style: Annotated[Optional[Union[str, Dict[str, Any]]], Field(description="Layer style")] = None, source_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced source.props (tileSize, attributions)")] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add a PMTiles Vector layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2624,7 +2717,15 @@ def add_pmtiles_raster_layer( style: Annotated[Optional[Union[str, Dict[str, Any]]], Field(description="Layer style")] = None, source_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced source.props (tileSize, attributions)")] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add a PMTiles Raster layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2722,7 +2823,15 @@ def add_geotiff_layer( "source_props for the same key." ))] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add a GeoTIFF raster layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2859,7 +2968,15 @@ def add_static_image_layer( style: Annotated[Optional[Union[str, Dict[str, Any]]], Field(description="Layer style")] = None, source_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced source.props (attributions, etc.)")] = None, layer_props: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Advanced layer-level props")] = None, - popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description="Click-popup options")] = None, + popup_options: Annotated[Optional[Union[Dict[str, Any], str]], Field(description=( + "Click-popup options. Accepts {'aliases': {layer_name: {field: alias}}} " + "and {'omit': {layer_name: [field, ...]}} sub-dicts. The outer " + "layer_name key MUST be the same string passed as this tool's `name` " + "arg (the layer's display name) — NOT a sublayer ID from params, " + "params.LAYERDEFS, layer_id, or any other identifier. The server " + "auto-corrects single-entry sub-dicts with mismatched outer keys, " + "but the LLM should still emit the correct shape." + ))] = None, ) -> Dict[str, Any]: """Add a Static Image overlay layer to an existing map.""" uuid_error = _validate_uuid_arg(map_uuid, "map_uuid", "create_map_visualization") @@ -2914,9 +3031,10 @@ def add_static_image_layer( # --------------------------------------------------------------------------- -# (Reserved space — `add_map_service_layer` umbrella was deleted here per -# Plan 2026-05-07-007. Tests previously targeting the umbrella have been -# migrated to per-tool oracle tests in test_per_source_type_layer_tools.py.) +# (Reserved space — `add_map_service_layer` umbrella was deleted here when +# per-source-type tools replaced it. Tests previously targeting the umbrella +# have been migrated to per-tool oracle tests in +# test_per_source_type_layer_tools.py.) # --------------------------------------------------------------------------- @@ -3292,10 +3410,9 @@ def patch_visualization( Returns `{error: "..."}` with a structured error class prefix on failure: `invalid_envelope`, `whitelist_rejected`, `invalid_uuid`. """ - # Plan 2026-05-07-002 Unit A: reject template placeholders / malformed - # UUIDs before any other work. Cheapest reject; most actionable error - # for the LLM (the prior tool's create_* return value is the recovery - # source of truth). + # Reject template placeholders / malformed UUIDs before any other work. + # Cheapest reject; most actionable error for the LLM (the prior tool's + # create_* return value is the recovery source of truth). uuid_error = _validate_uuid_arg( uuid, "uuid", @@ -3696,9 +3813,8 @@ def add_dynamic_map_layer( fetch failures surface through Map.js's existing visualization-error path (no new error handling here). """ - # Plan 2026-05-07-002 Unit A: same UUID validation as - # add_map_service_layer — reject template placeholders / malformed UUIDs - # at the boundary. + # Same UUID validation as the per-source-type add_* tools — reject + # template placeholders / malformed UUIDs at the boundary. uuid_error = _validate_uuid_arg( map_uuid, "map_uuid", "create_map_visualization" ) @@ -4058,7 +4174,7 @@ def register_runtime_plugin( authenticated write path; registration must go through the browser- side chatbox UI (which posts to tethysdash's ``runtime-plugins/sync/`` endpoint with the user's session credential). Re-enables once auth - plus a write-side HTTP path land -- see plan 2026-05-11-004 (deferred). + plus a write-side HTTP path land (deferred). The arguments are preserved on the tool signature so the prompt / slash-command surface (``test_prompts.py`` parity contract) keeps diff --git a/tethysdash_mcp/plugin_helpers.py b/tethysdash_mcp/plugin_helpers.py index 794c694..08e09b1 100644 --- a/tethysdash_mcp/plugin_helpers.py +++ b/tethysdash_mcp/plugin_helpers.py @@ -74,11 +74,10 @@ def get_plugin_prop(obj, name, default=None): "minZoom": (int, float), "maxZoom": (int, float), "minZoomQuery": (int, float), - # Plan 2026-05-07-004 Unit C: layer-level initial-visibility flag. - # Persists at configuration.layerVisibility via set_layer_visibility; - # Map.js:335-340 starts the layer hidden when False. Python-side - # superset is OK (JS drift guard at sourcePropertiesOptionsDrift.test.js - # only asserts Python ⊇ JS). + # Layer-level initial-visibility flag. Persists at + # configuration.layerVisibility via set_layer_visibility; Map.js:335-340 + # starts the layer hidden when False. Python-side superset is OK (the JS + # drift guard only asserts Python ⊇ JS). "visible": (bool,), } @@ -205,17 +204,15 @@ def get_allowed_source_prop_keys(source_type: str) -> set: }, "optional": { "attributions": "Attributions", - # Plan 2026-05-07-004 Unit B: renderer-consumed keys. + # Renderer-consumed keys. # # `bands`, `nodata`, `min`, `max` flow to OL's GeoTIFF source - # constructor at source.props. via set_source_properties - # (existing builder path). + # constructor at source.props. via set_source_properties. # # `rampName`, `rampMin`, `rampMax` are TethysDash auto-legend - # metadata read by Map.js (visualizations) at source. - # directly (siblings to `type`/`props`). The GeoTIFF branch - # of add_map_service_layer routes these three to source-top- - # level via set_source_top_level_props. + # metadata read by Map.js at source. directly (siblings + # to `type`/`props`). The GeoTIFF branch routes these to + # source-top-level via set_source_top_level_props. "bands": ( "Comma-separated band indices to render (e.g., '1,2,3'). " "Parsed at render time." @@ -528,10 +525,9 @@ def set_source_top_level_props(self, **kwargs): """Set properties at the source-top-level (siblings of `type` and `props`), as opposed to under `source.props`. - Plan 2026-05-07-004 Unit B: GeoTIFF auto-legend metadata - (`rampName`, `rampMin`, `rampMax`) is read by Map.js at - ``layer.configuration.props.source.`` directly — NOT under - ``source.props.``. Use this method instead of + GeoTIFF auto-legend metadata (`rampName`, `rampMin`, `rampMax`) is + read by Map.js at ``layer.configuration.props.source.`` directly + — NOT under ``source.props.``. Use this method instead of ``set_source_properties`` for keys whose consumer reads at source-top-level.