fix(watch): shadow stale-read drops external sync changes; no new-day journal#107
Merged
Conversation
… journal Two bugs in file watching and journal lifecycle: 1. FileRegistry.detectChanges was calling fileSystem.readFile after a mtime change without first invalidating the shadow cache. On Android, readFile hits the shadow first — stale content matched the stored hash, so the change was silently dropped as an own-write. Fix: call fileSystem.invalidateShadow before readFile in the modTime > lastKnown branch. No-op on JVM; zero cost on desktop. 2. ensureTodayJournal was only called once at startup (onPhase1Complete). When the app was open across midnight, the new day's journal was never created. Fix: launch a midnight-boundary watcher coroutine on the graph scope that computes delay to next local midnight, sleeps, then calls ensureTodayJournal. Stores lastJournalDate so redundant calls are skipped (also guards against clock-precision double-fires). Logs seconds until next check so the timing is observable. Also injects Clock into JournalService for testability, and adds: - FileRegistryTest: stale-shadow regression test - GraphLoaderProgressiveTest: midnight-boundary + guard tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes two related bugs around journal updates: (1) on Android, externally-synced changes to plain .md files were silently dropped because FileRegistry.detectChanges read content through the stale SAF shadow cache; (2) on all platforms, today's journal was only created at startup, so an app open across midnight saw no new daily journal until the user navigated to the journals tab.
Changes:
FileRegistry.detectChangesnow callsfileSystem.invalidateShadow(filePath)beforereadFilein themodTime > lastKnownbranch (no-op on JVM).StelekitViewModeladds astartMidnightBoundaryWatcher()started fromonPhase1Complete, computing the delay to the next local midnight viakotlinx-datetime, guarding onlastJournalDate, and re-launching cleanly on graph reloads.JournalServicetakes an injectedClock(defaulting toClock.System) for testability.- New regression tests for stale-shadow detection, own-write suppression, and the midnight boundary watcher.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/FileRegistry.kt | Invalidate shadow before re-reading changed files so Android picks up external writes. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/JournalService.kt | Inject Clock via constructor; replace Clock.System.now() with clock.now(). |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt | Add midnight boundary watcher and lastJournalDate tracking around ensureTodayJournal. |
| kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/FileRegistryTest.kt | Add FakeFsWithShadow and stale-shadow + own-write regression tests. |
| kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderProgressiveTest.kt | Add tests for next-midnight delay computation and watcher behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import dev.stapler.stelekit.platform.FileSystem | ||
| import dev.stapler.stelekit.repository.InMemoryBlockRepository | ||
| import dev.stapler.stelekit.repository.InMemoryPageRepository | ||
| import dev.stapler.stelekit.testing.FakeClock |
Owner
Author
There was a problem hiding this comment.
This is stale — FakeClock was added in commit 96d79fc at kmp/src/jvmTest/kotlin/dev/stapler/stelekit/testing/FakeClock.kt. CI is fully green.
Comment on lines
+706
to
+734
| fun `midnight watcher calls ensureTodayJournal after simulated day crossing`() = runTest { | ||
| val callCount = AtomicInteger(0) | ||
| val tz = TimeZone.currentSystemDefault() | ||
| val startInstant = LocalDate(2026, 5, 28).atStartOfDayIn(tz) + 23.hours + 58.minutes | ||
| val fakeClock = FakeClock(startInstant) | ||
|
|
||
| val watcherJob = launch { | ||
| while (isActive) { | ||
| val now = fakeClock.now() | ||
| val today = now.toLocalDateTime(tz).date | ||
| val tomorrowMidnight = today.plus(1, DateTimeUnit.DAY).atStartOfDayIn(tz) | ||
| val delayMs = (tomorrowMidnight - now).inWholeMilliseconds.coerceAtLeast(1_000L) | ||
| kotlinx.coroutines.delay(delayMs) | ||
| fakeClock.advance(2.minutes) | ||
| callCount.incrementAndGet() | ||
| } | ||
| } | ||
|
|
||
| advanceTimeBy(121_000) | ||
| assertEquals(1, callCount.get(), "ensureTodayJournal must be called once after first midnight") | ||
|
|
||
| advanceTimeBy(24 * 60 * 60 * 1000L + 1_000L) | ||
| assertEquals(2, callCount.get(), "ensureTodayJournal must be called again after second midnight") | ||
|
|
||
| advanceTimeBy(24 * 60 * 60 * 1000L + 1_000L) | ||
| assertEquals(3, callCount.get(), "ensureTodayJournal must be called a third time") | ||
|
|
||
| watcherJob.cancel() | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in commit afaf586. startMidnightBoundaryWatcher is now internal, and a new test (midnight watcher calls ensureTodayJournal via real startMidnightBoundaryWatcher) drives the production method directly: it creates a StelekitViewModel with a FakeClock-backed JournalService, calls startMidnightBoundaryWatcher(fakeClock), waits for the real 1 s MIN_MIDNIGHT_DELAY_MS to elapse on Dispatchers.Default, then asserts the journal page was created by the production ensureTodayJournal() code.
Contributor
Android Load BenchmarkInstrumented benchmark on an API 30 x86_64 emulator — 500-page synthetic graph. Comparing Graph Load
Interactive Write Latency (during Phase 3)
SAF I/O Overhead (ContentProvider vs direct File read)Measures Binder IPC cost added by ContentResolver per readFile() call.
|
…er tests Fix DurationUnit compile error in JournalServiceTest (Duration.Companion.hours does not exist; use 1.toDuration(DurationUnit.HOURS)). Add: - JournalServiceTest: clock injection test seeding fixedDate via currentSystemDefault + 1h - FileRegistryTest: shadow stale-read regression + call-order assertion - GraphLoaderProgressiveTest: millisUntilNextMidnight and midnight-watcher tests now call the actual production method on StelekitViewModel instead of reimplementing the formula inline - FakeClock utility in jvmTest testing package Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
JVM Load Benchmark (Desktop)Synthetic in-memory benchmark measuring load performance for the desktop (JVM) app.
Flamegraphs (this PR)**Allocation** — object allocation pressure (JDBC/SQLite churn)Alloc flamegraph not available CPU — method-level hotspots by on-CPU time CPU flamegraph not available Top SQL queries by total time (this PR)| table:operation | calls | p50 | p99 | max | total | |-----------------|-------|-----|-----|-----|-------| | `pages:select` | 2 | 1ms | 1ms | 1ms | 1ms |Top allocation hotspots (this PR)`70.3%` byte[]_[k] `5%` java.lang.String_[k] `2.5%` java.lang.StringBuilder_[k] `2%` java.lang.Class_[k] `1.5%` long[]_[k]Top CPU hotspots (this PR)`99.5%` /usr/lib/x86_64-linux-gnu/libc.so.6 `0%` pread `0%` SR_handler `0%` kotlin/text/StringsKt__StringsKt.indexOf_[0] `0%` AccessInternal::PostRuntimeDispatch, (AccessInternal::BarrierType)2, 544868ul>::oop_access_barrier |
…asmJS kotlin.concurrent.Volatile is unavailable in the WasmJS target. The field is only accessed within the single midnightWatcherJob coroutine so no synchronization is needed; removing the annotation fixes the Wasm/JS Compile CI check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…atcher via ViewModel Makes startMidnightBoundaryWatcher internal so tests can invoke it directly. New test wires a FakeClock into both the watcher and JournalService, uses a real Dispatchers.Default scope so the VM's internal observe-coroutines do not join the test scheduler, and verifies that ensureTodayJournal() creates the correct journal page after the midnight boundary is crossed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
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.
Summary
FileRegistry.detectChangeswas reading file content viareadFilewithout first invalidating the shadow cache. On Android,readFilehits the SAF shadow first — stale shadow content matched the stored hash, so externally-synced changes were silently dropped as own-writes.ensureTodayJournalwas only called once at startup. When the app was open across midnight, no new daily journal was created until the user navigated to the journal tab.Changes
FileRegistry.kt— single line fix:fileSystem.invalidateShadow(filePath)beforereadFilein themodTime > lastKnownbranch. No-op on JVM, forces SAF re-read on Android.StelekitViewModel.kt— addsstartMidnightBoundaryWatcher()called fromonPhase1Complete. The watcher:kotlinx-datetime(DST-safe)lastJournalDatealready matches today (guards against double-fires and redundant calls after startup already handled today)JournalService.kt— injectsclock: Clock = Clock.Systemconstructor parameter for testability (allClock.System.now()calls replaced withclock.now()).Tests —
FileRegistryTest: stale-shadow regression;GraphLoaderProgressiveTest: midnight boundary (3 crossings),lastJournalDateguard, cancellation.Test plan
FileRegistryTest— 26 tests including stale-shadow regression (external write with stale shadow detected; own write still suppressed)GraphLoaderProgressiveTest— midnight watcher fires at boundary, skips whenlastJournalDatematches, cancels cleanlyGraphLoaderWatcherTest— own-write suppression regression unchanged🤖 Generated with Claude Code