Reverse: a served-shorter episode is furniture, not advertising

Measured against the owner's 334 mirrored episodes, the symmetric
durationSlopMs rule from the previous round costs far more than it
protects: 31% of episodes are served shorter than their declared
length (feeds are sloppy by tens of seconds -- one show ran -47s,
-33s, +88s, -95s across episodes), and every break in every one of
them fell to the unjudgeable default and got labelled "Ad break" --
including closing credits at 37:41 of a 38:25 episode. The owner's
furniture toggle did nothing on nearly a third of his library.

You cannot insert advertising into a file and have it come out
shorter, so a negative gap is evidence that nothing was inserted,
not evidence the feed can't be trusted. Classify's insertion test is
one-sided again: inserted := servedMs<=0 || declaredMs<=0 ||
(servedMs-declaredMs) > adUnitMs. durationSlopMs (2000) is replaced
by adUnitMs (15000) -- advertising is sold in ~15s units, so an
overrun under one unit is the feed being approximate, not an
insertion. Both unjudgeable cases (servedMs<=0, declaredMs<=0) are
unchanged; furnitureKindAt's position logic is unchanged.

This reverses the FIX-1 decision in this same file that made the
test symmetric, on the theory that a feed wrong in the short
direction couldn't be trusted to judge the other direction either.
Rewrote the two pre-existing tests that encoded that decision
(TestAnEpisodeShorterThanDeclaredIsAlsoAllFurniture, folded as a
duplicate, and the renamed
TestAServedLengthShorterThanDeclaredByFarIsStillFurnitureByPosition)
plus one comment on an unchanged assertion, and added coverage for
the new threshold's band (a 10s overrun is now furniture, not ad).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B

2 files changed, 69 insertions, 48 deletions

diff --git a/server/internal/skips/kinds.go b/server/internal/skips/kinds.go
index 89ee1cb..66d4b21 100644
--- a/server/internal/skips/kinds.go
+++ b/server/internal/skips/kinds.go
@@ -16,9 +16,12 @@ const (
 )
 
 const (
-	// durationSlopMs is how far served and declared may differ before the difference is read as
-	// an insertion. Encoders disagree about a file's length by a fraction of a second.
-	durationSlopMs = 2000
+	// adUnitMs is how far served may exceed declared before the excess is read as an insertion.
+	// Advertising is sold in units of roughly fifteen seconds -- the owner's own observation, and
+	// borne out by the corpus -- so a served length exceeding declared by less than one such unit
+	// cannot be a real inserted advertisement: it is the feed being approximate, the same way
+	// encoders disagree about a file's length by a fraction of a second.
+	adUnitMs = 15_000
 	// edgeWindowMs is how close to either end a break must sit to be the opening or the closing
 	// rather than a break cue. Two minutes: Jon Stewart's opening runs 98 seconds and Search
 	// Engine's begins at 2:00.
@@ -29,29 +32,34 @@ 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 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.
+// episode served no more than one ad unit longer than declared had nothing inserted -- the excess
+// is the feed being approximate, not an advertisement -- so whatever recurs in it is the show's
+// own audio however strongly it matched.
 //
-// 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.
+// A served length *shorter* than declared is also not evidence of insertion, by any amount: you
+// cannot insert advertising into a file and have it come out shorter, so a negative gap is
+// evidence that nothing was inserted, not evidence that the feed can't be trusted. (An earlier
+// version of this function disagreed and called every materially-shorter episode advertising,
+// worried that a feed wrong about its own length in the short direction couldn't be trusted to
+// judge the other direction either. Measured against the owner's library of 334 mirrored
+// episodes, that caution cost far more than it protected: 31% of episodes are served shorter than
+// declared -- feeds are routinely sloppy by tens of seconds -- and every break in every one of
+// them fell to the unjudgeable default and got labelled advertising, including closing credits
+// three seconds from the end of a file. Reversed here: a negative gap is classified by position
+// like any other furniture.)
+//
+// Everything else -- served more than one ad unit longer than declared, or either length unknown
+// -- is called advertising. That is still deliberately the unrefined answer for the case that is
+// genuinely 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 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
+	// A served length more than one ad unit longer than declared is evidence of an insertion. A
+	// served length shorter than declared -- by any amount -- is not: a file cannot come out
+	// shorter than it started by having something added to it, so there is no insertion to find.
+	inserted := servedMs <= 0 || declaredMs <= 0 || (servedMs-declaredMs) > adUnitMs
 
 	for _, b := range breaks {
 		b.Kind = KindAd
diff --git a/server/internal/skips/kinds_test.go b/server/internal/skips/kinds_test.go
index 16c4e74..d5da13c 100644
--- a/server/internal/skips/kinds_test.go
+++ b/server/internal/skips/kinds_test.go
@@ -54,23 +54,27 @@ func TestAnEpisodeMatchingItsDeclaredLengthIsAllFurniture(t *testing.T) {
 	}
 }
 
-// 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 != 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)
+// The served file is sometimes *shorter* than declared, often by tens of seconds -- 31% of the
+// owner's 334 mirrored episodes come in short, one show as far off as -95s. A negative gap is not
+// ambiguous the way a missing length is: you cannot insert advertising into a file and have it
+// come out shorter, so a served length under declared is evidence that nothing was inserted, not
+// evidence that the feed can't be trusted. This episode is 47s short of its declared length and
+// its break sits three seconds from the end of the served audio -- unmistakably the closing
+// credits, not an ad. (An earlier round made this case symmetric with the long direction and
+// asserted KindAd here instead, on the theory that a feed wrong about its own length in the short
+// direction couldn't be trusted to judge the other direction either. Measured against the real
+// library that cost far more than it protected: every break in every one of those 31% of episodes
+// fell to the unjudgeable default and got labelled advertising. Reversed.)
+func TestAServedLengthShorterThanDeclaredIsFurnitureByPosition(t *testing.T) {
+	got := Classify([]AdBreak{{StartMs: 2_220_000, EndMs: 2_255_000}}, 2_258_000, 2_305_000)
+
+	if got[0].Kind != KindOutro {
+		t.Errorf("kind %q, want %q -- a break 3s from the end of a file served 47s short of declared is the closing credits, not an ad", got[0].Kind, KindOutro)
 	}
 }
 
-// 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.
+// A feed can also be wrong in the short direction by only a few seconds. A negative gap of any
+// size is non-ad now, so this no longer needs slop to pass -- but it still must pass.
 func TestASecondOfDisagreementInTheShortDirectionIsNotAnAdvertisement(t *testing.T) {
 	got := Classify([]AdBreak{{StartMs: 0, EndMs: 30_000}}, 3_040_000, 3_040_900)
 
@@ -79,20 +83,29 @@ func TestASecondOfDisagreementInTheShortDirectionIsNotAnAdvertisement(t *testing
 	}
 }
 
-// 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) {
+// FIX 1 worried that a show declaring 30:00 and served 29:30 -- 28:00 of programme plus a 1:30
+// pre-roll -- would have that pre-roll read as ad-free furniture and labelled KindIntro forever.
+// That worry assumed a negative gap could still be hiding an inserted ad. It can't: a file cannot
+// come out shorter than it started by having something added to it, so there is no insertion left
+// to find once the gap is negative -- the premise of FIX 1 is rejected. A served length short of
+// declared is classified by position like any other furniture, same as the case above.
+func TestAServedLengthShorterThanDeclaredByFarIsStillFurnitureByPosition(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)
+	if got[0].Kind != KindIntro {
+		t.Errorf("kind %q, want %q -- a negative gap is evidence nothing was inserted, so this is the show's own opening", got[0].Kind, KindIntro)
+	}
+}
+
+// Advertising is sold in units of roughly fifteen seconds, so a served length exceeding declared
+// by less than one such unit cannot be a real inserted advertisement -- it is the feed being
+// approximate. This used to be read as an ad (anything over the old 2s slop was); now it is
+// furniture, classified by position like any other non-insertion.
+func TestATenSecondOverrunUnderOneAdUnitIsNotAdvertising(t *testing.T) {
+	got := Classify([]AdBreak{{StartMs: 0, EndMs: 30_000}}, 1_810_000, 1_800_000)
+
+	if got[0].Kind != KindIntro {
+		t.Errorf("kind %q, want %q -- a 10s overrun is under one ad unit, so it's the feed being approximate, not an insertion", got[0].Kind, KindIntro)
 	}
 }