-
Notifications
You must be signed in to change notification settings - Fork 16
feat(serenity): opt-in per-prompt userIntent on brand-presence prompts endpoint #2858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f5dfdf3
b6a354f
278007d
fa5bba8
901de87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| import { ELEMENT_IDS } from './element-ids.js'; | ||
| import { mapWithConcurrency } from './concurrency.js'; | ||
| import { splitDateRangeIntoWeeksBackward } from './week-utils.js'; | ||
| import { INTENT_VALUE } from '../serenity/prompt-tags.js'; | ||
| import { | ||
| buildBrandsPayload, | ||
| transformBrandsToFilterDimensions, | ||
|
|
@@ -28,6 +29,7 @@ import { | |
| transformWeeksResponse, | ||
| buildPromptsPayload, | ||
| transformPromptsResponse, | ||
| INTENT_ENRICH_CONCURRENCY, | ||
| buildCitedDomainsPayload, | ||
| transformCitedDomainsResponse, | ||
| buildSentimentOverviewPayload, | ||
|
|
@@ -60,8 +62,9 @@ const STATS_TRENDS_WEEK_CONCURRENCY = 4; | |
| * payload builders and response transformers. | ||
| * | ||
| * @param {object} transport - Elements transport created by createElementsTransport(). | ||
| * @param {object} [log] - Optional logger (`{ warn }`) for non-fatal degradation paths. | ||
| */ | ||
| export function createElementsService(transport) { | ||
| export function createElementsService(transport, log) { | ||
| return { | ||
| /** | ||
| * Fetches filter dimensions for the URL Inspector dashboard. | ||
|
|
@@ -135,17 +138,82 @@ export function createElementsService(transport) { | |
| /** | ||
| * Fetches the prompts matching the given filters, plus their count. | ||
| * | ||
| * When `params.enrichUserIntent` is set (and the query is scoped to exactly | ||
| * one `projectId` — see below), each returned row also carries its OWN intent | ||
| * (`userIntent`). The intent isn't a column on the PROMPTS element, so it's | ||
| * derived with one `intent__<value>`-filtered call per Semrush intent, run in | ||
| * parallel and joined back to the base rows. | ||
| * | ||
| * Enrichment is non-fatal per intent value: each call catches its own failure | ||
| * and contributes nothing, so one failing intent drops only that intent's rows. | ||
| * The base call still propagates on failure. Without the flag, response shape | ||
| * and upstream call count are unchanged. | ||
| * | ||
| * SINGLE-SLICE ONLY: the join key is `(prompt, prompt_topic)` — the strongest | ||
| * identifier the element row exposes (it carries no `semrushPromptId`, geo, or | ||
| * language). That tuple is unique only WITHIN one project (= one geo+language | ||
| * slice), so enrichment is skipped unless exactly one `projectId` is requested; | ||
| * across markets the same text could map to different intents and no row field | ||
| * could disambiguate it. (A stable per-row id from upstream would lift this.) | ||
| * | ||
| * @param {string} workspaceId - Semrush workspace UUID. | ||
| * @param {object} params - Filter parameters (model/platform, topics, projectIds). | ||
| * @param {object} params - Filter parameters (model/platform, tags, projectIds, | ||
| * and `enrichUserIntent`). | ||
| * @returns {Promise<{count: number, prompts: object[]}>} `{ count, prompts }`. | ||
| */ | ||
| async getPrompts(workspaceId, params) { | ||
| const raw = await transport.fetchElement( | ||
| workspaceId, | ||
| ELEMENT_IDS.PROMPTS, | ||
| buildPromptsPayload(params), | ||
| const { enrichUserIntent, ...promptParams } = params ?? {}; | ||
| const basePromise = transport | ||
| .fetchElement(workspaceId, ELEMENT_IDS.PROMPTS, buildPromptsPayload(promptParams)) | ||
| .then(transformPromptsResponse); | ||
|
|
||
| // Enrich only when opted in AND scoped to a single slice (see the join-key | ||
| // note above); otherwise return the base rows unchanged. | ||
| if (!enrichUserIntent || (promptParams.projectIds ?? []).length !== 1) { | ||
| return basePromise; | ||
| } | ||
|
|
||
| // `(prompt, prompt_topic)` join key — unique within the single requested slice. | ||
| const rowKey = (row) => `${row?.prompt ?? ''} ${row?.prompt_topic ?? ''}`; | ||
|
|
||
| // Base call + one intent-filtered call per intent value, in parallel | ||
| // (~one extra round-trip). Each intent call degrades independently. | ||
| const intentPromise = mapWithConcurrency( | ||
| Object.values(INTENT_VALUE), | ||
| INTENT_ENRICH_CONCURRENCY, | ||
| async (value) => { | ||
| const key = value.toLowerCase(); | ||
| try { | ||
| const raw = await transport.fetchElement( | ||
| workspaceId, | ||
| ELEMENT_IDS.PROMPTS, | ||
| buildPromptsPayload({ ...promptParams, tags: [...(promptParams.tags ?? []), `intent__${value}`] }), | ||
| ); | ||
| return { key, rows: transformPromptsResponse(raw).prompts }; | ||
| } catch (e) { | ||
| log?.warn?.(`serenity userIntent enrichment: intent-filtered PROMPTS call failed for '${value}'`, { workspaceId, error: e?.message }); | ||
| return { key, rows: [] }; | ||
| } | ||
| }, | ||
| ); | ||
| return transformPromptsResponse(raw); | ||
|
|
||
| const [base, intentResults] = await Promise.all([basePromise, intentPromise]); | ||
|
|
||
| // (prompt, prompt_topic) → own intent. A prompt carries exactly one intent | ||
| // tag, so within a single slice it appears in at most one filtered result. | ||
| const intentByRow = new Map(); | ||
| for (const { key, rows } of intentResults) { | ||
| for (const row of rows) { | ||
| intentByRow.set(rowKey(row), key); | ||
| } | ||
| } | ||
|
|
||
| const prompts = base.prompts.map((p) => ({ | ||
| ...p, | ||
| userIntent: intentByRow.get(rowKey(p)) ?? '', | ||
| })); | ||
| // `count` mirrors the base response (currently equals the row count). | ||
| return { count: base.count, prompts }; | ||
| }, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (blocking): The non-enriched path returns Fix: |
||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (blocking): The
.catch(() => [])wraps the entiremapWithConcurrencypromise. IfmapWithConcurrencyrejects 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:
Then remove the outer
.catch(() => []). This also adds the logging visibility that the current silent catch lacks.