fix: Search the Newznab categories you configured - #669
Conversation
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
🦋 Changeset detectedLatest commit: 207996b 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: 48 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)
📝 WalkthroughWalkthroughNewznab providers now store RSS user IDs separately from categories. Provider updates preserve this ID while editing categories. Provider booleans use canonical values, and absorbed legacy Torznab entries default to TLS verification. ChangesProvider configuration updates
Estimated code review effort: 3 (Moderate) | ~20 minutes 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 |
![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
🤖 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 658-661: Update the legacy Torznab migration log in the
surrounding configuration migration logic to use the approved logger.fdebug(...)
method instead of logger.info(...), preserving the existing message and
formatting.
🪄 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: e614a07a-b5b8-44b7-817f-d62ec96b38c2
📒 Files selected for processing (9)
.changeset/newznab-categories-actually-searched.mdcomicarr/app/config/registry.pycomicarr/app/system/service.pycomicarr/config.pyfrontend/src/components/settings/SearchTab.tsxfrontend/src/types/config.tsfrontend/tests/pages/SettingsPage.test.tstests/unit/test_config_version_migrations.pytests/unit/test_system_domain.py
`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
1 participant
Three follow-ups surfaced while documenting Torznab configuration for #668 (docs PR: frankieramirez/comicarr-docs#9). All three were verified against the code before fixing.
1 — Newznab categories never reached the searcher
Field 5 of a Newznab record is
uid#categories: the uid is thei=parameter of the indexer's RSS URL, the rest is the category list. The Settings Categories box wrote straight into that field, mapping commas to#— so7030,7020was stored as7030#7020, which reads as uid7030with7020as the only category.It got worse on the way to runtime.
get_extras()rewrote the field —#to,, then a leading,back to#— andsearch.pydecides whether any category was configured by testing for#:A stored
1#7030arrived as1,7030, failed that test, and fell through to the built-in7030. Every Newznab search used 7030 regardless of configuration, unless the stored value already began with#— the one shape that survived the round trip.rsscheck.pywas getting1,7030as the RSS uid at the same time.Fixed by removing the rewrite so the storage contract is read the same way by
search.py,rsscheck.py, and the providers API, and by giving the uid its own field in Settings so the Categories box means categories:7030,70207030#7020, searched7030(default)1#7030#7020, searches7030,70207030,7020— a claim the search did not honour7030,7020, with RSS user ID1beside it5030— a TV category7030— Books/ComicsExisting indexers are re-read on upgrade. If the box now shows fewer categories than you remember typing, that is the part that was actually reaching your indexer; the rest was being discarded. Editing categories can no longer repoint the RSS feed at a different user — the existing uid is preserved when the client does not send one.
2 — Provider booleans were tolerated at the edge and disagreed on in the middle
_PROVIDER_BOOLEAN_VALUESaccepts eight spellings for the verify TLS and enabled fields. The consumers do not:search.pybool(int(field))—ValueErroron"True"field == "1"rsscheck.pybool(int(field))str(field) == "1"health.py, providers APItrue/yes/onSo an entry stored as
Truewas reported enabled on the Acquisition tab and skipped by the searcher — the health surface claiming a route was ready while nothing was querying it. A non-numeric verify took the search down outright.Both fields are now canonicalised to
"1"/"0"inparse_provider_extras, the single boundary every reader and writer passes through, so edge tolerance cannot become disagreement downstream.This shape was reachable. The legacy-torznab absorption added in #635 passed
self.TORZNAB_VERIFYthrough as a raw Python bool; it serialised to"True"and came back as"True"on the next startup. So an absorbed provider worked once and then broke every subsequent search. It now writes"1"/"0"at the append site too.3 —
TORZNAB_VERIFYdefaulted to FalseThat value exists only to seed the verify field of an absorbed
torznab_*entry. Every other field of such an entry comes from the operator — so aFalsehere silently downgraded TLS verification on a provider nobody chose to downgrade. NowTrue.Also
The absorption's log messages told operators to "configure the provider via the Settings UI (
extra_torznabs)". There is no Torznab editor — Settings → Search edits Newznab only, so anyone following that message found nothing. They now nameextra_torznabsunder[Torznab]inconfig.iniand show the entry format inline.Testing
New tests: Newznab category round-trip through the providers service, uid preservation on edit, uid-alone storage when categories are cleared, Torznab's lack of a uid prefix, boolean canonicalisation across all eight spellings and through serialisation, absorbed-entry verify defaults and reread, unmangled runtime categories, and the new Settings field end to end.
Two existing tests changed and both encoded the bug: one asserted the projection reported
5030,7000for a stored5030#7000— the projection agreeing with the operator's intent rather than with the search — and the other predatesrss_uid.uv run pytest tests/unit— 2414 passednpm run test:run(frontend) — 438 passednpm run typecheck— cleannpm run lint— all lanes green (modern, guards, backend, generated, frontend + prettier)🤖 Generated with Claude Code
https://claude.ai/code/session_01QjqbhfDFmmMeBvn5Pqbk4z
Summary by CodeRabbit
7030, with updated guidance for category configuration.