Track a queued save with a flag, not by comparing timestamps - #328
Merged
Merged
Conversation
IsSaveQueued answered SaveQueuedTime > LastSaveTime. DateTime.UtcNow advances in whole microseconds on macOS, so a QueueSave issued straight after a Save can read the same instant as the save did. The comparison then says nothing is queued, and the change is never written: not by SaveIfRequired after the debounce, and not by Dispose either. That is what failed TestMultipleSavesOnlyWriteOnceWithinDebouncePeriod on the macOS runner only: the debounce had elapsed (the test waited 3.1 s against a 3 s debounce) but the queued "Data2" was never saved. Linux and Windows read the clock finely enough that the two calls never tied. A save is outstanding because QueueSave ran after the last Save, which is an ordering fact, so it is now a flag set by QueueSave and cleared by a successful Save. SaveQueuedTime still drives the debounce. A regression test pins the tied-clock case deterministically. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RTMFFSY8FY5CpAhf6ikNH1
|
This was referenced Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before:
IsSaveQueued()returnedSaveQueuedTime > LastSaveTime. WhenQueueSave()ran right afterSave(), the twoDateTime.UtcNowreads could return the same value. The comparison then reported nothing queued, so the change was never written:SaveIfRequired()skipped it after the debounce, andDispose()did not flush it either. On main's CI run 36567126443 this failedTestMultipleSavesOnlyWriteOnceWithinDebouncePeriodon macOS only. The test waited 3.1 s against a 3 s debounce, so the debounce had elapsed, but"Data2"was never saved.After:
QueueSave()sets aHasQueuedSaveflag and a successfulSave()clears it.IsSaveQueued()reads the flag.SaveQueuedTimestill drives the debounce.Why macOS only: .NET's
DateTime.UtcNowon macOS comes fromclock_gettime(CLOCK_REALTIME), which advances in whole microseconds. ASavefollowed immediately by aQueueSavefits inside one microsecond. Linux reads the clock in nanoseconds and Windows uses the precise system time, so on those runners the two reads never tie. A wall clock set back would drop a queued save the same way. Whether a save is outstanding depends on the order of the two calls, not on when each one happened, so a flag is the correct model. This is a bug in the library, not in the test: an application callingSave()and thenQueueSave()could silently lose data.How:
AppData.cs: adds theHasQueuedSaveflag. The flag is cleared only after the write succeeds, so a failed write leaves the save queued.TestQueueSaveIsNotLostWhenTheClockHasNotAdvancedSinceTheLastSavereproduces the tied or backwards clock on every platform by settingLastSaveTimetoDateTime.MaxValuebeforeQueueSave(). It fails deterministically against the old comparison.TestSaveClearsQueuedSavepins the other half: a save clears the flag.CLAUDE.mdrecords why the flag replaced the timestamp comparison.I couldn't build or run the tests locally because there is no .NET SDK in this environment. CI will be the first to compile and run this change.
🤖 Generated with Claude Code
https://claude.ai/code/session_01RTMFFSY8FY5CpAhf6ikNH1
Generated by Claude Code