Repository navigation
Coach: retire the transcript on a new local day - #1542
Merged
ryanbr merged 1 commit intoAug 31, 2026
Merged
Conversation
The CoachViewModel outlives a night - Android keeps the process alive for days - and nothing ever reset _messages except clearing the API key or switching provider. So a question asked this morning was answered inside yesterday's conversation. The data was never stale: buildContext() re-reads the store on every send, and injectContext() prepends it to the FIRST user turn. But the assistant's own earlier turns below it state yesterday's figures, and the model stays consistent with them - which reads as "the coach only talks about my imported data" after a night of fresh strap data. Force-quitting the app (destroying the ViewModel) was the only cure. MAX_STORED_MESSAGES already bounds the transcript's SIZE for the same long-lived-session reason; this bounds its AGE. isStaleConversation() is strictly forward (>) rather than !=, so a clock moving backwards - flying west, a timezone change, an NTP correction - cannot wipe a conversation the user is in the middle of. Pure companion, pinned by CoachConversationDayTest (6 cases). Verified by mutation: relaxing > to != fails clockMovingBackwardsKeepsTheConversation and yearBoundaryIsJustAnotherDay. Verification: ./gradlew testFullDebugUnitTest - 4195 tests, 0 failures. Swift twin NOT included: CoachView/AICoach are app-target Swift, which no default CI covers and which needs Xcode on macOS to compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ryanbr
added a commit
that referenced
this pull request
Aug 31, 2026
The Apple half of #1542, which @dwehrmann deliberately left out rather than ship Swift they could not compile. That was the right call, and this is the evidence: it did NOT compile on the first attempt, and nothing in default CI would have said so. error: call to main actor-isolated static method 'isStaleConversation(lastEpochDay:todayEpochDay:)' in a synchronous nonisolated context AICoachEngine is @mainactor so its @published mutations stay on the main thread, and the statics inherited that isolation. Marked nonisolated rather than pushing @mainactor onto the tests: they are pure functions of their arguments, touch no published state, and the Kotlin twin is a plain companion object for the same reason. The iOS leg had passed on the broken commit - only the macOS leg compiles StrandTests - so a compile-only check would have called it green. Same bug, same rule. messages is @published and held for the app's lifetime, so the coach answered today's question inside yesterday's conversation; the data was never stale, it is the assistant's own earlier turns stating yesterday's figures. isStaleConversation is strictly forward (`>`, never `!=`) so a clock moving BACKWARDS cannot wipe a conversation mid-use, and the check sits after the guards so a send that never happens leaves the transcript alone. One thing is Swift-only because Swift-only needs it. Kotlin gets its epoch day from LocalDate.toEpochDay(); this side computes one, and the obvious way - dividing a startOfDay interval by 86,400 - is wrong, because a day is not always 86,400 seconds. localEpochDay uses calendar day arithmetic. The re-review then caught that the DST case written first was spring-forward, which is the obvious one and not the dangerous one: a 25-hour autumn day is where truncating whole days from a non-midnight origin could put two local midnights in one bucket, and a transcript would then survive a night once a year. Both directions pinned now, plus a zone west of UTC where the epoch instant falls on the previous local day. Also folded in, on request, and checking it corrected the claim that prompted it. I had said Android wipes the transcript on a PROVIDER SWITCH and Apple does not. Wrong - neither does, so that is shared behaviour rather than a divergence, and whether provider A's history should reach provider B is a real question but the same one on both platforms. The actual divergence was narrower: Kotlin's clearKey and disconnect have always emptied the transcript and the Swift twins never did, so on Apple "clear my key" removed the credential and kept the conversation, and disconnecting returned the user to the setup screen with the whole conversation behind it. Both retire it now, and conversationDay goes with the transcript on BOTH sides - Kotlin was not resetting it, which is harmless but is a field claiming something untrue. Nine Swift cases: six mirroring the Kotlin suite one-to-one, three Swift-only. app-build green on both legs and its Strand leg RUNS StrandTests, so they execute rather than merely compile. Android 4,947 tests, zero failures. Doc lint clean. Staging 397 / iOS 278.
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.
What this PR does
The Coach kept answering today's question inside yesterday's conversation.
CoachViewModel._messagesis in-memory and never reset except by clearing the API key or switching provider. The ViewModel outlives a night — Android keeps the process alive for days — so the transcript carries over.The data was never stale:
buildContext()re-reads the store on every send, andinjectContext()prepends it to the first user turn. But the assistant's own earlier turns below it state yesterday's figures, and the model stays consistent with them. It reads as "the coach only talks about my imported data" even after a night of fresh strap data. Force-quitting the app was the only cure.MAX_STORED_MESSAGESalready bounds the transcript's size for the same long-lived-session reason (the "gets laggy the longer the app runs" report). This bounds its age.The one design decision worth reviewing
isStaleConversation()is strictly forward (>), not!=, so a clock moving backwards — flying west, a timezone change, an NTP correction — cannot wipe a conversation the user is mid-way through. Only real elapsed days retire a transcript.Type of change
How it was tested
Not a BLE change; no hardware involved.
CoachConversationDayTest(6 cases) pins the rule as a pure companion — no ViewModel, no framework.>to!=failsclockMovingBackwardsKeepsTheConversationandyearBoundaryIsJustAnotherDay, so the test has teeth on the subtle part rather than pinning a tautology../gradlew testFullDebugUnitTest— 4195 tests, 0 failures.Reproduction, if you want to confirm the behaviour: ask the Coach something, leave the app backgrounded overnight (do not force-quit), then ask again the next day. It answers from the previous day's numbers.
Checklist
./gradlew testFullDebugUnitTest)docs/CONTRIBUTING.mdCross-platform parity — not done, and why
The Swift twin has the same shape (
AICoach/CoachViewholdmessageswith the same lifetime), so an iOS/macOS user has this bug unchanged.I did not include it:
CoachView/AICoachare app-target Swift, whichapp-build.ymlcovers only on demand and which needs Xcode on macOS to compile. Per CLAUDE.md — "a compile error there will pass every green check and still be broken" — I would rather flag the gap than ship Swift I could not build. Happy for a maintainer to twin it, or I can follow up if someone can compile it.Related issues
None open that I could find for this specific behaviour.