Skip to content

fix(timeline): normalize naive datetimes from DB to offset-aware - #29

Open
rogerdigital wants to merge 1 commit into
Einsia:mainfrom
rogerdigital:fix-timeline-naive-datetime
Open

fix(timeline): normalize naive datetimes from DB to offset-aware#29
rogerdigital wants to merge 1 commit into
Einsia:mainfrom
rogerdigital:fix-timeline-naive-datetime

Conversation

@rogerdigital

Copy link
Copy Markdown
Contributor

Summary

  • get_latest_end() and _row_to_block() could return naive datetimes when stored ISO strings lacked a TZ offset, causing TypeError on comparison with aware datetimes from _stem_to_dt() and _now() — this silently breaks the entire timeline pipeline after the first block is written
  • Add defensive astimezone() normalization in all DB read paths and in _capture_stem_in_window() as a belt-and-suspenders guard

Tests

  • test_get_latest_end_naive_string_returns_aware — naive ISO string → aware datetime
  • test_get_latest_end_aware_string_stays_aware — aware stays aware
  • test_get_latest_end_no_rows — empty table → None
  • test_row_to_block_naive_datetime_becomes_aware — query_recent returns aware blocks
  • test_capture_stem_in_window_naive_start_end — naive start/end don't crash, correct window membership

All 19 tests pass, ruff clean.

Closes #24

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request ensures that datetimes processed in the timeline aggregator and store are timezone-aware by normalizing naive datetimes using the .astimezone() method. These changes are applied to _capture_stem_in_window, get_latest_end, and _row_to_block, and are accompanied by new unit tests to verify correct handling of naive versus aware datetimes. Feedback indicates that the timestamp derived from the filename stem in the aggregator should also be normalized to prevent potential TypeError exceptions during comparison if it remains naive while the window bounds are aware.

Comment on lines +47 to +50
if start.tzinfo is None:
start = start.astimezone()
if end.tzinfo is None:
end = end.astimezone()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The comparison start <= ts < end on line 51 will still raise a TypeError if ts is naive and start/end are aware (or vice versa). While _stem_to_dt usually returns aware datetimes, it can return a naive one if the filename stem is exactly 20 characters and lacks a valid 'p' or 'm' offset prefix (e.g., 2026-04-21T17-07-32_). To be truly 'belt-and-suspenders' as intended by this PR, ts should also be normalized to an offset-aware datetime before the comparison.

Suggested change
if start.tzinfo is None:
start = start.astimezone()
if end.tzinfo is None:
end = end.astimezone()
if ts.tzinfo is None:
ts = ts.astimezone()
if start.tzinfo is None:
start = start.astimezone()
if end.tzinfo is None:
end = end.astimezone()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4c74c74 — added ts normalization before comparison.

get_latest_end() and _row_to_block() could return naive datetimes
when stored ISO strings lacked a TZ offset, causing TypeError on
comparison with aware datetimes. Normalize at all comparison
boundaries: DB read paths, _row_to_block, and _capture_stem_in_window.

Closes Einsia#24
@rogerdigital
rogerdigital force-pushed the fix-timeline-naive-datetime branch from 4c74c74 to 67d212b Compare May 9, 2026 16:07
@rogerdigital

Copy link
Copy Markdown
Contributor Author

@Xiao-ao-jiang-hu The ts normalization is already included in commit 67d212b (line 47-48 in aggregator.py). Gemini's review was on an earlier version — all three variables (ts, start, end) are normalized before comparison now. Ready for re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timeline aggregator crashes with TypeError on every tick after first block: naive vs aware datetime in get_latest_end

1 participant