Skip to content

fix(valuation): deterministic oldest-first canonical pick (chat#1889 row 10) #794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sweetmantech wants to merge 1 commit into
base: main
Choose a base branch
from
+24 −1
Open

fix(valuation): deterministic oldest-first canonical pick (chat#1889 row 10) #794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,20 @@ describe("findCanonicalArtistBySpotifyId", () => {

expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull();
});
// Row 10 (chat#1889): enrichSearchedArtistProfile bumps socials.updated_at
// mid-flow, so a newest-first pick can flip between the creation call and
// the valuation fallback. Oldest-first is stable — and after the row-9
// cleanup there is exactly one candidate anyway.
it("picks the OLDEST social's artist when several match", async () => {
vi.mocked(selectSocials).mockResolvedValue([
{ id: "social-newest", updated_at: "2026-07-29T00:00:00Z" },
{ id: "social-oldest", updated_at: "2025-05-30T00:00:00Z" },
] as never);
vi.mocked(selectAccountSocials).mockImplementation((async (args: { socialId: string }) => {
if (args.socialId === "social-oldest") return [{ account_id: "true-canonical" }];
return [{ account_id: "fresh-duplicate" }];
}) as never);

expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBe("true-canonical");
});
});
9 changes: 8 additions & 1 deletion lib/valuation/findCanonicalArtistBySpotifyId.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,14 @@ export async function findCanonicalArtistBySpotifyId(
const socials = (await selectSocials({ profileUrlContains: spotifyArtistId })) ?? [];
if (socials.length === 0) return null;

for (const social of socials) {
// Oldest first: enrichment bumps updated_at mid-flow, so a newest-first
// pick can flip between two lookups in the same add (chat#1889 row 10).
// The oldest social is the stable, true canonical.
Comment on lines +27 to +29

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify the ordering comment.

The previous implementation was not guaranteed to be newest-first; it used the database-returned order. Describe that order as unspecified rather than “newest-first” to keep the rationale accurate.

🤖 Prompt for 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.

In `@lib/valuation/findCanonicalArtistBySpotifyId.ts` around lines 27 - 29, Update
the ordering comment near findCanonicalArtistBySpotifyId to describe the prior
database-returned order as unspecified, rather than claiming it was
newest-first; retain the rationale that selecting the oldest social record
provides a stable canonical result.

const oldestFirst = [...socials].sort(
(a, b) => new Date(a.updated_at ?? 0).getTime() - new Date(b.updated_at ?? 0).getTime(),

@cubic-dev-ai cubic-dev-ai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Equal updated_at values are not deterministically resolved: this comparator returns 0, preserving the unspecified order returned by Supabase. Adding a stable secondary key (such as id) would prevent the canonical pick from flipping when socials share the same timestamp.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At lib/valuation/findCanonicalArtistBySpotifyId.ts, line 31:

<comment>Equal `updated_at` values are not deterministically resolved: this comparator returns `0`, preserving the unspecified order returned by Supabase. Adding a stable secondary key (such as `id`) would prevent the canonical pick from flipping when socials share the same timestamp.</comment>

<file context>
@@ -24,7 +24,14 @@ export async function findCanonicalArtistBySpotifyId(
+    // pick can flip between two lookups in the same add (chat#1889 row 10).
+    // The oldest social is the stable, true canonical.
+    const oldestFirst = [...socials].sort(
+      (a, b) => new Date(a.updated_at ?? 0).getTime() - new Date(b.updated_at ?? 0).getTime(),
+    );
+
</file context>
Suggested change
(a, b) => new Date(a.updated_at ?? 0).getTime() - new Date(b.updated_at ?? 0).getTime(),
(a, b) =>
new Date(a.updated_at ?? 0).getTime() - new Date(b.updated_at ?? 0).getTime() ||
a.id.localeCompare(b.id),

);
Comment on lines +30 to +32

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file outline =="
ast-grep outline lib/valuation/findCanonicalArtistBySpotifyId.ts --view expanded || true

echo "== file contents =="
cat -n lib/valuation/findCanonicalArtistBySpotifyId.ts

echo "== related test files =="
fd -a '.*canonical.*|.*findCanonical.*|spotify' . | sed 's#^\./##' | head -200

echo "== behavioral Date comparator probe =="
node - <<'JS'
function compare(a,b) {
  return new Date(a.updated_at ?? 0).getTime() - new Date(b.updated_at ?? 0).getTime();
}
const cases = [
  {updated_at: '2024-01-02T00:00:00Z'},
  {updated_at: '2024-01-03T00:00:00Z'},
  {updated_at: 'invalid'},
  {updated_at: '2024-01-05T00:00:00Z'},
];
for (let i = 1; i <= 10; i++) {
  const sorted = [...cases].sort(compare).map(c => c.updated_at);
  console.log(JSON.stringify(sorted));
}
console.log(new Date('invalid').getTime());
console.log(new Date('2024-01-03T00:00:00Z').getTime() - new Date('invalid').getTime());
JS

Repository: recoupable/api

Length of output: 3846

Normalize invalid timestamps before sorting.

new Date(value).getTime() returns NaN for malformed timestamps, and NaN - anything makes Array.sort’s comparator ineffective, so the oldest canonical social may not be selected reliably. Normalizing invalid values to the missing-timestamp fallback keeps canonical selection deterministic and consistent with the intended best-effort behavior.

🤖 Prompt for 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.

In `@lib/valuation/findCanonicalArtistBySpotifyId.ts` around lines 30 - 32, Update
the oldestFirst sorting comparator in findCanonicalArtistBySpotifyId to
normalize malformed updated_at values to the existing missing-timestamp fallback
before subtraction. Ensure both compared timestamps produce finite numeric
values so sorting and oldest canonical social selection remain deterministic.


for (const social of oldestFirst) {
const links = await selectAccountSocials({ socialId: social.id });
const linked = links.find(link => link.account_id);
if (linked?.account_id) return linked.account_id;
Expand Down
Loading