fix(catalog): merge Source Cooperative product records field-by-field so no enrichment is lost (#1738)

* fix(catalog): merge product records field-by-field so no enrichment is lost

mergeProducts preferred whichever duplicate record was richer, but then
replaced the whole record with a spread of both. When the two catalog
sources enrich different fields (the feed adds a description, the
/products/* call adds tags), the whole-object overwrite silently dropped
whichever field the later record left empty.

Merge tags and description per-field, keeping whichever record actually
carries each one, so a merged entry always preserves both enrichments
regardless of the order the sources are merged in.

* fix(catalog): keep featured flag and first-seen fields when merging product records

The field-by-field merge introduced in this PR regressed 'Featured' products:
fetchCatalog merges [featured, recent], and a feed record (featured: false)
landing second clobbered the API record's featured: true for any product in
both lists, dropping the Featured badge and ranking boost. It also let a
thinner feed record overwrite the first-seen title/url/updatedAt.

Per-field merge now: tags/description prefer whichever record has them,
featured is true if either source flags it, and everything else keeps the
first-seen (API) record. Adds regression tests.

* fix(catalog): prefer first-seen description and tags when both sides have values

Fill empty description/tags from the later record, but keep the first-seen
value when both are non-empty so feed text cannot replace the API record.
Tighten the URL regression test and cover the both-non-empty case.

* Address Claude review feedback

- Rewrite the stale `mergeProducts` doc comment: it still described the old
  "prefer the richer record" whole-object overwrite, which no longer matches
  the per-field merge (empty description/tags filled from a later record,
  `featured` OR-ed, everything else first-seen wins).
- Document the fill-only precedence as a deliberate scope limit, noting why it
  is safe for the sole caller (`fetchCatalog` merges `featured` before
  `recent`, and the feed contributes no tags) and flagging it for anyone who
  reuses the function with two sources that can each populate the same field.

* Address CodeRabbit review feedback

- Scope the `mergeProducts` doc comment's "later value is dropped" rule
  explicitly to `description` and `tags`, and note that `featured` is exempt
  (a later `true` always wins via the OR). The previous wording read as if the
  rule applied to every field, contradicting the featured semantics described
  one sentence earlier.
- Fix singular/plural agreement now that the sentence covers two fields.

---------

Co-authored-by: giswqs <giswqs@gmail.com>

Author: Ayush7614

packages/plugins/src/plugins/source-coop-api.ts

@@ -464,22 +464,39 @@ export function filterProducts(products: SourceCoopProduct[], query: string): So
     .map((entry) => entry.product);
 }
 
-/** Merges catalog sources, preferring the richer record for a duplicate id. */
+/**
+ * Merges catalog sources field-by-field for a duplicate id: empty `description`
+ * and `tags` values are filled from a later record, `featured` is true if any
+ * source flags it, and every other field keeps the first-seen value. The fill is
+ * deliberately one-way for `description` and `tags` only — when both sources
+ * carry a non-empty value for either, the later one is dropped rather than
+ * unioned (`featured` is unaffected; a later `true` always wins). That is safe
+ * for the only caller (`fetchCatalog` merges `featured` before `recent`, and the
+ * feed never contributes tags) but is worth revisiting before reusing this with
+ * two sources that can each populate the same field differently.
+ */
 export function mergeProducts(...groups: SourceCoopProduct[][]): SourceCoopProduct[] {
   const byId = new Map<string, SourceCoopProduct>();
   for (const group of groups) {
     for (const product of group) {
       const id = `${product.accountId}/${product.productId}`;
       const existing = byId.get(id);
-      // The feed carries no tags, `/products/*` does; prefer whichever record
-      // actually has them so a merged entry never loses searchable text.
-      if (
-        !existing ||
-        (existing.tags.length === 0 && product.tags.length > 0) ||
-        (!existing.description && product.description)
-      ) {
-        byId.set(id, { ...existing, ...product });
+      if (!existing) {
+        byId.set(id, product);
+        continue;
       }
+      // The feed carries no tags, `/products/*` does; the two sources can
+      // enrich different fields (one adds tags, the other a description). Fill
+      // only empty fields from the later record so neither enrichment is lost,
+      // and keep first-seen values when both sides already have one — matching
+      // title/url/updatedAt. Featured is true if either source flags it, so a
+      // feed record (`featured: false`) never un-flags an API featured product.
+      byId.set(id, {
+        ...existing,
+        description: existing.description || product.description,
+        tags: existing.tags.length > 0 ? existing.tags : product.tags,
+        featured: existing.featured || product.featured,
+      });
     }
   }
   return [...byId.values()];

tests/source-coop-api.test.ts

@@ -499,10 +499,67 @@ describe("mergeProducts", () => {
     assert.equal(merged[0].description, "Full");
   });
 
+  it("keeps both enrichments when the two records add different fields", () => {
+    const apiEntry = product({ tags: ["pmtiles"], description: "" });
+    const feedEntry = product({ tags: [], description: "A full description" });
+    const merged = mergeProducts([apiEntry], [feedEntry]);
+    assert.deepEqual(merged[0].tags, ["pmtiles"]);
+    assert.equal(merged[0].description, "A full description");
+  });
+
+  it("keeps both enrichments regardless of source order", () => {
+    const apiEntry = product({ tags: ["pmtiles"], description: "" });
+    const feedEntry = product({ tags: [], description: "A full description" });
+    const merged = mergeProducts([feedEntry], [apiEntry]);
+    assert.deepEqual(merged[0].tags, ["pmtiles"]);
+    assert.equal(merged[0].description, "A full description");
+  });
+
   it("keeps distinct ids apart", () => {
     const merged = mergeProducts([product({ productId: "a" })], [product({ productId: "b" })]);
     assert.equal(merged.length, 2);
   });
+
+  it("keeps the featured flag when a feed record merges in second", () => {
+    const apiEntry = product({ tags: ["pmtiles"], description: "Full", featured: true });
+    const feedEntry = product({ tags: [], description: "" });
+    const merged = mergeProducts([apiEntry], [feedEntry]);
+    assert.equal(merged.length, 1);
+    assert.equal(merged[0].featured, true);
+    assert.deepEqual(merged[0].tags, ["pmtiles"]);
+    assert.equal(merged[0].description, "Full");
+  });
+
+  it("keeps the first-seen title and url when a thinner record merges in second", () => {
+    const apiEntry = product({ title: "From API", tags: ["pmtiles"] });
+    const feedEntry = product({
+      title: "From feed",
+      tags: [],
+      url: "https://source.coop/acme/feed-buildings",
+    });
+    const merged = mergeProducts([apiEntry], [feedEntry]);
+    assert.equal(merged[0].title, "From API");
+    assert.equal(merged[0].url, "https://source.coop/acme/buildings");
+  });
+
+  it("keeps the first-seen description and tags when both sides are non-empty", () => {
+    const first = product({
+      title: "First",
+      description: "API description",
+      tags: ["pmtiles", "vector"],
+    });
+    const second = product({
+      title: "Second",
+      description: "Feed description",
+      tags: ["geojson"],
+      url: "https://source.coop/acme/other",
+    });
+    const merged = mergeProducts([first], [second]);
+    assert.equal(merged[0].description, "API description");
+    assert.deepEqual(merged[0].tags, ["pmtiles", "vector"]);
+    assert.equal(merged[0].title, "First");
+    assert.equal(merged[0].url, "https://source.coop/acme/buildings");
+  });
 });
 
 describe("fetchProduct", () => {