Found during review of #178 (T2-4).
save_project gained:
data = project.model_dump(by_alias=True, exclude_none=True, mode="json")
if project.schema_version is not None:
data["schema_version"] = project.schema_version
schema_version is a plain field with no alias and no exclude, so model_dump(by_alias=True, exclude_none=True) already emits it when it isn't None. Verified:
model_dump already contains schema_version? True -> 1
The two added lines change nothing. Preservation semantics — and the tests asserting them — hold identically without them.
Context
This is my own fault as reviewer, recorded so it doesn't look like a deliberate guard. The suggestion block was written against the ef9591a state, where save_project had an unconditional data["schema_version"] = SCHEMA_VERSION. As a replacement for that line it was correct; applied to the base branch it's a no-op.
Fix
Delete the two lines. If the intent is to document the invariant, a comment says it more honestly than code that appears to act but doesn't.
Mirrored in Jira: PTC-111 (Subtask of PTC-84, [Rewrite M2]). Dev work continues here.
Found during review of #178 (T2-4).
save_projectgained:schema_versionis a plain field with no alias and noexclude, somodel_dump(by_alias=True, exclude_none=True)already emits it when it isn't None. Verified:The two added lines change nothing. Preservation semantics — and the tests asserting them — hold identically without them.
Context
This is my own fault as reviewer, recorded so it doesn't look like a deliberate guard. The suggestion block was written against the
ef9591astate, wheresave_projecthad an unconditionaldata["schema_version"] = SCHEMA_VERSION. As a replacement for that line it was correct; applied to the base branch it's a no-op.Fix
Delete the two lines. If the intent is to document the invariant, a comment says it more honestly than code that appears to act but doesn't.
Mirrored in Jira: PTC-111 (Subtask of PTC-84, [Rewrite M2]). Dev work continues here.