Fix M4B re-grab loop with profile-driven quality matching

[timk75]

Summary

Fixes the M4B re-grab loop: a monitored audiobook whose file is already at/above cutoff is searched and re-downloaded on every cycle. Replaces #581 with the profile-driven matching approach we agreed on.

Root cause. The cutoff check classified an on-disk file into an ad-hoc label — the container "M4B", or a bare "256kbps" — and looked that string up in the profile's quality rungs. Real rungs are named by codec + bitrate ("AAC 256kbps"), so the label never matched, cutoffMet stayed false, and the audiobook was re-grabbed forever. The same string-label logic existed in three places with three different vocabularies: AudiobookStatusEvaluator (status display), AutomaticSearchService (the actual re-grab driver), and a near-duplicate in LibraryController.

Changes

Added

• QualityMatcher (listenarr.domain/Common): a pure, dependency-free matcher. Given a file's codec/bitrate/lossless facts and a profile, it walks profile.Qualities using the structured Codec / Bitrate / IsLossless fields — falling back to parsing the rung label for seed/legacy profiles that only set Quality + Priority — and: round-down (picks the highest rung the file meets or exceeds); returns a defined CodecMismatch when the file's codec isn't in the profile (instead of the old int.MaxValue silent fallback); matches OGG Vorbis by codec, maps AAC-in-M4B/M4A (incl. the legacy M4B codec group), and normalizes bits/sec → kbps in one place.
• 26 new unit tests (QualityMatcherTests), incl. the exact M4B-loop regression; QualityProfileBuilder extended.

Changed

• The status evaluator, automatic search, and library controller now share a single QualityCutoffEvaluator that calls the matcher.
• Inverted priority comparison corrected. AutomaticSearchService's cutoff check (Priority >= cutoff) and IsQualityBetter (cand.Priority > exist.Priority) compared rungs the wrong way round given the model's "lower number = higher quality" (matching the seed profiles, FE, and status evaluator). They now use the canonical direction.

Fixed

• The M4B re-grab loop (see Summary).
• Path-only / metadata-less files are no longer dropped before matching. AudiobookStatusEvaluator now forwards Path into AudioQualityInput (as QualityCutoffEvaluator does), and its candidate-format pre-filter falls back to the path extension when Format/Container are empty — so e.g. a tagless book.m4b with PreferredFormats = ["m4b"] reaches the matcher instead of returning quality-mismatch.

Removed

• ~380 lines of duplicated, drifting string-label detection logic across the three call sites.
• The frontend's client-side status recomputation in audiobookStatus.ts; the FE now trusts the server-computed status (overriding only for the transient "download in flight" case).

Testing

• Full backend suite green (711 tests), including the M4B-loop regression and the path-only fallback cases (FLAC-by-extension and a second path-only m4b case proving the fallback is format-agnostic).
• Frontend: 364 tests + type-check + eslint green.

Notes

Behaviour changes to confirm:

1. Inverted priority comparison corrected — this changes grab/skip decisions: a result is grabbed only when genuinely better than the existing file, and cutoff is met only when the file is at least cutoff quality.
2. Codec absent from profile ⇒ cutoff not met ⇒ such files keep being searched (the logical inverse of the old accidental "always at max priority" behaviour).
3. Round-down floor: a file below the lowest rung (or unknown bitrate with no VBR rung) maps to the worst rung for its codec — conservative, never over-claims.

Out of scope (deliberate): the PreferredFormats → PreferredContainers split + EF migration + frontend is tracked separately in #647. DownloadImportService.IsQualityBetter is left untouched — it's a different, profile-independent "acceptable downgrade?" gate, not the inverted-priority bug.

Closes #581.
Follow-up: #647.

[timk75]

Note for maintainers: I can't self-apply a version label (fork contributor). This is a backward-compatible bug fix, so it should take the patch label for the Validate PR version label check. Happy to adjust if you'd classify it differently.

[therobbiedavis]

One more thing I noticed while reviewing the status behavior: the backend now fixes the M4B/AAC cutoff path, but the frontend still has a separate client-side status recomputation in fe/src/utils/audiobookStatus.ts that can disagree with it.

For a detailed audiobook payload with format=m4b, codec=aac, bitrate=256000, and a profile cutoff/rung of AAC 256kbps, the backend AudiobookStatusEvaluator resolves this as quality-match, but the frontend utility derives the bare label 256kbps and then fails to find that in structured rungs like AAC 256kbps, so it returns quality-mismatch.

Since that file is outside this PR’s diff, this may be follow-up work, but otherwise users can still see “Below Cutoff” in richer library views even though the automatic search/backend cutoff logic is fixed.

[timk75]

Both review points addressed in 3496cd3:

1. Backend lossless detection — replied on the inline thread; IsLosslessFile now derives lossless-ness from the mapped codec groups (path-extension aware).

2. Frontend status recomputation (fe/src/utils/audiobookStatus.ts) — the client-side utility now mirrors the backend QualityMatcher (codec-group mapping + bitrate round-down, structured-rung aware) instead of comparing a bare derived label by string equality. An m4b/aac/256kbps file now resolves to quality-match against an AAC 256kbps rung, matching the backend evaluator, so richer library views no longer show "Below Cutoff" while the search/cutoff logic considers it satisfied. I kept the port deliberately parallel to the C# matcher (with a comment to that effect) so the two stay in lockstep.

Tests added on both sides; backend (dotnet test, 27/27 QualityMatcher) and frontend (vitest, 8/8) pass, plus vue-tsc and ESLint clean.

@therobbiedavis would you mind taking another look when you get a chance?

[timk75]

Heads-up on the force-push: it changed the commit message only — my local tooling had appended a stray trailer to the message, so I amended it out and re-pushed. No code changes; the tree is identical to the previous push (3496cd3b → 555024c3). Sorry for the noise on your notifications.

[therobbiedavis]

Getting much closer! Outside of the inline comment I left on audiobookStatus.ts, there's another single-source-of-truth concern on the backend status path:

QualityCutoffEvaluator now passes Path into QualityMatcher, but AudiobookStatusEvaluator has its own projection/filtering logic and drops AudiobookFormatSummary.Path. That lets automatic search and library status disagree for path-only files. Can we reuse the shared quality matching/projection logic here, or at least pass Path = file.Path, so status and cutoff evaluation stay aligned?

Sorry for the back and forth. I just had a brain blast for a cleaner method to handle this stuff this review round. Ideally the proper way to do this is we shouldn't be duplicating any logic and making sure the appropriate files have the authority and responsibility over the business logic, then we just import where necessary.

[timk75]

Pushed 86d1280 addressing both points from this round.

Backend AudiobookStatusEvaluator dropping Path — good catch. It now forwards Path = file.Path into AudioQualityInput, exactly the way QualityCutoffEvaluator.ToInput does, so automatic-search and library status agree for path-only files (metadata disabled / ffprobe missing / extraction failed). Added a regression test: ComputeStatus_ReturnsQualityMatch_ForPathOnlyFile_WhenProbeMetadataMissing (a metadata-less book.flac now resolves to quality-match).

Frontend — replied inline; the client-side matcher is gone and the FE now trusts the server-computed status.

Both AudiobookFormatSummary and AudiobookFile are still distinct carriers projecting into the shared AudioQualityInput (as the record's own doc-comment intends), so the projection itself stays per-carrier — but both now carry Path, and the actual matching/cutoff logic lives only in QualityMatcher.

Backend 710 tests green, frontend 364 tests + type-check + eslint green.

[therobbiedavis]

This is closer, but AudiobookStatusEvaluator can still reject path-only files before Path reaches QualityMatcher. The candidate filter only checks Format/Container, so a metadata-less book.flac with PreferredFormats = ["flac"] gets filtered out and returns quality-mismatch before the matcher can use the extension. The new regression test uses empty PreferredFormats, so it does not cover that path. Can we include the path extension in this preferred-format check too?

[timk75]

Fixed in 281f7be — good catch, you were right the filter rejected path-only files before Path reached the matcher.

Fix: AudiobookStatusEvaluator's candidate-format pre-filter now falls back to the path extension when Format/Container are empty (new ExtensionFromPath helper), mirroring what QualityMatcher.MapCodec already does. So a metadata-less book.flac with PreferredFormats = ["flac"] matches by extension instead of being dropped.

Tests: the regression test now uses PreferredFormats = ["flac"] (covering the filter path your previous note flagged), plus a second-format case — a path-only book.m4b with PreferredFormats = ["m4b"] — to prove the fallback is format-agnostic, not FLAC-specific. Full suite green (711).

Proactively audited the rest of the codebase for this same bug class (path-only / metadata-less files mishandled because code reads Codec/Container/Format but ignores the path extension), so we're not back here next round:

• Both AudioQualityInput construction sites set Path (QualityCutoffEvaluator.ToInput, AudiobookStatusEvaluator).
• All file-based cutoff/status/upgrade decisions converge on QualityMatcher via AudioQualityInput; EfAudiobookFileRepository.GetFormatSummariesAsync preserves Path into its (Format, Codec, Container) grouping, so the evaluator still receives it.
• The remaining "M4B"/"FLAC" string classifiers (SearchService, the *SearchProviders) operate on indexer release strings, not on-disk files — not this bug class.
• Frontend trusts server-computed status; no client-side codec/extension derivation remains.
• One non-bug note: DownloadImportService.DetermineQualityFromMetadata is a separate pre-Fix M4B re-grab loop with profile-driven quality matching #648 string classifier used only for import-dedup; it already has a path-extension fallback and fails open (allows import), so no file is mishandled. Could optionally be routed through QualityMatcher.IsLabelBetter later for consistency, but it's not a correctness issue.

Net: no other instances of the path-only bug remain.

[therobbiedavis]

LGTM. A couple small updates: The changelog is no longer needed. Please update your PR description to match the template.

We have some other PR's ahead in the queue to get merged so we may ask you to rebase before we merge.

[timk75]

Thanks! Both small updates done:

• CHANGELOG entry removed (42d120a9).
• PR description rewritten to match the template (Summary / Changes / Testing / Notes).

I've left the branch as-is rather than rebasing — just say the word once the PRs ahead are merged and I'll rebase onto canary right away.