From c312e15ec6127a791c4b042094a6c85724fd1b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Thu, 9 Jul 2026 15:16:47 +0200 Subject: [PATCH 01/16] feat(serenity): server-compute intent: on every prompt write path Removes the manual Intent picker requirement by computing intent: server-side via an LLM call on every Serenity/Semrush prompt write (manual create, edit, AI-generation) -- the intent companion to the already-merged type:branded work (#2772). Mirrors that same unified-layer pattern: a shared classifier factory parameterized with a Serenity-specific 5-value taxonomy (existing DRS 6-bucket behavior byte-for-byte unchanged), a request-scoped write-budget with a single budget-gated retry and hard skip-gate, and a terminal Informational default so a write never fails on classification. serenity-docs#32. Co-Authored-By: Claude Sonnet 5 --- src/controllers/serenity.js | 15 ++ src/support/intent-classifier.js | 72 ++++++-- src/support/serenity/brand-provisioning.js | 1 + .../serenity/handlers/markets-subworkspace.js | 41 ++++- .../serenity/handlers/prompts-subworkspace.js | 28 ++- src/support/serenity/handlers/prompts.js | 81 ++++++++- src/support/serenity/handlers/tags.js | 38 ++++ src/support/serenity/intent-classification.js | 166 ++++++++++++++++++ src/support/serenity/intent-taxonomy.js | 120 +++++++++++++ test/controllers/serenity.test.js | 2 + .../serenity/brand-provisioning.test.js | 1 + .../handlers/markets-subworkspace.test.js | 10 +- .../handlers/prompts-subworkspace.test.js | 25 +-- .../support/serenity/handlers/prompts.test.js | 69 +++++--- 14 files changed, 599 insertions(+), 70 deletions(-) create mode 100644 src/support/serenity/intent-classification.js create mode 100644 src/support/serenity/intent-taxonomy.js diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index 929e6744a2..cf417b7d21 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -67,6 +67,7 @@ import { MAX_TOPICS_ON_CREATE } from '../support/serenity/brand-provisioning.js' import { STANDARD_PROMPT_TAGS, PROJECT_STANDARD_TAGS } from '../support/serenity/prompt-tags.js'; import { marketForGeoTargetId } from '../support/serenity/locations.js'; import { brandNeedles, classifyBrandedTag } from '../support/serenity/branded-classifier.js'; +import { computeWriteDeadline } from '../support/serenity/intent-classification.js'; import AccessControlUtil from '../support/access-control-util.js'; import { resolveBrandUuid } from '../support/prompts-storage.js'; import { @@ -495,6 +496,9 @@ function SerenityController(context, log, env) { } const transport = buildTransport(ctx, imsToken); const classifyPromptType = await buildPromptTypeClassifier(ctx, auth.brandUuid); + // serenity-docs#32: one shared write-budget deadline for classify + create + // + publish, computed once at request entry. + const writeDeadline = computeWriteDeadline(); const result = auth.mode === 'subworkspace' ? await handleCreatePromptsSubworkspace( transport, @@ -502,6 +506,8 @@ function SerenityController(context, log, env) { ctx.data || {}, log, classifyPromptType, + ctx.env, + writeDeadline, ) : await handleCreatePrompts( transport, @@ -511,6 +517,8 @@ function SerenityController(context, log, env) { ctx.data || {}, log, classifyPromptType, + ctx.env, + writeDeadline, ); return createResponse(result, 200); } catch (e) { @@ -531,6 +539,7 @@ function SerenityController(context, log, env) { } const transport = buildTransport(ctx, imsToken); const classifyPromptType = await buildPromptTypeClassifier(ctx, auth.brandUuid); + const writeDeadline = computeWriteDeadline(); const result = auth.mode === 'subworkspace' ? await handleUpdatePromptSubworkspace( transport, @@ -539,6 +548,8 @@ function SerenityController(context, log, env) { ctx.data || {}, log, classifyPromptType, + ctx.env, + writeDeadline, ) : await handleUpdatePrompt( transport, @@ -549,6 +560,8 @@ function SerenityController(context, log, env) { ctx.data || {}, log, classifyPromptType, + ctx.env, + writeDeadline, ); return createResponse(result.body, result.status); } catch (e) { @@ -685,6 +698,7 @@ function SerenityController(context, log, env) { brandAliases, brandUrlSources, competitors, + env: ctx.env, // auth.brandUuid is an already-persisted brand row here (loadBrand // above), so the mapping-row upsert's FK to brands is satisfied — // see mapping-rows.js upsertMappingRow doc. @@ -1211,6 +1225,7 @@ function SerenityController(context, log, env) { brandAliases, brandUrlSources, competitors, + env: ctx.env, // `brand` was loaded via loadBrand above — an already-persisted // row, so the mapping-row upsert's FK to brands is satisfied. // Narrowed to the one model the mapping-row helpers touch — see diff --git a/src/support/intent-classifier.js b/src/support/intent-classifier.js index 51858e483d..d1d5271dca 100644 --- a/src/support/intent-classifier.js +++ b/src/support/intent-classifier.js @@ -186,12 +186,14 @@ export function contentToString(content) { /** * Extracts a JSON object from a model response that may include code fences or - * surrounding prose, then returns its normalized `intent` (or null). + * surrounding prose, then returns the parsed object (or null on any parse + * failure). Shared by every category spec — none of this extraction logic is + * taxonomy-specific. * * @param {string} content - Raw model output - * @returns {string|null} + * @returns {object|null} */ -function parseIntent(content) { +function parseModelJson(content) { if (!hasText(content)) { return null; } @@ -208,20 +210,58 @@ function parseIntent(content) { [raw] = brace; } try { - const parsed = JSON.parse(raw); - // normalizeIntent lowercases, applies the legacy remap, and validates - // against the 6 canonical buckets (else null) — single source of truth. - return normalizeIntent(parsed?.intent); + return JSON.parse(raw); } catch { return null; } } /** - * Creates an intent classifier bound to the Azure OpenAI credentials in `env`. + * The native/DRS 6-bucket category spec — the classifier's default + * configuration, reproducing today's behavior byte-for-byte (it reuses the + * existing {@link SYSTEM_PROMPT} export as-is, so there is nothing new to keep + * in sync). `parseResult` discards `confidence`, matching the pre-existing + * `parseIntent` behavior. + * + * @typedef {object} CategorySpec + * @property {string} systemPrompt - the system prompt for this taxonomy. + * @property {(parsed: object) => (string|null)} parseResult - validates + + * normalizes the parsed JSON body into a canonical value, or null. + * @property {number} [invokeTimeoutMs] - per-call timeout override; falls back + * to {@link resolveInvokeTimeoutMs} (env-driven) when omitted. + */ + +/** @type {CategorySpec} */ +export const DRS_CATEGORY_SPEC = Object.freeze({ + systemPrompt: SYSTEM_PROMPT, + // normalizeIntent lowercases, applies the legacy remap, and validates + // against the 6 canonical buckets (else null) — single source of truth. + parseResult: (parsed) => normalizeIntent(parsed?.intent), +}); + +/** + * Resolves the Azure OpenAI deployment (model) name from env, allowing a + * classifier-scoped override (`PROMPT_INTENT_CLASSIFICATION_DEPLOYMENT_NAME`) + * to take precedence over the shared per-env deployment used by other Azure + * OpenAI consumers (e.g. `org-detector`). Falls back to the shared deployment + * when the override is unset, so existing behavior is unchanged until the + * override is explicitly configured. + * + * @param {object} env - Environment variables + * @returns {string|undefined} + */ +function resolveDeploymentName(env = {}) { + return env.PROMPT_INTENT_CLASSIFICATION_DEPLOYMENT_NAME + || env.AZURE_OPEN_AI_API_DEPLOYMENT_NAME; +} + +/** + * Creates an intent classifier bound to the Azure OpenAI credentials in `env`, + * for the given taxonomy (`categorySpec`). Defaults to {@link DRS_CATEGORY_SPEC} + * so existing single-arg callers are unaffected. * * Returns a function `classify(text) => Promise` that is always - * best-effort: it resolves to a canonical bucket on success, or `null` on any + * best-effort: it resolves to a canonical value on success, or `null` on any * failure or when text/credentials are missing. It NEVER rejects. * * Classification runs whenever Azure OpenAI is configured. When the Azure @@ -231,17 +271,19 @@ function parseIntent(content) { * @param {object} context - Helix universal context * @param {object} context.env - Environment variables (Azure OpenAI creds) * @param {object} [context.log] - Logger + * @param {CategorySpec} [categorySpec] - the taxonomy to classify into; + * defaults to the native 6-bucket DRS taxonomy. * @returns {((text: string) => Promise)|null} */ -export function createIntentClassifier(context = {}) { +export function createIntentClassifier(context = {}, categorySpec = DRS_CATEGORY_SPEC) { const { env = {}, log = console } = context; const { AZURE_OPEN_AI_API_KEY: azureOpenAIApiKey, AZURE_OPEN_AI_API_INSTANCE_NAME: azureOpenAIApiInstanceName, - AZURE_OPEN_AI_API_DEPLOYMENT_NAME: azureOpenAIApiDeploymentName, AZURE_OPEN_AI_API_VERSION: azureOpenAIApiVersion, } = env; + const azureOpenAIApiDeploymentName = resolveDeploymentName(env); if (!hasText(azureOpenAIApiKey) || !hasText(azureOpenAIApiInstanceName) @@ -266,7 +308,8 @@ export function createIntentClassifier(context = {}) { return null; } - const invokeTimeoutMs = resolveInvokeTimeoutMs(env); + const invokeTimeoutMs = categorySpec.invokeTimeoutMs ?? resolveInvokeTimeoutMs(env); + const { systemPrompt, parseResult } = categorySpec; return async function classify(text) { // Mirror DRS: skip empty / whitespace-only text (no LLM call, intent null). @@ -279,14 +322,15 @@ export function createIntentClassifier(context = {}) { // On timeout this rejects and falls through to the non-fatal catch below. const response = await withTimeout( model.invoke([ - { role: 'system', content: SYSTEM_PROMPT }, + { role: 'system', content: systemPrompt }, { role: 'user', content: trimmed.slice(0, MAX_PROMPT_CHARS) }, ]), invokeTimeoutMs, ); // content may be a string OR an array of content parts; coerce either way. const content = contentToString(response?.content); - return parseIntent(content); + const parsed = parseModelJson(content); + return parsed ? parseResult(parsed) : null; } catch (e) { log.warn(`Intent classification failed for prompt; persisting null: ${e.message}`); return null; diff --git a/src/support/serenity/brand-provisioning.js b/src/support/serenity/brand-provisioning.js index c25b52d546..c259f75efc 100644 --- a/src/support/serenity/brand-provisioning.js +++ b/src/support/serenity/brand-provisioning.js @@ -198,6 +198,7 @@ export async function provisionBrandSubworkspace(context, { standardTags: [...STANDARD_PROMPT_TAGS], brandAliases, projectTags: [...PROJECT_STANDARD_TAGS], + env: context.env, brandUrlSources, competitors, // A project with neither models nor generated prompts would publish diff --git a/src/support/serenity/handlers/markets-subworkspace.js b/src/support/serenity/handlers/markets-subworkspace.js index 495b722d30..171f1e1333 100644 --- a/src/support/serenity/handlers/markets-subworkspace.js +++ b/src/support/serenity/handlers/markets-subworkspace.js @@ -34,8 +34,9 @@ import { listMarkets, resolveProject, mapPublishStatus, projectToSlice, } from '../subworkspace-projects.js'; import { ensureSubworkspace } from '../workspace-lifecycle.js'; -import { topicTag } from '../prompt-tags.js'; +import { topicTag, TAG_DIMENSION, INTENT_TAG } from '../prompt-tags.js'; import { classifyBrandedTag, needlesFromNames } from '../branded-classifier.js'; +import { classifyPromptIntents, AI_GEN_CLASSIFY_MAX, computeWriteDeadline } from '../intent-classification.js'; import { collectBrandUrlEntries, attachBrandUrlsToProject } from '../brand-urls.js'; import { buildReservedDomains, syncCompetitorBenchmarksForProject } from '../competitor-benchmarks.js'; import { collectAliasNames } from '../brand-aliases.js'; @@ -189,10 +190,14 @@ function validateCreateBody(body) { * @param {string[]} [options.brandNames=[]] - brand name + aliases for branded * classification via the shared {@link classifyBrandedTag} (whole-word match, * diacritic-folded, case-insensitive). + * @param {object} [options.env] - environment (Azure OpenAI creds), for intent + * classification (serenity-docs#32). + * @param {number} [options.writeDeadline] - shared request-write deadline. * @param {object} log - logger. */ async function generateAndAttachPrompts(transport, workspaceId, projectId, { - domain, country, topicCap = 0, standardTags = [], brandNames = [], + domain, country, topicCap = 0, standardTags = [], brandNames = [], env, + writeDeadline = computeWriteDeadline(), }, log) { const raw = await transport.getBrandTopics(workspaceId, { domain, country }); let topics = []; @@ -212,12 +217,34 @@ async function generateAndAttachPrompts(transport, workspaceId, projectId, { // so a prompt is classified identically no matter how it is written. const needles = needlesFromNames(Array.isArray(brandNames) ? brandNames : []); + // Strip any intent:* tag out of the caller's standardTags (today it's always + // the static seeded default, INTENT_TAG.INFORMATIONAL from STANDARD_PROMPT_TAGS) + // — it gets replaced per-prompt by the real classification below, capped at + // AI_GEN_CLASSIFY_MAX under the shared request deadline (serenity-docs#32). + const strippedStandardTags = standardTags + .filter((t) => !String(t).startsWith(`${TAG_DIMENSION.INTENT}:`)); + const allTexts = []; + selected.forEach((t) => { + (Array.isArray(t.prompts) ? t.prompts : []).forEach((p) => { + if (hasText(p) && !allTexts.includes(p)) { + allTexts.push(p); + } + }); + }); + const intentByText = await classifyPromptIntents( + allTexts.slice(0, AI_GEN_CLASSIFY_MAX), + { env, log, deadline: writeDeadline }, + ); + const promptsByText = {}; selected.forEach((t) => { const topic = topicTag(t.topic); (Array.isArray(t.prompts) ? t.prompts : []).forEach((p) => { if (hasText(p)) { - promptsByText[p] = [topic, ...standardTags, classifyBrandedTag(p, needles)]; + const intentTag = intentByText.get(p) ?? INTENT_TAG.INFORMATIONAL; + promptsByText[p] = [ + topic, ...strippedStandardTags, intentTag, classifyBrandedTag(p, needles), + ]; } }); }); @@ -294,6 +321,10 @@ async function generateAndAttachPrompts(transport, workspaceId, projectId, { * `brand_to_semrush_projects` mapping row for this project (best-effort, * never fails the create). Omit for a `brand` that is not yet a persisted * row — see `mapping-rows.js` `upsertMappingRow` doc. + * @param {object} [options.env] - environment (Azure OpenAI creds), threaded + * into intent classification when `generateTopics` is set (serenity-docs#32). + * @param {number} [options.writeDeadline] - shared request-write deadline; + * defaults to a fresh {@link computeWriteDeadline} when omitted. */ export async function handleCreateMarketSubworkspace( transport, @@ -314,6 +345,8 @@ export async function handleCreateMarketSubworkspace( competitors = [], publishMode = 'require', dataAccess = null, + env = null, + writeDeadline = computeWriteDeadline(), } = {}, ) { const errors = validateCreateBody(body); @@ -414,6 +447,8 @@ export async function handleCreateMarketSubworkspace( ...(Array.isArray(body.brandNames) ? body.brandNames : []), ...aliasNames, ], + env, + writeDeadline, }, log, ); diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index d144d343e7..67f4c4c25a 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -23,6 +23,7 @@ import { normalizePromptInput, createOnePrompt, makeTypeInjector, + makeIntentInjector, parseUpdatePromptBody, mapLimit, publishAffected, @@ -34,6 +35,7 @@ import { } from './prompts.js'; import { resolveProject, buildSliceProjectMap, sliceKey } from '../subworkspace-projects.js'; import { redactUpstreamMessage } from '../rest-transport.js'; +import { classifyPromptIntents } from '../intent-classification.js'; /** * Subworkspace-mode prompt handlers (serenity dual-mode, subworkspace path). Behaviourally @@ -123,6 +125,8 @@ export async function handleCreatePromptsSubworkspace( body, log, classifyPromptType, + env, + writeDeadline, ) { const inputs = Array.isArray(body?.prompts) ? body.prompts : []; if (inputs.length === 0) { @@ -137,6 +141,11 @@ export async function handleCreatePromptsSubworkspace( const projectsBySlice = await buildSliceProjectMap(transport, workspaceId, log); const injectComputedType = makeTypeInjector(transport, workspaceId, classifyPromptType, log); + const intentByText = await classifyPromptIntents( + inputs.map((raw) => String(raw?.text || '')), + { env, log, deadline: writeDeadline }, + ); + const injectComputedIntent = makeIntentInjector(transport, workspaceId, intentByText, log); const results = await mapLimit(inputs, BULK_CREATE_CONCURRENCY, async (raw) => { const input = normalizePromptInput(raw); @@ -159,8 +168,9 @@ export async function handleCreatePromptsSubworkspace( } const projectId = String(project.id); try { - // Unified layer: strip any caller-supplied type + inject the computed one. - const typed = await injectComputedType(projectId, input); + // Unified layer: strip any caller-supplied type/intent + inject the computed ones. + let typed = await injectComputedType(projectId, input); + typed = await injectComputedIntent(projectId, typed); const semrushPromptId = await createOnePrompt(transport, workspaceId, projectId, typed); return { created: { @@ -227,6 +237,8 @@ export async function handleUpdatePromptSubworkspace( body, log, classifyPromptType, + env, + writeDeadline, ) { const parsedBody = parseUpdatePromptBody(body); if (!parsedBody.ok) { @@ -257,12 +269,18 @@ export async function handleUpdatePromptSubworkspace( } const projectId = String(project.id); - // Recompute the type tag from the NEW text BEFORE the delete (see the flat-mode - // twin): the unified layer must not run between delete and create. + // Recompute the type AND intent tags from the NEW text BEFORE the delete (see + // the flat-mode twin): the unified layer must not run between delete and create. const injectComputedType = makeTypeInjector(transport, workspaceId, classifyPromptType, log); - const typed = await injectComputedType(projectId, { + const intentByText = await classifyPromptIntents( + [nextText], + { env, log, deadline: writeDeadline }, + ); + const injectComputedIntent = makeIntentInjector(transport, workspaceId, intentByText, log); + let typed = await injectComputedType(projectId, { text: nextText, geoTargetId, tags: nextTags, tagIds: nextTagIds, }); + typed = await injectComputedIntent(projectId, typed); try { await transport.deletePromptsByIds(workspaceId, projectId, [semrushPromptId]); diff --git a/src/support/serenity/handlers/prompts.js b/src/support/serenity/handlers/prompts.js index 72bdd5f9b0..7717e15e6e 100644 --- a/src/support/serenity/handlers/prompts.js +++ b/src/support/serenity/handlers/prompts.js @@ -19,8 +19,9 @@ import { redactUpstreamMessage } from '../rest-transport.js'; import { ERROR_CODES, isUpstreamGone } from '../errors.js'; import { normalizeGeoTargetId, normalizeLanguageCode, isValidTagIdFormat } from '../validation.js'; import { invalidateTagCacheForProject } from './markets.js'; -import { resolveTypeTagInjection } from './tags.js'; -import { TAG_DIMENSION } from '../prompt-tags.js'; +import { resolveTypeTagInjection, resolveIntentTagInjection } from './tags.js'; +import { TAG_DIMENSION, INTENT_TAG } from '../prompt-tags.js'; +import { classifyPromptIntents } from '../intent-classification.js'; // TWIN FILE: the slice→project orchestration here is paralleled by the // subworkspace-mode handlers in prompts-subworkspace.js. The duplication is @@ -336,6 +337,50 @@ export function makeTypeInjector(transport, semrushWorkspaceId, classifyPromptTy }; } +/** + * Applies a pre-computed, per-request `intent:*` classification map to a + * prompt write (serenity-docs#32). Unlike {@link makeTypeInjector}, the + * "compute the tag" step is a `Map` lookup, not a per-item classify call — + * intent is batch-classified ONCE per request (see `classifyPromptIntents` in + * `../intent-classification.js`) since it's an LLM call, not a cheap pure + * function. A text missing from `intentByText` (e.g. beyond the AI-gen classify + * cap) falls back to `INTENT_TAG.INFORMATIONAL`, the seeded standard value. + * + * Id-based resolution ({@link resolveIntentTagInjection}, one tag-tree read per + * distinct `intent:` value per project) is memoized for the request, mirroring + * {@link makeTypeInjector}'s memoization. + * + * @param {object} transport - Serenity transport (Semrush proxy client). + * @param {string} semrushWorkspaceId + * @param {Map} intentByText - text -> `intent:` wire tag. + * @param {object} [log] + * @returns {(projectId: string, input: { text: string, geoTargetId: number, + * tags: string[], tagIds: string[] | undefined }) => + * Promise<{ text: string, geoTargetId: number, tags: string[], + * tagIds: string[] | undefined }>} + */ +export function makeIntentInjector(transport, semrushWorkspaceId, intentByText, log) { + /** @type {Map>} */ + const cache = new Map(); + return async function injectComputedIntent(projectId, input) { + const intentTag = intentByText.get(input.text) ?? INTENT_TAG.INFORMATIONAL; + if (input.tagIds === undefined) { + const stripped = (Array.isArray(input.tags) ? input.tags : []) + .filter((t) => !String(t).startsWith(`${TAG_DIMENSION.INTENT}:`)); + return { ...input, tags: [...stripped, intentTag] }; + } + const key = `${projectId} ${intentTag}`; + let pending = cache.get(key); + if (!pending) { + pending = resolveIntentTagInjection(transport, semrushWorkspaceId, projectId, intentTag, log); + cache.set(key, pending); + } + const { computedId, intentTagIds } = await pending; + const stripped = input.tagIds.filter((id) => !intentTagIds.includes(id)); + return { ...input, tagIds: computedId ? [...stripped, computedId] : stripped }; + }; +} + /** * Validates + normalizes a PATCH prompt body's `text`/`tags`/`tagIds`, shared * by {@link handleUpdatePrompt} and its subworkspace twin. `tags` (names) and @@ -425,6 +470,8 @@ export async function handleCreatePrompts( body, log, classifyPromptType, + env, + writeDeadline, ) { const inputs = Array.isArray(body?.prompts) ? body.prompts : []; if (inputs.length === 0) { @@ -449,6 +496,14 @@ export async function handleCreatePrompts( classifyPromptType, log, ); + // Unified layer (serenity-docs#32): batch-classify every distinct text ONCE + // under the shared request deadline, then thread the resolved map into each + // per-item injection below (a per-item LLM call would be far too slow). + const intentByText = await classifyPromptIntents( + inputs.map((raw) => String(raw?.text || '')), + { env, log, deadline: writeDeadline }, + ); + const injectComputedIntent = makeIntentInjector(transport, semrushWorkspaceId, intentByText, log); const results = await mapLimit(inputs, BULK_CREATE_CONCURRENCY, async (raw) => { const input = normalizePromptInput(raw); @@ -471,8 +526,9 @@ export async function handleCreatePrompts( } const projectId = project.getSemrushProjectId(); try { - // Unified layer: strip any caller-supplied type + inject the computed one. - const typed = await injectComputedType(projectId, input); + // Unified layer: strip any caller-supplied type/intent + inject the computed ones. + let typed = await injectComputedType(projectId, input); + typed = await injectComputedIntent(projectId, typed); const semrushPromptId = await createOnePrompt( transport, semrushWorkspaceId, @@ -583,6 +639,8 @@ export async function handleUpdatePrompt( body, log, classifyPromptType, + env, + writeDeadline, ) { // `semrushPromptId` is validated as non-empty at the controller boundary // (serenity.js:259) before this handler is invoked over HTTP, so no @@ -620,18 +678,25 @@ export async function handleUpdatePrompt( } const projectId = project.getSemrushProjectId(); - // Recompute the type tag from the NEW text BEFORE the delete: the unified layer - // (tree read / on-demand tag create) must not run between delete and create, - // so a classification failure aborts cleanly with the old prompt still present. + // Recompute the type AND intent tags from the NEW text BEFORE the delete: the + // unified layer (tree read / on-demand tag create / LLM classify) must not run + // between delete and create, so a classification failure aborts cleanly with + // the old prompt still present (serenity-docs#31, #32). const injectComputedType = makeTypeInjector( transport, semrushWorkspaceId, classifyPromptType, log, ); - const typed = await injectComputedType(projectId, { + const intentByText = await classifyPromptIntents( + [nextText], + { env, log, deadline: writeDeadline }, + ); + const injectComputedIntent = makeIntentInjector(transport, semrushWorkspaceId, intentByText, log); + let typed = await injectComputedType(projectId, { text: nextText, geoTargetId, tags: nextTags, tagIds: nextTagIds, }); + typed = await injectComputedIntent(projectId, typed); try { await transport.deletePromptsByIds(semrushWorkspaceId, projectId, [semrushPromptId]); diff --git a/src/support/serenity/handlers/tags.js b/src/support/serenity/handlers/tags.js index 1c9e160d51..05a677baf6 100644 --- a/src/support/serenity/handlers/tags.js +++ b/src/support/serenity/handlers/tags.js @@ -332,6 +332,44 @@ export async function resolveTypeTagInjection( return { computedId: id, typeTagIds: id ? [...typeTagIds, id] : typeTagIds }; } +/** + * Resolves the id-based injection of a server-computed `intent:*` tag into a + * prompt write (serenity-docs#32). Same shape as {@link resolveTypeTagInjection} + * with `TAG_DIMENSION.INTENT` substituted for `TAG_DIMENSION.TYPE` — closed + * dimensions share the same resolve-or-create-on-demand semantics. + * + * @param {object} transport - Serenity transport (Semrush proxy client). + * @param {string} semrushWorkspaceId + * @param {string} projectId + * @param {string} wantTag - the computed `intent:` wire name. + * @param {object} [log] - logger. + * @returns {Promise<{ computedId: string | undefined, intentTagIds: string[] }>} + * `computedId` is the wanted value's id; `intentTagIds` is every `intent:` + * root id present after resolution (the strip set). + */ +export async function resolveIntentTagInjection( + transport, + semrushWorkspaceId, + projectId, + wantTag, + log, +) { + const roots = await listProjectTagTree(transport, semrushWorkspaceId, projectId, '', log); + const intentRoots = roots.items.filter( + (t) => typeof t.name === 'string' && t.name.startsWith(`${TAG_DIMENSION.INTENT}:`), + ); + const intentTagIds = intentRoots.map((t) => t.id).filter(Boolean); + const existing = intentRoots.find((t) => t.name === wantTag); + if (existing) { + return { computedId: existing.id, intentTagIds }; + } + // Create-on-demand for the wanted `intent:` root — same idempotency + // reasoning as `resolveTypeTagInjection` above. + const createdList = await transport.createProjectTags(semrushWorkspaceId, projectId, [wantTag]); + const { id } = pickTagIds(createdList, undefined); + return { computedId: id, intentTagIds: id ? [...intentTagIds, id] : intentTagIds }; +} + /** * Flat mode — the market's project id comes from the persisted * `BrandSemrushProject` mapping (same resolution as handleListTags). diff --git a/src/support/serenity/intent-classification.js b/src/support/serenity/intent-classification.js new file mode 100644 index 0000000000..60cc10d2dd --- /dev/null +++ b/src/support/serenity/intent-classification.js @@ -0,0 +1,166 @@ +/* + * Copyright 2026 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +// @ts-check + +import { createIntentClassifier, classifyIntents } from '../intent-classifier.js'; +import { INTENT_TAG } from './prompt-tags.js'; +import { SERENITY_INTENT_CATEGORY_SPEC, PER_CALL_MS } from './intent-taxonomy.js'; + +/** + * Serenity write-path request budget (serenity-docs#32) — a single shared + * deadline so classify + its budget-gated retry + create-fanout + publish + * together stay under the ~15s Fastly first-byte timeout the write already + * shares. `WRITE_SOFT_BUDGET_MS` leaves ~3s margin; `CREATE_PUBLISH_RESERVE_MS` + * is reserved for the write itself, so classification only gets the remainder. + */ +export const WRITE_SOFT_BUDGET_MS = 12000; +export const CREATE_PUBLISH_RESERVE_MS = 6000; + +// Classify concurrency. Deliberately a local constant rather than importing +// `BULK_CREATE_CONCURRENCY` from `./handlers/prompts.js` (which would create a +// circular import, since that module imports from here) — by design decision +// (serenity-docs#32) it currently mirrors that value. +const CLASSIFY_CONCURRENCY = 8; + +// AI-generation classify cap (serenity-docs#32, resolved 2026-07-09): 2 budget +// rounds (one call + one budget-gated retry) at CLASSIFY_CONCURRENCY each. +export const AI_GEN_CLASSIFY_MAX = CLASSIFY_CONCURRENCY * 2; + +/** + * Computes the request-scoped write deadline. Call once at controller entry, + * before any classify/create/publish work begins. + * + * @param {number} [now] - override for testing. + * @returns {number} epoch ms deadline. + */ +export function computeWriteDeadline(now = Date.now()) { + return now + WRITE_SOFT_BUDGET_MS; +} + +/** + * Remaining budget (ms) available for classification: whatever is left on the + * deadline, minus the reserve for create-fanout + publish. Never negative. + * + * @param {number} deadline - epoch ms deadline from {@link computeWriteDeadline}. + * @param {number} [now] - override for testing. + * @returns {number} + */ +function remainingClassifyBudget(deadline, now = Date.now()) { + return Math.max(0, (deadline - now) - CREATE_PUBLISH_RESERVE_MS); +} + +/** + * Batch-classifies prompt texts into `intent:` wire tags under the + * shared write-budget, applying the full fallback ladder from serenity-docs#32: + * + * 1. Hard skip-gate — if there's no room for even one call at entry, default + * everything immediately (no LLM calls attempted, counted `budget_skipped`). + * 2. Azure not configured — default everything (same in every environment; a + * `warn` + `prod_llm_unavailable` signal only in prod, `info` elsewhere). + * 3. First classify pass under the remaining budget. + * 4. One budget-gated retry for anything still unresolved — only if there's + * still room for a full per-call timeout; otherwise skip straight to default. + * 5. Terminal default `intent:Informational` for anything still unresolved. + * + * Every input text is guaranteed a value in the returned map — never missing — + * so callers never need a separate "no tag" branch. + * + * @param {string[]} texts - prompt texts to classify (deduplicated internally). + * @param {object} options + * @param {object} [options.env] - environment (Azure OpenAI creds). + * @param {object} [options.log] - logger. + * @param {number} options.deadline - the request's write deadline (epoch ms). + * @returns {Promise>} text -> `intent:` wire tag. + */ +export async function classifyPromptIntents(texts, { env, log = console, deadline }) { + // `env`/`log` may arrive explicitly `null` (some callers default optional + // options that way) — a default *parameter* only applies on `undefined`, so + // normalize here rather than relying on destructuring defaults. + const safeEnv = env || {}; + const safeLog = log || console; + const unique = [...new Set((texts || []).filter((t) => typeof t === 'string' && t.length > 0))]; + const result = new Map(); + if (unique.length === 0) { + return result; + } + + const counts = { + classified_ok: 0, retry_attempted: 0, retry_succeeded: 0, defaulted: 0, budget_skipped: 0, + }; + + const defaultAll = (list) => { + list.forEach((t) => result.set(t, INTENT_TAG.INFORMATIONAL)); + counts.defaulted += list.length; + }; + + if (remainingClassifyBudget(deadline) < PER_CALL_MS) { + counts.budget_skipped = unique.length; + defaultAll(unique); + safeLog?.info?.('serenity intent classification: budget_skipped (no room at entry)', { count: unique.length }); + return result; + } + + const classify = createIntentClassifier( + { env: safeEnv, log: safeLog }, + SERENITY_INTENT_CATEGORY_SPEC, + ); + if (typeof classify !== 'function') { + defaultAll(unique); + const message = 'serenity intent classification: Azure OpenAI is not configured; defaulting to Informational'; + if (safeEnv.AWS_ENV === 'prod') { + safeLog?.warn?.(`WARN: prod_llm_unavailable — ${message}`); + } else { + safeLog?.info?.(message); + } + return result; + } + + const firstPass = await classifyIntents(classify, unique, { + maxConcurrency: CLASSIFY_CONCURRENCY, + timeoutMs: remainingClassifyBudget(deadline), + }); + const stillUnresolved = []; + unique.forEach((t) => { + const value = firstPass.get(t); + if (value) { + result.set(t, value); + counts.classified_ok += 1; + } else { + stillUnresolved.push(t); + } + }); + + if (stillUnresolved.length > 0 && remainingClassifyBudget(deadline) >= PER_CALL_MS) { + counts.retry_attempted = stillUnresolved.length; + const retryPass = await classifyIntents(classify, stillUnresolved, { + maxConcurrency: CLASSIFY_CONCURRENCY, + timeoutMs: remainingClassifyBudget(deadline), + }); + const stillUnresolvedAfterRetry = []; + stillUnresolved.forEach((t) => { + const value = retryPass.get(t); + if (value) { + result.set(t, value); + counts.retry_succeeded += 1; + } else { + stillUnresolvedAfterRetry.push(t); + } + }); + defaultAll(stillUnresolvedAfterRetry); + } else { + defaultAll(stillUnresolved); + } + + safeLog?.info?.('serenity intent classification summary', counts); + return result; +} diff --git a/src/support/serenity/intent-taxonomy.js b/src/support/serenity/intent-taxonomy.js new file mode 100644 index 0000000000..1c0fe0c07c --- /dev/null +++ b/src/support/serenity/intent-taxonomy.js @@ -0,0 +1,120 @@ +/* + * Copyright 2026 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +// @ts-check + +import { tagFor, TAG_DIMENSION } from './prompt-tags.js'; + +/** + * Serenity's 5-value intent taxonomy (serenity-docs#32) — the LLM-facing + * category spec consumed by `createIntentClassifier` from + * `../intent-classifier.js`. This is deliberately a SEPARATE taxonomy from the + * native/DRS 6-bucket one (`../intent.js`): `INTENT_REMAP` there collapses + * `navigational→informational` / `commercial→transactional`, so the DRS + * classifier structurally cannot emit two of these five values — the category + * set must be injected, not remapped. + */ + +// The 5 bare category words the model must emit verbatim (Capitalized, +// case-sensitive) — matches the existing `INTENT_TAG` wire vocabulary in +// `prompt-tags.js` exactly, so no taxonomy change is needed on the write path. +export const SERENITY_INTENT_VALUES = Object.freeze([ + 'Informational', + 'Task', + 'Commercial', + 'Transactional', + 'Navigational', +]); + +// Validity floor, NOT a quality gate (calibration finding: the model is +// uniformly over-confident regardless of correctness — gpt-4.1 mean 0.959, +// gpt-4.1-mini mean 0.935 across 60 calibration prompts). Only catches a rare +// garbled / genuinely-uncertain output. Fixed code constant, not env-tunable. +export const PROMPT_INTENT_MIN_CONFIDENCE = 0.5; + +// Per-call timeout for the Serenity classifier — fixed by the shared +// request-budget design (serenity-docs#32), not env-driven like the native +// classifier's `PROMPT_INTENT_CLASSIFICATION_TIMEOUT_MS`. +export const PER_CALL_MS = 3000; + +// Validated 2026-07-07 against 60 real Lovesac prompts on the real prod +// `gpt-4.1` deployment (temperature 0, max_tokens 150, JSON output) — see the +// calibration report on serenity-docs#32. All five values fire correctly; the +// delegation→Task disambiguation rule below was validated 5/5 unanimous across +// gpt-4.1 / gpt-4.1-mini / gpt-4.1-nano / gpt-4o. +export const SERENITY_INTENT_SYSTEM_PROMPT = `You are a user intent classifier for AI assistant queries about a brand. Classify the given prompt into exactly one of 5 categories. + +The prompt may be in any language (English, Portuguese, German, Spanish, etc.). Classify based on meaning, not language. + +Categories: + +1. **Informational** - The user wants to KNOW, UNDERSTAND, or DISCOVER something — general lookups, definitions, explanations, or open-ended "what/how" questions that are not about buying or being personally advised. + Examples: "What is Adobe Firefly?", "How does a heat pump work?", "What's the history of this brand?" + +2. **Task** - The user wants the AI to RECOMMEND, DECIDE, or PICK something FOR THEM personally, OR wants step-by-step guidance to do something themselves. This is a personal ask ("recommend for me", "help me choose", "how do I set this up") — a self-directed research question about which is objectively best ("best X", "X vs Y") is NOT Task, it is Commercial. + Examples: "Can you recommend a couch for my apartment?", "What would you suggest for a small living room?", "How do I set up a glossary in my CMS?", "Help me choose between these two options." + Disambiguation: "Can you recommend/suggest X for me?" = Task (delegation — the AI decides). "What is the best X?" or "X vs Y" = Commercial (the user is researching, not delegating the decision). + +3. **Commercial** - The user is researching options themselves — comparing, ranking, or evaluating products/services, including "best X", "top X", "X vs Y", or general product research NOT phrased as a personal ask. + Examples: "Best AI workspace tool", "Top PDF editors 2024", "Figma vs Sketch for UI design", "Most durable couch brands", "Which is better, Notion or Confluence?" + +4. **Transactional** - The user wants to BUY, DOWNLOAD, SIGN UP, or take a direct commercial action. + Examples: "Adobe Creative Cloud pricing", "Free trial for Photoshop", "Where to buy this brand's products", "Download PDF editor free" + +5. **Navigational** - The user is trying to reach a specific known destination (a brand's site, page, app, or account), not researching or deciding. + Examples: "Adobe.com login", "Open Photoshop web", "Brand's official Instagram page" + +Decision rules: +- "Can you recommend/suggest X for me?", "What would you recommend?", "Help me choose" = **Task** (delegation — the AI decides for the user). "What is the best X?", "Best X for Y", "X vs Y" = **Commercial** (self-directed research), even though both start similarly. +- Step-by-step "how do I do X myself" = **Task**. +- Default to **Informational** if ambiguous. + +Output requirements (strict): Reply with ONLY valid JSON. Response is limited to ~150 tokens, so keep it short. +Output format: {"intent": "", "confidence": 0.0-1.0, "reasoning": ""} +Do not include markdown, code fences, or any text outside the JSON object.`; + +/** + * Validates a parsed model response against the Serenity taxonomy: the value + * must be one of the 5 canonical Capitalized literals AND confidence must meet + * the validity floor. Returns the ready-to-use wire tag (e.g. `intent:Task`) on + * success, mirroring `classifyBrandedTag` returning a ready `TYPE_TAG.*` string + * rather than a bare value — or `null` on any validation failure (garbled + * output, an unrecognized value, or a below-floor confidence, all treated as + * the same "soft failure" by the caller's retry/default ladder). + * + * @param {object} parsed - the parsed `{intent, confidence, reasoning}` body. + * @returns {string|null} the wire tag (`intent:`) or null. + */ +export function parseSerenityIntent(parsed) { + const value = String(parsed?.intent ?? ''); + const confidence = Number(parsed?.confidence); + if (!SERENITY_INTENT_VALUES.includes(value)) { + return null; + } + if (!Number.isFinite(confidence) || confidence < PROMPT_INTENT_MIN_CONFIDENCE) { + return null; + } + return tagFor(TAG_DIMENSION.INTENT, value); +} + +/** + * The Serenity category spec passed to `createIntentClassifier` (see + * `../intent-classifier.js`) for every Serenity write-path classification. + * + * @type {{ systemPrompt: string, invokeTimeoutMs: number, + * parseResult: (parsed: object) => (string|null) }} + */ +export const SERENITY_INTENT_CATEGORY_SPEC = Object.freeze({ + systemPrompt: SERENITY_INTENT_SYSTEM_PROMPT, + parseResult: parseSerenityIntent, + invokeTimeoutMs: PER_CALL_MS, +}); diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index f5681e4472..f536da4c55 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -1262,6 +1262,7 @@ describe('SerenityController', () => { brandAliases: ['Acme Inc', 'ACME'], brandUrlSources: { urls: [], socialAccounts: [], earnedContent: [] }, competitors: [], + env: {}, dataAccess: { BrandSemrushProject: ctx.dataAccess.BrandSemrushProject }, }); }); @@ -1861,6 +1862,7 @@ describe('SerenityController', () => { brandAliases: ['Acme Inc'], brandUrlSources: { urls: [], socialAccounts: [], earnedContent: [] }, competitors: [], + env: {}, dataAccess: { BrandSemrushProject: ctx.dataAccess.BrandSemrushProject }, }; const { firstCall, secondCall } = handlers.handleCreateMarketSubworkspace; diff --git a/test/support/serenity/brand-provisioning.test.js b/test/support/serenity/brand-provisioning.test.js index 6331606691..5a6736174c 100644 --- a/test/support/serenity/brand-provisioning.test.js +++ b/test/support/serenity/brand-provisioning.test.js @@ -198,6 +198,7 @@ describe('provisionBrandSubworkspace', () => { projectTags: PROJECT_STANDARD_TAGS, brandUrlSources: null, competitors: [], + env: { SEMRUSH_PROJECTS_BASE_URL: 'https://gw.example' }, publishMode: 'require', }); // The stub drives the sub-workspace title off the brand's name + id. diff --git a/test/support/serenity/handlers/markets-subworkspace.test.js b/test/support/serenity/handlers/markets-subworkspace.test.js index 43a41c2fc7..28c9be6898 100644 --- a/test/support/serenity/handlers/markets-subworkspace.test.js +++ b/test/support/serenity/handlers/markets-subworkspace.test.js @@ -452,8 +452,8 @@ describe('markets-subworkspace handlers', () => { expect(transport.createTaggedPrompts).to.have.been.calledOnce; const [, , promptsByText] = transport.createTaggedPrompts.firstCall.args; expect(promptsByText).to.deep.equal({ - 'best running shoes': ['topic:Running Shoes', 'source:ai', 'type:non-branded'], - 'top trail shoes': ['topic:Running Shoes', 'source:ai', 'type:branded'], + 'best running shoes': ['topic:Running Shoes', 'source:ai', 'intent:Informational', 'type:non-branded'], + 'top trail shoes': ['topic:Running Shoes', 'source:ai', 'intent:Informational', 'type:branded'], }); expect(res.body).to.include({ topicCount: 1, promptCount: 2, published: true }); // Models are STAGED (no inner publish) — only the single final publish runs, @@ -532,9 +532,9 @@ describe('markets-subworkspace handlers', () => { expect(res.status).to.equal(201); const [, , promptsByText] = transport.createTaggedPrompts.firstCall.args; expect(promptsByText).to.deep.equal({ - 'Best ACME running shoes': ['topic:Shoes', 'source:ai', 'type:branded'], - 'top trail sneakers from zoom': ['topic:Shoes', 'source:ai', 'type:branded'], - 'most comfortable sandals': ['topic:Shoes', 'source:ai', 'type:non-branded'], + 'Best ACME running shoes': ['topic:Shoes', 'source:ai', 'intent:Informational', 'type:branded'], + 'top trail sneakers from zoom': ['topic:Shoes', 'source:ai', 'intent:Informational', 'type:branded'], + 'most comfortable sandals': ['topic:Shoes', 'source:ai', 'intent:Informational', 'type:non-branded'], }); }); diff --git a/test/support/serenity/handlers/prompts-subworkspace.test.js b/test/support/serenity/handlers/prompts-subworkspace.test.js index 834902e96f..e886496282 100644 --- a/test/support/serenity/handlers/prompts-subworkspace.test.js +++ b/test/support/serenity/handlers/prompts-subworkspace.test.js @@ -47,6 +47,11 @@ function makeTransport(overrides = {}) { createTaggedPrompts: sinon.stub().resolves({ ids: ['new-prompt'] }), deletePromptsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(null), + // serenity-docs#32: intent injection always resolves/creates an + // `intent:*` tag id on the id-based path, even when a test isn't + // exercising intent classification specifically. + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), ...overrides, }; } @@ -141,8 +146,8 @@ describe('prompts-subworkspace handlers', () => { }, log); expect(result.created).to.have.length(1); expect(result.created[0]).to.include({ semrushPromptId: 'new-prompt-by-id' }); - expect(result.created[0].tagIds).to.deep.equal(['tag-cat-1', 'tag-child-1']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['p'], ['tag-cat-1', 'tag-child-1']); + expect(result.created[0].tagIds).to.deep.equal(['tag-cat-1', 'tag-child-1', 'intent-tag-id']); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['p'], ['tag-cat-1', 'tag-child-1', 'intent-tag-id']); expect(transport.createTaggedPrompts).to.not.have.been.called; }); @@ -154,11 +159,11 @@ describe('prompts-subworkspace handlers', () => { text: 'is Acme good?', tags: ['topic:X', 'type:non-branded'], geoTargetId: 2840, languageCode: 'en', }], }, log, classify); - expect(result.created[0].tags).to.deep.equal(['topic:X', 'type:branded']); + expect(result.created[0].tags).to.deep.equal(['topic:X', 'type:branded', 'intent:Informational']); expect(transport.createTaggedPrompts).to.have.been.calledOnceWithExactly( WS, 'p-us-en', - { 'is Acme good?': ['topic:X', 'type:branded'] }, + { 'is Acme good?': ['topic:X', 'type:branded', 'intent:Informational'] }, ); }); @@ -313,9 +318,9 @@ describe('prompts-subworkspace handlers', () => { }, log); expect(result.status).to.equal(200); expect(result.body.semrushPromptId).to.equal('new-prompt-by-id'); - expect(result.body.tagIds).to.deep.equal(['tag-cat-1']); + expect(result.body.tagIds).to.deep.equal(['tag-cat-1', 'intent-tag-id']); expect(transport.deletePromptsByIds).to.have.been.calledWith(WS, 'p-us-en', ['old-id']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['new'], ['tag-cat-1']); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['new'], ['tag-cat-1', 'intent-tag-id']); expect(transport.createTaggedPrompts).to.not.have.been.called; }); @@ -343,11 +348,11 @@ describe('prompts-subworkspace handlers', () => { text: 'now mentions Acme', tags: ['topic:X', 'type:non-branded'], geoTargetId: 2840, languageCode: 'en', }, log, classify); expect(result.status).to.equal(200); - expect(result.body.tags).to.deep.equal(['topic:X', 'type:branded']); + expect(result.body.tags).to.deep.equal(['topic:X', 'type:branded', 'intent:Informational']); expect(transport.createTaggedPrompts).to.have.been.calledOnceWithExactly( WS, 'p-us-en', - { 'now mentions Acme': ['topic:X', 'type:branded'] }, + { 'now mentions Acme': ['topic:X', 'type:branded', 'intent:Informational'] }, ); }); @@ -596,7 +601,7 @@ describe('prompts-subworkspace — defensive branch coverage', () => { languageCode: 'en', }, log); expect(result.status).to.equal(200); - expect(result.body.tags).to.deep.equal([]); + expect(result.body.tags).to.deep.equal(['intent:Informational']); }); // Line 275: `Array.isArray(resp?.ids)&&resp.ids.length>0?String(resp.ids[0]):''` else @@ -669,6 +674,6 @@ describe('prompts-subworkspace — defensive branch coverage', () => { languageCode: 'en', }, log); expect(result.status).to.equal(200); - expect(result.body.tags).to.deep.equal(['keep']); + expect(result.body.tags).to.deep.equal(['keep', 'intent:Informational']); }); }); diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index 75427ddf8b..ee307d65a0 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -155,7 +155,7 @@ describe('handlers/prompts.js — handleListPrompts', () => { text: 'hi', geoTargetId: 2840, languageCode: 'en', tags: ['keep', '', null], }], }, fakeLog()); - expect(createResult.created[0].tags).to.deep.equal(['keep']); + expect(createResult.created[0].tags).to.deep.equal(['keep', 'intent:Informational']); // PATCH — tags array with a falsy entry → dropped silently. dataAccess.BrandSemrushProject.findBySlice.resolves(project); @@ -170,7 +170,7 @@ describe('handlers/prompts.js — handleListPrompts', () => { }, fakeLog(), ); - expect(updateResult.body.tags).to.deep.equal(['keep']); + expect(updateResult.body.tags).to.deep.equal(['keep', 'intent:Informational']); }); // Branch coverage: object-form tags (`{id, name}`) and null `id` on the @@ -522,7 +522,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { geoTargetId: 2840, languageCode: 'en', text: 'hello', - tags: ['a'], + tags: ['a', 'intent:Informational'], }); expect(transport.publishProject).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en'); }); @@ -538,6 +538,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }), createTaggedPrompts: sinon.stub(), publishProject: sinon.stub().resolves(), + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), }; const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { @@ -553,9 +555,9 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { languageCode: 'en', text: 'hello', tags: [], - tagIds: ['tag-cat-1', 'tag-child-1'], + tagIds: ['tag-cat-1', 'tag-child-1', 'intent-tag-id'], }); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['tag-cat-1', 'tag-child-1']); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['tag-cat-1', 'tag-child-1', 'intent-tag-id']); expect(transport.createTaggedPrompts).to.not.have.been.called; }); @@ -612,6 +614,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }), createTaggedPrompts: sinon.stub(), publishProject: sinon.stub().resolves(), + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), }; const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { @@ -621,8 +625,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }, fakeLog()); expect(result.created[0].semrushPromptId).to.equal(''); - expect(result.created[0].tagIds).to.deep.equal(['keep']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep']); + expect(result.created[0].tagIds).to.deep.equal(['keep', 'intent-tag-id']); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep', 'intent-tag-id']); }); it('returns empty semrushPromptId (not the string "undefined") when createPromptsByIds returns an item with no id', async () => { @@ -636,6 +640,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }), createTaggedPrompts: sinon.stub(), publishProject: sinon.stub().resolves(), + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), }; const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { @@ -658,6 +664,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }), createTaggedPrompts: sinon.stub(), publishProject: sinon.stub().resolves(), + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), }; const tooLong = 'x'.repeat(201); @@ -670,8 +678,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }], }, fakeLog()); - expect(result.created[0].tagIds).to.deep.equal(['keep']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep']); + expect(result.created[0].tagIds).to.deep.equal(['keep', 'intent-tag-id']); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep', 'intent-tag-id']); }); it('caps a bulk-create tagIds array at MAX_TAG_IDS (50), mirroring the list-read query cap', async () => { @@ -685,6 +693,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }), createTaggedPrompts: sinon.stub(), publishProject: sinon.stub().resolves(), + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), }; const tooMany = Array.from({ length: 55 }, (_, i) => `tag-${i}`); @@ -694,8 +704,8 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }], }, fakeLog()); - expect(result.created[0].tagIds).to.have.lengthOf(50); - expect(result.created[0].tagIds).to.deep.equal(tooMany.slice(0, 50)); + expect(result.created[0].tagIds).to.have.lengthOf(51); + expect(result.created[0].tagIds).to.deep.equal([...tooMany.slice(0, 50), 'intent-tag-id']); }); it('skips a create row when tagIds sanitizes to empty (every entry malformed)', async () => { @@ -924,6 +934,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { }), createTaggedPrompts: sinon.stub(), publishProject: sinon.stub().resolves(), + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), }; const result = await handleUpdatePrompt( @@ -945,10 +957,10 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { languageCode: 'en', text: 'next', tags: [], - tagIds: ['tag-cat-1'], + tagIds: ['tag-cat-1', 'intent-tag-id'], }); expect(transport.deletePromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['sem-1']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['tag-cat-1']); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['tag-cat-1', 'intent-tag-id']); expect(transport.createTaggedPrompts).to.not.have.been.called; }); @@ -966,6 +978,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { }), createTaggedPrompts: sinon.stub(), publishProject: sinon.stub().resolves(), + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), }; const result = await handleUpdatePrompt( @@ -980,8 +994,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { fakeLog(), ); - expect(result.body.tagIds).to.deep.equal(['keep']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['keep']); + expect(result.body.tagIds).to.deep.equal(['keep', 'intent-tag-id']); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['keep', 'intent-tag-id']); }); it('drops malformed tagIds entries on PATCH like validateParentIdFormat does for parentId', async () => { @@ -998,6 +1012,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { }), createTaggedPrompts: sinon.stub(), publishProject: sinon.stub().resolves(), + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), }; const tooLong = 'x'.repeat(201); @@ -1016,8 +1032,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { fakeLog(), ); - expect(result.body.tagIds).to.deep.equal(['keep']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['keep']); + expect(result.body.tagIds).to.deep.equal(['keep', 'intent-tag-id']); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['keep', 'intent-tag-id']); }); it('400s when tagIds sanitizes to empty (every entry malformed)', async () => { @@ -1113,7 +1129,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { geoTargetId: 2840, languageCode: 'en', text: 'new text', - tags: ['fresh'], + tags: ['fresh', 'intent:Informational'], }); expect(transport.listPromptsByTags).to.have.callCount(0); expect(transport.deletePromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['sem-1']); @@ -1181,7 +1197,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { ); expect(result.status).to.equal(200); - expect(result.body.tags).to.deep.equal([]); + expect(result.body.tags).to.deep.equal(['intent:Informational']); }); // Branch coverage: upstream createTaggedPrompts returns no ids array → @@ -1258,6 +1274,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { deletePromptsByIds: sinon.stub().resolves(), createPromptsByIds: sinon.stub().rejects(createErr), publishProject: sinon.stub().resolves(), + listProjectTags: sinon.stub().resolves({ items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), }; const log = fakeLog(); @@ -1766,11 +1784,11 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) expect(result.created).to.have.lengthOf(1); // caller's type:non-branded stripped; computed type:branded appended. - expect(result.created[0].tags).to.deep.equal(['topic:X', 'type:branded']); + expect(result.created[0].tags).to.deep.equal(['topic:X', 'type:branded', 'intent:Informational']); expect(transport.createTaggedPrompts).to.have.been.calledOnceWithExactly( WORKSPACE, 'proj-us-en', - { 'is Acme good?': ['topic:X', 'type:branded'] }, + { 'is Acme good?': ['topic:X', 'type:branded', 'intent:Informational'] }, ); }); @@ -1787,7 +1805,7 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }], }, fakeLog(), classify); - expect(result.created[0].tags).to.deep.equal(['topic:X', 'type:non-branded']); + expect(result.created[0].tags).to.deep.equal(['topic:X', 'type:non-branded', 'intent:Informational']); }); }); @@ -1803,6 +1821,7 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) { id: 'tnb', name: 'type:non-branded' }, ], }), + createProjectTags: sinon.stub().resolves([{ id: 'intent-tag-id', name: 'intent:Informational' }]), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'is Acme good?' }], }), @@ -1818,12 +1837,12 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }], }, fakeLog(), classify); - expect(result.created[0].tagIds).to.deep.equal(['tag-cat-1', 'tb']); + expect(result.created[0].tagIds).to.deep.equal(['tag-cat-1', 'tb', 'intent-tag-id']); expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly( WORKSPACE, 'proj-us-en', ['is Acme good?'], - ['tag-cat-1', 'tb'], + ['tag-cat-1', 'tb', 'intent-tag-id'], ); }); }); @@ -1843,7 +1862,7 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }, fakeLog(), classify); expect(result.status).to.equal(200); - expect(result.body.tags).to.deep.equal(['topic:X', 'type:branded']); + expect(result.body.tags).to.deep.equal(['topic:X', 'type:branded', 'intent:Informational']); }); }); From e852b82aeee538eb8a3ec116a46dd6a4b3d4aeef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Thu, 9 Jul 2026 16:14:23 +0200 Subject: [PATCH 02/16] feat(serenity): add deferPublish for CSV chunking on the bulk-create path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds body.deferPublish to skip publishAffected on the bulk-create path (flat + subworkspace), so elmo-ui's CSV chunking (serenity-docs#32) can create drafts-only per chunk and publish once on the final, non-deferred chunk of an import — no new endpoint needed since a single CSV import always targets one project. Response now echoes published:true/false. --- .../serenity/handlers/prompts-subworkspace.js | 10 ++++- src/support/serenity/handlers/prompts.js | 17 ++++++- .../handlers/prompts-subworkspace.test.js | 26 +++++++++++ .../support/serenity/handlers/prompts.test.js | 44 +++++++++++++++++++ 4 files changed, 94 insertions(+), 3 deletions(-) diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index 67f4c4c25a..4f6e055251 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -215,6 +215,12 @@ export async function handleCreatePromptsSubworkspace( invalidateTagCacheForProject(workspaceId, pid); } + if (body?.deferPublish === true) { + return { + created, skipped, failed, published: false, + }; + } + const publishErrors = await publishAffected(transport, workspaceId, affectedProjectIds, log); // publishAffected returns { projectId, message } records whose message is // ALREADY redacted (redactUpstreamMessage) — pubErr is a record, not a raw error. @@ -222,7 +228,9 @@ export async function handleCreatePromptsSubworkspace( failed.push({ text: '', status: 502, message: `publish: ${pubErr.message}` }); } - return { created, skipped, failed }; + return { + created, skipped, failed, published: true, + }; } /** diff --git a/src/support/serenity/handlers/prompts.js b/src/support/serenity/handlers/prompts.js index 7717e15e6e..2911783fa8 100644 --- a/src/support/serenity/handlers/prompts.js +++ b/src/support/serenity/handlers/prompts.js @@ -460,7 +460,12 @@ export async function mapLimit(items, limit, mapper) { * POST /serenity/prompts — bulk create. * Each input must carry `(geoTargetId, languageCode, text, tags?)`. Inputs * are grouped by slice; the matching BrandSemrushProject row resolves the - * upstream project; publish runs once per affected project at the end. + * upstream project; publish runs once per affected project at the end — + * unless `body.deferPublish` is true (serenity-docs#32 CSV-chunking), in + * which case the create is a draft-only write and the caller is responsible + * for triggering a publish itself (e.g. a normal, non-deferred call on the + * last chunk of an import, which publishes every project touched across the + * whole import since a single CSV import always targets one project). */ export async function handleCreatePrompts( transport, @@ -582,6 +587,12 @@ export async function handleCreatePrompts( invalidateTagCacheForProject(semrushWorkspaceId, pid); } + if (body?.deferPublish === true) { + return { + created, skipped, failed, published: false, + }; + } + const publishErrors = await publishAffected( transport, semrushWorkspaceId, @@ -598,7 +609,9 @@ export async function handleCreatePrompts( }); } - return { created, skipped, failed }; + return { + created, skipped, failed, published: true, + }; } /** diff --git a/test/support/serenity/handlers/prompts-subworkspace.test.js b/test/support/serenity/handlers/prompts-subworkspace.test.js index e886496282..b640d4ae3b 100644 --- a/test/support/serenity/handlers/prompts-subworkspace.test.js +++ b/test/support/serenity/handlers/prompts-subworkspace.test.js @@ -246,6 +246,32 @@ describe('prompts-subworkspace handlers', () => { expect(result.failed).to.have.length(1); expect(result.failed[0].message).to.match(/^publish:/); }); + + // serenity-docs#32: CSV chunking creates drafts-only per chunk + // (deferPublish: true), publishing once on the final, non-deferred chunk. + it('skips publishProject and reports published:false when body.deferPublish is true', async () => { + const transport = makeTransport(); + const result = await handleCreatePromptsSubworkspace(transport, WS, { + prompts: [{ + text: 'p', tags: ['x'], geoTargetId: 2840, languageCode: 'en', + }], + deferPublish: true, + }, log); + expect(result.created).to.have.length(1); + expect(result.published).to.equal(false); + expect(transport.publishProject).to.not.have.been.called; + }); + + it('publishes and reports published:true when body.deferPublish is absent', async () => { + const transport = makeTransport(); + const result = await handleCreatePromptsSubworkspace(transport, WS, { + prompts: [{ + text: 'p', tags: ['x'], geoTargetId: 2840, languageCode: 'en', + }], + }, log); + expect(result.published).to.equal(true); + expect(transport.publishProject).to.have.been.calledOnce; + }); }); describe('handleUpdatePromptSubworkspace', () => { diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index ee307d65a0..e46624e8fb 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -816,6 +816,50 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { message: 'publish: publish boom', }); }); + + // serenity-docs#32: CSV chunking on elmo-ui creates drafts-only per chunk + // (deferPublish: true) and publishes once on the final, non-deferred chunk. + it('skips publishProject and reports published:false when body.deferPublish is true', async () => { + const project = makeProject({ + semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', + }); + const dataAccess = makeDataAccess([project]); + const transport = { + createTaggedPrompts: sinon.stub().resolves({ ids: ['new-sem-id'] }), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { + prompts: [{ + text: 'ok', geoTargetId: 2840, languageCode: 'en', tags: [], + }], + deferPublish: true, + }, fakeLog()); + + expect(result.created).to.have.lengthOf(1); + expect(result.published).to.equal(false); + expect(transport.publishProject).to.not.have.been.called; + }); + + it('publishes and reports published:true when body.deferPublish is absent', async () => { + const project = makeProject({ + semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', + }); + const dataAccess = makeDataAccess([project]); + const transport = { + createTaggedPrompts: sinon.stub().resolves({ ids: ['new-sem-id'] }), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { + prompts: [{ + text: 'ok', geoTargetId: 2840, languageCode: 'en', tags: [], + }], + }, fakeLog()); + + expect(result.published).to.equal(true); + expect(transport.publishProject).to.have.been.calledOnce; + }); }); describe('handlers/prompts.js — handleUpdatePrompt', () => { From 634896c304423367a415911d208536d1377ce509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Thu, 9 Jul 2026 17:03:42 +0200 Subject: [PATCH 03/16] test(serenity): fix IT tagIds-length assertion; add intent-classification/taxonomy coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test/it/shared/tests/serenity.js: the id-based create IT test asserted an exact tagIds length of 3, missing the new computed intent: tag alongside the existing computed type: tag — now expects 4 (this was breaking CI's it-postgres job). - Adds the dedicated unit tests for the two new modules (intent-classification.js, intent-taxonomy.js) called for in the original implementation plan but not yet written: the full fallback ladder (hard skip-gate, Azure-unconfigured prod/non-prod logging, first-pass success, budget-gated retry success/ exhaustion, retry-skipped-on-no-budget), and parseSerenityIntent's validity- floor/value-matching behavior. Mirrors resolveTypeTagInjection's existing test shape for the new resolveIntentTagInjection sibling in tags.test.js. - Closes the codecov patch-coverage gap flagged on PR #2785 (all three files now at 100% line/branch coverage). --- test/it/shared/tests/serenity.js | 11 +- test/support/serenity/handlers/tags.test.js | 64 +++++++ .../serenity/intent-classification.test.js | 173 ++++++++++++++++++ test/support/serenity/intent-taxonomy.test.js | 65 +++++++ 4 files changed, 309 insertions(+), 4 deletions(-) create mode 100644 test/support/serenity/intent-classification.test.js create mode 100644 test/support/serenity/intent-taxonomy.test.js diff --git a/test/it/shared/tests/serenity.js b/test/it/shared/tests/serenity.js index fe136a7fcb..d209c3eaea 100644 --- a/test/it/shared/tests/serenity.js +++ b/test/it/shared/tests/serenity.js @@ -433,11 +433,14 @@ export default function serenityTests(getHttpClient, resetData, resetMocks = asy expect(created.status).to.equal(200); expect(created.body.created).to.have.lengthOf(1); expect(created.body.created[0].semrushPromptId).to.be.a('string').that.is.not.empty; - // The write path now server-computes a branded/non-branded `type:` tag and - // appends it to the supplied tagIds, so the created prompt carries the two - // supplied tags plus one computed type tag. + // The write path now server-computes a branded/non-branded `type:` tag AND + // an intent: tag (serenity-docs#31, #32) and appends both to the + // supplied tagIds, so the created prompt carries the two supplied tags plus + // the two computed ones. Azure OpenAI is not configured in this IT + // environment, so intent classification deterministically defaults to + // `intent:Informational` (never null/omitted — see the fallback ladder). expect(created.body.created[0].tagIds).to.include.members([category.body.id, child.body.id]); - expect(created.body.created[0].tagIds).to.have.lengthOf(3); + expect(created.body.created[0].tagIds).to.have.lengthOf(4); expect(created.body.failed).to.deep.equal([]); // by_tags correlation: the id-based create embeds the tag ids, so filtering the prompt list diff --git a/test/support/serenity/handlers/tags.test.js b/test/support/serenity/handlers/tags.test.js index 8bcf90e567..5560353c70 100644 --- a/test/support/serenity/handlers/tags.test.js +++ b/test/support/serenity/handlers/tags.test.js @@ -1144,4 +1144,68 @@ describe('serenity tags handler (POST /serenity/tags)', () => { expect(res.typeTagIds).to.deep.equal([]); }); }); + + describe('resolveIntentTagInjection (serenity-docs#32)', () => { + let handler; + beforeEach(async () => { + handler = await import('../../../../src/support/serenity/handlers/tags.js'); + }); + + it('resolves the wanted intent value id + all intent ids from the project roots (no create)', async () => { + const transport = makeTransport({ + listProjectTags: sinon.stub().resolves({ + page: 1, + total: 3, + items: [ + { id: 'cat-1', name: 'category:Footwear' }, + { id: 'ii', name: 'intent:Informational' }, + { id: 'it', name: 'intent:Task' }, + ], + }), + }); + + const res = await handler.resolveIntentTagInjection(transport, WORKSPACE, 'proj-1', 'intent:Task', fakeLog()); + + expect(res.computedId).to.equal('it'); + expect(res.intentTagIds).to.have.members(['ii', 'it']); + expect(transport.createProjectTags).to.not.have.been.called; + }); + + it('creates the intent value on-demand when a legacy project lacks it', async () => { + const transport = makeTransport({ + listProjectTags: sinon.stub().resolves({ page: 1, total: 0, items: [] }), + createProjectTags: sinon.stub().resolves([{ id: 'new-it', name: 'intent:Task' }]), + }); + + const res = await handler.resolveIntentTagInjection(transport, WORKSPACE, 'proj-1', 'intent:Task', fakeLog()); + + expect(res.computedId).to.equal('new-it'); + expect(res.intentTagIds).to.deep.equal(['new-it']); + expect(transport.createProjectTags) + .to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-1', ['intent:Task']); + }); + + it('propagates a transport failure while listing the project tag tree', async () => { + const transport = makeTransport({ + listProjectTags: sinon.stub().rejects(new Error('listProjectTags 502')), + }); + + await expect( + handler.resolveIntentTagInjection(transport, WORKSPACE, 'proj-1', 'intent:Task', fakeLog()), + ).to.be.rejectedWith(/listProjectTags 502/); + expect(transport.createProjectTags).to.not.have.been.called; + }); + + it('yields computedId undefined (skips injection) when the create response carries no id', async () => { + const transport = makeTransport({ + listProjectTags: sinon.stub().resolves({ page: 1, total: 0, items: [] }), + createProjectTags: sinon.stub().resolves([{}]), + }); + + const res = await handler.resolveIntentTagInjection(transport, WORKSPACE, 'proj-1', 'intent:Task', fakeLog()); + + expect(res.computedId).to.equal(undefined); + expect(res.intentTagIds).to.deep.equal([]); + }); + }); }); diff --git a/test/support/serenity/intent-classification.test.js b/test/support/serenity/intent-classification.test.js new file mode 100644 index 0000000000..d6229dc2f4 --- /dev/null +++ b/test/support/serenity/intent-classification.test.js @@ -0,0 +1,173 @@ +/* + * Copyright 2026 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import { expect } from 'chai'; +import sinon from 'sinon'; +import esmock from 'esmock'; + +import { CREATE_PUBLISH_RESERVE_MS } from '../../../src/support/serenity/intent-classification.js'; +import { PER_CALL_MS } from '../../../src/support/serenity/intent-taxonomy.js'; + +const log = { + info: sinon.stub(), warn: sinon.stub(), error: sinon.stub(), debug: sinon.stub(), +}; + +async function loadWithClassifier({ classify, classifyIntentsStub } = {}) { + return esmock('../../../src/support/serenity/intent-classification.js', { + '../../../src/support/intent-classifier.js': { + createIntentClassifier: sinon.stub().returns(classify), + classifyIntents: classifyIntentsStub || sinon.stub().resolves(new Map()), + }, + }); +} + +describe('intent-classification.js — classifyPromptIntents (serenity-docs#32)', () => { + afterEach(() => { + sinon.reset(); + }); + + it('returns an empty map for an empty/falsy text list without touching the classifier', async () => { + const classifyIntentsStub = sinon.stub(); + const { classifyPromptIntents } = await loadWithClassifier({ + classify: () => {}, classifyIntentsStub, + }); + const result = await classifyPromptIntents([], { env: {}, log, deadline: Date.now() + 100000 }); + expect(result.size).to.equal(0); + expect(classifyIntentsStub).to.not.have.been.called; + }); + + it('treats a null/undefined texts argument as empty', async () => { + const classifyIntentsStub = sinon.stub(); + const { classifyPromptIntents } = await loadWithClassifier({ + classify: () => {}, classifyIntentsStub, + }); + const result = await classifyPromptIntents(null, { + env: {}, log, deadline: Date.now() + 100000, + }); + expect(result.size).to.equal(0); + expect(classifyIntentsStub).to.not.have.been.called; + }); + + it('hard skip-gate: defaults everything and logs budget_skipped when no room at entry, without constructing a classifier', async () => { + const classifyIntentsStub = sinon.stub(); + const { classifyPromptIntents, computeWriteDeadline } = await loadWithClassifier({ + classify: () => {}, classifyIntentsStub, + }); + // deadline already effectively exhausted (in the past). + const deadline = computeWriteDeadline(Date.now() - 100000); + const result = await classifyPromptIntents(['a', 'b'], { env: {}, log, deadline }); + expect(result.get('a')).to.equal('intent:Informational'); + expect(result.get('b')).to.equal('intent:Informational'); + expect(classifyIntentsStub).to.not.have.been.called; + expect(log.info).to.have.been.calledWithMatch(/budget_skipped/); + }); + + it('defaults everything with an info log (non-prod) when the classifier is unavailable (Azure not configured)', async () => { + const { classifyPromptIntents } = await loadWithClassifier({ classify: null }); + const result = await classifyPromptIntents(['a'], { + env: { AWS_ENV: 'dev' }, log, deadline: Date.now() + 100000, + }); + expect(result.get('a')).to.equal('intent:Informational'); + expect(log.info).to.have.been.calledWithMatch(/Azure OpenAI is not configured/); + expect(log.warn).to.not.have.been.called; + }); + + it('defaults everything with a WARN + prod_llm_unavailable log in prod when the classifier is unavailable', async () => { + const { classifyPromptIntents } = await loadWithClassifier({ classify: null }); + const result = await classifyPromptIntents(['a'], { + env: { AWS_ENV: 'prod' }, log, deadline: Date.now() + 100000, + }); + expect(result.get('a')).to.equal('intent:Informational'); + expect(log.warn).to.have.been.calledWithMatch(/prod_llm_unavailable/); + }); + + it('treats a null/undefined env or log as if omitted, without throwing', async () => { + const { classifyPromptIntents } = await loadWithClassifier({ classify: null }); + const result = await classifyPromptIntents(['a'], { + env: null, log: null, deadline: Date.now() + 100000, + }); + expect(result.get('a')).to.equal('intent:Informational'); + }); + + it('uses the first-pass classification result and counts classified_ok, no retry when everything resolves', async () => { + const classifyIntentsStub = sinon.stub().resolves(new Map([ + ['a', 'intent:Task'], ['b', 'intent:Commercial'], + ])); + const { classifyPromptIntents } = await loadWithClassifier({ + classify: () => {}, classifyIntentsStub, + }); + const result = await classifyPromptIntents(['a', 'b'], { + env: {}, log, deadline: Date.now() + 100000, + }); + expect(result.get('a')).to.equal('intent:Task'); + expect(result.get('b')).to.equal('intent:Commercial'); + expect(classifyIntentsStub).to.have.been.calledOnce; + expect(log.info).to.have.been.calledWithMatch(/summary/, sinon.match({ classified_ok: 2, retry_attempted: 0 })); + }); + + it('retries once for unresolved texts when the budget allows, and uses the retry result on success', async () => { + const classifyIntentsStub = sinon.stub(); + classifyIntentsStub.onCall(0).resolves(new Map([['a', 'intent:Task']])); // 'b' left unresolved + classifyIntentsStub.onCall(1).resolves(new Map([['b', 'intent:Navigational']])); + const { classifyPromptIntents } = await loadWithClassifier({ + classify: () => {}, classifyIntentsStub, + }); + // Ample budget so the retry gate passes. + const result = await classifyPromptIntents(['a', 'b'], { + env: {}, log, deadline: Date.now() + 100000, + }); + expect(result.get('a')).to.equal('intent:Task'); + expect(result.get('b')).to.equal('intent:Navigational'); + expect(classifyIntentsStub).to.have.been.calledTwice; + expect(log.info).to.have.been.calledWithMatch(/summary/, sinon.match({ classified_ok: 1, retry_attempted: 1, retry_succeeded: 1 })); + }); + + it('defaults to Informational when the retry also fails to resolve a text', async () => { + const classifyIntentsStub = sinon.stub(); + classifyIntentsStub.onCall(0).resolves(new Map()); // nothing resolved + classifyIntentsStub.onCall(1).resolves(new Map()); // retry also resolves nothing + const { classifyPromptIntents } = await loadWithClassifier({ + classify: () => {}, classifyIntentsStub, + }); + const result = await classifyPromptIntents(['a'], { + env: {}, log, deadline: Date.now() + 100000, + }); + expect(result.get('a')).to.equal('intent:Informational'); + expect(classifyIntentsStub).to.have.been.calledTwice; + expect(log.info).to.have.been.calledWithMatch(/summary/, sinon.match({ defaulted: 1, retry_attempted: 1 })); + }); + + it('skips the retry (defaults immediately) when the deadline has no room left for a second call', async () => { + const t0 = 1_000_000_000; + const clock = sinon.useFakeTimers(t0); + try { + // Entry hard-skip-gate passes (remaining ≈ PER_CALL_MS + 50ms of slack), but + // the mocked first-pass classify call "spends" 60ms of wall-clock via the + // fake timer, so by the time the retry-gate check runs, remaining budget has + // dropped below PER_CALL_MS — the retry is skipped, not attempted. + const deadline = t0 + CREATE_PUBLISH_RESERVE_MS + PER_CALL_MS + 50; + const classifyIntentsStub = sinon.stub().callsFake(async () => { + clock.tick(60); + return new Map(); // nothing resolved on the first pass + }); + const { classifyPromptIntents } = await loadWithClassifier({ + classify: () => {}, classifyIntentsStub, + }); + const result = await classifyPromptIntents(['a'], { env: {}, log, deadline }); + expect(result.get('a')).to.equal('intent:Informational'); + expect(classifyIntentsStub).to.have.been.calledOnce; + expect(log.info).to.have.been.calledWithMatch(/summary/, sinon.match({ retry_attempted: 0, defaulted: 1 })); + } finally { + clock.restore(); + } + }); +}); diff --git a/test/support/serenity/intent-taxonomy.test.js b/test/support/serenity/intent-taxonomy.test.js new file mode 100644 index 0000000000..6675322fda --- /dev/null +++ b/test/support/serenity/intent-taxonomy.test.js @@ -0,0 +1,65 @@ +/* + * Copyright 2026 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import { expect } from 'chai'; + +import { + parseSerenityIntent, + SERENITY_INTENT_VALUES, + PROMPT_INTENT_MIN_CONFIDENCE, + SERENITY_INTENT_CATEGORY_SPEC, +} from '../../../src/support/serenity/intent-taxonomy.js'; + +describe('intent-taxonomy.js — parseSerenityIntent (serenity-docs#32)', () => { + it('returns the ready-to-use wire tag for a valid value at/above the confidence floor', () => { + expect(parseSerenityIntent({ intent: 'Task', confidence: PROMPT_INTENT_MIN_CONFIDENCE })) + .to.equal('intent:Task'); + expect(parseSerenityIntent({ intent: 'Informational', confidence: 0.99 })) + .to.equal('intent:Informational'); + }); + + it('accepts every canonical value', () => { + SERENITY_INTENT_VALUES.forEach((value) => { + expect(parseSerenityIntent({ intent: value, confidence: 1 })).to.equal(`intent:${value}`); + }); + }); + + it('returns null for a value outside the 5 canonical Capitalized literals (case-sensitive)', () => { + expect(parseSerenityIntent({ intent: 'informational', confidence: 1 })).to.equal(null); + expect(parseSerenityIntent({ intent: 'not-a-real-value', confidence: 1 })).to.equal(null); + expect(parseSerenityIntent({ intent: undefined, confidence: 1 })).to.equal(null); + }); + + it('returns null when confidence is below the validity floor', () => { + expect(parseSerenityIntent({ intent: 'Task', confidence: PROMPT_INTENT_MIN_CONFIDENCE - 0.01 })) + .to.equal(null); + }); + + it('returns null when confidence is missing, non-numeric, or non-finite', () => { + expect(parseSerenityIntent({ intent: 'Task' })).to.equal(null); + expect(parseSerenityIntent({ intent: 'Task', confidence: 'high' })).to.equal(null); + expect(parseSerenityIntent({ intent: 'Task', confidence: NaN })).to.equal(null); + expect(parseSerenityIntent({ intent: 'Task', confidence: Infinity })).to.equal(null); + }); + + it('returns null for a garbled/empty parsed body', () => { + expect(parseSerenityIntent({})).to.equal(null); + expect(parseSerenityIntent(null)).to.equal(null); + expect(parseSerenityIntent(undefined)).to.equal(null); + }); + + it('exposes the category spec with the fixed per-call timeout, not env-driven', () => { + expect(SERENITY_INTENT_CATEGORY_SPEC.parseResult).to.equal(parseSerenityIntent); + expect(SERENITY_INTENT_CATEGORY_SPEC.invokeTimeoutMs).to.be.a('number').greaterThan(0); + expect(SERENITY_INTENT_CATEGORY_SPEC.systemPrompt).to.be.a('string').with.length.greaterThan(0); + }); +}); From 7dc33116929a12a96f1d9fc07d3f8a26a7403917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Sat, 11 Jul 2026 15:23:02 +0200 Subject: [PATCH 04/16] fix(serenity): thread writeDeadline from request entry; validate deferPublish Address MysticatBot blocking review findings on PR #2785. writeDeadline clock drift: compute the shared write-budget deadline once at the true request entry (createBrandForOrg, createMarket, activate) and thread it through provisionBrandSubworkspace into handleCreateMarketSubworkspace, mirroring the create/update prompt paths. Previously the onboarding/brand- provisioning flow relied on the default-parameter fallback, which computed the deadline function-entry-late (after auth + loadBrand + provisioning), so total elapsed time could exceed the ~15s Fastly first-byte timeout the budget protects. The default now only fires for direct/test callers. deferPublish validation + observability: add validateDeferPublish() to reject a present-but-non-boolean deferPublish at the write boundary (400) instead of silently treating it as "publish", and log a structured line with created/ skipped/failed counts when the flag fires. Applied to both handleCreatePrompts and its subworkspace twin. Co-Authored-By: Claude Sonnet 5 --- src/controllers/brands.js | 8 +++++ src/controllers/serenity.js | 11 ++++++ src/support/serenity/brand-provisioning.js | 8 ++++- .../serenity/handlers/prompts-subworkspace.js | 7 +++- src/support/serenity/handlers/prompts.js | 24 ++++++++++++- test/controllers/serenity.test.js | 18 ++++++++-- .../serenity/brand-provisioning.test.js | 19 ++++++++-- .../handlers/prompts-subworkspace.test.js | 28 +++++++++++++++ .../support/serenity/handlers/prompts.test.js | 36 +++++++++++++++++++ 9 files changed, 151 insertions(+), 8 deletions(-) diff --git a/src/controllers/brands.js b/src/controllers/brands.js index 1497efd900..1278e5ca71 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -63,6 +63,7 @@ import { import { listViewableResourceIds } from '../support/state-access-mapping-utils.js'; import { isFacsRebacResource } from '../routes/facs-capabilities.js'; import { provisionBrandSubworkspace, releaseProvisionedWorkspace } from '../support/serenity/brand-provisioning.js'; +import { computeWriteDeadline } from '../support/serenity/intent-classification.js'; import { ensureMarketSite } from '../support/serenity/site-linkage.js'; import { upsertMappingRow, linkSiteToLiveRows } from '../support/serenity/mapping-rows.js'; import { createSerenityTransport, SerenityTransportError } from '../support/serenity/rest-transport.js'; @@ -1444,6 +1445,12 @@ function BrandsController(ctx, log, env) { const { spaceCatId } = context.params || {}; const brandData = context.data; + // One shared write-budget deadline for the whole request, computed at the + // true request entry (before auth/loadBrand/provisioning) so intent + // classification during provisioning budgets against the real request start + // rather than function-entry time deep in the call stack (serenity-docs#32). + const writeDeadline = computeWriteDeadline(); + // Hoisted above the try so the catch can run compensation: if a Semrush // sub-workspace was provisioned but the brand row failed to persist, the // catch releases the orphaned allocation (see below). @@ -1644,6 +1651,7 @@ function BrandsController(ctx, log, env) { // market's CI competitor list. Like URLs, they come from the create // payload (the brand row isn't written yet). competitors: brandData.competitors, + writeDeadline, }, log); provisionedWorkspaceId = provisioned.semrushSubWorkspaceId; provisionedInitialMarket = { diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index cf417b7d21..1312c06eb9 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -653,6 +653,10 @@ function SerenityController(context, log, env) { const createMarket = async (ctx) => { try { + // Shared write-budget deadline, computed once at request entry so intent + // classification during topic/prompt generation budgets against the true + // request start (serenity-docs#32). + const writeDeadline = computeWriteDeadline(); const imsToken = await resolveSemrushImsToken(ctx); const auth = await authorize(ctx); if (auth.error) { @@ -699,6 +703,7 @@ function SerenityController(context, log, env) { brandUrlSources, competitors, env: ctx.env, + writeDeadline, // auth.brandUuid is an already-persisted brand row here (loadBrand // above), so the mapping-row upsert's FK to brands is satisfied — // see mapping-rows.js upsertMappingRow doc. @@ -1021,6 +1026,11 @@ function SerenityController(context, log, env) { */ const activate = async (ctx) => { try { + // Shared write-budget deadline, computed once at request entry so intent + // classification during per-market topic/prompt generation budgets against + // the true request start rather than per-market function entry + // (serenity-docs#32). + const writeDeadline = computeWriteDeadline(); const imsToken = await resolveSemrushImsToken(ctx); const auth = await authorize(ctx); if (auth.error) { @@ -1226,6 +1236,7 @@ function SerenityController(context, log, env) { brandUrlSources, competitors, env: ctx.env, + writeDeadline, // `brand` was loaded via loadBrand above — an already-persisted // row, so the mapping-row upsert's FK to brands is satisfied. // Narrowed to the one model the mapping-row helpers touch — see diff --git a/src/support/serenity/brand-provisioning.js b/src/support/serenity/brand-provisioning.js index c259f75efc..db79cfe994 100644 --- a/src/support/serenity/brand-provisioning.js +++ b/src/support/serenity/brand-provisioning.js @@ -19,6 +19,7 @@ import { createSerenityTransport, SerenityTransportError } from './rest-transpor import { resolveWorkspaceId } from './workspace-resolver.js'; import { RELEASE_ALLOCATION } from './workspace-lifecycle.js'; import { handleCreateMarketSubworkspace } from './handlers/markets-subworkspace.js'; +import { computeWriteDeadline } from './intent-classification.js'; import { STANDARD_PROMPT_TAGS, PROJECT_STANDARD_TAGS } from './prompt-tags.js'; // Re-exported for callers/tests that drive brand provisioning. The tag @@ -81,6 +82,10 @@ export function initialMarketProjectName(market, languageCode) { * @param {object[]} [params.competitors] - the brand's competitors ("other * brands to track") tracked as region-filtered project benchmarks (domain-only). * Best-effort: a failed sync is logged and skipped, never aborts provisioning. + * @param {number} [params.writeDeadline] - shared request-write deadline (epoch + * ms), computed once at controller entry and threaded down so intent + * classification budgets against the true request start (serenity-docs#32); + * defaults to a fresh {@link computeWriteDeadline} for direct/test callers. * @param {object} [log] * @returns {Promise<{ * semrushSubWorkspaceId: string, @@ -103,7 +108,7 @@ export function initialMarketProjectName(market, languageCode) { export async function provisionBrandSubworkspace(context, { spaceCatId, brandId, brandName, market, languageCode, brandDomain, modelIds = [], brandAliases = [], brandUrlSources = null, competitors = [], - generateTopics = true, + generateTopics = true, writeDeadline = computeWriteDeadline(), }, log = console) { if (!hasText(brandName)) { throw new ErrorWithStatusCode('brandName is required for Semrush provisioning', 400); @@ -199,6 +204,7 @@ export async function provisionBrandSubworkspace(context, { brandAliases, projectTags: [...PROJECT_STANDARD_TAGS], env: context.env, + writeDeadline, brandUrlSources, competitors, // A project with neither models nor generated prompts would publish diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index 4f6e055251..65b2309e75 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -32,6 +32,7 @@ import { MAX_TAG_IDS, BULK_CREATE_CONCURRENCY, BULK_PROMPTS_MAX_ITEMS, + validateDeferPublish, } from './prompts.js'; import { resolveProject, buildSliceProjectMap, sliceKey } from '../subworkspace-projects.js'; import { redactUpstreamMessage } from '../rest-transport.js'; @@ -138,6 +139,7 @@ export async function handleCreatePromptsSubworkspace( 400, ); } + const deferPublish = validateDeferPublish(body); const projectsBySlice = await buildSliceProjectMap(transport, workspaceId, log); const injectComputedType = makeTypeInjector(transport, workspaceId, classifyPromptType, log); @@ -215,7 +217,10 @@ export async function handleCreatePromptsSubworkspace( invalidateTagCacheForProject(workspaceId, pid); } - if (body?.deferPublish === true) { + if (deferPublish) { + log?.info?.('serenity create-prompts (subworkspace): deferPublish set — prompts written as draft, publish skipped', { + workspaceId, created: created.length, skipped: skipped.length, failed: failed.length, + }); return { created, skipped, failed, published: false, }; diff --git a/src/support/serenity/handlers/prompts.js b/src/support/serenity/handlers/prompts.js index 2911783fa8..2897216f80 100644 --- a/src/support/serenity/handlers/prompts.js +++ b/src/support/serenity/handlers/prompts.js @@ -50,6 +50,24 @@ export const BULK_CREATE_CONCURRENCY = 8; // of them. Defense-in-depth, not a correctness gate. export const BULK_PROMPTS_MAX_ITEMS = 500; +/** + * Validates the optional `deferPublish` body flag (serenity-docs#32 CSV-chunking). + * Present-but-non-boolean is a hard 400 (so a caller typo like `"yes"`/`1` is + * rejected at the write boundary rather than silently treated as "publish"); + * absent, `false`, or `true` are all accepted. Returns the resolved boolean + * (absent → false). + * + * @param {object} body - request body. + * @returns {boolean} whether the caller asked to skip the trailing publish. + */ +export function validateDeferPublish(body) { + const deferPublish = body?.deferPublish; + if (deferPublish !== undefined && typeof deferPublish !== 'boolean') { + throw new ErrorWithStatusCode('deferPublish must be a boolean', 400); + } + return deferPublish === true; +} + // Builds { tagName → semrushTagId } from the upstream prompt item. // Object-form tags (the normal Semrush shape) carry both name and id. // String-form tags (defensive fallback) are included with an empty id so @@ -488,6 +506,7 @@ export async function handleCreatePrompts( 400, ); } + const deferPublish = validateDeferPublish(body); const projects = await dataAccess.BrandSemrushProject.allByBrandId(brandId); const projectsBySlice = new Map(); @@ -587,7 +606,10 @@ export async function handleCreatePrompts( invalidateTagCacheForProject(semrushWorkspaceId, pid); } - if (body?.deferPublish === true) { + if (deferPublish) { + log?.info?.('serenity create-prompts: deferPublish set — prompts written as draft, publish skipped', { + brandId, created: created.length, skipped: skipped.length, failed: failed.length, + }); return { created, skipped, failed, published: false, }; diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index f536da4c55..33ee20bb34 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -1253,7 +1253,12 @@ describe('SerenityController', () => { // so topic generation defaults off (today's behavior is unchanged). brandUuid // is already a persisted row (loadBrand), so dataAccess is threaded through // for the mapping-row upsert (mapping-rows.js). - expect(handlers.handleCreateMarketSubworkspace.firstCall.args[7]) + // writeDeadline is a request-scoped epoch-ms deadline (dynamic) — asserted + // as a number, then dropped before the deep-equal. + const { writeDeadline, ...marketOptions } = handlers.handleCreateMarketSubworkspace + .firstCall.args[7]; + expect(writeDeadline).to.be.a('number'); + expect(marketOptions) .to.deep.equal({ generateTopics: false, topicCap: 0, @@ -1866,8 +1871,15 @@ describe('SerenityController', () => { dataAccess: { BrandSemrushProject: ctx.dataAccess.BrandSemrushProject }, }; const { firstCall, secondCall } = handlers.handleCreateMarketSubworkspace; - expect(firstCall.args[7]).to.deep.equal(expectedOpts); - expect(secondCall.args[7]).to.deep.equal(expectedOpts); + // writeDeadline is computed ONCE at activate entry, so every market in the + // batch receives the SAME dynamic epoch-ms deadline — assert that, then + // drop it before comparing the rest of the options bag. + const { writeDeadline: dl1, ...opts1 } = firstCall.args[7]; + const { writeDeadline: dl2, ...opts2 } = secondCall.args[7]; + expect(dl1).to.be.a('number'); + expect(dl2).to.equal(dl1); + expect(opts1).to.deep.equal(expectedOpts); + expect(opts2).to.deep.equal(expectedOpts); }); it('activate reads the brand URL sources once and applies them to every market', async () => { diff --git a/test/support/serenity/brand-provisioning.test.js b/test/support/serenity/brand-provisioning.test.js index 5a6736174c..2228cfb755 100644 --- a/test/support/serenity/brand-provisioning.test.js +++ b/test/support/serenity/brand-provisioning.test.js @@ -188,8 +188,11 @@ describe('provisionBrandSubworkspace', () => { expect(body.brandDomain).to.equal('acme.com'); expect(body.brandNames).to.deep.equal(['Acme']); // Brand-create attaches LLMs, generates+attaches topic-tagged prompts, then - // publishes best-effort. - expect(options).to.deep.equal({ + // publishes best-effort. writeDeadline is a request-scoped epoch-ms deadline + // (dynamic) — asserted as a number, then dropped before the deep-equal. + const { writeDeadline, ...restOptions } = options; + expect(writeDeadline).to.be.a('number'); + expect(restOptions).to.deep.equal({ modelIds: ['m-1', 'm-2'], generateTopics: true, topicCap: MAX_TOPICS_ON_CREATE, @@ -207,6 +210,18 @@ describe('provisionBrandSubworkspace', () => { expect(brandStub.getSemrushSubWorkspaceId()).to.equal(undefined); }); + it('forwards a caller-supplied writeDeadline to the create handler (computed once at request entry, not defaulted here)', async () => { + const { provisionBrandSubworkspace } = await loadModule({ + resolveWorkspaceId, handleCreateMarketSubworkspace, + }); + // A deadline far in the future so it is unmistakably the passed-in value, + // not a fresh computeWriteDeadline() default (which would be ~now + 12s). + const writeDeadline = Date.now() + 999999; + await provisionBrandSubworkspace(buildContext(), { ...baseParams, writeDeadline }); + const options = handleCreateMarketSubworkspace.firstCall.args[7]; + expect(options.writeDeadline).to.equal(writeDeadline); + }); + it('resolves the IMS token via resolveSemrushImsToken and forwards it to createSerenityTransport (promise-token path)', async () => { const context = { ...buildContext(), diff --git a/test/support/serenity/handlers/prompts-subworkspace.test.js b/test/support/serenity/handlers/prompts-subworkspace.test.js index b640d4ae3b..3e3cf2d44f 100644 --- a/test/support/serenity/handlers/prompts-subworkspace.test.js +++ b/test/support/serenity/handlers/prompts-subworkspace.test.js @@ -22,6 +22,7 @@ import { handleBulkDeletePromptsSubworkspace, } from '../../../../src/support/serenity/handlers/prompts-subworkspace.js'; import { SerenityTransportError } from '../../../../src/support/serenity/rest-transport.js'; +import { ErrorWithStatusCode } from '../../../../src/support/utils.js'; use(chaiAsPromised); use(sinonChai); @@ -262,6 +263,33 @@ describe('prompts-subworkspace handlers', () => { expect(transport.publishProject).to.not.have.been.called; }); + it('logs a structured line (observability) when deferPublish fires', async () => { + const transport = makeTransport(); + const spyLog = { info: sinon.stub(), error: sinon.stub(), warn: sinon.stub() }; + await handleCreatePromptsSubworkspace(transport, WS, { + prompts: [{ + text: 'p', tags: ['x'], geoTargetId: 2840, languageCode: 'en', + }], + deferPublish: true, + }, spyLog); + expect(spyLog.info).to.have.been.calledWithMatch( + /deferPublish set/, + sinon.match({ + workspaceId: WS, created: 1, skipped: 0, failed: 0, + }), + ); + }); + + it('400s when deferPublish is present but not a boolean', async () => { + const transport = makeTransport(); + await expect(handleCreatePromptsSubworkspace(transport, WS, { + prompts: [{ + text: 'p', tags: ['x'], geoTargetId: 2840, languageCode: 'en', + }], + deferPublish: 'yes', + }, log)).to.be.rejectedWith(ErrorWithStatusCode, /deferPublish must be a boolean/); + }); + it('publishes and reports published:true when body.deferPublish is absent', async () => { const transport = makeTransport(); const result = await handleCreatePromptsSubworkspace(transport, WS, { diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index e46624e8fb..f88a7a716e 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -841,6 +841,42 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { expect(transport.publishProject).to.not.have.been.called; }); + it('logs a structured line (observability) when deferPublish fires', async () => { + const project = makeProject({ + semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', + }); + const dataAccess = makeDataAccess([project]); + const transport = { + createTaggedPrompts: sinon.stub().resolves({ ids: ['new-sem-id'] }), + publishProject: sinon.stub().resolves(), + }; + const log = fakeLog(); + + await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { + prompts: [{ + text: 'ok', geoTargetId: 2840, languageCode: 'en', tags: [], + }], + deferPublish: true, + }, log); + + expect(log.info).to.have.been.calledWithMatch( + /deferPublish set/, + sinon.match({ + brandId: BRAND, created: 1, skipped: 0, failed: 0, + }), + ); + }); + + it('400s when deferPublish is present but not a boolean', async () => { + const dataAccess = makeDataAccess([]); + await expect(handleCreatePrompts(({}), dataAccess, BRAND, WORKSPACE, { + prompts: [{ + text: 'ok', geoTargetId: 2840, languageCode: 'en', tags: [], + }], + deferPublish: 'yes', + }, fakeLog())).to.be.rejectedWith(ErrorWithStatusCode, /deferPublish must be a boolean/); + }); + it('publishes and reports published:true when body.deferPublish is absent', async () => { const project = makeProject({ semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', From 0b5cd7e32e8736a6ef8f24495156d7115768ee44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Tue, 14 Jul 2026 10:14:23 +0200 Subject: [PATCH 05/16] feat(serenity): add intent-classification observability (serenity-docs#32) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instrument the Serenity write-path intent classifier with EMF metrics and structured logs, layered additively on top of the existing classify/retry/ default ladder (no behavior change): - IntentOutcome counters (classified_ok, low_confidence, retry_attempted, retry_succeeded, defaulted, budget_skipped) dimensioned by WritePath + Workspace. - IntentValueDistribution per WritePath (5 values + defaulted bucket) — the primary degradation signal. - Per-call latency p50/p95, batch duration, and a per-call timeout tally. - A real ProdLlmUnavailable counter (plus the existing warn) on both classifier-construction failure and repeated-invoke failure; prod detection now uses resolveEnvironment (ENV/AWS_ENV) instead of AWS_ENV only. - Distinguish low_confidence soft-failures via inspectSerenityIntent and log truncated reasoning (capped, 200 chars) for debuggability. All emission is best-effort and never throws into the classify path. All 5 call sites pass writePath + workspaceId. 100% patch coverage on the two changed source modules. Co-Authored-By: Claude Sonnet 5 --- .../serenity/handlers/markets-subworkspace.js | 4 +- .../serenity/handlers/prompts-subworkspace.js | 12 +- src/support/serenity/handlers/prompts.js | 12 +- src/support/serenity/intent-classification.js | 212 +++++++++++++++- src/support/serenity/intent-taxonomy.js | 52 +++- .../serenity/intent-classification.test.js | 235 ++++++++++++++++++ test/support/serenity/intent-taxonomy.test.js | 36 +++ 7 files changed, 536 insertions(+), 27 deletions(-) diff --git a/src/support/serenity/handlers/markets-subworkspace.js b/src/support/serenity/handlers/markets-subworkspace.js index 171f1e1333..73f0ff46e2 100644 --- a/src/support/serenity/handlers/markets-subworkspace.js +++ b/src/support/serenity/handlers/markets-subworkspace.js @@ -233,7 +233,9 @@ async function generateAndAttachPrompts(transport, workspaceId, projectId, { }); const intentByText = await classifyPromptIntents( allTexts.slice(0, AI_GEN_CLASSIFY_MAX), - { env, log, deadline: writeDeadline }, + { + env, log, deadline: writeDeadline, writePath: 'ai-gen', workspaceId, + }, ); const promptsByText = {}; diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index 65b2309e75..4b1510e02a 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -145,7 +145,13 @@ export async function handleCreatePromptsSubworkspace( const injectComputedType = makeTypeInjector(transport, workspaceId, classifyPromptType, log); const intentByText = await classifyPromptIntents( inputs.map((raw) => String(raw?.text || '')), - { env, log, deadline: writeDeadline }, + { + env, + log, + deadline: writeDeadline, + writePath: deferPublish ? 'csv' : 'create', + workspaceId, + }, ); const injectComputedIntent = makeIntentInjector(transport, workspaceId, intentByText, log); @@ -287,7 +293,9 @@ export async function handleUpdatePromptSubworkspace( const injectComputedType = makeTypeInjector(transport, workspaceId, classifyPromptType, log); const intentByText = await classifyPromptIntents( [nextText], - { env, log, deadline: writeDeadline }, + { + env, log, deadline: writeDeadline, writePath: 'edit', workspaceId, + }, ); const injectComputedIntent = makeIntentInjector(transport, workspaceId, intentByText, log); let typed = await injectComputedType(projectId, { diff --git a/src/support/serenity/handlers/prompts.js b/src/support/serenity/handlers/prompts.js index 2897216f80..cc69524d2e 100644 --- a/src/support/serenity/handlers/prompts.js +++ b/src/support/serenity/handlers/prompts.js @@ -525,7 +525,13 @@ export async function handleCreatePrompts( // per-item injection below (a per-item LLM call would be far too slow). const intentByText = await classifyPromptIntents( inputs.map((raw) => String(raw?.text || '')), - { env, log, deadline: writeDeadline }, + { + env, + log, + deadline: writeDeadline, + writePath: deferPublish ? 'csv' : 'create', + workspaceId: semrushWorkspaceId, + }, ); const injectComputedIntent = makeIntentInjector(transport, semrushWorkspaceId, intentByText, log); @@ -725,7 +731,9 @@ export async function handleUpdatePrompt( ); const intentByText = await classifyPromptIntents( [nextText], - { env, log, deadline: writeDeadline }, + { + env, log, deadline: writeDeadline, writePath: 'edit', workspaceId: semrushWorkspaceId, + }, ); const injectComputedIntent = makeIntentInjector(transport, semrushWorkspaceId, intentByText, log); let typed = await injectComputedType(projectId, { diff --git a/src/support/serenity/intent-classification.js b/src/support/serenity/intent-classification.js index 60cc10d2dd..7624c45009 100644 --- a/src/support/serenity/intent-classification.js +++ b/src/support/serenity/intent-classification.js @@ -13,8 +13,66 @@ // @ts-check import { createIntentClassifier, classifyIntents } from '../intent-classifier.js'; -import { INTENT_TAG } from './prompt-tags.js'; -import { SERENITY_INTENT_CATEGORY_SPEC, PER_CALL_MS } from './intent-taxonomy.js'; +import { emitMetric, resolveEnvironment } from '../metrics-emf.js'; +import { INTENT_TAG, TAG_DIMENSION, tagFor } from './prompt-tags.js'; +import { + SERENITY_INTENT_CATEGORY_SPEC, + SERENITY_INTENT_VALUES, + inspectSerenityIntent, + PER_CALL_MS, +} from './intent-taxonomy.js'; + +/** + * CloudWatch namespace for Serenity write-path intent-classification metrics + * (serenity-docs#32 Observability). Mirrors the `Mysticat/Brands` convention in + * `src/controllers/brands.js`. + */ +const SERENITY_METRICS_NAMESPACE = 'Mysticat/Serenity'; + +// Cap on the number of soft-failure reasoning samples logged per request, so a +// low-confidence storm produces one bounded structured line, not a log flood. +const SOFT_FAILURE_LOG_CAP = 10; + +/** + * Nearest-rank percentile of a NON-EMPTY ascending-sorted numeric array. Used for + * per-call latency p50/p95 without pulling in a stats dep; the sole caller only + * invokes it once at least one call has been timed. + * + * @param {number[]} sortedAsc - non-empty values sorted ascending. + * @param {number} p - percentile in [0, 100]. + * @returns {number} + */ +function percentile(sortedAsc, p) { + const rank = Math.ceil((p / 100) * sortedAsc.length) - 1; + const idx = Math.min(sortedAsc.length - 1, Math.max(0, rank)); + return sortedAsc[idx]; +} + +/** + * Best-effort EMF emit for a single Serenity intent-classification metric. Wraps + * {@link emitMetric} (which already swallows its own errors) in an extra + * try/catch — belt-and-suspenders matching the `emitBrandDemotionBlocked` + * pattern in `brands.js` — so instrumentation can never throw into the classify + * path. + * + * @param {string} name - CloudWatch metric name. + * @param {number} value - metric value. + * @param {string} unit - CloudWatch unit ('Count', 'Milliseconds', ...). + * @param {object} dimensions - dimension key/value pairs (null values dropped). + * @param {object} env - environment (for {@link resolveEnvironment}). + */ +function emitIntentMetric(name, value, unit, dimensions, env) { + try { + emitMetric( + { + name, value, unit, dimensions, + }, + { environment: resolveEnvironment(env), namespace: SERENITY_METRICS_NAMESPACE }, + ); + } catch { + // best-effort: metric emission must never affect the classify path + } +} /** * Serenity write-path request budget (serenity-docs#32) — a single shared @@ -75,14 +133,27 @@ function remainingClassifyBudget(deadline, now = Date.now()) { * Every input text is guaranteed a value in the returned map — never missing — * so callers never need a separate "no tag" branch. * + * Emits best-effort observability (serenity-docs#32) — outcome counters, + * value distribution, per-call/batch latency, timeout/`budget_skipped` and + * `prod_llm_unavailable` — via EMF, dimensioned by `writePath` (and workspace on + * the outcome counters). All emission is best-effort and never throws into the + * classify path; the classify/retry/default decision logic is unchanged. + * * @param {string[]} texts - prompt texts to classify (deduplicated internally). * @param {object} options * @param {object} [options.env] - environment (Azure OpenAI creds). * @param {object} [options.log] - logger. * @param {number} options.deadline - the request's write deadline (epoch ms). + * @param {string} [options.writePath] - which write-path this call serves, for + * the metric `WritePath` dimension: `'create' | 'edit' | 'csv' | 'ai-gen'`. + * @param {string} [options.workspaceId] - the Semrush workspace id, used as the + * per-customer `Workspace` dimension on the outcome counters so one catalog's + * degradation isn't hidden under a healthy aggregate. * @returns {Promise>} text -> `intent:` wire tag. */ -export async function classifyPromptIntents(texts, { env, log = console, deadline }) { +export async function classifyPromptIntents(texts, { + env, log = console, deadline, writePath = 'unknown', workspaceId, +}) { // `env`/`log` may arrive explicitly `null` (some callers default optional // options that way) — a default *parameter* only applies on `undefined`, so // normalize here rather than relying on destructuring defaults. @@ -95,37 +166,138 @@ export async function classifyPromptIntents(texts, { env, log = console, deadlin } const counts = { - classified_ok: 0, retry_attempted: 0, retry_succeeded: 0, defaulted: 0, budget_skipped: 0, + classified_ok: 0, + low_confidence: 0, + retry_attempted: 0, + retry_succeeded: 0, + defaulted: 0, + budget_skipped: 0, }; + // Distribution of the emitted classified wire tags (excludes `defaulted`, + // which is reported as its own bucket). Keyed by wire tag. + const valueCounts = Object.create(null); + // Per-call LLM latencies (ms) and a heuristic per-call timeout tally. + const callDurations = []; + let callTimeouts = 0; + let batchStart = 0; + // Truncated soft-failure reasoning samples for one bounded structured log line. + const softFailures = []; const defaultAll = (list) => { list.forEach((t) => result.set(t, INTENT_TAG.INFORMATIONAL)); counts.defaulted += list.length; }; + const resolve = (t, value, ok) => { + result.set(t, value); + counts[ok] += 1; + valueCounts[value] = (valueCounts[value] || 0) + 1; + }; + + // Emit all per-request EMF metrics from the current counters/timings. Called + // on every terminal path so the classified-vs-fallback ratio is always + // derivable. Best-effort: each emit is individually wrapped. + const emitObservability = () => { + const outcomeDims = { WritePath: writePath, Workspace: workspaceId }; + Object.entries(counts).forEach(([outcome, count]) => { + if (count > 0) { + emitIntentMetric('IntentOutcome', count, 'Count', { ...outcomeDims, Outcome: outcome }, safeEnv); + } + }); + SERENITY_INTENT_VALUES.forEach((word) => { + const tag = tagFor(TAG_DIMENSION.INTENT, word); + const c = valueCounts[tag] || 0; + if (c > 0) { + emitIntentMetric('IntentValueDistribution', c, 'Count', { WritePath: writePath, Value: word }, safeEnv); + } + }); + if (counts.defaulted > 0) { + emitIntentMetric('IntentValueDistribution', counts.defaulted, 'Count', { WritePath: writePath, Value: 'defaulted' }, safeEnv); + } + if (callDurations.length > 0) { + const sorted = [...callDurations].sort((a, b) => a - b); + emitIntentMetric('PerCallLatencyP50Ms', percentile(sorted, 50), 'Milliseconds', { WritePath: writePath }, safeEnv); + emitIntentMetric('PerCallLatencyP95Ms', percentile(sorted, 95), 'Milliseconds', { WritePath: writePath }, safeEnv); + if (callTimeouts > 0) { + emitIntentMetric('PerCallTimeout', callTimeouts, 'Count', { WritePath: writePath }, safeEnv); + } + } + if (batchStart > 0) { + emitIntentMetric('ClassifyBatchDurationMs', Date.now() - batchStart, 'Milliseconds', { WritePath: writePath }, safeEnv); + } + }; + + const emitProdLlmUnavailable = (message) => { + emitIntentMetric('ProdLlmUnavailable', 1, 'Count', { WritePath: writePath }, safeEnv); + safeLog?.warn?.(`WARN: prod_llm_unavailable — ${message}`); + }; + if (remainingClassifyBudget(deadline) < PER_CALL_MS) { counts.budget_skipped = unique.length; defaultAll(unique); safeLog?.info?.('serenity intent classification: budget_skipped (no room at entry)', { count: unique.length }); + emitObservability(); return result; } + // Per-request category spec: wraps the taxonomy's `inspectSerenityIntent` so + // the caller can count `low_confidence` soft-failures apart from other + // unresolved cases and capture their `reasoning`, while still returning the + // same `string|null` tag the retry/default ladder expects (behavior unchanged). + const observedSpec = { + systemPrompt: SERENITY_INTENT_CATEGORY_SPEC.systemPrompt, + invokeTimeoutMs: SERENITY_INTENT_CATEGORY_SPEC.invokeTimeoutMs, + parseResult: (parsed) => { + const { tag, reason, reasoning } = inspectSerenityIntent(parsed); + if (reason !== 'ok') { + if (reason === 'low_confidence') { + counts.low_confidence += 1; + } + if (softFailures.length < SOFT_FAILURE_LOG_CAP) { + softFailures.push({ reason, reasoning: String(reasoning || '').slice(0, 200) }); + } + } + return tag; + }, + }; + const classify = createIntentClassifier( { env: safeEnv, log: safeLog }, - SERENITY_INTENT_CATEGORY_SPEC, + observedSpec, ); if (typeof classify !== 'function') { defaultAll(unique); const message = 'serenity intent classification: Azure OpenAI is not configured; defaulting to Informational'; - if (safeEnv.AWS_ENV === 'prod') { - safeLog?.warn?.(`WARN: prod_llm_unavailable — ${message}`); + if (resolveEnvironment(safeEnv) === 'prod') { + emitProdLlmUnavailable(message); } else { safeLog?.info?.(message); } + emitObservability(); return result; } - const firstPass = await classifyIntents(classify, unique, { + // Wrap the single-text classifier to record per-call latency (and a heuristic + // per-call timeout: a null result that took ~the full per-call budget). The + // classifier itself is best-effort and never rejects; the try/finally is + // defensive so timing is recorded even if that contract ever changes. + const timedClassify = async (text) => { + const t0 = Date.now(); + let r = null; + try { + r = await classify(text); + return r; + } finally { + const dt = Date.now() - t0; + callDurations.push(dt); + if (r === null && dt >= PER_CALL_MS) { + callTimeouts += 1; + } + } + }; + + batchStart = Date.now(); + const firstPass = await classifyIntents(timedClassify, unique, { maxConcurrency: CLASSIFY_CONCURRENCY, timeoutMs: remainingClassifyBudget(deadline), }); @@ -133,8 +305,7 @@ export async function classifyPromptIntents(texts, { env, log = console, deadlin unique.forEach((t) => { const value = firstPass.get(t); if (value) { - result.set(t, value); - counts.classified_ok += 1; + resolve(t, value, 'classified_ok'); } else { stillUnresolved.push(t); } @@ -142,7 +313,7 @@ export async function classifyPromptIntents(texts, { env, log = console, deadlin if (stillUnresolved.length > 0 && remainingClassifyBudget(deadline) >= PER_CALL_MS) { counts.retry_attempted = stillUnresolved.length; - const retryPass = await classifyIntents(classify, stillUnresolved, { + const retryPass = await classifyIntents(timedClassify, stillUnresolved, { maxConcurrency: CLASSIFY_CONCURRENCY, timeoutMs: remainingClassifyBudget(deadline), }); @@ -150,8 +321,7 @@ export async function classifyPromptIntents(texts, { env, log = console, deadlin stillUnresolved.forEach((t) => { const value = retryPass.get(t); if (value) { - result.set(t, value); - counts.retry_succeeded += 1; + resolve(t, value, 'retry_succeeded'); } else { stillUnresolvedAfterRetry.push(t); } @@ -161,6 +331,22 @@ export async function classifyPromptIntents(texts, { env, log = console, deadlin defaultAll(stillUnresolved); } + // Repeated-invoke-failure signal (serenity-docs#32): calls were actually made + // (the classifier was constructed) but not a single one resolved — the LLM is + // effectively unavailable, distinct from `budget_skipped` where no call runs. + if (callDurations.length > 0 + && counts.classified_ok + counts.retry_succeeded === 0 + && resolveEnvironment(safeEnv) === 'prod') { + emitProdLlmUnavailable('running on prod but the intent LLM returned no classifications for any prompt; prompts are being assigned the Informational default, not classified'); + } + + if (softFailures.length > 0) { + safeLog?.info?.('serenity intent classification soft failures', { + low_confidence: counts.low_confidence, + samples: softFailures, + }); + } safeLog?.info?.('serenity intent classification summary', counts); + emitObservability(); return result; } diff --git a/src/support/serenity/intent-taxonomy.js b/src/support/serenity/intent-taxonomy.js index 1c0fe0c07c..274cee1df6 100644 --- a/src/support/serenity/intent-taxonomy.js +++ b/src/support/serenity/intent-taxonomy.js @@ -82,6 +82,48 @@ Output requirements (strict): Reply with ONLY valid JSON. Response is limited to Output format: {"intent": "", "confidence": 0.0-1.0, "reasoning": ""} Do not include markdown, code fences, or any text outside the JSON object.`; +/** + * @typedef {object} SerenityIntentInspection + * @property {string|null} tag - the wire tag (`intent:`) or null on any + * soft failure. + * @property {'ok'|'invalid_value'|'low_confidence'} reason - why the result is + * what it is: `ok` (usable tag), `invalid_value` (garbled / unrecognized + * value), or `low_confidence` (recognized value below the validity floor). + * @property {number} confidence - the parsed confidence (NaN if unparseable). + * @property {string} reasoning - the model's `reasoning` field ('' if absent), + * surfaced so the caller can log it on soft failures without re-running. + */ + +/** + * Inspects a parsed model response against the Serenity taxonomy, distinguishing + * the soft-failure modes the {@link parseSerenityIntent} boolean result collapses + * away — so the caller's observability layer can count `low_confidence` apart + * from other unresolved cases and log the `reasoning` for debuggability. The + * retry/default behavior is unchanged: any non-`ok` reason still yields a null + * `tag`, which the caller folds into the same fallback ladder. + * + * @param {object} parsed - the parsed `{intent, confidence, reasoning}` body. + * @returns {SerenityIntentInspection} + */ +export function inspectSerenityIntent(parsed) { + const value = String(parsed?.intent ?? ''); + const confidence = Number(parsed?.confidence); + const reasoning = typeof parsed?.reasoning === 'string' ? parsed.reasoning : ''; + if (!SERENITY_INTENT_VALUES.includes(value)) { + return { + tag: null, reason: 'invalid_value', confidence, reasoning, + }; + } + if (!Number.isFinite(confidence) || confidence < PROMPT_INTENT_MIN_CONFIDENCE) { + return { + tag: null, reason: 'low_confidence', confidence, reasoning, + }; + } + return { + tag: tagFor(TAG_DIMENSION.INTENT, value), reason: 'ok', confidence, reasoning, + }; +} + /** * Validates a parsed model response against the Serenity taxonomy: the value * must be one of the 5 canonical Capitalized literals AND confidence must meet @@ -95,15 +137,7 @@ Do not include markdown, code fences, or any text outside the JSON object.`; * @returns {string|null} the wire tag (`intent:`) or null. */ export function parseSerenityIntent(parsed) { - const value = String(parsed?.intent ?? ''); - const confidence = Number(parsed?.confidence); - if (!SERENITY_INTENT_VALUES.includes(value)) { - return null; - } - if (!Number.isFinite(confidence) || confidence < PROMPT_INTENT_MIN_CONFIDENCE) { - return null; - } - return tagFor(TAG_DIMENSION.INTENT, value); + return inspectSerenityIntent(parsed).tag; } /** diff --git a/test/support/serenity/intent-classification.test.js b/test/support/serenity/intent-classification.test.js index d6229dc2f4..7b395a1e63 100644 --- a/test/support/serenity/intent-classification.test.js +++ b/test/support/serenity/intent-classification.test.js @@ -16,17 +16,64 @@ import esmock from 'esmock'; import { CREATE_PUBLISH_RESERVE_MS } from '../../../src/support/serenity/intent-classification.js'; import { PER_CALL_MS } from '../../../src/support/serenity/intent-taxonomy.js'; +import { classifyIntents as realClassifyIntents } from '../../../src/support/intent-classifier.js'; +import { resolveEnvironment as realResolveEnvironment } from '../../../src/support/metrics-emf.js'; const log = { info: sinon.stub(), warn: sinon.stub(), error: sinon.stub(), debug: sinon.stub(), }; +// Shared EMF spy; a fresh reference per load so assertions target one call log. +let emitMetricSpy; + +function metricsMock() { + emitMetricSpy = sinon.spy(); + return { + '../../../src/support/metrics-emf.js': { + emitMetric: emitMetricSpy, + resolveEnvironment: realResolveEnvironment, + }, + }; +} + +// Collects the metrics emitted for a given name into {value, dimensions} rows. +function emittedMetric(name) { + return emitMetricSpy.getCalls() + .map((c) => c.args[0]) + .filter((m) => m.name === name); +} + async function loadWithClassifier({ classify, classifyIntentsStub } = {}) { return esmock('../../../src/support/serenity/intent-classification.js', { '../../../src/support/intent-classifier.js': { createIntentClassifier: sinon.stub().returns(classify), classifyIntents: classifyIntentsStub || sinon.stub().resolves(new Map()), }, + ...metricsMock(), + }); +} + +// Loads the module wired to the REAL batch runner (`classifyIntents`) so the +// per-call timing wrapper and the injected category spec's `parseResult` (which +// counts `low_confidence` soft-failures) are actually exercised. `classifyByText` +// maps a prompt text to the parsed model body `parseResult` will inspect (or +// `null`/undefined to simulate a call that produced nothing). +async function loadWithRealBatchRaw({ makeClassify }) { + return esmock('../../../src/support/serenity/intent-classification.js', { + '../../../src/support/intent-classifier.js': { + createIntentClassifier: (_ctx, spec) => makeClassify(spec), + classifyIntents: realClassifyIntents, + }, + ...metricsMock(), + }); +} + +async function loadWithRealBatch({ classifyByText }) { + return loadWithRealBatchRaw({ + makeClassify: (spec) => async (text) => { + const parsed = classifyByText(text); + return parsed === undefined || parsed === null ? null : spec.parseResult(parsed); + }, }); } @@ -171,3 +218,191 @@ describe('intent-classification.js — classifyPromptIntents (serenity-docs#32)' } }); }); + +describe('intent-classification.js — observability (serenity-docs#32)', () => { + afterEach(() => { + sinon.reset(); + }); + + it('emits IntentOutcome counters dimensioned by WritePath + Workspace, and IntentValueDistribution by WritePath', async () => { + const classifyIntentsStub = sinon.stub().resolves(new Map([ + ['a', 'intent:Task'], ['b', 'intent:Task'], ['c', 'intent:Commercial'], + ])); + const { classifyPromptIntents } = await loadWithClassifier({ + classify: () => {}, classifyIntentsStub, + }); + await classifyPromptIntents(['a', 'b', 'c'], { + env: { AWS_ENV: 'stage' }, + log, + deadline: Date.now() + 100000, + writePath: 'create', + workspaceId: 'ws-42', + }); + + const outcome = emittedMetric('IntentOutcome').find((m) => m.dimensions.Outcome === 'classified_ok'); + expect(outcome).to.include({ value: 3, unit: 'Count' }); + expect(outcome.dimensions).to.include({ WritePath: 'create', Workspace: 'ws-42', Outcome: 'classified_ok' }); + + const values = emittedMetric('IntentValueDistribution'); + const task = values.find((m) => m.dimensions.Value === 'Task'); + const commercial = values.find((m) => m.dimensions.Value === 'Commercial'); + expect(task).to.include({ value: 2 }); + expect(task.dimensions).to.include({ WritePath: 'create', Value: 'Task' }); + expect(task.dimensions).to.not.have.property('Workspace'); + expect(commercial).to.include({ value: 1 }); + // Environment resolves via AWS_ENV for the emit sink options (not a dimension here). + expect(realResolveEnvironment({ AWS_ENV: 'stage' })).to.equal('stage'); + }); + + it('emits a budget_skipped IntentOutcome and a defaulted value bucket on the hard skip-gate', async () => { + const { classifyPromptIntents, computeWriteDeadline } = await loadWithClassifier({ + classify: () => {}, + }); + const deadline = computeWriteDeadline(Date.now() - 100000); + await classifyPromptIntents(['a', 'b'], { + env: {}, log, deadline, writePath: 'csv', workspaceId: 'ws-1', + }); + const skipped = emittedMetric('IntentOutcome').find((m) => m.dimensions.Outcome === 'budget_skipped'); + expect(skipped).to.include({ value: 2 }); + const defaulted = emittedMetric('IntentValueDistribution').find((m) => m.dimensions.Value === 'defaulted'); + expect(defaulted).to.include({ value: 2 }); + }); + + it('emits a ProdLlmUnavailable counter (and keeps the warn) when the classifier cannot be constructed in prod', async () => { + const { classifyPromptIntents } = await loadWithClassifier({ classify: null }); + await classifyPromptIntents(['a'], { + env: { AWS_ENV: 'prod' }, log, deadline: Date.now() + 100000, writePath: 'create', + }); + const prodUnavailable = emittedMetric('ProdLlmUnavailable'); + expect(prodUnavailable).to.have.length(1); + expect(prodUnavailable[0]).to.include({ value: 1 }); + expect(prodUnavailable[0].dimensions).to.include({ WritePath: 'create' }); + expect(log.warn).to.have.been.calledWithMatch(/prod_llm_unavailable/); + }); + + it('does NOT emit ProdLlmUnavailable on the construction failure in non-prod', async () => { + const { classifyPromptIntents } = await loadWithClassifier({ classify: null }); + await classifyPromptIntents(['a'], { + env: { ENV: 'stage' }, log, deadline: Date.now() + 100000, writePath: 'create', + }); + expect(emittedMetric('ProdLlmUnavailable')).to.have.length(0); + }); + + it('counts low_confidence apart from defaulted, logs truncated reasoning, and folds it into the retry/default ladder', async () => { + // 'a' resolves; 'b' is a below-floor low-confidence soft failure on every pass. + const parsedByText = { + a: { intent: 'Task', confidence: 0.97, reasoning: 'clear delegation' }, + b: { + intent: 'Commercial', + confidence: 0.2, + reasoning: 'x'.repeat(500), + }, + }; + const { classifyPromptIntents } = await loadWithRealBatch({ + classifyByText: (t) => parsedByText[t], + }); + const result = await classifyPromptIntents(['a', 'b'], { + env: {}, log, deadline: Date.now() + 100000, writePath: 'create', workspaceId: 'ws-9', + }); + expect(result.get('a')).to.equal('intent:Task'); + expect(result.get('b')).to.equal('intent:Informational'); // defaulted after retry + + // low_confidence counted per soft-failure call (pass + retry = 2), distinct from defaulted (1). + expect(log.info).to.have.been.calledWithMatch( + /summary/, + sinon.match({ low_confidence: 2, defaulted: 1, classified_ok: 1 }), + ); + const lc = emittedMetric('IntentOutcome').find((m) => m.dimensions.Outcome === 'low_confidence'); + expect(lc).to.include({ value: 2 }); + + // Truncated reasoning is logged (sliced to 200 chars), not the full 500. + const softLog = log.info.getCalls().find((c) => /soft failures/.test(c.args[0])); + expect(softLog).to.not.equal(undefined); + expect(softLog.args[1].samples[0].reasoning).to.have.length(200); + expect(softLog.args[1].samples[0].reason).to.equal('low_confidence'); + }); + + it('caps the number of logged soft-failure reasoning samples at 10', async () => { + const texts = Array.from({ length: 15 }, (_, i) => `t${i}`); + const { classifyPromptIntents } = await loadWithRealBatch({ + classifyByText: () => ({ intent: 'bogus-value', confidence: 0.99 }), + }); + await classifyPromptIntents(texts, { + env: {}, log, deadline: Date.now() + 100000, writePath: 'create', + }); + const softLog = log.info.getCalls().find((c) => /soft failures/.test(c.args[0])); + expect(softLog.args[1].samples).to.have.length(10); + // invalid_value is logged but is NOT counted as low_confidence. + expect(softLog.args[1].samples[0].reason).to.equal('invalid_value'); + expect(softLog.args[1].low_confidence).to.equal(0); + }); + + it('emits per-call latency (p50/p95), a per-call timeout tally, and a prod repeated-invoke-failure signal', async () => { + const t0 = 1_000_000_000; + const clock = sinon.useFakeTimers(t0); + try { + // Every call "spends" the full per-call budget and resolves null → a + // heuristic timeout, and no text ever resolves → repeated-invoke-failure. + const { classifyPromptIntents } = await loadWithRealBatchRaw({ + makeClassify: () => async () => { + clock.tick(PER_CALL_MS); + return null; + }, + }); + const result = await classifyPromptIntents(['a'], { + env: { AWS_ENV: 'prod' }, + log, + deadline: t0 + 100000, + writePath: 'edit', + workspaceId: 'ws-7', + }); + expect(result.get('a')).to.equal('intent:Informational'); + + const p50 = emittedMetric('PerCallLatencyP50Ms'); + const p95 = emittedMetric('PerCallLatencyP95Ms'); + expect(p50[0]).to.include({ value: PER_CALL_MS, unit: 'Milliseconds' }); + expect(p95[0]).to.include({ value: PER_CALL_MS, unit: 'Milliseconds' }); + + const timeouts = emittedMetric('PerCallTimeout'); + // first pass + retry, both timed out. + expect(timeouts[0]).to.include({ value: 2 }); + + const batch = emittedMetric('ClassifyBatchDurationMs'); + expect(batch[0].unit).to.equal('Milliseconds'); + + const prodUnavailable = emittedMetric('ProdLlmUnavailable'); + expect(prodUnavailable).to.have.length(1); + expect(prodUnavailable[0].dimensions).to.include({ WritePath: 'edit' }); + } finally { + clock.restore(); + } + }); + + it('does NOT emit the repeated-invoke-failure signal in non-prod even when nothing resolves', async () => { + const { classifyPromptIntents } = await loadWithRealBatch({ + classifyByText: () => null, + }); + await classifyPromptIntents(['a'], { + env: { AWS_ENV: 'stage' }, log, deadline: Date.now() + 100000, writePath: 'create', + }); + expect(emittedMetric('ProdLlmUnavailable')).to.have.length(0); + }); + + it('never throws into the classify path when emitMetric itself throws', async () => { + const classifyIntentsStub = sinon.stub().resolves(new Map([['a', 'intent:Task']])); + const mod = await esmock('../../../src/support/serenity/intent-classification.js', { + '../../../src/support/intent-classifier.js': { + createIntentClassifier: sinon.stub().returns(() => {}), + classifyIntents: classifyIntentsStub, + }, + '../../../src/support/metrics-emf.js': { + emitMetric: () => { throw new Error('emf boom'); }, + resolveEnvironment: realResolveEnvironment, + }, + }); + const result = await mod.classifyPromptIntents(['a'], { + env: {}, log, deadline: Date.now() + 100000, writePath: 'create', + }); + expect(result.get('a')).to.equal('intent:Task'); + }); +}); diff --git a/test/support/serenity/intent-taxonomy.test.js b/test/support/serenity/intent-taxonomy.test.js index 6675322fda..15ec66b9c4 100644 --- a/test/support/serenity/intent-taxonomy.test.js +++ b/test/support/serenity/intent-taxonomy.test.js @@ -14,6 +14,7 @@ import { expect } from 'chai'; import { parseSerenityIntent, + inspectSerenityIntent, SERENITY_INTENT_VALUES, PROMPT_INTENT_MIN_CONFIDENCE, SERENITY_INTENT_CATEGORY_SPEC, @@ -63,3 +64,38 @@ describe('intent-taxonomy.js — parseSerenityIntent (serenity-docs#32)', () => expect(SERENITY_INTENT_CATEGORY_SPEC.systemPrompt).to.be.a('string').with.length.greaterThan(0); }); }); + +describe('intent-taxonomy.js — inspectSerenityIntent (serenity-docs#32 observability)', () => { + it('returns ok with the wire tag and surfaced fields for a valid, confident result', () => { + const r = inspectSerenityIntent({ intent: 'Task', confidence: 0.97, reasoning: 'delegated pick' }); + expect(r).to.deep.equal({ + tag: 'intent:Task', reason: 'ok', confidence: 0.97, reasoning: 'delegated pick', + }); + }); + + it('flags a recognized value below the confidence floor as low_confidence, still tag null', () => { + const r = inspectSerenityIntent({ + intent: 'Commercial', confidence: PROMPT_INTENT_MIN_CONFIDENCE - 0.01, reasoning: 'unsure', + }); + expect(r.tag).to.equal(null); + expect(r.reason).to.equal('low_confidence'); + expect(r.reasoning).to.equal('unsure'); + }); + + it('flags an unrecognized/garbled value as invalid_value before the confidence check', () => { + expect(inspectSerenityIntent({ intent: 'nope', confidence: 0.99 }).reason).to.equal('invalid_value'); + expect(inspectSerenityIntent({}).reason).to.equal('invalid_value'); + expect(inspectSerenityIntent(null).reason).to.equal('invalid_value'); + }); + + it('defaults reasoning to an empty string when absent or non-string', () => { + expect(inspectSerenityIntent({ intent: 'Task', confidence: 1 }).reasoning).to.equal(''); + expect(inspectSerenityIntent({ intent: 'Task', confidence: 1, reasoning: 42 }).reasoning).to.equal(''); + }); + + it('parseSerenityIntent is the tag projection of inspectSerenityIntent', () => { + const parsed = { intent: 'Navigational', confidence: 0.9, reasoning: 'go to site' }; + expect(parseSerenityIntent(parsed)).to.equal(inspectSerenityIntent(parsed).tag); + expect(parseSerenityIntent({ intent: 'x' })).to.equal(inspectSerenityIntent({ intent: 'x' }).tag); + }); +}); From 0b0c0ce276d0f36f272fafeffd2d8ea1216bf80f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Tue, 14 Jul 2026 11:35:32 +0200 Subject: [PATCH 06/16] test(serenity): assert soft-failure log carries no prompt text Co-Authored-By: Claude Sonnet 5 --- .../serenity/intent-classification.test.js | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/support/serenity/intent-classification.test.js b/test/support/serenity/intent-classification.test.js index 7b395a1e63..5862598329 100644 --- a/test/support/serenity/intent-classification.test.js +++ b/test/support/serenity/intent-classification.test.js @@ -280,6 +280,17 @@ describe('intent-classification.js — observability (serenity-docs#32)', () => expect(log.warn).to.have.been.calledWithMatch(/prod_llm_unavailable/); }); + it('fires the prod path (warn + counter) when prod is signalled via ENV, not just AWS_ENV', async () => { + // Regression: prod detection resolves through resolveEnvironment (AWS_ENV || ENV), + // so ENV=prod alone must still trip the prod path — the old AWS_ENV-only check missed this. + const { classifyPromptIntents } = await loadWithClassifier({ classify: null }); + await classifyPromptIntents(['a'], { + env: { ENV: 'prod' }, log, deadline: Date.now() + 100000, writePath: 'create', + }); + expect(emittedMetric('ProdLlmUnavailable')).to.have.length(1); + expect(log.warn).to.have.been.calledWithMatch(/prod_llm_unavailable/); + }); + it('does NOT emit ProdLlmUnavailable on the construction failure in non-prod', async () => { const { classifyPromptIntents } = await loadWithClassifier({ classify: null }); await classifyPromptIntents(['a'], { @@ -322,6 +333,20 @@ describe('intent-classification.js — observability (serenity-docs#32)', () => expect(softLog.args[1].samples[0].reason).to.equal('low_confidence'); }); + it('logs only {reason, reasoning} per soft-failure sample — never the prompt text', async () => { + const secret = 'super-secret-prompt-text-that-must-not-be-logged'; + const { classifyPromptIntents } = await loadWithRealBatch({ + classifyByText: () => ({ intent: 'Commercial', confidence: 0.1, reasoning: 'low conf' }), + }); + await classifyPromptIntents([secret], { + env: {}, log, deadline: Date.now() + 100000, writePath: 'create', + }); + const softLog = log.info.getCalls().find((c) => /soft failures/.test(c.args[0])); + expect(softLog).to.not.equal(undefined); + expect(softLog.args[1].samples[0]).to.have.keys(['reason', 'reasoning']); + expect(JSON.stringify(softLog.args)).to.not.contain(secret); + }); + it('caps the number of logged soft-failure reasoning samples at 10', async () => { const texts = Array.from({ length: 15 }, (_, i) => `t${i}`); const { classifyPromptIntents } = await loadWithRealBatch({ @@ -405,4 +430,26 @@ describe('intent-classification.js — observability (serenity-docs#32)', () => }); expect(result.get('a')).to.equal('intent:Task'); }); + + it('never throws when emitMetric throws on the latency + prod_llm_unavailable paths', async () => { + // Real batch runner so the per-call timing wrapper runs (populating + // callDurations → latency emits), in prod with nothing resolving so the + // repeated-invoke-failure ProdLlmUnavailable site also fires — all while + // emitMetric throws on every call. + const mod = await esmock('../../../src/support/serenity/intent-classification.js', { + '../../../src/support/intent-classifier.js': { + createIntentClassifier: () => async () => null, + classifyIntents: realClassifyIntents, + }, + '../../../src/support/metrics-emf.js': { + emitMetric: () => { throw new Error('emf boom'); }, + resolveEnvironment: realResolveEnvironment, + }, + }); + const result = await mod.classifyPromptIntents(['a', 'b'], { + env: { AWS_ENV: 'prod' }, log, deadline: Date.now() + 100000, writePath: 'create', + }); + expect(result.get('a')).to.equal('intent:Informational'); + expect(result.get('b')).to.equal('intent:Informational'); + }); }); From a020f7bbc3d57cee65b27dc5e0f43cbf7d3c0959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Tue, 21 Jul 2026 16:18:34 +0200 Subject: [PATCH 07/16] fix(serenity): reject empty edit text and modernize edit-path tests Fix 1: parseUpdatePromptBody accepted empty/whitespace text on PATCH (only rejected `undefined`), so an edit could reach renamePrompt with a blank prompt that then got classified/defaulted. Now trims and rejects empty text with a 400, mirroring the create path (normalizePromptInput) including the `|| ''` falsy coercion. Shared with the subworkspace twin, so both edit paths are covered. Fix 2: no behavior change. The always-reclassify on edit is intentional and unavoidable: the handler is not sent the old text and upstream has no GET-by-id, so it cannot detect a no-op text edit, and classification must run before the rename for failure-safety. Documented the rationale with a code comment; added a regression test that a rename returning is_updated:false still writes tags and publishes. Fix 3: modernized the stale handleUpdatePrompt tests (and the three edit-path twins in prompts-subworkspace.test.js) that still stubbed the retired delete+create flow. They failed at runtime and one referenced an undefined `createErr`. They now exercise the rename + replace-mode tag-write flow (409 collision, non-404 rename error, tag-write failure after rename, 404 promptNotFound plus its generic-Error negative). Added the intent tag to the recompute assertions. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/support/serenity/handlers/prompts.js | 28 ++- .../handlers/prompts-subworkspace.test.js | 24 ++- .../support/serenity/handlers/prompts.test.js | 181 +++++++++++++++--- 3 files changed, 195 insertions(+), 38 deletions(-) diff --git a/src/support/serenity/handlers/prompts.js b/src/support/serenity/handlers/prompts.js index 284d6ed69d..c1cbc3b0f5 100644 --- a/src/support/serenity/handlers/prompts.js +++ b/src/support/serenity/handlers/prompts.js @@ -533,7 +533,18 @@ export function parseUpdatePromptBody(body) { }, }; } - const text = String(body.text); + // Mirror the create contract (`normalizePromptInput`): empty or whitespace-only + // text is rejected here rather than passed on to `renamePrompt`, where it would + // be classified and written as a blank prompt. `|| ''` also coerces a falsy + // non-string (`null`, `0`, `false`) to empty, matching create exactly. + const text = String(body.text || '').trim(); + if (!text) { + return { + ok: false, + status: 400, + body: { error: 'invalidRequest', message: 'text must be a non-empty string' }, + }; + } const tagIds = sanitizeTagIds(body.tagIds); if (tagIds.length === 0) { return { @@ -819,10 +830,19 @@ export async function handleUpdatePrompt( } const projectId = project.getSemrushProjectId(); - // Recompute the type AND intent tags from the NEW text BEFORE the delete: the - // unified layer (tree read / on-demand tag create / LLM classify) must not run - // between delete and create, so a classification failure aborts cleanly with + // Recompute the type AND intent tags from the NEW text BEFORE the rename: the + // unified layer (tree read / on-demand tag create / LLM classify) must run + // before any upstream write, so a classification failure aborts cleanly with // the old prompt still present (serenity-docs#31, #32). + // + // This runs UNCONDITIONALLY, even when the PATCH does not change the text. The + // upstream provider has no GET-by-id and the handler is not sent the old text + // (the body is the full next state — see the docblock above), so it cannot + // know whether the text actually changed. `renamePrompt`'s `is_updated: false` + // reports a no-op only AFTER the rename, too late to gate a classify that has + // to run first for failure-safety. Skipping the reclassification would require + // the client to send the old text — a contract change deliberately out of + // scope here (keep the edit path a single straight line). const injectComputedType = makeTypeInjector( transport, semrushWorkspaceId, diff --git a/test/support/serenity/handlers/prompts-subworkspace.test.js b/test/support/serenity/handlers/prompts-subworkspace.test.js index 4ea8617999..f6162a212e 100644 --- a/test/support/serenity/handlers/prompts-subworkspace.test.js +++ b/test/support/serenity/handlers/prompts-subworkspace.test.js @@ -339,7 +339,7 @@ describe('prompts-subworkspace handlers', () => { expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( WS, 'p-us-en', - [{ id: 'old-id', references: ['tag-1'], replace: true }], + [{ id: 'old-id', references: ['tag-1', TAG_IDS.intentInformational], replace: true }], ); expect(transport.deletePromptsByIds).to.not.have.been.called; expect(transport.createPromptsByIds).to.not.have.been.called; @@ -398,10 +398,15 @@ describe('prompts-subworkspace handlers', () => { text: 'new', tagIds: ['tag-cat-1', '', undefined], geoTargetId: 2840, languageCode: 'en', }, log); expect(result.status).to.equal(200); - expect(result.body.semrushPromptId).to.equal('new-prompt-by-id'); + // The id is preserved — the edit is in place, never a re-create. + expect(result.body.semrushPromptId).to.equal('old-id'); expect(result.body.tagIds).to.deep.equal(['tag-cat-1', TAG_IDS.intentInformational]); - expect(transport.deletePromptsByIds).to.have.been.calledWith(WS, 'p-us-en', ['old-id']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['new'], ['tag-cat-1', TAG_IDS.intentInformational]); + expect(transport.renamePrompt).to.have.been.calledOnceWithExactly(WS, 'p-us-en', 'old-id', 'new'); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WS, + 'p-us-en', + [{ id: 'old-id', references: ['tag-cat-1', TAG_IDS.intentInformational], replace: true }], + ); }); it('400s when the slice key is invalid', async () => { @@ -436,8 +441,15 @@ describe('prompts-subworkspace handlers', () => { expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( WS, 'p-us-en', - ['now mentions Acme'], - [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.intentInformational], + [{ + id: 'old-id', + references: [ + TAG_IDS.categoryRunningShoes, + TAG_IDS.typeBranded, + TAG_IDS.intentInformational, + ], + replace: true, + }], ); }); diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index e084217421..959b250f2e 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -1004,6 +1004,49 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { expect(result.body.error).to.equal('invalidRequest'); }); + // Mirror of the create-path contract (normalizePromptInput trims + rejects + // empty): an edit must not slip an empty prompt past validation into the + // rename, where it would be classified and written blank. + it('400s when text is an empty string', async () => { + const dataAccess = makeDataAccess([]); + + const result = await handleUpdatePrompt( + {}, + dataAccess, + BRAND, + WORKSPACE, + 'sem-1', + { + geoTargetId: 2840, languageCode: 'en', text: '', tagIds: ['tag-1'], + }, + fakeLog(), + ); + + expect(result.status).to.equal(400); + expect(result.body.error).to.equal('invalidRequest'); + expect(result.body.message).to.match(/non-empty/); + }); + + it('400s when text is whitespace-only', async () => { + const dataAccess = makeDataAccess([]); + + const result = await handleUpdatePrompt( + {}, + dataAccess, + BRAND, + WORKSPACE, + 'sem-1', + { + geoTargetId: 2840, languageCode: 'en', text: ' ', tagIds: ['tag-1'], + }, + fakeLog(), + ); + + expect(result.status).to.equal(400); + expect(result.body.error).to.equal('invalidRequest'); + expect(result.body.message).to.match(/non-empty/); + }); + it('edits text+tagIds in place (rename + replace-mode tag write), id unchanged', async () => { const project = makeProject({ semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', @@ -1013,10 +1056,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().resolves({ - page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'next' }], existing_count: 0, - }), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'next', is_updated: true }), + updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), }; @@ -1041,8 +1082,53 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { text: 'next', tagIds: ['tag-cat-1', TAG_IDS.intentInformational], }); - expect(transport.deletePromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['sem-1']); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['tag-cat-1', TAG_IDS.intentInformational]); + expect(transport.renamePrompt).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', 'sem-1', 'next'); + // Replace-mode tag write with the injector's full output (caller tag + intent). + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ id: 'sem-1', references: ['tag-cat-1', TAG_IDS.intentInformational], replace: true }], + ); + }); + + // Guards the documented always-reclassify invariant: an unchanged-text edit + // comes back from rename as `is_updated: false`, but the handler still writes + // the (re-classified) tag set and publishes — it has no old text and upstream + // has no GET-by-id, so it cannot short-circuit. A future "optimization" that + // skips the tag write on is_updated:false would break this and fail here. + it('still writes tags + publishes when rename reports is_updated:false (no-op text)', async () => { + const project = makeProject({ + semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', + }); + const dataAccess = makeDataAccess([]); + dataAccess.BrandSemrushProject.findBySlice.resolves(project); + + const transport = { + listProjectTags: makeListProjectTagsStub(), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'same', is_updated: false }), + updatePromptTagsByIds: sinon.stub().resolves(null), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleUpdatePrompt( + transport, + dataAccess, + BRAND, + WORKSPACE, + 'sem-1', + { + geoTargetId: 2840, languageCode: 'en', text: 'same', tagIds: ['tag-cat-1'], + }, + fakeLog(), + ); + + expect(result.status).to.equal(200); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ id: 'sem-1', references: ['tag-cat-1', TAG_IDS.intentInformational], replace: true }], + ); + expect(transport.publishProject).to.have.been.called; }); it('drops falsy tagIds entries on PATCH before the tag write', async () => { @@ -1054,10 +1140,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().resolves({ - page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'next' }], existing_count: 0, - }), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'next', is_updated: true }), + updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), }; @@ -1074,7 +1158,11 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { ); expect(result.body.tagIds).to.deep.equal(['keep', TAG_IDS.intentInformational]); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['keep', TAG_IDS.intentInformational]); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ id: 'sem-1', references: ['keep', TAG_IDS.intentInformational], replace: true }], + ); }); it('drops malformed tagIds entries on PATCH like validateParentIdFormat does for parentId', async () => { @@ -1086,10 +1174,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().resolves({ - page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'next' }], - }), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'next', is_updated: true }), + updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), }; const tooLong = 'x'.repeat(201); @@ -1110,7 +1196,11 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { ); expect(result.body.tagIds).to.deep.equal(['keep', TAG_IDS.intentInformational]); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['next'], ['keep', TAG_IDS.intentInformational]); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ id: 'sem-1', references: ['keep', TAG_IDS.intentInformational], replace: true }], + ); }); it('400s when tagIds sanitizes to empty (every entry malformed)', async () => { @@ -1220,14 +1310,14 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const dataAccess = makeDataAccess([]); dataAccess.BrandSemrushProject.findBySlice.resolves(project); - // The prompt-gone 404 is gated by isUpstreamGone which requires - // SerenityTransportError specifically. A generic Error with .status=404 - // must NOT trip the promptNotFound path. + // The prompt-gone 404 is gated by isUpstreamGone, which requires a + // SerenityTransportError. A rename that reports the prompt is gone maps to + // promptNotFound and never reaches the tag write. const err = new SerenityTransportError(404, 'not found'); const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().rejects(err), - createPromptsByIds: sinon.stub(), + renamePrompt: sinon.stub().rejects(err), + updatePromptTagsByIds: sinon.stub(), }; const result = await handleUpdatePrompt( @@ -1247,6 +1337,37 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { expect(transport.updatePromptTagsByIds).to.have.callCount(0); }); + // The negative of the case above: isUpstreamGone requires a + // SerenityTransportError, so a generic Error carrying .status=404 must NOT be + // treated as promptNotFound — it propagates like any other upstream failure. + it('generic Error with status 404 → throws (not promptNotFound)', async () => { + const project = makeProject({ + semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', + }); + const dataAccess = makeDataAccess([]); + dataAccess.BrandSemrushProject.findBySlice.resolves(project); + + const err = Object.assign(new Error('plain 404'), { status: 404 }); + const transport = { + listProjectTags: makeListProjectTagsStub(), + renamePrompt: sinon.stub().rejects(err), + updatePromptTagsByIds: sinon.stub(), + }; + + await expect(handleUpdatePrompt( + transport, + dataAccess, + BRAND, + WORKSPACE, + 'sem-1', + { + geoTargetId: 2840, languageCode: 'en', text: 'x', tagIds: ['tag-1'], + }, + fakeLog(), + )).to.be.rejectedWith(/plain 404/); + expect(transport.updatePromptTagsByIds).to.have.callCount(0); + }); + // The collision contract (serenity-docs#63 decision 2): a rename onto a // sibling prompt's exact text is refused upstream with 409 and NOTHING has // mutated — the handler propagates it untouched so the controller's mapError @@ -1261,8 +1382,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const err = new SerenityTransportError(409, 'conflict'); const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().resolves({}), // no items + renamePrompt: sinon.stub().rejects(err), + updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), }; @@ -1291,8 +1412,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const err = Object.assign(new Error('upstream 503'), { status: 503 }); const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().rejects(err), - createPromptsByIds: sinon.stub().resolves({ items: [{ id: 'should-not-happen' }] }), + renamePrompt: sinon.stub().rejects(err), + updatePromptTagsByIds: sinon.stub().resolves(null), }; await expect(handleUpdatePrompt( @@ -1322,8 +1443,8 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const tagErr = Object.assign(new Error('tag write boom'), { status: 500 }); const transport = { listProjectTags: makeListProjectTagsStub(), - deletePromptsByIds: sinon.stub().resolves(), - createPromptsByIds: sinon.stub().rejects(createErr), + renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'x', is_updated: true }), + updatePromptTagsByIds: sinon.stub().rejects(tagErr), publishProject: sinon.stub().resolves(), }; @@ -1989,7 +2110,11 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) 'proj-us-en', [{ id: 'old-id', - references: [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded], + references: [ + TAG_IDS.categoryRunningShoes, + TAG_IDS.typeBranded, + TAG_IDS.intentInformational, + ], replace: true, }], ); From 279f99d27014e5ada339f3a40eb974d180ce5b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 09:43:04 +0200 Subject: [PATCH 08/16] fix(serenity): finish LLMO-6190 headroom wiring (type-check + metering) The feature branch left the dynamic-allocation headroom guard half-wired, breaking type-check + build and a metering test: - markets-subworkspace.js generateAndAttachPrompts: the caller threaded a headroom guard as a 6th arg and the JSDoc documented it as required, but the function never declared or used it (TS8024 + TS2554). Add the param and front headroom.ensure({ prompts: texts.size }, { includeDrafted: true }) before the metered createPromptsByIds write loop. - prompts-subworkspace.js handleCreatePromptsSubworkspace: removed the redundant pre-publish ensure that re-created a second headroom guard (no-shadow) and issued a second getWorkspaceResources read. LLMO-6190 meters ONCE before the write (the create is the metered write, not publish); the pre-write ensure already reserves the whole batch, so the pre-publish read was a leftover. - prompts-subworkspace.test.js: the dynamic-allocation test passed its options object in the env positional slot instead of the 8th, so the guard was never enabled; corrected to match the controller/signature. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../serenity/handlers/markets-subworkspace.js | 9 ++++++++- .../serenity/handlers/prompts-subworkspace.js | 14 -------------- .../serenity/handlers/prompts-subworkspace.test.js | 4 +++- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/support/serenity/handlers/markets-subworkspace.js b/src/support/serenity/handlers/markets-subworkspace.js index 7ba4b111c4..dc93522dff 100644 --- a/src/support/serenity/handlers/markets-subworkspace.js +++ b/src/support/serenity/handlers/markets-subworkspace.js @@ -224,7 +224,7 @@ function validateCreateBody(body) { async function generateAndAttachPrompts(transport, workspaceId, projectId, { domain, country, topicCap = 0, brandNames = [], provisioned, env, writeDeadline = computeWriteDeadline(), -}, log) { +}, log, headroom) { const raw = await transport.getBrandTopics(workspaceId, { domain, country }); let topics = []; if (Array.isArray(raw)) { @@ -313,6 +313,13 @@ async function generateAndAttachPrompts(transport, workspaceId, projectId, { } } + // PROMPT metering seam (Rainer, live-verified LLMO-6190): front headroom sized + // on the real generated prompt count (`texts.size`) BEFORE the metered + // `createPromptsByIds` writes below — the disguised-quota 405 fires there, not + // at publish. No-op when the flag is OFF; NOT optional-chained, a caller that + // forgets to thread the guard must fail loud. + await headroom.ensure({ prompts: texts.size }, { includeDrafted: true }); + for (const { items, tagIds } of byTagSet.values()) { // eslint-disable-next-line no-await-in-loop await transport.createPromptsByIds(workspaceId, projectId, items, tagIds); diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index 839c5a5c3e..bc1fd1f5ac 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -242,20 +242,6 @@ export async function handleCreatePromptsSubworkspace( invalidateTagCacheForProject(workspaceId, pid); } - // PROMPT metering seam: the just-created prompts are drafted synchronously across the affected - // projects of THIS child; size headroom from `used + drafted` (includeDrafted, staleness-immune) - // before the publish. One workspace-level top-up covers all affected projects (the allocation is - // per sub-workspace, not per project). No-op when the flag is OFF; skipped when nothing was - // created so the OFF path and the empty path issue zero headroom reads. - if (affectedProjectIds.length > 0) { - const headroom = createHeadroomGuard( - transport, - { enabled: dynamicAllocation, subWorkspaceId: workspaceId, parentWorkspaceId }, - log, - ); - await headroom.ensure({}, { includeDrafted: true }); - } - if (deferPublish) { log?.info?.('serenity create-prompts (subworkspace): deferPublish set — prompts written as draft, publish skipped', { workspaceId, created: created.length, skipped: skipped.length, failed: failed.length, diff --git a/test/support/serenity/handlers/prompts-subworkspace.test.js b/test/support/serenity/handlers/prompts-subworkspace.test.js index f6162a212e..d19e2642b6 100644 --- a/test/support/serenity/handlers/prompts-subworkspace.test.js +++ b/test/support/serenity/handlers/prompts-subworkspace.test.js @@ -202,7 +202,9 @@ describe('prompts-subworkspace handlers', () => { prompts: [{ text: 'p', tagIds: ['tag-1'], geoTargetId: 2840, languageCode: 'en', }], - }, log, undefined, { dynamicAllocation: true, parentWorkspaceId: 'parent-ws' }); + }, log, undefined, undefined, undefined, { + dynamicAllocation: true, parentWorkspaceId: 'parent-ws', + }); expect(result.created).to.have.length(1); expect(transport.getWorkspaceResources).to.have.been.calledOnceWith(WS); expect(transport.getWorkspaceResources) From 5596202017db577eea645d53ba83d23208a9961e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 10:35:59 +0200 Subject: [PATCH 09/16] fix(serenity): wire create-prompts publish through headroom.retryOnQuota MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the LLMO-6190 create-prompts fronting: handleCreatePromptsSubworkspace now passes headroom.retryOnQuota as publishAffected's wrapPublish, so a disguised metered-405 on a project publish gets one bounded top-up+retry per project before being recorded as a failure (the JSDoc on publishAffected already documented this caller contract; the wiring was missing). Reuses the single guard created before the write — no extra guard, no extra read on the success path. Also fixes the stale positional call in the dynamic-allocation-fronting test (options were passed in the env slot instead of the 8th), matching the controller and the sibling test. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/support/serenity/handlers/prompts-subworkspace.js | 11 ++++++++++- .../serenity/dynamic-allocation-fronting.test.js | 2 ++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index bc1fd1f5ac..a350ec6e6d 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -251,7 +251,16 @@ export async function handleCreatePromptsSubworkspace( }; } - const publishErrors = await publishAffected(transport, workspaceId, affectedProjectIds, log); + // Route each project's publish through the headroom guard's retryOnQuota (LLMO-6190 item 4): + // a disguised metered-405 gets ONE bounded top-up+retry per project before being recorded as a + // failure. No-op passthrough when the flag is OFF (the guard's retryOnQuota is a plain call). + const publishErrors = await publishAffected( + transport, + workspaceId, + affectedProjectIds, + log, + (fn) => headroom.retryOnQuota(fn), + ); // publishAffected returns { projectId, message } records whose message is // ALREADY redacted (redactUpstreamMessage) — pubErr is a record, not a raw error. for (const pubErr of publishErrors) { diff --git a/test/support/serenity/dynamic-allocation-fronting.test.js b/test/support/serenity/dynamic-allocation-fronting.test.js index 2dd8e95fe9..38dd001b4c 100644 --- a/test/support/serenity/dynamic-allocation-fronting.test.js +++ b/test/support/serenity/dynamic-allocation-fronting.test.js @@ -546,6 +546,8 @@ describe('dynamic-allocation fronting — retryOnQuota wiring', () => { }, log, undefined, + undefined, + undefined, { dynamicAllocation: true, parentWorkspaceId: MASTER }, ); // The retry succeeded, so the create is NOT recorded as a publish failure. From f1616012799e3bfa6288c544bafb57457918e765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 15:55:50 +0200 Subject: [PATCH 10/16] refactor(serenity): resolveIntentValueInjection as thin wrapper over resolveClosedValueInjection Collapse the duplicated body into a delegation to resolveClosedValueInjection( DIMENSION.INTENT), mirroring resolveTypeValueInjection exactly. Behavior- preserving: same param order, same { computedId, intentTagIds } shape, same fail-closed (502) path. Add the resolveIntentValueInjection unit block as a twin of the type cases (resolve+strip, on-demand create, 502, transport error). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/support/serenity/tag-tree.js | 13 ++---- test/support/serenity/tag-tree.test.js | 56 ++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/support/serenity/tag-tree.js b/src/support/serenity/tag-tree.js index 7ab1bb0b1a..b649060ae8 100644 --- a/src/support/serenity/tag-tree.js +++ b/src/support/serenity/tag-tree.js @@ -678,18 +678,13 @@ export async function resolveIntentValueInjection( wantValue, log, ) { - const roots = await ensureDimensionRoots(transport, semrushWorkspaceId, projectId, log); - const intentRootId = rootIdOf(roots, DIMENSION.INTENT); - const { byName } = await ensureChildren( + const { computedId, valueTagIds } = await resolveClosedValueInjection( transport, semrushWorkspaceId, projectId, - intentRootId, - [wantValue], + DIMENSION.INTENT, + wantValue, log, ); - return { - computedId: /** @type {string} */ (byName.get(wantValue)), - intentTagIds: [...byName.values()], - }; + return { computedId, intentTagIds: valueTagIds }; } diff --git a/test/support/serenity/tag-tree.test.js b/test/support/serenity/tag-tree.test.js index 3ca43b2961..0d12812a52 100644 --- a/test/support/serenity/tag-tree.test.js +++ b/test/support/serenity/tag-tree.test.js @@ -22,6 +22,7 @@ import { provisionDimensionTree, ensureClosedValue, resolveTypeValueInjection, + resolveIntentValueInjection, resolveClosedValueInjection, findTagsInTree, assertParentWithinDimension, @@ -573,6 +574,61 @@ describe('serenity tag-tree', () => { }); }); + // Twin of the `resolveTypeValueInjection` block above for the `intent` closed + // dimension (serenity-docs#32). Both are thin wrappers over + // `resolveClosedValueInjection`; this pins the intent-specific return key + // (`intentTagIds`) and the fail-closed contract independently of the type twin. + describe('resolveIntentValueInjection', () => { + it('returns the wanted value id plus EVERY id under the intent root', async () => { + const transport = { + listProjectTags: makeListProjectTagsStub(), + createProjectTags: sinon.stub(), + }; + const res = await resolveIntentValueInjection(transport, WS, PROJECT, 'Task', fakeLog()); + expect(res.computedId).to.equal(TAG_IDS.intentTask); + // The strip set is every intent id — the caller may not set the value itself. + expect(res.intentTagIds).to.have.members([ + TAG_IDS.intentInformational, + TAG_IDS.intentTask, + TAG_IDS.intentCommercial, + TAG_IDS.intentTransactional, + TAG_IDS.intentNavigational, + ]); + expect(transport.createProjectTags).to.not.have.been.called; + }); + + it('creates the value on demand when a project predating the taxonomy lacks it', async () => { + const { listProjectTags, createProjectTags } = makeProvisioningTransportStubs(); + const transport = { listProjectTags, createProjectTags }; + const res = await resolveIntentValueInjection(transport, WS, PROJECT, 'Task', fakeLog()); + expect(res.computedId).to.equal('created:created::intent:Task'); + expect(res.intentTagIds).to.deep.equal(['created:created::intent:Task']); + }); + + it('502s rather than skip injection when the intent root cannot be resolved', async () => { + // A prompt written without the server-computed `intent` tag stays + // unclassified forever: the client may not set that dimension itself. + const transport = { + listProjectTags: makeListProjectTagsStub({ '': [] }), + createProjectTags: sinon.stub().resolves([]), + }; + const err = await resolveIntentValueInjection(transport, WS, PROJECT, 'Task', fakeLog()) + .then(() => null, (e) => e); + expect(err).to.be.an('error'); + expect(err.status).to.equal(502); + }); + + it('propagates a transport failure while reading the tag tree', async () => { + const transport = { + listProjectTags: sinon.stub().rejects(new Error('listProjectTags 502')), + createProjectTags: sinon.stub(), + }; + await expect(resolveIntentValueInjection(transport, WS, PROJECT, 'Task', fakeLog())) + .to.be.rejectedWith(/listProjectTags 502/); + expect(transport.createProjectTags).to.not.have.been.called; + }); + }); + describe('findTagsInTree', () => { it('places several ids in one walk and reports each ancestry', async () => { const transport = { listProjectTags: makeListProjectTagsStub() }; From b8f0ec042abbbcc307842c38e150f30af30dc8b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 15:55:51 +0200 Subject: [PATCH 11/16] test(serenity): intent strip-by-id collision + DRS system-prompt parity guards Add the spec's bolded acceptance test: a customer category named Commercial survives a write that computes intent=Commercial (strip is by intent-root id, never by name). Add a byte-for-byte guard that DRS_CATEGORY_SPEC.systemPrompt is the same SYSTEM_PROMPT reference, locking the native 6-bucket prompt. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/support/intent-classifier.test.js | 9 +++++ .../support/serenity/handlers/prompts.test.js | 39 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/test/support/intent-classifier.test.js b/test/support/intent-classifier.test.js index 8705702306..c0e6b304eb 100644 --- a/test/support/intent-classifier.test.js +++ b/test/support/intent-classifier.test.js @@ -21,6 +21,8 @@ import yaml from 'js-yaml'; import { classifyIntents, contentToString, + DRS_CATEGORY_SPEC, + SYSTEM_PROMPT, } from '../../src/support/intent-classifier.js'; import { INTENT_VALUES } from '../../src/support/intent.js'; @@ -66,6 +68,13 @@ describe('intent-classifier', () => { afterEach(() => sinon.restore()); describe('createIntentClassifier', () => { + // Byte-for-byte parity guard for the native/DRS path: the 6-bucket refactor + // reuses SYSTEM_PROMPT as-is for DRS_CATEGORY_SPEC.systemPrompt, so the + // native prompt must be strictly the same string, not a copy that can drift. + it('reuses SYSTEM_PROMPT verbatim as the DRS category spec system prompt', () => { + expect(DRS_CATEGORY_SPEC.systemPrompt).to.equal(SYSTEM_PROMPT); + }); + it('returns null when Azure OpenAI is not configured', async () => { const { mod, ctorSpy } = await loadWithModel(() => ({ content: '{}' })); const classify = mod.createIntentClassifier({ env: {}, log }); diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index ad5fb2a367..667ab043d9 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -2265,6 +2265,45 @@ describe('handlers/prompts.js — unified intent classification (serenity-docs#3 expect(transport.listProjectTags).to.have.callCount(4); expect(c.tagIds).to.deep.equal(['z', TAG_IDS.intentCommercial]); }); + + // Strip-by-root, not strip-by-name (the intent twin of the `type` collision + // gate): a customer category may legitimately be named `Commercial` without + // being the `intent` value. Only ids beneath the `intent` root are the + // server's to overwrite, even when the computed intent shares that bare name. + it('leaves a same-named category tag alone while stripping the real intent value', async () => { + const decoyCategoryId = 'category-commercial-decoy'; + const levels = dimensionTreeLevels(); + levels[TAG_IDS.categoryRoot] = [ + ...levels[TAG_IDS.categoryRoot], + { + id: decoyCategoryId, + name: 'Commercial', + parent_id: TAG_IDS.categoryRoot, + children_count: 0, + path: [{ id: TAG_IDS.categoryRoot, name: 'category' }], + }, + ]; + const transport = { + listProjectTags: makeListProjectTagsStub(levels), + createProjectTags: sinon.stub(), + }; + // The server classifies this text's intent as `Commercial`. + const intentByText = new Map([['is Acme worth it?', 'Commercial']]); + const inject = makeIntentInjector(transport, WORKSPACE, intentByText, fakeLog()); + + // The caller supplies the decoy CATEGORY named `Commercial` plus a stale + // `intent` id; only the intent-root id is stripped and replaced. + const out = await inject('proj-1', { + text: 'is Acme worth it?', + geoTargetId: 2840, + tagIds: [decoyCategoryId, TAG_IDS.intentInformational], + }); + + // The `Commercial` CATEGORY survives; the intent-root id is stripped and the + // computed `Commercial` intent id injected. + expect(out.tagIds).to.deep.equal([decoyCategoryId, TAG_IDS.intentCommercial]); + expect(transport.createProjectTags).to.not.have.been.called; + }); }); describe('handlers/prompts.js — deferPublish (serenity-docs#32 CSV-chunking)', () => { From 4442c57e99faff330bf6969a437b0228f86ef398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 16:12:24 +0200 Subject: [PATCH 12/16] fix: drop accidentally committed node_modules symlink The reconciliation merge (683f7b951) tracked a node_modules symlink pointing at an absolute local path, which breaks npm ci / checkouts on every other machine and CI runner. Untrack it and add a bare node_modules .gitignore entry (the existing node_modules/ only matches directories, not the symlink file) so it can't recur. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 1 + node_modules | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 120000 node_modules diff --git a/.gitignore b/.gitignore index 4b7a93b153..60bb458e63 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ coverage .nyc_output/ node_modules/ +node_modules junit dist tmp diff --git a/node_modules b/node_modules deleted file mode 120000 index aea6571ddb..0000000000 --- a/node_modules +++ /dev/null @@ -1 +0,0 @@ -/Users/ntoccane/Desktop/mysticat-workspace/spacecat-api-service/node_modules \ No newline at end of file From 02f4ec622569ea7d979c26a8c0115f2793b85084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 16:24:39 +0200 Subject: [PATCH 13/16] docs+feat(serenity): document deferPublish/published; log AI-gen classify cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address MysticatBot review on #2785: - OpenAPI: add the deferPublish request flag and the published response field to the create-prompts schemas (were undocumented — CLAUDE.md requires the spec track endpoint changes). - Observability: log one line when the AI-gen classify cap (AI_GEN_CLASSIFY_MAX) binds, so a silently-defaulted tail is visible rather than invisible. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/openapi/schemas.yaml | 19 ++++++++++++++++++- .../serenity/handlers/markets-subworkspace.js | 13 +++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/docs/openapi/schemas.yaml b/docs/openapi/schemas.yaml index a2f25b688c..d622736164 100644 --- a/docs/openapi/schemas.yaml +++ b/docs/openapi/schemas.yaml @@ -11123,10 +11123,20 @@ SerenityCreatePromptsRequest: minItems: 1 maxItems: 500 items: { $ref: '#/SerenityPromptInput' } + deferPublish: + type: boolean + default: false + description: > + When true, the created prompts are written to the upstream draft layer + but the project is NOT published as part of this request (the response + carries `published: false`). Used for chunked CSV imports, where the + client posts several chunks and issues a single publish after the last + one (serenity-docs#32). Omitted/false publishes once at the end of the + call as usual. SerenityCreatePromptsResponse: type: object - required: [created, skipped, failed] + required: [created, skipped, failed, published] properties: created: type: array @@ -11150,6 +11160,13 @@ SerenityCreatePromptsResponse: languageCode: { type: string } status: { type: integer } message: { type: string } + published: + type: boolean + description: > + Whether the project was published as part of this request. Always true + unless the request set `deferPublish: true`, in which case the prompts + were written as drafts and the caller must publish separately + (serenity-docs#32). SerenityUpdatePromptRequest: type: object diff --git a/src/support/serenity/handlers/markets-subworkspace.js b/src/support/serenity/handlers/markets-subworkspace.js index 7df7d06023..23c9bf05c6 100644 --- a/src/support/serenity/handlers/markets-subworkspace.js +++ b/src/support/serenity/handlers/markets-subworkspace.js @@ -283,6 +283,19 @@ async function generateAndAttachPrompts(transport, workspaceId, projectId, { // the `intent` vocabulary provisioned above, so its id is a Map lookup — no // extra upstream call. const allTexts = [...texts]; + // The cap is a designed budget bound, not an error: texts beyond it are not + // classified and take the seeded `Informational` default (see the partition + // below). Emit one line when it binds so a silently-defaulted tail is + // observable rather than invisible (serenity-docs#32 observability). + if (allTexts.length > AI_GEN_CLASSIFY_MAX) { + log?.info?.('generateAndAttachPrompts: AI-gen classify cap hit — tail defaults to Informational', { + workspaceId, + projectId, + total: allTexts.length, + classified: AI_GEN_CLASSIFY_MAX, + defaultedByCap: allTexts.length - AI_GEN_CLASSIFY_MAX, + }); + } const intentByText = await classifyPromptIntents( allTexts.slice(0, AI_GEN_CLASSIFY_MAX), { From a4c937f2d4f01ed1c59665b0ebbcd9dee9cf457c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 16:24:39 +0200 Subject: [PATCH 14/16] test(serenity): AI-gen cap boundary + deferPublish partial-batch coverage Address MysticatBot minor findings on #2785: - boundary test at AI_GEN_CLASSIFY_MAX+1: only the first MAX texts are classified, the overflow defaults to Informational, and the cap-hit logs. - deferPublish partial-batch: published:false alongside a mixed created/failed batch, with no publish triggered. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../handlers/markets-subworkspace.test.js | 46 +++++++++++++++++++ .../support/serenity/handlers/prompts.test.js | 37 +++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/test/support/serenity/handlers/markets-subworkspace.test.js b/test/support/serenity/handlers/markets-subworkspace.test.js index f36f5c7e68..e5c1f51501 100644 --- a/test/support/serenity/handlers/markets-subworkspace.test.js +++ b/test/support/serenity/handlers/markets-subworkspace.test.js @@ -1586,6 +1586,52 @@ describe('markets-subworkspace — defensive branch coverage', () => { expect(transport.createPromptsByIds).to.have.been.calledWithExactly(WS, 'new-proj', ['about it'], [SOURCE_AI_ID, TAG_IDS.intentInformational, TAG_IDS.typeNonBranded]); }); + // Boundary: at exactly AI_GEN_CLASSIFY_MAX + 1 texts, only the first MAX are + // sent to the classifier; the overflow text is never classified and takes the + // Informational default, and the cap-hit is logged (serenity-docs#32). + it('generateAndAttachPrompts: classifies only up to AI_GEN_CLASSIFY_MAX and logs the defaulted overflow', async () => { + const capLog = { info: sinon.spy(), error: sinon.spy(), warn: sinon.spy() }; + const classifySpy = sinon.stub().resolves(new Map([['p1', 'Transactional'], ['p2', 'Transactional']])); + const handler = await esmock( + '../../../../src/support/serenity/handlers/markets-subworkspace.js', + { + '../../../../src/support/serenity/intent-classification.js': { + classifyPromptIntents: classifySpy, + // Shrink the cap to 2 so a 3-text batch exercises the boundary. + AI_GEN_CLASSIFY_MAX: 2, + computeWriteDeadline: () => Date.now() + 12000, + }, + }, + ); + const transport = makeTransport({ + getBrandTopics: sinon.stub().resolves([ + { topic: 'T', volume: 10, prompts: ['p1', 'p2', 'p3'] }, + ]), + }); + const res = await handler.handleCreateMarketSubworkspace( + transport, + makeBrand(), + PARENT, + { ...createBody, brandNames: ['Trail'] }, + capLog, + null, + null, + { generateTopics: true, publishMode: 'skip' }, + ); + expect(res.status).to.equal(201); + // Only the first 2 texts were handed to the classifier — the slice bound. + expect(classifySpy).to.have.been.calledOnce; + expect(classifySpy.firstCall.args[0]).to.deep.equal(['p1', 'p2']); + // p1/p2 classified Transactional; p3 (beyond the cap) defaults Informational. + expect(transport.createPromptsByIds).to.have.been.calledWithExactly(WS, 'new-proj', ['p1', 'p2'], [...STANDARD_IDS.slice(0, 1), TAG_IDS.intentTransactional, TAG_IDS.typeNonBranded]); + expect(transport.createPromptsByIds).to.have.been.calledWithExactly(WS, 'new-proj', ['p3'], [...STANDARD_IDS.slice(0, 1), TAG_IDS.intentInformational, TAG_IDS.typeNonBranded]); + // The cap-hit is observable, not silent. + expect(capLog.info).to.have.been.calledWithMatch( + 'generateAndAttachPrompts: AI-gen classify cap hit — tail defaults to Informational', + sinon.match({ total: 3, classified: 2, defaultedByCap: 1 }), + ); + }); + // The two guards below both fire when the tag tree, freshly provisioned, still // does not contain a name we asked for. That is not hypothetical: upstream tag // writes land in the project's DRAFT layer while a default read serves the LIVE diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index 667ab043d9..46d57e2670 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -2371,6 +2371,43 @@ describe('handlers/prompts.js — deferPublish (serenity-docs#32 CSV-chunking)', }], }, fakeLog())).to.be.rejectedWith(ErrorWithStatusCode, /deferPublish must be a boolean/); }); + + it('reports published:false with a mixed created/failed batch (deferPublish, partial failure)', async () => { + const dataAccess = makeDataAccess([project()]); + // One prompt writes cleanly; the other 500s upstream. deferPublish is set, so + // neither the success nor the failure triggers a publish. + const transport = { + listProjectTags: makeListProjectTagsStub(), + createPromptsByIds: sinon.stub().callsFake(async (_ws, _pid, texts) => { + if (texts[0] === 'boom') { + throw Object.assign(new Error('upstream failure'), { status: 500 }); + } + return { page: 1, total: 1, items: [{ id: `new-${texts[0]}`, name: texts[0] }] }; + }), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { + deferPublish: true, + prompts: [ + { + text: 'ok', geoTargetId: 2840, languageCode: 'en', tagIds: ['tag-cat-1'], + }, + { + text: 'boom', geoTargetId: 2840, languageCode: 'en', tagIds: ['tag-cat-1'], + }, + ], + }, fakeLog()); + + expect(result.published).to.equal(false); + expect(result.created).to.have.lengthOf(1); + expect(result.created[0].text).to.equal('ok'); + expect(result.failed).to.have.lengthOf(1); + expect(result.failed[0].text).to.equal('boom'); + expect(result.failed[0].status).to.equal(500); + // deferPublish means no publish regardless of the partial failure. + expect(transport.publishProject).to.not.have.been.called; + }); }); // origin-dimension.md §3 (LLMO-6275): `origin` is derived from the write path, From ad26ff3ca79776f5b0500a9178a5200e3b610886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 16:35:03 +0200 Subject: [PATCH 15/16] test(serenity): add published to createSerenityPrompts contract fixture The OpenAPI contract test's sample response predates the published field and failed AJV validation once published became required on the response schema. The handler always returns published, so add it (true) to the fixture. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/openapi-contract/serenity-api.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/openapi-contract/serenity-api.test.js b/test/openapi-contract/serenity-api.test.js index 0304385e3d..ea6400dd9f 100644 --- a/test/openapi-contract/serenity-api.test.js +++ b/test/openapi-contract/serenity-api.test.js @@ -129,6 +129,7 @@ const FIXTURES = { }], skipped: [], failed: [], + published: true, }, data: { prompts: [{ From 60eb337ed25031aaa6fccff7049b135acb340028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Toccane?= Date: Wed, 22 Jul 2026 17:30:11 +0200 Subject: [PATCH 16/16] fix(serenity): classify trimmed text on create paths + address review Addresses review feedback on #2785: - Correctness: handleCreatePrompts / handleCreatePromptsSubworkspace classified the UN-trimmed raw text, but makeIntentInjector looks up by input.text (which normalizePromptInput trims). A whitespace-padded prompt (common in CSV import) missed the map and silently defaulted to Informational despite a real classification. Classify the trimmed text so the keys align; add a regression test that a padded input keeps its non-default intent. - Defensive: comment the "classify key == lookup key" contract at both call sites so the alignment is not silently broken by a future edit. - Docs: document PROMPT_INTENT_CLASSIFICATION_DEPLOYMENT_NAME (the classifier- scoped Azure deployment override) in the serenity operator guide's env table. - .gitignore: collapse the redundant node_modules entries to a single bare `node_modules` (matches the dir AND a stray symlink; the trailing-slash form would only match directories). Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 1 - docs/serenity.md | 3 +- .../serenity/handlers/prompts-subworkspace.js | 7 ++- src/support/serenity/handlers/prompts.js | 7 ++- .../support/serenity/handlers/prompts.test.js | 48 +++++++++++++++++++ 5 files changed, 62 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 60bb458e63..c68e3334a0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,5 @@ coverage .nyc_output/ -node_modules/ node_modules junit dist diff --git a/docs/serenity.md b/docs/serenity.md index b05cddc448..09f1806f6d 100644 --- a/docs/serenity.md +++ b/docs/serenity.md @@ -8,11 +8,12 @@ api-service exposes nine endpoints that front the Adobe-hosted Semrush AIO API a ## Environment configuration -The proxy is configured via a single env var that is **required at runtime** — Lambda fails at first request if it is unset. +`SEMRUSH_PROJECTS_BASE_URL` is **required at runtime** — Lambda fails at first request if it is unset. The server-side intent classifier (serenity-docs#32) adds one optional override. | Variable | Required | Source | Purpose | |---|---|---|---| | `SEMRUSH_PROJECTS_BASE_URL` | yes (no source default) | Vault `dx_mysticat//api-service`; locally `.env` | Upstream host for the Semrush AIO REST API. Must be `https://…`. Trailing slashes are stripped. Per-environment value so the production target can differ from the hackathon host without a code change. | +| `PROMPT_INTENT_CLASSIFICATION_DEPLOYMENT_NAME` | no (falls back to `AZURE_OPEN_AI_API_DEPLOYMENT_NAME`) | Vault `dx_mysticat//api-service`; locally `.env` | Classifier-scoped Azure OpenAI deployment (model) name for server-side prompt-intent classification (serenity-docs#32). Takes precedence over the shared `AZURE_OPEN_AI_API_DEPLOYMENT_NAME` other Azure consumers use (e.g. `org-detector`), so intent classification can target a different model without affecting them. Unset ⇒ shared deployment; behavior unchanged until explicitly configured. | ### Vault writes (dev / stage / prod) diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index f67ca48776..d2cd16b402 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -159,8 +159,13 @@ export async function handleCreatePromptsSubworkspace( // Unified layer (serenity-docs#32): batch-classify every distinct text ONCE // under the shared request deadline, then thread the resolved map into each // per-item injection below. + // Classify the TRIMMED text: `makeIntentInjector` looks up the map by + // `input.text`, which `normalizePromptInput` has already trimmed, so the + // classify key must be trimmed to match — otherwise a whitespace-padded prompt + // (common in CSV import) misses the map and silently defaults to Informational + // despite a real classification. const intentByText = await classifyPromptIntents( - inputs.map((raw) => String(raw?.text || '')), + inputs.map((raw) => String(raw?.text || '').trim()), { env, log, diff --git a/src/support/serenity/handlers/prompts.js b/src/support/serenity/handlers/prompts.js index e0adaf7ce3..e3ac67a7d4 100644 --- a/src/support/serenity/handlers/prompts.js +++ b/src/support/serenity/handlers/prompts.js @@ -687,8 +687,13 @@ export async function handleCreatePrompts( // Unified layer (serenity-docs#32): batch-classify every distinct text ONCE // under the shared request deadline, then thread the resolved map into each // per-item injection below (a per-item LLM call would be far too slow). + // Classify the TRIMMED text: `makeIntentInjector` looks up the map by + // `input.text`, which `normalizePromptInput` has already trimmed, so the + // classify key must be trimmed to match — otherwise a whitespace-padded prompt + // (common in CSV import) misses the map and silently defaults to Informational + // despite a real classification. const intentByText = await classifyPromptIntents( - inputs.map((raw) => String(raw?.text || '')), + inputs.map((raw) => String(raw?.text || '').trim()), { env, log, diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index 46d57e2670..1928cf518f 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -14,6 +14,7 @@ import { use, expect } from 'chai'; import chaiAsPromised from 'chai-as-promised'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; +import esmock from 'esmock'; import { handleListPrompts, @@ -2576,3 +2577,50 @@ describe('handlers/prompts.js — origin derivation (origin-dimension.md §3)', }); }); }); + +// Regression (Alicia review, serenity-docs#32): the create paths must classify the +// TRIMMED text. `makeIntentInjector` looks up the classification map by `input.text`, +// which `normalizePromptInput` has already trimmed — so if the classify input is NOT +// trimmed, a whitespace-padded prompt misses the map and silently defaults to +// Informational despite a real classification. The fake classifier below keys its +// result by exactly the text it receives (like the real `classifyIntents`), so an +// un-trimmed classify input would reintroduce the miss and fail this test. +describe('handlers/prompts.js — intent classify/lookup key alignment (serenity-docs#32)', () => { + const project = () => makeProject({ + semrushProjectId: 'proj-us-en', geoTargetId: 2840, languageCode: 'en', + }); + + it('classifies the trimmed text so a whitespace-padded prompt keeps its real (non-default) intent', async () => { + const { handleCreatePrompts: handleCreate } = await esmock( + '../../../../src/support/serenity/handlers/prompts.js', + { + '../../../../src/support/serenity/intent-classification.js': { + classifyPromptIntents: async (texts) => new Map( + texts.map((t) => [t, /buy/i.test(t) ? 'Transactional' : 'Informational']), + ), + }, + }, + ); + + const dataAccess = makeDataAccess([project()]); + const transport = { + listProjectTags: makeListProjectTagsStub(), + createPromptsByIds: sinon.stub().resolves({ + page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'buy now' }], + }), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleCreate(transport, dataAccess, BRAND, WORKSPACE, { + prompts: [{ + text: ' buy now ', geoTargetId: 2840, languageCode: 'en', tagIds: ['tag-cat-1'], + }], + }, fakeLog(), classifyByBrandMention); + + // The padded prompt resolves to Transactional (its real classification), NOT the + // Informational default — proving the classify key was trimmed to match the + // injector's `input.text` lookup. + expect(result.created[0].tagIds).to.include(TAG_IDS.intentTransactional); + expect(result.created[0].tagIds).to.not.include(TAG_IDS.intentInformational); + }); +});