fix(data-access): reject sparse type lists, name rejected values, clear unregistered entity types (LLMO-7445)

Author: Tanase Butcaru

docs/adr/0002-embedding-client-and-semantic-index-utils.md

@@ -40,7 +40,7 @@ Following ADR-0001's test (*true for every consumer → shared; specific to one
 
 **Consumers own:**
 - Caching. The durable query-embedding cache (`semantic_query_embedding`) and its key policy (`hash(normalizedText)+model+dims`) live in the data-access helpers/consumer, not the client — staleness/keying is a consumer concern.
-- Model/dimension values come from `SEMANTIC_MATCHING_CONFIG` (`embeddingModel`, `embeddingDims`) in `data-access`, which owns the stored index; consumers import it and pass it as `model` / `dims`. It is a code constant, not env config: the label is persisted on every row and in the cache key, so writer and reader must never disagree, and changing it is a re-embed. `embeddingDims` must match the `vector(N)` columns. The helpers still accept any `model` / `dims`, so a same-dims model change can run the online re-embed below.
+- Passing the model/dimension on every write and search. They pass the shared `SEMANTIC_MATCHING_CONFIG` (`embeddingModel`, `embeddingDims`), which `data-access` defines because it owns the stored index, and do not hardcode the values. It is a code constant, not env config: the label is persisted on every row and in the cache key, so writer and reader agree as long as they run the same data-access version, and changing it is a re-embed plus upgrading both. `embeddingDims` must match the `vector(N)` columns. The helpers still accept any `model` / `dims`, so a same-dims model change can run the online re-embed below.
 - Translating a client failure into an HTTP response (api-service returns 503; the audit-worker write path degrades best-effort).
 
 ### 4. Semantic-index helpers live in `data-access`, mirroring the URL index
@@ -49,6 +49,8 @@ Following ADR-0001's test (*true for every consumer → shared; specific to one
 
 **Type registries.** `OPPORTUNITY_SEMANTIC_SOURCE_TYPES` (kinds of embedded text: `topic`, later `claim`) and `OPPORTUNITY_SEMANTIC_ENTITY_TYPES` (opportunity types, from `OPPORTUNITY_TYPES`) are exported from this layer and are the only values the writer stores and the reader accepts; anything else is a `ValidationError`. The reader takes `sourceTypes` (required, non-empty) and optional `entityTypes` (omitted or empty = all), deduped, so a list can never exceed its registry. The DB does not duplicate the registries: the RPC only rejects an empty source-type list and NULL elements. A team that wants its opportunities searchable adds its source kind and/or opportunity type to the registries, indexes with them, and callers opt in by passing them.
 
+Adding an entity type: (1) if the opportunity type is new, add it to `OPPORTUNITY_TYPES` and release `spacecat-shared-utils`; (2) add it to `OPPORTUNITY_SEMANTIC_ENTITY_TYPES`, bump data-access's utils pin, and release data-access; (3) bump data-access in the writer (audit-worker) and the reader (api-service). The writer rejects the type until step 3, so index only after it. A new source kind is the same minus step 1.
+
 ## Consequences
 
 - `gpt-client` gains an embeddings surface and its own `AZURE_EMBEDDING_*` config (documented in its README); the intended cost, not a drawback.

packages/spacecat-shared-data-access/README.md

@@ -180,7 +180,7 @@ const matches = await lookupOpportunitiesByVectors(postgrestClient, {
 
 `OPPORTUNITY_SEMANTIC_SOURCE_TYPES` (kinds of embedded text) and `OPPORTUNITY_SEMANTIC_ENTITY_TYPES` (opportunity types) are the registries for `source_type` and `entity_type`: the writer and reader reject any value outside them. To index a new kind or opportunity type, add it to the registry first.
 
-`SEMANTIC_MATCHING_CONFIG` (`embeddingModel`, `embeddingDims`) is the embedding generation the writer and reader both use; pass it as `model` / `dims` rather than hardcoding the values. It is a code constant, not env config, so the two sides can never disagree.
+`SEMANTIC_MATCHING_CONFIG` (`embeddingModel`, `embeddingDims`) is the embedding generation the writer and reader both use; pass it as `model` / `dims` rather than hardcoding the values. It is a code constant, not env config, so writer and reader agree as long as both run the same data-access version; changing it means a re-embed plus upgrading both consumers.
 
 The batched helpers group their calls internally (search in groups of up to 20 vectors, cache reads/touches in groups of 50 hashes), so callers pass whole lists rather than looping per item.
 

packages/spacecat-shared-data-access/src/util/semantic-index.utils.js

@@ -99,7 +99,7 @@ function assertModel(value) {
 function assertAllowed(value, name, registry) {
   const allowed = Object.values(registry);
   if (!allowed.includes(value)) {
-    throw new ValidationError(`${name} must be one of: ${allowed.join(', ')}`);
+    throw new ValidationError(`${name} must be one of: ${allowed.join(', ')} (got ${JSON.stringify(value)})`);
   }
 }
 
@@ -118,8 +118,15 @@ function toAllowedList(values, name, registry, { required }) {
     throw new ValidationError(`${name} must be an array`);
   }
   const allowed = Object.values(registry);
-  if (!values.every((value) => allowed.includes(value))) {
-    throw new ValidationError(`${name} must only contain: ${allowed.join(', ')}`);
+
+  const rejected = [];
+  for (const value of values) {
+    if (!allowed.includes(value)) {
+      rejected.push(value);
+    }
+  }
+  if (rejected.length > 0) {
+    throw new ValidationError(`${name} must only contain: ${allowed.join(', ')} (got ${JSON.stringify(rejected)})`);
   }
   return [...new Set(values)];
 }
@@ -339,7 +346,6 @@ export async function syncOpportunitySemantic(postgrestClient, {
   assertClient(postgrestClient);
   assertId(siteId, 'siteId');
   assertId(entityId, 'entityId');
-  assertAllowed(entityType, 'entityType', OPPORTUNITY_SEMANTIC_ENTITY_TYPES);
   assertAllowed(sourceType, 'sourceType', OPPORTUNITY_SEMANTIC_SOURCE_TYPES);
 
   const table = 'opportunity_semantic_embedding';
@@ -353,6 +359,8 @@ export async function syncOpportunitySemantic(postgrestClient, {
     return 0;
   }
 
+  // Only stored rows carry entityType; clearing must still work for a type since removed.
+  assertAllowed(entityType, 'entityType', OPPORTUNITY_SEMANTIC_ENTITY_TYPES);
   await upsertRows(postgrestClient, table, rows, 'entity_id,source_type,source_hash', entityId);
 
   const keep = new Set(rows.map((r) => r.source_hash));
@@ -456,15 +464,19 @@ export async function lookupOpportunitiesByVectors(postgrestClient, {
       p_min_score: minScore,
     });
     if (error) {
-      throw new DataAccessError(`Failed semantic search for site ${siteId}`, { siteId, sourceTypes: sourceTypeList }, error);
+      throw new DataAccessError(
+        `Failed semantic search for site ${siteId}`,
+        { siteId, sourceTypes: sourceTypeList, entityTypes: entityTypeList },
+        error,
+      );
     }
     for (const row of data ?? []) {
       // Checked against this group, not all results, so a bad index can't land in another group.
       const { query_index: queryIndex } = row;
       if (!Number.isInteger(queryIndex) || queryIndex < 0 || queryIndex >= group.length) {
         throw new DataAccessError(
           `Unexpected query_index ${queryIndex} from ${SEMANTIC_SEARCH_RPC}`,
-          { siteId, sourceTypes: sourceTypeList },
+          { siteId, sourceTypes: sourceTypeList, entityTypes: entityTypeList },
         );
       }
       results[offset + queryIndex].push({

packages/spacecat-shared-data-access/test/unit/util/semantic-index.utils.test.js

@@ -219,21 +219,31 @@ describe('semantic-index.utils', () => {
         siteId: SITE_ID, entityType: ENTITY_TYPE, sourceType: SOURCE_TYPE,
       })).to.be.rejectedWith(ValidationError, 'entityId is required');
       await expect(syncOpportunitySemantic(client, {
-        siteId: SITE_ID, entityId: ENTITY_ID, sourceType: SOURCE_TYPE,
-      })).to.be.rejectedWith(ValidationError, 'entityType must be one of: cited-analysis, reddit-analysis, youtube-analysis');
+        siteId: SITE_ID, entityId: ENTITY_ID, sourceType: SOURCE_TYPE, sources: [src('x')],
+      })).to.be.rejectedWith(ValidationError, 'entityType must be one of: cited-analysis, reddit-analysis, youtube-analysis (got undefined)');
       await expect(syncOpportunitySemantic(client, {
-        siteId: SITE_ID, entityId: ENTITY_ID, entityType: 'broken-backlinks', sourceType: SOURCE_TYPE,
-      })).to.be.rejectedWith(ValidationError, 'entityType must be one of');
+        siteId: SITE_ID, entityId: ENTITY_ID, entityType: 'wikipedia-analysis', sourceType: SOURCE_TYPE, sources: [src('x')],
+      })).to.be.rejectedWith(ValidationError, '(got "wikipedia-analysis")');
       await expect(syncOpportunitySemantic(client, {
         siteId: SITE_ID, entityId: ENTITY_ID, entityType: ENTITY_TYPE,
-      })).to.be.rejectedWith(ValidationError, 'sourceType must be one of: topic');
+      })).to.be.rejectedWith(ValidationError, 'sourceType must be one of: topic (got undefined)');
       await expect(syncOpportunitySemantic(client, {
         siteId: SITE_ID, entityId: ENTITY_ID, entityType: ENTITY_TYPE, sourceType: 'claim',
-      })).to.be.rejectedWith(ValidationError, 'sourceType must be one of');
+      })).to.be.rejectedWith(ValidationError, '(got "claim")');
       expect(client.calls.upsert).to.have.length(0);
       expect(client.calls.delete).to.have.length(0);
     });
 
+    it('clears an entity whose entityType is no longer registered', async () => {
+      const client = makeClient();
+      const count = await syncOpportunitySemantic(client, {
+        siteId: SITE_ID, entityId: ENTITY_ID, entityType: 'wikipedia-analysis', sourceType: SOURCE_TYPE, sources: [],
+      });
+      expect(count).to.equal(0);
+      expect(client.calls.delete).to.have.length(1);
+      expect(client.calls.upsert).to.have.length(0);
+    });
+
     it('clears the (entity, sourceType) slice when sources is empty', async () => {
       const client = makeClient();
       const count = await syncOpportunitySemantic(client, {
@@ -527,7 +537,11 @@ describe('semantic-index.utils', () => {
       await expect(call({ sourceTypes: ['topic', 'claim'] })).to.be.rejectedWith(ValidationError, 'sourceTypes must only contain: topic');
       await expect(call({ sourceTypes: ['*'] })).to.be.rejectedWith(ValidationError, 'sourceTypes must only contain');
       await expect(call({ entityTypes: 'cited-analysis' })).to.be.rejectedWith(ValidationError, 'entityTypes must be an array');
-      await expect(call({ entityTypes: ['cited-analysis', null] })).to.be.rejectedWith(ValidationError, 'entityTypes must only contain: cited-analysis, reddit-analysis, youtube-analysis');
+      await expect(call({ entityTypes: ['cited-analysis', null] })).to.be.rejectedWith(ValidationError, 'entityTypes must only contain: cited-analysis, reddit-analysis, youtube-analysis (got [null])');
+      await expect(call({ entityTypes: ['wikipedia-analysis'] })).to.be.rejectedWith(ValidationError, '(got ["wikipedia-analysis"])');
+      // eslint-disable-next-line no-sparse-arrays
+      await expect(call({ sourceTypes: [, 'topic'] })).to.be.rejectedWith(ValidationError, 'sourceTypes must only contain: topic (got [null])');
+      await expect(call({ entityTypes: new Array(1) })).to.be.rejectedWith(ValidationError, 'entityTypes must only contain');
       await expect(call({ model: undefined })).to.be.rejectedWith(ValidationError, 'model is required');
       await expect(call({ dims: undefined })).to.be.rejectedWith(ValidationError, 'dims must be a positive integer');
       await expect(call({ vectors: 'x' })).to.be.rejectedWith(ValidationError, 'vectors must be an array');
@@ -597,9 +611,13 @@ describe('semantic-index.utils', () => {
       await lookupOpportunitiesByVectors(client, {
         ...base, entityTypes: [], vectors: [[0.1, 0.2]],
       });
+      await lookupOpportunitiesByVectors(client, {
+        ...base, entityTypes: null, vectors: [[0.1, 0.2]],
+      });
       expect(client.calls.rpc[0].params.p_source_types).to.deep.equal([SOURCE_TYPE]);
       expect(client.calls.rpc[0].params.p_entity_types).to.deep.equal(['reddit-analysis', 'cited-analysis']);
       expect(client.calls.rpc[1].params.p_entity_types).to.equal(null);
+      expect(client.calls.rpc[2].params.p_entity_types).to.equal(null);
     });
 
     it('exposes frozen source/entity type registries', () => {
@@ -655,10 +673,16 @@ describe('semantic-index.utils', () => {
       expect(client.calls.rpc[0].params).to.include({ p_limit: 5, p_min_score: 0.3 });
     });
 
-    it('wraps an RPC error', async () => {
+    it('wraps an RPC error with the searched type lists', async () => {
       const client = makeClient({ rpcResult: { data: null, error: { message: 'boom' } } });
-      await expect(lookupOpportunitiesByVectors(client, { ...base, vectors: [[0.1, 0.2]] }))
-        .to.be.rejectedWith(DataAccessError, 'Failed semantic search');
+      const err = await lookupOpportunitiesByVectors(client, {
+        ...base, entityTypes: ['reddit-analysis'], vectors: [[0.1, 0.2]],
+      }).catch((e) => e);
+      expect(err).to.be.instanceOf(DataAccessError);
+      expect(err.message).to.include('Failed semantic search');
+      expect(err.details).to.deep.equal({
+        siteId: SITE_ID, sourceTypes: [SOURCE_TYPE], entityTypes: ['reddit-analysis'],
+      });
     });
 
     it('rejects a query_index outside the current group (never spills into another group)', async () => {