Skip to content

fix(tools): name case-sensitive arg_names + 100-col grid in configure_popup_modal_layer popup_config description - #13

Merged
romer8 merged 1 commit into
mainfrom
fix/popup-config-args-and-cols-description
May 21, 2026
Merged

fix(tools): name case-sensitive arg_names + 100-col grid in configure_popup_modal_layer popup_config description#13
romer8 merged 1 commit into
mainfrom
fix/popup-config-args-and-cols-description

Conversation

@romer8

@romer8 romer8 commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Two distinct LLM-routing bugs surfaced in turn-2 testing of the popup-modal MCP surface, both caused by missing anchoring text in the `configure_popup_modal_layer`'s `popup_config` field description. One commit, two CRITICAL clauses added.

Root causes (full /ce-debug 2026-05-21 turn 2)

Bug A — arg-name case mismatch

User prompt: "embed the GeoGLOWS Forecast Plot plugin with River ID = ${feature.comid}"

`list_intake_plugins` (prior turn) returned:
```json
{"source": "geoglows_forecast_plot", "arg_names": ["river_ID"]}
```

LLM emitted:
```json
"args_string": "{\"river_id\": \"${feature.comid}\"}"
```

Lowercase `river_id` vs canonical `river_ID`. The LLM normalized the user's natural-language label and ignored the authoritative `arg_names`. At runtime, plugin `run()` looked up `river_ID` → undefined → no data fetched. Edit Visualization modal's "River ID" form input (bound to key `river_ID`) rendered empty.

Bug B — popup gridItem too small (Bootstrap 12-col default)

User prompt said "filling the popup grid." LLM emitted `w=12, h=12` — Bootstrap convention. But the popup uses tethysdash's `DashboardLayout` with `colCount=100` (`DashboardLayout.js:30`). React UI's `buildNewGridItem` defaults to `w=20, h=20`. The LLM's `w=12` rendered the tile at ~12% of popup width — visibly too small (geoglows_forecast_plot placeholder text wrapped vertically in a narrow column).

Fix

Extend the `popup_config` field description with two CRITICAL clauses anchoring:

  1. gridItems[].args keys to the case-sensitive `arg_names` from `list_intake_plugins` / `list_available_visualizations` (with explicit anti-pattern call-out: "if arg_names contains river_ID, the key MUST be river_ID — NOT river_id, River ID, or riverId").

  2. gridItems[].position fields (x, y, w, h) to the popup's 100-column react-grid-layout, with explicit negation of Bootstrap's 12-col convention: "the SAME grid system as the main tethysdash dashboard — NOT Bootstrap's 12-column grid."

Tests

2 new in `TestPopupConfigFieldDescription`:

  • `test_field_description_names_args_case_sensitivity` — asserts "case-sensitive" + "arg_names" appear
  • `test_field_description_names_popup_grid_col_count` — asserts "100-column" present + Bootstrap 12-col explicitly negated

Full suite: 937 passed (935 baseline + 2 new).

Escalation path

If a subsequent LLM still gets either case or cols wrong after this tightening, the next step is server-side case-insensitive arg-name normalization (Option B from the /ce-debug session) — fetch the source's `arg_names` from `TETHYSDASH_BASE_URL` and case-fold-match the keys server-side. That adds a network call per `configure_popup_modal_layer` invocation; deferred until we observe description-tightening fail.

Workspace memory `feedback_proactive_over_reactive_llm_routing.md` notes: "pattern-matching against open-vocabulary model output is a losing fight." Description tightening has worked for PR #11 (popup_options aliases) and partially for the bigger PRIORITY-clause work; it's our cheapest tool but not infinitely reliable. We measure first, escalate second.

Manual smoke after merge

Restart MCP server. Re-run the turn-2 prompt:

Enable the Custom Popup Modal on the China Flowlines layer with the title template "GeoGLOWS Forecast — ${feature.comid}". Inside the popup, embed the GeoGLOWS Forecast Plot plugin with River ID = ${feature.comid}, filling the popup grid.

Expected:

  • `args_string` contains `"river_ID"` (capital ID), not `"river_id"`
  • gridItem dims close to `w=100, h=40-60` (fills popup width)
  • At runtime: popup opens with title substituted, GeoGLOWS Forecast Plot fetches with the real comid value
  • Edit Visualization modal: "River ID" field populated with `${feature.comid}`

…_popup_modal_layer popup_config description

Debug session 2026-05-21 turn 2 with gemini-flash on the popup-modal
test prompt. Two distinct LLM-routing bugs in the gridItems shape; one
shared description-tightening fix.

## Bug A — arg-name case mismatch

User prompt asked to "embed the GeoGLOWS Forecast Plot plugin with
River ID = ${feature.comid}". list_intake_plugins (called in a prior
turn) returned:

    {"source": "geoglows_forecast_plot",
     "arg_names": ["river_ID"]}     ← capital ID

LLM emitted:

    gridItems[0].args = {"river_id": "${feature.comid}"}   ← lowercase

The LLM normalized the user's natural-language "River ID" to
snake_case and ignored the authoritative arg_names. Persisted args
had the wrong key. At runtime the GeoGLOWS plugin looked up
args["river_ID"] → undefined → no data fetched. Edit Visualization
modal's "River ID" form input (bound to key "river_ID") rendered
empty. User-visible: "Failed to retrieve data" in the popup +
empty form field in the visualization config.

## Bug B — popup gridItem too small (12-col Bootstrap default)

User prompt said "filling the popup grid." LLM emitted:

    gridItems[0] = {x: 0, y: 0, w: 12, h: 12}    ← Bootstrap 12-col

But the popup uses tethysdash's DashboardLayout with colCount=100
(DashboardLayout.js:30). React UI's PopupLayoutEditor.buildNewGridItem
defaults to w=20, h=20 — already 20% of popup width. The LLM's w=12
rendered the tile at ~12% width — visibly too small (screenshot:
geoglows_forecast_plot placeholder text wrapped vertically in a
narrow column).

## Fix — description-only tightening (per /ce-debug option A)

Both bugs are description-routing failures. The popup_config field
description didn't anchor:

(1) gridItems[].args keys to the authoritative arg_names from the
    discovery tools (list_intake_plugins / list_available_visualizations).
(2) gridItems[].position to the popup's 100-column grid (and explicitly
    negate the Bootstrap 12-col default).

This commit extends the field description with two CRITICAL clauses:

  "CRITICAL — gridItems[].args keys are case-sensitive and MUST match
   the exact arg_names returned by list_intake_plugins /
   list_available_visualizations verbatim. The arg_names field is the
   authoritative source; do NOT normalize to snake_case from the
   user's natural-language phrasing or label. If list_intake_plugins
   returns arg_names containing river_ID (capital ID), the key MUST
   be river_ID — NOT river_id, River ID, or riverId."

  "CRITICAL — gridItems[].position fields (x, y, w, h) are cells in
   the popup's 100-column react-grid-layout (the SAME grid system as
   the main tethysdash dashboard — NOT Bootstrap's 12-column grid)."

Per feedback_no_examples_in_tool_descriptions.md the river_ID example
is naming the canonical arg_name shape, not a copy-paste value the LLM
should reuse blindly — anchoring text only.

## Tests

2 new in TestPopupConfigFieldDescription:
- test_field_description_names_args_case_sensitivity: asserts
  "case-sensitive" + "arg_names" appear in the popup_config field
  description.
- test_field_description_names_popup_grid_col_count: asserts the
  100-column convention is named AND Bootstrap 12-col is explicitly
  negated.

Full suite: 937 passed (935 baseline + 2 new). Description-only;
zero behavior change; no new failure modes.

## Escalation note

Per the /ce-debug analysis, if a subsequent LLM still gets either case
or cols wrong after this tightening, the next step is server-side
case-insensitive arg-name normalization (Option B from the same
session) — fetch the source's arg_names from TETHYSDASH_BASE_URL and
case-fold-match the keys server-side. That adds a network call per
configure_popup_modal_layer invocation; it's deferred until we observe
description-tightening fail.
@romer8
romer8 merged commit d0644d0 into main May 21, 2026
2 checks passed
@romer8
romer8 deleted the fix/popup-config-args-and-cols-description branch May 21, 2026 23:53
romer8 added a commit that referenced this pull request May 22, 2026
…s keys (#14)

Debug session 2026-05-21 third turn: gemini-flash STILL emitted
args = {"river_id": "${feature.comid}"} for the geoglows_forecast_plot
plugin despite PR #13's description-tightening that explicitly named
"case-sensitive" and gave the river_ID counterexample. Two confirmed
LLM failures on case normalization → description-only is insufficient
for this class of bug (per feedback_proactive_over_reactive_llm_routing
.md: "pattern-matching against open-vocabulary model output is a losing
fight"). Escalation path agreed in /ce-debug Option B: server-side
case-insensitive arg-name normalization.

## Implementation

Two new helpers in mcp_server.py:

1. _fetch_plugin_arg_names(source) — hits the same
   TETHYSDASH_BASE_URL/visualizations/list/ endpoint as
   list_intake_plugins, returns the declared arg_names list for the
   given source. Returns:
   - list[str] when source found with declared args
   - [] when source found but has no declared args (Default registry
     types like Map/Text whose args are open-shape)
   - None when source not found OR fetch failed (network down,
     malformed response, etc.)

2. _normalize_args_case(args, arg_names) — case-fold-matches LLM-
   emitted args keys against arg_names. Rules:
   - arg_names is None → pass through unchanged (don't reject; the
     LLM may know something we don't)
   - arg_names empty → pass through unchanged
   - Exact match → preserved
   - Case-fold match → rewrite to canonical case
   - No match for a key → keep as-is (cheap path — runtime tile error
     boundary handles the bad arg downstream)
   - Two LLM keys collide on case-fold → return None to signal
     structured envelope error

Wired into configure_popup_modal_layer's gridItem normalization loop:
for each gridItem, fetch its source's arg_names, normalize args
in-place before json.dumps. Soft-fails to pass-through behavior when
TETHYSDASH_BASE_URL is unset or fetch fails — better to ship the
LLM's verbatim args than to reject the whole flow when the registry
is unreachable.

## Bonus: tighter h-hint in description

Per user feedback "the height can be reduced a bit", refined the
position field description's h guidance:
- h~25-30 for a single time-series plot (common case)
- h~35-40 for a tall plot or one with thick legends
- h~15-20 for a card or short text block
- avoid h>50 unless the popup will scroll

The LLM had picked h=40 which was visually too tall; h=25-30 is the
right default for a typical geoglows-forecast-plot use case.

## Tests

7 new in TestPopupConfigArgsCaseNormalization:
- test_lowercase_key_rewritten_to_canonical_case (motivating case)
- test_exact_match_preserved
- test_unknown_source_args_pass_through
- test_default_registry_source_no_declared_args
- test_fetch_failure_soft_fails_to_pass_through
- test_case_fold_collision_rejected_with_structured_error
- test_normalization_runs_per_gridItem_independently

Plus 5 direct unit cases on the _normalize_args_case helper verified
during dev (exact, case-fold, None passthrough, empty passthrough,
unknown-key passthrough, collision).

Full suite: 944 passed (937 baseline including PR #13's 2 description
tests + 7 new case-normalize tests). No regression to existing
TestToolHappyPath / TestEnvelopeContract — TETHYSDASH_BASE_URL is
unset in the test env so _fetch_plugin_arg_names returns None and
passthrough preserves prior behavior.

## Trade-off acknowledged

Adds one HTTP fetch per configure_popup_modal_layer invocation (per
gridItem source). No caching in v1 — list_intake_plugins is the
authoritative source and the fetch is already what list_intake_plugins
does. Future optimization (per-request memoization or a TTL cache)
can be added if observed latency justifies.

Network failure mode is soft (warning logged, pass-through preserved),
so an unreachable backend doesn't block the popup-config flow — just
removes the safety net.
romer8 added a commit that referenced this pull request May 22, 2026
Eight PRs from the 2026-05-21 popup-modal arc, grouped into Added /
Changed / Fixed / Tests / Documentation per Keep a Changelog. New tool
+ slash-prompt for configure_popup_modal_layer (PR #10), case-fold
args normalization for popup gridItems + render_plugin +
add_dynamic_map_layer (PRs #14 + #15), ESRI sublayer-ID extraction
from LAYERDEFS + ESRI Image guard widening (PRs #12 + #17), WMS
attr_key from wms_layers (PR #16), popup_options outer-key
normalization across all 11 add_*_layer tools (PR #11), tool
description tightening for case-sensitive arg_names + 100-col grid
(PR #13). Suite grew 930 → 961.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant