Repository navigation
Fix session-long memory/perf growth: Coach transcript cap + off-hot-path log mirror - #741
Merged
Merged
Conversation
The AI Coach kept its whole chat history in one app-lifetime object and appended forever with no cap — Swift `AICoachEngine.messages` (a `let` on AppModel) and Kotlin `CoachViewModel._messages` (a ViewModel that survives tab-switching). Only a process kill cleared it, so a long chat session grew memory and the (non-lazy) transcript render cost until the app got laggy and had to be reopened — the reported 'feels like a RAM problem, reopening fixes it'. The network payload was already windowed (maxHistoryMessages / AiCoach), but that never trimmed the STORED/rendered array. Cap the stored array to the newest 40 messages on both engines (cap >> the wire window, so what's sent is unchanged), and render the iOS transcript in a LazyVStack. Swift and Kotlin kept in parity.
Every diagnostic line ran LiveState.append(log:), which did an O(n) Array.removeFirst once the 5k ring was full AND synchronously mirrored up to 2000 lines to UserDefaults — on EVERY line. That per-line cost has grown release over release as more diagnostics (offload/backfill, #700, #714, #720) funnel through this one sink, so a busy strap session gets laggier the longer it stays connected. Batch it: amortize the trim (let the ring overrun by a 256-line slack, then trim in one shot) and mirror the durable tail every 32 lines instead of every line, with an explicit flush on disconnect (clearBiometrics) so a finished session's tail is always persisted for the scheduled export. What gets stored/persisted is unchanged; only how often. Buffer stays hard- bounded (<= maxLogLines + slack). iOS-only: Android's log() already uses an O(1) ArrayDeque with no per-line disk write, so no Kotlin twin is needed.
DX23876
added a commit
to DX23876/noop
that referenced
this pull request
Jul 24, 2026
…first Pulls the newest upstream commits (up to 7ae27b1) into the diverged fork. Conflicts resolved keeping the fork's design/behaviour and adopting upstream's improvements where they don't fight it: - StressView / MetricExplorerView: adopt upstream's adaptive-width range selector on its own row (ryanbr#743/ryanbr#748) but keep the fork's curated ExploreRange.displayCases (not upstream's allCases) and the !categoryEchoesTitle overline suppression. - SleepView: keep the "Last night's deficit" tile on all platforms; upstream's ryanbr#743 hid it on iOS for its own compact layout, which this fork doesn't adopt. - CoachView: keep the fork's richer chatBody (timestamps, grouping, arrival animation); upstream's simpler transcript rewrite is dropped. - AICoach: keep the fork's richer coach (reply.text/toolsUsed, flushPendingCharts, brief origin + day-stamp) but route all message appends through upstream's appendMessage() so the fork gains the maxStoredMessages RAM cap (ryanbr#741). - docs/OURA_PROTOCOL.md: take upstream's more detailed Ring-4-APK protocol facts and the observed-undecoded-tags table (ryanbr#747). - Both Localizable.xcstrings: semantic key/lang-level merge — fork values win per language, upstream fills only missing languages. Fork keeps all its keys (superset: 3992 vs upstream 3242); EN/DE/FR/ES coverage unchanged (i18n audit: 89 gaps each, same as before). pt-PT (upstream ryanbr#769) absorbed as gap-fills only. Android + Watch + tooling changes auto-merged (the fork keeps the Android tree as non-parity dead code). Verified: NOOPiOS + Strand (macOS) build clean; StrandTests 67/67; i18n audit shows no EN/DE/FR/ES regression. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
simoncad7
pushed a commit
to simoncad7/noop
that referenced
this pull request
Aug 17, 2026
Fix session-long memory/perf growth: Coach transcript cap + off-hot-path log mirror
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.
Why
A user reported (no logs, just the symptom): "performance dropped version by version… it got really laggy and I have to reopen the app to fix it. It seems like a RAM problem." That signature — progressive slowdown over a session that a restart clears — points at in-memory growth. An investigation across both platforms surfaced two independent causes; this PR fixes both, as two revert-safe commits.
1. Coach: cap the stored chat transcript (both platforms)
The AI Coach held its entire chat history in a single app-lifetime object and appended forever with no cap:
AICoachEngine.messages(@Published), andcoachis aletonAppModel, so it lives for the whole process; the transcript was rendered in a plainVStack(every bubble resident).CoachViewModel._messages; the ViewModel survives tab-switching, so it also only cleared on process death.The network payload was already windowed (
windowedMessages()/maxHistoryMessages), which is easy to misread as "history is bounded" — but that only trims what's sent, never the stored/rendered array. So an active chat grew RAM (and layout cost) until you reopened the app — matching the report exactly.Fix: cap the stored array to the newest 40 messages on both engines (cap ≫ the 10-message wire window, so what's sent is unchanged), and render the iOS transcript in a
LazyVStack. Android keeps itsColumn— a 40-item cap makes a lazy list unnecessary, and nesting aLazyColumnin the existing scroll column risks Compose's same-direction-scroll crash. Swift/Kotlin kept in parity.2. LiveState: take the durable log mirror off the per-line hot path (iOS only)
LiveState.append(log:)ran on every diagnostic line and did two things per line: an O(n)Array.removeFirstonce the 5k ring was full, and a synchronous mirror of up to 2000 lines toUserDefaults. That per-line cost has grown release over release as more diagnostics (offload/backfill, #700, #714, #720) funnel through this one sink — so a busy strap session gets laggier the longer it stays connected (a good fit for "dropped version by version").Fix: amortize the trim (let the ring overrun by a 256-line slack, then trim in one shot) and mirror the durable tail every 32 lines instead of every line, with an explicit flush on disconnect (
clearBiometrics) so a finished session's tail is always persisted for the scheduled export. What gets stored/persisted is unchanged — only how often. Buffer stays hard-bounded (≤maxLogLines + slack).iOS-only by design: Android's
log()already uses an O(1)ArrayDequewith no per-line disk write (WhoopBleClient.kt), so it needs no twin — the same kind of documented asymmetry as the #700 clock-retry.Verification — please read
aapt2/d8as x86-64 only). So this has had a careful manual review but no compile. Per the app-target CI gap in CLAUDE.md, someone should build both before merge:xcodebuild … -scheme NOOPiOS build(andStrandmacOS, sinceAICoach/LiveStateare shared)../gradlew compileFullDebugKotlin.Changes are comment + small-diff only, no new user-facing strings (i18n gate unaffected), and each commit is independent so either can be dropped.
Confidence on the diagnosis
The Coach cap (#1) is a confirmed unbounded leak regardless of the reporter — worth landing on its own merits. Whether it's the cause for this specific user depends on whether they use the Coach; the diag-sink cost (#2) affects every strap user and better matches "version by version." I've asked the reporter for platform + version + whether they use the Coach to confirm which dominates.