What happens
CloudDatabase.to_dict in src/mock_vws/database.py omits four fields which CloudDatabase carries:
total_recoscurrent_month_recosprevious_month_recosreco_threshold
from_dict therefore restores them to their class defaults rather than to the values the original object had. ImageTarget.to_dict in src/mock_vws/target.py drops the same three reco fields plus reco_rating.
So from_dict(to_dict(database)) != database whenever any of those fields is non-default.
Why it matters
Nothing breaks today, because every one of those fields is 0 or "" for every database the mock creates, so the lossy round trip is invisible.
It stops being invisible with #3356, which is about making recognition counts seedable so that reco reports can be non-empty. The Docker backend round-trips every database through to_dict and from_dict on every request, in get_all_cloud_databases at src/mock_vws/_flask_server/vws.py:110. A seeded reco count would survive in the in-process backend and be silently reset to zero in Docker, which is exactly the kind of backend-specific difference that is expensive to debug.
reco_threshold has the same problem and is already configurable on the dataclass, so it is reachable today by anyone constructing a CloudDatabase directly.
Suggested resolution
Add the missing fields to CloudDatabaseDict, ImageTargetDict and the two to_dict / from_dict pairs.
More usefully, add a round-trip test asserting from_dict(to_dict(x)) == x for both classes with every field set to a non-default value. That pins the property rather than the current field list, so the next field added to either dataclass cannot quietly fall out of the serialisation. Doing this before #3356 means that work does not have to discover the problem first.
Note that ImageTarget round trips are lossy in one further respect by design: to_dict writes the computed tracking_rating and from_dict rebuilds the target with a HardcodedTargetTrackingRater, so the rater identity is not preserved. That is deliberate and should stay, but the test will need to account for it.
What happens
CloudDatabase.to_dictinsrc/mock_vws/database.pyomits four fields whichCloudDatabasecarries:total_recoscurrent_month_recosprevious_month_recosreco_thresholdfrom_dicttherefore restores them to their class defaults rather than to the values the original object had.ImageTarget.to_dictinsrc/mock_vws/target.pydrops the same three reco fields plusreco_rating.So
from_dict(to_dict(database)) != databasewhenever any of those fields is non-default.Why it matters
Nothing breaks today, because every one of those fields is
0or""for every database the mock creates, so the lossy round trip is invisible.It stops being invisible with #3356, which is about making recognition counts seedable so that reco reports can be non-empty. The Docker backend round-trips every database through
to_dictandfrom_dicton every request, inget_all_cloud_databasesatsrc/mock_vws/_flask_server/vws.py:110. A seeded reco count would survive in the in-process backend and be silently reset to zero in Docker, which is exactly the kind of backend-specific difference that is expensive to debug.reco_thresholdhas the same problem and is already configurable on the dataclass, so it is reachable today by anyone constructing aCloudDatabasedirectly.Suggested resolution
Add the missing fields to
CloudDatabaseDict,ImageTargetDictand the twoto_dict/from_dictpairs.More usefully, add a round-trip test asserting
from_dict(to_dict(x)) == xfor both classes with every field set to a non-default value. That pins the property rather than the current field list, so the next field added to either dataclass cannot quietly fall out of the serialisation. Doing this before #3356 means that work does not have to discover the problem first.Note that
ImageTargetround trips are lossy in one further respect by design:to_dictwrites the computedtracking_ratingandfrom_dictrebuilds the target with aHardcodedTargetTrackingRater, so the rater identity is not preserved. That is deliberate and should stay, but the test will need to account for it.