From 55f4bf0b2c6df32411de84c0a383885a403236f4 Mon Sep 17 00:00:00 2001 From: Jeff Crouse Date: Sun, 2 Aug 2026 22:02:20 -0400 Subject: [PATCH] feat(feedback): fence off the play_events rows that mean 0.5 by construction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1. Three decisions wait on ADR-0004's data — its own completion-ratio threshold, ADR-0005's RADIO weights, and #53 — and the data does not start where everyone assumed. Until #57 the web client delivered a play the moment listening crossed `min(duration / 2, 4 min)` and sent `completion_ratio` as measured at that instant, never revising it. Verified again on the live database on 2026-08-02, by day and context: **795 of 823 rows predate the fix**, and every context before 2026-08-01 shows completions averaging 0.42–0.50 — including `library`, because the web derives context from the queue source and sends `library` for a library queue exactly as the native app does. From 2026-08-01 the same context reads 0.972 and 1.000, with skips at ≤0.1. So the clock restarted on 2026-08-01, not 2026-07-27: a month of trustworthy data lands around 2026-09-01. `FEEDBACK_TRUSTWORTHY_SINCE` and `trustworthy_feedback_only` make the boundary one call away instead of a matter of discipline. A date, not a client filter, because `play_events` records no client and `context` does not stand in for one — the good native rows cannot be separated from the bad web rows beside them, and selecting on the ratio would be circular. Excluded rather than deleted, for a reason the investigation turned up: the damage is not limited to the ratio. A track abandoned at 55% was recorded as a completion at ~0.5 rather than as a skip, so `outcome` is unreliable before the cutoff and skips are *under*-counted. What survives is that `skipped` and `rejected` rows describe real abandonments. `ambient._negative_signal` counts exactly those two over a rolling 90-day window — so **the live recommender was never poisoned**, it has been running on a slightly weak negative signal that heals around 2026-10-30. ADR-0005 also gains the `Implementation:` block it never had, which is why it looked unbuilt in a survey while `RADIO` had been shipping for weeks. Two follow-ups recorded: a `client` column, cheap now and worthless retroactively, which is the only thing that would have made this separable; and that whether the web still reports at all is unverified — every post-cutoff row carries the context the native app hardcodes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW --- backend/app/services/listening_feedback.py | 59 +++++++++++++++++++ backend/tests/test_listening_feedback.py | 43 ++++++++++++++ ...004-listening-feedback-is-event-sourced.md | 47 ++++++++++++++- ...ranking-engine-serves-ambient-and-radio.md | 12 ++++ 4 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 backend/app/services/listening_feedback.py create mode 100644 backend/tests/test_listening_feedback.py diff --git a/backend/app/services/listening_feedback.py b/backend/app/services/listening_feedback.py new file mode 100644 index 00000000..95400dc8 --- /dev/null +++ b/backend/app/services/listening_feedback.py @@ -0,0 +1,59 @@ +"""Which `PlayEvent` rows may be used to reason about *how much* of a track was heard. + +ADR-0004 leaves the completion-ratio threshold for `skipped` to be decided empirically +"once data exists". Data exists — but not all of it means what it says, and the boundary +is a date rather than anything recorded on the row. + +**What happened.** Until `familiar` #57 the web client delivered a play the moment +listening crossed ``min(duration / 2, 4 min)`` and sent ``completion_ratio`` as measured +*at that instant*. Nothing revised it afterwards, so a web play landed at almost exactly +0.5 whether the listener heard half the track or all of it. Measured on the live database +on 2026-08-01: **289 of 357 completed events sat in the 0.5–0.6 bucket**, against native +client rows correctly reading 0.95–1.00. + +**Why a date and not a client column.** `play_events` records no client, and `context` +does not stand in for one — the web derives it from the queue source and sends `library` +for a library queue, exactly as the native app does. So the good native rows from before +the fix cannot be separated from the bad web rows beside them. Selecting on the ratio +itself would be circular: it is the variable being measured. A date cutoff is the only +separator that does not assume the answer, and it costs the native rows in that window. + +**The damage is not limited to the ratio.** Because a play was *reported* at the halfway +mark, a track abandoned at 55% was recorded as a completion at ~0.5 rather than as a skip. +So before the cutoff the `outcome` labels are unreliable too, and skips are *under*-counted +rather than merely imprecise. + +**What is still safe to use from before the cutoff.** Rows recorded as `skipped` or +`rejected` describe real abandonments and keep their ratios; they are simply an incomplete +census. `ambient._negative_signal` counts exactly those two outcomes over a rolling 90-day +window, so the recommender was never poisoned — it has been running on a slightly weaker +negative signal, which heals as the window passes the cutoff. That is the reason these rows +are kept rather than deleted. + +Anything reasoning about `completion_ratio` or trusting `outcome == 'completed'` must go +through `trustworthy_feedback_only` below. +""" + +from datetime import datetime + +from sqlalchemy import ColumnElement + +from app.db.models import PlayEvent + +# The first day whose events were all recorded by clients that report at the end of a +# track. `familiar` #57 shipped 2026-08-01; every row from that date on has been verified +# against the live database as completions clustering at ≥0.9 and skips at ≤0.1. +# +# Naive UTC, because `PlayEvent.started_at` is TIMESTAMP WITHOUT TIME ZONE — see +# `app.utils.time.utcnow` for why the whole codebase is naive here. +FEEDBACK_TRUSTWORTHY_SINCE = datetime(2026, 8, 1) + + +def trustworthy_feedback_only() -> ColumnElement[bool]: + """The filter every completion-ratio query must apply. + + A function rather than a bare comparison so the reason travels with the call: a + `WHERE started_at >= ` sitting in an analysis script explains nothing, and the + next person to write one would reasonably remove it. + """ + return PlayEvent.started_at >= FEEDBACK_TRUSTWORTHY_SINCE diff --git a/backend/tests/test_listening_feedback.py b/backend/tests/test_listening_feedback.py new file mode 100644 index 00000000..3ae83f67 --- /dev/null +++ b/backend/tests/test_listening_feedback.py @@ -0,0 +1,43 @@ +"""The cutoff that keeps ADR-0004's threshold from being derived from a bug.""" + +from datetime import datetime + +import pytest +from sqlalchemy import select + +from app.db.models import PlayEvent +from app.services.listening_feedback import ( + FEEDBACK_TRUSTWORTHY_SINCE, + trustworthy_feedback_only, +) + + +def test_cutoff_is_the_day_the_web_client_started_reporting_at_the_end(): + """`familiar` #57. Moving this earlier silently readmits the 0.5-by-construction rows.""" + assert FEEDBACK_TRUSTWORTHY_SINCE == datetime(2026, 8, 1) + + +def test_cutoff_is_naive_to_match_the_column(): + """`PlayEvent.started_at` is TIMESTAMP WITHOUT TIME ZONE; an aware bound raises.""" + assert FEEDBACK_TRUSTWORTHY_SINCE.tzinfo is None + + +def test_filter_compiles_into_the_expected_predicate(): + clause = str( + select(PlayEvent.id).where(trustworthy_feedback_only()).compile() + ) + assert "play_events.started_at >=" in clause + + +@pytest.mark.parametrize( + ("started_at", "trustworthy"), + [ + (datetime(2026, 7, 31, 23, 59, 59), False), # the last contaminated day + (datetime(2026, 8, 1, 0, 0, 0), True), # the boundary is inclusive + (datetime(2026, 8, 2), True), + (datetime(2026, 7, 27), False), # the first day of data at all + ], +) +def test_boundary_is_inclusive_and_excludes_everything_before(started_at, trustworthy): + """Expressed as data rather than as a date comparison, so an off-by-one shows up.""" + assert (started_at >= FEEDBACK_TRUSTWORTHY_SINCE) is trustworthy diff --git a/docs/decisions/ADR-0004-listening-feedback-is-event-sourced.md b/docs/decisions/ADR-0004-listening-feedback-is-event-sourced.md index b4c2e480..9cee4097 100644 --- a/docs/decisions/ADR-0004-listening-feedback-is-event-sourced.md +++ b/docs/decisions/ADR-0004-listening-feedback-is-event-sourced.md @@ -158,4 +158,49 @@ Record listening as **events**, and derive aggregates from them rather than only [ADR-0005](ADR-0005-one-ranking-engine-serves-ambient-and-radio.md) depends on. History starts at the migration. - **Follow-up:** Decide the completion-ratio threshold for `skipped` empirically once data exists, - rather than fixing it by convention now. + rather than fixing it by convention now. **Blocked until roughly 2026-09-01 — see below.** + +**The data did not start accumulating when this shipped.** Until `familiar` #57 the web client +delivered a play the moment listening crossed `min(duration / 2, 4 min)` and sent +`completion_ratio` as measured at that instant, never revising it. A web play therefore landed at +almost exactly 0.5 whether the listener heard half the track or all of it. Measured on the live +database on 2026-08-01: **289 of 357 completed events sat in the 0.5–0.6 bucket**, against native +rows correctly reading 0.95–1.00. + +Verified again on 2026-08-02, by day and context. Every context before 2026-08-01 shows completions +averaging 0.42–0.50 — including `library`, because the web derives `context` from the queue source +and sends `library` for a library queue exactly as the native app does. From 2026-08-01 the same +context reads 0.972 and 1.000, and skips cluster at ≤0.1. Of 823 rows, **795 predate the fix**. + +Three consequences worth stating plainly, because the first is the one that would have gone +unnoticed: + +1. **The clock restarted on 2026-08-01, not 2026-07-27.** A month of trustworthy data lands around + 2026-09-01. This follow-up, ADR-0005's weight tuning, and `familiar` #53 all inherit that date. +2. **The contaminated rows are excluded, not deleted** (`services/listening_feedback.py`: + `FEEDBACK_TRUSTWORTHY_SINCE`, `trustworthy_feedback_only`). `play_events` records no client and + `context` does not stand in for one, so the good native rows cannot be separated from the bad web + rows beside them; selecting on the ratio would be circular, since that is the variable being + measured. A date is the only separator that does not assume the answer, and it costs those native + rows. Deleting would also throw away rows that are still useful — see 3. +3. **`outcome` is unreliable before the cutoff too, and skips are under-counted.** A track abandoned + at 55% was recorded as a completion at ~0.5 rather than as a skip. What survives is that rows + marked `skipped` or `rejected` describe real abandonments; they are an incomplete census rather + than a wrong one. `ambient._negative_signal` counts exactly those two outcomes over a rolling + 90-day window, so **the live recommender was never poisoned** — it has been running on a slightly + weak negative signal, which heals as the window passes the cutoff (around 2026-10-30). + +Volume in the clean window, as of 2026-08-02: **28 events across 26 distinct tracks over two days**. +At that rate a month yields roughly 400 events — enough to place a completion-ratio threshold, thin +for ADR-0005's weight tuning, which may want longer or a narrower first pass. + +- **Follow-up:** `play_events` records no client, which is the only reason the good native rows from + before the cutoff had to be discarded with the bad web ones. A nullable `client` column, sent by + all three clients, would make the next contamination separable instead of fatal to a whole window. + Cheap now, worthless applied retroactively. +- **Follow-up:** Whether the web client still reports at all is **unverified**. Every row since the + cutoff carries `context = 'library'`, which is what the native app hardcodes; the web's other + contexts (`other`, `null`, `playlist`) stop on 2026-07-31. That is equally consistent with nobody + having used the web app since, and cannot be told apart from the rows alone — which is the + preceding follow-up restated as a live question. Settle it by playing one track in the web app and + looking for a non-`library` context, not by reasoning about the data. diff --git a/docs/decisions/ADR-0005-one-ranking-engine-serves-ambient-and-radio.md b/docs/decisions/ADR-0005-one-ranking-engine-serves-ambient-and-radio.md index 963b7778..07f6a1f5 100644 --- a/docs/decisions/ADR-0005-one-ranking-engine-serves-ambient-and-radio.md +++ b/docs/decisions/ADR-0005-one-ranking-engine-serves-ambient-and-radio.md @@ -4,6 +4,18 @@ Status: accepted Date: 2026-07-26 +Implementation: +- Shipped: `services/ranking_profiles.py` carries the profiles, with `RADIO` at line 92 and the + registry at 121; ambient and radio both rank through it, and the negative signal from ADR-0004 + arrives via `ambient._negative_signal`. +- Recorded late, on 2026-08-02. The decision had been executed for weeks with no `Implementation:` + block, which is how it came to look unbuilt in a survey of the set. +- **Both follow-ups below are blocked until roughly 2026-09-01**, and not for the reason the dates + suggest. ADR-0004's data did not begin accumulating usably until `familiar` #57 landed on + 2026-08-01: 795 of the first 823 rows carry a completion ratio of ~0.5 by construction. Tuning + `RADIO` against them would fit the weights to a client bug. See ADR-0004's Implementation section + for the measurement and for `FEEDBACK_TRUSTWORTHY_SINCE`, which any tuning query must apply. + Extends [ADR-0004](ADR-0004-listening-feedback-is-event-sourced.md). ## Context