mirror of
https://github.com/SoPat712/allstarr.git
synced 2026-10-06 13:55:39 -04:00
fix(matching): prefer qualified local candidates
This commit is contained in:
4 files changed
+138
-13
No files matched your search
@@ -636,6 +636,94 @@ public sealed class TrackMatchDecisionEngineTests(ITestOutputHelper output)
|
||||
Assert.Equal(score.Components["preferenceScore"], decision.Confidence);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void AcceptanceQualifiedLocalCandidateWinsOverHigherProviderRoute()
|
||||
{
|
||||
var scope = Scope();
|
||||
var source = Source() with { Isrc = "USAAA2600001" };
|
||||
var local = Candidate(scope) with
|
||||
{
|
||||
CanonicalRecordingId = null,
|
||||
Album = null,
|
||||
AlbumArtist = null,
|
||||
DurationMilliseconds = 230_000,
|
||||
IsLocal = true
|
||||
};
|
||||
var provider = Candidate(scope) with
|
||||
{
|
||||
LibraryTrackId = Guid.CreateVersion7(),
|
||||
BackendItemId = "provider-route",
|
||||
CanonicalRecordingId = null,
|
||||
Isrc = source.Isrc,
|
||||
IsLocal = false,
|
||||
ProviderOrigin = ProviderOrigin.BuiltIn
|
||||
};
|
||||
var engine = new TrackMatchDecisionEngine();
|
||||
|
||||
var scores = engine.ScoreCandidates(source, [provider, local]);
|
||||
var localScore = scores.Single(score => score.LibraryTrackId == local.LibraryTrackId);
|
||||
var providerScore = scores.Single(score => score.LibraryTrackId == provider.LibraryTrackId);
|
||||
var decision = engine.Decide(scope, source, [provider, local]);
|
||||
|
||||
Assert.True(providerScore.Confidence > localScore.Components!["preferenceScore"]);
|
||||
Assert.True(localScore.Components["preferenceScore"] >= decision.AcceptThreshold);
|
||||
Assert.Equal(TrackMatchReviewState.Accepted, decision.State);
|
||||
Assert.Equal(local.LibraryTrackId, decision.SelectedLibraryTrackId);
|
||||
Assert.Equal(local.LibraryTrackId, decision.Candidates[0].LibraryTrackId);
|
||||
Assert.Equal(localScore.Components["preferenceScore"], decision.Confidence);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void AcceptanceQualifiedLocalCandidateIsRetainedAheadOfTwentyProviderRoutes()
|
||||
{
|
||||
var scope = Scope();
|
||||
var source = Source() with { Isrc = "USAAA2600002" };
|
||||
var local = Candidate(scope) with
|
||||
{
|
||||
CanonicalRecordingId = null,
|
||||
Album = null,
|
||||
AlbumArtist = null,
|
||||
DurationMilliseconds = 230_000
|
||||
};
|
||||
var providers = Enumerable.Range(0, 20).Select(index => Candidate(scope) with
|
||||
{
|
||||
LibraryTrackId = Guid.CreateVersion7(),
|
||||
BackendItemId = $"provider-{index}",
|
||||
CanonicalRecordingId = null,
|
||||
Isrc = source.Isrc,
|
||||
IsLocal = false,
|
||||
ProviderOrigin = ProviderOrigin.BuiltIn
|
||||
});
|
||||
|
||||
var decision = new TrackMatchDecisionEngine().Decide(
|
||||
scope, source, [.. providers, local]);
|
||||
|
||||
Assert.Equal(TrackMatchReviewState.Accepted, decision.State);
|
||||
Assert.Equal(local.LibraryTrackId, decision.SelectedLibraryTrackId);
|
||||
Assert.Equal(local.LibraryTrackId, decision.Candidates[0].LibraryTrackId);
|
||||
Assert.Equal(20, decision.Candidates.Count);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void MultipleAcceptanceQualifiedLocalRecordingsRemainAmbiguous()
|
||||
{
|
||||
var scope = Scope();
|
||||
var first = Candidate(scope);
|
||||
var second = first with
|
||||
{
|
||||
LibraryTrackId = Guid.CreateVersion7(),
|
||||
BackendItemId = "local-2",
|
||||
CanonicalRecordingId = Guid.CreateVersion7(),
|
||||
DurationMilliseconds = 240_000
|
||||
};
|
||||
|
||||
var decision = new TrackMatchDecisionEngine().Decide(scope, Source(), [first, second]);
|
||||
|
||||
Assert.Equal(TrackMatchReviewState.Ambiguous, decision.State);
|
||||
Assert.Null(decision.SelectedLibraryTrackId);
|
||||
Assert.Contains("ambiguous_top_candidates", decision.Warnings);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ExtensionPreferencePenaltyFavorsAnOtherwiseEqualBuiltInCandidate()
|
||||
{
|
||||
|
||||
@@ -138,7 +138,7 @@ public sealed class TrackMatchPolicy
|
||||
|
||||
public sealed class TrackMatchDecisionEngine
|
||||
{
|
||||
public const string AlgorithmVersion = "normalized-v13";
|
||||
public const string AlgorithmVersion = "normalized-v14";
|
||||
|
||||
private readonly TrackMatchPolicy _policy;
|
||||
|
||||
@@ -277,10 +277,8 @@ public sealed class TrackMatchDecisionEngine
|
||||
scope);
|
||||
}
|
||||
|
||||
var scores = ScoreCandidates(source, visible)
|
||||
.Take(20)
|
||||
.ToList();
|
||||
if (scores.Count == 0)
|
||||
var rankedScores = ScoreCandidates(source, visible);
|
||||
if (rankedScores.Count == 0)
|
||||
{
|
||||
return Result(
|
||||
scopedCandidates.Count > 0 && manualOverride?.RejectedLibraryTrackIds?.Count > 0
|
||||
@@ -294,9 +292,27 @@ public sealed class TrackMatchDecisionEngine
|
||||
scope);
|
||||
}
|
||||
|
||||
// A backend-local candidate that independently clears the automatic
|
||||
// acceptance bar is the useful library answer. Provider routes remain
|
||||
// fallbacks and must not displace it merely by scoring a few points
|
||||
// higher.
|
||||
var acceptedLocal = rankedScores.FirstOrDefault(score =>
|
||||
score.IsLocal &&
|
||||
PreferenceScore(score) >= _policy.AcceptThreshold &&
|
||||
HasStrongArtistEvidence(score));
|
||||
var scores = (acceptedLocal == null
|
||||
? rankedScores
|
||||
: [acceptedLocal, .. rankedScores.Where(score =>
|
||||
score.LibraryTrackId != acceptedLocal.LibraryTrackId)])
|
||||
.Take(20)
|
||||
.ToList();
|
||||
var best = scores[0];
|
||||
var selected = visible.Single(candidate => candidate.LibraryTrackId == best.LibraryTrackId);
|
||||
var runnerUp = scores.Skip(1).FirstOrDefault(score =>
|
||||
var competingScores = acceptedLocal == null
|
||||
? rankedScores
|
||||
: rankedScores.Where(score => score.IsLocal);
|
||||
var runnerUp = competingScores.FirstOrDefault(score =>
|
||||
score.LibraryTrackId != best.LibraryTrackId &&
|
||||
!SameRecordingIdentity(
|
||||
selected,
|
||||
visible.Single(candidate => candidate.LibraryTrackId == score.LibraryTrackId)));
|
||||
@@ -322,9 +338,7 @@ public sealed class TrackMatchDecisionEngine
|
||||
}
|
||||
|
||||
var decisionScore = PreferenceScore(best);
|
||||
var strongArtistEvidence = best.Components == null ||
|
||||
!best.Components.TryGetValue("artist", out var artistScore) ||
|
||||
artistScore >= 0.7;
|
||||
var strongArtistEvidence = HasStrongArtistEvidence(best);
|
||||
var state = decisionScore >= _policy.AcceptThreshold && strongArtistEvidence
|
||||
? TrackMatchReviewState.Accepted
|
||||
: decisionScore >= _policy.SuggestThreshold && strongArtistEvidence
|
||||
@@ -349,6 +363,11 @@ public sealed class TrackMatchDecisionEngine
|
||||
scope);
|
||||
}
|
||||
|
||||
private static bool HasStrongArtistEvidence(TrackMatchCandidateScore score) =>
|
||||
score.Components == null ||
|
||||
!score.Components.TryGetValue("artist", out var artistScore) ||
|
||||
artistScore >= 0.7;
|
||||
|
||||
private TrackMatchCandidateScore ScoreCandidate(
|
||||
ExternalTrackMatchSnapshot source,
|
||||
LocalTrackMatchCandidate candidate)
|
||||
|
||||
@@ -84,6 +84,7 @@
|
||||
let matchOpen = $state(false);
|
||||
let selectedMatch = $state<MatchReviewItem | null>(null);
|
||||
let matchLoading = $state("");
|
||||
let trackMenuOpen = $state<number | null>(null);
|
||||
const trackColumnOptions = [
|
||||
{ id: "position", label: "Playlist number" },
|
||||
{ id: "artist", label: "Artist" },
|
||||
@@ -857,8 +858,11 @@
|
||||
<td class="track-duration">{formatDuration(track.durationMs)}</td>
|
||||
{/if}
|
||||
<td class="track-menu">
|
||||
<Popover.Root>
|
||||
<Popover.Trigger class="track-menu-trigger" aria-label={`Technical details for ${track.title}`}><MoreHorizontal size={18} aria-hidden="true" /></Popover.Trigger>
|
||||
<Popover.Root
|
||||
open={trackMenuOpen === track.sourcePosition}
|
||||
onOpenChange={(open) => trackMenuOpen = open ? track.sourcePosition : null}
|
||||
>
|
||||
<Popover.Trigger id={`track-details-${track.sourcePosition}`} class="track-menu-trigger" aria-label={`Technical details for ${track.title}`}><MoreHorizontal size={18} aria-hidden="true" /></Popover.Trigger>
|
||||
<Popover.Portal>
|
||||
<Popover.Content class="bits-menu track-details-menu" sideOffset={4} align="end">
|
||||
<div class="track-technical">
|
||||
@@ -872,7 +876,14 @@
|
||||
{#if track.externalSnapshotId}
|
||||
<Button
|
||||
variant="secondary"
|
||||
onclick={(event) => void openTrackMatch(track.externalSnapshotId, event.currentTarget)}
|
||||
size="sm"
|
||||
onclick={() => {
|
||||
trackMenuOpen = null;
|
||||
void openTrackMatch(
|
||||
track.externalSnapshotId,
|
||||
document.getElementById(`track-details-${track.sourcePosition}`) ?? undefined,
|
||||
);
|
||||
}}
|
||||
>Review match</Button>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
@@ -3018,12 +3018,19 @@ test("Playlist details use a responsive dialog and track rows open mapping revie
|
||||
await dialog.getByRole("button", { name: "Technical details for Test song" }).click();
|
||||
await expect(page).toHaveURL(/#\/library\/playlists$/);
|
||||
const trackDetails = page.locator(".track-details-menu");
|
||||
await expect(trackDetails.getByRole("button", { name: "Review match" })).toBeVisible();
|
||||
const reviewMatch = trackDetails.getByRole("button", { name: "Review match" });
|
||||
await expect(reviewMatch).toBeVisible();
|
||||
await expect.poll(() => trackDetails.evaluate((panel) => {
|
||||
const bounds = panel.getBoundingClientRect();
|
||||
return panel.contains(document.elementFromPoint(bounds.left + bounds.width / 2, bounds.top + 8));
|
||||
})).toBe(true);
|
||||
await reviewMatch.click();
|
||||
await expect(trackDetails).toBeHidden();
|
||||
const reviewedMatch = page.getByRole("dialog", { name: "Test song" });
|
||||
await expect(reviewedMatch).toBeVisible();
|
||||
await page.keyboard.press("Escape");
|
||||
await expect(reviewedMatch).toBeHidden();
|
||||
await expect(dialog.getByRole("button", { name: "Technical details for Test song" })).toBeFocused();
|
||||
await dialog.getByRole("button", { name: "Open mapping details for Test song" }).focus();
|
||||
await page.keyboard.press("Enter");
|
||||
await expect(page).toHaveURL(/#\/library\/playlists$/);
|
||||
|
||||
Reference in new issue
Block a user