feat: add opportunities by-topics semantic lookup endpoint (LLMO-7445)

[tb-adbe]

What

Read endpoint (step 4 of 4) for the Lookup Service topic dimension — POST /sites/:siteId/opportunities/by-topics returns, per topic, the site's opportunities semantically related to it (LLMO-7445, M2 v1). Built on the M1 by-url read patterns.

• src/support/lookup-by-topic.js engine (mirrors the by-url engine):
  1. parse body { topics[], k?, minScore?, status?, fields?, locale? } — topics 1–100, drop-don't-fail; k default 10 (max 100); minScore default 0.1 (LOOKUP_TOPIC_MIN_SCORE env).
  2. per distinct topic → resolve the query vector: semantic_query_embedding cache hit (+ best-effort touch), else batch AzureEmbeddingClient.createEmbeddings (native 1536) + cache upsert.
  3. lookupOpportunitiesByVector — site-scoped ANN cosine RPC (dedupes to distinct opportunities, best score, floor, top-k).
  4. union → hydrate (batchGetByKeys) → site-ownership re-filter + FACS/PLG → status-filter (hide IGNORED by default; status overrides) → project (lightweight default + fields opt-in).
  5. response: results[] (one per input topic, ranked { opportunityId, score }) + a single opportunities id→DTO map.
• Controller getByTopic — mirrors getByUrl (requirePostgrestClient → 503, locale, log, site-ownership filter) plus embedding-client construction (503 on misconfig).
• Route POST /sites/:siteId/opportunities/by-topics (plural, matching by-urls); :siteId classified in facs-capabilities.js; opportunity:read in required-capabilities.js.
• OpenAPI — LookupByTopicRequest, OpportunitiesByTopicResponse, path + examples.

Tests

• Unit tests: engine (test/support/lookup-by-topic.test.js) + isolated controller (test/controllers/opportunities-by-topic.test.js).
• IT (test/it/postgres/opportunities-by-topics*) seeds opportunity_semantic_embedding + a cache-hit semantic_query_embedding with deterministic 1536-dim unit vectors (no real Azure call). Gated: the IT docker-compose.yml db → pgvector/pgvector:pg16 (done) and the mysticat-data-service image must bump to the step-1 release before it passes in CI.

Notes

• New env: AZURE_EMBEDDING_* (same deployment as the audit-worker).
• Depends on the published spacecat-shared (step 2, data-access + gpt-client) and the data-service schema/RPC (step 1) — bump both deps after those land.

Related Issues

• Jira: LLMO-7445
• Part of the Lookup Service topic-dimension chain: adobe/mysticat-data-service#1064 (step 1) → feat: add Azure embedding client and semantic index utils (LLMO-7445) spacecat-shared#1933 (step 2) → feat: index offsite opportunity topics for semantic lookup (LLMO-7445) spacecat-audit-worker#2982 (step 3) → this PR (step 4)

[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 - two must-fix items before this ships.
Complexity: HIGH - large diff (1670 lines, 20 files); API surface + FACS signal.
Changes: Adds a POST /sites/:siteId/opportunities/by-topics semantic lookup endpoint that embeds topic text, runs pgvector ANN searches, and returns ranked opportunities per topic with full FACS/access-control enforcement (20 files).
Note: CI checks are currently failing - resolve before merge.

Must fix before merge

1. [Important] Missing length validation on createEmbeddings return value - src/support/lookup-by-topic.js:148 (details inline)
2. [Important] Sequential PostgREST calls serialize latency across cache reads and ANN searches - src/support/lookup-by-topic.js:133 (details inline)

Non-blocking (4): minor issues and suggestions

• nit: Module doc says POST .../opportunities/by-topic (singular) but the route is by-topics (plural) - src/support/lookup-by-topic.js:6
• suggestion: The MAX_LOOKUP_MATCHES guard at src/support/lookup-by-topic.js:169 fires after all ANN loops complete. Consider checking allIds.length incrementally inside the per-topic loop and bailing early once the cap is reached, to avoid running all RPC calls before rejecting.
• suggestion: touchQueryEmbedding awaits inside the for-loop but its .catch() already swallows errors - collect the promises and settle them after the loop rather than blocking each iteration - src/support/lookup-by-topic.js:140
• nit: The IT suite (test/it/postgres/opportunities-by-topics.test.js) will fail CI until the data-service image is bumped to include the semantic migrations. Consider gating the describe block with a before() hook that probes the table and calls this.skip() if absent, so it does not block unrelated PRs.

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 4m 16s | Cost: $6.89 | Commit: 289235b1b89043d5b71444665c48098e827d657b
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

issue (blocking): Missing length validation on createEmbeddings return value.

resolveTopicVectors calls embeddingClient.createEmbeddings(misses) and indexes into the result with vectors[i], assuming the returned array length matches misses.length. If the embedding service returns fewer vectors than inputs (partial failure, truncation, or a future SDK change), vectors[i] is undefined, which gets stored in the query cache and passed to the ANN RPC. This is a silent data corruption path - the cache would contain an undefined vector, and the RPC would either throw an opaque error or return garbage results.

Fix: After const vectors = await embeddingClient.createEmbeddings(misses), add a guard:

if (!Array.isArray(vectors) || vectors.length !== misses.length) {
  throw new Error(`Embedding response length mismatch: expected ${misses.length}, got ${vectors?.length}`);
}

This turns a silent corruption into a loud 5xx the caller can act on.

[MysticatBot]

issue (blocking): Sequential PostgREST calls serialize latency across cache reads and ANN searches.

resolveTopicVectors loops sequentially with await getQueryEmbedding and await touchQueryEmbedding per text (lines 133-145), then the engine loops sequentially with await lookupOpportunitiesByVector per distinct topic (lines 235-248). With 100 distinct topics (the upper bound), this is up to 200+ sequential PostgREST round-trips before hydration even starts. At typical production latencies (5-10ms per call), 100 topics could add 1-2 seconds of pure serialization overhead.

The createEmbeddings(misses) call is already batched, which is the right shape. The cache reads and ANN searches should match.

Fix: Batch the cache reads with Promise.all(texts.map(text => getQueryEmbedding(...))). Run the touchQueryEmbedding calls as fire-and-forget with Promise.allSettled (they already swallow errors). Parallelize the ANN searches with Promise.all over the distinct texts after all vectors are resolved. If you need to limit concurrency at the upper bound, use a bounded concurrency pool rather than pure sequential.

[tb-adbe]

Thanks @MysticatBot — addressed in 85f48f0.

Must-fix

1. createEmbeddings length guard — added after the batch embed: a non-array or length-mismatched response now throws a clear error instead of caching an undefined vector and passing it to the ANN RPC. (Note: the concrete AzureEmbeddingClient already validates response length; this makes the engine robust against any EmbeddingProvider that doesn't.)
2. Sequential PostgREST latency — the cache reads and the per-topic ANN searches now run through a bounded-concurrency pool (limit 20) instead of sequentially, so a 100-topic request no longer serializes ~200 round-trips while staying well under the shared PostgREST pool. Best-effort touch/upsert cache writes are fire-and-forget, settled after the loop.

Non-blocking
3. Doc by-topic → by-topics — fixed the module-doc endpoint path.
4. MAX_LOOKUP_MATCHES early-bail — mooted by #2: with the ANN searches now running in parallel there's no sequential loop to bail out of, so the cap check stays a single post-gather guard.
5. touchQueryEmbedding await-in-loop — folded into #2: the touch/upsert writes are collected and settled after the loop (bounded) rather than blocking each iteration.
6. IT gating — leaving it-postgres red intentionally for now. It's not just the by-topics describe: the shared seed (clearData's DELETE FROM semantic_query_embedding + the two semantic insertRows) requires the new tables, so the suite is gated on the mysticat-data-service semantic migrations shipping. That's step 1 of this feature's rollout (data-service merges + releases first); this endpoint PR is step 4. We'll green CI by bumping the IT docker-compose data-service image to that release once it lands, and validate the by-topics IT for real then — rather than adding a skip-probe that would mask the dependency.

Engine stays at 100% coverage.

[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 - well-structured addition that cleanly mirrors the established by-urls pattern with proper auth layering.
Complexity: HIGH - new API surface, FACS signal, 20 changed files across 1729 lines.
Changes: adds POST /sites/:siteId/opportunities/by-topics semantic lookup endpoint with pgvector ANN search, embedding resolution, bounded concurrency pool, durable query cache, full auth chain (site ownership + Summit-PLG + FACS composite), and comprehensive unit/IT coverage (20 files).
Note: Recommend a human read before merge - shared-contract change (docs/openapi/* files modified). The bot review is a complement to, not a replacement for, a human read here.
Note: CI checks are currently failing (it-postgres) - expected, pending data-service migration dependency.

Non-blocking (7): minor issues and suggestions

• nit: parseLookupTopics silently drops invalid entries without surfacing a count or warning to the caller - consistent with by-urls but worth noting for API consumers debugging mismatched result lengths - src/support/lookup-by-topic.js:93-95
• nit: OpenAPI LookupByTopicRequest.minScore.default: 0.1 is misleading for code-gen tools since the runtime default is env-configurable via LOOKUP_TOPIC_MIN_SCORE - consider removing the schema-level default and keeping it in the description only - docs/openapi/schemas.yaml (LookupByTopicRequest)
• nit: embedding client config error logged at error level when it is a known deployment configuration state (missing env var), not an unexpected failure - warn would be more appropriate - src/controllers/opportunities.js:289
• suggestion: projectLookup omits forceFields compared to the by-url version's projectLookupResults - verify this is intentional (by-topic has no suggestion cross-join needing opportunityId force-include) - src/support/lookup-by-topic.js:149-166
• suggestion: add a focused unit test verifying mapWithConcurrency respects its concurrency bound (max in-flight count) - the pool is tested indirectly through the engine but a direct test would catch regressions to unbounded fan-out - src/support/lookup-by-topic.js:64-76
• suggestion: add a controller-level assertion that filterOpportunitiesByFacsComposite and filterForSummitPlg are wired into the filterEntities callback - currently only the site-ownership re-filter is tested at the controller level, and the auth-chain steps are covered only by the IT suite - test/controllers/opportunities-by-topic.test.js
• suggestion: consider testing locale passthrough to OpportunityDto.toJSON at the controller level - the format validation is tested (400 on invalid) but the forwarding of a valid locale to the DTO serializer is not asserted - test/controllers/opportunities-by-topic.test.js

Previously flagged, now resolved

• Embedding length guard now validates createEmbeddings response length matches input
• Sequential PostgREST calls replaced with bounded-concurrency mapWithConcurrency (limit 20)
• Module doc header corrected from by-topic to by-topics
• touchQueryEmbedding await-in-loop folded into bounded writes
• MAX_LOOKUP_MATCHES early-bail issue mooted by concurrency rewrite
• Cache-hit access bump now passes textHash to skip re-hashing (new commit bc1fde8)

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 6m 30s | Cost: $13.31 | Commit: 85f48f04efa29f523158f985b1aa9e8daa5cf22d
If this code review was useful, please react with 👍. Otherwise, react with 👎.}