From 93b82ad593abc30f549347fe588b022f514271d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 12:40:46 +0000 Subject: [PATCH] Notebook form: no silent number clamp (explicit 2**53-1 limit, shown and refused), optional parameters can be left unset Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01Y2SN8SuEakTdiWy3F2WUqN --- docs/PROTOCOL.md | 4 + docs/REGRESSION_LEDGER.md | 2 +- labconstrictor_tools/notebook.py | 103 ++++++++++++++++++++----- tests/roundtrip_cases.py | 7 +- tests/roundtrip_harness.py | 2 + tests/roundtrip_known_failures.py | 30 ------- tests/test_notebook_unset_and_range.py | 83 ++++++++++++++++++++ 7 files changed, 179 insertions(+), 52 deletions(-) create mode 100644 tests/test_notebook_unset_and_range.py diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 8be37f7..1f1ddb7 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -21,6 +21,10 @@ Value rules (enforced by the worker, each violation is a `FAILURE` with code `in Input `type`: `string integer float boolean choice image labels table file folder`. `nullable: true` (additive, protocol 1): the parameter is optional AND has no default value (`= None` / `Optional[...]`). A host must let the user leave it unset and then omit it from the request (the tool receives `None`); showing 0 or an empty string instead is a bug. +The notebook form does this with a "set" check box beside each nullable number, text, choice or yes/no (unchecked = greyed out and left out). +Number limits: the protocol, the command line and snippets carry integers of any size and floats up to the double range. A number box +never clamps silently. The notebook form has no made-up bound; because the page exchanges JSON numbers, an integer beyond +-(2**53 - 1) +(`notebook.MAX_FORM_INTEGER`) is refused with an `invalid_parameter` message, and the limit is in the control's tooltip. Declared `Min`/`Max` are enforced. `group` (heading), `advanced: true` (listed under collapsed "Advanced settings") and `enabled_when: {"param", "equals"?}` (the parameter only applies when `param` is set/true, or equals one of the listed values) are presentation hints, additive within protocol 1: a host that ignores them shows the flat form, so a tool must accept every parameter whether or not it is "enabled". Group names are shown where they first appear. Interaction hints (additive within protocol 1; a host that ignores them shows a plain form, so the tool must work either way): * `group_collapsed: true` (with `group`): the group is an accordion section that starts folded. diff --git a/docs/REGRESSION_LEDGER.md b/docs/REGRESSION_LEDGER.md index 87057bb..1cfb03c 100644 --- a/docs/REGRESSION_LEDGER.md +++ b/docs/REGRESSION_LEDGER.md @@ -29,7 +29,7 @@ a finding deletes its entries there and moves its row up to the table above. | 2026-10-08 | F5: a tool that returns the wrong kind of value for an output gets a raw Python exception code | `build_results` and its writers trust the shape and type of what a tool returns | pending | `test_roundtrip_failures.py::WhatAToolReturns.test_the_wrong_type_is_reported_with_a_tool_error_code` | | 2026-10-08 | F6: an Affine output with NaN or infinity is delivered as null | `_as_3x3` does not check finiteness (points and shapes do) | pending | `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`echo_affine/invalid/nan-cell`) | | 2026-10-08 | F7: table text cells such as `NA`, `null`, `None` arrive as NaN | pandas default NA values | pending | `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`echo_table/text-that-pandas-calls-missing`) | -| 2026-10-08 | F8, F9: the notebook form clamps numbers beyond 10**9 and cannot leave an optional parameter unset (the tool receives 0, empty text, False, the first choice) | `BoundedIntText` / `Text` / `Dropdown` controls always hold a value | pending | `test_roundtrip_matrix.py::Matrix.test_notebook_form` (`echo_optional_*/omitted`, `echo_int/ok/2147483648`) | +| 2026-10-08 | F8, F9: the notebook form clamps numbers beyond 10**9 and cannot leave an optional parameter unset (the tool receives 0, empty text, False, the first choice) | `BoundedIntText` / `Text` / `Dropdown` controls always hold a value | this PR | `test_roundtrip_matrix.py::Matrix.test_notebook_form` (`echo_optional_*/omitted`, `echo_int/ok/2147483648`), `test_notebook_unset_and_range.py` | | 2026-10-08 | F10: a copied Windows command line changes a text with backslashes before a quote | `command.quote(windows=True)` does not double backslashes in front of a quote | pending | `test_roundtrip_command.py::QuotingHelper.test_windows_quoting_keeps_every_text_one_argument`, `test_roundtrip_command.py::QuotingHelper.test_windows_quoting_survives_the_real_process_start` (the Windows jobs of CI show it on a real process start) | | 2026-10-08 | F11, F12: a malformed shared-memory descriptor, or a non-text file/folder value, raises KeyError / FileNotFoundError / TypeError | no validation before `SharedMemory(...)` and `Path(...)` | pending | `test_roundtrip_properties.py::Properties.test_only_toolerror_image_shared_memory`, `test_roundtrip_properties.py::Properties.test_only_toolerror_file_value` | | 2026-10-08 | F13: the worker destroys the host's shared-memory block when it exits (and says so in crash messages) | Python's resource tracker unlinks every block the worker attached to | pending | `test_roundtrip_shared_memory.py::SharedMemoryOwnership.test_the_block_and_the_stderr_are_left_alone` | diff --git a/labconstrictor_tools/notebook.py b/labconstrictor_tools/notebook.py index 2500549..e421aab 100644 --- a/labconstrictor_tools/notebook.py +++ b/labconstrictor_tools/notebook.py @@ -17,10 +17,16 @@ from .introspection import describe_tool from .structures import ParamSchema, Result -# bounds of a number box whose parameter declares no Min / Max (the widget needs finite limits) -UNBOUNDED_INT = 10**9 -UNBOUNDED_FLOAT = 1e9 +# A number box whose parameter declares no Min / Max has NO made-up limit (it used to clamp silently at 1e9). The one real +# limit is the browser's: the form talks to the page as JSON numbers (IEEE doubles), so an integer beyond 2**53 - 1 would +# be shown and sent back rounded. Such an integer is therefore REFUSED with a message (never changed), and the limit is +# written in the control's tooltip. Floats are doubles on both sides and need no limit. The worker protocol, the command +# line and the snippets carry integers of any size; the other hosts' limits are listed in docs/PROTOCOL.md. +MAX_FORM_INTEGER = 2**53 - 1 FLOAT_STEP = 1e-4 # arrow-key step of a float box +SET_LABEL = ( + "set" # the check box that turns an optional parameter on (unchecked = unset: the tool receives None) +) class ToolForm: @@ -39,6 +45,11 @@ def __init__(self, function: Callable[..., Any]) -> None: self.function = function self.schema = describe_tool(function.__lc_tool__) self.controls = {p["name"]: self._control(p) for p in self.schema["inputs"]} + # optional parameters without a default (nullable) get a "set" box; unchecked = the control is greyed out and the + # parameter is left out of the request, so the tool receives None (as on the command line and in Napari/Fiji/QuPath) + self.set_toggles = { + p["name"]: self._set_toggle(p["name"]) for p in self.schema["inputs"] if self._is_unsettable(p) + } self.run_button = widgets.Button(description="Run", button_style="primary") self.progress = widgets.FloatProgress( min=0, max=1, layout=widgets.Layout(width="100%", visibility="hidden") @@ -50,10 +61,48 @@ def __init__(self, function: Callable[..., Any]) -> None: "%s
%s" % (self.schema["label"], self.schema.get("description", "")) ) self.widget = widgets.VBox( - [title, *self.controls.values(), self.run_button, self.progress, self.status, self.output] + [title, *self._rows(), self.run_button, self.progress, self.status, self.output] ) self.results: list[Result] | None = None + # ---- optional ("not set") parameters + @staticmethod + def _is_unsettable(param: ParamSchema) -> bool: + """Why: only a nullable number, text, choice or yes/no needs a 'not set' state; a path box is unset when it is empty.""" + return bool(param.get("nullable")) and param["type"] in ( + "integer", + "float", + "string", + "choice", + "boolean", + ) + + def _set_toggle(self, name: str) -> Any: + """The 'set' check box of one optional parameter (starts unchecked, so the parameter starts unset).""" + toggle = self._w.Checkbox( + value=False, + description=SET_LABEL, + indent=False, + tooltip="Optional: leave unchecked to not set this value (the tool then receives nothing)", + layout=self._w.Layout(width="auto"), + ) + control = self.controls[name] + control.disabled = True + + def apply(change: Any) -> None: + control.disabled = not change["new"] + + toggle.observe(apply, names="value") + return toggle + + def _rows(self) -> list[Any]: + """The form rows: each control, with its 'set' box beside it when the parameter is optional.""" + rows = [] + for name, control in self.controls.items(): + toggle = self.set_toggles.get(name) + rows.append(control if toggle is None else self._w.HBox([control, toggle])) + return rows + # ---- controls (same type mapping as the other hosts) def _control(self, param: ParamSchema) -> Any: w, kind = self._w, param["type"] @@ -72,20 +121,26 @@ def _control(self, param: ParamSchema) -> Any: }.get(kind, "") control = w.Text(value="" if default is None else str(default), placeholder=hint, **common) elif kind == "integer": - control = w.BoundedIntText( - value=int(default or 0), - min=param.get("minimum", -UNBOUNDED_INT), - max=param.get("maximum", UNBOUNDED_INT), - **common, - ) + if ( + "minimum" in param and "maximum" in param + ): # both limits declared by the tool: the box enforces them + control = w.BoundedIntText( + value=int(default or 0), min=param["minimum"], max=param["maximum"], **common + ) + else: # one or no limit: no made-up bound; a declared one is checked by the shared validation at run time + control = w.IntText(value=int(default or 0), **common) + tooltip = (tooltip + " " if tooltip else "") + "Whole numbers up to +-%d." % MAX_FORM_INTEGER elif kind == "float": - control = w.BoundedFloatText( - value=float(default or 0), - min=param.get("minimum", -UNBOUNDED_FLOAT), - max=param.get("maximum", UNBOUNDED_FLOAT), - step=FLOAT_STEP, - **common, - ) + if "minimum" in param and "maximum" in param: + control = w.BoundedFloatText( + value=float(default or 0), + min=param["minimum"], + max=param["maximum"], + step=FLOAT_STEP, + **common, + ) + else: + control = w.FloatText(value=float(default or 0), step=FLOAT_STEP, **common) elif kind == "boolean": control = w.Checkbox(value=bool(default), **{**common, "layout": w.Layout()}) elif kind == "choice": @@ -100,10 +155,20 @@ def _control(self, param: ParamSchema) -> Any: return control def values(self) -> dict[str, Any]: - """Control values -> the inputs the worker would receive (empty optional paths are left out).""" + """Control values -> the inputs the worker would receive (empty optional paths and unset optional parameters are + left out). Raises ToolError for an integer the form cannot hold exactly (never clamps).""" inputs: dict[str, Any] = {} for param in self.schema["inputs"]: - value = self.controls[param["name"]].value + name = param["name"] + if name in self.set_toggles and not self.set_toggles[name].value: + continue # optional and not set: the tool receives None + value = self.controls[name].value + if param["type"] == "integer" and abs(value) > MAX_FORM_INTEGER: + raise T.ToolError( + "invalid_parameter", + "'%s' is %d, beyond the +-%d the form can hold exactly: type a smaller number, or run the tool " + "from the command line or a snippet" % (param["label"], value, MAX_FORM_INTEGER), + ) if param["type"] in ("image", "labels", "table", "file", "folder") and not str(value).strip(): continue inputs[param["name"]] = value diff --git a/tests/roundtrip_cases.py b/tests/roundtrip_cases.py index 6b92e92..9ae4064 100644 --- a/tests/roundtrip_cases.py +++ b/tests/roundtrip_cases.py @@ -488,6 +488,9 @@ def labels_cases() -> list[Case]: # ------------------------------------------------------------------------------------------------------ scalars +MAX_FORM_INTEGER = ( + 2**53 - 1 +) # oracle: the largest integer the notebook form holds exactly (a JSON number); beyond it the form refuses INT_OK = [ 0, 1, @@ -625,7 +628,7 @@ def bad( v, v, "int", - all_paths if abs(v) <= 2**31 else ("worker", "cli", "snippet"), + all_paths if abs(v) <= MAX_FORM_INTEGER else ("worker", "cli", "snippet"), ) for v in (3.0, -2.0, 1000.0, 0.0, -0.0): ok("integral-float/%r" % v, "echo_int", v, int(v), "int", ("worker", "snippet")) @@ -678,7 +681,7 @@ def bad( v, v, "float", - all_paths if (abs(v) < 1e9 or v == 1e15) else ("worker", "cli", "snippet"), + all_paths, ) for v in (3, -7, 0, 10**20, 2**53 + 1, 10**300): ok( diff --git a/tests/roundtrip_harness.py b/tests/roundtrip_harness.py index 2dd543a..fd0c600 100644 --- a/tests/roundtrip_harness.py +++ b/tests/roundtrip_harness.py @@ -365,6 +365,8 @@ def run_notebook(session: Session, case: Case, wire: dict[str, Any], folder: Pat form = ToolForm(getattr(module, case.tool)) for name, value in wire.items(): form.controls[name].value = value + if name in form.set_toggles: # an optional parameter the case sets: tick its "set" box + form.set_toggles[name].value = True old = tempfile.tempdir tempfile.tempdir = str( folder diff --git a/tests/roundtrip_known_failures.py b/tests/roundtrip_known_failures.py index 18faf42..601ff14 100644 --- a/tests/roundtrip_known_failures.py +++ b/tests/roundtrip_known_failures.py @@ -84,20 +84,6 @@ class Finding: "documented in docs/PROTOCOL.md", "silent loss of the cell's content (a gene called NA, a status 'None')", ), - "F8": Finding( - "The notebook form clamps integers and floats beyond 10**9 without telling the person", - "labconstrictor_tools.notebook.ToolForm(echo_int).controls['value'].value = 2**31; form.run() reports 1000000000", - "the value is kept (a text box for unbounded numbers) or the form refuses it with a message", - "BoundedIntText/BoundedFloatText with the made-up limits UNBOUNDED_INT / UNBOUNDED_FLOAT silently change the value", - ), - "F9": Finding( - "The notebook form cannot leave an optional parameter unset", - "ToolForm(echo_optional_int): run() without touching the control sends 0; the string sends ''; the bool False; the " - "choice its first option", - "an unset optional parameter is omitted from the inputs so the tool receives None (docs/PROTOCOL.md: 'showing 0 or an " - "empty string instead is a bug')", - "the tool receives 0 / '' / False / the first option: 'unset' cannot be expressed in the notebook", - ), "F10": Finding( "command.quote(windows=True) breaks texts with backslashes before a quote", 'command.quote(\'x y\\\\"z\', windows=True) and command.quote("C:\\\\My Data\\\\", windows=True): parse the result with ' @@ -280,22 +266,6 @@ def _add(finding: str, *keys: str) -> None: "terminal:echo_table/text-that-pandas-calls-missing", "worker:echo_table/text-that-pandas-calls-missing", ) -_add( - "F8", - "notebook:echo_float/ok/0x1.c6bf526340000p+49", - "notebook:echo_int/ok/-2147483648", - "notebook:echo_int/ok/2147483647", - "notebook:echo_int/ok/2147483648", - "notebook:echo_optional_int/set/1099511627776", -) -_add( - "F9", - "notebook:echo_optional_bool/omitted", - "notebook:echo_optional_choice/omitted", - "notebook:echo_optional_float/omitted", - "notebook:echo_optional_int/omitted", - "notebook:echo_optional_string/omitted", -) _add( "F10", "winquote:quote-after-backslashes", diff --git a/tests/test_notebook_unset_and_range.py b/tests/test_notebook_unset_and_range.py new file mode 100644 index 0000000..99f34da --- /dev/null +++ b/tests/test_notebook_unset_and_range.py @@ -0,0 +1,83 @@ +"""Notebook form: an optional parameter can be left unset (F9) and numbers are never clamped silently (F8).""" + +import unittest +from typing import Annotated + +import _paths # noqa: F401 (must come first) + +from labconstrictor_tools import Max, Min, Scalars, tool +from labconstrictor_tools.notebook import MAX_FORM_INTEGER, ToolForm + +try: + import ipywidgets # noqa: F401 + + HAVE_WIDGETS = True +except ImportError: + HAVE_WIDGETS = False + + +@tool("Optional things") +def optional_things( + n: int | None = None, + x: float | None = None, + s: str | None = None, + flag: bool | None = None, + big: int = 0, + bounded: Annotated[int, Min(0), Max(10)] = 1, +) -> Scalars: + return {"n": n, "x": x, "s": s, "flag": flag, "big": big, "bounded": bounded} + + +@unittest.skipUnless(HAVE_WIDGETS, "ipywidgets not installed") +class UnsetAndRange(unittest.TestCase): + def received(self, form): + results = form.run() + self.assertIsNotNone(results, form.status.value) + return results[0]["values"] + + def test_optional_parameters_start_unset_and_the_tool_receives_none(self): + form = ToolForm(optional_things) + self.assertEqual(set(form.set_toggles), {"n", "x", "s", "flag"}) + self.assertTrue(all(form.controls[k].disabled for k in form.set_toggles)) + got = self.received(form) + self.assertEqual([got[k] for k in ("n", "x", "s", "flag")], [None] * 4) + + def test_ticking_set_sends_the_value_even_zero_empty_or_false(self): + form = ToolForm(optional_things) + for name in form.set_toggles: + form.set_toggles[name].value = True + self.assertFalse(form.controls[name].disabled) + got = self.received(form) + self.assertEqual([got[k] for k in ("n", "x", "s", "flag")], [0, 0.0, "", False]) + form.set_toggles["n"].value = False + self.assertTrue(form.controls["n"].disabled) + self.assertIsNone(self.received(form)["n"]) + + def test_required_parameters_have_no_set_box(self): + form = ToolForm(optional_things) + self.assertNotIn("big", form.set_toggles) + self.assertFalse(form.controls["big"].disabled) + + def test_numbers_beyond_a_billion_are_kept(self): + form = ToolForm(optional_things) + form.controls["big"].value = 3 * 10**9 + self.assertEqual(self.received(form)["big"], 3 * 10**9) + + def test_an_integer_the_form_cannot_hold_exactly_is_refused_with_a_message(self): + form = ToolForm(optional_things) + form.controls["big"].value = MAX_FORM_INTEGER + 1 + self.assertIsNone(form.run()) + self.assertIn("invalid_parameter", form.status.value) + self.assertIn(str(MAX_FORM_INTEGER), form.status.value) + form.controls["big"].value = MAX_FORM_INTEGER + self.assertEqual(self.received(form)["big"], MAX_FORM_INTEGER) + + def test_a_declared_min_max_pair_is_still_enforced_by_the_box(self): + form = ToolForm(optional_things) + self.assertEqual(type(form.controls["bounded"]).__name__, "BoundedIntText") + form.controls["bounded"].value = 99 + self.assertEqual(form.controls["bounded"].value, 10) + + +if __name__ == "__main__": + unittest.main()