add C90 linting rule#536
Conversation
📝 WalkthroughWalkthroughRefactors indexer system by constraining repository base class generics to Pydantic ChangesIndexer System Refactoring & Type Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@media_manager/common/repository.py`:
- Line 2: Remove the now-unused module-level TypeVar import and assignment:
delete the TypeVar import from the top of the module and remove the `T =
TypeVar("T")` binding, since BaseRepository uses PEP 695 class-level type params
(`class BaseRepository[T, S: BaseModel]`) and those make the module-level `T`
and `S` TypeVar declarations dead code; ensure no other symbols reference the
removed `T` binding before committing.
In `@media_manager/indexer/indexers/jackett.py`:
- Around line 179-182: The movie query branch is incorrectly gating movie ID
params on TV capability flags; update the conditionals in the movie helper so
they check movie capabilities instead of TV ones — replace uses of
search_capabilities.supports_tv_search_tvdb with
search_capabilities.supports_movie_search_tvdb and
search_capabilities.supports_tv_search_tmdb with
search_capabilities.supports_movie_search_tmdb (keeping the param checks for
"tvdbid" and "tmdbid" and assignment to query_params["tvdbid"]/["tmdbid"]
intact) to ensure movie ID params are included when movie search support exists.
In `@media_manager/indexer/indexers/torznab_mixin.py`:
- Line 35: The code dereferences item.find("enclosure") directly (used when
setting is_usenet and later at the second occurrence), which will raise if the
enclosure node is missing; change both spots to first assign enclosure =
item.find("enclosure"), check if enclosure is None and if so log a warning
(include identifying context like item title or guid) and skip/continue that
item, otherwise access enclosure.attrib["type"] (and other attrib keys)
safely—look for the assignments to is_usenet and the later direct
enclosure.attrib usage in torznab_mixin.py and wrap them with this guard to
avoid exceptions.
- Line 83: The check for uploadvolumefactor uses int(value) which raises on
valid decimal strings like "2.0"; change the logic that currently does if
int(value) == 2 to parse as a float instead (e.g. val = float(value); if val ==
2.0) and wrap the conversion in a try/except ValueError to gracefully skip or
handle malformed values; update the comparison site that references
uploadvolumefactor/value in torznab_mixin.py so numeric decimal inputs are
accepted.
In `@media_manager/indexer/utils.py`:
- Around line 26-29: The pass/fail gate in media_manager/indexer/utils.py
currently treats query_result.score <= 0 as a failure which contradicts
downstream logic that retains score >= 0; update the condition to treat only
negative scores as failures (i.e., change the check from <= 0 to < 0 around
query_result.score) so that the function returning (query_result, True) matches
downstream filtering; keep the same return shapes (return query_result, False /
True) and adjust any nearby comments/logs referencing the threshold if present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c0cb428b-1d8c-4803-8d7d-2fa4f453d668
📒 Files selected for processing (5)
media_manager/common/repository.pymedia_manager/indexer/indexers/jackett.pymedia_manager/indexer/indexers/torznab_mixin.pymedia_manager/indexer/utils.pyruff.toml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-push (arm64, ubuntu-24.04-arm)
- GitHub Check: build-and-push (amd64, ubuntu-24.04)
🔇 Additional comments (3)
ruff.toml (2)
9-9: Pending-rule comment is now aligned with the config state.Line 9 correctly removes
C90from the “to be enabled” note, which keeps the comment accurate.
12-12:C90is correctly enabled inextend-select.Line 12 implements the PR objective directly and cleanly.
media_manager/common/repository.py (1)
18-18: ⚡ Quick winNo action required — the project's minimum Python version (3.13) fully supports PEP 695 syntax.
The project specifies
requires-python = ">=3.13"in pyproject.toml. Since PEP 695 type parameter syntax requires Python 3.12+, the code is compatible with all supported Python versions.> Likely an incorrect or invalid review comment.
| @@ -2,6 +2,7 @@ | |||
| from typing import Any, TypeVar | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify TypeVar is not used anywhere else in the file
rg -n "TypeVar" media_manager/common/repository.pyRepository: maxdorninger/MediaManager
Length of output: 123
🏁 Script executed:
cat -n media_manager/common/repository.py | head -30Repository: maxdorninger/MediaManager
Length of output: 1071
🏁 Script executed:
rg -n "^from media_manager.common.repository import|^import media_manager.common.repository" --type pyRepository: maxdorninger/MediaManager
Length of output: 350
Remove unused TypeVar import and dead code assignment.
The PEP 695 class-level type parameter syntax on line 18 (class BaseRepository[T, S: BaseModel]) introduces T and S as implicit type parameters, making the module-level T = TypeVar("T") on line 14 and the TypeVar import on line 2 unused dead code.
♻️ Proposed fix
-from typing import Any, TypeVar
+from typing import Any
from uuid import UUID-T = TypeVar("T")
EntityId = UUID | int | str📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from typing import Any, TypeVar | |
| from typing import Any |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@media_manager/common/repository.py` at line 2, Remove the now-unused
module-level TypeVar import and assignment: delete the TypeVar import from the
top of the module and remove the `T = TypeVar("T")` binding, since
BaseRepository uses PEP 695 class-level type params (`class BaseRepository[T, S:
BaseModel]`) and those make the module-level `T` and `S` TypeVar declarations
dead code; ensure no other symbols reference the removed `T` binding before
committing.
| elif search_capabilities.supports_tv_search_tvdb and "tvdbid" in params: | ||
| query_params["tvdbid"] = params["tvdbid"] | ||
| elif search_capabilities.supports_tv_search_tmdb and "tmdbid" in params: | ||
| query_params["tmdbid"] = params["tmdbid"] |
There was a problem hiding this comment.
Movie query path checks TV capability flags by mistake.
At Line 179-182, the movie helper uses supports_tv_search_* flags. This can suppress valid movie ID parameters and reduce query quality.
Proposed fix
- elif search_capabilities.supports_tv_search_tvdb and "tvdbid" in params:
+ elif search_capabilities.supports_movie_search_tvdb and "tvdbid" in params:
query_params["tvdbid"] = params["tvdbid"]
- elif search_capabilities.supports_tv_search_tmdb and "tmdbid" in params:
+ elif search_capabilities.supports_movie_search_tmdb and "tmdbid" in params:
query_params["tmdbid"] = params["tmdbid"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@media_manager/indexer/indexers/jackett.py` around lines 179 - 182, The movie
query branch is incorrectly gating movie ID params on TV capability flags;
update the conditionals in the movie helper so they check movie capabilities
instead of TV ones — replace uses of search_capabilities.supports_tv_search_tvdb
with search_capabilities.supports_movie_search_tvdb and
search_capabilities.supports_tv_search_tmdb with
search_capabilities.supports_movie_search_tmdb (keeping the param checks for
"tvdbid" and "tmdbid" and assignment to query_params["tvdbid"]/["tmdbid"]
intact) to ensure movie ID params are included when movie search support exists.
| is_usenet = ( | ||
| item.find("enclosure").attrib["type"] != "application/x-bittorrent" | ||
| ) | ||
| is_usenet = item.find("enclosure").attrib["type"] != "application/x-bittorrent" |
There was a problem hiding this comment.
Guard enclosure access before using attrib fields.
At Line 35 and Line 53, item.find("enclosure") is dereferenced directly. A missing enclosure node will raise and discard the item via the outer exception path; handle this explicitly and skip cleanly with a warning.
Proposed fix
- is_usenet = item.find("enclosure").attrib["type"] != "application/x-bittorrent"
+ enclosure = item.find("enclosure")
+ if enclosure is None:
+ log.warning("Torznab item missing enclosure, skipping.")
+ return None
+ is_usenet = enclosure.attrib.get("type") != "application/x-bittorrent"
...
- download_url=str(item.find("enclosure").attrib["url"]),
+ download_url=str(enclosure.attrib.get("url", "")),Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@media_manager/indexer/indexers/torznab_mixin.py` at line 35, The code
dereferences item.find("enclosure") directly (used when setting is_usenet and
later at the second occurrence), which will raise if the enclosure node is
missing; change both spots to first assign enclosure = item.find("enclosure"),
check if enclosure is None and if so log a warning (include identifying context
like item title or guid) and skip/continue that item, otherwise access
enclosure.attrib["type"] (and other attrib keys) safely—look for the assignments
to is_usenet and the later direct enclosure.attrib usage in torznab_mixin.py and
wrap them with this guard to avoid exceptions.
| elif name == "downloadvolumefactor": | ||
| self._add_leech_flags(float(value), flags) | ||
| elif name == "uploadvolumefactor": | ||
| if int(value) == 2: |
There was a problem hiding this comment.
uploadvolumefactor parsing can fail for decimal values.
At Line 83, int(value) will fail for valid values like "2.0", which can cause items to be dropped. Parse as float and compare numerically.
Proposed fix
- elif name == "uploadvolumefactor":
- if int(value) == 2:
+ elif name == "uploadvolumefactor":
+ if float(value) == 2.0:
flags.append("doubleupload")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if int(value) == 2: | |
| elif name == "uploadvolumefactor": | |
| if float(value) == 2.0: | |
| flags.append("doubleupload") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@media_manager/indexer/indexers/torznab_mixin.py` at line 83, The check for
uploadvolumefactor uses int(value) which raises on valid decimal strings like
"2.0"; change the logic that currently does if int(value) == 2 to parse as a
float instead (e.g. val = float(value); if val == 2.0) and wrap the conversion
in a try/except ValueError to gracefully skip or handle malformed values; update
the comparison site that references uploadvolumefactor/value in torznab_mixin.py
so numeric decimal inputs are accepted.
| if query_result.score <= 0: | ||
| return query_result, False | ||
|
|
||
| return query_result, True |
There was a problem hiding this comment.
Non-positive pass check conflicts with downstream retention of zero-score results.
At Line 26, score 0 is marked as failed, but downstream filtering still keeps score >= 0, so zero-score entries are effectively retained despite failing. This causes inconsistent scoring behaviour and misleading logs.
Proposed fix
- if query_result.score <= 0:
+ if query_result.score < 0:
return query_result, False📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if query_result.score <= 0: | |
| return query_result, False | |
| return query_result, True | |
| if query_result.score < 0: | |
| return query_result, False | |
| return query_result, True |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@media_manager/indexer/utils.py` around lines 26 - 29, The pass/fail gate in
media_manager/indexer/utils.py currently treats query_result.score <= 0 as a
failure which contradicts downstream logic that retains score >= 0; update the
condition to treat only negative scores as failures (i.e., change the check from
<= 0 to < 0 around query_result.score) so that the function returning
(query_result, True) matches downstream filtering; keep the same return shapes
(return query_result, False / True) and adjust any nearby comments/logs
referencing the threshold if present.
1 participant
This PR enables the C90 listing rule, which prevents too complex code.
Summary by CodeRabbit
Refactor
Chores