feat(data-access): batched semantic index writer and suggestion lookup (LLMO-7445)

[tb-adbe]

1. Abstract

Moves the semantic-index helpers to the redesigned tables: one batched writer and two readers for opportunities and suggestions. The type registries are removed from utils.

2. Reasoning

The first version of the semantic index had separate shapes per table, a source_type column, and type registries in utils, so every new producer or text kind needed a shared release. Each producer would also have had to embed, batch, and dedupe on its own. The data-service redesign (#1153) gives all three tables the same text columns and splits source_type into match_type and match_field_type. This PR moves the shared helpers to that shape and puts the embed and write logic in one place.

3. High-level overview of the changes

utils

• Removes OPPORTUNITY_SEMANTIC_SOURCE_TYPES, OPPORTUNITY_SEMANTIC_ENTITY_TYPES, and the suggestion equivalents, along with their .d.ts types. This reverts feat(utils): add semantic lookup source/entity type registries (LLMO-7445) #1959.

data-access (semantic-index.utils.js)

• The match type is never a parameter. Each dimension has its own functions that set match_type internally, so callers can't index or search under the wrong lookup. Today that's topics; claims follow the same pattern when that dimension lands. SEMANTIC_MATCH_TYPES is internal.
• indexSemanticTopics(pg, embeddingClient, { target, siteId, entities: [{ entityId, entityType, fields: [{ matchFieldType, texts }] }] }) replaces syncOpportunitySemantic. target is opportunity or suggestion.
  • It full-replaces each (entity, matchFieldType) group: it keeps unchanged texts, embeds and inserts new ones, and deletes stale ones. Fields that aren't listed (and other match types) are left alone, and texts: [] clears a field.
  • A stored row from another embedding model or dims counts as stale: it is re-embedded and overwritten in place on the entity's next refresh (a model change is a hard cutover per entity; mysticat-data-service#1153 review).
  • Texts are deduped across entities, looked up in the query cache first, and the misses are embedded in batches of 256.
  • It reads semantic_query_embedding but never writes to it.
• embedQueries(pg, embeddingClient, texts, { timeoutMs, log }) is the query side. It normalizes the texts, serves cached vectors, embeds the distinct misses in one call within a time budget, and returns the cache writes as a best-effort cacheWrites promise.
  • It rejects empty or over-2048-character texts (once normalized) and more than MAX_QUERY_TEXTS (256) texts with a ValidationError, and treats a cache read failure as all misses (logged and returned as cacheError).
• lookupOpportunitiesByTopic and the new lookupSuggestionsByTopic replace lookupOpportunitiesByVectors. They take optional matchFieldTypes / entityTypes filters (free-form, deduped, max 100 values).
  • lookupSuggestionsByTopic checks statuses against Suggestion.STATUSES and adds opportunityStatuses.
• No function takes the embedding generation: the writer, embedQueries and both lookups read SEMANTIC_MATCHING_CONFIG.embedding themselves.
• siteId and entityId must be UUIDs, and each writer field reports a rejected count (texts dropped as empty or too long).
• entityType and matchFieldType are free-form, length-bounded strings, so there are no registries left.
• New exports: SEMANTIC_TARGETS, SEMANTIC_SEARCH_RPCS (target → RPC), MAX_QUERY_TEXTS and EmbeddingUnavailableError.
• getQueryEmbeddings, upsertQueryEmbeddings and touchQueryEmbeddings are now internal, so only embedQueries writes the cache that the writer and readers trust. The cache row stores the normalized text.

Release type. This is a breaking change to the semantic API in both packages, released as feat (minor). No external code consumes these helpers yet: the writer in audit-worker is behind OFFSITE_SEMANTIC_INDEX_ENABLED (off), consumers pin exact versions, and their PRs adopting this API come next. The old helpers also stop working once mysticat-data-service#1153 runs.

Removed exports (to list in the squash commit body):

• utils: OPPORTUNITY_SEMANTIC_SOURCE_TYPES, OPPORTUNITY_SEMANTIC_ENTITY_TYPES, SUGGESTION_SEMANTIC_SOURCE_TYPES, SUGGESTION_SEMANTIC_ENTITY_TYPES and their .d.ts types;
• data-access: syncOpportunitySemantic, lookupOpportunitiesByVectors, SEMANTIC_SEARCH_RPC, the re-exported OPPORTUNITY_SEMANTIC_SOURCE_TYPES / OPPORTUNITY_SEMANTIC_ENTITY_TYPES, and the cache helpers getQueryEmbeddings / upsertQueryEmbeddings / touchQueryEmbeddings (no consumer imports them); the lookups no longer accept model / dims.

4. Required information

• Jira / issue: LLMO-7445
• ADR(s): docs/adr/0002-embedding-client-and-semantic-index-utils.md (Decision 4 amended; outdated paragraphs marked superseded)
• Other: mysticat-data-service#1153 (table and RPC redesign)

6. Affected / used mysticat-workspace projects

• mysticat-data-service: contract. The helpers write the redesigned tables and call the new RPC signatures (p_match_type, p_match_field_types) from feat: implement strict 500 version limit for configurations #1153.
• spacecat-audit-worker: consumed. It switches its writer to indexSemanticTopics when it bumps data-access (spacecat-audit-worker#27, draft; the writer is flag-off today).
• spacecat-api-service: consumed. The by-topics endpoint switches to embedQueries, lookupOpportunitiesByTopic and matchFieldTypes, and drops sourceTypes, when it bumps data-access (spacecat-api-service#79, draft).

8. Test plan

(a) Nothing beyond unit tests yet. The helpers are first exercised end to end when the audit-worker and api-service follow-ups run against a local data-service with #1153 applied.

(b) dev: after #1153 is deployed and the consumer PRs ship, enable OFFSITE_SEMANTIC_INDEX_ENABLED and run an offsite audit. Check that rows land in opportunity_semantic_embedding with match_type='topic', then call the by-topics endpoint and confirm it returns matches.

9. Deployment & merge order

• mysticat-data-service#1153: depends-on. Its RPC signatures must be deployed before any consumer uses this release.
• spacecat-audit-worker / spacecat-api-service follow-ups: blocked by this PR (they bump to its release).

Order: data-service #1153 → this PR (release) → audit-worker + api-service bumps.

🤖 Generated with review-kit

[github-actions]

This PR will trigger a minor release when merged.

[MysticatBot]

Hey @tb-adbe,

⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.

Verdict: Request changes - the batched writer is solid and well tested, but the read-side embedQueries skips the input bounds the writer applies.
Complexity: HIGH - large diff (about 1,500 changed lines) across two packages with a breaking API reshape.
Changes: replaces the per-table semantic helpers with one batched indexSemanticTexts writer, an embedQueries read helper and a new lookupSuggestionsByVectors, and removes the semantic type registries from utils (12 files).
Note: Recommend a human read before merge - this change amends docs/adr/0002-embedding-client-and-semantic-index-utils.md (architectural decision record). The bot review is a complement to, not a replacement for, a human read here.
Note: CI checks were still pending at review time.

Must fix before merge

1. [Important] embedQueries does not apply MAX_SOURCE_TEXT_LENGTH to query texts, so long queries re-embed on every call and never cache - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:718 (details inline)
2. [Important] embedQueries has no cap on text count and sends all misses in one unbatched embed call - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:733 (details inline)

Non-blocking (10): minor issues and suggestions

• suggestion: the stored-row diff ignores model/dims, so changing SEMANTIC_MATCHING_CONFIG.embedding leaves old vectors marked unchanged and search silently stops matching; select and compare them - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:336
• suggestion: unchanged rows never get a corrected entityType, so entityTypes filters can miss them; compare stored entity_type or document that it is immutable - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:614
• suggestion: mixed input like ['ok', <3000 chars>] silently drops the bad text while full-replace still prunes the group; throw or return a rejected count - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:365
• suggestion: the length check runs before toLowerCase(), which can lengthen some strings; check the normalized key against the limit - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:241
• suggestion: the cache-read catch swallows any error into result.cacheError with no log; narrow it or accept a log and warn like embedQueries - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:632
• suggestion: timeoutMs is not validated; 0, negative or NaN fails every miss as EmbeddingUnavailableError - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:709
• suggestion: entityId is only checked for non-empty; validating it as a UUID fails fast and keeps .in() filters well-formed - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:358
• suggestion: removed exports (syncOpportunitySemantic, SEMANTIC_SEARCH_RPC, the *_SEMANTIC_* registries) ship as a minor; add a BREAKING CHANGE: footer or an explicit note in the squash commit body - packages/spacecat-shared-utils/src/index.js:92
• suggestion: the semantic exports have no .d.ts declarations (pre-existing, but the surface grew) - packages/spacecat-shared-data-access/src/service/index.js:75
• nit: indexSemanticTexts/embedQueries pin model/dims from config while the lookups still require callers to pass them; pick one convention - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:858

Open design questions (3): not blocking

• A semantic_query_embedding read failure fails every embedQueries call, while the writer treats the same failure as best-effort; is that intended for the synchronous API path? - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:720
• The global query cache stores normalized query text from every tenant; is SELECT restricted to the writer role, and are the hits/misses counts kept internal? - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:754
• ADR-0002 still describes adding types to the removed registries above the amendment; should that text carry an inline superseded note? - docs/adr/0002-embedding-client-and-semantic-index-utils.md:15

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-5-5[1m] | Duration: 1m 1s | Cost: $4.50 | Commit: 7f6abf8726f5b79151d7b40082ea9b31baf8c289
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

issue (blocking): embedQueries only runs hashQueryText, which rejects empty strings but not over-length ones. The writer bounds texts at MAX_SOURCE_TEXT_LENGTH through cleanTopicText, and that JSDoc says both sides "reject the same input".

Failure: a query longer than 2048 characters misses the cache and gets embedded. The cache upsert then breaks the text CHECK that the :71 comment says this constant matches. That error is swallowed as a warn at :745, so every repeat of the query pays for a new embedding and logs again. If the provider rejects the length instead, the caller sees EmbeddingUnavailableError (an outage) for what is really a bad request.

Fix: run each text through cleanTopicText and throw ValidationError on null. Reject rather than truncate, so the stored hash matches what was embedded. Add a test with a 2049-character query.

Blocking basis: round1

[tb-adbe]

Fixed in d65e289: embedQueries now runs every text through cleanTopicText and throws ValidationError (texts[i] must be a non-empty string of at most 2048 characters) instead of truncating, so the cached hash always matches what was embedded. cleanTopicText now checks the normalized key (what is stored), so the writer and reader share the exact bound. Tests cover a 2049-character query.

[MysticatBot]

issue (blocking): texts has no upper bound. All misses go to createEmbeddings in one call, with no dedupe and none of the writer's EMBEDDING_BATCH_SIZE batching.

Failure: a request with more distinct uncached texts than the per-request input cap noted at :68 always fails as EmbeddingUnavailableError. Below that cap, one request can still drive one large paid embed call, and duplicate misses are sent more than once. The timeout at :686 only abandons the race; it does not cancel the underlying request.

Fix: dedupe misses by normalized text and fan the vectors back out to their positions. Then either reject inputs above a MAX_QUERY_TEXTS (at most EMBEDDING_BATCH_SIZE) with a ValidationError before the cache read, or batch within the time budget.

Blocking basis: round1

[tb-adbe]

Fixed in d65e289: misses are deduped by normalized text and fanned back out to their positions, and more than MAX_QUERY_TEXTS (= EMBEDDING_BATCH_SIZE, 256) distinct texts is rejected with a ValidationError before the cache read, so the misses always fit one embed call. hits / misses now count distinct texts. api-service caps topics at 100, so it's unaffected. Tests cover the dedupe, the cap and exactly MAX_QUERY_TEXTS.

[tb-adbe]

Thanks. Both blocking findings are fixed in d65e289 (inline replies). Note the review ran on the first commit; indexSemanticTexts / lookupSuggestionsByVectors are now indexSemanticTopics / lookupSuggestionsByTopic. The rest:

Non-blocking

• model/dims ignored by the stored-row diff: fixed in 6c699d2. Rows from another generation are re-embedded and overwritten in place.
• entityType on unchanged rows: it's the opportunity type, which never changes for an existing opportunity. Documented as fixed per entity in the JSDoc rather than compared (that would only add re-embeds).
• Mixed valid/invalid texts: kept as drop-and-continue (offsite topics come from LLM output; failing the whole batch for one bad title is worse), but each field now reports a rejected count, and the audit-worker logs it as rejectedTopicCount. An input where every entry is invalid still throws.
• Length checked before toLowerCase(): fixed, cleanTopicText checks the normalized key.
• Writer cache-read catch: not silent. cacheError is returned, and the audit-worker wrapper logs it as queryCacheErrorMessage.
• timeoutMs: validated as a positive finite number.
• entityId (and siteId): validated as UUIDs.
• Removed exports shipping as a minor: deliberate. These APIs were published days ago, only the LLMO-7445 code paths (being updated in lockstep) use them, and they stop working once mysticat-data-service#1153 runs, so a major would only force every consumer to bump. The PR description now lists the removals, for the squash commit body.
• .d.ts: pre-existing gap; data-access declares no util helpers at all (url-index included). Happy to add declarations for the semantic and url-index utils in a follow-up.
• model/dims convention: the lookups now read SEMANTIC_MATCHING_CONFIG.embedding too, like the writer and embedQueries; callers no longer pass them.

Design questions

• Query-cache read failure on the read path: now best-effort like the writer. It logs a warning, treats everything as a miss and embeds within the timeout (at most one call, given the cap above).
• Global query cache: grants match every data-service table (anon SELECT/INSERT, writer UPDATE/DELETE), and PostgREST is only reachable internally or through the IP-allowlisted CloudFront. Rows hold normalized topic text only, with no site, org or tenant column. hits/misses are only logged by api-service and never returned to clients.
• ADR-0002: the outdated paragraphs now carry an inline "Superseded by the amendment below" note, and the amendment covers the generation and read-side bounds.

[MysticatBot]

Hey @tb-adbe,

⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.

Verdict: Approve - both earlier blocking findings are fixed and tested. What is left is ADR wording, a few test gaps and optional hardening.
Complexity: HIGH - large diff (about 2,800 diff lines) across two packages with a breaking API reshape.
Changes: moves the semantic-index helpers to the redesigned tables with a batched indexSemanticTopics writer, embedQueries, and topic lookups for opportunities and suggestions, and removes the semantic type registries from utils (12 files).
Note: Recommend a human read before merge - this change amends docs/adr/0002-embedding-client-and-semantic-index-utils.md (architectural decision record). The bot review is a complement to, not a replacement for, a human read here.
Note: CI checks were still pending at review time. Merge once the Test job passes the coverage thresholds.

Non-blocking (12): minor issues and suggestions

• nit: this Consequences bullet still names lookupOpportunitiesByVectors and caller-passed model/dims, with no superseded marker. Rename it to lookupOpportunitiesByTopic / lookupSuggestionsByTopic or point it at the amendment - docs/adr/0002-embedding-client-and-semantic-index-utils.md:71
• nit: these _Superseded by the amendment below:_ markers sit in the middle of sentences and are followed by a double space. Move them to the start of the paragraph, as at lines 43 and 72 - docs/adr/0002-embedding-client-and-semantic-index-utils.md:48
• nit: the Alternatives entry that rejects embedding inside data-access is reversed by the amendment but has no inline marker - docs/adr/0002-embedding-client-and-semantic-index-utils.md:80
• nit: the error says "at most 2048 characters", but the check runs on the normalized key. Add "once normalized" so the message matches the JSDoc - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:746
• suggestion: with no log, a failed query-cache read in embedQueries leaves no trace, and the catch also swallows code bugs. Return cacheError as the writer does, or narrow the catch to DataAccessError - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:772
• suggestion: only distinct texts are capped. Many copies of one text cost a single embed call but return one vector each, and searchByVectors then sends one sequential RPC per 20 vectors. Cap texts.length or vectors.length at MAX_QUERY_TEXTS - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:798
• suggestion: embedWithinBudget abandons the race on timeout but does not abort the paid createEmbeddings call, and timed-out misses are never cached, so retries pay again. Pass an AbortSignal if the client supports one - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:702
• suggestion: MAX_QUERY_TEXTS = EMBEDDING_BATCH_SIZE couples the read-side API limit to a write-side batching constant. A separate constant with a "must be <= EMBEDDING_BATCH_SIZE" comment keeps the two decisions independent - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:74
• suggestion: the writer now copies vectors from the query cache into persistent index rows, so a bad cache row now persists past cache eviction. Record in the ADR that this relies on the internal-only PostgREST trust boundary - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:616
• suggestion: add a test for a stored old-model row whose text is not resubmitted, asserting it is deleted by id - packages/spacecat-shared-data-access/test/unit/util/semantic-index.utils.test.js:430
• suggestion: add an embedQueries test that mixes cache hits with duplicate inputs (for example ['Cached', 'cached ', 'fresh']) and asserts hits, misses, the fanned-back vectors and one touched hash - packages/spacecat-shared-data-access/test/unit/util/semantic-index.utils.test.js:564
• suggestion: in the exactly-MAX_QUERY_TEXTS test, also assert that the misses went out in a single embed call - packages/spacecat-shared-data-access/test/unit/util/semantic-index.utils.test.js:578

Open design questions (2): not blocking

• After a model change, rows for entities that are never refreshed (closed opportunities, suggestions under resolved opportunities) stay forever and are invisible to search. Is a one-off sweep (delete where model is not current) part of the model-change runbook? - docs/adr/0002-embedding-client-and-semantic-index-utils.md:71
• Should upsertQueryEmbeddings stay a public export, now that both readers and the writer trust the global cache it writes to? - packages/spacecat-shared-data-access/src/service/index.js:54

Previously flagged, now resolved

• embedQueries now runs every text through cleanTopicText and rejects empty or over-length normalized keys with ValidationError. A test covers a 2049-character query.
• embedQueries dedupes misses, rejects more than MAX_QUERY_TEXTS distinct texts before the cache read, and maps vectors back to input order. Tests cover the cap and the at-limit-plus-repeat case.
• Stored rows from another model or dims are re-embedded and overwritten in place. timeoutMs, siteId and entityId are validated, and a query-cache read failure on the read path is now best-effort.

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-5-5[1m] | Duration: 0m 48s | Cost: $4.72 | Commit: d65e28968f42d2c6f79150a1e2a38e8d96e0e747
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[tb-adbe]

Thanks. Addressed in e960850, all but one:

• ADR: the generation bullet in Consequences now names the topic lookups and says the generation comes from config. The superseded markers sit at the start of their paragraphs, and the reversed "embedding inside data-access" and * wildcard alternatives are marked too.
• The embedQueries error now says "once normalized".
• A cache read failure is also returned as cacheError, like the writer, so it's visible without a log.
• texts.length is capped at MAX_QUERY_TEXTS, so repeats count too.
• MAX_QUERY_TEXTS is its own constant (256), with a note to keep it at most EMBEDDING_BATCH_SIZE.
• The ADR amendment records that the writer copies cache vectors into index rows and relies on the internal-only PostgREST boundary.
• New tests: an old-generation row that isn't resubmitted is deleted by id; hits mixed with repeats (['Cached', 'cached ', 'fresh']) give one touched hash and the right vectors; exactly MAX_QUERY_TEXTS goes out in one embed call.
• Not done: aborting the embed call on timeout. AzureEmbeddingClient.createEmbeddings takes no abort signal today, so it needs a gpt-client change first. Happy to follow up.

Design questions:

• Model change: yes. The ADR now says never-refreshed entities keep old-generation rows (ignored by search) and that the model-change runbook ends with a one-off sweep deleting rows whose model/dims isn't current.
• upsertQueryEmbeddings: no consumer imports the cache helpers, so getQueryEmbeddings / upsertQueryEmbeddings / touchQueryEmbeddings are now internal; only embedQueries writes the cache. They're added to the removed-exports list in the description.

[MysticatBot]

Hey @tb-adbe,

⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.

Verdict: Approve - the new commit addresses the earlier suggestions with targeted tests. What is left is wording, an optional guard test, and light hardening.
Complexity: HIGH - large diff (about 2,800 diff lines) across two packages with a breaking API reshape.
Changes: moves the semantic-index helpers to the redesigned tables with a batched indexSemanticTopics writer, embedQueries, and topic lookups for opportunities and suggestions, and removes the semantic type registries from utils (12 files).
Note: Recommend a human read before merge - this change amends docs/adr/0002-embedding-client-and-semantic-index-utils.md (architectural decision record). The bot review is a complement to, not a replacement for, a human read here.
Note: CI checks were still pending at review time.

Non-blocking (4): minor issues and suggestions

• suggestion: "nothing outside data-access can write vectors that the writer and readers trust" claims more than the code enforces. The package has no exports map, so a deep import of src/util/semantic-index.utils.js still reaches upsertQueryEmbeddings. Drop the export keyword from the three cache helpers (tests can use a test-only re-export), or soften the line to match the ADR: not part of the public API, trusted on the assumption that only embedQueries writes the cache - packages/spacecat-shared-data-access/README.md:194
• nit: "logged and returned as cacheError), like the writer" reads as if the writer also logs. indexSemanticTopics returns cacheError without logging it. Attach "like the writer" to the all-misses behavior only - docs/adr/0002-embedding-client-and-semantic-index-utils.md:67
• suggestion: MAX_QUERY_TEXTS <= EMBEDDING_BATCH_SIZE is only a comment, and the single-embed-call test stays green if either constant changes. Add expect(MAX_QUERY_TEXTS).to.be.at.most(EMBEDDING_BATCH_SIZE) - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:74
• suggestion: cacheError is the raw DataAccessError, which carries table names and the PostgREST cause. Add a JSDoc note that callers log it and never return it to clients - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:807

Open design questions (1): not blocking

• Will the old-generation row sweep in the model-change runbook be a script, or only runbook prose? - docs/adr/0002-embedding-client-and-semantic-index-utils.md:72

Previously flagged, now resolved

• MAX_QUERY_TEXTS is its own constant, and the cap now counts every entry, not just distinct ones, checked before any work. A test covers repeated inputs.
• embedQueries returns cacheError on a cache read failure, and the error message says "once normalized".
• The query-cache helpers are no longer exported from service/index.js, and the ADR records the cache-copy trust boundary and the model-change sweep.
• ADR superseded markers now start their paragraphs, and the Consequences and Alternatives entries point at the new lookups and the amendment.
• New tests cover deleting an old-generation row, fanning cached repeats back out, and sending one embed call at the limit.

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-5-5[1m] | Duration: 0m 46s | Cost: $2.74 | Commit: e960850ac120b4c38d3b09c20541e7b489be0bcc
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[rarescheseli]

Hey @tb-adbe,

⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.

Verdict: Approve - the writer, embedQueries and both topic lookups are correct and well tested; what is left is type declarations and doc accuracy, none blocking.
Complexity: HIGH - large diff across two packages with a breaking reshape of the semantic API.
Changes: moves the semantic-index helpers to the redesigned tables with a batched indexSemanticTopics writer, embedQueries, and topic lookups for opportunities and suggestions, and removes the semantic type registries from utils (12 files).
Note: Recommend a human read before merge - this change amends docs/adr/0002-embedding-client-and-semantic-index-utils.md (architectural decision record), and the docs claim a cache trust boundary the code does not enforce (heuristic). The bot review is a complement to, not a replacement for, a human read here.

Non-blocking (5): minor issues and suggestions

• suggestion: the root types entry (src/index.d.ts) still declares none of the semantic exports, while the README tells consumers to import indexSemanticTopics, embedQueries, lookupOpportunitiesByTopic, lookupSuggestionsByTopic, SEMANTIC_TARGETS, MAX_QUERY_TEXTS and EmbeddingUnavailableError from the package root. The gap predates this PR, but the PR rewrites the whole surface. Please file the follow-up you offered (semantic + url-index declarations) before api-service adopts this API - packages/spacecat-shared-data-access/src/index.d.ts:13
• suggestion: "nothing outside data-access can write vectors that the writer and readers trust" is stronger than the code. getQueryEmbeddings / upsertQueryEmbeddings / touchQueryEmbeddings are still exported at semantic-index.utils.js:430, :475, :519, and package.json has no exports map, so a deep import reaches them. Drop the export keywords (tests can import via a test-only re-export) or reword to "not part of the public API" - packages/spacecat-shared-data-access/README.md:194
• nit: "(logged and returned as cacheError), like the writer" reads as if the writer logs too; indexSemanticTopics only returns cacheError. Attach "like the writer" to the all-misses behavior only - docs/adr/0002-embedding-client-and-semantic-index-utils.md:67
• suggestion: MAX_QUERY_TEXTS <= EMBEDDING_BATCH_SIZE is only a comment, and the one-embed-call test stays green if either constant changes. Add expect(MAX_QUERY_TEXTS).to.be.at.most(EMBEDDING_BATCH_SIZE) - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:74
• suggestion: cacheError is the raw DataAccessError (table names, PostgREST cause). Note in the JSDoc that callers log it and never return it to clients - packages/spacecat-shared-data-access/src/util/semantic-index.utils.js:729

[solaris007]

🎉 This PR is included in version @adobe/spacecat-shared-data-access-v4.39.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[solaris007]

🎉 This PR is included in version @adobe/spacecat-shared-utils-v1.128.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀