fix(api): fix SimulatedProbeResult ser/deser

[sfoster1]

We weren't deserializing the simulation standin properly, which broke runlog downloading. Now we are.

Also, we shouldn't be dumping it to the runlog anymore. That was happening because of pydantic/pydantic#6830 which causes mis-serialization of fields typed as Model | NativeType, where NativeType is some scalar python type (i.e. float, int) and Model is some pydantic model. With the model first, serializing the container will always result in the serialization of the default construction of the model, even if the attribute actually contained the native type. Flipping the order makes it work correctly, for some reason. Luckily this is consistent enough to be testable.

Closes RQA-4147
Closes EXEC-1429
Apparently,
Closes EXEC-1358

[SyntaxColoring]

Thank you. Just minor suggestions.

I think this closes EXEC-1358, yeah? Meaning we can delete this workaround:

opentrons/api/src/opentrons/protocol_engine/types/liquid_level_detection.py

Lines 116 to 131 in b977033

	# TODO(cm): 3/21/25: refactor SimulatedLiquidProbe in a way that
	# doesn't require models like this one that are just using it to
	# need a custom validator
	@field_validator("probed_height", "probed_volume", mode="before")
	@classmethod
	def validate_simulated_probe_result(
	cls, input_val: object
	) -> LiquidTrackingType | None:
	"""Return the appropriate input to WellInfoSummary from json data."""
	if input_val is None:
	return None
	if isinstance(input_val, LiquidTrackingType):
	return input_val
	if isinstance(input_val, str) and input_val == "SimulatedProbeResult":
	return SimulatedProbeResult()
	raise ValueError(f"Invalid input value {input_val} to WellInfoSummary")

[SyntaxColoring]

Nit: Can we use object instead of Any here? It doesn't matter so much for the return type, but it's meaningfully safer for the data arg.

The examples in the Pydantic docs recommend Any but I think that's a bad recommendation.

Suggested change

	def model_validator(cls, data: Any) -> Any:
	def validate_model(cls, data: object) -> object:

(Also renaming to validate_model for symmetry with the existing serialize_model)

[SyntaxColoring]

To sate my paranoia, can we expand this test to cover the case where the JSON has a value that doesn't represent a SimulatedProbeResult?

Something like:

Suggested change

	class TestModel(BaseModel):
	"""Test model for deserializing SimulatedProbeResults."""
	value: float | SimulatedProbeResult
	def test_deserializes_simulated_liquid_probe() -> None:
	"""Should be able to roundtrip our simulated results."""
	base = TestModel(value=SimulatedProbeResult())
	serialized = base.model_dump_json()
	deserialized = TestModel.model_validate_json(serialized)
	assert isinstance(deserialized.value, SimulatedProbeResult)
	def test_simulated_probe_result_serdes() -> None:
	"""Should be able to roundtrip our simulated results."""
	class TestModel(BaseModel):
	"""Test model for deserializing SimulatedProbeResults."""
	value: float | SimulatedProbeResult
	# Should be able to roundtrip our simulated results:
	base = TestModel(value=SimulatedProbeResult())
	serialized = base.model_dump_json()
	deserialized = TestModel.model_validate_json(serialized)
	assert isinstance(deserialized.value, SimulatedProbeResult)
	# Should allow parsing to fall back to other union elements (in this case a float):
	assert TestModel.model_validate_json('{"value": 1.23}') == TestModel(value=1.23)
	# Parsing should totally error out if the value doesn't fit into any of the union elements:
	with pytest.raises(ValidationError):
	TestModel.model_validate_json(
	'{"value": "not a valid float or SimulatedProbeResult"}'
	)

[caila-marashaj]

weird that it works like this but nice !