diff --git a/ROADMAP.md b/ROADMAP.md index 4a076bd..5b61dad 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -30,7 +30,9 @@ ucon is a dimensional analysis library for engineers building systems where unit | v0.6.x | LogMap + Nines | Complete | | v0.6.x | Dimensional Type Safety | Complete | | v0.7.0 | MCP Error Suggestions | Complete | -| v0.7.x | MCP Compute + Schema Constraints | Planned | +| v0.7.1 | MCP Error Infrastructure for Multi-Step Chains | Complete | +| v0.7.2 | Compute Tool | Planned | +| v0.7.x | Schema-Level Dimension Constraints | Planned | | v0.8.0 | String Parsing | Planned | | v0.9.0 | Constants + Logarithmic Units | Planned | | v0.10.0 | Scientific Computing | Planned | @@ -257,18 +259,19 @@ Building on v0.5.x baseline: --- -## v0.7.1 — Pre-Compute Foundations (Planned) +## v0.7.1 — MCP Error Infrastructure for Multi-Step Chains (Complete) **Theme:** Architectural prerequisites for multi-step factor-label chains. -- [ ] SI symbol coverage audit (ensure `A`, `V`, `W`, etc. in case-sensitive registry) -- [ ] Add `step: int | None` field to `ConversionError` for chain error localization -- [ ] Extract `_resolve_unit(name, parameter)` helper to reduce try/except duplication -- [ ] Add `build_parse_error` builder for malformed composite expressions -- [ ] Document priority alias invariant for contributors +- [x] SI symbol coverage audit (`A` for ampere, `kat` for katal) +- [x] `catalytic_activity` dimension and `katal` unit added +- [x] `step: int | None` field in `ConversionError` for chain error localization +- [x] `resolve_unit()` helper to reduce try/except duplication +- [x] `build_parse_error` builder for malformed composite expressions +- [x] Priority alias invariant documented for contributors **Outcomes:** -- Expressions like `V/mA`, `mA·h`, `µA/cm²` resolve correctly +- Expressions like `V/mA`, `mA·h`, `µA/cm²`, `mkat` resolve correctly - Error responses can localize failures to specific steps in a chain - MCP server code is DRY and ready for compute's N-factor resolution - `ParseError` wrapped in structured `ConversionError` like other error types diff --git a/tests/ucon/mcp/test_server.py b/tests/ucon/mcp/test_server.py index afec497..7825b4f 100644 --- a/tests/ucon/mcp/test_server.py +++ b/tests/ucon/mcp/test_server.py @@ -519,5 +519,60 @@ def test_bad_dimension_filter(self): ) +class TestParseErrorHandling(unittest.TestCase): + """Test that malformed unit expressions return structured errors.""" + + @classmethod + def setUpClass(cls): + try: + from ucon.mcp.server import convert, check_dimensions + from ucon.mcp.suggestions import ConversionError + cls.convert = staticmethod(convert) + cls.check_dimensions = staticmethod(check_dimensions) + cls.ConversionError = ConversionError + cls.skip_tests = False + except ImportError: + cls.skip_tests = True + + def setUp(self): + if self.skip_tests: + self.skipTest("mcp not installed") + + def test_unbalanced_parens_from_unit(self): + """Test that unbalanced parentheses in from_unit returns parse_error.""" + result = self.convert(1, "W/(m^2*K", "W/(m^2*K)") + self.assertIsInstance(result, self.ConversionError) + self.assertEqual(result.error_type, "parse_error") + self.assertEqual(result.parameter, "from_unit") + self.assertIn("parse", result.error.lower()) + + def test_unbalanced_parens_to_unit(self): + """Test that unbalanced parentheses in to_unit returns parse_error.""" + result = self.convert(1, "W/(m^2*K)", "W/(m^2*K") + self.assertIsInstance(result, self.ConversionError) + self.assertEqual(result.error_type, "parse_error") + self.assertEqual(result.parameter, "to_unit") + + def test_parse_error_in_check_dimensions(self): + """Test that parse errors work in check_dimensions too.""" + result = self.check_dimensions("m/s)", "m/s") + self.assertIsInstance(result, self.ConversionError) + self.assertEqual(result.error_type, "parse_error") + self.assertEqual(result.parameter, "unit_a") + + def test_parse_error_hints_helpful(self): + """Test that parse error hints are helpful.""" + result = self.convert(1, "kg*(m/s^2", "N") + self.assertIsInstance(result, self.ConversionError) + self.assertEqual(result.error_type, "parse_error") + # Should have hints about syntax + hints_str = str(result.hints) + self.assertTrue( + "parenthes" in hints_str.lower() or + "syntax" in hints_str.lower() or + "parse" in hints_str.lower() + ) + + if __name__ == '__main__': unittest.main() diff --git a/ucon/core.py b/ucon/core.py index 30e53d2..86e8830 100644 --- a/ucon/core.py +++ b/ucon/core.py @@ -85,6 +85,7 @@ class Dimension(Enum): angular_momentum = Vector(-1, 2, 1, 0, 0, 0, 0, 0) area = Vector(0, 2, 0, 0, 0, 0, 0, 0) capacitance = Vector(4, -2, -1, 2, 0, 0, 0, 0) + catalytic_activity = Vector(-1, 0, 0, 0, 0, 0, 1, 0) # mol/s (katal) charge = Vector(1, 0, 0, 1, 0, 0, 0, 0) conductance = Vector(3, -2, -1, 2, 0, 0, 0, 0) conductivity = Vector(3, -3, -1, 2, 0, 0, 0, 0) diff --git a/ucon/mcp/server.py b/ucon/mcp/server.py index d6440a0..aef5e7f 100644 --- a/ucon/mcp/server.py +++ b/ucon/mcp/server.py @@ -9,17 +9,16 @@ from mcp.server.fastmcp import FastMCP from pydantic import BaseModel -from ucon import Dimension, get_unit_by_name +from ucon import Dimension from ucon.core import Number, Scale, Unit, UnitProduct from ucon.graph import DimensionMismatch, ConversionNotFound from ucon.mcp.suggestions import ( ConversionError, - build_unknown_unit_error, + resolve_unit, build_dimension_mismatch_error, build_no_path_error, build_unknown_dimension_error, ) -from ucon.units import UnknownUnitError mcp = FastMCP("ucon") @@ -91,16 +90,14 @@ def convert(value: float, from_unit: str, to_unit: str) -> ConversionResult | Co ConversionError if the conversion fails, with suggestions for correction. """ # 1. Parse source unit - try: - src = get_unit_by_name(from_unit) - except UnknownUnitError: - return build_unknown_unit_error(from_unit, parameter="from_unit") + src, err = resolve_unit(from_unit, parameter="from_unit") + if err: + return err # 2. Parse target unit - try: - dst = get_unit_by_name(to_unit) - except UnknownUnitError: - return build_unknown_unit_error(to_unit, parameter="to_unit") + dst, err = resolve_unit(to_unit, parameter="to_unit") + if err: + return err # 3. Perform conversion try: @@ -230,15 +227,13 @@ def check_dimensions(unit_a: str, unit_b: str) -> DimensionCheck | ConversionErr DimensionCheck indicating compatibility and the dimension of each unit. ConversionError if a unit string cannot be parsed. """ - try: - a = get_unit_by_name(unit_a) - except UnknownUnitError: - return build_unknown_unit_error(unit_a, parameter="unit_a") + a, err = resolve_unit(unit_a, parameter="unit_a") + if err: + return err - try: - b = get_unit_by_name(unit_b) - except UnknownUnitError: - return build_unknown_unit_error(unit_b, parameter="unit_b") + b, err = resolve_unit(unit_b, parameter="unit_b") + if err: + return err dim_a = a.dimension if isinstance(a, Unit) else a.dimension dim_b = b.dimension if isinstance(b, Unit) else b.dimension diff --git a/ucon/mcp/suggestions.py b/ucon/mcp/suggestions.py index 3c1223b..22771db 100644 --- a/ucon/mcp/suggestions.py +++ b/ucon/mcp/suggestions.py @@ -19,6 +19,10 @@ from pydantic import BaseModel +from ucon import get_unit_by_name +from ucon.parsing import ParseError +from ucon.units import UnknownUnitError + if TYPE_CHECKING: from ucon.core import Dimension, Unit, UnitProduct @@ -31,9 +35,13 @@ class ConversionError(BaseModel): error : str Human-readable description of what went wrong. error_type : str - One of: "unknown_unit", "dimension_mismatch", "no_conversion_path". + One of: "unknown_unit", "dimension_mismatch", "no_conversion_path", + "parse_error". parameter : str | None Which input caused the error (e.g., "from_unit", "to_unit", "unit_a"). + step : int | None + For multi-step chains (compute tool), the 0-indexed step where the + error occurred. None for single conversions. got : str | None What the agent provided (dimension or unit name). expected : str | None @@ -49,6 +57,7 @@ class ConversionError(BaseModel): error: str error_type: str parameter: str | None = None + step: int | None = None got: str | None = None expected: str | None = None likely_fix: str | None = None @@ -196,12 +205,53 @@ def _get_dimension_name(unit) -> str: return dim.name +# ----------------------------------------------------------------------------- +# Unit Resolution Helper +# ----------------------------------------------------------------------------- + + +def resolve_unit( + name: str, + parameter: str, + step: int | None = None, +): + """Try to parse a unit string, returning a structured error on failure. + + This helper reduces try/except boilerplate in MCP tools. + + Parameters + ---------- + name : str + The unit string to parse. + parameter : str + Which parameter this is (e.g., "from_unit", "to_unit"). + step : int | None + For multi-step chains, the 0-indexed step. + + Returns + ------- + tuple[Unit | UnitProduct, None] | tuple[None, ConversionError] + On success: (parsed_unit, None) + On failure: (None, ConversionError) + """ + try: + return get_unit_by_name(name), None + except UnknownUnitError: + return None, build_unknown_unit_error(name, parameter=parameter, step=step) + except ParseError as e: + return None, build_parse_error(name, str(e), parameter=parameter, step=step) + + # ----------------------------------------------------------------------------- # Error Builders # ----------------------------------------------------------------------------- -def build_unknown_unit_error(bad_name: str, parameter: str) -> ConversionError: +def build_unknown_unit_error( + bad_name: str, + parameter: str, + step: int | None = None, +) -> ConversionError: """Build a ConversionError for an unknown unit. Parameters @@ -210,6 +260,8 @@ def build_unknown_unit_error(bad_name: str, parameter: str) -> ConversionError: The unrecognized unit string. parameter : str Which parameter was bad (e.g., "from_unit", "to_unit"). + step : int | None + For multi-step chains, the 0-indexed step where the error occurred. Returns ------- @@ -242,6 +294,7 @@ def build_unknown_unit_error(bad_name: str, parameter: str) -> ConversionError: error=f"Unknown unit: '{bad_name}'", error_type="unknown_unit", parameter=parameter, + step=step, likely_fix=likely_fix, hints=hints, ) @@ -252,6 +305,7 @@ def build_dimension_mismatch_error( to_unit_str: str, src_unit, dst_unit, + step: int | None = None, ) -> ConversionError: """Build a ConversionError for a dimension mismatch. @@ -265,6 +319,8 @@ def build_dimension_mismatch_error( The parsed source unit. dst_unit : Unit or UnitProduct The parsed target unit. + step : int | None + For multi-step chains, the 0-indexed step where the error occurred. Returns ------- @@ -294,6 +350,7 @@ def build_dimension_mismatch_error( f"{src_dim_name} is not compatible with {dst_dim_name}", error_type="dimension_mismatch", parameter="to_unit", + step=step, got=src_dim_name, expected=src_dim_name, # Expected same dimension as source hints=hints, @@ -306,6 +363,7 @@ def build_no_path_error( src_unit, dst_unit, exception: Exception, + step: int | None = None, ) -> ConversionError: """Build a ConversionError for a missing conversion path. @@ -321,6 +379,8 @@ def build_no_path_error( The parsed target unit. exception : Exception The ConversionNotFound exception. + step : int | None + For multi-step chains, the 0-indexed step where the error occurred. Returns ------- @@ -382,12 +442,52 @@ def build_no_path_error( error=f"No conversion path from '{from_unit_str}' to '{to_unit_str}'", error_type="no_conversion_path", parameter=None, + step=step, got=src_dim_name, expected=dst_dim_name, hints=hints, ) +def build_parse_error( + bad_expression: str, + error_message: str, + parameter: str, + step: int | None = None, +) -> ConversionError: + """Build a ConversionError for a malformed unit expression. + + Parameters + ---------- + bad_expression : str + The malformed unit string (e.g., "W/(m²*K" with unbalanced parens). + error_message : str + The parse error message from the parser. + parameter : str + Which parameter was bad (e.g., "from_unit", "to_unit"). + step : int | None + For multi-step chains, the 0-indexed step where the error occurred. + + Returns + ------- + ConversionError + Structured error with parse error details. + """ + hints = [ + f"Parse error: {error_message}", + "Check for unbalanced parentheses or invalid characters", + "Valid syntax: m/s, kg*m/s^2, W/(m²·K)", + ] + + return ConversionError( + error=f"Cannot parse unit expression: '{bad_expression}'", + error_type="parse_error", + parameter=parameter, + step=step, + hints=hints, + ) + + def build_unknown_dimension_error(bad_dimension: str) -> ConversionError: """Build a ConversionError for an unknown dimension filter. @@ -433,8 +533,10 @@ def build_unknown_dimension_error(bad_dimension: str) -> ConversionError: __all__ = [ "ConversionError", + "resolve_unit", "build_unknown_unit_error", "build_dimension_mismatch_error", "build_no_path_error", + "build_parse_error", "build_unknown_dimension_error", ] diff --git a/ucon/units.py b/ucon/units.py index ab90a69..4b186df 100644 --- a/ucon/units.py +++ b/ucon/units.py @@ -47,7 +47,7 @@ def __init__(self, name: str): # -- International System of Units (SI) -------------------------------- -ampere = Unit(name='ampere', dimension=Dimension.current, aliases=('I', 'amp')) +ampere = Unit(name='ampere', dimension=Dimension.current, aliases=('A', 'I', 'amp')) becquerel = Unit(name='becquerel', dimension=Dimension.frequency, aliases=('Bq',)) celsius = Unit(name='celsius', dimension=Dimension.temperature, aliases=('°C', 'degC')) coulomb = Unit(name='coulomb', dimension=Dimension.charge, aliases=('C',)) @@ -58,6 +58,7 @@ def __init__(self, name: str): hertz = Unit(name='hertz', dimension=Dimension.frequency, aliases=('Hz',)) joule = Unit(name='joule', dimension=Dimension.energy, aliases=('J',)) joule_per_kelvin = Unit(name='joule_per_kelvin', dimension=Dimension.entropy, aliases=('J/K',)) +katal = Unit(name='katal', dimension=Dimension.catalytic_activity, aliases=('kat',)) kelvin = Unit(name='kelvin', dimension=Dimension.temperature, aliases=('K',)) kilogram = Unit(name='kilogram', dimension=Dimension.mass, aliases=('kg',)) liter = Unit(name='liter', dimension=Dimension.volume, aliases=('L', 'l')) @@ -220,6 +221,28 @@ def have(name: str) -> bool: _UNIT_REGISTRY: Dict[str, Unit] = {} _UNIT_REGISTRY_CASE_SENSITIVE: Dict[str, Unit] = {} +# ----------------------------------------------------------------------------- +# Priority Alias Invariant (for contributors) +# ----------------------------------------------------------------------------- +# +# When a unit alias could be misinterpreted as a scale prefix + unit symbol, +# add it to _PRIORITY_ALIASES or _PRIORITY_SCALED_ALIASES to prevent ambiguity. +# +# Examples: +# - "min" could parse as milli-inch (m + in), but should be minute +# - "mcg" could fail (no "mc" prefix), but should be microgram +# - "cc" could fail, but should be cubic centimeter (cm³) +# +# Rule: If a unit string starts with a valid scale prefix AND the remainder +# is a valid unit symbol, check whether the whole string should be treated +# as a single unit. If so, add it to: +# +# _PRIORITY_ALIASES - for unscaled units (e.g., "min" -> minute) +# _PRIORITY_SCALED_ALIASES - for scaled units (e.g., "mcg" -> microgram) +# +# The parser checks these sets BEFORE attempting prefix decomposition. +# ----------------------------------------------------------------------------- + # Priority aliases that must match exactly before prefix decomposition. # Prevents ambiguous parses like "min" -> milli-inch instead of minute. _PRIORITY_ALIASES: set = {'min'}