2 files changed, 59 insertions, 14 deletions
diff --git a/server/internal/skips/kinds.go b/server/internal/skips/kinds.go
index e2518dd..89ee1cb 100644
--- a/server/internal/skips/kinds.go
+++ b/server/internal/skips/kinds.go
@@ -29,19 +29,29 @@ const (
 //
 // The length gap is the evidence, and the point of using it is that the detector does not
 // produce it: the feed declares the ad-free length and the file is whatever was served. An
-// episode no longer than its declared length had nothing inserted, so whatever recurs in it is
-// the show's own audio however strongly it matched.
+// episode served at the same length as declared -- within encoding slop, in either direction --
+// had nothing inserted, so whatever recurs in it is the show's own audio however strongly it
+// matched.
 //
-// Everything else is called advertising. That is deliberately the unrefined answer -- an
-// ad-carrying episode also contains its own intro and cue -- but it matches what the app does
-// today, and separating the two inside such an episode needs the furniture library, which cannot
-// confirm anything until a second fetch occasion has passed.
+// Everything else is called advertising, including a served length materially *shorter* than
+// declared: a feed wrong about its own length in that direction cannot be trusted to judge
+// whether anything was inserted in the other one either, and treating "shorter" as proof of
+// nothing-inserted let a real pre-roll (a feed declaring 30:00 and serving 29:30) fall into the
+// opening window and get labelled the show's own intro. Calling everything else advertising is
+// also deliberately the unrefined answer for the case that is judgeable -- an ad-carrying episode
+// also contains its own intro and cue -- but it matches what the app does today, and separating
+// the two inside such an episode needs the furniture library, which cannot confirm anything until
+// a second fetch occasion has passed.
 func Classify(breaks []AdBreak, servedMs, declaredMs int64) []AdBreak {
 	out := make([]AdBreak, 0, len(breaks))
 	// If we can't judge (missing served or declared length), treat conservatively as inserted ads.
-	// If both lengths are known and the file is longer than declared by more than encoding slop,
-	// it contains inserted ads.
-	inserted := servedMs <= 0 || declaredMs <= 0 || (servedMs > 0 && declaredMs > 0 && servedMs-declaredMs > durationSlopMs)
+	// If both lengths are known and they differ by more than encoding slop in either direction,
+	// the feed's declared length cannot be trusted as the ad-free baseline, so treat it as inserted.
+	gapMs := servedMs - declaredMs
+	if gapMs < 0 {
+		gapMs = -gapMs
+	}
+	inserted := servedMs <= 0 || declaredMs <= 0 || gapMs > durationSlopMs
 
 	for _, b := range breaks {
 		b.Kind = KindAd
@@ -60,7 +70,10 @@ func Classify(breaks []AdBreak, servedMs, declaredMs int64) []AdBreak {
 // by the nearest edge.
 func furnitureKindAt(b AdBreak, servedMs int64) BreakKind {
 	if servedMs <= 0 {
-		return KindCue // shouldn't happen but be safe
+		// Unreachable today -- Classify sets inserted = true whenever servedMs <= 0, so this is
+		// never called with one. Fail-safe anyway: if that precondition ever moves, an
+		// unjudgeable length must still read as advertising, not as the show's own furniture.
+		return KindAd
 	}
 
 	// Use a window size proportional to the episode length for very short episodes.
diff --git a/server/internal/skips/kinds_test.go b/server/internal/skips/kinds_test.go
index 544b9ea..16c4e74 100644
--- a/server/internal/skips/kinds_test.go
+++ b/server/internal/skips/kinds_test.go
@@ -54,13 +54,45 @@ func TestAnEpisodeMatchingItsDeclaredLengthIsAllFurniture(t *testing.T) {
 	}
 }
 
-// The served file is sometimes *shorter* than declared -- Sleepy History runs about 1:36 short on
-// every episode. A feed can be wrong in that direction too, and it is still not advertising.
+// The served file is sometimes *shorter* than declared by more than encoding slop -- and that is
+// exactly the case Classify cannot trust: a feed that understates how much was served is wrong in
+// the same way one that overstates it is, and the fix for FIX 1 made the test symmetric instead of
+// one-sided. This used to assert KindOutro (the old one-sided rule read any served-at-or-under-
+// declared length as ad-free); it now asserts KindAd, because a feed wrong by 96 seconds cannot be
+// trusted to judge whether anything was inserted, which is exactly the unjudgeable case Classify
+// already answers with KindAd. Kept rather than deleted: it is still the case that documents why.
 func TestAnEpisodeShorterThanDeclaredIsAlsoAllFurniture(t *testing.T) {
 	got := Classify([]AdBreak{{StartMs: 2_793_000, EndMs: 2_989_000}}, 2_963_000, 3_059_000)
 
-	if got[0].Kind != KindOutro {
-		t.Errorf("kind %q, want %q -- a short file has no insertion either", got[0].Kind, KindOutro)
+	if got[0].Kind != KindAd {
+		t.Errorf("kind %q, want %q -- a feed wrong by 96s in the short direction can't be trusted either", got[0].Kind, KindAd)
+	}
+}
+
+// A feed can also be wrong in the short direction by only a few seconds, which is exactly what
+// slop exists to absorb -- this is the symmetric case of TestASecondOfDisagreementIsNotAnAdvertisement.
+func TestASecondOfDisagreementInTheShortDirectionIsNotAnAdvertisement(t *testing.T) {
+	got := Classify([]AdBreak{{StartMs: 0, EndMs: 30_000}}, 3_040_000, 3_040_900)
+
+	if got[0].Kind == KindAd {
+		t.Error("900ms of rounding in the short direction was read as inserted advertising")
+	}
+}
+
+// FIX 1: a show that declares 30:00 and is served 29:30 -- 28:00 of programme plus a 1:30
+// pre-roll -- used to pass Classify's one-sided test (servedMs - declaredMs is negative, so
+// nothing was read as inserted) and the pre-roll at 0-90s fell inside the opening window and was
+// labelled KindIntro. With the furniture toggle off, that advertisement would play in full on
+// every episode of the show forever, with nothing anywhere reporting it as advertising. The rule
+// has to be symmetric: only a served length within slop of declared, in either direction, is
+// evidence that nothing was inserted. A served length materially shorter than declared means the
+// feed cannot be trusted as a judge, which is the unjudgeable case Classify already answers with
+// KindAd.
+func TestAServedLengthShorterThanDeclaredByMoreThanSlopIsUnjudgeableAndTreatedAsAdvertising(t *testing.T) {
+	got := Classify([]AdBreak{{StartMs: 0, EndMs: 90_000}}, 1_770_000, 1_800_000)
+
+	if got[0].Kind != KindAd {
+		t.Errorf("kind %q, want %q -- a feed declaring 30:00 and serving 29:30 can't be trusted to judge whether its own pre-roll is advertising", got[0].Kind, KindAd)
 	}
 }