Skip to content

v0.5.0: Consolidate query cluster + remove NetCDF support - #20

Merged
romer8 merged 10 commits into
mainfrom
feat/query-consolidation-v0.5.0
May 21, 2026
Merged

v0.5.0: Consolidate query cluster + remove NetCDF support#20
romer8 merged 10 commits into
mainfrom
feat/query-consolidation-v0.5.0

Conversation

@romer8

@romer8 romer8 commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Catalog 12 → 9 tools: replaces the 4-tool query/resolve cluster (query_output_file, query_output_file_from_output_selector, query_output_files_from_output_selector, resolve_output_file) with one unified query_files_by_selector using file_name/index as clearly-optional filters.
  • Slash prompts 11 → 8: deletes query_by_url, resolve_file_by_index, resolve_file_by_name (their target tools are gone).
  • NetCDF support removed: drops xarray + h5netcdf deps (~30MB+ Docker image reduction); two new envelope classes (unsupported_format:, no_supported_files:) mark the new format boundary.
  • Mixed-format selectors preserve R2 parity: result envelopes surface _excluded_netcdf_count when non-zero so the parquet-only filter is never silent.

This is a breaking release. See CHANGELOG.md v0.5.0 entry for the migration table.

Commits (squash-on-merge per the plan's PR strategy)

  1. chore: drop product-specific MCP client list from docstring — small unrelated docstring tidy (was already on local main; folded in for branch coherence)
  2. feat(query): add unified query_files_by_selector tool — Unit 1
  3. feat(prompts): retarget plot_timeseries to query_files_by_selector — Unit 3
  4. feat(prompts): delete query_by_url + resolve_file_by_* slash prompts — Unit 4
  5. feat(query)!: delete 4 legacy query/resolve tools + dead helpers — Unit 2
  6. feat(deps)!: remove NetCDF support and xarray/h5netcdf deps — Unit 5
  7. ci: update smoke-gate constants for v0.5.0 catalog — Unit 6
  8. docs(changelog): v0.5.0 entry — Unit 7

Verification

  • Test suite: 160 tests pass (full suite). New tests cover the unified tool's behavior + edge cases (XOR validator, empty/whitespace file_name, negative/out-of-range index, unsupported_format:/no_supported_files: envelopes, mixed-format _excluded_netcdf_count, parquet-only index semantics).
  • Description-contract test: test_tool_descriptions.py enforces positive AND negative invariants on query_files_by_selector's description (no concrete s3:// URLs, no example filenames, no inline SQL — per feedback_no_examples_in_tool_descriptions.md).
  • Catalog inventory: in-process Client(mcp) confirms 9 tools, 8 prompts, all 8 deleted tool names absent.
  • Smoke-gate dry-run: local replay of the release.yml Python script against the in-process Client passes (asserts EXPECTED_MIN_TOOLS=9, REMOVED-set absent, REQUIRED-set present, REQUIRED_PROMPTS present).

Test plan

  • pytest full suite (160 pass)
  • Local smoke-gate dry-run (matches release.yml constants)
  • Live deploy smoke (post-merge): WIF auto-deploy on tag push runs the smoke gate against the Cloud Run revision before traffic shifts. If gate passes, traffic shifts automatically.
  • Behavioral smoke (post-deploy, Unit 8): replay the original failing prompt from the 2026-05-20 debug session against the deployed v0.5.0 endpoint. Outcome documented in this PR's comments after the deploy completes.

Migration

External consumers hardcoded to deleted tool names break at deploy — accepted per the v0.5.0 Compatibility Policy (docs/plans/2026-05-20-001-...-plan.md and CHANGELOG). External consumers using :latest Docker tag will hit this at the next pull; pin to :v0.4 to defer (tag availability to be confirmed in registry).

Links

  • Plan: docs/plans/2026-05-20-001-refactor-nrds-mcps-query-consolidation-plan.md
  • Brainstorm: docs/brainstorms/2026-05-20-nrds-mcps-query-cluster-consolidation-requirements.md
  • Debug session that prompted the consolidation: /ce:debug 2026-05-20 (conversation history)

🤖 Generated with Claude Code

romer8 added 9 commits May 20, 2026 16:32
Generalize 'chatbox-core, Claude Desktop, Cursor, Cline, etc.' to 'MCP
clients' — the docstring's load-bearing content is the failure mode, not
which clients are affected.
Introduces the v0.5.0 query cluster's single tool replacing the legacy
4-tool surface (query_output_file, query_output_file_from_output_selector,
query_output_files_from_output_selector, resolve_output_file).

The tool handles three branches via clearly-optional filter args:
- file_name set: exact-name lookup in the parquet-filtered list
- index set: 0-based into the parquet-filtered list (NetCDF files do
  not consume index slots)
- both None: query all parquet files for the selector

Mixed-format selectors silently filter to parquet AND surface the
exclusion count as _excluded_netcdf_count on the result envelope when
non-zero — preserving R2 'no silent default' parity.

Two new envelope classes:
- unsupported_format: file_name points at .nc/.nc4 (cheap pre-S3 check)
- no_supported_files: selector resolves to zero parquet files

Description extracted to _tool_descriptions.py for lockstep contract
testing per docs/solutions/best-practices/lockstep-rule-description-
string-drift-2026-05-11.md. Lockstep test asserts positive AND negative
invariants (no concrete URLs, no inline SQL, no example filenames) per
feedback_no_examples_in_tool_descriptions.md.

Plan: docs/plans/2026-05-20-001-refactor-nrds-mcps-query-consolidation-plan.md
Brainstorm: docs/brainstorms/2026-05-20-nrds-mcps-query-cluster-consolidation-requirements.md
The plot_timeseries slash prompt now drives query_files_by_selector
instead of the soon-to-be-deleted query_output_file_from_output_selector.

Prompt-surface changes:
- Drop the 'index' arg from the user-facing slash signature. The new
  tool defaults to 'query all parquet files for the selector'; the
  prompt template instructs the LLM to omit file_name and index so
  the SQL WHERE feature_id = ... filters across the full union.
- 7 args remain (was 8): variable, feature_id, model, forecast, date,
  cycle, vpu. All still required:true with format hints.
- Narrative-only args (variable, feature_id) unchanged.

Test fixture lockstep updates:
- PLOT_TIMESERIES_ARG_NAMES: drop 'index'
- PLOT_TIMESERIES_DESCRIPTIONS: drop 'index' entry
- OVERLAPPING_ARG_NAMES: drop 'index'
- _selector_tool_schema target string: query_files_by_selector
- All assertions referring to the parity tool name updated

Plan unit: docs/plans/2026-05-20-001-refactor-nrds-mcps-query-consolidation-plan.md
(Unit 3 — Retarget plot_timeseries slash prompt)
The three deleted prompts targeted tools (query_output_file,
resolve_output_file) that are being removed in v0.5.0 — they have no
valid target in the new catalog and serve no purpose under the unified
query_files_by_selector workflow:

- query_by_url: user with an ad-hoc URL can run their own DuckDB query
  locally; the server-side direct-URL escape is gone per Compatibility
  Policy (NRDS data is fetched via the selector path).
- resolve_file_by_index, resolve_file_by_name: users wanting an S3 URL
  read the 'path' field that list_available_output_files already
  populates per entry; no separate resolve step needed.

Test fixture changes:
- QUERY_LOOKUP_PROMPTS: shrunk to {lookup_feature} only
- QUERY_LOOKUP_HINTS: dropped s3_url, query, index, file_name entries
- QUERY_LOOKUP_PROMPT_TO_TOOL: dropped 3 mappings
- Historical docstring comment cleanup

29 parametrized prompt tests removed alongside the deleted prompts
(8 prompts/list test + 7 hint-description + 7 bracket-render +
7 substitution-render). lookup_feature parametrization remains.

Plan unit: docs/plans/2026-05-20-001-refactor-nrds-mcps-query-consolidation-plan.md
(Unit 4 — Delete 3 unused slash prompts)
Removes the legacy query cluster now that query_files_by_selector
covers all valid use cases:

Deleted tools (from tools.py + logic.py):
- query_output_file               (direct-URL query; no NRDS workflow needs it)
- query_output_file_from_output_selector  (singular default-index footgun)
- query_output_files_from_output_selector (replaced by query_files_by_selector)
- resolve_output_file             (list_available_output_files already
                                   surfaces the path field per entry)

Deleted dead helpers (from utils_rest.py):
- _duckdb_query_parquet           (single-file; only consumer was query_output_file)
- _detect_output_file_kind        (parquet-vs-netcdf branch; consumer was
                                   query_output_file)
- _validate_nrds_output_file_url  (URL guard for arbitrary external input)
- _normalize_output_file_url      (s3:// → https:// translator)

Test migrations:
- test_exception_handling.py: test_query_output_file_returns_envelope_on_binder_exception
  → test_query_files_by_selector_returns_envelope_on_binder_exception
  (BinderException recovery path is load-bearing for the new tool too;
  _duckdb_query_parquets raises the same exception class)
- test_middleware.py::test_pattern_mismatch_envelope_surfaces_field_description
  retargeted from query_output_files_from_output_selector to
  query_files_by_selector (same date Field, same pattern)
- test_middleware.py::test_xor_violation_returns_envelope_not_raise
  retargeted from resolve_output_file to query_files_by_selector.
  Note: the middleware-convert-from-raise path is no longer exercised by
  a public tool (query_files_by_selector returns _error_payload directly);
  the test now asserts the dict envelope shape rather than the
  middleware-flattened string envelope.
- test_query_output_files.py: 4 tests deleted (replaced by test_query_files_by_selector.py)

Documentation:
- nextgen_mcp/README.md tool list updated (9 tools, query section
  collapsed to query_files_by_selector + adds get_hydrofabric_pmtiles_layers
  which was missing from the original list)
- prompts.py comment block updated to drop reference to deleted
  query_by_url / resolve_file_by_* variants and document why
- utils_rest.py docstring example updated from query_output_file's
  query arg to query_files_by_selector's query arg

Verified post-merge catalog: 9 tools, 8 prompts (matches plan target).

Plan unit: docs/plans/2026-05-20-001-refactor-nrds-mcps-query-consolidation-plan.md
(Unit 2 — Delete 4 legacy tools + dead helpers + migrate exception test)
Drops NetCDF as a supported format. NRDS NetCDF outputs are still
available in S3; users query them via netCDF-aware tooling (xarray,
h5netcdf, etc.) locally rather than via this server.

Code removed:
- utils_rest._get_troute_df  (~15 lines: t-route crosswalk loader)
- utils_rest._duckdb_query_netcdf  (~12 lines: in-memory pandas→DuckDB)
- _io_config.open_fsspec_file  (~18 lines: NetCDF-only fsspec wrapper;
  sole live caller was _get_troute_df)
- utils_rest: 'import xarray as xr' (top-level import)
- logic.py: '_duckdb_query_netcdf' and '_get_troute_df' imports
- logic.py:get_output_file: '.parquet OR .nc' filter trimmed to '.parquet' only

Deps removed:
- xarray (was ~30MB on its own)
- h5netcdf
- (h5py was not in the lock — only optional for h5netcdf)

Docstring/comment updates:
- _io_config.py module docstring: drop 'NetCDF via xarray' from the IO
  inventory; module now lists 'S3 via fsspec, DuckDB httpfs' only
- test_query_output_files.py docstring: explain what's left here vs what
  moved to test_query_files_by_selector.py and test_exception_handling.py

What remains intentionally:
- The 'NetCDF' references in logic.py:_resolve_parquet_files_for_query
  are the unsupported_format: envelope text (telling the LLM what's not
  supported and what external tooling to use instead) — NOT live code
  references to xarray/h5netcdf modules
- The 'no_supported_files:' envelope still surfaces a netcdf_files count
  so users see the scale of the parquet-only filter
- The _excluded_netcdf_count field on result envelopes preserves R2
  ('no silent default') parity for mixed-format selectors

Plan unit: docs/plans/2026-05-20-001-refactor-nrds-mcps-query-consolidation-plan.md
(Unit 5 — Remove NetCDF code paths + deps)
EXPECTED_MIN_TOOLS: 11 → 9 (post-merge catalog is exactly 9 tools)

REMOVED tuple adds the 4 query-cluster tools deleted in v0.5.0:
- query_output_file
- query_output_file_from_output_selector
- query_output_files_from_output_selector
- resolve_output_file

The existing 4 REMOVED entries (create_plotly_chart_*,
build_hydrofabric_feature_map_config, query_hydrofabric_parquet_file)
from prior cleanups stay as historical regression guards.

REQUIRED tuple swaps query_output_files_from_output_selector (deleted)
for query_files_by_selector (added). lookup_hydrofabric_feature
remains.

REQUIRED_PROMPTS unchanged (plot_timeseries still exists; only its
target tool changed).

NOTE: query_output_files_from_output_selector moves from REQUIRED to
REMOVED in this same commit. Failing to update either side would
produce contradictory smoke-gate assertions and block the v0.5.0
deploy on tag push.

Both embedded smoke python blocks validated to compile cleanly via
local dry-run; YAML parses cleanly. Live verification happens at
v0.5.0 tag push via WIF auto-deploy pipeline.

Plan unit: docs/plans/2026-05-20-001-refactor-nrds-mcps-query-consolidation-plan.md
(Unit 6 — Update CI smoke-gate constants in release.yml)
Documents the breaking changes for external consumers:
- 4 tools deleted (query cluster consolidation)
- 3 slash prompts deleted (alongside their target tools)
- NetCDF support removed (xarray + h5netcdf deps dropped)
- 1 new tool (query_files_by_selector) with file_name/index filters
- 2 new envelope classes (unsupported_format:, no_supported_files:)
- 1 new optional result field (_excluded_netcdf_count)

Includes a Migration table mapping each deleted tool to its replacement
workflow, plus :v0.4 tag-pinning guidance for :latest consumers per
the v0.5.0 Compatibility Policy.

Plan unit: docs/plans/2026-05-20-001-refactor-nrds-mcps-query-consolidation-plan.md
(Unit 7 — CHANGELOG v0.5.0 entry)
Observed 2026-05-20 against nemotron-3-nano:30b: small models sometimes
emit string literals like '<nil>' / 'None' / 'null' when they want to
pass None to an Optional[str] arg. Pydantic's str pattern matcher
rejected these, the InputValidationEnvelopeMiddleware fired, and the LLM
retried with actual None — recovery worked but cost a round-trip.

This patch adds a _coerce_none_string BeforeValidator that strips common
null-literals to None BEFORE the pattern check. The literal pattern
constraint is also dropped (defaulting + validation is handled
server-side via 'ensemble or "1"' for medium_range); the LLM still
sees ensemble's purpose in the description.

Per user clarification 2026-05-20: 'ensemble is only available for
medium_range and it is always 1.' Description updated to reflect this
(current data uses ensemble=1; defaults to 1 when omitted; ignored for
short_range and analysis_assim_extend). The arg is preserved for
forward-compat with future ensemble dimensions (historical bucket data
included ensemble=16; current data is ensemble=1 only).

Applied to both query_files_by_selector_tool and
list_available_output_files_tool (same null-literal hazard).

Tests:
- 12 parametrized unit tests for _coerce_none_string covering all
  documented null-literals + valid pass-through values
- 1 integration test via in-process Client verifies the BeforeValidator
  is wired correctly on the tool wrapper (ensemble='<nil>' coerced to
  None at dispatch, tool call succeeds)

Per workspace memory feedback_corruption_recovery_via_edit.md, this is
a deliberate carve-out from the 'don't add runtime detectors' rule:
small-model null-literal emission is a structural-not-semantic syntax
leak (the schema rejects, not the content) and the coercion saves a
guaranteed round-trip per small-model call without changing behavior
for any other model.

Plan unit: docs/plans/2026-05-20-001-refactor-nrds-mcps-query-consolidation-plan.md
(optional follow-up surfaced during Unit 8 smoke testing against
nemotron-3-nano:30b)
@romer8

romer8 commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Unit 8 pre-deploy smoke validation — local server, 5 prompts across 2 models

Ran the four NRDS canonical workflows against the local server (python -m nextgen_mcp.mcp_server on feat/query-consolidation-v0.5.0) via the chatbox UI. All five prompts produced correct visualizations. Detailed below.

Headline outcomes

Test Prompt scope Model Round-trips Tool selection Recoveries
1 Time series, all files gpt-oss:120b 5 query_files_by_selector (no filter) 1 BinderException (datetimetime) + 3 exploratory
2 Table, all files gpt-oss:120b 1 query_files_by_selector (no filter) 0
3 Card, all files gpt-oss:120b 1 query_files_by_selector (no filter) 0
4 Time series, first file (short_range) gpt-oss:120b 2 list_available_output_filesquery_files_by_selector (no filter; single-file selector) 0
5 Time series, first file (medium_range) nemotron-3-nano:30b 4 query_files_by_selector (index=0 set explicitly) 1 ensemble='<nil>' + 1 data + data_uri collision

Validated against v0.5.0 requirements

  • R1 (split-path inflation eliminated): zero deleted-tool calls in any trace
  • R2 (no silent default): nemotron correctly set index=0 explicitly on "first output file" against the 10-file medium_range selector
  • R3 (slash UX preserved): plot_timeseries slash dispatches correctly via the retargeted prompt
  • R4 (model-agnostic): same catalog serves both gpt-oss:120b (frontier) and nemotron-3-nano:30b (workshop-class); recovery patterns work cross-model
  • R5 (12→9 catalog): 9 tools served, no unknown-tool errors
  • R6 (NetCDF removed): server runs without xarray/h5netcdf installed

Recovery patterns exercised

  1. BinderException recovery (test 1): available_columns surfaced in envelope, LLM retried with valid column. Migrated test_query_files_by_selector_returns_envelope_on_binder_exception path validated end-to-end.
  2. Pydantic null-literal coercion (test 5): nemotron emitted ensemble='<nil>', validator rejected, envelope returned, LLM retried with None. This recovery now eliminated entirely via the _coerce_none_string BeforeValidator (latest commit 73e5efd).
  3. data XOR data_uri collision (test 5, tethysdash side): downstream create_plotly_chart rejected with invalid_args, LLM retried with just data. Result-by-reference protocol working as designed.

Latest commit on the branch: ensemble null-literal coercion (73e5efd)

In response to nemotron's <nil> emission, added _coerce_none_string BeforeValidator stripping common null-literals (<nil>, None, null, nil, undefined, empty/whitespace) to actual None BEFORE the Pydantic pattern check fires. Applied to ensemble arg on both query_files_by_selector and list_available_output_files. Per the user's domain clarification: current data uses ensemble=1 only for medium_range; arg defaults to 1 when omitted; ignored for short_range and analysis_assim_extend. Description updated to reflect this; pattern constraint dropped (defaulting handled server-side).

12 new parametrized unit tests + 1 integration test cover the helper and its wiring through the @mcp.tool wrapper.

Test suite

  • 173 tests pass (was 160 before the ensemble coercion commit)
  • Description-contract: positive + negative invariants on query_files_by_selector description hold (no concrete URLs, no inline SQL, no example filenames)
  • Local smoke-gate dry-run: EXPECTED_MIN_TOOLS=9, REMOVED set absent, REQUIRED set present, plot_timeseries prompt present

Issues observed (not blocking, tracked for follow-up)

  1. Test 1's 5 round-trips for time series — gpt-oss:120b doesn't pick time immediately from the available_columns list; explores nudgetypeSELECT * first. Model-level prompt-handling issue, not a tool-shape regression. Possible mitigation: stronger description hint on column naming conventions. Defer to v0.5.1 if signal accumulates.
  2. Test 2 missed WHERE feature_id = 2862044 filter — ambiguous prompt phrasing led the LLM to drop the filter; result table shows max flow across all features instead of just feature 2862044. Model-level mis-interpretation; pre-existing concern; chatbox UX could surface executed SQL to help users sanity-check.

Recommendation

Merge with squash-on-merge per the plan's PR strategy → tag v0.5.0 to trigger WIF auto-deploy → re-run the same 5 prompts against the deployed Cloud Run endpoint as the canonical post-deploy Unit 8. If they match these local results, v0.5.0 is fully validated.

Test data captured 2026-05-20 from local in-process testing on feat/query-consolidation-v0.5.0.

Code comments now lead with the WHY rather than the dated origin or
plan-number annotation. Detailed history lives in CHANGELOG.md and
docs/.

Removed:
- 'Plan unit:' and 'Plan 2026-MM-XXX' annotations
- 'Observed YYYY-MM-DD against <model>' dated incident attributions
- 'docs/(plans|brainstorms|solutions)/2026-...' path references
- 'feedback_*.md' workspace-memory citations
- 'Bug Nb regression', 'Migrated in v0.5.0' commit-style preambles
- README/docstring backreferences to specific brainstorm/plan files

Kept:
- Technical rationale (WHY each rule exists)
- Test-data values that happen to contain dates (real payload mocks)
- README's 'Claude Desktop' / 'Claude Code CLI' references (real
  product names with their actual setup commands)
- Migration markers that name surviving vs deleted tool names
  ('query_files_by_selector replaces 4-tool query cluster')

Net change ~120 lines removed, no functional change.
Tests: 173/173 pass.
@romer8
romer8 merged commit 6a8cc2a into main May 21, 2026
2 checks passed
@romer8
romer8 deleted the feat/query-consolidation-v0.5.0 branch May 21, 2026 00:47
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