fix(tools): widen ESRI Image attr_key resolution to fire for popup_options too

[romer8]

Summary

Closes the flagged secondary Class A bug from the 2026-05-21 debug arc audit. add_esri_image_layer's attr_key resolution (PR #12) was gated on if attribute_variables: — so popup_options.aliases provided WITHOUT attribute_variables got the display-name fallback. React's popup-table render path (getImageArcGISRestLayerAttributes) keys by the service-side sublayer name fetched from ?f=json, identical to the attribute_variables path. Mismatch → silent click-time lookup miss.

Root cause (verified)

Server (mcp_server.py:2061): if attribute_variables: triggered _resolve_esri_layer_name. With popup_options alone, attr_key stayed = name. PR #11's outer-key normalization rewrote popup_options.{aliases,omit} to attr_key — which was the wrong key.

React (utilities.js getImageArcGISRestLayerAttributes): keys results by layer.name from the ESRI service's ?f=json response — same as the attribute_variables path.

Fix

Widen the guard:

-    if attribute_variables:
+    if attribute_variables or popup_options:

The resolver is already a soft-fail (returns None on network failure → fallback to display name preserved). Inline comment expanded to explain both code paths and the React popup-render contract.

No new helper. No behavior change for the existing attribute_variables case. Just makes the existing resolution machinery fire for one more condition.

Tests

5 new in TestAddEsriImagePopupOptionsAttrKeyResolution:

• test_popup_options_aliases_alone_uses_resolved_sublayer_name — motivating case
• test_popup_options_omit_alone_uses_resolved_sublayer_name — same for omit
• test_both_attribute_variables_and_popup_options_same_attr_key — both fields keyed by same resolved name
• test_popup_options_alone_resolver_fails_falls_back_to_display_name — soft-fail preserved
• test_neither_attribute_variables_nor_popup_options_no_resolver_call — regression: no wasted network call

Full suite: 949 passed (944 baseline + 5 new).

Debug arc completion

This is the 8th PR in the 2026-05-21 debug arc. Class A surface is now fully covered:

Layer type	Status
ESRI Image — attribute_variables	✅ PR #12
ESRI Image — popup_options	✅ This PR
WMS — both fields	✅ PR #16
ESRI Feature / GeoJSON / KML	✅ Not affected (verified)
Tile types / GeoTIFF / static image / PMTiles raster	✅ Not applicable (not queryable at the attribute level)
PMTiles Vector	🟡 Low-risk, deferred until observed

Combined with PR #15 (render_plugin + add_dynamic_map_layer args case-fold) and PR #14 (configure_popup_modal_layer args case-fold), all known Class A + Class B surfaces are addressed.

Manual smoke after merge

Restart MCP server (with TETHYSDASH_BASE_URL set). Try a prompt like:

Add an ESRI Image layer for the China Flowlines from $URL, and in the popup table show "comid" with the alias "River ID" (no variable_input, just a column rename).

Expected: server log resolves to "Flow Forecast" (or whatever the actual sublayer name is); persisted attributeAliases keyed by the resolved name; popup table column header shows "River ID" at runtime.