From e11121f8394eb93a98264d5bad59e276a3511109 Mon Sep 17 00:00:00 2001
From: Alex Luck <luck@siftstack.com>
Date: Wed, 30 Sep 2026 12:13:52 -0700
Subject: [PATCH 1/4] handle unknown pydantic keys

---
 python/CHANGELOG.md                           |  5 ++
 .../low_level_wrappers/test_results.py        | 73 ++++++++++++++++++-
 .../test_incremental_replay.py                | 62 ++++++++++++++++
 .../_tests/sift_types/test_base.py            | 43 +++++++++++
 python/lib/sift_client/errors.py              | 24 ++++++
 python/lib/sift_client/sift_types/_base.py    | 30 ++++++++
 6 files changed, 233 insertions(+), 4 deletions(-)

diff --git a/python/CHANGELOG.md b/python/CHANGELOG.md
index 8decb8a361..78b39a4947 100644
--- a/python/CHANGELOG.md
+++ b/python/CHANGELOG.md
@@ -3,6 +3,11 @@ All notable changes to this project will be documented in this file.
 
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## Unreleased
+
+- Fix a test-results log that could never finish uploading. An update whose fields were all `None`, or whose keys were not fields of the update model, produced an empty field mask. The client logged it without complaint, then the API rejected it at import. The replay cursor only advances past a line that succeeded, so every retry stopped at the same line and stranded the rest of the log. Such an update is now a no-op that is neither logged nor sent, and a logged one is skipped at import. Re-run `import-test-result-log` to finish a log written by an earlier version.
+- A key that is not a field of a create or update model now warns, instead of being dropped in silence. The warning suggests a near match when the key resembles a real field. Add `error::sift_client.errors.SiftWarning` to `filterwarnings` to make it fail the call.
+
 ## [v0.22.1] - September 29, 2026
 
 - Fix streaming ingestion processes crashing with SIGSEGV or SIGABRT at interpreter exit. The `sift-stream-bindings` tokio runtime was never shut down, so a runtime thread that finished work while Python was finalizing re-entered the interpreter and killed the process after the program had completed. The bindings now stop the runtime from an `atexit` hook and expose `shutdown(timeout=5.0)` for callers that exit with `os._exit()`. If runtime threads outlive the timeout, `shutdown()` issues a `RuntimeWarning`. Requires `sift-stream-bindings` 0.5.2. ([#804](https://github.com/sift-stack/sift/pull/804))
diff --git a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
index e0d33dec8c..8571754c27 100644
--- a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
+++ b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
@@ -2,8 +2,9 @@
 
 import logging
 import uuid
+import warnings
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, NamedTuple, TypeVar, cast
+from typing import TYPE_CHECKING, Any, Callable, NamedTuple, TypeVar, cast
 
 from google.protobuf import json_format
 from grpc import RpcError, StatusCode
@@ -51,6 +52,7 @@
 )
 from sift_client._internal.low_level_wrappers.base import DEFAULT_PAGE_SIZE, LowLevelClientBase
 from sift_client._internal.pytest_plugin.audit_log import log_event
+from sift_client.errors import SiftWarning, caller_stacklevel
 from sift_client.sift_types.test_report import (
     TestMeasurement,
     TestMeasurementCreate,
@@ -122,6 +124,33 @@ def _mark_simulated(instance: _EntityT) -> _EntityT:
         instance.__dict__["_simulated"] = True
         return instance
 
+    @classmethod
+    def _no_op_update(
+        cls,
+        entity_name: str,
+        existing: _EntityT | None,
+        simulated: Callable[[], _EntityT],
+    ) -> _EntityT:
+        """Short-circuit an update whose field mask is empty.
+
+        An empty mask asks the API to change nothing, and the API rejects it.
+        Logging one is worse than sending it: at import the entry stops the
+        replay at its line, and no retry gets past it.
+
+        Warn, since either every field was ``None`` or the keys were dropped as
+        unknown (see ``ModelCreateUpdateBase._warn_on_unknown_keys``). Return
+        ``existing`` when the caller has it, otherwise a response built from the
+        request and stamped simulated, since it never reached Sift.
+        """
+        warnings.warn(
+            f"Update to {entity_name} requested no field changes; ignored.",
+            SiftWarning,
+            stacklevel=caller_stacklevel(),
+        )
+        if existing is not None:
+            return existing
+        return cls._mark_simulated(simulated())
+
     @staticmethod
     def simulate_create_test_report_response(
         request: CreateTestReportRequest,
@@ -534,7 +563,8 @@ async def update_test_report(
             simulate: If True, return a simulated response without making an API call.
 
         Returns:
-            The updated TestReport.
+            The updated TestReport, or the report unchanged when the update names
+            no fields (see ``_no_op_update``).
         """
         if request is None:
             if update is None:
@@ -542,6 +572,13 @@ async def update_test_report(
             test_report_proto, field_mask = update.to_proto_with_mask()
             request = UpdateTestReportRequest(test_report=test_report_proto, update_mask=field_mask)
 
+        if not request.update_mask.paths:
+            return self._no_op_update(
+                "TestReport",
+                existing,
+                lambda: self.simulate_update_test_report_response(request),
+            )
+
         if log_file is not None or simulate:
             if log_file is not None:
                 await log_request_to_file(log_file, "UpdateTestReport", request)
@@ -689,7 +726,8 @@ async def update_test_step(
             simulate: If True, return a simulated response without making an API call.
 
         Returns:
-            The updated TestStep.
+            The updated TestStep, or the step unchanged when the update names no
+            fields (see ``_no_op_update``).
         """
         if request is None:
             if update is None:
@@ -700,6 +738,13 @@ async def update_test_step(
                 field_mask.paths.append("error_info")
             request = UpdateTestStepRequest(test_step=test_step_proto, update_mask=field_mask)
 
+        if not request.update_mask.paths:
+            return self._no_op_update(
+                "TestStep",
+                existing,
+                lambda: self.simulate_update_test_step_response(request),
+            )
+
         if log_file is not None or simulate:
             if log_file is not None:
                 await log_request_to_file(log_file, "UpdateTestStep", request)
@@ -892,7 +937,8 @@ async def update_test_measurement(
             simulate: If True, return a simulated response without making an API call.
 
         Returns:
-            The updated TestMeasurement.
+            The updated TestMeasurement, or the measurement unchanged when the
+            update names no fields (see ``_no_op_update``).
         """
         if request is None:
             if update is None:
@@ -902,6 +948,13 @@ async def update_test_measurement(
                 test_measurement=test_measurement_proto, update_mask=field_mask
             )
 
+        if not request.update_mask.paths:
+            return self._no_op_update(
+                "TestMeasurement",
+                existing,
+                lambda: self.simulate_update_test_measurement_response(request),
+            )
+
         if log_file is not None or simulate:
             if log_file is not None:
                 await log_request_to_file(log_file, "UpdateTestMeasurement", request)
@@ -1226,6 +1279,12 @@ async def _replay_update_report(
         orig_report_id = request.test_report.test_report_id
         mapped_report_id = self._map_id(id_map, orig_report_id)
         request.test_report.test_report_id = mapped_report_id
+        # An empty mask asks the server to change nothing, and the API rejects it.
+        # Clients before the write-time guard logged such entries, and the cursor
+        # only advances past a line that succeeded, so one of them stopped every
+        # retry at the same place. Nothing to apply here, so count it skipped.
+        if not request.update_mask.paths:
+            return _EntryIds(orig_report_id or None, mapped_report_id or None, skipped=True)
         # Batch/simulate replays the whole log in order, so a missing report means
         # the log is malformed. Incremental replay may have created the report on an
         # earlier tick (its real ID lives in id_map), so state.report is legitimately
@@ -1251,6 +1310,9 @@ async def _replay_update_step(
         orig_step_id = request.test_step.test_step_id
         mapped_step_id = self._map_id(id_map, orig_step_id)
         request.test_step.test_step_id = mapped_step_id
+        # No paths means nothing to apply; see _replay_update_report.
+        if not request.update_mask.paths:
+            return _EntryIds(orig_step_id or None, mapped_step_id or None, skipped=True)
         existing_step = state.steps_by_id.get(mapped_step_id)
         if simulate and existing_step is None:
             raise ValueError(f"UpdateTestStep for unknown step: {orig_step_id}")
@@ -1275,6 +1337,9 @@ async def _replay_update_measurement(
         orig_meas_id = request.test_measurement.measurement_id
         mapped_meas_id = self._map_id(id_map, orig_meas_id)
         request.test_measurement.measurement_id = mapped_meas_id
+        # No paths means nothing to apply; see _replay_update_report.
+        if not request.update_mask.paths:
+            return _EntryIds(orig_meas_id or None, mapped_meas_id or None, skipped=True)
         existing_meas = state.measurements_by_id.get(mapped_meas_id)
         if simulate and existing_meas is None:
             raise ValueError(f"UpdateTestMeasurement for unknown measurement: {orig_meas_id}")
diff --git a/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py b/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py
index 63bbc85333..6a6d1f7281 100644
--- a/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py
+++ b/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py
@@ -24,6 +24,7 @@
     # Aliased so pytest doesn't try to collect the `Test`-prefixed client as a suite.
     TestResultsLowLevelClient as ResultsLowLevelClient,
 )
+from sift_client.errors import SiftWarning
 from sift_client.sift_types.test_report import (
     TestMeasurement,
     TestMeasurementCreate,
@@ -723,6 +724,67 @@ async def test_resume_propagates_errors_other_than_a_missing_report(tmp_path):
         await client.import_log_file(log_file)
 
 
+@pytest.mark.asyncio
+async def test_resume_skips_an_update_with_an_empty_mask(tmp_path):
+    """A logged update carrying no field paths is skipped, not sent.
+
+    Clients before the write-time guard logged one whenever every field of an
+    update was None, or its keys were dropped as unknown. The API rejects an
+    empty mask, and the cursor only advances past a line that succeeded, so such
+    a line stranded the rest of the log on every retry.
+    """
+    log_file = tmp_path / "empty_mask.jsonl"
+    client = ResultsLowLevelClient(grpc_client=MagicMock())
+
+    report = await client.create_test_report(test_report=_report_create(), log_file=log_file)
+    # Written by hand: the guard in update_test_report now refuses to log this.
+    with log_file.open("a") as handle:
+        handle.write(
+            f'[UpdateTestReport] {{"testReport":{{"testReportId":"{report.id_}"}},'
+            '"updateMask":""}\n'
+        )
+    update = TestReportUpdate(status=TestStatus.FAILED)
+    update.resource_id = report.id_
+    await client.update_test_report(update=update, log_file=log_file)
+
+    LogTracking(last_uploaded_line=1, id_map={report.id_: "real-report"}).save(log_file)
+
+    client.update_test_report = AsyncMock(return_value=_make_report("real-report"))
+
+    await client.import_log_file(log_file, incremental=True)
+
+    # Only the real update was sent; the empty-mask line never reached the API.
+    client.update_test_report.assert_awaited_once()
+    sent = client.update_test_report.await_args.kwargs["request"]
+    assert sent.test_report.status == TestStatus.FAILED.value
+    # Both remaining lines are behind the cursor, so a later tick re-sends neither.
+    assert LogTracking.load(log_file).last_uploaded_line == 3
+
+
+@pytest.mark.asyncio
+async def test_update_with_no_fields_is_not_logged(tmp_path):
+    """An update that changes nothing writes no log entry and returns the entity.
+
+    The API rejects a no-op update, so logging one plants an entry that can never
+    replay. The caller gets the entity back unchanged, plus a warning.
+    """
+    log_file = tmp_path / "no_fields.jsonl"
+    client = ResultsLowLevelClient(grpc_client=MagicMock())
+
+    report = await client.create_test_report(test_report=_report_create(), log_file=log_file)
+    lines_before = log_file.read_text().count("\n")
+
+    update = TestReportUpdate(run_id=None)
+    update.resource_id = report.id_
+    with pytest.warns(SiftWarning, match="requested no field changes"):
+        returned = await client.update_test_report(
+            update=update, log_file=log_file, existing=report
+        )
+
+    assert returned is report
+    assert log_file.read_text().count("\n") == lines_before
+
+
 @pytest.mark.asyncio
 async def test_incremental_and_new_report_are_rejected_together(tmp_path):
     """The two flags contradict each other, so asking for both is an error.
diff --git a/python/lib/sift_client/_tests/sift_types/test_base.py b/python/lib/sift_client/_tests/sift_types/test_base.py
index a0c3cfc584..2264c79761 100644
--- a/python/lib/sift_client/_tests/sift_types/test_base.py
+++ b/python/lib/sift_client/_tests/sift_types/test_base.py
@@ -2,11 +2,13 @@
 
 from __future__ import annotations
 
+import warnings
 from datetime import datetime, timezone
 from typing import ClassVar
 from unittest.mock import MagicMock
 
 import pytest
+from pydantic import ConfigDict, ValidationError
 from sift.calculated_channels.v2.calculated_channels_pb2 import (
     CalculatedChannel as CalculatedChannelProto,
 )
@@ -14,6 +16,7 @@
     CreateCalculatedChannelRequest,
 )
 
+from sift_client.errors import SiftWarning
 from sift_client.sift_types._base import (
     BaseType,
     MappingHelper,
@@ -226,6 +229,46 @@ def test_update_requires_resource_id(self):
             model.to_proto_with_mask()
 
 
+class TestUnknownKeys:
+    """Tests for the warning on input keys that are not fields of the model."""
+
+    def test_unknown_key_warns_and_is_ignored(self):
+        """An unrecognized key contributes nothing to the field mask."""
+        with pytest.warns(SiftWarning, match="Unknown field `tags` for SimpleUpdateModel"):
+            model = SimpleUpdateModel.model_validate({"tags": ["a"], "name": "new_name"})
+
+        model.resource_id = "test_id"
+        _, mask = model.to_proto_with_mask()
+        assert mask.paths == ["name"]
+
+    def test_unknown_key_suggests_a_near_match(self):
+        """A misspelled or camelCase key is usually a typo for a real field."""
+        with pytest.warns(SiftWarning, match=r"did you mean `description`\?"):
+            SimpleUpdateModel.model_validate({"descriptionn": "x"})
+
+    def test_unknown_key_on_a_create_model_warns(self):
+        """A create drops the key too, which loses the value instead of no-opping."""
+        with pytest.warns(SiftWarning, match="Unknown field `unit` for SimpleCreateModel"):
+            SimpleCreateModel.model_validate({"name": "n", "unit": "volts"})
+
+    def test_known_keys_do_not_warn(self):
+        """The common case stays quiet, including a field explicitly set to None."""
+        with warnings.catch_warnings():
+            warnings.simplefilter("error", SiftWarning)
+            SimpleUpdateModel.model_validate({"name": "new_name", "description": None})
+
+    def test_extra_forbid_model_is_left_to_raise(self):
+        """A model that opts into extra="forbid" reports the key itself."""
+
+        class StrictUpdateModel(SimpleUpdateModel):
+            model_config = ConfigDict(extra="forbid")
+
+        with warnings.catch_warnings():
+            warnings.simplefilter("error", SiftWarning)
+            with pytest.raises(ValidationError):
+                StrictUpdateModel.model_validate({"tags": ["a"]})
+
+
 class TestMappingHelper:
     """Tests for MappingHelper functionality."""
 
diff --git a/python/lib/sift_client/errors.py b/python/lib/sift_client/errors.py
index 657c6a9488..c87f1350a8 100644
--- a/python/lib/sift_client/errors.py
+++ b/python/lib/sift_client/errors.py
@@ -1,7 +1,31 @@
 from __future__ import annotations
 
+import inspect
+from pathlib import Path
 from typing import NoReturn
 
+_PACKAGE_ROOT = str(Path(__file__).resolve().parent)
+
+
+def caller_stacklevel() -> int:
+    """Return the ``warnings.warn`` stacklevel of the first frame outside the SDK.
+
+    A fixed stacklevel points at SDK or pydantic internals, which tells the
+    caller nothing about which of their lines caused the warning.
+
+    Call this from the function that calls ``warnings.warn``.
+    """
+    frame = inspect.currentframe()
+    frame = frame.f_back if frame is not None else None  # the function calling warn()
+    level = 1
+    while frame is not None:
+        filename = frame.f_code.co_filename
+        if not filename.startswith(_PACKAGE_ROOT) and "pydantic" not in filename:
+            return level
+        frame = frame.f_back
+        level += 1
+    return 2
+
 
 class SiftWarning(UserWarning):
     """Base warning for Sift generated warnings."""
diff --git a/python/lib/sift_client/sift_types/_base.py b/python/lib/sift_client/sift_types/_base.py
index 7af93087e6..a06e99039e 100644
--- a/python/lib/sift_client/sift_types/_base.py
+++ b/python/lib/sift_client/sift_types/_base.py
@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import difflib
+import warnings
 from abc import ABC, abstractmethod
 from datetime import datetime
 from enum import Enum
@@ -16,6 +18,8 @@
 from google.protobuf import field_mask_pb2, message
 from pydantic import BaseModel, ConfigDict, Field, PrivateAttr, model_validator
 
+from sift_client.errors import SiftWarning, caller_stacklevel
+
 if TYPE_CHECKING:
     from sift_client.client import SiftClient
 
@@ -113,6 +117,32 @@ class ModelCreateUpdateBase(BaseModel, ABC):
     def __init__(self, **data: Any):
         super().__init__(**data)
 
+    @model_validator(mode="before")
+    @classmethod
+    def _warn_on_unknown_keys(cls, data: Any) -> Any:
+        """Warn for input keys that are not fields of this model.
+
+        Pydantic's default ``extra="ignore"`` drops an unrecognized key in
+        silence. A create then omits that field, and an update carries an empty
+        field mask, which the API rejects.
+
+        Warn rather than raise, since a reporting mistake must not fail a test
+        run in progress. ``filterwarnings`` promotes it to an error. Models that
+        set ``extra="forbid"`` raise on their own, so they are left alone.
+        """
+        if not isinstance(data, dict) or cls.model_config.get("extra") == "forbid":
+            return data
+        known = sorted(cls.model_fields)
+        for key in [key for key in data if key not in cls.model_fields]:
+            close = difflib.get_close_matches(str(key), known, n=1, cutoff=0.6)
+            hint = f" (did you mean `{close[0]}`?)" if close else ""
+            warnings.warn(
+                f"Unknown field `{key}` for {cls.__name__}{hint}; ignored.",
+                SiftWarning,
+                stacklevel=caller_stacklevel(),
+            )
+        return data
+
     @model_validator(mode="after")
     def _check_mapping_helpers(self):
         if self._to_proto_helpers:

From 3f8cb1f941af0b6ce454f9935b1ffe7177cec222 Mon Sep 17 00:00:00 2001
From: Alex Luck <luck@siftstack.com>
Date: Wed, 30 Sep 2026 15:38:52 -0700
Subject: [PATCH 2/4] python(fix): address review on empty update masks

Move caller_stacklevel out of the public errors module. mkdocs publishes
non-underscore names from there, and a stack-walking utility is not an error
type; it now sits beside the other generic helpers in _internal/util.

Add SiftIgnoredInputWarning so filterwarnings can target an unknown model
field and a no-op update without promoting every other Sift warning.

Stop fabricating an entity when a live no-op update has nothing to return.
_skip_empty_update now hands back a simulated response only on the log and
simulate paths, and raises otherwise. The fabricated entity carried
_simulated=True, which the pytest plugin reads to resolve the report link, so
a caller passing an ID string could not tell it from a real read.

Return from the unknown-key validator before sorting the field names, so the
path where every key is known does no extra work.
---
 python/CHANGELOG.md                           |  4 +-
 .../low_level_wrappers/test_results.py        | 53 ++++++++++++-------
 python/lib/sift_client/_internal/util/util.py | 26 +++++++++
 .../test_incremental_replay.py                |  4 +-
 .../_tests/sift_types/test_base.py            | 29 +++++++---
 python/lib/sift_client/errors.py              | 36 +++++--------
 python/lib/sift_client/sift_types/_base.py    | 16 ++++--
 7 files changed, 111 insertions(+), 57 deletions(-)

diff --git a/python/CHANGELOG.md b/python/CHANGELOG.md
index 78b39a4947..9af11c02c6 100644
--- a/python/CHANGELOG.md
+++ b/python/CHANGELOG.md
@@ -3,10 +3,10 @@ All notable changes to this project will be documented in this file.
 
 This project adheres to [Semantic Versioning](http://semver.org/).
 
-## Unreleased
+## [Unreleased]
 
 - Fix a test-results log that could never finish uploading. An update whose fields were all `None`, or whose keys were not fields of the update model, produced an empty field mask. The client logged it without complaint, then the API rejected it at import. The replay cursor only advances past a line that succeeded, so every retry stopped at the same line and stranded the rest of the log. Such an update is now a no-op that is neither logged nor sent, and a logged one is skipped at import. Re-run `import-test-result-log` to finish a log written by an earlier version.
-- A key that is not a field of a create or update model now warns, instead of being dropped in silence. The warning suggests a near match when the key resembles a real field. Add `error::sift_client.errors.SiftWarning` to `filterwarnings` to make it fail the call.
+- A key that is not a field of a create or update model now warns, instead of being dropped in silence. The warning suggests a near match when the key resembles a real field. Both this and an update that names no fields raise the new `SiftIgnoredInputWarning`. Add `error::sift_client.errors.SiftIgnoredInputWarning` to `filterwarnings` to make either one fail the call.
 
 ## [v0.22.1] - September 29, 2026
 
diff --git a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
index 8571754c27..23810a152b 100644
--- a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
+++ b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
@@ -4,7 +4,7 @@
 import uuid
 import warnings
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Callable, NamedTuple, TypeVar, cast
+from typing import TYPE_CHECKING, Any, NamedTuple, TypeVar, cast
 
 from google.protobuf import json_format
 from grpc import RpcError, StatusCode
@@ -52,7 +52,8 @@
 )
 from sift_client._internal.low_level_wrappers.base import DEFAULT_PAGE_SIZE, LowLevelClientBase
 from sift_client._internal.pytest_plugin.audit_log import log_event
-from sift_client.errors import SiftWarning, caller_stacklevel
+from sift_client._internal.util.util import caller_stacklevel
+from sift_client.errors import SiftIgnoredInputWarning
 from sift_client.sift_types.test_report import (
     TestMeasurement,
     TestMeasurementCreate,
@@ -125,11 +126,11 @@ def _mark_simulated(instance: _EntityT) -> _EntityT:
         return instance
 
     @classmethod
-    def _no_op_update(
+    def _skip_empty_update(
         cls,
         entity_name: str,
         existing: _EntityT | None,
-        simulated: Callable[[], _EntityT],
+        simulated: _EntityT | None,
     ) -> _EntityT:
         """Short-circuit an update whose field mask is empty.
 
@@ -138,18 +139,31 @@ def _no_op_update(
         replay at its line, and no retry gets past it.
 
         Warn, since either every field was ``None`` or the keys were dropped as
-        unknown (see ``ModelCreateUpdateBase._warn_on_unknown_keys``). Return
-        ``existing`` when the caller has it, otherwise a response built from the
-        request and stamped simulated, since it never reached Sift.
+        unknown (see ``ModelCreateUpdateBase._warn_on_unknown_keys``), then return
+        the entity unchanged. ``simulated`` is the response to hand back on the
+        log and simulate paths, where a synthesized entity is the correct answer,
+        and None on a live call, where it would not be.
+
+        Raises:
+            ValueError: On a live call with no ``existing`` entity. Nothing
+                changed server-side and there is no entity to return, so the
+                alternative is a fabricated one, which callers cannot tell from a
+                real read.
         """
         warnings.warn(
             f"Update to {entity_name} requested no field changes; ignored.",
-            SiftWarning,
+            SiftIgnoredInputWarning,
             stacklevel=caller_stacklevel(),
         )
         if existing is not None:
             return existing
-        return cls._mark_simulated(simulated())
+        if simulated is not None:
+            return cls._mark_simulated(simulated)
+        raise ValueError(
+            f"Update to {entity_name} named no fields to change. Pass the "
+            f"{entity_name} rather than its ID to get it back unchanged, or name "
+            "at least one field to update."
+        )
 
     @staticmethod
     def simulate_create_test_report_response(
@@ -564,7 +578,7 @@ async def update_test_report(
 
         Returns:
             The updated TestReport, or the report unchanged when the update names
-            no fields (see ``_no_op_update``).
+            no fields (see ``_skip_empty_update``).
         """
         if request is None:
             if update is None:
@@ -572,11 +586,12 @@ async def update_test_report(
             test_report_proto, field_mask = update.to_proto_with_mask()
             request = UpdateTestReportRequest(test_report=test_report_proto, update_mask=field_mask)
 
+        simulating = log_file is not None or simulate
         if not request.update_mask.paths:
-            return self._no_op_update(
+            return self._skip_empty_update(
                 "TestReport",
                 existing,
-                lambda: self.simulate_update_test_report_response(request),
+                self.simulate_update_test_report_response(request) if simulating else None,
             )
 
         if log_file is not None or simulate:
@@ -727,7 +742,7 @@ async def update_test_step(
 
         Returns:
             The updated TestStep, or the step unchanged when the update names no
-            fields (see ``_no_op_update``).
+            fields (see ``_skip_empty_update``).
         """
         if request is None:
             if update is None:
@@ -738,11 +753,12 @@ async def update_test_step(
                 field_mask.paths.append("error_info")
             request = UpdateTestStepRequest(test_step=test_step_proto, update_mask=field_mask)
 
+        simulating = log_file is not None or simulate
         if not request.update_mask.paths:
-            return self._no_op_update(
+            return self._skip_empty_update(
                 "TestStep",
                 existing,
-                lambda: self.simulate_update_test_step_response(request),
+                self.simulate_update_test_step_response(request) if simulating else None,
             )
 
         if log_file is not None or simulate:
@@ -938,7 +954,7 @@ async def update_test_measurement(
 
         Returns:
             The updated TestMeasurement, or the measurement unchanged when the
-            update names no fields (see ``_no_op_update``).
+            update names no fields (see ``_skip_empty_update``).
         """
         if request is None:
             if update is None:
@@ -948,11 +964,12 @@ async def update_test_measurement(
                 test_measurement=test_measurement_proto, update_mask=field_mask
             )
 
+        simulating = log_file is not None or simulate
         if not request.update_mask.paths:
-            return self._no_op_update(
+            return self._skip_empty_update(
                 "TestMeasurement",
                 existing,
-                lambda: self.simulate_update_test_measurement_response(request),
+                self.simulate_update_test_measurement_response(request) if simulating else None,
             )
 
         if log_file is not None or simulate:
diff --git a/python/lib/sift_client/_internal/util/util.py b/python/lib/sift_client/_internal/util/util.py
index 28f69ef921..3d886556d5 100644
--- a/python/lib/sift_client/_internal/util/util.py
+++ b/python/lib/sift_client/_internal/util/util.py
@@ -1,16 +1,42 @@
 from __future__ import annotations
 
+import inspect
+import os
+from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
 if TYPE_CHECKING:
     from collections.abc import Iterator
 
+_PACKAGE_ROOT = str(Path(__file__).resolve().parent.parent.parent)
+_PYDANTIC_PATH = f"{os.sep}pydantic{os.sep}"
+
 
 def count_non_none(*args: Any) -> int:
     """Count the number of non-none arguments."""
     return sum(1 for arg in args if arg is not None)
 
 
+def caller_stacklevel() -> int:
+    """Return the ``warnings.warn`` stacklevel of the first frame outside the SDK.
+
+    A fixed stacklevel points at SDK or pydantic internals, which tells the
+    caller nothing about which of their lines caused the warning.
+
+    Call this from the function that calls ``warnings.warn``.
+    """
+    frame = inspect.currentframe()
+    frame = frame.f_back if frame is not None else None  # the function calling warn()
+    level = 1
+    while frame is not None:
+        filename = frame.f_code.co_filename
+        if not filename.startswith(_PACKAGE_ROOT) and _PYDANTIC_PATH not in filename:
+            return level
+        frame = frame.f_back
+        level += 1
+    return 2
+
+
 def chunked(items: list[Any], size: int) -> Iterator[list[Any]]:
     """Yield successive chunks of at most ``size`` items."""
     for i in range(0, len(items), size):
diff --git a/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py b/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py
index 6a6d1f7281..02156bf74c 100644
--- a/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py
+++ b/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py
@@ -24,7 +24,7 @@
     # Aliased so pytest doesn't try to collect the `Test`-prefixed client as a suite.
     TestResultsLowLevelClient as ResultsLowLevelClient,
 )
-from sift_client.errors import SiftWarning
+from sift_client.errors import SiftIgnoredInputWarning
 from sift_client.sift_types.test_report import (
     TestMeasurement,
     TestMeasurementCreate,
@@ -776,7 +776,7 @@ async def test_update_with_no_fields_is_not_logged(tmp_path):
 
     update = TestReportUpdate(run_id=None)
     update.resource_id = report.id_
-    with pytest.warns(SiftWarning, match="requested no field changes"):
+    with pytest.warns(SiftIgnoredInputWarning, match="requested no field changes"):
         returned = await client.update_test_report(
             update=update, log_file=log_file, existing=report
         )
diff --git a/python/lib/sift_client/_tests/sift_types/test_base.py b/python/lib/sift_client/_tests/sift_types/test_base.py
index 2264c79761..1562e70d60 100644
--- a/python/lib/sift_client/_tests/sift_types/test_base.py
+++ b/python/lib/sift_client/_tests/sift_types/test_base.py
@@ -16,13 +16,14 @@
     CreateCalculatedChannelRequest,
 )
 
-from sift_client.errors import SiftWarning
+from sift_client.errors import SiftIgnoredInputWarning
 from sift_client.sift_types._base import (
     BaseType,
     MappingHelper,
     ModelCreate,
     ModelUpdate,
 )
+from sift_client.sift_types.annotation import PhaseCreate
 
 
 class SimpleCreateModel(ModelCreate[CreateCalculatedChannelRequest]):
@@ -234,7 +235,9 @@ class TestUnknownKeys:
 
     def test_unknown_key_warns_and_is_ignored(self):
         """An unrecognized key contributes nothing to the field mask."""
-        with pytest.warns(SiftWarning, match="Unknown field `tags` for SimpleUpdateModel"):
+        with pytest.warns(
+            SiftIgnoredInputWarning, match="Unknown field `tags` for SimpleUpdateModel"
+        ):
             model = SimpleUpdateModel.model_validate({"tags": ["a"], "name": "new_name"})
 
         model.resource_id = "test_id"
@@ -243,18 +246,20 @@ def test_unknown_key_warns_and_is_ignored(self):
 
     def test_unknown_key_suggests_a_near_match(self):
         """A misspelled or camelCase key is usually a typo for a real field."""
-        with pytest.warns(SiftWarning, match=r"did you mean `description`\?"):
+        with pytest.warns(SiftIgnoredInputWarning, match=r"did you mean `description`\?"):
             SimpleUpdateModel.model_validate({"descriptionn": "x"})
 
     def test_unknown_key_on_a_create_model_warns(self):
         """A create drops the key too, which loses the value instead of no-opping."""
-        with pytest.warns(SiftWarning, match="Unknown field `unit` for SimpleCreateModel"):
+        with pytest.warns(
+            SiftIgnoredInputWarning, match="Unknown field `unit` for SimpleCreateModel"
+        ):
             SimpleCreateModel.model_validate({"name": "n", "unit": "volts"})
 
     def test_known_keys_do_not_warn(self):
         """The common case stays quiet, including a field explicitly set to None."""
         with warnings.catch_warnings():
-            warnings.simplefilter("error", SiftWarning)
+            warnings.simplefilter("error", SiftIgnoredInputWarning)
             SimpleUpdateModel.model_validate({"name": "new_name", "description": None})
 
     def test_extra_forbid_model_is_left_to_raise(self):
@@ -264,10 +269,22 @@ class StrictUpdateModel(SimpleUpdateModel):
             model_config = ConfigDict(extra="forbid")
 
         with warnings.catch_warnings():
-            warnings.simplefilter("error", SiftWarning)
+            warnings.simplefilter("error", SiftIgnoredInputWarning)
             with pytest.raises(ValidationError):
                 StrictUpdateModel.model_validate({"tags": ["a"]})
 
+    def test_extra_forbid_is_detected_through_inheritance(self):
+        """The real forbid model sets the config on a parent, not on itself.
+
+        ``PhaseCreate`` inherits from ``AnnotationCreateBase``, so the skip relies
+        on pydantic merging parent config into the subclass. Asserting it against
+        a locally defined model would not exercise that merge.
+        """
+        with warnings.catch_warnings():
+            warnings.simplefilter("error", SiftIgnoredInputWarning)
+            with pytest.raises(ValidationError):
+                PhaseCreate.model_validate({"name": "p", "state": "open"})
+
 
 class TestMappingHelper:
     """Tests for MappingHelper functionality."""
diff --git a/python/lib/sift_client/errors.py b/python/lib/sift_client/errors.py
index c87f1350a8..9e853f37a0 100644
--- a/python/lib/sift_client/errors.py
+++ b/python/lib/sift_client/errors.py
@@ -1,31 +1,7 @@
 from __future__ import annotations
 
-import inspect
-from pathlib import Path
 from typing import NoReturn
 
-_PACKAGE_ROOT = str(Path(__file__).resolve().parent)
-
-
-def caller_stacklevel() -> int:
-    """Return the ``warnings.warn`` stacklevel of the first frame outside the SDK.
-
-    A fixed stacklevel points at SDK or pydantic internals, which tells the
-    caller nothing about which of their lines caused the warning.
-
-    Call this from the function that calls ``warnings.warn``.
-    """
-    frame = inspect.currentframe()
-    frame = frame.f_back if frame is not None else None  # the function calling warn()
-    level = 1
-    while frame is not None:
-        filename = frame.f_code.co_filename
-        if not filename.startswith(_PACKAGE_ROOT) and "pydantic" not in filename:
-            return level
-        frame = frame.f_back
-        level += 1
-    return 2
-
 
 class SiftWarning(UserWarning):
     """Base warning for Sift generated warnings."""
@@ -35,6 +11,18 @@ class SiftExperimentalWarning(SiftWarning):
     """Warning for experimental features."""
 
 
+class SiftIgnoredInputWarning(SiftWarning):
+    """Input the SDK accepted but could not act on.
+
+    Raised for a key that is not a field of a create or update model, and for an
+    update that names no fields to change. Both are caller mistakes that the SDK
+    tolerates rather than fails, so this class exists to let a caller promote
+    exactly these to errors without also promoting unrelated Sift warnings::
+
+        filterwarnings = error::sift_client.errors.SiftIgnoredInputWarning
+    """
+
+
 class SiftCredentialsError(ValueError):
     """Raised when Sift credentials cannot be resolved.
 
diff --git a/python/lib/sift_client/sift_types/_base.py b/python/lib/sift_client/sift_types/_base.py
index a06e99039e..7acec28abd 100644
--- a/python/lib/sift_client/sift_types/_base.py
+++ b/python/lib/sift_client/sift_types/_base.py
@@ -18,7 +18,8 @@
 from google.protobuf import field_mask_pb2, message
 from pydantic import BaseModel, ConfigDict, Field, PrivateAttr, model_validator
 
-from sift_client.errors import SiftWarning, caller_stacklevel
+from sift_client._internal.util.util import caller_stacklevel
+from sift_client.errors import SiftIgnoredInputWarning
 
 if TYPE_CHECKING:
     from sift_client.client import SiftClient
@@ -127,18 +128,23 @@ def _warn_on_unknown_keys(cls, data: Any) -> Any:
         field mask, which the API rejects.
 
         Warn rather than raise, since a reporting mistake must not fail a test
-        run in progress. ``filterwarnings`` promotes it to an error. Models that
-        set ``extra="forbid"`` raise on their own, so they are left alone.
+        run in progress. ``filterwarnings`` promotes it to an error. A model that
+        sets ``extra="forbid"`` raises on its own and is left alone; that is what
+        ``PhaseCreate`` does, and forbidding here instead would abort a live test
+        run over a dropped field.
         """
         if not isinstance(data, dict) or cls.model_config.get("extra") == "forbid":
             return data
+        unknown = [key for key in data if key not in cls.model_fields]
+        if not unknown:
+            return data
         known = sorted(cls.model_fields)
-        for key in [key for key in data if key not in cls.model_fields]:
+        for key in unknown:
             close = difflib.get_close_matches(str(key), known, n=1, cutoff=0.6)
             hint = f" (did you mean `{close[0]}`?)" if close else ""
             warnings.warn(
                 f"Unknown field `{key}` for {cls.__name__}{hint}; ignored.",
-                SiftWarning,
+                SiftIgnoredInputWarning,
                 stacklevel=caller_stacklevel(),
             )
         return data

From ab2085d765d50500bc33d6efcd2c0f69703d6831 Mon Sep 17 00:00:00 2001
From: Alex Luck <luck@siftstack.com>
Date: Wed, 30 Sep 2026 17:04:28 -0700
Subject: [PATCH 3/4] clean up

---
 python/CHANGELOG.md                                |  4 ++--
 .../_internal/low_level_wrappers/test_results.py   | 12 +-----------
 .../low_level_wrappers/test_incremental_replay.py  | 14 ++------------
 python/lib/sift_client/errors.py                   | 10 +++-------
 4 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/python/CHANGELOG.md b/python/CHANGELOG.md
index 9af11c02c6..21c8c60e30 100644
--- a/python/CHANGELOG.md
+++ b/python/CHANGELOG.md
@@ -5,8 +5,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
-- Fix a test-results log that could never finish uploading. An update whose fields were all `None`, or whose keys were not fields of the update model, produced an empty field mask. The client logged it without complaint, then the API rejected it at import. The replay cursor only advances past a line that succeeded, so every retry stopped at the same line and stranded the rest of the log. Such an update is now a no-op that is neither logged nor sent, and a logged one is skipped at import. Re-run `import-test-result-log` to finish a log written by an earlier version.
-- A key that is not a field of a create or update model now warns, instead of being dropped in silence. The warning suggests a near match when the key resembles a real field. Both this and an update that names no fields raise the new `SiftIgnoredInputWarning`. Add `error::sift_client.errors.SiftIgnoredInputWarning` to `filterwarnings` to make either one fail the call.
+- Fix issue where a test-results log containing an update that named no fields could never finish importing. Re-run `import-test-result-log` to finish a log written by an earlier version.
+- A key that is not a field of a create or update model now warns with `SiftIgnoredInputWarning`, and suggests a near match.
 
 ## [v0.22.1] - September 29, 2026
 
diff --git a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
index 23810a152b..585698091d 100644
--- a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
+++ b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
@@ -134,16 +134,6 @@ def _skip_empty_update(
     ) -> _EntityT:
         """Short-circuit an update whose field mask is empty.
 
-        An empty mask asks the API to change nothing, and the API rejects it.
-        Logging one is worse than sending it: at import the entry stops the
-        replay at its line, and no retry gets past it.
-
-        Warn, since either every field was ``None`` or the keys were dropped as
-        unknown (see ``ModelCreateUpdateBase._warn_on_unknown_keys``), then return
-        the entity unchanged. ``simulated`` is the response to hand back on the
-        log and simulate paths, where a synthesized entity is the correct answer,
-        and None on a live call, where it would not be.
-
         Raises:
             ValueError: On a live call with no ``existing`` entity. Nothing
                 changed server-side and there is no entity to return, so the
@@ -151,7 +141,7 @@ def _skip_empty_update(
                 real read.
         """
         warnings.warn(
-            f"Update to {entity_name} requested no field changes; ignored.",
+            f"Update to {entity_name} requested no field changes and was ignored.",
             SiftIgnoredInputWarning,
             stacklevel=caller_stacklevel(),
         )
diff --git a/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py b/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py
index 02156bf74c..847f5d474a 100644
--- a/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py
+++ b/python/lib/sift_client/_tests/_internal/low_level_wrappers/test_incremental_replay.py
@@ -726,13 +726,7 @@ async def test_resume_propagates_errors_other_than_a_missing_report(tmp_path):
 
 @pytest.mark.asyncio
 async def test_resume_skips_an_update_with_an_empty_mask(tmp_path):
-    """A logged update carrying no field paths is skipped, not sent.
-
-    Clients before the write-time guard logged one whenever every field of an
-    update was None, or its keys were dropped as unknown. The API rejects an
-    empty mask, and the cursor only advances past a line that succeeded, so such
-    a line stranded the rest of the log on every retry.
-    """
+    """A logged update carrying no field paths is skipped, not sent."""
     log_file = tmp_path / "empty_mask.jsonl"
     client = ResultsLowLevelClient(grpc_client=MagicMock())
 
@@ -763,11 +757,7 @@ async def test_resume_skips_an_update_with_an_empty_mask(tmp_path):
 
 @pytest.mark.asyncio
 async def test_update_with_no_fields_is_not_logged(tmp_path):
-    """An update that changes nothing writes no log entry and returns the entity.
-
-    The API rejects a no-op update, so logging one plants an entry that can never
-    replay. The caller gets the entity back unchanged, plus a warning.
-    """
+    """An update that changes nothing writes no log entry and returns the entity."""
     log_file = tmp_path / "no_fields.jsonl"
     client = ResultsLowLevelClient(grpc_client=MagicMock())
 
diff --git a/python/lib/sift_client/errors.py b/python/lib/sift_client/errors.py
index 9e853f37a0..b8f511e60e 100644
--- a/python/lib/sift_client/errors.py
+++ b/python/lib/sift_client/errors.py
@@ -12,14 +12,10 @@ class SiftExperimentalWarning(SiftWarning):
 
 
 class SiftIgnoredInputWarning(SiftWarning):
-    """Input the SDK accepted but could not act on.
+    """Warning for input the SDK ignored.
 
-    Raised for a key that is not a field of a create or update model, and for an
-    update that names no fields to change. Both are caller mistakes that the SDK
-    tolerates rather than fails, so this class exists to let a caller promote
-    exactly these to errors without also promoting unrelated Sift warnings::
-
-        filterwarnings = error::sift_client.errors.SiftIgnoredInputWarning
+    Covers an unrecognized field on a Create/Update pydantic model, and an update
+    that names no fields to change.
     """
 
 

From f3fd5725bdd15dee73a3e3fa5759c0f3183794e8 Mon Sep 17 00:00:00 2001
From: Alex Luck <luck@siftstack.com>
Date: Wed, 30 Sep 2026 17:22:58 -0700
Subject: [PATCH 4/4] consolidate boolean logic

---
 .../_internal/low_level_wrappers/test_results.py            | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
index 585698091d..91c035b389 100644
--- a/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
+++ b/python/lib/sift_client/_internal/low_level_wrappers/test_results.py
@@ -584,7 +584,7 @@ async def update_test_report(
                 self.simulate_update_test_report_response(request) if simulating else None,
             )
 
-        if log_file is not None or simulate:
+        if simulating:
             if log_file is not None:
                 await log_request_to_file(log_file, "UpdateTestReport", request)
             return self._mark_simulated(
@@ -751,7 +751,7 @@ async def update_test_step(
                 self.simulate_update_test_step_response(request) if simulating else None,
             )
 
-        if log_file is not None or simulate:
+        if simulating:
             if log_file is not None:
                 await log_request_to_file(log_file, "UpdateTestStep", request)
             return self._mark_simulated(
@@ -962,7 +962,7 @@ async def update_test_measurement(
                 self.simulate_update_test_measurement_response(request) if simulating else None,
             )
 
-        if log_file is not None or simulate:
+        if simulating:
             if log_file is not None:
                 await log_request_to_file(log_file, "UpdateTestMeasurement", request)
             return self._mark_simulated(