Decide the refresh mtime match from the stored value, not a fixed window

[yarikoptic]

Builds on #1910 — targets its branch. Not a fix for #1907 (that is #1910's); it addresses the inverse defect the fixed window introduces, and applies the same reasoning to the second site that shares the assumption.

MTIME_TOLERANCE = 2.0 absorbs the coarsest filesystem granularity, but applies that window everywhere — including on filesystems that store the mtime exactly. There, an asset genuinely replaced under two seconds later at an identical size compares "same" and is silently not refreshed. Unlike #1907, nobody would ever notice: the symptom is a stale file, not a slow download.

Since the gap can only have been introduced by the destination quantizing the value we set with os.utime(), require it to be a gap the observed value can account for: the stored mtime must be a multiple of some known granularity that is itself wider than the gap. A filesystem truncating to whole seconds cannot have produced a stored …21.651, so against that value even a millisecond of drift is a real change. No probing of the destination and no state — the evidence is the value already being compared.

Added as is_same_mtime() beside is_same_time() in utils.py, since it replaces a use of the latter and answers the same kind of question.

_populate_dandiset_yaml() compared its own os.utime()-written mtime with a bare >=, so it gets the same treatment via _is_local_file_current(). Not a straight substitution: that comparison is one-sided on purpose — a genuinely newer local copy is current too, and an equality test alone would call it "not the record" and clobber exactly the local edits the check exists to protect. The >= arm stays; is_same_mtime() is added as the second.

Why not sense the filesystem (as #1908 did)

Three reasons a probe does not pay for itself:

1. Bootstrap. The comparison happens at the start of a refresh run, before we have written anything, so the granularity must be known before any write we could learn from. That leaves a probe file written into the destination each run (needing a writable dir and a network round trip on exactly the mounts that motivated this), or persisting a measured value (new state, new invalidation problem). Learning from our own os.utime() is free but only happens on runs that download something — not the pure "nothing changed" run where the tolerance is the whole ballgame.
2. It measures the wrong quantity. Granularity is one term in the round-trip error, not the error. FAT stores mtimes in local time, so a DST transition shifts every mtime by 3600 s and a probe run afterwards measures 2 s and learns nothing. Same for SMB/NFS attribute caching and clock skew. The probe also measures the directory it writes into, while the comparison uses op.realpath(path), which a symlink can put on another filesystem.
3. The error directions are asymmetric. Too small is download -e refresh never skips anything on filesystems without sub-second mtimes #1907: every asset re-transferred, every run, forever. Too large needs an asset that changed at byte-identical size with an mtime moving under 2 s. Precision optimizes the direction that costs nothing.

Verdicts: fixed window vs. this

scenario	want	flat 2 s	this
UNCHANGED ext4 / 1 s fs / 2 s fs / exFAT	skip	✅ ×4	✅ ×4
CHANGED ext4, +0.4 s	redownload	❌	✅
CHANGED ext4, +1.9 s	redownload	❌	✅
CHANGED exFAT, +0.4 s	redownload	❌	✅
CHANGED 1 s fs (odd sec), +0.9 s	redownload	❌	✅
CHANGED whole-second record, +1.4 s	redownload	❌	❌
stale 1 hour	redownload	✅	✅
wrong verdicts		5	1

Never worse; its one miss is a strict subset of the fixed window's.

Residual limitation: when the stored value lands on an exact even whole second it is indistinguishable from a FAT value, and behaves like the fixed window. Roughly 1-in-10⁶ for microsecond-resolution blobDateModified, and it still requires an identical size.

Why nanoseconds, and the 1 µs floor

os.utime() takes the time as seconds in a C double, so even an exact filesystem round-trips it slightly off — 22 ns measured on tmpfs. That, rather than any filesystem property, is what is_same_time()'s 1 µs default was really absorbing; it is now named MTIME_ROUNDTRIP_SLACK_NS. Comparison is in integer nanoseconds via st_mtime_ns so the multiple-of test is exact.

Testing

• test_is_same_mtime — 9-case unit matrix, beside the is_same_time() tests.
• test_download_file_refresh_detects_subsecond_change — end-to-end; fails against the fixed window.
• test_is_local_file_current_coarse_mtime_fs — pins all three branches; fails against the bare >= on every coarse granularity, and against an is_same_mtime()-only check on all four, including the exact-filesystem case.
• coarse_mtime_fs gains exFAT's 10 ms, and quantizes in integer nanoseconds so that granularity truncates cleanly.

flake8, isort, mypy and black clean on the touched files. test_utils.py: 89 passed vs. 80 on the base, with an identical set of pre-existing network failures. test_download.py: only the pre-existing test_download_000027* families fail (versioneer rejects 0+untagged…dirty in a tagless clone).

[codecov]

Codecov Report

❌ Patch coverage is 90.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.32%. Comparing base (32b7bab) to head (1a29756).

Files with missing lines	Patch %	Lines
dandi/utils.py	60.00%	4 Missing ⚠️
dandi/consts.py	0.00%	2 Missing ⚠️

Additional details and impacted files

@@                               Coverage Diff                               @@
##           claude/filesystem-estimate-precision-ys7fuz    #1912      +/-   ##
===============================================================================
+ Coverage                                        77.26%   77.32%   +0.06%     
===============================================================================
  Files                                               89       89              
  Lines                                            13264    13313      +49     
===============================================================================
+ Hits                                             10248    10294      +46     
- Misses                                            3016     3019       +3     

Flag	Coverage Δ
unittests	77.32% <90.00%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[CodyCBakerPhD]

Merge if you consider it as a requirement to get the main (important) fix in

There, an asset genuinely replaced under two seconds later at an identical size compares "same" and is silently not refreshed. Unlike #1907, nobody would ever notice: the symptom is a stale file, not a slow download.

Can you describe a real user interaction event that would cause this issue?

Should I even mention the fact that people are told not to locally edit the dandiset.yaml anyway and such edits will be ignored (AFAIK) on upload regardless?

[yarikoptic]

First, to unblock the merge question: this PR is not a requirement for #1910. #1910 is standalone and fixes the reported bug; this is a refinement stacked on its branch. Merge #1910 whenever it is ready — if it lands first I will retarget this to master and re-verify.

A real user event for the asset case

Straight answer: I cannot describe a likely human-driven one, and I would rather say that than manufacture one. Reaching it needs two blob versions at the same path with byte-identical size and blobDateModified under 2 s apart. A person working interactively does not produce that.

The reachable instances are machine-driven:

• An automated pipeline that re-uploads after a retry, or two dandi upload processes racing on the same path.
• Zarr is the one place the two conditions co-occur naturally rather than coincidentally. _download_zarr() routes every entry through the same _download_file() with mtime=entry.modified, and uncompressed chunks of a fixed shape and dtype are exactly equal in size (compressed ones usually are not). Chunk rewrites within one upload land seconds apart. So a zarr being rewritten while someone refreshes it is where "same size, mtime moved a little" stops being a coincidence — thousands of same-size entries, sub-second write spacing. Still a race, and I have not reproduced it against a live archive.

So the honest framing is not "users hit this". It is that the fix costs nothing to have: no probe, no cached state, no lock, comparable line count, and it is never worse than the constant (its one wrong verdict is a strict subset of the constant's). There is no tradeoff being purchased. If it did cost real complexity, your point would carry and the constant would be the right call.

dandiset.yaml — you are right, and I will say so plainly

I checked rather than assume, and the justification I gave in the PR body is weaker than I wrote it:

• dandi upload ignores local dandiset.yaml edits by default — upload_dandiset_metadata: bool = False in dandi/upload.py.
• The code says why, at the DandisetMetadataFile branch: "This is a temporary measure to avoid breaking web UI dandiset metadata schema assumptions. All edits should happen online."

There is an opt-in escape hatch, --upload-dandiset-metadata (documented in docs/source/cmdline/upload.rst), so edits are not unconditionally ignored. But that is an explicitly temporary, discouraged path — so "protects a locally edited dandiset.yaml" is a thin justification, and yes, worth mentioning that people are told not to edit it.

What survives for that hunk is consistency, not user impact: _populate_dandiset_yaml() held the same "our own os.utime() value round-trips exactly" premise this PR removes from _download_file(), and leaving one of the two behind is how they drift. That is a much smaller claim than the one in the PR body, and if you would rather that hunk be dropped to keep this to a single idea, that is a reasonable call — it is one commit (696987a) plus its test and reverts cleanly.

The merge decision here is @yarikoptic's, not mine.

---

Generated by Claude Code

[yarikoptic]

There, an asset genuinely replaced under two seconds later at an identical size compares "same" and is silently not refreshed. Unlike #1907, nobody would ever notice: the symptom is a stale file, not a slow download.

Can you describe a real user interaction event that would cause this issue?

Thinking about it myself (in addition to claude code above), indeed tricky/unlikely to happen in real world, e.g. two parallel processes downloading from the same dandiset while it being modified "upstream" and coinciding to the same moment of time for download of that file.

For me it was more of an overall "design" shortcoming for such quantification of mtimes. But indeed, seems pragmatically there should be no such cases. For the sake of simplicity, let's then postpone this