From 9064e3e532e3361de6198ce04746cf836776e8b4 Mon Sep 17 00:00:00 2001
From: Julian Stirling <julian@julianstirling.co.uk>
Date: Thu, 26 Feb 2026 11:01:12 +0000
Subject: [PATCH 01/10] Failing test for generics in setting types when loading

---
 tests/test_settings.py | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/tests/test_settings.py b/tests/test_settings.py
index afd5bd5a..a97618af 100644
--- a/tests/test_settings.py
+++ b/tests/test_settings.py
@@ -40,9 +40,11 @@ def __init__(self, **kwargs: Any) -> None:
     stringsetting: str = lt.setting(default="foo")
     "A string setting."
 
-    dictsetting: dict = lt.setting(default_factory=lambda: {"a": 1, "b": 2})
+    dictsetting: dict["str", int] = lt.setting(default_factory=lambda: {"a": 1, "b": 2})
     "A dictionary setting."
 
+    tuplesetting: tuple[int, int] = lt.setting(default_factory=lambda: (1, 2))
+
     modelsetting: MyModel = lt.setting(default_factory=lambda: MyModel(a=0, b="string"))
     "A setting that is a BaseModel."
 
@@ -114,6 +116,7 @@ def _settings_dict(
     floatsetting=1.0,
     stringsetting="foo",
     dictsetting=None,
+    tuplesetting=None,
     modelsetting=None,
     localonlysetting="Local-only default.",
     localonly_boolsetting=False,
@@ -126,11 +129,14 @@ def _settings_dict(
         dictsetting = {"a": 1, "b": 2}
     if modelsetting is None:
         modelsetting = {"a": 0, "b": "string"}
+    if tuplesetting is None:
+        tuplesetting = (1, 2)
     return {
         "boolsetting": boolsetting,
         "floatsetting": floatsetting,
         "stringsetting": stringsetting,
         "dictsetting": dictsetting,
+        "tuplesetting": list(tuplesetting),  # Convert to list so json matches.
         "modelsetting": modelsetting,
         "localonlysetting": localonlysetting,
         "localonly_boolsetting": localonly_boolsetting,
@@ -245,22 +251,33 @@ def test_settings_dict_internal_update(tempdir):
         assert not os.path.isfile(setting_file)
 
 
-def test_settings_load(tempdir):
+def test_settings_load(tempdir, caplog):
     """Check settings can be loaded from disk when added to server"""
     server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir)
     setting_file = _get_setting_file(server, "thing")
     del server
-    setting_json = json.dumps(_settings_dict(floatsetting=3.0, stringsetting="bar"))
+    setting_json = json.dumps(
+        _settings_dict(
+            floatsetting=3.0,
+            stringsetting="bar",
+            dictsetting={"c": 3},
+            tuplesetting=(77, 22),
+        )
+    )
     # Create setting file
     with open(setting_file, "w", encoding="utf-8") as file_obj:
         file_obj.write(setting_json)
-    # Add thing to server and check new settings are loaded
-    server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir)
+    with caplog.at_level(logging.WARNING):
+        # Add thing to server and check new settings are loaded
+        server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir)
+    assert len(caplog.records) == 0
     thing = server.things["thing"]
     assert isinstance(thing, ThingWithSettings)
     assert not thing.boolsetting
     assert thing.stringsetting == "bar"
     assert thing.floatsetting == 3.0
+    assert thing.dictsetting == {"c": 3}
+    assert thing.tuplesetting == (77, 22)
 
 
 def test_load_extra_settings(caplog, tempdir):

From 29a71f6f54e08c592b58f06b6d94f29e7c222f25 Mon Sep 17 00:00:00 2001
From: Julian Stirling <julian@julianstirling.co.uk>
Date: Thu, 26 Feb 2026 13:35:24 +0000
Subject: [PATCH 02/10] Bugfix for setting settings of Generic type

---
 src/labthings_fastapi/properties.py | 18 +++++++++++++-----
 src/labthings_fastapi/thing.py      |  2 ++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index 4eda6517..a16736b9 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -872,12 +872,20 @@ def model_to_value(self, value: BaseModel) -> Value:
             type.
         :raises TypeError: if the supplied value cannot be converted to the right type.
         """
-        if isinstance(value, self.value_type):
+        # If it is a root model unrap the value
+        value = value.root if isinstance(value, RootModel) else value
+        try:
+            if isinstance(value, self.value_type):
+                return value
+        except TypeError:
+            # In the case that the self.value_type is a typing.GenericAlias or some
+            # complicated associated type like typing._UnionGenericAlias then
+            # isinstace itself with throw a TypeError.
+            # Instead create root model for validation.
+            ValidationModel = RootModel[self.value_type]
+            # Use the model to validate the value before setting.
+            ValidationModel(value)
             return value
-        elif isinstance(value, RootModel):
-            root = value.root
-            if isinstance(root, self.value_type):
-                return root
         msg = f"Model {value} isn't {self.value_type} or a RootModel wrapping it."
         raise TypeError(msg)
 
diff --git a/src/labthings_fastapi/thing.py b/src/labthings_fastapi/thing.py
index 2f245714..0795c324 100644
--- a/src/labthings_fastapi/thing.py
+++ b/src/labthings_fastapi/thing.py
@@ -237,6 +237,8 @@ def load_settings(self) -> None:
                         f"An extra key {name} was found in the settings file. "
                         "It will be deleted the next time settings are saved."
                     )
+                except TypeError:
+                    self.logger.warning(f"Failed to load {name} with a TypeError.")
         except (
             FileNotFoundError,
             JSONDecodeError,

From ed251b2dcfad4dc94e284dd5b5b5552cf927ac5f Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Fri, 27 Feb 2026 01:42:14 +0000
Subject: [PATCH 03/10] Neater fix for validating property input

Rather than separately load a model, then set the property from a model (which requires some checks that are a bit fragile), I've refactored to add a `validate` method to `PropertyInfo`.

This creates and then (if needed) unpacks a model in one function, removing the need to check the model is the correct model.

I've added a few extra tests to make sure `validate` works for the cases I missed when releasing v0.0.15.

I think the logic in `validate` is much simpler and more robust than the isinstance checks that caused the problem.
---
 src/labthings_fastapi/properties.py         | 74 +++++++++++----------
 src/labthings_fastapi/thing.py              |  3 +-
 src/labthings_fastapi/utilities/__init__.py | 19 +++++-
 tests/test_property.py                      | 39 +++++++++--
 tests/test_settings.py                      | 17 +++--
 5 files changed, 104 insertions(+), 48 deletions(-)

diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index a16736b9..d92b03e6 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -62,7 +62,7 @@ class attribute. Documentation is in strings immediately following the
 from weakref import WeakSet
 
 from fastapi import Body, FastAPI
-from pydantic import BaseModel, ConfigDict, RootModel, create_model
+from pydantic import BaseModel, ConfigDict, RootModel, ValidationError, create_model
 
 from .thing_description import type_to_dataschema
 from .thing_description._model import (
@@ -71,7 +71,11 @@ class attribute. Documentation is in strings immediately following the
     PropertyAffordance,
     PropertyOp,
 )
-from .utilities import labthings_data, wrap_plain_types_in_rootmodel
+from .utilities import (
+    LabThingsRootModelWrapper,
+    labthings_data,
+    wrap_plain_types_in_rootmodel,
+)
 from .utilities.introspection import return_type
 from .base_descriptor import (
     DescriptorInfoCollection,
@@ -859,35 +863,44 @@ def model_instance(self) -> BaseModel:  # noqa: DOC201
                 raise TypeError(msg)
             return cls(root=value)
 
-    def model_to_value(self, value: BaseModel) -> Value:
-        r"""Convert a model to a value for this property.
+    def validate(self, value: Any) -> Value:
+        """Use the validation logic in `self.model`.
+
+        This method should accept anything that `pydantic` can convert to the
+        right type, and return a correctly-typed value. It also enforces any
+        constraints that were set on the property.
 
-        Even properties with plain types are sometimes converted to or from a
-        `pydantic.BaseModel` to allow conversion to/from JSON. This is a convenience
-        method that accepts a model (which should be an instance of ``self.model``\ )
-        and unwraps it when necessary to get the plain Python value.
+        :param value: The new value, in any form acceptable to the property's
+            model. Usually this means you can use either the correct type, or
+            the value as loaded from JSON.
 
-        :param value: A `.BaseModel` instance to convert.
-        :return: the value, with `.RootModel` unwrapped so it matches the descriptor's
-            type.
-        :raises TypeError: if the supplied value cannot be converted to the right type.
+        :return: the new value, with the correct type.
+
+        :raises ValidationError: if the supplied value can't be loaded by
+            the property's model. This is the exception raised by ``model_validate``
+        :raises TypeError: if the property has a ``model`` that's inconsistent
+            with its value type. This should never happen.
         """
-        # If it is a root model unrap the value
-        value = value.root if isinstance(value, RootModel) else value
         try:
-            if isinstance(value, self.value_type):
-                return value
-        except TypeError:
-            # In the case that the self.value_type is a typing.GenericAlias or some
-            # complicated associated type like typing._UnionGenericAlias then
-            # isinstace itself with throw a TypeError.
-            # Instead create root model for validation.
-            ValidationModel = RootModel[self.value_type]
-            # Use the model to validate the value before setting.
-            ValidationModel(value)
-            return value
-        msg = f"Model {value} isn't {self.value_type} or a RootModel wrapping it."
-        raise TypeError(msg)
+            if self.model is self.value_type and issubclass(self.value_type, BaseModel):
+                # If there's no RootModel wrapper, the value_type is a model and may be
+                # used directly for validation. The `issubclass` should not be
+                # necessary, but it's helpful for `mypy` and it does no harm to check.
+                return self.value_type.model_validate(value)
+            elif issubclass(self.model, LabThingsRootModelWrapper):
+                # If a plain type has been wrapped in a RootModel, use that to validate
+                # and then set the property to the root value.
+                model = self.model.model_validate(value)
+                return model.root
+            else:
+                # This should be unreachable, because either `self.value_type` is
+                # a BaseModel and so `value_type` and `model` are identical, or
+                # `model` is a `LabThingsRootModelWrapper` wrapping the value type.
+                msg = f"Property {self.name} has an inconsistent model. This is "
+                msg += f"most likely a LabThings bug. {self.model=}, {self.value_type=}"
+                raise TypeError(msg)
+        except ValidationError:
+            raise  # This is needed for flake8 to be happy with the docstring
 
 
 class PropertyCollection(DescriptorInfoCollection[Owner, PropertyInfo], Generic[Owner]):
@@ -1147,13 +1160,6 @@ def set_without_emit(self, value: Value) -> None:
         obj = self.owning_object_or_error()
         self.get_descriptor().set_without_emit(obj, value)
 
-    def set_without_emit_from_model(self, value: BaseModel) -> None:
-        """Set the value from a model instance, unwrapping RootModels as needed.
-
-        :param value: the model to extract the value from.
-        """
-        self.set_without_emit(self.model_to_value(value))
-
 
 class SettingCollection(DescriptorInfoCollection[Owner, SettingInfo], Generic[Owner]):
     """Access to metadata on all the properties of a `.Thing` instance or subclass.
diff --git a/src/labthings_fastapi/thing.py b/src/labthings_fastapi/thing.py
index 0795c324..aca9a5f4 100644
--- a/src/labthings_fastapi/thing.py
+++ b/src/labthings_fastapi/thing.py
@@ -224,8 +224,7 @@ def load_settings(self) -> None:
                 try:
                     setting = self.settings[name]
                     # Load the key from the JSON file using the setting's model
-                    model = setting.model.model_validate(value)
-                    setting.set_without_emit_from_model(model)
+                    setting.set_without_emit(setting.validate(value))
                 except ValidationError:
                     self.logger.warning(
                         f"Could not load setting {name} from settings file "
diff --git a/src/labthings_fastapi/utilities/__init__.py b/src/labthings_fastapi/utilities/__init__.py
index d1942d1f..13173b4b 100644
--- a/src/labthings_fastapi/utilities/__init__.py
+++ b/src/labthings_fastapi/utilities/__init__.py
@@ -2,7 +2,7 @@
 
 from __future__ import annotations
 from collections.abc import Mapping
-from typing import Any, Dict, Iterable, TYPE_CHECKING, Optional
+from typing import Any, Dict, Generic, Iterable, TYPE_CHECKING, Optional, TypeVar
 from weakref import WeakSet
 from pydantic import BaseModel, ConfigDict, Field, RootModel, create_model
 from pydantic.dataclasses import dataclass
@@ -95,6 +95,21 @@ def labthings_data(obj: Thing) -> LabThingsObjectData:
     return obj.__dict__[LABTHINGS_DICT_KEY]
 
 
+WrappedT = TypeVar("WrappedT")
+
+
+class LabThingsRootModelWrapper(RootModel[WrappedT], Generic[WrappedT]):
+    """A RootModel subclass for automatically-wrapped types.
+
+    There are several places where LabThings needs a model, but may only
+    have a plain Python type. This subclass indicates to LabThings that
+    a type has been automatically wrapped, and will need to be unwrapped
+    in order for the value to have the correct type.
+
+    It has no additional functionality.
+    """
+
+
 def wrap_plain_types_in_rootmodel(
     model: type, constraints: Mapping[str, Any] | None = None
 ) -> type[BaseModel]:
@@ -131,7 +146,7 @@ def wrap_plain_types_in_rootmodel(
         return create_model(
             f"{model!r}",
             root=(model, Field(**constraints)),
-            __base__=RootModel,
+            __base__=LabThingsRootModelWrapper,
         )
     except SchemaError as e:
         for error in e.errors():
diff --git a/tests/test_property.py b/tests/test_property.py
index 4bcd97e3..3125f418 100644
--- a/tests/test_property.py
+++ b/tests/test_property.py
@@ -308,9 +308,15 @@ class Example(lt.Thing):
         intprop: int = lt.property(default=0)
         """A normal, simple, integer property."""
 
+        positive: int = lt.property(default=0, gt=0)
+        """A positive integer property."""
+
         badprop: int = lt.property(default=1)
         """An integer property that I will break later."""
 
+        tupleprop: tuple[int, str] = lt.property(default=(42, "the answer"))
+        """A tuple property, to check subscripted generics work."""
+
     # We will break `badprop` by setting its model to something that's
     # neither the type nor a rootmodel.
     badprop = Example.badprop
@@ -331,11 +337,15 @@ class BadIntModel(pydantic.BaseModel):
     assert isinstance(model, pydantic.RootModel)
     assert model.root == 15
 
-    # Check we can unwrap a RootModel correctly
-    IntModel = example.properties["intprop"].model
-    assert example.properties["intprop"].model_to_value(IntModel(root=17)) == 17
-    with pytest.raises(TypeError):
-        example.properties["intprop"].model_to_value(BadIntModel(root=17))
+    # Check we can validate properly
+    intprop = example.properties["intprop"]
+    assert intprop.validate(15) == 15  # integers pass straight through
+    assert intprop.validate(-15) == -15
+    # A RootModel instance ought still to validate
+    assert intprop.validate(intprop.model(root=42)) == 42
+    # A wrong model won't, though.
+    with pytest.raises(pydantic.ValidationError):
+        intprop.validate(BadIntModel(root=42))
 
     # Check that a broken `_model` raises the right error
     # See above for where we manually set badprop._model to something that's
@@ -344,6 +354,24 @@ class BadIntModel(pydantic.BaseModel):
     assert example.badprop == 3
     with pytest.raises(TypeError):
         _ = example.properties["badprop"].model_instance
+    with pytest.raises(TypeError):
+        # The value is fine, but the model has been set to an invalid type.
+        # This error shouldn't be seen in production.
+        example.properties["badprop"].validate(0)
+
+    # Check validation applies constraints
+    positive = example.properties["positive"]
+    assert positive.validate(42) == 42
+    with pytest.raises(pydantic.ValidationError):
+        positive.validate(0)
+
+    # Check validation works for subscripted generics
+    tupleprop = example.properties["tupleprop"]
+    assert tupleprop.validate((1, "two")) == (1, "two")
+
+    for val in [0, "str", ("str", 0)]:
+        with pytest.raises(pydantic.ValidationError):
+            tupleprop.validate(val)
 
 
 def test_readonly_metadata():
@@ -379,6 +407,7 @@ def _set_funcprop(self, val: int) -> None:
     example = create_thing_without_server(Example)
 
     td = example.thing_description()
+    assert td.properties is not None  # This is mostly for type checking
 
     # Check read-write properties are not read-only
     for name in ["prop", "funcprop"]:
diff --git a/tests/test_settings.py b/tests/test_settings.py
index a97618af..02abdfb9 100644
--- a/tests/test_settings.py
+++ b/tests/test_settings.py
@@ -41,13 +41,17 @@ def __init__(self, **kwargs: Any) -> None:
     "A string setting."
 
     dictsetting: dict["str", int] = lt.setting(default_factory=lambda: {"a": 1, "b": 2})
-    "A dictionary setting."
-
-    tuplesetting: tuple[int, int] = lt.setting(default_factory=lambda: (1, 2))
+    "A dictionary setting. This is a subscripted generic."
 
     modelsetting: MyModel = lt.setting(default_factory=lambda: MyModel(a=0, b="string"))
     "A setting that is a BaseModel."
 
+    tuplesetting: tuple[int, int] = lt.setting(default=(1, 2))
+    """A tuple setting that specifies component types.
+    
+    This is a "subscripted generic" and so has a tendency to break `isinstance` tests.
+    """
+
     @lt.setting
     def floatsetting(self) -> float:
         """A float setting."""
@@ -116,8 +120,8 @@ def _settings_dict(
     floatsetting=1.0,
     stringsetting="foo",
     dictsetting=None,
-    tuplesetting=None,
     modelsetting=None,
+    tuplesetting=(1, 2),
     localonlysetting="Local-only default.",
     localonly_boolsetting=False,
 ):
@@ -136,8 +140,10 @@ def _settings_dict(
         "floatsetting": floatsetting,
         "stringsetting": stringsetting,
         "dictsetting": dictsetting,
-        "tuplesetting": list(tuplesetting),  # Convert to list so json matches.
         "modelsetting": modelsetting,
+        # tuples and lists are indestinguishable in JSON, so we convert
+        # tuplesetting to a list so the comparison will work.
+        "tuplesetting": list(tuplesetting),
         "localonlysetting": localonlysetting,
         "localonly_boolsetting": localonly_boolsetting,
     }
@@ -160,6 +166,7 @@ def test_setting_available():
     assert thing.floatsetting == 1.0
     assert thing.localonlysetting == "Local-only default."
     assert thing.dictsetting == {"a": 1, "b": 2}
+    assert thing.tuplesetting == (1, 2)
     assert thing.modelsetting == MyModel(a=0, b="string")
 
 

From 8e5ba95bbcde09a6faeaec038bc436836bde8301 Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Fri, 27 Feb 2026 02:05:46 +0000
Subject: [PATCH 04/10] Add more test cases for `PropertyInfo.validate()`

Just to make sure, I'm checking a generic model and a BaseModel too.
---
 tests/test_property.py | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tests/test_property.py b/tests/test_property.py
index 3125f418..fe285dbd 100644
--- a/tests/test_property.py
+++ b/tests/test_property.py
@@ -304,6 +304,10 @@ def prop(self) -> bool:
 def test_propertyinfo(mocker):
     """Test out the PropertyInfo class."""
 
+    class MyModel(pydantic.BaseModel):
+        a: int
+        b: str
+
     class Example(lt.Thing):
         intprop: int = lt.property(default=0)
         """A normal, simple, integer property."""
@@ -317,6 +321,17 @@ class Example(lt.Thing):
         tupleprop: tuple[int, str] = lt.property(default=(42, "the answer"))
         """A tuple property, to check subscripted generics work."""
 
+        modelprop: MyModel = lt.property(default_factory=lambda: MyModel(a=1, b="two"))
+        """A property typed as a model."""
+
+        rootmodelprop: pydantic.RootModel[int | None] = lt.property(
+            default_factory=lambda: pydantic.RootModel[int | None](root=None)
+        )
+        """A very verbosely defined optional integer.
+        
+        This tests a model that's also a subscripted generic.
+        """
+
     # We will break `badprop` by setting its model to something that's
     # neither the type nor a rootmodel.
     badprop = Example.badprop
@@ -373,6 +388,28 @@ class BadIntModel(pydantic.BaseModel):
         with pytest.raises(pydantic.ValidationError):
             tupleprop.validate(val)
 
+    # Check validation for a model
+    modelprop = example.properties["modelprop"]
+    assert modelprop.validate(MyModel(a=3, b="four")) == MyModel(a=3, b="four")
+    m = MyModel(a=3, b="four")
+    assert modelprop.validate(m) is m
+    assert modelprop.validate({"a": 5, "b": "six"}) == MyModel(a=5, b="six")
+    for invalid in [{"c": 5}, (4, "f"), None]:
+        with pytest.raises(pydantic.ValidationError):
+            modelprop.validate(invalid)
+
+    # Check again for an odd rootmodel
+    rootmodelprop = example.properties["rootmodelprop"]
+    m = rootmodelprop.validate(42)
+    assert isinstance(m, pydantic.RootModel)
+    assert m.root == 42
+    assert m == pydantic.RootModel[int | None](root=42)
+    assert rootmodelprop.validate(m) is m  # RootModel passes through
+    assert rootmodelprop.validate(None).root is None
+    for invalid in ["seven", {"root": None}, 14.5, pydantic.RootModel[int](root=0)]:
+        with pytest.raises(pydantic.ValidationError):
+            modelprop.validate(invalid)
+
 
 def test_readonly_metadata():
     """Check read-only data propagates to the Thing Description."""

From 443aea89a5fb14256fc8046d6ca288a7ec4fe81e Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Fri, 27 Feb 2026 12:49:53 +0000
Subject: [PATCH 05/10] Update src/labthings_fastapi/properties.py

Swap the order of the two cases for clarity.

Co-authored-by: Julian Stirling <julian@julianstirling.co.uk>
---
 src/labthings_fastapi/properties.py | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index d92b03e6..a62af7ea 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -882,23 +882,23 @@ def validate(self, value: Any) -> Value:
             with its value type. This should never happen.
         """
         try:
-            if self.model is self.value_type and issubclass(self.value_type, BaseModel):
-                # If there's no RootModel wrapper, the value_type is a model and may be
-                # used directly for validation. The `issubclass` should not be
-                # necessary, but it's helpful for `mypy` and it does no harm to check.
-                return self.value_type.model_validate(value)
-            elif issubclass(self.model, LabThingsRootModelWrapper):
+            if issubclass(self.model, LabThingsRootModelWrapper):
                 # If a plain type has been wrapped in a RootModel, use that to validate
                 # and then set the property to the root value.
                 model = self.model.model_validate(value)
                 return model.root
-            else:
-                # This should be unreachable, because either `self.value_type` is
-                # a BaseModel and so `value_type` and `model` are identical, or
-                # `model` is a `LabThingsRootModelWrapper` wrapping the value type.
-                msg = f"Property {self.name} has an inconsistent model. This is "
-                msg += f"most likely a LabThings bug. {self.model=}, {self.value_type=}"
-                raise TypeError(msg)
+
+            if issubclass(self.value_type, BaseModel) and self.model is self.value_type:
+                # If there's no RootModel wrapper, the value was defined in code as a
+                # Pydantic model. This means `value_type` and `model` should both
+                # be that same class.
+                return self.value_type.model_validate(value)
+
+            # This should be unreachable, because `model` is a `LabThingsRootModelWrapper`
+            # wrapping the value type, or the values' type should be a BaseModel.
+            msg = f"Property {self.name} has an inconsistent model. This is "
+            msg += f"most likely a LabThings bug. {self.model=}, {self.value_type=}"
+            raise TypeError(msg)
         except ValidationError:
             raise  # This is needed for flake8 to be happy with the docstring
 

From 7d5050986f35a2fde542d0d2ddb803d26799e709 Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Fri, 27 Feb 2026 23:10:07 +0000
Subject: [PATCH 06/10] Don't swallow TypeError when loading settings.

As discussed in the PR thread, this now hard errors if the settings file is not a valid dictionary, or if TypeErrors occur when loading settings. The latter probably indicates LabThings or Thing code is broken, it should not result from an incorrect settings file.

I also fixed a line that was too long.
---
 src/labthings_fastapi/properties.py | 5 +++--
 src/labthings_fastapi/thing.py      | 7 +------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index a62af7ea..9c374a04 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -894,8 +894,9 @@ def validate(self, value: Any) -> Value:
                 # be that same class.
                 return self.value_type.model_validate(value)
 
-            # This should be unreachable, because `model` is a `LabThingsRootModelWrapper`
-            # wrapping the value type, or the values' type should be a BaseModel.
+            # This should be unreachable, because `model` is a
+            # `LabThingsRootModelWrapper` wrapping the value type, or the value type
+            # should be a BaseModel.
             msg = f"Property {self.name} has an inconsistent model. This is "
             msg += f"most likely a LabThings bug. {self.model=}, {self.value_type=}"
             raise TypeError(msg)
diff --git a/src/labthings_fastapi/thing.py b/src/labthings_fastapi/thing.py
index aca9a5f4..d4188ecc 100644
--- a/src/labthings_fastapi/thing.py
+++ b/src/labthings_fastapi/thing.py
@@ -201,9 +201,7 @@ def load_settings(self) -> None:
             so if action is needed (e.g. updating hardware with the loaded settings)
             it should be taken in ``__enter__``.
 
-        :raises TypeError: if the JSON file does not contain a dictionary. This is
-            handled internally and logged, so the exception doesn't propagate
-            outside of the function.
+        :raises TypeError: if the JSON file does not contain a dictionary.
         """
         setting_storage_path = self._thing_server_interface.settings_file_path
         thing_name = type(self).__name__
@@ -236,13 +234,10 @@ def load_settings(self) -> None:
                         f"An extra key {name} was found in the settings file. "
                         "It will be deleted the next time settings are saved."
                     )
-                except TypeError:
-                    self.logger.warning(f"Failed to load {name} with a TypeError.")
         except (
             FileNotFoundError,
             JSONDecodeError,
             PermissionError,
-            TypeError,
         ):
             # Note that if the settings file is missing, we should already have returned
             # before attempting to load settings.

From 180b23f757f46becf97fe3fb96e9a21e44558f71 Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Fri, 27 Feb 2026 23:35:55 +0000
Subject: [PATCH 07/10] Tidy up settings file load error handling.

I've added an extra try: block enclosing just the part that loads the JSON file. I think this neatly separates errors loading the file from errors applying the settings it contains.

I have reverted to the old behaviour, of warning for corrupt files and overwriting them. We can add options to make these hard errors in the future.
---
 src/labthings_fastapi/thing.py | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/labthings_fastapi/thing.py b/src/labthings_fastapi/thing.py
index d4188ecc..175c7618 100644
--- a/src/labthings_fastapi/thing.py
+++ b/src/labthings_fastapi/thing.py
@@ -212,12 +212,24 @@ def load_settings(self) -> None:
 
         # Stop recursion by not allowing settings to be saved as we're reading them.
         self._disable_saving_settings = True
-
         try:
-            with open(setting_storage_path, "r", encoding="utf-8") as file_obj:
-                settings = json.load(file_obj)
-                if not isinstance(settings, Mapping):
-                    raise TypeError("The settings file must be a JSON object.")
+            # The inner try: block ensures we only catch these errors while loading the
+            # file, not when applying the settings.
+            try:
+                with open(setting_storage_path, "r", encoding="utf-8") as file_obj:
+                    settings = json.load(file_obj)
+                    if not isinstance(settings, Mapping):
+                        raise TypeError("The settings file must be a JSON object.")
+            except (FileNotFoundError, PermissionError, TypeError, JSONDecodeError):
+                # Note that if the settings file is missing, we should already have
+                # returned before attempting to load settings.
+                self.logger.warning(
+                    "Error loading settings for %s from %s, could not load a JSON "
+                    "object. Settings for this Thing will be reset to default.",
+                    thing_name,
+                    setting_storage_path,
+                )
+                return
             for name, value in settings.items():
                 try:
                     setting = self.settings[name]
@@ -234,18 +246,6 @@ def load_settings(self) -> None:
                         f"An extra key {name} was found in the settings file. "
                         "It will be deleted the next time settings are saved."
                     )
-        except (
-            FileNotFoundError,
-            JSONDecodeError,
-            PermissionError,
-        ):
-            # Note that if the settings file is missing, we should already have returned
-            # before attempting to load settings.
-            self.logger.warning(
-                "Error loading settings for %s. "
-                "Settings for this Thing will be reset to default.",
-                thing_name,
-            )
         finally:
             self._disable_saving_settings = False
 

From b945a8d6374e465bb49f72788856beeb13764f2e Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Fri, 27 Feb 2026 23:42:34 +0000
Subject: [PATCH 08/10] Fix docstring now I've reverted the behaviour.

---
 src/labthings_fastapi/thing.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/labthings_fastapi/thing.py b/src/labthings_fastapi/thing.py
index 175c7618..9b9ce561 100644
--- a/src/labthings_fastapi/thing.py
+++ b/src/labthings_fastapi/thing.py
@@ -201,7 +201,9 @@ def load_settings(self) -> None:
             so if action is needed (e.g. updating hardware with the loaded settings)
             it should be taken in ``__enter__``.
 
-        :raises TypeError: if the JSON file does not contain a dictionary.
+        :raises TypeError: if the JSON file does not contain a dictionary. This is
+            handled internally and logged, so the exception doesn't propagate
+            outside of the function.
         """
         setting_storage_path = self._thing_server_interface.settings_file_path
         thing_name = type(self).__name__

From 580bdabb82f169b74105bd5670f7ba9548e42ae3 Mon Sep 17 00:00:00 2001
From: Julian Stirling <julian@julianstirling.co.uk>
Date: Sat, 28 Feb 2026 09:04:20 +0000
Subject: [PATCH 09/10] Split reaidng settings file from loading settings.

The nested exception logic was getting confusing I worried the
cognative complexity and the length of the function was too high.
There is no reason to be in disable_saving_settings mode when
loading the file. As such all of the file loading can be
done before this starts.

Moving it to it's own function provides simplifies it significantly.
---
 src/labthings_fastapi/thing.py | 73 ++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 26 deletions(-)

diff --git a/src/labthings_fastapi/thing.py b/src/labthings_fastapi/thing.py
index 9b9ce561..71b3b3f0 100644
--- a/src/labthings_fastapi/thing.py
+++ b/src/labthings_fastapi/thing.py
@@ -188,6 +188,50 @@ def thing_description(request: Request) -> ThingDescription:
         async def websocket(ws: WebSocket) -> None:
             await websocket_endpoint(self, ws)
 
+    def _read_settings_file(self) -> Mapping[str, Any] | None:
+        """Read the settings file and return a mapping of saved settings or None.
+
+        This function handles reading the settings from the disk. It is designed
+        to be called by `load_settings`. Any exceptions caused by file handling or
+        file corruption are caught and logged as warnings.
+
+        :return: A Mapping of setting name to setting value, or None if no settings
+            could be read from file.
+        """
+        setting_storage_path = self._thing_server_interface.settings_file_path
+        thing_name = type(self).__name__
+        if not os.path.exists(setting_storage_path):
+            # If the settings file doesn't exist, we have nothing to do - the settings
+            # are already initialised to their default values.
+            return None
+
+        # Load the settings.
+        try:
+            with open(setting_storage_path, "r", encoding="utf-8") as file_obj:
+                settings = json.load(file_obj)
+        except (FileNotFoundError, PermissionError, JSONDecodeError):
+            # Note that if the settings file is missing, we should already have
+            # returned before attempting to load settings.
+            self.logger.warning(
+                "Error loading settings for %s from %s, could not load a JSON "
+                "object. Settings for this Thing will be reset to default.",
+                thing_name,
+                setting_storage_path,
+            )
+            return None
+
+        if not isinstance(settings, Mapping):
+            self.logger.warning(
+                "Error loading settings for %s from %s. The file does not contain a "
+                "Mapping",
+                thing_name,
+                setting_storage_path,
+            )
+            return None
+
+        # The settings are loaded and are a Mapping. Return them.
+        return settings
+
     def load_settings(self) -> None:
         """Load settings from json.
 
@@ -200,38 +244,15 @@ def load_settings(self) -> None:
             Note that no notifications will be triggered when the settings are set,
             so if action is needed (e.g. updating hardware with the loaded settings)
             it should be taken in ``__enter__``.
-
-        :raises TypeError: if the JSON file does not contain a dictionary. This is
-            handled internally and logged, so the exception doesn't propagate
-            outside of the function.
         """
-        setting_storage_path = self._thing_server_interface.settings_file_path
-        thing_name = type(self).__name__
-        if not os.path.exists(setting_storage_path):
-            # If the settings file doesn't exist, we have nothing to do - the settings
-            # are already initialised to their default values.
+        settings = self._read_settings_file()
+        if settings is None:
+            # Return if no settings can be loaded from file.
             return
 
         # Stop recursion by not allowing settings to be saved as we're reading them.
         self._disable_saving_settings = True
         try:
-            # The inner try: block ensures we only catch these errors while loading the
-            # file, not when applying the settings.
-            try:
-                with open(setting_storage_path, "r", encoding="utf-8") as file_obj:
-                    settings = json.load(file_obj)
-                    if not isinstance(settings, Mapping):
-                        raise TypeError("The settings file must be a JSON object.")
-            except (FileNotFoundError, PermissionError, TypeError, JSONDecodeError):
-                # Note that if the settings file is missing, we should already have
-                # returned before attempting to load settings.
-                self.logger.warning(
-                    "Error loading settings for %s from %s, could not load a JSON "
-                    "object. Settings for this Thing will be reset to default.",
-                    thing_name,
-                    setting_storage_path,
-                )
-                return
             for name, value in settings.items():
                 try:
                     setting = self.settings[name]

From cb06e57ab19a2a1677fa3e8ce76cdf36934661a4 Mon Sep 17 00:00:00 2001
From: Julian Stirling <julian@julianstirling.co.uk>
Date: Sat, 28 Feb 2026 09:44:20 +0000
Subject: [PATCH 10/10] Add test specifically for the JSON setting file not
 containing an Object

---
 tests/test_settings.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/test_settings.py b/tests/test_settings.py
index 02abdfb9..cc46b7d3 100644
--- a/tests/test_settings.py
+++ b/tests/test_settings.py
@@ -331,3 +331,23 @@ def test_try_loading_corrupt_settings(tempdir, caplog):
         assert len(caplog.records) == 1
         assert caplog.records[0].levelname == "WARNING"
         assert caplog.records[0].name == "labthings_fastapi.things.thing"
+
+
+def test_try_loading_setting_file_without_mapping(tempdir, caplog):
+    """Load from setting file that isn't a JSON object."""
+    # Create the server once, so we can get the settings path
+    server = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir)
+    setting_file = _get_setting_file(server, "thing")
+    del server
+
+    with open(setting_file, "w", encoding="utf-8") as file_obj:
+        file_obj.write('["I", "am", "a", "list"]')
+    # Recreate the server and check for the warning in logs
+    with caplog.at_level(logging.WARNING):
+        # Add thing to server
+        _ = lt.ThingServer({"thing": ThingWithSettings}, settings_folder=tempdir)
+        assert len(caplog.records) == 1
+        assert caplog.records[0].levelname == "WARNING"
+        assert caplog.records[0].name == "labthings_fastapi.things.thing"
+        expected_msg = "The file does not contain a Mapping"
+        assert expected_msg in caplog.records[0].message