The two migrate session paths disagree on v0 event timestamps

[feiiiiii5]

🔴 Required Information

Describe the Bug:

ADK has two v0 migration implementations — migrate_from_sqlalchemy_pickle and migrate_from_sqlalchemy_sqlite — and they read the same v0 events.timestamp column under two conventions, so the same v0 database migrated through the two paths yields timestamps that differ by the host's UTC offset. adk migrate session calls only the pickle implementation (migration runner); the SQLite implementation is the separate module that SqliteSessionService recommends.

• the sqlite path calls StorageEvent.to_event(), which reads the column with utc_datetime_to_timestamp (naive value forced to UTC);
• the pickle path reads the raw row and calls timestamp.timestamp() on the naive value, i.e. local time, with a comment saying v0 wrote it that way.

Each is right for one era of v0 writer: before 1b8ed3d (2026-08-13) StorageEvent.from_event wrote naive local time, and 1b8ed3d changed the write path to naive UTC (and the read path to match) without touching the pickle migration. A naive value carries no marker of which convention wrote it.

Steps to Reproduce:

Build a v0 database whose event row holds a naive local datetime (what pre-1b8ed3d9 v0 stored), run each migration on its own copy, compare the results:

Reproduction script (self-contained, no network or model)

import tempfile
from datetime import datetime, timezone
from pathlib import Path

from google.adk.events.event_actions import EventActions
from google.adk.sessions.migration import migrate_from_sqlalchemy_pickle as mfsp
from google.adk.sessions.migration import migrate_from_sqlalchemy_sqlite as mfss
from google.adk.sessions.schemas import v0
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker

EPOCH = 1750000000.0
local_naive = datetime.fromtimestamp(EPOCH)  # what pre-1b8ed3d9 v0 stored
tmp = Path(tempfile.mkdtemp())


def build(path):
    engine = create_engine(f"sqlite:///{path}")
    v0.Base.metadata.create_all(engine)
    s = sessionmaker(bind=engine)()
    now = datetime.now(timezone.utc)
    s.add(v0.StorageSession(app_name="a", user_id="u", id="s", state={},
                            create_time=now, update_time=now))
    s.add(v0.StorageEvent(id="e1", app_name="a", user_id="u", session_id="s",
                          invocation_id="i", author="user",
                          actions=EventActions(), timestamp=local_naive))
    s.commit()
    s.close()
    return f"sqlite:///{path}"


mfsp.migrate(build(tmp / "src.db"), f"sqlite:///{tmp / 'dest_pickle.db'}")
mfss.migrate(build(tmp / "src_sqlite.db"), str(tmp / "dest_sqlite.db"))

Expected Behavior:

Both paths migrate the event to the same instant, and the convention is documented once.

Observed Behavior:

On a UTC+8 host, for EPOCH = 1750000000.0 (2025-06-15 15:06:40 UTC):

path	migrated value	vs. correct instant
pickle	2025-06-15 15:06:40 (naive UTC)	0
sqlite	1750028800.0	+28800 s (+8 h)

Every event migrated from a pre-1b8ed3d9 database lands 8 hours in the future, so ordering, turn durations and time-based filters are wrong afterwards. The shift is the host's offset at that local time, so it varies per row across DST boundaries.

Environment Details:

• ADK Library Version: main @ 044a1ec
• Desktop OS: macOS
• Python Version: 3.14.5
• Host timezone: Asia/Shanghai (UTC+8)

Model Information:

• Are you using LiteLLM: No
• Which model is being used: N/A (offline migration script)

---

🟡 Optional Information

Regression:

The sqlite path became wrong for old rows on 2026-08-13 (1b8ed3d); the two paths have disagreed since the pickle migration was written.

Logs:

dest_pickle.db: id=e1 column_timestamp='2025-06-15 15:06:40.000000'   # correct
dest_sqlite.db: id=e1 column_timestamp=1750028800.0                  # +8h

Additional Context:

test_migrate_from_sqlalchemy_pickle_reads_naive_timestamp_as_local pins the local reading on the pickle side, and its docstring gives as premise that StorageEvent.from_event used datetime.fromtimestamp — true until 1b8ed3d, not true now. I would rather ask than declare one side wrong, so three options:

1. treat the v0 column as naive local in both paths (right for every database written before 1b8ed3d, which I assume is the realistic input, and per-row DST-correct);
2. keep both on UTC (right only for rows written after 1b8ed3d);
3. add --v0-timestamps {local,utc}, default local, and document it.

I lean to 1 plus a note that post-1b8ed3d9 v0 rows will be shifted, since a silently 8-hour-wrong history is harder to notice than a documented conversion. Whichever you pick, I would also add a test that runs one fixture through both paths and asserts they agree.

Screenshots / Video: N/A.

How often has this issue occurred?:

• Always (100%) for pre-1b8ed3d9 v0 databases migrated with the sqlite path on a non-UTC host

[olaflaitinen]

I confirmed this by reading the code on main (044a1ec). The pickle path reads the naive v0 value as local time at migrate_from_sqlalchemy_pickle.py:155, while the sqlite path goes through StorageEvent.to_event() (migrate_from_sqlalchemy_sqlite.py:130), which reads it as UTC at v0.py:401. Note that adk migrate session only uses the pickle path (migration_runner.py:34-38); the sqlite module is the one SqliteSessionService points users to (sqlite_session_service.py:187-195).

On the write side, the convention depends on the ADK version that wrote the row: StorageEvent.from_event used datetime.fromtimestamp (naive local) until 1b8ed3d9, first released in v2.7.0, and naive UTC since then, while the 1.x line (v1.39.1) still writes naive local. Since DatabaseSessionService keeps reading and writing legacy v0 databases without migration (database_session_service.py:296-305), a single v0 database upgraded across v2.7.0 can contain rows in both conventions, and the runtime read path now also interprets older rows as UTC.

The two straightforward fixes are to make both migration paths read v0 timestamps as UTC (matching the current v0 read/write path), or to make both read them as local time (matching every row written before v2.7.0 and by 1.x).

Could a maintainer decide which interpretation should be canonical for v0 data? Once that is settled, I would like to take this on: align both paths, update the existing pickle test, and add a test that runs one fixture through both migrations and asserts identical timestamps.

[feiiiiii5]

Thanks for checking the historical writer versions and the CLI entry point. The local-versus-UTC decision still needs maintainer direction. Please link a PR here once that is settled; I’ll keep this report open for that decision.

[surajksharma07]

Reproduced this on main (IST host, the sqlite path lands +19800s for a pre-2.7.0 row). Also checked v2.6.3 and there it's the pickle output that reads back shifted so the two paths have never agreed end to end.

Lean towards option 1 too. As a workaround in migrate_from_sqlalchemy_sqlite, after to_event(), re-read a naive storage_event.timestamp with .timestamp() (local), same as the pickle path. With that both paths agree locally and the sessions tests still pass.

@feiiiiii5 could you try it on your actual database and let me know if the migrated times look right? Also was that database ever written to by v2.7.0+? Those rows would end up shifted the other way.

Flagging the local vs UTC call to the sessions owners. If you pick up the PR please test it well first including a test that runs one fixture through both paths under a non-UTC TZ.

[feiiiiii5]

I cannot answer the database question usefully — I never had a real production v0 database. The report came from reading the two migration modules against each other, and the evidence in #7300 is the two code paths plus the writer-version history you also traced. So there is nothing on my side to validate the timestamps against.

What I can do is narrow the choice, and I think your option 1 is the one to put to the sessions owners.

The pre-2.7.0 rows are the ones at risk in the other direction. If a database was only ever written by 1.x or by 2.x before 2.7.0, its timestamp column holds naive local time. Reading those as UTC shifts every event by the host's UTC offset — your +19800s on IST. Making both paths read local time is the only interpretation that is correct for every row that can exist in such a database, because 2.7.0 is the only release that writes naive UTC, and a database has to have been touched by it to hold such rows.

The counter-argument I can still see is the runtime read path. StorageEvent.to_event() reads as UTC today, and DatabaseSessionService still reads legacy v0 rows without migrating, so agreeing on local would leave migration and runtime disagreeing for pre-2.7.0 data. That is a real cost, but it is a fixable one, and it points at a second change in v0.py rather than at the migration paths. Option 1 also keeps the migration honest about what it holds: it copies rows without changing them.

On your workaround: re-reading storage_event.timestamp with .timestamp() after to_event() would make the two paths agree, but it bakes the local-time reading into a third place and leaves to_event()'s own interpretation uncorrected — a caller that skips the workaround still gets the shifted value. I would rather the two migrations agree and the disagreement with the runtime be visible in one place.

I agree this needs the sessions owners, and I agree it should not be picked up before they decide. I have kept the issue open for that and have not opened a PR. Your point about testing under a non-UTC TZ is the right constraint for whoever does take it — TZ and time.tzset() in the test, not just a machine that happens to be in one.

[olaflaitinen]

Thanks for reproducing this. Option 1 (naive local in both paths) makes sense to me as well, for the reason @feiiiiii5 gives: every row written by 1.x or by 2.x before 2.7.0 is naive local, and only a database touched by 2.7.0 or later can hold naive UTC rows.

I will hold off on a PR until the sessions owners decide. When they do, I plan to put the naive-local conversion in one helper used by both migration modules instead of patching the value after to_event(), so the two migrations agree in a single place and the remaining difference with the runtime read path in v0.py stays visible. The test will run one v0 fixture through both paths with TZ set and time.tzset() called inside the test, for two non-UTC zones including one with DST.

[feiiiiii5]

Agreed on all three points, and thank you for holding off rather than opening a second PR while the sessions owners decide — that is the part I would have asked for.

Putting the conversion in one shared helper is better than anything I was going to propose. My workaround (re-reading storage_event.timestamp after to_event()) would have made the two migrations agree but left v0.py's own read path disagreeing with both, and that disagreement would have shown up as a third bug report later rather than as one visible difference. Your version keeps it in one place and leaves the remaining gap legible.

The TZ + time.tzset() test with two non-UTC zones is the part I would not have thought to do — I was going to suggest one. A single zone would pass with a fix that is off by the host's offset, and DST is where a hardcoded offset breaks.

One thing to watch that I did not check: DatabaseSessionService still reads v0 rows without migrating, and it goes through StorageEvent.to_event(). If you align the migrations to naive local, the runtime read path becomes the odd one out for every pre-2.7.0 database. I would not change v0.py in the same PR — it is a different decision and the maintainers may want it separately — but it is worth saying explicitly in the PR description which readers will still see shifted timestamps after the fix lands, so the fix does not read as "this is now correct" when it is "these two paths now agree with each other."

I have not written a PR and will not open a competing one. If your PR stalls or the sessions owners pick a different option, say so and I will pick it up with the evidence in this thread.

[surajksharma07]

Fair point @feiiiiii5. The snippet was only meant as a stop-gap for someone migrating today not as the PR shape. One shared naive-local helper for both migrations with the v0.py runtime read called out in the PR description is cleaner. Nothing has changed upstream since: origin/main is still 044a1ec so this still affects v2.7.0 through v2.10.0.

@olaflaitinen one thing for the DST zone in the test: naive local is lossy in the fall-back hour. Checked America/New_York on 2025-11-02: 05:30Z and 06:30Z are both stored as 01:30 and any local-time reader returns the first so the second row comes back 3600s early. No helper can recover that. I'd keep the DST fixture out of the repeated hour (or assert fold=0 explicitly) and list it next to the runtime-read caveat. The spring-forward gap can't occur in real v0 data.

cc @DeanChensj for the local vs UTC decision on v0 timestamps. Option 1 + the shared helper is where this thread has landed and @olaflaitinen is ready to pick it up once that's settled.

[olaflaitinen]

Thanks, good catch. I'll keep the DST fixtures away from the repeated fall-back hour, add a separate test that pins the ambiguous 01:30 case in America/New_York on 2025-11-02 to the first occurrence (fold=0) so the behavior is explicit, and list the fall-back hour next to the runtime-read caveat in the PR description.