Reading a sheet may rewrite its updated_at (and restart a half-made character's 30-day clock) #988

New issue

Closed

opened 2026-10-01 15:54:14 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-10-01 15:54:14 +00:00

Collaborator

Observed while building lane 11 PR 17 (#737), not yet reproduced or investigated.

What was seen. In the demo seed's tests, reading some seeded sheets through sheet_service's read path (sheet_out) left their updated_at at the time of the read rather than the time of their last save.

Why it would matter.

CharacterSheet.updated_at has onupdate=utcnow (app/models/sheet.py), so any column write during a read moves it. A GET should not write.
A character still being started is cleared 30 days after updated_at (sheet_creation.KEEP_FOR, D249). If a read moves it, opening Members (which lists them) or the sheet would restart that clock, and "Carry on with …" would never expire for a sheet somebody keeps looking at.
"Saved at …" and version timestamps read from the same column family.

To do.

Reproduce: read a sheet twice through route #1 with a clock patch between, and assert updated_at is unchanged; do the same for a creating sheet via GET …/sheets/creations.
If it moves, find the write (candidates: the lazy projection rebuild refresh_projections, a resources refill on read, or an attribute assigned on the ORM object during evaluation) and stop it touching the sheet row, or exclude updated_at the way write_projection already does for the page.
Check whether it predates PR 17.

Low priority unless step 1 shows the 30-day clock is affected.

Observed while building lane 11 PR 17 (#737), **not yet reproduced or investigated**. **What was seen.** In the demo seed's tests, reading some seeded sheets through `sheet_service`'s read path (`sheet_out`) left their `updated_at` at the time of the read rather than the time of their last save. **Why it would matter.** - `CharacterSheet.updated_at` has `onupdate=utcnow` (`app/models/sheet.py`), so any column write during a read moves it. A GET should not write. - A character still being started is cleared 30 days after `updated_at` (`sheet_creation.KEEP_FOR`, D249). If a read moves it, opening Members (which lists them) or the sheet would restart that clock, and "Carry on with …" would never expire for a sheet somebody keeps looking at. - "Saved at …" and version timestamps read from the same column family. **To do.** 1. Reproduce: read a sheet twice through route #1 with a clock patch between, and assert `updated_at` is unchanged; do the same for a `creating` sheet via `GET …/sheets/creations`. 2. If it moves, find the write (candidates: the lazy projection rebuild `refresh_projections`, a resources refill on read, or an attribute assigned on the ORM object during evaluation) and stop it touching the sheet row, or exclude `updated_at` the way `write_projection` already does for the page. 3. Check whether it predates PR 17. Low priority unless step 1 shows the 30-day clock is affected.

claude-bot referenced this issue

2026-10-01 16:13:25 +00:00

Lane 11 PR 17: the J7 journey and demo seed wave B (#737) #989

claude-bot commented

2026-10-01 16:16:39 +00:00

Author

Collaborator

Investigated on build/lane11-17-j7 (dc59a5b5): not reproducible; closing.

Two scratch tests against the real routes, each setting updated_at ten days back with a plain UPDATE, then reading and re-selecting the column:

A character being started: GET …/sheets/creations, GET …/sheets/{id}/creation and GET …/members each leave updated_at exactly as it was. The 30-day clock (D249) is not restarted by looking.
A finished sheet: GET …/lore/{id}/sheet as its player, as a GM and as another member leaves it unchanged.

Reading the code agrees: every assignment to a CharacterSheet column is in a write path (patch, apply_resources, rests, creation, level up). Resource changes (HP −5 and so on) do move updated_at through the model's onupdate, which is intended.

What the seed's test saw was most likely the seed's own doing: it back-dates rows through the ORM, and a later flush that touches another column on the same row lets onupdate=utcnow replace the back-dated value. That is a seed ordering detail, not a product behaviour, and the seed already dates its histories after the last write.

The scratch tests were not committed; test_q7_an_unfinished_character_is_cleared_after_30_days already covers the clear itself.

Investigated on `build/lane11-17-j7` (dc59a5b5): **not reproducible; closing.** Two scratch tests against the real routes, each setting `updated_at` ten days back with a plain `UPDATE`, then reading and re-selecting the column: - **A character being started:** `GET …/sheets/creations`, `GET …/sheets/{id}/creation` and `GET …/members` each leave `updated_at` exactly as it was. The 30-day clock (D249) is not restarted by looking. - **A finished sheet:** `GET …/lore/{id}/sheet` as its player, as a GM and as another member leaves it unchanged. Reading the code agrees: every assignment to a `CharacterSheet` column is in a write path (`patch`, `apply_resources`, rests, creation, level up). Resource changes (HP −5 and so on) do move `updated_at` through the model's `onupdate`, which is intended. What the seed's test saw was most likely the seed's own doing: it back-dates rows through the ORM, and a later flush that touches another column on the same row lets `onupdate=utcnow` replace the back-dated value. That is a seed ordering detail, not a product behaviour, and the seed already dates its histories after the last write. The scratch tests were not committed; `test_q7_an_unfinished_character_is_cleared_after_30_days` already covers the clear itself.

claude-bot closed this issue

2026-10-01 16:16:43 +00:00

claude-bot referenced this issue

2026-10-01 16:37:07 +00:00

Build lane 11: stat blocks and character sheets (WIKI-19) #737