feat(serenity): opt-in per-prompt userIntent on brand-presence prompts endpoint

[jjenscodee]

Summary

Adds an opt-in userIntent enrichment to the Serenity brand-presence prompts endpoint (GET /v2/orgs/:orgId/brands/:brandId/serenity/brand-presence/prompts). When ?userIntent=true, each returned row gains its own classified intent (userIntent) alongside the existing prompt / prompt_topic / primary_intent / volume.

How

Per-prompt intent is not a column on the Semrush PROMPTS element, so it's derived the same way branded% already is: the base PROMPTS call + one tags:['intent__<Value>']-filtered call per Semrush intent, run in parallel (mapWithConcurrency) and joined back by prompt text. Non-fatal — a classification-call failure degrades each row to userIntent: '', never failing the request. Without the flag, response shape and upstream call count are unchanged (protects the existing branded% / intent-coverage caller, useSerenityPromptHealthStats).

Why

The Topic Intent Alignment tile (project-elmo-ui#2478) compares each topic's expected intent against the distribution of its prompts' own intents. It previously joined against the ~1,000-capped prompt vocabulary; this lets it read per-prompt intent off the full, uncapped bp rows. Complements Niccolo's server-computed classification (serenity-docs#40, #2785) which populates the intent tag this reads back.

Verified

• npm run type-check clean · npx eslint clean · unit tests (test/support/elements/elements-service.test.js + test/controllers/elements.test.js) — 131 passing.
• intent__<Value> tag filter verified live against the PROMPTS element (Lovesac: intent__Informational=1156, intent__Commercial=128 of 1488).

Notes / follow-ups

• OpenAPI: this endpoint is currently an undocumented POC (its siblings stats/sentiment are documented; prompts/weeks/cited-domains aren't). Left undocumented, consistent with that — happy to add a full entry if preferred.
• Integration test: the elements bp endpoints are POC with deferred ITs; the enrichment is unit-covered. Can add a serenity IT (vendor-mock intent__ tags) if we want IT coverage here.
• Coordinate with @ntoccane on exposing per-prompt intent as this read-out, to align with the classification design.

🤖 Generated with Claude Code

[github-actions]

This PR will trigger a minor release when merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[MysticatBot]

Hey @jjenscodee,

⚠ 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 contract/resilience issues in the enrichment path need fixing before merge.
Complexity: MEDIUM - medium diff; API surface.
Changes: Adds opt-in per-prompt userIntent enrichment to the brand-presence prompts endpoint via parallel intent-filtered Semrush calls joined by prompt text (6 files).

Must fix before merge

1. [Important] Count semantics diverge in enriched path - src/support/elements/elements-service.js:197 (details inline)
2. [Important] All-or-nothing degradation on intent fan-out catch - src/support/elements/elements-service.js:178 (details inline)

Non-blocking (4): minor issues and suggestions

• nit: enrichUserIntent flag leaks through spread into buildPromptsPayload calls; destructure it out before spreading - src/support/elements/elements-service.js:174
• nit: JSDoc for getPrompts contains a duplicated "Non-fatal" sentence - src/support/elements/elements-service.js:144
• suggestion: Add a controller-level test that passes ?userIntent=true and asserts enrichUserIntent: true reaches the service (only the false case is covered today) - test/controllers/elements.test.js
• suggestion: When this endpoint graduates from POC, add an OpenAPI entry for the userIntent query param to stay in sync with the documented OpenAPI-first convention.

---

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

[MysticatBot]

issue (blocking): The non-enriched path returns base.count (from transformPromptsResponse, which may carry a server-reported total for pagination). The enriched path returns { count: prompts.length, prompts }, replacing that with the local array length. If count ever differs from the row count (e.g. pagination metadata), this silently changes the contract for consumers who opt into userIntent=true.

Fix: return { count: base.count, prompts };

[MysticatBot]

issue (blocking): The .catch(() => []) wraps the entire mapWithConcurrency promise. If mapWithConcurrency rejects on the first unhandled rejection (standard Promise behavior), a single failing intent call wipes ALL intent data - even if 4 of 5 calls succeeded. The stated guarantee is "non-fatal per row" but the implementation is "non-fatal per batch."

Fix: move error handling inside the callback so each intent value degrades independently:

async (value) => {
  try {
    const raw = await transport.fetchElement(...);
    return { key: value.toLowerCase(), rows: transformPromptsResponse(raw).prompts };
  } catch {
    log.warn('intent enrichment failed for value', { value, workspaceId });
    return { key: value.toLowerCase(), rows: [] };
  }
}

Then remove the outer .catch(() => []). This also adds the logging visibility that the current silent catch lacks.

[jjenscodee]

Thanks @MysticatBot — pushed fixes for both blocking issues (commit 278007d4):

• ✅ Count semantics (elements-service.js:197) — enriched path now returns base.count instead of the local array length, preserving the endpoint's count contract for userIntent=true callers.
• ✅ All-or-nothing degradation (elements-service.js:178) — moved the error handling inside the per-intent callback (each value returns { key, rows: [] } on failure) and removed the outer .catch, so one failing intent-filtered call now drops only that intent's rows instead of wiping all intent data.

Non-blocking items noted for follow-up: destructure enrichUserIntent before spreading into buildPromptsPayload; dedupe the JSDoc "non-fatal" sentence; add a ?userIntent=true controller test; add the OpenAPI entry when this endpoint graduates from POC. (Per-intent catch currently swallows silently — the service has no logger injected; happy to thread one through if preferred.)

Re-requesting review.

[MysticatBot]

Hey @jjenscodee,

⚠ 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 prior blocking findings are correctly addressed; no remaining issues warrant blocking.
Complexity: MEDIUM - medium diff; API surface.
Changes: Adds opt-in per-prompt userIntent enrichment to the brand-presence prompts endpoint via parallel intent-filtered Semrush calls joined by prompt text (6 files).

Non-blocking (4): minor issues and suggestions

• suggestion: Add a controller-level test that passes ?userIntent=true and asserts enrichUserIntent: true reaches the service - the false case is covered but the true wiring is untested at the integration level - test/controllers/elements.test.js
• nit: The per-intent catch block silently returns empty rows with no logging - when a logger is injected into the service, add a log.warn with the intent value and workspace ID for operational visibility - src/support/elements/elements-service.js:176
• nit: enrichUserIntent propagates into intent-filtered buildPromptsPayload calls via spread - harmless today (the builder destructures only known keys) but destructuring it out before spreading would make the intent explicit - src/support/elements/elements-service.js:170
• nit: If a prompt text appeared in multiple intent-filtered results, last-write-wins on the intentByPrompt Map - the JSDoc correctly notes this shouldn't happen ("a prompt carries exactly one intent tag") but a brief inline comment on the .set() call would document the assumption for future readers - src/support/elements/elements-service.js:192

Previously flagged, now resolved

• Count semantics in enriched path now returns base.count instead of local array length
• Per-intent error isolation moved inside the callback so one failing intent drops only that intent's rows

---

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

[JayKid]

Multi-persona review — Approve with reservations

Bot's two blocking items are addressed (all-or-nothing fix is genuinely correct + tested).
Nothing blocks a POC merge, but two items deserve action before this feeds the prod tile:

Major — join rests on unverified upstream invariants. The join keys on free-text
prompt (elements-service.js:158–174) and assumes prompt text is unique per result set
AND each prompt has exactly one intent tag. Neither is enforced; if either breaks, rows get
silently mislabeled — defeating the Topic Intent Alignment tile's purpose. Please confirm both
with @ntoccane, or key on a stable id / prompt|prompt_topic tuple. (Confidence: Medium)

Major — silent degradation, no observability. Bare catch {} at :149 discards the error;
a failing intent__X call is indistinguishable from a genuinely intent-less prompt (both '').
Thread log into the service and log.warn the caught error. (Confidence: High)

Minor — the count "fix" is a no-op. transformPromptsResponse always sets
count = prompts.length and there's no server total, so base.count === prompts.length here
always. The comment at :169–170 ("may be a server-reported total") describes a field that
doesn't exist — please correct or drop it. (Confidence: High)

Minor: no controller test for userIntent=true wiring; base-call-failure path on the
enriched branch untested; enrichUserIntent leaks into buildPromptsPayload via spread
(harmless — destructure it out); "non-fatal" documented 3×.

Question: is PROMPTS row-capped? If so, base and intent calls cap independently → false-negative
intents for prompts beyond an intent call's cap.

_{🤖 Multi-persona review (Architect · DBA · Security · Tester/Junior) via Claude Code}

[jjenscodee]

Thanks @JayKid — solid review. Addressed in fa5bba8b:

• ✅ Join invariants (Major). Now keys on the (prompt, prompt_topic) tuple and is gated to single-slice calls (exactly one projectId). Rationale: the PROMPTS element row exposes no semrushPromptId/geo/language, and a prompt's true identity is semrushPromptId within a (geoTargetId, languageCode) slice — so a cross-market join can't be disambiguated from row data at all. Rather than risk silent mislabeling, enrichment is skipped unless one project is requested (our only consumer, the alignment tile, always is). The clean long-term fix is a stable per-row id from upstream — raised with @ntoccane.
• ✅ Observability (Major). Threaded a logger into the Elements service; the per-intent catch now log.warns the swallowed error instead of degrading silently.
• ✅ Count "fix" is a no-op (Minor). Correct — dropped the misleading "server-reported total" comment; base.count currently equals the row count.
• ✅ Nits. Destructured enrichUserIntent out before the buildPromptsPayload spread; deduped the "non-fatal" JSDoc; added a ?userIntent=true controller-wiring test + a (prompt, prompt_topic) tuple-disambiguation test.

Row-cap question: not observed for Lovesac (1,488 returned whole), but you're right it's a real risk if the element caps — the single-slice guard bounds the blast radius, and it's on the list for @ntoccane along with the stable-id ask. Left the OpenAPI entry for when this endpoint graduates from POC.

Verified: type-check ✓, eslint ✓, unit tests 133 passing.

[JayKid]

Re-review after fa5bba8b — Approve ✅

Both Major findings from my earlier review are resolved, and the join fix is handled the right way — gated rather than papered over.

Major — join invariants ✅ Now keys on the (prompt, prompt_topic) tuple and hard-gates enrichment to exactly one projectId, so the cross-market ambiguity can't arise (multi-slice requests never enrich). Correctly documents that the row exposes no semrushPromptId/geo/language and that a stable per-row id is the long-term fix (raised with @ntoccane). New tests cover both the tuple disambiguation and the single-slice gate.

Major — observability ✅ Logger threaded through the service; the per-intent catch now log.warns the swallowed error with workspaceId + message instead of degrading silently.

Minors ✅ Misleading count comment dropped; enrichUserIntent destructured out before the payload spread; ?userIntent=true controller-wiring test added; "non-fatal" JSDoc deduped.

Remaining items are all non-blocking, author's discretion:

• Base-call-failure path on the enriched branch still untested (JSDoc promises it propagates).
• The old ANDs intent__X with a pre-existing tag assertion was replaced rather than kept — the enrichment tag-merge is no longer explicitly asserted.
• Row-cap risk acknowledged and bounded by the single-slice guard; deferred to @ntoccane.

Clean, conservative POC change. LGTM.

_{🤖 Multi-persona review (Architect · DBA · Security · Tester/Junior) via Claude Code}

[solaris007]

🎉 This PR is included in version 1.673.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀