Skip to content

SequenceGroup.updated_at relies on hand-written stamps at every call site #273

Description

@Chouffe

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:302PATCH /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

  1. Add onupdate=lambda: datetime.now(UTC) to SequenceGroup.updated_at (Python-side default, no migration).
  2. Delete the three manual group.updated_at = datetime.now(UTC) lines.
  3. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions