Skip to content

Commit 8ded080

Browse files
committed
Fix RSS guid collisions and episode titles flagged by Codex review
Codex review on PR #828 found two real bugs and one real stability issue in the guid/title logic just added: - Episodes/seasons of the same show share the show's media_id, so the old tmdb:<media_id> guid fallback collapsed every episode of an un-enriched show onto one guid. - Episode Item.title stores the episode's own name (e.g. "Pilot"), not the show's title, so episode feed titles were unmatchable. - Guid identity shifted when provider_external_ids gained an imdb_id after the item was first published, making existing subscribers see a "new" duplicate entry. Replace the external-id-preference guid with one built from the item's own immutable natural key (source:media_type:media_id[:sN[:eN]]) — unique per episode and stable across metadata enrichment — and prefix episode/season titles with the parent show's title via a small batched lookup, mirroring the existing _attach_owner_media_statuses pattern. Also codify PR monitoring as a repo default in AGENTS.md: proactively watch any PR opened here to green and verify automated review comments against the code before acting on them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B92VR5siWNQE2BjGGwbch8
1 parent f6fc056 commit 8ded080

3 files changed

Lines changed: 89 additions & 36 deletions

File tree

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,12 @@ Notes:
368368
- Review summaries should call out behavior changes, files touched, validation run, and remaining risk.
369369
- Commit messages should use a short imperative title, then 1–3 bullet clarifications in the body. Optional issue lines: `Fixes #123` / `Refs #456`.
370370
371+
### PR Monitoring (Default Behavior)
372+
- For any PR opened in this repo — whether the user opened it or Claude did — proactively subscribe to its GitHub activity (e.g. `subscribe_pr_activity`) without waiting to be asked.
373+
- Drive CI to fully green regardless of cause. Diagnose real failures and fix them; only treat a run as flaky if it died before any test body ran (checkout, dependency install, lost runner), and say so when re-running rather than silently retrying.
374+
- For automated review comments (e.g. Codex/`chatgpt-codex-connector`), verify each claim against the actual code before acting — read the flagged lines and the code paths they describe. Fix genuine, correctness-affecting findings. For anything speculative, stylistic-only, or that misreads the code, leave a brief reply explaining why rather than churning changes to satisfy noise.
375+
- Keep the PR's subscription alive until it merges or closes; webhooks don't reliably deliver CI success or new-push events, so re-check state after periods of silence rather than assuming green.
376+
371377
## Security / Safety Notes
372378
- `.env` contains secrets and API keys; do not commit it.
373379
- A process-wide log record factory redacts credentials before any handler writes them (`src/app/log_safety.py`, installed by `src/config/__init__.py`). Do not move the installation later in the start sequence, and do not widen its `except` clause: both faults are silent. See `docs/architecture/log-redaction.md`.

src/lists/feeds.py

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from django.utils.feedgenerator import Rss201rev2Feed
88
from django.views.decorators.http import require_GET
99

10-
from app.models import MediaManager, MediaTypes, Sources
10+
from app.models import Item, MediaManager, MediaTypes, Sources
1111
from app.providers import tmdb
1212
from app.templatetags.app_tags import media_url
1313
from lists.models import CustomList, CustomListItem
@@ -89,6 +89,30 @@ def _attach_owner_media_statuses(self, list_items, owner):
8989
list_item.feed_status = status_by_item_id.get(list_item.item_id, "")
9090
list_item.feed_description = self._build_item_description(list_item.item)
9191

92+
def _attach_show_titles(self, list_items):
93+
"""Attach the parent show's title to episode/season list items."""
94+
show_keys = {
95+
(list_item.item.source, list_item.item.media_id)
96+
for list_item in list_items
97+
if list_item.item.media_type
98+
in (MediaTypes.EPISODE.value, MediaTypes.SEASON.value)
99+
}
100+
if not show_keys:
101+
return
102+
103+
sources = {source for source, _media_id in show_keys}
104+
media_ids = {media_id for _source, media_id in show_keys}
105+
shows = Item.objects.filter(
106+
media_type=MediaTypes.TV.value,
107+
source__in=sources,
108+
media_id__in=media_ids,
109+
)
110+
title_by_key = {(show.source, show.media_id): show.title for show in shows}
111+
112+
for list_item in list_items:
113+
key = (list_item.item.source, list_item.item.media_id)
114+
list_item.feed_show_title = title_by_key.get(key)
115+
92116
def _build_item_description(self, item):
93117
"""Return a local feed description without provider lookups."""
94118
manual_metadata = getattr(item, "manual_metadata", None) or {}
@@ -134,12 +158,13 @@ def items(self, obj):
134158
.order_by("-date_added")
135159
)
136160
self._attach_owner_media_statuses(list_items, obj.owner)
161+
self._attach_show_titles(list_items)
137162
return list_items
138163

139164
def item_title(self, item):
140165
"""Return the item title with S01E02/year markers for automation tools."""
141166
media_item = item.item
142-
title = media_item.title
167+
title = getattr(item, "feed_show_title", None) or media_item.title
143168

144169
if media_item.season_number is not None:
145170
title += f" S{media_item.season_number:02d}"
@@ -160,34 +185,21 @@ def item_link(self, item):
160185
"""Return the item URL."""
161186
return self.request.build_absolute_uri(media_url(item.item))
162187

163-
def _external_guid(self, media_item):
164-
"""Return a stable external id for the item, or None if unresolved."""
165-
external_ids = media_item.provider_external_ids or {}
166-
167-
imdb_id = external_ids.get("imdb_id")
168-
if imdb_id:
169-
return f"imdb:{imdb_id}"
170-
171-
tvdb_id = external_ids.get("tvdb_id")
172-
if tvdb_id:
173-
return f"tvdb:{tvdb_id}"
174-
175-
tmdb_id = external_ids.get("tmdb_id")
176-
if tmdb_id:
177-
return f"tmdb:{tmdb_id}"
178-
179-
if media_item.source == Sources.TMDB.value:
180-
return f"tmdb:{media_item.media_id}"
188+
def item_guid(self, item):
189+
"""Return a guid from the item's own immutable natural key."""
190+
media_item = item.item
191+
guid = f"{media_item.source}:{media_item.media_type}:{media_item.media_id}"
181192

182-
return None
193+
if media_item.season_number is not None:
194+
guid += f":s{media_item.season_number}"
195+
if media_item.episode_number is not None:
196+
guid += f":e{media_item.episode_number}"
183197

184-
def item_guid(self, item):
185-
"""Return a stable external id, falling back to the detail link."""
186-
return self._external_guid(item.item) or self.item_link(item)
198+
return guid
187199

188200
def item_guid_is_permalink(self, item):
189201
"""Return whether the guid is a dereferenceable URL."""
190-
return self._external_guid(item.item) is None
202+
return False
191203

192204
def item_categories(self, item):
193205
"""Return the media type as an RSS category for filtering."""

src/lists/tests/test_views.py

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3361,13 +3361,19 @@ def test_movie_title_includes_year(self):
33613361

33623362
self.assertIn("RSS Feed Movie (2024)", titles)
33633363

3364-
def test_episode_title_includes_season_episode_markers(self):
3365-
"""Episode items get "S01E02" markers for automation-tool matching."""
3364+
def test_episode_title_uses_show_title_not_episode_name(self):
3365+
"""Episode titles are prefixed with the show's title, not the episode's own name."""
3366+
Item.objects.create(
3367+
media_id="tmdb-show-1",
3368+
source=Sources.TMDB.value,
3369+
media_type=MediaTypes.TV.value,
3370+
title="RSS Feed Show",
3371+
)
33663372
episode = Item.objects.create(
3367-
media_id="tmdb-episode-1",
3373+
media_id="tmdb-show-1",
33683374
source=Sources.TMDB.value,
33693375
media_type=MediaTypes.EPISODE.value,
3370-
title="RSS Feed Show",
3376+
title="Pilot",
33713377
season_number=1,
33723378
episode_number=2,
33733379
)
@@ -3378,9 +3384,38 @@ def test_episode_title_includes_season_episode_markers(self):
33783384
titles = [item.findtext("title") for item in root.findall("./channel/item")]
33793385

33803386
self.assertIn("RSS Feed Show S01E02", titles)
3387+
self.assertNotIn("Pilot S01E02", titles)
33813388

3382-
def test_guid_uses_imdb_id_when_available(self):
3383-
"""Items with a resolved IMDb id get a stable, non-permalink guid."""
3389+
def test_guid_unique_per_episode_of_same_show(self):
3390+
"""Episodes of the same show get distinct guids, not a shared show-level guid."""
3391+
episode1 = Item.objects.create(
3392+
media_id="tmdb-show-2",
3393+
source=Sources.TMDB.value,
3394+
media_type=MediaTypes.EPISODE.value,
3395+
title="Episode One",
3396+
season_number=1,
3397+
episode_number=1,
3398+
)
3399+
episode2 = Item.objects.create(
3400+
media_id="tmdb-show-2",
3401+
source=Sources.TMDB.value,
3402+
media_type=MediaTypes.EPISODE.value,
3403+
title="Episode Two",
3404+
season_number=1,
3405+
episode_number=2,
3406+
)
3407+
CustomListItem.objects.create(custom_list=self.custom_list, item=episode1)
3408+
CustomListItem.objects.create(custom_list=self.custom_list, item=episode2)
3409+
3410+
response = self.client.get(reverse("list_rss", args=[self.custom_list.id]))
3411+
root = ET.fromstring(response.content)
3412+
guids = {item.findtext("guid") for item in root.findall("./channel/item")}
3413+
3414+
self.assertIn("tmdb:episode:tmdb-show-2:s1:e1", guids)
3415+
self.assertIn("tmdb:episode:tmdb-show-2:s1:e2", guids)
3416+
3417+
def test_guid_ignores_provider_external_ids(self):
3418+
"""Guid is derived from the item's own natural key, not mutable external ids."""
33843419
movie = Item.objects.create(
33853420
media_id="tmdb-movie-2",
33863421
source=Sources.TMDB.value,
@@ -3398,18 +3433,18 @@ def test_guid_uses_imdb_id_when_available(self):
33983433
if item.findtext("title") == "RSS Feed Movie With IMDb"
33993434
)
34003435

3401-
self.assertEqual(guid.text, "imdb:tt1234567")
3436+
self.assertEqual(guid.text, "tmdb:movie:tmdb-movie-2")
34023437
self.assertEqual(guid.get("isPermaLink"), "false")
34033438

3404-
def test_guid_falls_back_to_link_without_external_id(self):
3405-
"""Items with no resolvable external id keep a permalink guid."""
3439+
def test_guid_is_never_a_permalink(self):
3440+
"""Guid is always a stable id, never the internal detail-page URL."""
34063441
response = self.client.get(reverse("list_rss", args=[self.custom_list.id]))
34073442
root = ET.fromstring(response.content)
34083443
item = root.find("./channel/item")
34093444
guid = item.find("guid")
34103445

3411-
self.assertEqual(guid.text, item.findtext("link"))
3412-
self.assertEqual(guid.get("isPermaLink"), "true")
3446+
self.assertEqual(guid.text, "igdb:game:rss-1")
3447+
self.assertEqual(guid.get("isPermaLink"), "false")
34133448

34143449
def test_item_category_reflects_media_type(self):
34153450
"""RSS items expose their media type as a category."""

0 commit comments

Comments
 (0)