Personalized pipeline: refresh snapshot on first-run too
Reproduced: selecting Fresh Tape (or any kind never generated before)
and running the pipeline silently skipped — UI showed
"No tracks in Fresh Tape — skipping sync" with no clue why.
Root cause: ensure_playlist auto-creates the playlist row on first
access with `track_count=0` and `last_generated_at=NULL`, but
`is_stale=0` by default (the column default — fresh rows aren't
"stale", they're "never generated"). Pipeline only refreshed when
`is_stale=True` OR `refresh_first=True`, so first-run rows fell
through both branches → read the empty snapshot → skip.
Fix: pipeline now also refreshes when `existing.last_generated_at is
None`. Same control flow, one extra condition:
if refresh_first OR is_stale OR last_generated_at is None:
refresh
else:
read existing snapshot
This is the right signal: "has the generator ever run for this row"
is exactly what `last_generated_at` tracks (the column is set in
`_persist_snapshot` after every successful refresh).
Stubs in test_handlers_personalized_pipeline.py updated to expose
`last_generated_at` on their SimpleNamespace returns so the new
attribute read doesn't AttributeError. Fresh stubs get a non-None
timestamp so they're treated as already-generated; the new test
`test_never_generated_snapshot_triggers_first_refresh` pins the
first-run-forces-refresh behavior with `last_generated_at=None`.
This commit is contained in:
parent
08725094db
commit
d861a40277
2 changed files with 60 additions and 9 deletions
|
|
@ -196,16 +196,20 @@ def _build_payloads_for_kinds(
|
|||
continue
|
||||
|
||||
try:
|
||||
# Determine whether to refresh: explicit user flag, OR the
|
||||
# snapshot is marked stale because the underlying source
|
||||
# data (discovery_pool, curated_playlists) changed since
|
||||
# last generation. Either way → refresh; otherwise just
|
||||
# read the existing snapshot.
|
||||
# Refresh when ANY of:
|
||||
# - explicit user flag (cron use case: regenerate each run)
|
||||
# - snapshot marked stale by upstream data refresher
|
||||
# - playlist was never generated yet (auto-created by
|
||||
# ensure_playlist; track_count=0, last_generated_at=NULL).
|
||||
# Without this branch, a first-run pipeline reads the
|
||||
# empty snapshot and silently skips — user picks a kind,
|
||||
# hits run, gets "No tracks to sync" with no clue why.
|
||||
if refresh_first:
|
||||
record = manager.refresh_playlist(kind, variant, profile_id)
|
||||
else:
|
||||
existing = manager.ensure_playlist(kind, variant, profile_id)
|
||||
if existing.is_stale:
|
||||
needs_first_gen = existing.last_generated_at is None
|
||||
if existing.is_stale or needs_first_gen:
|
||||
record = manager.refresh_playlist(kind, variant, profile_id)
|
||||
else:
|
||||
record = existing
|
||||
|
|
|
|||
|
|
@ -151,8 +151,11 @@ class TestEmptyConfig:
|
|||
|
||||
class _StubManagerNoTracks:
|
||||
def ensure_playlist(self, kind, variant, profile_id):
|
||||
# last_generated_at non-None so pipeline treats the snapshot as
|
||||
# already-generated-but-empty (rather than first-run-needs-gen).
|
||||
return SimpleNamespace(
|
||||
id=1, name=f'{kind}-{variant}', kind=kind, variant=variant,
|
||||
is_stale=False, last_generated_at='2026-05-15T20:00:00',
|
||||
)
|
||||
|
||||
def refresh_playlist(self, kind, variant, profile_id):
|
||||
|
|
@ -173,7 +176,7 @@ class _StubManagerWithTracks:
|
|||
return SimpleNamespace(
|
||||
id=hash((kind, variant)) % 10000,
|
||||
name=f'{kind}-{variant or "S"}', kind=kind, variant=variant,
|
||||
is_stale=False,
|
||||
is_stale=False, last_generated_at='2026-05-15T20:00:00',
|
||||
)
|
||||
|
||||
def refresh_playlist(self, kind, variant, profile_id):
|
||||
|
|
@ -183,7 +186,7 @@ class _StubManagerWithTracks:
|
|||
return SimpleNamespace(
|
||||
id=hash((kind, variant)) % 10000,
|
||||
name=f'{kind}-{variant or "S"}', kind=kind, variant=variant,
|
||||
is_stale=False,
|
||||
is_stale=False, last_generated_at='2026-05-15T20:00:00',
|
||||
)
|
||||
|
||||
def get_playlist_tracks(self, playlist_id):
|
||||
|
|
@ -283,11 +286,13 @@ class TestPayloadBuilding:
|
|||
def ensure_playlist(self, kind, variant, profile_id):
|
||||
return SimpleNamespace(
|
||||
id=1, name=kind, kind=kind, variant=variant, is_stale=True,
|
||||
last_generated_at='2026-05-15T20:00:00',
|
||||
)
|
||||
def refresh_playlist(self, kind, variant, profile_id):
|
||||
refresh_called.append((kind, variant))
|
||||
return SimpleNamespace(
|
||||
id=1, name=kind, kind=kind, variant=variant, is_stale=False,
|
||||
last_generated_at='2026-05-15T20:00:00',
|
||||
)
|
||||
def get_playlist_tracks(self, _id):
|
||||
return [SimpleNamespace(
|
||||
|
|
@ -315,11 +320,13 @@ class TestPayloadBuilding:
|
|||
def ensure_playlist(self, kind, variant, profile_id):
|
||||
return SimpleNamespace(
|
||||
id=1, name=kind, kind=kind, variant=variant, is_stale=False,
|
||||
last_generated_at='2026-05-15T20:00:00',
|
||||
)
|
||||
def refresh_playlist(self, *_a, **_k):
|
||||
refresh_called.append('called')
|
||||
return SimpleNamespace(
|
||||
id=1, name='x', kind='x', variant='', is_stale=False,
|
||||
last_generated_at='2026-05-15T20:00:00',
|
||||
)
|
||||
def get_playlist_tracks(self, _id):
|
||||
return [SimpleNamespace(
|
||||
|
|
@ -335,6 +342,43 @@ class TestPayloadBuilding:
|
|||
)
|
||||
assert refresh_called == []
|
||||
|
||||
def test_never_generated_snapshot_triggers_first_refresh(self):
|
||||
"""First-run case: pipeline picks a brand-new kind, ensure_playlist
|
||||
auto-creates the row with track_count=0 and last_generated_at=None.
|
||||
Without this branch the pipeline would read the empty snapshot and
|
||||
silently skip — user picked a kind and got nothing. With the branch,
|
||||
last_generated_at=None forces a refresh so the generator actually runs."""
|
||||
deps = _build_deps()
|
||||
refresh_called = []
|
||||
|
||||
class _NeverGenMgr:
|
||||
def ensure_playlist(self, kind, variant, profile_id):
|
||||
return SimpleNamespace(
|
||||
id=1, name=kind, kind=kind, variant=variant,
|
||||
is_stale=False, last_generated_at=None,
|
||||
)
|
||||
def refresh_playlist(self, kind, variant, profile_id):
|
||||
refresh_called.append((kind, variant))
|
||||
return SimpleNamespace(
|
||||
id=1, name=kind, kind=kind, variant=variant,
|
||||
is_stale=False, last_generated_at='2026-05-15T20:00:00',
|
||||
)
|
||||
def get_playlist_tracks(self, _id):
|
||||
return [SimpleNamespace(
|
||||
track_name='Generated', artist_name='A', album_name='Al',
|
||||
spotify_track_id='sp-new', itunes_track_id=None,
|
||||
deezer_track_id=None, duration_ms=200000,
|
||||
)]
|
||||
|
||||
payloads = _build_payloads_for_kinds(
|
||||
deps, _NeverGenMgr(),
|
||||
[{'kind': 'fresh_tape'}],
|
||||
profile_id=1, automation_id=None, refresh_first=False,
|
||||
)
|
||||
assert refresh_called == [('fresh_tape', '')]
|
||||
assert len(payloads) == 1
|
||||
assert payloads[0]['tracks_json'][0]['name'] == 'Generated'
|
||||
|
||||
def test_manager_exception_swallowed_continues_to_next(self):
|
||||
deps = _build_deps()
|
||||
|
||||
|
|
@ -345,7 +389,10 @@ class TestPayloadBuilding:
|
|||
self.calls.append(kind)
|
||||
if kind == 'broken':
|
||||
raise RuntimeError('manager boom')
|
||||
return SimpleNamespace(id=1, name=kind, kind=kind, variant=variant, is_stale=False)
|
||||
return SimpleNamespace(
|
||||
id=1, name=kind, kind=kind, variant=variant, is_stale=False,
|
||||
last_generated_at='2026-05-15T20:00:00',
|
||||
)
|
||||
def get_playlist_tracks(self, _id):
|
||||
return []
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue