review: scope owner resolution to the caller, project the avatar columns, chunk the id list

Three of cubic's six findings were real:
- P2 (the important one): owner resolution considered EVERY account_catalogs
link for a catalog, including accounts the caller reads nothing through. One
catalog on prod carries four owner links; with a different row order the card
would have named a stranger — wrong attribution, and a disclosure of another
account's name and avatar. resolveCatalogOwners now takes the authorized
owner set and drops links outside it before the organization tie-break.
- P2: selectAccountInfos selected `*`, pulling knowledge entries and AI
instructions to render an avatar. Projected to account_id + image.
- P2: one .in() carrying every catalog id can outgrow PostgREST's URL limit,
and selectPlaycountSnapshots reports that failure as [] — silently costing
every band its real catalog age. Chunked at 50 ids per query.
The aggregate-null finding was already fixed in e0d6f5d (retry + log). The
JSDoc's "one Spotify call" claim is corrected to what the code now does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: sweetmantech

lib/catalog/__tests__/getCatalogsHandler.test.ts

@@ -108,6 +108,7 @@ describe("getCatalogsHandler", () => {
 
 expect(resolveCatalogOwners).toHaveBeenCalledWith({
 catalogIds: ["c1"],
+ownerIds: [accountId,organizationId],
 organizationIds: [organizationId],
 });
 });
@@ -122,7 +123,11 @@ describe("getCatalogsHandler", () => {
 
 expect(body).toEqual({status: "success",catalogs: []});
 expect(getCatalogValuations).toHaveBeenCalledWith([]);
-expect(resolveCatalogOwners).toHaveBeenCalledWith({catalogIds: [],organizationIds: []});
+expect(resolveCatalogOwners).toHaveBeenCalledWith({
+catalogIds: [],
+ownerIds: [accountId],
+organizationIds: [],
+});
 });
 
 it("returns 500 with a generic error, not the raw exception message",async()=>{

lib/catalog/__tests__/resolveCatalogOwners.test.ts

@@ -28,7 +28,11 @@ describe("resolveCatalogOwners", () => {
 beforeEach(()=>vi.clearAllMocks());
 
 it("returns an empty map without querying when there are no catalogs",async()=>{
-constowners=awaitresolveCatalogOwners({catalogIds: [],organizationIds: [org]});
+constowners=awaitresolveCatalogOwners({
+catalogIds: [],
+ownerIds: [person,org],
+organizationIds: [org],
+});
 
 expect(owners.size).toBe(0);
 expect(selectCatalogOwnerLinks).not.toHaveBeenCalled();
@@ -43,7 +47,11 @@ describe("resolveCatalogOwners", () => {
 {account_id: person,image: "https://img/person.png"},
 ]asnever);
 
-constowners=awaitresolveCatalogOwners({catalogIds: [catalogA],organizationIds: [org]});
+constowners=awaitresolveCatalogOwners({
+catalogIds: [catalogA],
+ownerIds: [person,org],
+organizationIds: [org],
+});
 
 expect(owners.get(catalogA)).toEqual({
 id: person,
@@ -60,7 +68,11 @@ describe("resolveCatalogOwners", () => {
 {account_id: org,image: "https://img/org.png"},
 ]asnever);
 
-constowners=awaitresolveCatalogOwners({catalogIds: [catalogA],organizationIds: [org]});
+constowners=awaitresolveCatalogOwners({
+catalogIds: [catalogA],
+ownerIds: [person,org],
+organizationIds: [org],
+});
 
 expect(owners.get(catalogA)?.is_organization).toBe(true);
 });
@@ -73,7 +85,11 @@ describe("resolveCatalogOwners", () => {
 vi.mocked(selectAccounts).mockResolvedValue([{id: org,name: "Duetti"}]asnever);
 vi.mocked(selectAccountInfos).mockResolvedValue([]);
 
-constowners=awaitresolveCatalogOwners({catalogIds: [catalogA],organizationIds: [org]});
+constowners=awaitresolveCatalogOwners({
+catalogIds: [catalogA],
+ownerIds: [person,org],
+organizationIds: [org],
+});
 
 expect(owners.get(catalogA)?.id).toBe(org);
 expect(owners.get(catalogA)?.is_organization).toBe(true);
@@ -84,7 +100,11 @@ describe("resolveCatalogOwners", () => {
 vi.mocked(selectAccounts).mockResolvedValue([{id: org,name: "Recoup"}]asnever);
 vi.mocked(selectAccountInfos).mockResolvedValue([{account_id: org,image: null}]asnever);
 
-constowners=awaitresolveCatalogOwners({catalogIds: [catalogB],organizationIds: [org]});
+constowners=awaitresolveCatalogOwners({
+catalogIds: [catalogB],
+ownerIds: [person,org],
+organizationIds: [org],
+});
 
 expect(owners.get(catalogB)).toEqual({
 id: org,
@@ -99,7 +119,11 @@ describe("resolveCatalogOwners", () => {
 vi.mocked(selectAccounts).mockResolvedValue([]);
 vi.mocked(selectAccountInfos).mockResolvedValue([]);
 
-constowners=awaitresolveCatalogOwners({catalogIds: [catalogA],organizationIds: []});
+constowners=awaitresolveCatalogOwners({
+catalogIds: [catalogA],
+ownerIds: [person],
+organizationIds: [],
+});
 
 expect(owners.get(catalogA)).toEqual({
 id: person,
@@ -108,4 +132,25 @@ describe("resolveCatalogOwners", () => {
 is_organization: false,
 });
 });
+
+it("ignores owner links outside the set the caller reads through",async()=>{
+conststranger="550e8400-e29b-41d4-a716-446655440222";
+vi.mocked(selectCatalogOwnerLinks).mockResolvedValue([
+link(catalogA,stranger),
+link(catalogA,person),
+]);
+vi.mocked(selectAccounts).mockResolvedValue([{id: person,name: "Sweetman.eth"}]asnever);
+vi.mocked(selectAccountInfos).mockResolvedValue([]);
+
+constowners=awaitresolveCatalogOwners({
+catalogIds: [catalogA],
+ownerIds: [person],
+organizationIds: [],
+});
+
+// A catalog can be linked to accounts this caller has nothing to do with —
+// naming one would be wrong attribution and a disclosure of their identity.
+expect(owners.get(catalogA)?.id).toBe(person);
+expect(selectAccounts).toHaveBeenCalledWith([person]);
+});
 });

lib/catalog/getCatalogValuations.ts

@@ -15,6 +15,9 @@ const SPOTIFY_ALBUM_BATCH = 20;
 /** Requests in flight at once — fast enough for a list page, polite to the rate limit. */
 constSPOTIFY_CONCURRENCY=8;
 
+/** Catalog ids per snapshots query, so one `.in()` can't outgrow PostgREST's URL limit. */
+constCATALOG_ID_BATCH=50;
+
 /**
  * Value a set of catalogs in one pass — the list-page counterpart of the
  * single-catalog derivation in `getCatalogMeasurementsHandler`, using the same
@@ -23,11 +26,13 @@ const SPOTIFY_CONCURRENCY = 8;
  *
  * The naive version of this is `getCatalogEarliestReleaseDate` per catalog,
  * which is a snapshot query plus a Spotify round trip each. Instead the album
- * ids for every catalog are read in **one** snapshots query and fetched from
- * Spotify in **one** batched call, then each catalog takes the earliest release
- * date among its own albums. That leaves the per-catalog aggregate RPC — which
- * has no batched form — as the only work that scales with catalog count, and
- * those run concurrently.
+ * ids for every catalog are read in one snapshots query per 50 catalogs and
+ * fetched from Spotify 20 per request, 8 requests at a time, then each catalog
+ * takes the earliest release date among its own albums. That leaves the
+ * per-catalog aggregate RPC — which has no batched form — as the only work that
+ * scales one-for-one with catalog count, and those run concurrently.
+ *
+ * Measured on a 41-catalog account: 730 deduped album ids, ~3.1s warm.
  *
  * Best-effort on the age input, exactly like the single-catalog path: no
  * snapshot, no album ids, or an unavailable Spotify all fall back to the
@@ -98,7 +103,17 @@ async function aggregateWithRetry(catalogId: string) {
 asyncfunctiongetEarliestReleaseDates(catalogIds: string[]): Promise<Map<string,string>>{
 constearliest=newMap<string,string>();
 
-constsnapshots=awaitselectPlaycountSnapshots({catalogs: catalogIds});
+// One .in() with every catalog id would eventually exceed PostgREST's URL
+// limit, and selectPlaycountSnapshots reports that failure as [] — which here
+// would silently mean "no release dates", i.e. every band computed at the
+// default age. Chunked so the request length stays bounded.
+constcatalogChunks: string[][]=[];
+for(leti=0;i<catalogIds.length;i+=CATALOG_ID_BATCH){
+catalogChunks.push(catalogIds.slice(i,i+CATALOG_ID_BATCH));
+}
+constsnapshots=(
+awaitPromise.all(catalogChunks.map(catalogs=>selectPlaycountSnapshots({ catalogs })))
+).flat();
 constalbumIdsByCatalog=newMap<string,string[]>();
 for(constsnapshotofsnapshots){
 // selectPlaycountSnapshots is newest-first, so the first snapshot carrying

lib/catalog/getCatalogsHandler.ts

@@ -44,7 +44,7 @@ export async function getCatalogsHandler(
 constorganizationIds=ownerIds.filter(ownerId=>ownerId!==validated.accountId);
 const[valuations,owners]=awaitPromise.all([
 getCatalogValuations(catalogIds),
-resolveCatalogOwners({ catalogIds, organizationIds }),
+resolveCatalogOwners({ catalogIds,ownerIds,organizationIds }),
 ]);
 
 returnNextResponse.json(

lib/catalog/resolveCatalogOwners.ts

@@ -22,37 +22,48 @@ export type CatalogOwner = {
  * is one of the caller's organizations, the same set `getCatalogOwnerIds`
  * resolves for visibility (chat#1938).
  *
+ * **Only owners the caller was authorized through are eligible.** A catalog can
+ * be linked to accounts that have nothing to do with this caller — one catalog
+ * on prod carries four owner links — and naming a stranger on the card would be
+ * both wrong attribution and a disclosure of another account's name and avatar.
+ * Links outside `ownerIds` are dropped before the organization tie-break.
+ *
  * Three batched reads regardless of how many catalogs are passed.
  *
  * @param params.catalogIds - Catalogs to resolve owners for
- * @param params.organizationIds - The caller's organization ids
+ * @param params.ownerIds - The owner set the caller reads through: their account plus its organizations
+ * @param params.organizationIds - The caller's organization ids, a subset of ownerIds
  * @returns catalog id → owner
  */
 exportasyncfunctionresolveCatalogOwners({
  catalogIds,
+ ownerIds,
  organizationIds,
 }: {
 catalogIds: string[];
+ownerIds: string[];
 organizationIds: string[];
 }): Promise<Map<string,CatalogOwner>>{
 constowners=newMap<string,CatalogOwner>();
 if(!catalogIds.length)returnowners;
 
 constlinks=awaitselectCatalogOwnerLinks(catalogIds);
 constorganizations=newSet(organizationIds);
+constauthorized=newSet(ownerIds);
 
 constownerIdByCatalog=newMap<string,string>();
 for(constlinkoflinks){
+if(!authorized.has(link.account))continue;
 constcurrent=ownerIdByCatalog.get(link.catalog);
 if(!current||(organizations.has(link.account)&&!organizations.has(current))){
 ownerIdByCatalog.set(link.catalog,link.account);
 }
 }
 
-constownerIds=[...newSet(ownerIdByCatalog.values())];
+constresolvedOwnerIds=[...newSet(ownerIdByCatalog.values())];
 const[accounts,infos]=awaitPromise.all([
-selectAccounts(ownerIds),
-selectAccountInfos(ownerIds),
+selectAccounts(resolvedOwnerIds),
+selectAccountInfos(resolvedOwnerIds),
 ]);
 constnameById=newMap(accounts.map(account=>[account.id,account.name??null]));
 constimageById=newMap(infos.map(info=>[info.account_id,info.image??null]));

lib/supabase/account_info/selectAccountInfos.ts

@@ -1,20 +1,27 @@
 importsupabasefrom"../serverClient";
 importtype{Tables}from"@/types/database.types";
 
+/** Only what an avatar needs — `account_info` also carries knowledge blobs and instructions. */
+exporttypeAccountInfoAvatar=Pick<Tables<"account_info">,"account_id"|"image">;
+
 /**
  * Retrieves `account_info` rows for several accounts at once — the plural
  * sibling of `selectAccountInfo`, which reads one account and is the wrong
  * shape for a list response that would otherwise issue a query per row.
  *
+ * Projects `account_id` and `image` only: `account_info` also holds knowledge
+ * entries and AI instructions, which a list of avatars has no use for and would
+ * pay for in payload and deserialization.
+ *
  * @param accountIds - The account IDs to read info for
  * @returns The matching rows, or [] when none were asked for or the query fails
  */
-exportasyncfunctionselectAccountInfos(accountIds: string[]): Promise<Tables<"account_info">[]>{
+exportasyncfunctionselectAccountInfos(accountIds: string[]): Promise<AccountInfoAvatar[]>{
 if(!accountIds.length)return[];
 
 const{ data, error }=awaitsupabase
 .from("account_info")
-.select("*")
+.select("account_id, image")
 .in("account_id",accountIds);
 
 if(error){