fix: Make pack search workable and absorb legacy torznab config - #635
Conversation
Three fixes for the pack-search issue cluster reported by Freddster: - #632: pack eligibility no longer requires ENABLE_32P — any torrent or Torznab provider qualifies once torrent search is enabled. - #633: AllowPacks and IgnoreType finally have a write path. New PATCH /api/series/{id}/search-settings endpoint plus a "Search options" section on the series detail page with Allow packs / Ignore book type toggles. Also hardens rsscheck's truthiness checks so a stored "0" string is not read as enabled. - #631: populated legacy torznab_* fields under [Torznab] are folded into extra_torznabs on every config read (not just CONFIG_VERSION < 8); incomplete entries log a loud warning instead of sitting silently inert. Fixes #631, fixes #632, fixes #633. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M42w2uyDjCg6tEymhJGkzf
🦋 Changeset detectedLatest commit: 0c60b37 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change migrates legacy Torznab configuration into ChangesSearch configuration and settings
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SeriesDetailPage
participant useUpdateSeriesSearchSettings
participant SeriesRouter
participant SeriesService
participant ComicsQueries
SeriesDetailPage->>useUpdateSeriesSearchSettings: submit search setting
useUpdateSeriesSearchSettings->>SeriesRouter: PATCH search-settings
SeriesRouter->>SeriesService: validate and update
SeriesService->>ComicsQueries: persist normalized values
ComicsQueries-->>SeriesService: return refreshed settings
SeriesService-->>SeriesDetailPage: return updated settings
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
CodeRabbit review on #635: _validate_loaded_provider_extras rejects provider ids reserved by built-ins (experimental=101, DDL=200/201) and duplicate provider names, either of which would fail startup right after the legacy absorb. Skip reserved ids when assigning the migrated entry's id and warn-and-skip on a name collision instead of migrating. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M42w2uyDjCg6tEymhJGkzf
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@comicarr/config.py`:
- Around line 560-574: Update the _scrub() method to set self.TORZNAB_VERIFY to
None alongside the other legacy Torznab attributes after removing the parser
options.
- Around line 594-602: Update the legacy provider migration logic around
canonical_name and existing_names to include names from both self.EXTRA_TORZNABS
and the Newznab provider-extra list before appending the migrated Torznab entry.
Preserve the existing reserved-name check and warning behavior, and add a
regression test covering a Newznab/Torznab name collision.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 14944095-c622-4d63-90b8-cd9bdab01b65
📒 Files selected for processing (13)
.changeset/legacy-torznab-fields-migrate.md.changeset/pack-search-actually-works.mdcomicarr/app/series/queries.pycomicarr/app/series/router.pycomicarr/app/series/service.pycomicarr/config.pycomicarr/rsscheck.pycomicarr/search.pyfrontend/src/hooks/useSeries.tsfrontend/src/pages/SeriesDetailPage.tsxfrontend/src/types/entities.tstests/unit/test_config_version_migrations.pytests/unit/test_series_domain.py
- Clear TORZNAB_VERIFY alongside the other legacy attributes in _scrub() - Check the migrated name against BOTH extras lists: provider-name validation shares one namespace across newznabs and torznabs, so a legacy torznab entry reusing a Newznab name would have failed Config.read(). Regression test added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M42w2uyDjCg6tEymhJGkzf
* fix: Search the Newznab categories you configured Three follow-ups from the documentation pass on #668, all found while checking what the Search Providers page should actually say. 1. Newznab categories never reached the searcher. Field 5 of a Newznab record is `uid#categories`, but the Settings Categories box wrote straight into it, so `7030,7020` was stored as `7030#7020` -- uid 7030, one category. Worse, `get_extras()` rewrote the field on its way to runtime ('#' to ',', then the leading ',' back to '#'), leaving a shape `search.py` reads as "no category configured". Every Usenet query fell back to the built-in 7030 unless the stored value happened to begin with '#'. The rewrite is gone, the uid has its own Settings field, and the categories box means categories. New indexers default to 7030 (Books/Comics) rather than 5030, a TV category. 2. Provider booleans were tolerated at the edge and disagreed on in the middle. `_PROVIDER_BOOLEAN_VALUES` accepts eight spellings; `search.py` reads verify as `bool(int(field))` and filters enabled with `== "1"`, while `health.py` and the providers API accept true/yes/on. An entry stored as `True` was therefore reported enabled by the Acquisition tab and skipped by the searcher, and a non-numeric verify raised ValueError mid-search. Both fields are canonicalised in `parse_provider_extras`, the one boundary every reader and writer passes through. The legacy-torznab absorption added in #635 wrote exactly that shape: it passed `self.TORZNAB_VERIFY` through as a raw bool, which serialised to "True" and came back as "True" on the next startup. It now writes "1"/"0" itself as well. 3. `TORZNAB_VERIFY` defaulted to False, so an absorbed legacy entry inherited every field from the operator except TLS verification, which it silently turned off. Now True. Also: the absorption's log messages pointed operators at "the Settings UI (extra_torznabs)". There is no Torznab editor -- Settings/Search edits Newznab only. They now name `extra_torznabs` under `[Torznab]` in config.ini and show the entry format. Docs for all of this: frankieramirez/comicarr-docs#9. Tests: category round-trip and uid preservation through the providers service, boolean canonicalisation across all eight spellings and through serialisation, absorbed-entry verify defaults, unmangled runtime categories, and the new Settings field. 2414 backend and 438 frontend tests pass; `npm run lint` green on all lanes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QjqbhfDFmmMeBvn5Pqbk4z * fix: Normalize category lists to survive how operators type them `7030, 7020` was stored with the space intact and would have reached the indexer as `cat=7030, 7020`. It never showed before because the categories were not reaching the searcher at all; now that they are, the field has to tolerate padding and empty entries. Applies to both provider types and to the projection, so what Settings displays, what is stored, and what is queried are the same list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QjqbhfDFmmMeBvn5Pqbk4z --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Fixes the pack-search issue cluster reported by @Freddster — all three verified against the codebase before fixing.
#632 — pack gate no longer requires 32P
NZB_SEARCH()demandedall([ENABLE_TORRENT_SEARCH, ENABLE_32P])before any pack release could match, blocking packs from every other torrent/Torznab provider. The gate now requires onlyENABLE_TORRENT_SEARCH.#633 — AllowPacks / IgnoreType finally have a write path
The
comics.AllowPacksandcomics.IgnoreTypecolumns were read extensively bysearch.py/search_filer.py/rsscheck.pybut never written anywhere — so packs could never match and booktype mismatches were always fatal, for every series, in every install.PATCH /api/series/{comic_id}/search-settingsendpoint (partial update, boolean-validated) → service →db.upsert("comics", …)useUpdateSeriesSearchSettingshook); both columns added to the series projectionAllowPacks(Text) as"1"/"0",IgnoreType(Integer) as1/0rsscheck.py'sbool(...)truthiness on both flags — a stored"0"string would have read as enabled via the RSS temp-table path#631 — legacy torznab_* fields are absorbed, not silently ignored
The single-provider
[Torznab]fields were only consumed by theCONFIG_VERSION < 8migration, so re-populating them on a modern config did nothing, with no warning. A new_absorb_legacy_torznab()runs on every config read: a complete entry is folded intoextra_torznabs(fresh provider id, deduped by host) and the stale keys scrubbed from config.ini; an incomplete entry logs a loud warning pointing at the Settings UI.Caveat / follow-up
The non-DDL pack parsing pipeline in
search_filer.pyis partially vestigial in this fork — only DDL entries getpack=Truetoday — so Ignore book type is the flag doing most of the work for torrent bundle matching. Full Torznab pack-range parsing (issue-range extraction + per-issue reconciliation for torrent packs) is a follow-up.Testing
npm run lintlanes all green (modern, guards, backend, generated, frontend + prettier); frontendtsc --noEmitcleanFixes #631, fixes #632, fixes #633.
🤖 Generated with Claude Code
https://claude.ai/code/session_01M42w2uyDjCg6tEymhJGkzf
Summary by CodeRabbit
New Features
Bug Fixes