Python(fix): handle pytest plugin empty updates

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 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
 
 - 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))

python/lib/sift_client/_internal/low_level_wrappers/test_results.py

@@ -2,6 +2,7 @@
 
 import logging
 import uuid
+import warnings
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, NamedTuple, TypeVar, cast
 
@@ -51,6 +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._internal.util.util import caller_stacklevel
+from sift_client.errors import SiftIgnoredInputWarning
 from sift_client.sift_types.test_report import (
     TestMeasurement,
     TestMeasurementCreate,
@@ -122,6 +125,36 @@ def _mark_simulated(instance: _EntityT) -> _EntityT:
         instance.__dict__["_simulated"] = True
         return instance
 
+    @classmethod
+    def _skip_empty_update(
+        cls,
+        entity_name: str,
+        existing: _EntityT | None,
+        simulated: _EntityT | None,
+    ) -> _EntityT:
+        """Short-circuit an update whose field mask is empty.
+
+        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 and was ignored.",
+            SiftIgnoredInputWarning,
+            stacklevel=caller_stacklevel(),
+        )
+        if existing is not None:
+            return existing
+        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(
         request: CreateTestReportRequest,
@@ -534,15 +567,24 @@ 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 ``_skip_empty_update``).
         """
         if request is None:
             if update is None:
                 raise ValueError("Either update or request must be provided")
             test_report_proto, field_mask = update.to_proto_with_mask()
             request = UpdateTestReportRequest(test_report=test_report_proto, update_mask=field_mask)
 
-        if log_file is not None or simulate:
+        simulating = log_file is not None or simulate
+        if not request.update_mask.paths:
+            return self._skip_empty_update(
+                "TestReport",
+                existing,
+                self.simulate_update_test_report_response(request) if simulating else None,
+            )
+
+        if simulating:
             if log_file is not None:
                 await log_request_to_file(log_file, "UpdateTestReport", request)
             return self._mark_simulated(
@@ -689,7 +731,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 ``_skip_empty_update``).
         """
         if request is None:
             if update is None:
@@ -700,7 +743,15 @@ async def update_test_step(
                 field_mask.paths.append("error_info")
             request = UpdateTestStepRequest(test_step=test_step_proto, update_mask=field_mask)
 
-        if log_file is not None or simulate:
+        simulating = log_file is not None or simulate
+        if not request.update_mask.paths:
+            return self._skip_empty_update(
+                "TestStep",
+                existing,
+                self.simulate_update_test_step_response(request) if simulating else None,
+            )
+
+        if simulating:
             if log_file is not None:
                 await log_request_to_file(log_file, "UpdateTestStep", request)
             return self._mark_simulated(
@@ -892,7 +943,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 ``_skip_empty_update``).
         """
         if request is None:
             if update is None:
@@ -902,7 +954,15 @@ async def update_test_measurement(
                 test_measurement=test_measurement_proto, update_mask=field_mask
             )
 
-        if log_file is not None or simulate:
+        simulating = log_file is not None or simulate
+        if not request.update_mask.paths:
+            return self._skip_empty_update(
+                "TestMeasurement",
+                existing,
+                self.simulate_update_test_measurement_response(request) if simulating else None,
+            )
+
+        if simulating:
             if log_file is not None:
                 await log_request_to_file(log_file, "UpdateTestMeasurement", request)
             return self._mark_simulated(
@@ -1226,6 +1286,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 +1317,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 +1344,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}")

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):

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 SiftIgnoredInputWarning
 from sift_client.sift_types.test_report import (
     TestMeasurement,
     TestMeasurementCreate,
@@ -723,6 +724,57 @@ 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."""
+    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."""
+    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(SiftIgnoredInputWarning, 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.

python/lib/sift_client/_tests/sift_types/test_base.py

@@ -2,24 +2,28 @@
 
 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,
 )
 from sift.calculated_channels.v2.calculated_channels_pb2 import (
     CreateCalculatedChannelRequest,
 )
 
+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]):
@@ -226,6 +230,62 @@ 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(
+            SiftIgnoredInputWarning, 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(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(
+            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", SiftIgnoredInputWarning)
+            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", 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."""
 

python/lib/sift_client/errors.py

@@ -11,6 +11,14 @@ class SiftExperimentalWarning(SiftWarning):
     """Warning for experimental features."""
 
 
+class SiftIgnoredInputWarning(SiftWarning):
+    """Warning for input the SDK ignored.
+
+    Covers an unrecognized field on a Create/Update pydantic model, and an update
+    that names no fields to change.
+    """
+
+
 class SiftCredentialsError(ValueError):
     """Raised when Sift credentials cannot be resolved.