diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-10-31 07:03:03 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-10-31 07:03:03 +0000 |
| commit | cbcd04cfb3c11ae0a171f4fce1954214baeff074 (patch) | |
| tree | b8ba3d1eb823cf2d532ac07591fed918d655e8d5 /app/src | |
| parent | 43a78fba9c46704fed7db3fb5f6b138db2fb0820 (diff) | |
A break says which detector found it, and the episode screen says so
Three detectors now cut audio out of episodes and the app said nothing about
which. The listener judging a skip wants to know what judged it, and they are
not equally sure of themselves: one compares waveforms across a batch of
episodes, the other reads what was said and checks itself against the declared
length. A break both found is marked as both, because agreement by entirely
different means is the strongest thing either can offer.
"Found by the audio and the words", under the numbers and quieter than them --
it explains them rather than competing with them.
Schema 19. Existing rows default to the audio scan because that is where every
break in the database came from; the words could not produce one until this
week. "Unknown" would have been honest about the column and useless about the
rows. A server that says nothing gets the same default, for the same reason.
The coverage gate caught the new field's default never being exercised, which
was fair: every call site passes it, and the case it exists for is an episode
with nothing cut and nothing to account for.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'app/src')
9 files changed, 250 insertions, 2 deletions
diff --git a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt index 49d3406..a261149 100644 --- a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt +++ b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt @@ -856,4 +856,54 @@ class MigrationTest { assertEquals("the crown jewel must survive", 161803L, it.getLong(0)) } } + + /** + * A break gains the name of the detector that found it, and existing rows default to the + * audio scan -- which is where every break in the database came from before the words + * could produce one. + */ + @Test + fun migrate18To19NamesWhereABreakCameFrom() { + helper.createDatabase(name, 18).apply { + execSQL( + """INSERT INTO subscriptions + (id, feedUrl, kind, title, addedAtMs, skipIntroMs, skipOutroMs, dripEnabled, + dripIntervalMs, dripCount, dripOldestFirst, skipAdChapters, bedtime) + VALUES ('s1', 'https://example.com/feed.xml', 'PODCAST', 'A Show', 1000, 0, 0, + 0, 86400000, 1, 1, 1, 0)""" + ) + execSQL( + """INSERT INTO episodes + (id, subscriptionId, guid, title, mediaUrl, mediaType, discoveredAtMs, + positionMs, downloadState) + VALUES ('e1', 's1', 'g1', 'One', 'https://cdn/1.mp3', 'audio/mpeg', 1000, + 314159, 'NONE')""" + ) + execSQL( + """INSERT INTO episode_midrolls (episodeId, candidateId, startMs, endMs, kind, partners) + VALUES ('e1', 'server-0', 60000, 90000, 'ad', 3)""" + ) + close() + } + + val db = helper.runMigrationsAndValidate(name, 19, true, AppDatabase.MIGRATION_18_19) + + db.query("SELECT origin, startMs FROM episode_midrolls WHERE episodeId = 'e1'").use { + it.moveToFirst() + assertEquals("a break that predates the words came from the audio", "audio", it.getString(0)) + assertEquals(60000L, it.getLong(1)) + } + db.execSQL( + "INSERT INTO episode_midrolls (episodeId, candidateId, startMs, endMs, kind, origin) " + + "VALUES ('e1', 'server-1', 600000, 660000, 'ad', 'words')" + ) + db.query("SELECT origin FROM episode_midrolls WHERE candidateId = 'server-1'").use { + it.moveToFirst() + assertEquals("words", it.getString(0)) + } + db.query("SELECT positionMs FROM episodes WHERE id = 'e1'").use { + it.moveToFirst() + assertEquals("the crown jewel must survive", 314159L, it.getLong(0)) + } + } } diff --git a/app/src/main/java/org/terst/listen/data/AppDatabase.kt b/app/src/main/java/org/terst/listen/data/AppDatabase.kt index 5b9219d..dc9c094 100644 --- a/app/src/main/java/org/terst/listen/data/AppDatabase.kt +++ b/app/src/main/java/org/terst/listen/data/AppDatabase.kt @@ -34,7 +34,7 @@ class Converters { EpisodeTranscriptEntity::class, TranscriptCueEntity::class, ], - version = 18, + version = 19, exportSchema = true, ) @TypeConverters(Converters::class) @@ -484,6 +484,21 @@ abstract class AppDatabase : RoomDatabase() { } } + /** + * A break gains the name of the detector that found it. + * + * Existing rows default to the audio scan, because that is where every break in the + * database came from: the words could not produce one until now. Defaulting to + * "unknown" would have been honest about the column and useless about the rows. + */ + val MIGRATION_18_19 = object : Migration(18, 19) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + "ALTER TABLE `episode_midrolls` ADD COLUMN `origin` TEXT NOT NULL DEFAULT 'audio'" + ) + } + } + fun get(context: Context): AppDatabase = instance ?: synchronized(this) { instance ?: build(context.applicationContext).also { instance = it } } @@ -497,7 +512,7 @@ abstract class AppDatabase : RoomDatabase() { MIGRATION_6_7, MIGRATION_7_8, MIGRATION_8_9, MIGRATION_9_10, MIGRATION_10_11, MIGRATION_11_12, MIGRATION_12_13, MIGRATION_13_14, MIGRATION_14_15, MIGRATION_15_16, MIGRATION_16_17, - MIGRATION_17_18, + MIGRATION_17_18, MIGRATION_18_19, ) .addCallback(SEED_DEFAULT_FILTERS) // No fallbackToDestructiveMigration. A listener's playback positions and play diff --git a/app/src/main/java/org/terst/listen/data/Entities.kt b/app/src/main/java/org/terst/listen/data/Entities.kt index 5a81e5d..1ef3bde 100644 --- a/app/src/main/java/org/terst/listen/data/Entities.kt +++ b/app/src/main/java/org/terst/listen/data/Entities.kt @@ -476,6 +476,16 @@ data class EpisodeMidrollEntity( * before this column existed, and a server that did not say, both read as 0. */ @ColumnInfo(defaultValue = "0") val partners: Int = 0, + /** + * Which detector found this: "audio" for the cross-correlation scan over several episodes' + * waveforms, "words" for subtraction or recurrence over their transcripts, or both names + * where the two agreed. + * + * Shown when a break is judged. The listener deciding whether a skip was right wants to + * know what decided it, and the two are not equally sure of themselves. Rows written before + * the words could find anything default to "audio", which is where they all came from. + */ + @ColumnInfo(defaultValue = "audio") val origin: String = "audio", ) /** diff --git a/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt b/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt index 3f5d04b..66b5fcc 100644 --- a/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt +++ b/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt @@ -27,6 +27,14 @@ data class EpisodeSkips( * answers and must stay different. */ val status: String?, + /** + * What decided the breaks -- the audio scan, the words, or both. + * + * Null when nothing is cut. It is here because the listener judging a skip wants to know + * what judged it first, and the two detectors are not equally sure of themselves: one + * compares waveforms across a batch of episodes, the other reads what was said. + */ + val foundBy: String? = null, ) { /** The three numbers on one line, known ones only; null when none is known. */ val numbers: String? = listOfNotNull( @@ -162,8 +170,29 @@ object SkipSummary { .takeIf { ranges.isNotEmpty() } ?.let { clock(it) }, status = if (ranges.isEmpty()) statusWithoutBreaks(episode) else null, + foundBy = foundBy(ranges), ) + /** + * Names the detectors behind an episode's breaks, in a fixed order so the line reads the + * same every time. + * + * A break the two agreed on arrives already saying so, and counts for both. + */ + internal fun foundBy(ranges: List<EpisodeMidrollEntity>): String? { + if (ranges.isEmpty()) return null + val origins = ranges.flatMap { it.origin.split("+") }.map { it.trim() }.toSet() + val named = listOfNotNull( + "the audio".takeIf { "audio" in origins }, + "the words".takeIf { "words" in origins }, + ) + return when (named.size) { + 0 -> null + 1 -> "Found by ${named.single()}" + else -> "Found by ${named.joinToString(" and ")}" + } + } + private fun statusWithoutBreaks(episode: EpisodeEntity): String = when { // Nothing has been downloaded yet, so the skips cannot have been checked either; say what // is actually happening instead of "Not checked yet". diff --git a/app/src/main/java/org/terst/listen/server/ServerSkips.kt b/app/src/main/java/org/terst/listen/server/ServerSkips.kt index 8dcd115..87aa104 100644 --- a/app/src/main/java/org/terst/listen/server/ServerSkips.kt +++ b/app/src/main/java/org/terst/listen/server/ServerSkips.kt @@ -151,6 +151,9 @@ class ServerSkips( endMs = range.endMs, kind = range.kind, partners = range.partners, + // An older server says nothing, and everything it ever found came from + // the audio scan, so that is the honest default rather than "unknown". + origin = range.origin.ifBlank { "audio" }, ) ) } diff --git a/app/src/main/java/org/terst/listen/server/SkipServer.kt b/app/src/main/java/org/terst/listen/server/SkipServer.kt index a05eaf7..68a04ed 100644 --- a/app/src/main/java/org/terst/listen/server/SkipServer.kt +++ b/app/src/main/java/org/terst/listen/server/SkipServer.kt @@ -62,6 +62,11 @@ data class ServerBreak( val kind: String = "ad", /** How many other episodes carried this audio; 0 when the server does not say. */ val partners: Int = 0, + /** + * Which detector found it: "audio", "words", or both names where they agreed. Empty when + * the server did not say, which is what an older server sends. + */ + val origin: String = "", ) /** diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeTimeline.kt b/app/src/main/java/org/terst/listen/ui/EpisodeTimeline.kt index cd8c4b6..6fe77e5 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeTimeline.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeTimeline.kt @@ -149,6 +149,15 @@ fun SkipReadout(skips: EpisodeSkips, breaks: List<SkipSummary.BreakMarker>) { Column(verticalArrangement = Arrangement.spacedBy(2.dp)) { skips.numbers?.let { Text(it, style = MaterialTheme.typography.bodyMedium) } skips.status?.let { Text(it, style = MaterialTheme.typography.bodySmall) } + // What decided the cut, which is the question anyone has about a skip they disagree + // with. Quieter than the numbers: it explains them rather than competing with them. + skips.foundBy?.let { + Text( + it, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } if (breaks.any { SkipSummary.isAdvertising(it.kind) }) { LegendEntry(MaterialTheme.colorScheme.tertiary, SkipSummary.LEGEND_ADVERTISING) } diff --git a/app/src/test/java/org/terst/listen/fingerprint/FoundBySummaryTest.kt b/app/src/test/java/org/terst/listen/fingerprint/FoundBySummaryTest.kt new file mode 100644 index 0000000..819d2a9 --- /dev/null +++ b/app/src/test/java/org/terst/listen/fingerprint/FoundBySummaryTest.kt @@ -0,0 +1,75 @@ +package org.terst.listen.fingerprint + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.terst.listen.data.EpisodeMidrollEntity + +/** + * What the episode screen says about which detector decided a skip. + * + * The listener judging a skip wants to know what judged it, and the two are not equally sure + * of themselves: one compares waveforms across a batch of episodes, the other reads what was + * said. A break the two agreed on counts for both, because agreement by entirely different + * means is the strongest thing either can offer. + */ +class FoundBySummaryTest { + + private fun range(origin: String, startMs: Long = 0) = EpisodeMidrollEntity( + episodeId = "e1", candidateId = "c$startMs", startMs = startMs, endMs = startMs + 1000, + origin = origin, + ) + + @Test + fun `breaks from the audio scan alone say so`() { + assertEquals("Found by the audio", SkipSummary.foundBy(listOf(range("audio")))) + } + + @Test + fun `breaks from the words alone say so`() { + assertEquals("Found by the words", SkipSummary.foundBy(listOf(range("words")))) + } + + @Test + fun `an episode with some of each names both`() { + val got = SkipSummary.foundBy(listOf(range("audio"), range("words", 5000))) + assertEquals("Found by the audio and the words", got) + } + + /** A break both detectors found arrives already saying so, and counts for both. */ + @Test + fun `a break the two agreed on names both`() { + assertEquals("Found by the audio and the words", SkipSummary.foundBy(listOf(range("audio+words")))) + } + + /** The order is fixed, so the line does not rearrange itself between episodes. */ + @Test + fun `the order does not depend on which break came first`() { + val one = SkipSummary.foundBy(listOf(range("words"), range("audio", 5000))) + val other = SkipSummary.foundBy(listOf(range("audio"), range("words", 5000))) + assertEquals(one, other) + } + + @Test + fun `nothing cut says nothing`() { + assertNull(SkipSummary.foundBy(emptyList())) + } + + /** A row written by a version that did not record it is not worth a sentence. */ + @Test + fun `an unrecognised origin says nothing`() { + assertNull(SkipSummary.foundBy(listOf(range("")))) + } + + /** + * A readout built without naming a detector explains nothing, which is right: the default + * belongs to the case where nothing has been cut and there is nothing to account for. + */ + @Test + fun `a readout that names no detector says nothing about one`() { + val skips = EpisodeSkips(served = "15:16", declared = "13:08", cut = null, status = "None found") + + assertNull(skips.foundBy) + assertEquals("Served 15:16 \u00b7 Declared 13:08", skips.numbers) + } +} diff --git a/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt b/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt index 3b0d671..0d5f686 100644 --- a/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt +++ b/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt @@ -516,4 +516,56 @@ class ServerSkipsTest { assertEquals(emptyList<Long>(), transcripts.cueRows.filter { it.advertising }.map { it.startMs }) } + + /* + * A break records which detector found it. + * + * The listener judging a skip wants to know what decided it, and the two detectors are not + * equally sure of themselves: one compares waveforms across a batch of episodes, the other + * reads what was said. + */ + + @Test + fun `records which detector found each break`() = runTest { + val midrolls = FakeMidrollDao() + val transport = StubTransport("""{"status":"ready","audioUrl":"/audio/a?token=t"}""") + + subject(transport, Fakes(midrolls = midrolls)).record( + episode, + ServerEpisode( + ready = true, audioUrl = "x", durationMs = null, introMs = null, outroMs = null, + detected = true, + breaks = listOf( + ServerBreak(60_000, 90_000, "ad", partners = 3, origin = "audio"), + ServerBreak(600_000, 660_000, "ad", origin = "words"), + ), + ), + ) + + assertEquals( + listOf("audio", "words"), + midrolls.rangeRows.sortedBy { it.startMs }.map { it.origin }, + ) + } + + /** + * A server that says nothing found it by the audio scan: that is where every break came + * from before the words could produce one, so "unknown" would be honest about the field + * and useless about the break. + */ + @Test + fun `a break from a server that does not say came from the audio`() = runTest { + val midrolls = FakeMidrollDao() + + subject(StubTransport(null), Fakes(midrolls = midrolls)).record( + episode, + ServerEpisode( + ready = true, audioUrl = "x", durationMs = null, introMs = null, outroMs = null, + detected = true, + breaks = listOf(ServerBreak(60_000, 90_000, "ad")), + ), + ) + + assertEquals("audio", midrolls.rangeRows.single().origin) + } } |
