Skip to content

chore: make trace_cli datetime.fromtimestamp() tz-aware (ruff DTZ006)#134

Open
alexzhu0 wants to merge 1 commit intoFlowElement-ai:mainfrom
alexzhu0:chore/dtz-aware-trace-cli-timestamps
Open

chore: make trace_cli datetime.fromtimestamp() tz-aware (ruff DTZ006)#134
alexzhu0 wants to merge 1 commit intoFlowElement-ai:mainfrom
alexzhu0:chore/dtz-aware-trace-cli-timestamps

Conversation

@alexzhu0
Copy link
Copy Markdown
Contributor

@alexzhu0 alexzhu0 commented May 1, 2026

Summary

Make the 4 datetime.fromtimestamp() calls in m_flow/debug/trace_cli.py explicitly timezone-aware, using the locale-preserving pattern:

datetime.fromtimestamp(x, tz=timezone.utc).astimezone()

This is ruff rule DTZ006datetime.fromtimestamp(x) without a tz= silently uses the system local timezone, which is what Python's own docs call out as a footgun.

Behavior

These calls drive CLI output that's meant to display the operator's local wall-clock time (e.g. [14:23:05.123] event). The new form preserves that exactly — tz=timezone.utc parses the epoch unambiguously, then .astimezone() converts to the system's local zone. The resulting strftime() string is byte-identical to what the naive version produced.

Verified:

$ python -c "
from datetime import datetime, timezone
ts = 1730000000.0
a = datetime.fromtimestamp(ts).strftime('%Y-%m-%d %H:%M:%S')
b = datetime.fromtimestamp(ts, tz=timezone.utc).astimezone().strftime('%Y-%m-%d %H:%M:%S')
print(a == b)  # True
"
True

Scope

Sibling PR #129 fixed the datetime.utcnow() (DTZ003) occurrences in production code. This PR tackles the remaining DTZ006 sites, which all live in the trace debug CLI.

$ ruff check m_flow/debug/trace_cli.py --select DTZ
All checks passed!

I affirm that all code in every commit of this pull request conforms to the terms of the M-flow Developer Certificate of Origin

@FlowElement-ai
Copy link
Copy Markdown
Owner

Thanks for this cleanup! Two small things hold this back from merging:

  1. DCO: I added the affirmation to the PR body for you, the Verify DCO statement check should pass now.
  2. Fork PR lightweight checks still fails because m_flow/ingestion/documents/classify_documents.py (the file your dict.fromkeys autofix touches) is unformatted under the repo-pinned ruff 0.13.1 in uv.lock. Newer ruff joins the 16-element image-extension list into one line (>120 chars), but 0.13.1 wants the list re-expanded.

To unblock, please run locally and push:

uv sync --group dev
uv run ruff format m_flow/ingestion/documents/classify_documents.py
git commit --amend --no-edit && git push --force-with-lease

Same idea applies to your other open PRs that 're hitting the same fork-lite check. After that the queue should go green and we can merge the batch. Thanks!

FlowElement-ai added a commit that referenced this pull request May 1, 2026
#137)

The repo's `uv.lock` pins `ruff==0.13.1` and the fork-lite CI job runs
`uv run ruff format --check .` against the whole tree. After PR #131
(``dict.fromkeys`` autofix) was merged via a different ruff version, the
16-element ``_IMAGE_EXTS`` list ended up on a single line that exceeds
the project's 120-char limit. ruff 0.13.1 wants the list re-expanded;
the resulting drift was silently breaking every subsequent fork PR's
``Fork PR lightweight checks`` step (observed on PRs #131, #134, #135).

Re-run ``ruff==0.13.1 format`` on the file. Tree is now ``1061 files
already formatted`` under the locked ruff version.

Co-authored-by: Junting Hua <juntinghua@Juntings-MacBook-Pro.local>
FlowElement-ai added a commit that referenced this pull request May 1, 2026
…t upper bound (#142)

`pyproject.toml` declared `ruff>=0.9.0,<=0.15.10` but `uv.lock` was still
pinned at 0.13.1, releases behind. Contributors who installed ruff
manually (or whose editor picked up a newer ruff) ran format autofixes
that 0.13.1 would refuse, producing CI noise on every fork PR even when
the code was logically correct.

This was the direct root cause of:

- PR #137 (the one-off `classify_documents.py` reformat I had to land)
- The persistent `Fork PR lightweight checks` failures on PR #131,
  #134, and #135 — all caused by `ruff==0.13.1` reformatting files the
  contributors' newer ruff considered correct
- Several earlier rounds of "fork-lite is red even though the diff
  looks fine" debugging

`uv lock --upgrade-package ruff` snaps the lock file to the constraint
ceiling (0.15.10). Verification on main:

- `ruff==0.15.10 format --check .` → 1061 files already formatted
- `ruff==0.15.10 check .` → all checks passed

So the upgrade introduces zero source code change and aligns the CI
ruff with what contributors and their editors are running.

Co-authored-by: Junting Hua <juntinghua@Juntings-MacBook-Pro.local>
@FlowElement-ai
Copy link
Copy Markdown
Owner

Quick update: I just merged #142 which bumps the lockfile ruff from 0.13.1 → 0.15.10 to match the pyproject.toml ceiling. That was the version that disagreed with your autofix output on every recent fork PR.

Triggered a fork-lite rerun on this PR with the new lock — it still failed, which means your ruff is even newer than 0.15.10. The smallest unblock is one of:

# either downgrade locally to match the repo
uv tool install ruff@0.15.10
ruff format .

# or just re-run with whatever ruff comes from the project venv
uv run ruff format .

git add -u && git commit --amend --no-edit && git push --force-with-lease

After that the fork-lite check should go green and we can merge. Sorry for the back and forth on this one — the lockfile drift was the underlying cause.

`datetime.fromtimestamp(x)` is ambiguous — it implicitly uses the system
local timezone. Python's own docs suggest passing `tz=` explicitly.

These are CLI debug views meant to display the operator's local time,
so the semantics-preserving form is:

    datetime.fromtimestamp(x, tz=timezone.utc).astimezone()

which produces an aware datetime in the system's local timezone —
`strftime()` output is byte-identical to the naive version (verified
with a representative epoch) but the behavior no longer depends on
an implicit default.

ruff --select DTZ006 on m_flow/debug/trace_cli.py is now clean.
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.

2 participants