[TVC] Refactor TVC description generation and fix BBCode parser issues'#1247
[TVC] Refactor TVC description generation and fix BBCode parser issues'#1247Lusephur wants to merge 14 commits intoAudionut:masterfrom
Conversation
TVC changed the site use of the bbc description box to something hybrid. if a /n appears after [center] it injects <br> breaking the centring /n/n becomes <br><br> This is a quick fix, removing the errors the new site parser produces.
Collapse multiple newlines into a single newline for cleaner output.
TVC changed the site use of the bbc description box to something hybrid. if a/nappears after [center] it injects <br> breaking the centring/n/nbecomes <br><br> This is a quick fix, removing the errors the new site parser produces. and some refactoring for ease of editing for others
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors Changes
Sequence Diagram(s)(No sequence diagrams generated — changes are internal refactoring of description generation and I/O without a new multi-component sequential feature requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/trackers/TVC.py`:
- Around line 95-121: In _build_disc_info, the first-disc special-case only
handles DVD and skips when discs[0]['type'] == "BDMV", losing its summary;
modify _build_disc_info to handle a BDMV first disc (e.g., if discs[0]['type']
== "BDMV" append its summary in a spoiler like the loop does) or simplify by
removing the special-case and iterating over all discs (use the same BDMV/DVD
handling logic used in the existing for disc in discs[1:] loop) so the BDMV
summary for discs[0] is included; make changes around the first-disc check and
the for loop in _build_disc_info to ensure BDMV summaries are appended.
🧹 Nitpick comments (4)
src/trackers/TVC.py (4)
89-93: Synchronousos.makedirsin async flow.The rest of the codebase provides
COMMON.makedirsas an async wrapper (seesrc/trackers/COMMON.pyline 41). This call is unlikely to block noticeably for local dirs, but for consistency you could use the async version.
355-365: Redundant import ofconsoleinside exception handler.
consoleis already imported at the module level (line 18). The local import on line 364 is unnecessary.♻️ Proposed fix
except Exception as e: - from src.console import console console.print(f"[yellow]Warning: Failed to write description file: {e}[/yellow]")
301-325: Variablerequired_countserves dual purpose (minimum threshold and display cap).On line 313 it acts as a minimum (
screens_count < required_count), and on line 318 it acts as a maximum (image_list[:required_count]). This works but may confuse future readers. Consider renaming to something likescreenshot_countto better reflect its dual role.
800-837: Duplicated episode-list construction logic.Lines 809–820 and 826–837 are nearly identical blocks building the
episodeslist fromseason_info. Consider extracting this into a small helper to reduce duplication.♻️ Proposed refactor
Extract a helper method:
def _extract_episodes(self, season_info: dict[str, Any]) -> list[dict[str, str]]: episodes = [] for ep in cast(list[dict[str, Any]], season_info.get('episodes', [])): season_num = str(ep.get('season_number', 0)) episode_num = str(ep.get('episode_number', 0)) code = f"S{season_num.zfill(2)}E{episode_num.zfill(2)}" episodes.append({ "code": code, "title": (ep.get("name") or "").strip(), "airdate": ep.get("air_date") or "", "overview": (ep.get("overview") or "").strip() }) return episodesThen replace both blocks with
meta['episodes'] = self._extract_episodes(season_info).
fixing first disc BDMV summary is silently skipped in description. And added note that presently TVC doesn't allow BDMV uploads
Lusephur
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/trackers/TVC.py`:
- Around line 307-331: The config value required_count fetched in
_add_screenshots may be a string; coerce it to an int (with try/except) before
using it for comparison and slicing: read required_count from
self.config['TRACKERS'][self.tracker].get('image_count',
self.MIN_SCREENSHOTS_REQUIRED), attempt int(required_count), on ValueError or
TypeError fall back to self.MIN_SCREENSHOTS_REQUIRED, and ensure it's >= 0; then
use that integer for the screens_count < required_count check and for slicing
image_list[:required_count].
- Around line 956-993: The code accesses self.config['IMAGES'][img_key] without
ensuring the IMAGES section or the specific icon key exists, which can raise
KeyError; update the loop that builds parts (the link_configs / for id_key,
url_func, img_key in link_configs) to first check if 'IMAGES' in self.config and
img_key in self.config['IMAGES'] before reading self.config['IMAGES'][img_key],
and if the check fails either skip appending the image link or use a safe
fallback URL/string so missing config entries don't crash the method.
🧹 Nitpick comments (5)
src/trackers/TVC.py (5)
351-359: Double-newlines in description builders are immediately collapsed — consider using single\nfor clarity.Multiple helper methods (e.g.,
_build_disc_info,_build_movie_desc,_build_notes_section) deliberately emit\n\n, but_normalize_tvc_formattingthen collapses all\n{2,}→\n. The code works correctly for TVC's parser requirements, but the intentional double-newlines in builders are misleading since they'll never survive to the output. Using single\nin the builders (or adding a comment explaining the collapse) would improve readability.
361-371: Redundant import ofconsoleon line 370.
consoleis already imported at module level (line 18). The local import is unnecessary.Proposed fix
except Exception as e: - from src.console import console console.print(f"[yellow]Warning: Failed to write description file: {e}[/yellow]")
806-843: Duplicated episode-list construction logic between the two branches.Lines 815–826 and 832–843 are nearly identical — both build the
episodeslist fromseason_info. Consider extracting the episode parsing into a small helper to reduce duplication.Proposed helper extraction
+ def _parse_episodes_from_season(self, season_info: dict[str, Any]) -> list[dict[str, str]]: + episodes: list[dict[str, str]] = [] + for ep in cast(list[dict[str, Any]], season_info.get('episodes', [])): + season_num = str(ep.get('season_number', 0)) + episode_num = str(ep.get('episode_number', 0)) + code = f"S{season_num.zfill(2)}E{episode_num.zfill(2)}" + episodes.append({ + "code": code, + "title": (ep.get("name") or "").strip(), + "airdate": ep.get("air_date") or "", + "overview": (ep.get("overview") or "").strip() + }) + return episodesThen both branches become:
meta['episodes'] = self._parse_episodes_from_season(season_info)
291-305: Minor: sentence splitting may break on abbreviations (e.g., "U.S.", "Dr.").The regex
(?<=[.!?])\s+will split after periods in abbreviations. This is an edge case that rarely matters in TV/movie overviews but worth noting.
550-555: Variabletypeshadows the Python built-in.Consider renaming to
release_typeormedia_typeto avoid shadowingtype().
| def _add_screenshots( | ||
| self, | ||
| meta: Meta, | ||
| image_list: list[dict[str, Any]] | ||
| ) -> str: | ||
| """Add screenshots section if requirements are met.""" | ||
| screens_count = int(meta.get('screens', 0) or 0) | ||
| required_count = self.config['TRACKERS'][self.tracker].get( | ||
| 'image_count', | ||
| self.MIN_SCREENSHOTS_REQUIRED | ||
| ) | ||
|
|
||
| if not image_list or screens_count < required_count: | ||
| return "" | ||
|
|
||
| parts = ["\n[b]Screenshots[/b]\n"] | ||
|
|
||
| for img in image_list[:required_count]: | ||
| web_url = img['web_url'] | ||
| img_url = img['img_url'] | ||
| parts.append( | ||
| f"[url={web_url}][img={self.SCREENSHOT_THUMB_SIZE}]{img_url}[/img][/url]" | ||
| ) | ||
|
|
||
| return "".join(parts) |
There was a problem hiding this comment.
required_count from config may be a string, causing TypeError on comparison and slicing.
self.config['TRACKERS'][self.tracker].get('image_count', ...) could return a string if loaded from a config file (YAML/INI). Line 319 compares screens_count (int) to required_count (potentially str), and line 324 uses it as a slice bound — both will raise TypeError in Python 3.
Proposed fix
- required_count = self.config['TRACKERS'][self.tracker].get(
+ required_count = int(self.config['TRACKERS'][self.tracker].get(
'image_count',
self.MIN_SCREENSHOTS_REQUIRED
- )
+ ))🤖 Prompt for AI Agents
In `@src/trackers/TVC.py` around lines 307 - 331, The config value required_count
fetched in _add_screenshots may be a string; coerce it to an int (with
try/except) before using it for comparison and slicing: read required_count from
self.config['TRACKERS'][self.tracker].get('image_count',
self.MIN_SCREENSHOTS_REQUIRED), attempt int(required_count), on ValueError or
TypeError fall back to self.MIN_SCREENSHOTS_REQUIRED, and ensure it's >= 0; then
use that integer for the screens_count < required_count check and for slicing
image_list[:required_count].
Remove unused import statement for console.
Refactor description building for movie and TV pack releases to use multi-block style with [center] and [pre] tags. Update various parts to improve formatting and consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/trackers/TVC.py`:
- Around line 213-255: _build_episode_desc currently returns early when
'episode_overview' is missing which causes logo, broadcast info, external links
and screenshots to be dropped; modify _build_episode_desc (and similarly
_build_tv_pack_desc) to remove the early return and instead treat
episode_overview (or season_air_first_date) as optional: keep the parts list
construction for logo, episode_name, broadcast info, links (get_links) and
screenshots (_add_screenshots) regardless of overview presence, only skip adding
the overview block when it's missing, and return "".join(parts) so
links/screenshots always appear even if TMDB metadata is absent.
🧹 Nitpick comments (3)
src/trackers/TVC.py (3)
355-355: Remove commented-out code.Line 355 has
convert_pre_to_codecommented out. If this was intentionally disabled for the new multi-block style using[pre]tags, remove the dead line to keep the code clean.
967-1010:get_linksreturns a header even when no links are generated.If none of the ID keys (
imdb_id,tmdb_id, etc.) are present inmeta, the method returns"[b]External Info Sources:[/b]\n\n"with no actual links. This orphaned header would appear in the description. Consider building the header conditionally or moving it after the loop.Proposed fix
- parts = ["[b]External Info Sources:[/b]\n\n"] + parts = [] # Configuration for each link type: (meta_key, url_builder, config_image_key) link_configs = [ ... ] for id_key, url_func, img_key in link_configs: if meta.get(id_key, 0): ... - return "".join(parts) + if parts: + return "[b]External Info Sources:[/b]\n\n" + "".join(parts) + return ""
306-320: Remove_format_overviewas it is unused code.This method is defined on line 306 but is never called anywhere in the codebase. No description builders or other methods reference it.
| def _build_episode_desc( | ||
| self, | ||
| meta: Meta, | ||
| image_list: list[dict[str, Any]] | ||
| ) -> str: | ||
| """Build description for single episode releases (FNP-style multi-block).""" | ||
| if 'episode_overview' not in meta: | ||
| return "" | ||
|
|
||
| parts = [] | ||
|
|
||
| # Logo in its own center block | ||
| if meta.get("logo"): | ||
| logo_size = self.config['DEFAULT'].get('logo_size', self.DEFAULT_LOGO_SIZE) | ||
| parts.append(f"[center][img={logo_size}]{meta['logo']}[/img][/center]\n\n") | ||
|
|
||
| # Episode title in pre tags with its own center block | ||
| episode_name = meta.get('episode_name', '').strip() | ||
| if episode_name: | ||
| parts.append(f"[center][pre]{episode_name}[/pre][/center]\n\n") | ||
|
|
||
| # Overview in pre tags with its own center block | ||
| overview = meta.get('episode_overview', '').strip() | ||
| if overview: | ||
| parts.append(f"[center][pre]{overview}[/pre][/center]\n\n") | ||
|
|
||
| # Broadcast info (without pre tags) | ||
| if 'episode_airdate' in meta: | ||
| channel = meta.get('networks', 'N/A') | ||
| formatted_date = self.format_date_ddmmyyyy(meta['episode_airdate']) | ||
| parts.append(f"[center][b]Broadcast on:[/b] {channel} on {formatted_date}[/center]\n\n") | ||
|
|
||
| # External links | ||
| links = self.get_links(meta).strip() | ||
| if links: | ||
| parts.append(f"[center]{links}[/center]\n\n") | ||
|
|
||
| # Screenshots | ||
| screenshots = self._add_screenshots(meta, image_list).strip() | ||
| if screenshots: | ||
| parts.append(f"[center]{screenshots}[/center]\n\n") | ||
|
|
||
| return "".join(parts) |
There was a problem hiding this comment.
Early return drops screenshots and links when episode_overview is missing.
If episode_overview is not in meta (e.g., TMDB fetch failed), _build_episode_desc returns "", which silently omits not only the overview but also the logo, broadcast info, external links, and screenshots. The same pattern exists in _build_tv_pack_desc (line 178–179) for missing season_air_first_date.
Consider restructuring so that links and screenshots are always included, and only the missing metadata is skipped:
Proposed approach for _build_episode_desc
def _build_episode_desc(
self,
meta: Meta,
image_list: list[dict[str, Any]]
) -> str:
"""Build description for single episode releases (FNP-style multi-block)."""
- if 'episode_overview' not in meta:
- return ""
-
parts = []
# Logo in its own center block
if meta.get("logo"):
logo_size = self.config['DEFAULT'].get('logo_size', self.DEFAULT_LOGO_SIZE)
parts.append(f"[center][img={logo_size}]{meta['logo']}[/img][/center]\n\n")
# Episode title in pre tags with its own center block
episode_name = meta.get('episode_name', '').strip()
if episode_name:
parts.append(f"[center][pre]{episode_name}[/pre][/center]\n\n")
# Overview in pre tags with its own center block
overview = meta.get('episode_overview', '').strip()
if overview:
parts.append(f"[center][pre]{overview}[/pre][/center]\n\n")🤖 Prompt for AI Agents
In `@src/trackers/TVC.py` around lines 213 - 255, _build_episode_desc currently
returns early when 'episode_overview' is missing which causes logo, broadcast
info, external links and screenshots to be dropped; modify _build_episode_desc
(and similarly _build_tv_pack_desc) to remove the early return and instead treat
episode_overview (or season_air_first_date) as optional: keep the parts list
construction for logo, episode_name, broadcast info, links (get_links) and
screenshots (_add_screenshots) regardless of overview presence, only skip adding
the overview block when it's missing, and return "".join(parts) so
links/screenshots always appear even if TMDB metadata is absent.
Removed 'pre' tags from various parts of the output for a cleaner presentation. TVC doesn't support them
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/trackers/TVC.py (1)
553-558:⚠️ Potential issue | 🟡 MinorThe
not desccheck is unreachable.
unit3d_edit_descguarantees a non-empty return via the fallback on lines 950–951 ("[center][i]No description available[/i][/center]\n"). The warning on line 557 about "DESCRIPTION file not found" is also misleading sinceunit3d_edit_descbuilds the description in-memory — a falsy return would not indicate a missing file.Proposed fix — remove the dead check
desc = await self.unit3d_edit_desc(meta, self.tracker, self.signature, image_list) - if not desc: - console.print(f"[yellow]Warning: DESCRIPTION file not found at {descfile_path}") - desc = ""
🧹 Nitpick comments (5)
src/trackers/TVC.py (5)
352-364: Remove commented-out code.Line 355 contains a commented-out call to
convert_pre_to_code. Per the PR objective (removingpretags), this line should be removed rather than left as a comment.Proposed fix
def _apply_bbcode_transforms(self, desc: str, comparison: bool) -> str: """Apply BBCode transformations.""" bbcode = BBCODE() -# desc = bbcode.convert_pre_to_code(desc) desc = bbcode.convert_hide_to_spoiler(desc)
373-382:_write_description_fileuses synchronous I/O wrapped inasyncio.to_threaddespiteaiofilesbeing available.The file already imports and uses
aiofileselsewhere (line 662). Consider using it here for consistency.Proposed fix
async def _write_description_file(self, filepath: str, content: str) -> None: """Write description content to file asynchronously.""" try: - def _write(): - with open(filepath, "w", encoding="utf-8") as f: - f.write(content) - - await asyncio.to_thread(_write) + async with aiofiles.open(filepath, "w", encoding="utf-8") as f: + await f.write(content) except Exception as e: console.print(f"[yellow]Warning: Failed to write description file: {e}[/yellow]")
927-934: Inconsistent truthiness check fortv_pack.Lines 929/931 use
meta.get('tv_pack') == 1/!= 1, while line 579 usesmeta['tv_pack']as a boolean directly. Sincemeta['tv_pack']is always initialized for TV content, prefer consistent boolean truthiness checks.Proposed fix
- elif meta['category'] == "TV" and meta.get('tv_pack') == 1: + elif meta['category'] == "TV" and meta['tv_pack']: desc_parts.append(self._build_tv_pack_desc(meta, image_list)) - elif meta['category'] == "TV" and meta.get('tv_pack') != 1: + elif meta['category'] == "TV" and not meta['tv_pack']: desc_parts.append(self._build_episode_desc(meta, image_list))Based on learnings,
meta['tv_pack']is always initialized for TV content insrc/getseasonep.pybefore tracker upload methods are called.
817-854: Duplicated episode-list construction logic in both branches.The
tv_packbranch (lines 818–837 and 838–854) builds theepisodeslist identically regardless of whether the data comes from a TMDB API call ormeta['tmdb_season_data']. Extract the episode-building loop into a small helper to reduce duplication.Proposed approach
+ def _extract_episodes_from_season(self, season_info: dict[str, Any]) -> list[dict[str, str]]: + """Extract episode list from season info dict.""" + episodes: list[dict[str, str]] = [] + for ep in cast(list[dict[str, Any]], season_info.get('episodes', [])): + season_num = str(ep.get('season_number', 0)) + episode_num = str(ep.get('episode_number', 0)) + code = f"S{season_num.zfill(2)}E{episode_num.zfill(2)}" + episodes.append({ + "code": code, + "title": (ep.get("name") or "").strip(), + "airdate": ep.get("air_date") or "", + "overview": (ep.get("overview") or "").strip() + }) + return episodesThen both branches simply call:
meta['episodes'] = self._extract_episodes_from_season(season_info)
306-320: Remove unused_format_overviewmethod.This method is defined but never called anywhere in the codebase. All overview data throughout the file (in
_build_movie_desc,_build_tv_pack_desc, and_build_episode_list) is appended directly without formatting.Either apply this formatting to the actual overview usage sites or remove the method entirely.
Updated description formatting for movie and TV releases to remove pre tags and improve clarity.
bloody tvc and whatever changes they made to the description box. Still ain't rendering right, the site parses the DESCRIPTION.txt and places <br><br> in where a blank line should be, breaking centering and other additions.
2 participants
TVC changed the site use of the bbc description box to something hybrid. if a
/nappears after [center] it injects<br>breaking the centring/n/nbecomes<br><br>This is a quick fix, removing the errors the new site parser produces. and some refactoring for easy updating
Summary by CodeRabbit
New Features
Improvements