Follow-up to #216 / #272.
Context
#216 was sequences_annotations.updated_at never being stamped. #272 fixed it — and the audit alongside it — by moving the stamp onto the column itself:
updated_at: Optional[datetime] = Field(
default=None,
sa_column=Column(DateTime(timezone=True), onupdate=lambda: datetime.now(UTC)),
)
SequenceGroup was left out of that PR to keep the diff to the reported issue. It still stamps by hand.
Current state
Three sites mutate a group, and all three stamp correctly today — this is not a live bug for these paths:
sequence_annotations.py:913 — group-label write during validated-group fan-out
sequence_annotations.py:1400 — bulk-annotate group label
sequence_groups.py:302 — PATCH /sequence_groups/{id} (guarded by if changes:)
Why it's worth fixing anyway
Correctness depends on every future author remembering a line that nothing enforces. That is precisely how #216 happened: the annotation stamp was never wired up, and because NULL is a silent value rather than an error, it went unnoticed until it cost a debugging detour during the #212 verification session. The group model is one forgotten line away from the same failure, and the same silence.
One actual gap
Membership changes do not stamp the group at all. DELETE /sequence_groups/{group_id}/members/{sequence_id} (sequence_groups.py:336) and the re-include endpoint (:361) mutate only the Sequence row — seq.sequence_group_id / seq.is_group_excluded — so the group's own updated_at is untouched when its membership changes.
Worth deciding deliberately rather than by accident: should "group last modified" cover membership, or only the group's own columns? Note onupdate alone would not close this, since no UPDATE is emitted against sequence_groups on those paths — it needs an explicit touch either way.
Proposed fix
- Add
onupdate=lambda: datetime.now(UTC) to SequenceGroup.updated_at (Python-side default, no migration).
- Delete the three manual
group.updated_at = datetime.now(UTC) lines.
- Add a test that a group PATCH stamps
updated_at — and, if we decide membership counts, one for the member endpoints too.
Also worth a decision: User
crud_user.py:74,107 hand-stamps User.updated_at the same way. Converging it on onupdate would be consistent, but note that an explicit assignment always wins over onupdate, so the manual lines must be removed for the hook to take effect — leaving both in place would be misleading rather than redundant.
Follow-up to #216 / #272.
Context
#216 was
sequences_annotations.updated_atnever being stamped. #272 fixed it — and the audit alongside it — by moving the stamp onto the column itself:SequenceGroupwas left out of that PR to keep the diff to the reported issue. It still stamps by hand.Current state
Three sites mutate a group, and all three stamp correctly today — this is not a live bug for these paths:
sequence_annotations.py:913— group-label write during validated-group fan-outsequence_annotations.py:1400— bulk-annotate group labelsequence_groups.py:302—PATCH /sequence_groups/{id}(guarded byif changes:)Why it's worth fixing anyway
Correctness depends on every future author remembering a line that nothing enforces. That is precisely how #216 happened: the annotation stamp was never wired up, and because NULL is a silent value rather than an error, it went unnoticed until it cost a debugging detour during the #212 verification session. The group model is one forgotten line away from the same failure, and the same silence.
One actual gap
Membership changes do not stamp the group at all.
DELETE /sequence_groups/{group_id}/members/{sequence_id}(sequence_groups.py:336) and there-includeendpoint (:361) mutate only theSequencerow —seq.sequence_group_id/seq.is_group_excluded— so the group's ownupdated_atis untouched when its membership changes.Worth deciding deliberately rather than by accident: should "group last modified" cover membership, or only the group's own columns? Note
onupdatealone would not close this, since no UPDATE is emitted againstsequence_groupson those paths — it needs an explicit touch either way.Proposed fix
onupdate=lambda: datetime.now(UTC)toSequenceGroup.updated_at(Python-side default, no migration).group.updated_at = datetime.now(UTC)lines.updated_at— and, if we decide membership counts, one for the member endpoints too.Also worth a decision:
Usercrud_user.py:74,107hand-stampsUser.updated_atthe same way. Converging it ononupdatewould be consistent, but note that an explicit assignment always wins overonupdate, so the manual lines must be removed for the hook to take effect — leaving both in place would be misleading rather than redundant.