feat(serenity): server-compute intent:<Value> on every prompt write path

[byteclimber]

Summary

• Implements serenity-docs#32 — removes the manual Intent picker requirement by computing intent:<Value> server-side via an LLM call on every Serenity/Semrush prompt write path (manual create, edit, AI-generation), mirroring the already-merged type:branded unified layer (feat(serenity): server-compute branded/non-branded prompt type on every write path #2772, serenity-docs#31).
• Parameterizes the existing createIntentClassifier (src/support/intent-classifier.js) with a categorySpec, defaulting to the native/DRS 6-bucket taxonomy reproduced byte-for-byte, and adds a Serenity-specific 5-value config (src/support/serenity/intent-taxonomy.js) with the validated system prompt + confidence floor.
• Adds the request-budget / fallback-ladder orchestration (src/support/serenity/intent-classification.js): a single shared write-deadline, one budget-gated retry, a hard skip-gate, and a terminal Informational default — a classification failure never blocks a write.
• Wires an intent:* injector (mirroring makeTypeInjector/resolveTypeTagInjection) into handleCreatePrompts(Subworkspace) and handleUpdatePrompt(Subworkspace) (classify-before-delete), and into AI-generation (generateAndAttachPrompts, capped at AI_GEN_CLASSIFY_MAX=16, replacing the old static intent:Informational default).
• Full design/decision record: serenity-docs PR #40.

Test plan

• npm run lint / npm run type-check clean
• Full unit suite: 14309 passing, 5 failing — identical to main's baseline (pre-existing/unrelated: missing @adobe/spacecat-shared-akamai-client package, unrelated Suggestions/GeoExperiment tests)
• Extended existing (serenity-docs#31)-style test coverage in prompts.test.js, prompts-subworkspace.test.js, markets-subworkspace.test.js, serenity.test.js, brand-provisioning.test.js to assert the injected intent:<Value> tag alongside the existing type:* assertions
• elmo-ui changes (picker removal, read-only display, CSV) are a separate follow-up PR, per the issue's own sequencing

[github-actions]

This PR will trigger a minor release when merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[MysticatBot]

Hey @byteclimber,

Verdict: Request changes - two issues to address in the budget model and API surface.
Complexity: MEDIUM - substantial feature across 18 files with API surface changes.
Changes: Adds server-side LLM-based intent classification on every Serenity prompt write path, with budget-gated retries and graceful fallback to Informational (18 files).

Must fix before merge

1. [Important] writeDeadline clock drift in brand-provisioning/onboarding flow - src/support/serenity/handlers/markets-subworkspace.js:347 (details inline)
2. [Important] deferPublish body flag lacks input validation and observability - src/support/serenity/handlers/prompts.js:590 (details inline)

Non-blocking (4): minor issues and suggestions

• nit: O(n^2) dedup via allTexts.includes(p) is redundant since classifyPromptIntents already deduplicates internally via new Set(...) - src/support/serenity/handlers/markets-subworkspace.js:229
• suggestion: Log when AI_GEN_CLASSIFY_MAX cap is binding (prompts beyond 16 silently default with no observability signal) - src/support/serenity/handlers/markets-subworkspace.js:245
• suggestion: Consider narrowing ctx.env to only the 6 keys the classifier needs before threading through handler layers, reducing the trust surface - src/controllers/serenity.js:506
• suggestion: The positional parameter lists (8-9 args) are approaching the threshold where a context-bag refactor ({ classifyPromptType, env, writeDeadline }) would prevent ordering bugs - track as follow-up

---

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

[MysticatBot]

issue (blocking): writeDeadline = computeWriteDeadline() as a default parameter computes the deadline at function-entry time, not at HTTP request entry. In the brand-provisioning/onboarding flow (handleOnboarding line 698, handleRefreshMarketOnboarding line 1225), this function is called after auth + loadBrand + provisioning steps (potentially 3-7s into the request). Combined with the 12s budget, the effective request time can exceed the ~15s Fastly first-byte timeout the budget was designed to protect.

Fix: Thread writeDeadline from the controller through provisionBrandSubworkspace into handleCreateMarketSubworkspace, mirroring the pattern already established for the flat/subworkspace create/update paths. The default-parameter fallback then only fires in tests.

[MysticatBot]

issue (blocking): deferPublish is a new request-body flag that changes behavior (skips the publish step entirely) with no input validation at the controller boundary. If a client accidentally sends deferPublish: true (stale state, copy-paste), prompts silently remain unpublished with no signal to the brand owner.

Fix: (1) Validate deferPublish as a boolean at the controller/Joi schema level so non-boolean values are rejected. (2) Add a structured log when deferPublish fires so the behavior is observable in production. The response already returns published: false which is good, but server-side observability helps detect misuse patterns.

[MysticatBot]

Hey @byteclimber,

Verdict: Approve - both prior blocking findings are properly resolved; no new issues.
Complexity: HIGH - large diff (19 files); API surface changes.
Changes: Adds server-side LLM-based intent classification on every Serenity prompt write path, with budget-gated retries and graceful fallback to Informational. The new commit threads writeDeadline from controller entry and validates the deferPublish body flag (18 files).

Note: CI build is still pending - verify before merge.

Non-blocking (2): minor issues and suggestions

• suggestion: Default-parameter writeDeadline = computeWriteDeadline() in provisionBrandSubworkspace, generateAndAttachPrompts, and handleCreateMarketSubworkspace silently re-computes a fresh deadline when omitted. All production callers currently pass the value explicitly, but a future caller that forgets the parameter gets a deadline computed partway through the request rather than at HTTP entry. Consider making it required (callers supply Infinity or an explicit value in tests) or adding a log.warn when the default fires in non-test environments - src/support/serenity/brand-provisioning.js:108
• suggestion: Server-computed tags (type:* + intent:*) append after sanitizeTagIds caps user-supplied tagIds at MAX_TAG_IDS (50), so the final array can reach 52. This is by-design and tested, but a one-line comment near sanitizeTagIds documenting the effective max after server injection would prevent confusion if the upstream Semrush API ever enforces a hard cap - src/support/serenity/handlers/prompts.js:50

Previously flagged, now resolved

• writeDeadline clock drift: now computed once at controller entry and threaded via parameter to all write paths
• deferPublish validation: validateDeferPublish() rejects non-boolean with 400, structured logging emits when the flag fires

Note: QA review did not complete and is not included.

---

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

[MysticatBot]

Hey @byteclimber,

Verdict: Approve - observability addition is well-isolated and does not affect classification behavior.
Complexity: HIGH - large diff (19 files); API surface changes.
Changes: Adds CloudWatch EMF metrics (outcome counters, latency percentiles, value distribution) and structured soft-failure logging to the intent-classification write path, with a privacy regression test confirming prompt text never leaks into logs (19 files).

Note: CI build is still pending - verify before merge.

Non-blocking (3): minor issues and suggestions

• suggestion: The Workspace dimension on IntentOutcome creates 24N unique CloudWatch metric streams (4 write paths x 6 outcomes x N workspaces). At scale this becomes expensive and makes dashboarding unwieldy. The workspace ID is already in the structured log line (safeLog.info('summary', counts)) for ad-hoc drill-down via Logs Insights. Consider dropping Workspace from EMF dimensions and keeping only WritePath + Outcome (bounded cardinality) - src/support/serenity/intent-classification.js:202
• suggestion: emitObservability() is called at the end of classifyPromptIntents after the result Map is fully populated. If it throws (e.g. an unexpected error in the forEach iteration), the already-computed result would be lost. A top-level try { emitObservability(); } catch { /* best-effort */ } at the call site would make the never-throw guarantee structural rather than relying on each internal emit being individually wrapped - src/support/serenity/intent-classification.js:330
• nit: PerCallTimeout counter fires when r === null && dt >= PER_CALL_MS, which conflates actual LLM timeouts with slow non-timeout errors (e.g. a network failure that happens to take longer than PER_CALL_MS). The metric name implies timeout specifically. Consider renaming to PerCallSlowFailure or checking for a timeout-specific error type - src/support/serenity/intent-classification.js:286

Previously flagged, now resolved

• writeDeadline clock drift: computed once at controller entry and threaded via parameter (addressed in prior commit)
• deferPublish validation: validateDeferPublish() rejects non-boolean with 400, structured logging emits when the flag fires (addressed in prior commit)

---

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

[byteclimber]

Related: #2825 (origin-dimension rename) touches the same files (prompt-tags.js, tag-tree.js) — worth reviewing/merging in a coordinated order to avoid conflicts.

[byteclimber]

Merged main into this branch (683f7b9) to clear the conflicts, and folded in the edit-path + LLMO-6190 headroom fixes (was #2875, now merged). Full unit suite green (15507 passing), type-check + lint clean.

Reviewer, please scrutinize these reconciliation decisions on the shared prompt write path (main's origin-injection/PE-facade work landed in parallel with this branch's intent classification):

1. Three-dimension injection composition. Prompts now carry type + origin + intent. The write path chains main's makePromptTagInjector (type+origin) with this branch's makeIntentInjector (intent); they act on disjoint tag dimensions so they compose. Injection order is type → origin → intent (asserted across the updated unit + IT expectations). This is the riskiest part of the merge — worth a careful look at handlers/prompts.js + prompts-subworkspace.js.
2. Edit path recomputes type + intent but leaves origin untouched (create/update asymmetry, origin-dimension.md §3). No originValue passed to the injector on update.
3. Create-write metering. Kept main's headroom.retryOnQuota wrapper on the create write; this branch's pre-publish ensure was removed in favor of metering-at-write (spec §105-110). Net: a metered-405 on create recovers via retryOnQuota; publish is not re-gated.

No behavior of main's origin/PE-facade work was changed — only extended to coexist with intent. If any of the above diverges from the origin-dimension.md / LLMO-6190 intent, flag it here.

[MysticatBot]

Verdict: REQUEST_CHANGES | Complexity: HIGH | Re-review (7 new commits since last APPROVE at f3f9bd7)

---

Critical

1. Committed node_modules symlink breaks all checkouts

A tracked symlink node_modules -> /Users/ntoccane/Desktop/mysticat-workspace/spacecat-api-service/node_modules was added. The .gitignore has node_modules/ (trailing slash = directory only), so git tracks the symlink (a file). This will:

• Break npm ci / npm install on every other machine and CI runner (the target path doesn't exist)
• Prevent git from creating the node_modules/ directory while a tracked symlink with that name exists
• Expose the author's filesystem layout

Fix: Remove the tracked symlink (git rm node_modules) and add a bare node_modules line (no trailing slash) to .gitignore to prevent recurrence.

---

Important

2. makeIntentInjector caches rejected promises permanently

The intent injector caches the resolveIntentValueInjection Promise keyed by (projectId, intentValue). If the LLM call fails with a transient error (e.g., 502), the rejected promise is cached for the lifetime of the request. Every subsequent prompt with the same key awaits the same rejected promise and fails — a single transient failure poisons the entire batch for that intent value.

Suggestion: Evict the cache entry on rejection, or wrap with a retry-aware cache that only stores resolved values.

3. Budget model can overrun the 6s classification reserve

The cursor = unique.length abort stops new iterations but does not cancel in-flight LLM workers (no AbortController). Under Azure slowness (e.g., 2.5s/call), workers already dispatched continue to completion past budget expiry. The publish step may then start 9s+ into the Lambda, risking Fastly/CDN gateway timeout on the full response.

Suggestion: Wire an AbortController signal into the fetch calls and abort in-flight requests when the budget expires, rather than relying solely on the cursor trick.

4. No circuit breaker for repeated Azure OpenAI failures

When Azure OpenAI is down, every write request still attempts the full classification budget (~6s) before defaulting. With high write volume this adds significant latency across all requests during the outage window.

Suggestion: Track consecutive request-level classification failures; skip classification for 30-60s after N consecutive failures (lightweight circuit breaker). The ProdLlmUnavailable metric already detects the condition.

5. OpenAPI spec not updated for new deferPublish query parameter

The PR adds deferPublish (boolean query param) to the create-prompts endpoints but does not update the OpenAPI spec. Per the repo's CLAUDE.md: "Update OpenAPI spec when adding or modifying API endpoints."

6. computeWriteDeadline() evaluated per call site, not per request

Multiple function signatures use writeDeadline = computeWriteDeadline() as a default parameter. This means any caller that omits the argument gets a fresh deadline computed at their call time rather than the request-entry deadline — defeating the single-clock design. Currently safe (the controller always passes explicitly), but a latent trap for future callers.

---

Minor

8 minor findings (click to expand)

• .gitignore needs bare node_modules entry — current node_modules/ only matches directories, not symlink files
• Magic numbers for budget constants — AI_GEN_CLASSIFY_MAX = 16, PER_CALL_MS = 3000, CLASSIFY_CONCURRENCY = 8 lack explanatory names or comments for the derivation
• allTexts.slice(0, AI_GEN_CLASSIFY_MAX) silently drops tail prompts — texts beyond index 16 are never classified and silently default to Informational with no log/metric noting the cap was hit
• published response field not in OpenAPI spec — new response shape undocumented
• No boundary test at exactly AI_GEN_CLASSIFY_MAX (16/17 items) — the cap behavior is untested at the boundary
• deferPublish partial-batch scenario untested — no test verifies published: false alongside mixed created/failed arrays
• Twin-file duplication continues to grow — prompts.js / prompts-subworkspace.js now wire intent injection identically; no convergence tracker visible
• Flat-mode handleCreatePrompts missing retryOnQuota on publish — subworkspace twin passes it but flat-mode does not (pre-existing gap, worth tracking)

---

Notes

• CI status: build pending at time of review; type-check and validate-pr-title passing.
• Prior findings (last review): writeDeadline clock drift and deferPublish validation issues from earlier reviews appear addressed in the new commits.
• Design assessment: The core architecture (batch-classify once per request, budget-gated retry ladder, terminal default to Informational, three-dimension tag composition on disjoint dimensions) is sound. The fail-open design ensures classification failures never block writes. CloudWatch EMF observability is well-structured.
• Spec divergence detected: OpenAPI spec requires update before merge (see Important #5).

---

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

[MysticatBot]

Mysticat review failed: Claude CLI crashed (exit 1): stderr= stdout={"type":"result","subtype":"error_max_budget_usd","duration_ms":3,"duration_api_ms":1260206,"is_error":true,"num_turns":1,"stop_reason":null,"session_id":"e3fef99f-3f60-4b2a-956e-d802192838a5","total_cost_usd":10.153901549999999,"usage":{"input_tokens":0,"cache_creation_input_tokens":0,"cache_read_i

[MysticatBot]

Hey @byteclimber,

Verdict: Approve - all prior blocking findings are resolved; no new issues.
Complexity: HIGH - large diff (23 files); API surface changes.
Changes: Adds server-side LLM-based intent classification on every Serenity prompt write path, with budget-gated retries and graceful fallback to Informational. The new commits fix the node_modules symlink, document deferPublish/published in the OpenAPI spec, add cap-hit logging, and cover boundary + partial-batch scenarios in tests (23 files).
Note: Recommend a human read before merge - shared contract change (OpenAPI spec modified: docs/openapi/schemas.yaml). The bot review is a complement to, not a replacement for, a human read here.
Note: CI build/integration checks are still pending - verify before merge.

Non-blocking (3): minor issues and suggestions

• suggestion: Workspace dimension on the IntentOutcome EMF metric creates unbounded CloudWatch metric streams (one per workspace). Consider dropping Workspace from EMF dimensions and keeping only WritePath + Outcome (bounded cardinality); the workspace ID is already in the structured log line for ad-hoc drill-down via Logs Insights - src/support/serenity/intent-classification.js:202
• nit: .gitignore now has both node_modules/ (directories only) and bare node_modules (any path component); the bare entry subsumes the directory match, making the trailing-slash line redundant - .gitignore:3
• suggestion: The AI-gen cap boundary test exercises cap+1 (3 texts with cap=2). Consider also asserting at exactly cap (2 texts, no log fired) to confirm the > check is not an off-by-one >= - test/support/serenity/handlers/markets-subworkspace.test.js:1586

Previously flagged, now resolved

• node_modules symlink removed (4442c57)
• OpenAPI spec updated with deferPublish request field and published response field (02f4ec6)
• AI-gen classify cap now logs when it binds, making the defaulted tail observable (02f4ec6)
• Boundary test at AI_GEN_CLASSIFY_MAX added (a4c937f)
• deferPublish partial-batch scenario now tested (a4c937f)
• Prior Important findings (rejected-promise cache, budget overrun, circuit breaker) re-evaluated: all mirror established codebase patterns, are bounded by the request lifecycle and fail-open design, and are observable via existing metrics - accepted as reasonable v1 tradeoffs

---

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

[aliciadriani]

Correctness: trim mismatch silently defaults classified intent to Informational on the create paths

In both handleCreatePrompts and handleCreatePromptsSubworkspace, the classification map is built from un-trimmed raw text:

const intentByText = await classifyPromptIntents(
  inputs.map((raw) => String(raw?.text || '')),   // NOT trimmed
  { env, log, deadline: writeDeadline, writePath: deferPublish ? 'csv' : 'create', workspaceId },
);

classifyPromptIntents returns a Map keyed by exactly those strings (classifyIntents does results.set(text, intent), and the orchestrator's resolve/defaultAll re-key by the same raw t). But makeIntentInjector looks it up by the normalized text:

const intentValue = intentByText.get(input.text) ?? INTENT_VALUE.INFORMATIONAL;

…and normalizePromptInput does String(input?.text || '').trim(). So for any prompt whose text has leading/trailing whitespace, intentByText.get(trimmedText) misses, and the prompt silently falls back to Informational — even though the LLM was actually called and produced a real classification (wasted call + discarded result).

Impact: the flagship use case here is chunked CSV import (writePath: 'csv'), where cell whitespace is common. Affected rows get the wrong intent tag with no error surfaced.

Why the tests don't catch it: every create/CSV fixture uses clean text ('hello', 'p', 'is Acme good?'), so the keys happen to match.

Fix: classify the normalized text so the keys line up, e.g. inputs.map((raw) => String(raw?.text || '').trim()), and add a regression test with a whitespace-padded input asserting the classified (non-default) intent id.

For reference: the edit path (handleUpdatePrompt) and AI-gen path (generateAndAttachPrompts) are not affected — both classify and look up the identical string (nextText / members of allTexts).

[aliciadriani]

Should fix: new env var PROMPT_INTENT_CLASSIFICATION_DEPLOYMENT_NAME is undocumented

resolveDeploymentName in src/support/intent-classifier.js introduces a classifier-scoped Azure deployment override that takes precedence over the shared AZURE_OPEN_AI_API_DEPLOYMENT_NAME:

function resolveDeploymentName(env = {}) {
  return env.PROMPT_INTENT_CLASSIFICATION_DEPLOYMENT_NAME
    || env.AZURE_OPEN_AI_API_DEPLOYMENT_NAME;
}

It falls back safely when unset (behavior unchanged until explicitly configured), but the new variable isn't listed anywhere operators would look (env docs / deploy config). Please document it so the override is discoverable.

[aliciadriani]

Should fix (defensive): make the "classify key == lookup key" contract explicit

This is a follow-up to the trim-mismatch bug. The bug class is easy to reintroduce because the two injectors normalize differently:

• makePromptTagInjector (type) classifies inline on input.text (already normalized), so it's self-consistent.
• makeIntentInjector (intent) relies on a map pre-computed ahead of the loop, then looked up by input.text — so the pre-compute must use the exact same normalized string the injector will look up by.

Once the classify input is fixed to use normalized text, a short comment at the classifyPromptIntents call site (e.g. "pass the same normalized text normalizePromptInput produces, since makeIntentInjector looks up by input.text") would keep a future edit from silently breaking the key alignment again.

[aliciadriani]

Nit: redundant .gitignore entry

The diff adds node_modules while node_modules/ already exists one line above — the new line is redundant and can be dropped.

[byteclimber]

Thanks @aliciadriani — all four addressed in 60eb337ed:

1. Trim mismatch (correctness). Fixed — handleCreatePrompts and handleCreatePromptsSubworkspace now classify String(raw?.text || '').trim(), so the classify key matches makeIntentInjector's input.text lookup. Added a regression test (intent classify/lookup key alignment) with a whitespace-padded input asserting it keeps its real (non-default) intent; I verified it fails if the trim is reverted, so it genuinely guards the bug. Confirmed the edit/AI-gen paths were already consistent, as you noted.

2. Undocumented env var. Documented PROMPT_INTENT_CLASSIFICATION_DEPLOYMENT_NAME in the serenity operator guide's env table (docs/serenity.md → Environment configuration), including the fallback to AZURE_OPEN_AI_API_DEPLOYMENT_NAME and that it's unset-safe.

3. Make the contract explicit. Added a comment at both classifyPromptIntents call sites documenting that the classify input must use the same normalized text makeIntentInjector looks up by.

4. .gitignore nit — one intentional deviation, flagging for your call. Rather than dropping the bare node_modules (leaving node_modules/), I did the reverse: dropped node_modules/ and kept the bare node_modules. Reason: node_modules/ (trailing slash) matches directories only, so it would not ignore a node_modules symlink — which is exactly the stray artifact that triggered this cleanup. The bare form matches both, so this is the single non-redundant entry that stays symlink-safe. Happy to switch to whichever you prefer.

Full unit suite green (15701) + type-check + lint.