Skip to content

Comments

fix(enrichment): preserve recs on MusicBrainz HTTP failure#474

Merged
RicherTunes merged 1 commit intomainfrom
fix/topup-flaky-enrichment-drop
Feb 13, 2026
Merged

fix(enrichment): preserve recs on MusicBrainz HTTP failure#474
RicherTunes merged 1 commit intomainfrom
fix/topup-flaky-enrichment-drop

Conversation

@RicherTunes
Copy link
Owner

Summary

  • Production bug: MusicBrainzResolver.EnrichWithMbidsAsync catch block silently dropped recommendations when ResolveMbidAsync threw (HTTP failure to MusicBrainz API). The original item was not added to the result list, causing silent data loss in the recommendation pipeline.
  • Test fix: BrainarrOrchestratorTopUpTests created real resolvers that hit the MusicBrainz API, making results non-deterministic. Now injects pass-through mock IMusicBrainzResolver and IArtistMbidResolver.
  • CLAUDE.md: Updated flaky test entry with corrected root cause.

Test plan

  • TopUp test passes 5/5 runs locally (was flaky before)
  • Full test suite: 2269 passed, 0 failed, 9 skipped
  • CI passes on this branch

🤖 Generated with Claude Code

MusicBrainzResolver.EnrichWithMbidsAsync silently dropped recommendations
when ResolveMbidAsync threw (HTTP failure to MusicBrainz API) — the catch
block logged the error but did not add the original item to the result list.
This caused the TopUp test to be non-deterministic: when MusicBrainz was
unreachable, "Lana Del Rey" disappeared from the pipeline, leaving only 1
result instead of the expected 2.

Production fix: add result.Add(rec) in the catch block (matching the
existing correct behavior in ArtistMbidResolver).

Test fix: inject pass-through mock IMusicBrainzResolver and
IArtistMbidResolver so the test never makes external HTTP calls.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@RicherTunes RicherTunes merged commit 7d6f727 into main Feb 13, 2026
28 checks passed
RicherTunes added a commit that referenced this pull request Feb 13, 2026
All 3 known flaky tests have been resolved:
- E2EHermeticGateTests: [Collection("LoggingTests")] added
- LoggerWarnOnceTests: ClearWarnOnceKeysForTests() in constructor
- TopUpTests: MusicBrainzResolver catch block + mock resolvers (#474)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant