From b541cc231442cdb4eea38f16f675b034763125f7 Mon Sep 17 00:00:00 2001 From: avi Date: Fri, 18 Sep 2026 22:05:04 -0500 Subject: [PATCH] Never swallow detail-screen updates: revision field on DetailUi MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit THE root cause behind "summary changed but the open screen kept the old voice until I left to the dashboard and back": the app mutates ONE DetailUi instance in place and republishes it with copy(). StateFlow drops any assignment equal() to its current value — and a copy of the very object it already holds is ALWAYS equal (same mutated fields). So every publish after the first was silently dropped. The screen only ever refreshed by accident, when some other collected flow (models, playback, library rows) happened to trigger a recomposition that re-read the mutated fields mid-run; the final summary publish, with nothing else changing at that moment, just vanished. Engine logs proved the app fetched the fresh sarcastic summary — the UI received it and threw it away. DetailUi gains a rev discriminator bumped on every publish (23 sites), so no update can ever conflate away. Two regression tests pin the conflation semantics. --- .../kotlin/com/shonar/desktop/DesktopState.kt | 68 +++++++++++-------- .../com/shonar/desktop/JobProgressTest.kt | 20 ++++++ 2 files changed, 59 insertions(+), 29 deletions(-) diff --git a/app/src/main/kotlin/com/shonar/desktop/DesktopState.kt b/app/src/main/kotlin/com/shonar/desktop/DesktopState.kt index 5fc14a6..e0aa473 100644 --- a/app/src/main/kotlin/com/shonar/desktop/DesktopState.kt +++ b/app/src/main/kotlin/com/shonar/desktop/DesktopState.kt @@ -66,6 +66,16 @@ class DesktopState(private val appDir: File = defaultAppDir()) { * an already-transcribed file (no live transcript in memory). */ var reportText: String? = null, var error: String? = null, + /** Emission discriminator. The whole class mutates ONE DetailUi + * instance in place and republishes with copy(); StateFlow drops + * any assignment that `equals()` the current value — and a copy + * of the very object the flow already holds is ALWAYS equal + * (same mutated fields). That silently swallowed updates, the + * most visible one being the finished summary: the job landed, + * the report on disk rewrote, but the open screen stayed on the + * old voice until the user left to the library and back. Every + * publish bumps this so no update can ever conflate away. */ + val rev: Long = 0L, ) private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) @@ -912,7 +922,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { _liveProgress.value = _liveProgress.value.toMutableMap().apply { remove(old.name) } - if (_detail.value?.file == old) _detail.value = _detail.value?.copy(file = target) + if (_detail.value?.file == old) _detail.value = _detail.value?.copy(file = target, rev = System.nanoTime()) rescan() return null } @@ -1351,17 +1361,17 @@ class DesktopState(private val appDir: File = defaultAppDir()) { // idle "Transcribe" button for a file already working. if (d.remoteId == null && file.name in inFlight) { d.busy = _liveProgress.value[file.name]?.label ?: "queued for transcription…" - if (_detail.value?.file == file) _detail.value = d.copy() + if (_detail.value?.file == file) _detail.value = d.copy(rev = System.nanoTime()) while (file.name in inFlight && _detail.value?.file == file) { delay(500) d.busy = _liveProgress.value[file.name]?.label ?: "uploading…" - if (_detail.value?.file == file) _detail.value = d.copy() + if (_detail.value?.file == file) _detail.value = d.copy(rev = System.nanoTime()) } if (_detail.value?.file != file) return@launch d.busy = null d.remoteId = loadMapping(file)?.recordingId d.reportText = runCatching { reportFile(file).readText() }.getOrNull() - if (_detail.value?.file == file) _detail.value = d.copy() + if (_detail.value?.file == file) _detail.value = d.copy(rev = System.nanoTime()) } // Files that predate remote-id mapping have no sidecar: adopt // the server recording by exact title match so Re-summarize @@ -1386,7 +1396,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { d.transcript = t ?: d.transcript d.summary = s saveReport(d) - if (_detail.value?.file == file) _detail.value = d.copy() + if (_detail.value?.file == file) _detail.value = d.copy(rev = System.nanoTime()) } } // If the file is mid-pipeline (pump running, or reprocess started @@ -1412,7 +1422,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { } d.jobs = jobs jobLabel(jobs)?.let { d.busy = it } ?: run { d.busy = null } - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) val active = jobs.any { it.status == "running" || it.status == "queued" } sawActive = sawActive || active if (!active) break @@ -1436,7 +1446,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { d.busy = null d.reportText = runCatching { reportFile(file).readText() }.getOrNull() ?: d.reportText - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) rescanStatuses() } } @@ -1469,7 +1479,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { } fun setOverride(model: String?) { - _detail.value?.let { it.overrideModel = model; _detail.value = it.copy() } + _detail.value?.let { it.overrideModel = model; _detail.value = it.copy(rev = System.nanoTime()) } } fun transcribe() { @@ -1483,7 +1493,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { if (d.file.name in inFlight || isRunning(d.file.name)) { d.busy = _liveProgress.value[d.file.name]?.label ?: "Already queued…" d.error = null - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) return } // A file with a known server recording re-runs the pipeline IN @@ -1509,7 +1519,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { d.busy = "Uploading…" d.uploadProgress = 0f d.error = null - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) val draft = RecordingDraft( id = UUID.randomUUID().toString(), title = d.file.nameWithoutExtension, @@ -1528,12 +1538,12 @@ class DesktopState(private val appDir: File = defaultAppDir()) { // always change something on screen, immediately. setLive(d.file.name, LiveProgress("uploading…", p)) rescanStatuses() - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) } }.getOrElse { d.busy = null d.error = it.message ?: "Upload failed." - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) return@run } d.remoteId = ref.key @@ -1541,7 +1551,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { title = d.file.nameWithoutExtension)) d.busy = "Transcribing…" d.uploadProgress = null - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) // Poll jobs until the WHOLE pipeline is terminal. Breaking on // transcribe alone froze the screen: the last d.jobs snapshot // still showed summarize running at 0%, and nothing refreshed @@ -1557,7 +1567,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { // doesn't make the job look dead. setLive(d.file.name, jobProgress(jobs)) rescanStatuses() - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) val t = jobs.firstOrNull { it.jobType == "transcribe" }?.status val s = jobs.firstOrNull { it.jobType == "summarize" }?.status if ((t == null || t in TERMINAL) && (s == null || s in TERMINAL)) break @@ -1568,20 +1578,20 @@ class DesktopState(private val appDir: File = defaultAppDir()) { if (failed != null) { d.busy = null d.error = failed.error ?: "Transcription failed." - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) return@run } d.transcript = runCatching { parseTranscript(provider.fetchTranscript(ref.key)) } .getOrNull() d.summary = runCatching { parseSummary(provider.fetchSummary(ref.key)) }.getOrNull() d.busy = null - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) if (d.transcript != null) { saveReport(d) rescan() } else { d.error = "Transcription finished but no transcript was returned." - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) } } } @@ -1605,7 +1615,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { else "Summarizing… 0%" d0.uploadProgress = null d0.error = null - _detail.value = d0.copy() + _detail.value = d0.copy(rev = System.nanoTime()) runCatching { provider.reprocess(remoteId, job, model, tone) }.onFailure { if (job == "transcribe" && it is ProviderError.NotFound) { // Mapping points at a deleted recording: drop it so the @@ -1614,7 +1624,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { d0.remoteId = null d0.busy = null d0.error = "That server recording no longer exists — press Transcribe to upload again." - _detail.value = d0.copy() + _detail.value = d0.copy(rev = System.nanoTime()) return@launch } else if (it is ProviderError.Transient && it.message?.contains("already running") == true) { @@ -1628,7 +1638,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { } else { d0.busy = null d0.error = it.message ?: "Reprocess failed." - _detail.value = d0.copy() + _detail.value = d0.copy(rev = System.nanoTime()) return@launch } } @@ -1649,7 +1659,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { d0.error = "Lost contact with the engine while " + "processing — reopen this recording to check." d0.busy = null - _detail.value = d0.copy() + _detail.value = d0.copy(rev = System.nanoTime()) return@launch } continue @@ -1663,7 +1673,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { // detail screen (the pump path already does this via setLive). setLive(d0.file.name, jobProgress(jobs)) rescanStatuses() - _detail.value = d0.copy() + _detail.value = d0.copy(rev = System.nanoTime()) val st = jobs.firstOrNull { it.jobType == job }?.status if (st in TERMINAL) break // Job row missing from a healthy fetch: give it a few @@ -1676,7 +1686,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { if (failed != null) { d0.busy = null d0.error = failed.error ?: "That stage failed." - _detail.value = d0.copy() + _detail.value = d0.copy(rev = System.nanoTime()) return@launch } d0.transcript = runCatching { parseTranscript(provider.fetchTranscript(remoteId)) } @@ -1684,7 +1694,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { d0.summary = runCatching { parseSummary(provider.fetchSummary(remoteId)) } .getOrNull() d0.busy = null - _detail.value = d0.copy() + _detail.value = d0.copy(rev = System.nanoTime()) d0.summary?.let { announceSummaryReady(d0.file, it.version) } if (d0.transcript != null || d0.summary != null) { saveReport(d0) @@ -1710,7 +1720,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { // opened for a since-renamed/deleted file). Say what's wrong. d.error = "This file has no server recording to summarize — " + "press Transcribe to upload it first." - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) return } d.remoteId = remoteId @@ -1730,7 +1740,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { if (remoteId == null) { d.error = "This file has no server recording to save against — " + "press Transcribe to upload it first." - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) return } d.remoteId = remoteId @@ -1738,7 +1748,7 @@ class DesktopState(private val appDir: File = defaultAppDir()) { scope.launch { d.busy = "Saving transcript…" d.error = null - _detail.value = d.copy() + _detail.value = d.copy(rev = System.nanoTime()) runCatching { parseTranscript(provider.updateTranscript(remoteId, org.json.JSONObject().put("text", text).toString())) } .onSuccess { saved -> @@ -1747,13 +1757,13 @@ class DesktopState(private val appDir: File = defaultAppDir()) { saveReport(d) } d.busy = null - if (_detail.value?.file == d.file) _detail.value = d.copy() + if (_detail.value?.file == d.file) _detail.value = d.copy(rev = System.nanoTime()) rescanStatuses() } .onFailure { d.busy = null d.error = it.message ?: "Transcript save failed." - if (_detail.value?.file == d.file) _detail.value = d.copy() + if (_detail.value?.file == d.file) _detail.value = d.copy(rev = System.nanoTime()) } } } diff --git a/app/src/test/kotlin/com/shonar/desktop/JobProgressTest.kt b/app/src/test/kotlin/com/shonar/desktop/JobProgressTest.kt index a8f6bf4..2aa95fe 100644 --- a/app/src/test/kotlin/com/shonar/desktop/JobProgressTest.kt +++ b/app/src/test/kotlin/com/shonar/desktop/JobProgressTest.kt @@ -207,4 +207,24 @@ class JobProgressTest { assertEquals("dry, witty", DesktopState.reportVoice("*Voice: dry, witty*")) } + + // ---- DetailUi revision: mutated-in-place publishes must not conflate ---- + + @Test fun `mutated copy without rev equals stale value (the old bug)`() { + // Documents WHY DetailUi.rev exists: the app mutates one instance + // and republishes copy(); StateFlow would drop that assignment as + // "no change" — the finished summary never reached the open screen. + val d = DesktopState.DetailUi(file = java.io.File("/tmp/x.m4a")) + val published = d.copy() + d.summary = null // mutate the SAME instance + assertEquals(published, d.copy()) + } + + @Test fun `rev bump makes every republish distinct`() { + val d = DesktopState.DetailUi(file = java.io.File("/tmp/x.m4a")) + val before = d.copy(rev = 1L) + d.summary = null + val after = d.copy(rev = 2L) + org.junit.Assert.assertNotEquals(before, after) + } }