Sets up the next session to migrate models.py DateTime columns to timezone=True and remove the defensive .replace(tzinfo=None) shell. Surfaces evidence and open questions (SQLAlchemy/SQLite round-trip behaviour, existing data migration, pw_changed_at marker semantics) rather than pre-baking an implementation plan that could bury false premises. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
164 lines
7.3 KiB
Markdown
164 lines
7.3 KiB
Markdown
# Timezone-aware datetime migration — handoff
|
|
|
|
> Follow-up artifact spawned by the timeago-shared-display work
|
|
> (`docs/superpowers/specs/2026-05-16-timeago-shared-display-design.md`).
|
|
> This is **not an implementation plan** — it is a brief for the next
|
|
> session to brainstorm the actual spec from evidence. Handoffs that
|
|
> pre-bake a plan tend to bury false premises; treat the "open
|
|
> questions" below as load-bearing.
|
|
|
|
## Context
|
|
|
|
`left4me`'s `l4d2web` package uses naive datetimes throughout the
|
|
runtime because `models.py` declares `DateTime` columns without
|
|
`timezone=True`. SQLAlchemy strips tzinfo on store (SQLite has no
|
|
native TZ type) and returns naive datetimes on read. The
|
|
`now_utc()` factory at `models.py:22-23` writes aware UTC, but the DB
|
|
layer collapses it back to naive on round-trip.
|
|
|
|
The pattern's cost is a defensive shell of `.replace(tzinfo=None)`
|
|
calls sprinkled across routes and services, plus a permanent mental
|
|
tax: *all naive datetimes in this codebase are UTC by convention, but
|
|
nothing in the type system says so.* Aware-everywhere would let the
|
|
defensive shell come down, the convention become explicit, and the
|
|
`_ensure_utc` defensive line in
|
|
`l4d2web/l4d2web/services/timeago.py` become a no-op safety net rather
|
|
than load-bearing.
|
|
|
|
This was discussed during the timeago-shared-display brainstorm and
|
|
deliberately scoped out to keep the diff focused. The user explicitly
|
|
asked for this handoff so the work isn't forgotten.
|
|
|
|
## What is currently known (evidence)
|
|
|
|
### The convention is documented in code
|
|
|
|
`l4d2web/l4d2web/routes/profile_routes.py:86-88` carries the canonical
|
|
explanation:
|
|
|
|
```python
|
|
# Strip tz so the marker matches what a subsequent DB read returns
|
|
# (SQLite DateTime columns don't preserve tzinfo).
|
|
user.password_changed_at = now_utc().replace(tzinfo=None)
|
|
new_marker = user.password_changed_at.isoformat()
|
|
```
|
|
|
|
The `pw_changed_at` session marker (`auth.py:52`, set in many routes)
|
|
is compared as an ISO string, so the on-DB and on-session
|
|
representations must match. Today they're both naive; an aware
|
|
migration must keep both sides in sync.
|
|
|
|
### Known workaround sites (definitive enumeration not done)
|
|
|
|
From a grep at the time of the timeago work:
|
|
|
|
- `l4d2web/l4d2web/routes/profile_routes.py:88` — password-changed-at write
|
|
- `l4d2web/l4d2web/routes/server_routes.py:210, 234` — `cutoff` / `recent_cutoff` for live-state staleness
|
|
- `l4d2web/l4d2web/routes/page_routes.py:174` — `cutoff` for the same
|
|
- `l4d2web/l4d2web/services/live_state_poller.py:37` — `_now()` helper returns naive UTC
|
|
|
|
The session that picks this up **must re-enumerate fresh** — the
|
|
list above is a starting point, not an exhaustive inventory. New
|
|
sites may have appeared after this handoff was written.
|
|
|
|
### Column declarations to flip
|
|
|
|
`l4d2web/l4d2web/models.py` declares ~20 `DateTime` columns without
|
|
`timezone=True`. Concrete count and list to be produced fresh
|
|
during the next session — column lists drift.
|
|
|
|
### What's *already* aware
|
|
|
|
- `now_utc()` (`models.py:22-23`) returns `datetime.now(UTC)`
|
|
- Most route-local time variables built via `datetime.now(UTC)` *before*
|
|
the defensive `.replace(tzinfo=None)` is applied
|
|
- `timeago.py::_ensure_utc` (added in this session) handles both
|
|
inputs defensively
|
|
|
|
### Tests touching naive datetimes
|
|
|
|
`l4d2web/tests/*.py` contain ~30+ instances of
|
|
`datetime.now(UTC).isoformat()` for the `pw_changed_at` session
|
|
marker, and at least a few `datetime.now(UTC).replace(tzinfo=None)` /
|
|
`now - timedelta(...)` patterns for fixture construction (e.g.
|
|
`test_servers.py:491, 535, 540`). These will need to be reviewed
|
|
once the production-side convention flips.
|
|
|
|
## Open questions the next session must answer
|
|
|
|
These are load-bearing. Do not draft an implementation plan that
|
|
assumes any particular answer.
|
|
|
|
1. **SQLite + `DateTime(timezone=True)` round-trip behaviour in
|
|
SQLAlchemy 2.x:** does storing an aware UTC datetime and reading it
|
|
back yield an *aware* UTC datetime (Python 3.13 + SQLAlchemy 2.x +
|
|
pysqlite)? Or does it still strip and require a custom
|
|
`TypeDecorator`? Spike with a throwaway test before assuming.
|
|
|
|
2. **Existing data:** the production SQLite DB at left4.me has rows
|
|
stored as text-without-timezone (current behaviour). If the new
|
|
column type changes how SQLAlchemy parses those strings on read,
|
|
we may need a one-shot data migration (rewrite values to include
|
|
`+00:00`). Verify behaviour on a copy of the production DB.
|
|
|
|
3. **Alembic migrations** (`l4d2web/migrations/` if it exists, or
|
|
wherever migrations live): do any existing migration files
|
|
reference `DateTime` columns? If yes, do those need to be updated
|
|
too? Any backfills that build datetime literals?
|
|
|
|
4. **The session `pw_changed_at` marker round-trip** is the highest-risk
|
|
site. It's compared as an ISO string in `auth.py:52`. After the
|
|
migration, both sides must produce the same ISO form (either both
|
|
naive UTC or both aware UTC). Plan must spell out the marker
|
|
format and where the conversion happens.
|
|
|
|
5. **Comparisons crossing the boundary** (DB-side aware vs Python-side
|
|
naive constants, or vice versa) will raise `TypeError` post-migration.
|
|
`server_routes.py:210` (`latest.last_seen_at >= cutoff`) is the
|
|
canonical example. Enumerate every such comparison; ensure both
|
|
sides get fixed in lockstep.
|
|
|
|
6. **Scope cut for tests:** do the test fixtures need to flip in the
|
|
same PR, or can the production code flip first and tests trail?
|
|
(Likely needs to flip together — comparisons in test assertions
|
|
will break if either side is mixed.)
|
|
|
|
## Suggested first steps for the next session
|
|
|
|
1. Read this handoff and the parent spec
|
|
(`docs/superpowers/specs/2026-05-16-timeago-shared-display-design.md`).
|
|
2. Re-enumerate `.replace(tzinfo=None)` sites:
|
|
`grep -rn 'replace(tzinfo=None)' l4d2web/`.
|
|
3. Re-enumerate `DateTime` column declarations:
|
|
`grep -n 'mapped_column(DateTime' l4d2web/l4d2web/models.py`.
|
|
4. Run the round-trip spike (open question #1) in a throwaway script
|
|
or test. Document the actual behaviour observed.
|
|
5. Take a copy of the production SQLite DB; read existing
|
|
datetime rows back through the candidate column definition.
|
|
Document whether they parse as aware (and to what timezone) or
|
|
require backfill.
|
|
6. Brainstorm scope and ordering with the user — only then write
|
|
the spec.
|
|
|
|
## Pointers — files to read first
|
|
|
|
- `docs/superpowers/specs/2026-05-16-timeago-shared-display-design.md`
|
|
— parent spec, "Out of scope" + "Follow-up spec" sections.
|
|
- `l4d2web/l4d2web/models.py:1-30, 22-23` — `now_utc()` factory, first
|
|
few column declarations.
|
|
- `l4d2web/l4d2web/routes/profile_routes.py:75-93` — the documented
|
|
workaround comment that gave the convention its name.
|
|
- `l4d2web/l4d2web/auth.py:40-60` — session marker comparison.
|
|
- `l4d2web/l4d2web/services/timeago.py` — `_ensure_utc` defensive
|
|
line that becomes a no-op after the migration.
|
|
|
|
## Anti-goals
|
|
|
|
- **Don't bundle this with unrelated cleanup.** It's a mechanical
|
|
refactor when sized correctly. Adding "while I'm in there"
|
|
scope grows the blast radius.
|
|
- **Don't skip the round-trip spike.** Believing SQLAlchemy's
|
|
`timezone=True` "just works" on SQLite without checking is exactly
|
|
the kind of false premise this handoff is shaped to prevent.
|
|
- **Don't trust this handoff's enumerations as exhaustive.** They
|
|
are starting points from a point-in-time grep. Re-run them.
|