diff --git a/src/specify_cli/workflows/overlays/schema.py b/src/specify_cli/workflows/overlays/schema.py index 0a018b7af0..969bbd94a9 100644 --- a/src/specify_cli/workflows/overlays/schema.py +++ b/src/specify_cli/workflows/overlays/schema.py @@ -60,7 +60,13 @@ def _validate_safe_id( def _parse_edit(edit_raw: dict[str, Any], idx: int) -> tuple[OverlayEdit | None, str | None]: """Parse a single edit dict into an OverlayEdit or an error string.""" - shorthand_keys = [key for key in _SHORTHAND_OPERATION_KEYS if key in edit_raw] + # Iterate ``edit_raw`` rather than ``_SHORTHAND_OPERATION_KEYS``: the latter + # is a frozenset, whose iteration order varies between processes with + # string-hash randomization, so the error messages built from this list + # named the offending keys in a different order on every run for the very + # same overlay file. Dict keys are always hashable, so the membership test + # is safe in this direction too. + shorthand_keys = [key for key in edit_raw if key in _SHORTHAND_OPERATION_KEYS] has_operation = "operation" in edit_raw operation: str | None = None diff --git a/tests/workflows/test_overlay_schema.py b/tests/workflows/test_overlay_schema.py index 77e0432eca..a91efb0a36 100644 --- a/tests/workflows/test_overlay_schema.py +++ b/tests/workflows/test_overlay_schema.py @@ -124,6 +124,57 @@ def test_multiple_operation_fields_rejected(self): assert overlay is None assert any("multiple" in e.lower() for e in errors), errors + @pytest.mark.parametrize( + "first,second", + [("remove", "insert_after"), ("insert_after", "remove")], + ids=["remove-first", "insert_after-first"], + ) + def test_multiple_operation_keys_reported_in_declaration_order( + self, first, second + ): + """The message must name the keys in the order the user wrote them. + + Collecting the keys by iterating the ``_SHORTHAND_OPERATION_KEYS`` + frozenset made the order depend on per-process string-hash + randomization, so the same overlay file produced a different message on + every run. Whatever fixed order a frozenset happens to have in a given + process, one of these two parametrizations contradicts it -- so this + pair fails deterministically without the fix, in every process. + """ + overlay, errors = validate_overlay_yaml( + { + "id": "ov", + "extends": "wf", + "edits": [{first: "a", second: "a"}], + } + ) + assert overlay is None + assert errors == [ + f"Edit at index 0 has multiple operation keys: {first!r}, {second!r}." + ] + + @pytest.mark.parametrize( + "first,second", + [("remove", "insert_after"), ("insert_after", "remove")], + ids=["remove-first", "insert_after-first"], + ) + def test_shorthand_mixed_with_operation_names_first_declared_key( + self, first, second + ): + """``shorthand_keys[0]`` must be the first key the user declared.""" + overlay, errors = validate_overlay_yaml( + { + "id": "ov", + "extends": "wf", + "edits": [{first: "a", second: "a", "operation": "replace"}], + } + ) + assert overlay is None + assert errors == [ + f"Edit at index 0 mixes shorthand operation key ({first!r}) " + f"with explicit 'operation' field." + ] + def test_invalid_operation_field_rejected(self): overlay, errors = validate_overlay_yaml( {