test: add license-free pytest suite; share + harden res1d helpers (#11)

Adds a real unit-test suite for the pure, license-free logic and wires it
into CI, then folds two small robustness fixes the tests now cover.

- tests/ (test_schema, test_units, test_registry) + pytest.ini (testpaths=tests
  so collection skips the gitignored _scratch model copies); CI runs `pytest`.
- contracts/schema.match_columns: the res1d column-resolution rule (node `X`
  stays distinct from reach `X.2`) extracted from results_worker and plot_worker,
  which now share it instead of duplicating the matcher.
- mike_results_read: compute the peak on the FULL series and splice the argmax
  back into the downsampled output, so a low max_points can no longer hide the
  true peak; response now also carries peak_value / peak_time / downsampled.

Verified on the Sirius_RTC HD result: Discharge:Link_29 peak 0.79886 @ 00:05 is
preserved with max_points=100 (step=14 would otherwise drop it); node exact-match
read and plot still work; 14 tests pass in 0.5s.

Fixes #6
Fixes #7
Fixes #8

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: Zhonghao1995

.github/workflows/ci.yml

@@ -26,3 +26,8 @@ jobs:
 
       - name: Smoke test (license-free tool discovery)
         run: python scripts/smoke_test.py
+
+      - name: Unit tests (license-free)
+        run: |
+          pip install pytest
+          pytest

mikeplus_mcp/contracts/schema.py

@@ -31,6 +31,18 @@ def parse_column(col: str) -> dict:
     return {"quantity": quantity, "element_id": element_id, "chainage": chainage}
 
 
+def match_columns(columns, quantity: str, element: str) -> list:
+    """res1d columns whose ``Quantity:ElementId`` matches ``quantity``/``element``.
+
+    Matches an exact node series (``Quantity:Element``) OR a reach series carrying a
+    chainage suffix (``Quantity:Element:chainage``). The trailing ``':'`` guard keeps a
+    node ``X`` distinct from a reach ``X.2`` (the reach id contains the dot, so it never
+    matches the ``X:`` prefix). Returns the original column objects, in input order.
+    """
+    base = f"{quantity}:{element}"
+    return [c for c in columns if str(c) == base or str(c).startswith(base + ":")]
+
+
 def summarize(df, quantities=None) -> list[dict]:
     """Per-quantity peak summary (canonical). ``df`` = mikeio1d ``res.read()``."""
     cols_by_q: dict[str, list] = {}

mikeplus_mcp/workers/plot_worker.py

@@ -12,7 +12,7 @@ def main() -> None:
     out = payload["__out"]
     try:
         import mikeio1d
-        from mikeplus_mcp.contracts import plot_style
+        from mikeplus_mcp.contracts import plot_style, schema
         from mikeplus_mcp.contracts.units import unit_for
 
         action = payload.get("action", "rain_flow")
@@ -40,10 +40,7 @@ def main() -> None:
             df = res.read()
 
             def series_for(quantity, element):
-                base = f"{quantity}:{element}"
-                # exact (node) OR 'base:' prefix (reach w/ chainage) — keeps node 'X'
-                # distinct from reach 'X.2'
-                cols = [c for c in df.columns if str(c) == base or str(c).startswith(base + ":")]
+                cols = schema.match_columns(df.columns, quantity, element)
                 if not cols:
                     raise ValueError(f"no series for {quantity}:{element}")
                 return df[cols[0]], str(cols[0])

mikeplus_mcp/workers/results_worker.py

@@ -74,17 +74,22 @@ def main() -> None:
             quantity = payload["quantity"]
             element = payload["element"]
             df = res.read()
-            base = f"{quantity}:{element}"
-            # exact (node, no chainage) OR 'base:' prefix (reach w/ chainage); avoids
-            # matching reach 'C14154801.2' when the node 'C14154801' was requested
-            cols = [c for c in df.columns if str(c) == base or str(c).startswith(base + ":")]
+            cols = schema.match_columns(df.columns, quantity, element)
             if not cols:
                 raise ValueError(f"no series for quantity={quantity!r} element={element!r}")
             col = cols[0]
-            s = df[col]
+            full = df[col]
+            # peak computed on the FULL series so downsampling can never hide it
+            pv = full.max()
+            has_peak = pv == pv  # False only when NaN
             max_pts = int(payload.get("max_points", 5000))
-            step = max(1, len(s) // max_pts)
-            s = s.iloc[::step]
+            step = max(1, len(full) // max_pts)
+            s = full.iloc[::step]
+            if step > 1 and has_peak:
+                # stride decimation skips extrema — splice the true peak back in
+                t_peak = full.idxmax()
+                if t_peak not in s.index:
+                    s = full.reindex(s.index.union([t_peak]))
             meta = schema.parse_column(col)
             result = {
                 "ok": True,
@@ -95,6 +100,9 @@ def main() -> None:
                 "chainage": meta["chainage"],
                 "unit": unit_for(quantity),
                 "n_points": int(len(s)),
+                "downsampled": bool(step > 1),
+                "peak_value": float(pv) if has_peak else None,
+                "peak_time": str(full.idxmax()) if has_peak else None,
                 "times": [str(t) for t in s.index],
                 "values": [float(v) for v in s.values],
             }

pytest.ini

@@ -0,0 +1,2 @@
+[pytest]
+testpaths = tests

tests/test_registry.py

@@ -0,0 +1,40 @@
+"""The auto-discovery registry must surface exactly the expected tools with valid schemas.
+
+Imports stay license-free: discovery imports the ``tools/`` modules, which only pull in
+lightweight types + the worker launcher (never mikeplus / mikeio).
+"""
+from mikeplus_mcp.registry import discover_tools
+
+EXPECTED = {
+    "mike_model_info",
+    "mike_get_values",
+    "mike_set_values",
+    "mike_run",
+    "mike_results_list",
+    "mike_results_summary",
+    "mike_results_read",
+    "mike_plot_rain_flow",
+    "mike_plot_timeseries",
+    "mike_plot_network",
+}
+
+
+def test_discovers_exactly_the_expected_tools():
+    names = [t.name for t in discover_tools()]
+    assert set(names) == EXPECTED
+
+
+def test_tool_names_are_unique():
+    names = [t.name for t in discover_tools()]
+    assert len(names) == len(set(names))
+
+
+def test_every_tool_has_a_valid_input_schema():
+    for t in discover_tools():
+        assert t.description.strip(), f"{t.name} has no description"
+        schema = t.input_schema
+        assert schema.get("type") == "object", f"{t.name} schema is not an object"
+        assert "properties" in schema, f"{t.name} schema has no properties"
+        # every declared required field must be a defined property
+        for req in schema.get("required", []):
+            assert req in schema["properties"], f"{t.name} requires undefined field {req!r}"

tests/test_schema.py

@@ -0,0 +1,83 @@
+"""Unit tests for the engine-agnostic schema helpers (no license / no mikeio)."""
+import numpy as np
+import pandas as pd
+
+from mikeplus_mcp.contracts import schema
+
+
+def test_parse_column_node():
+    assert schema.parse_column("WaterLevel:C14150801") == {
+        "quantity": "WaterLevel",
+        "element_id": "C14150801",
+        "chainage": None,
+    }
+
+
+def test_parse_column_reach_with_chainage():
+    out = schema.parse_column("Discharge:C14150801.2:30.71")
+    assert out["quantity"] == "Discharge"
+    assert out["element_id"] == "C14150801.2"  # the dot belongs to the reach id
+    assert out["chainage"] == 30.71
+
+
+def test_parse_column_non_numeric_chainage_falls_back_to_string():
+    assert schema.parse_column("X:Y:notnum")["chainage"] == "notnum"
+
+
+# real-world column shapes taken from the Sirius_RTC HD result
+COLS = [
+    "WaterLevel:C14150801",            # node
+    "WaterLevel:C14150801.2:30.71",    # reach (synthetic) sharing the id stem
+    "Discharge:C14150801.2:30.71",     # reach
+    "Discharge:Link_29:33.5333",       # reach with a named link
+]
+
+
+def test_match_columns_node_is_not_confused_with_reach():
+    # node 'C14150801' must NOT match reach 'C14150801.2'
+    assert schema.match_columns(COLS, "WaterLevel", "C14150801") == ["WaterLevel:C14150801"]
+
+
+def test_match_columns_reach_prefix():
+    assert schema.match_columns(COLS, "Discharge", "C14150801.2") == ["Discharge:C14150801.2:30.71"]
+    assert schema.match_columns(COLS, "Discharge", "Link_29") == ["Discharge:Link_29:33.5333"]
+
+
+def test_match_columns_no_match():
+    assert schema.match_columns(COLS, "Discharge", "Nope") == []
+
+
+def _frame():
+    idx = pd.date_range("2020-01-01", periods=5, freq="h")
+    return pd.DataFrame(
+        {
+            "Discharge:Link_1:10": [0.0, 1.0, 2.0, 1.0, 0.0],
+            "Discharge:Link_2:5": [0.0, 0.0, 5.0, 0.0, 0.0],   # global Discharge peak
+            "WaterLevel:Node_1": [1.0, 1.0, 1.0, 1.0, 1.0],
+            "WaterLevel:Node_2": [np.nan] * 5,                  # all-NaN column
+        },
+        index=idx,
+    )
+
+
+def test_summarize_picks_global_peak_and_time():
+    rows = {r["quantity"]: r for r in schema.summarize(_frame())}
+    assert set(rows) == {"Discharge", "WaterLevel"}
+
+    dis = rows["Discharge"]
+    assert dis["peak_value"] == 5.0
+    assert dis["peak_element"] == "Link_2"
+    assert dis["peak_time"].startswith("2020-01-01 02:00")
+    assert dis["unit"] == "m3/s"
+    assert dis["n_series"] == 2
+
+
+def test_summarize_skips_all_nan_within_quantity():
+    rows = {r["quantity"]: r for r in schema.summarize(_frame())}
+    # WaterLevel:Node_2 is all-NaN; the peak must come from Node_1, not crash/NaN
+    assert rows["WaterLevel"]["peak_value"] == 1.0
+
+
+def test_summarize_quantity_filter():
+    rows = schema.summarize(_frame(), quantities=["Discharge"])
+    assert [r["quantity"] for r in rows] == ["Discharge"]

tests/test_units.py

@@ -0,0 +1,12 @@
+"""Unit tests for the canonical unit vocabulary."""
+from mikeplus_mcp.contracts.units import unit_for
+
+
+def test_known_quantities():
+    assert unit_for("Discharge") == "m3/s"
+    assert unit_for("WaterLevel") == "m"
+    assert unit_for("FlowVelocity") == "m/s"
+
+
+def test_unknown_quantity_returns_empty_string():
+    assert unit_for("SomethingMadeUp") == ""