diff --git a/docs/openapi/prompts-v2-api.yaml b/docs/openapi/prompts-v2-api.yaml index 3b6de631f..b4b547c49 100644 --- a/docs/openapi/prompts-v2-api.yaml +++ b/docs/openapi/prompts-v2-api.yaml @@ -254,6 +254,10 @@ v2-prompts-by-brand-and-id: tags: - customer-config summary: Update a single prompt + description: >- + `origin` in the body is not honored on PATCH — it is set once at create + time (see `V2PromptInput.origin`) and can never be changed afterward, so + any `origin` sent here is ignored. operationId: updatePromptByBrandAndId security: - ims_key: [ ] diff --git a/docs/openapi/schemas.yaml b/docs/openapi/schemas.yaml index 781597e45..ddf79f3a2 100644 --- a/docs/openapi/schemas.yaml +++ b/docs/openapi/schemas.yaml @@ -6643,6 +6643,11 @@ V2PromptInput: type: string enum: [ai, human] default: human + description: >- + Who authored the prompt. Service-principal only: honored (and + validated against the enum) only when the caller authenticates as an + S2S service; for a user-authenticated request this field is ignored + and always overridden to `human`, regardless of what is sent. source: type: string description: The source system or process that created the prompt diff --git a/src/controllers/brands.js b/src/controllers/brands.js index bc349baba..3e5bf27f9 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -554,6 +554,11 @@ function BrandsController(ctx, log, env) { const { postgrestClient } = context.dataAccess.services; const updatedBy = context.attributes?.authInfo?.profile?.email || 'system'; + // Service principals (e.g. llmo-data-retrieval-service) authenticate over + // s2sAuthWrapper, which sets `context.s2sConsumer`; a user principal never + // carries it. Gates whether a per-prompt `origin` in the body is honored — + // see `resolveOriginForWrite`. + const isServicePrincipal = Boolean(context.s2sConsumer); const brandUuid = await resolveBrandUuid(spaceCatId, brandId, postgrestClient); if (!brandUuid) { @@ -567,6 +572,7 @@ function BrandsController(ctx, log, env) { postgrestClient, updatedBy, classifyIntent: classifyIntent ?? undefined, + isServicePrincipal, }); return createResponse({ created, updated, prompts: outPrompts }, 201); diff --git a/src/support/prompts-storage.js b/src/support/prompts-storage.js index 1f8ae67c4..1e25e3bfa 100644 --- a/src/support/prompts-storage.js +++ b/src/support/prompts-storage.js @@ -18,6 +18,33 @@ import { classifyIntents } from './intent-classifier.js'; import { throwOnPgConstraintViolation } from './errors.js'; import { assertPermittedSource } from './prompt-sources.js'; import { INTENT_VALUES, normalizeIntent } from './intent.js'; +import { ORIGIN_VALUE } from './serenity/prompt-tags.js'; + +const ORIGIN_VALUES = new Set(Object.values(ORIGIN_VALUE)); + +/** + * Resolves the `origin` a prompt write should carry, from the caller's + * authenticated principal rather than trusting the request body outright. + * + * A SERVICE principal (e.g. llmo-data-retrieval-service, which POSTs with a + * hardcoded `origin: "ai"`) may assert either enum value; an unrecognized + * value is treated as absent rather than rejected — a write must never fail + * over this field. A USER principal's asserted value is always ignored: a + * human hitting this endpoint is definitionally authoring the prompt + * themselves, so the body's `origin` (if any) is silently overridden, never + * used to reject the request. + * + * @param {*} candidate - the request body's asserted `origin`, if any. + * @param {boolean} isServicePrincipal - true when the caller authenticated as + * an S2S service (see `context.s2sConsumer` at the controller boundary). + * @returns {'ai' | 'human'} + */ +export function resolveOriginForWrite(candidate, isServicePrincipal) { + if (isServicePrincipal && ORIGIN_VALUES.has(candidate)) { + return candidate; + } + return ORIGIN_VALUE.HUMAN; +} // Re-exported for backward compatibility — `normalizeIntent`/`INTENT_VALUES` now // live in `./intent.js` so the LLM intent classifier can reuse them without an @@ -406,7 +433,9 @@ function mapRowToPrompt(row) { name: row.name, regions: row.regions || [], status: row.status || 'active', - origin: row.origin || 'human', + // No fallback: the column has zero NULLs in production, so `|| 'human'` + // was dead code that would otherwise silently mislabel a null as human. + origin: row.origin, source: row.source || 'config', intent: row.intent ?? null, createdAt: row.created_at, @@ -723,6 +752,9 @@ function buildPromptKey({ text, regions, source }) { * text without an explicit intent. Non-fatal: a null result leaves intent unset. * @param {number} [params.classifyIntentBatchTimeoutMs] - Cap on the classifier * batch (ms); the upsert proceeds without intent once it elapses. + * @param {boolean} [params.isServicePrincipal] - true when the caller + * authenticated as an S2S service; gates whether a per-prompt `origin` in + * the request body is honored (see {@link resolveOriginForWrite}). * @returns {Promise<{created: number, updated: number, prompts: object[]}>} */ export async function upsertPrompts({ @@ -733,6 +765,7 @@ export async function upsertPrompts({ updatedBy = 'system', classifyIntent, classifyIntentBatchTimeoutMs = 8000, + isServicePrincipal = false, }) { if (!postgrestClient?.from) { throw new Error('PostgREST client is required for prompts'); @@ -825,7 +858,7 @@ export async function upsertPrompts({ category_id: categoryUuid, topic_id: topicUuid, status: p.status || 'active', - origin: p.origin || 'human', + origin: resolveOriginForWrite(p.origin, isServicePrincipal), source, intent: normalizeIntent(p.intent), updated_by: updatedBy, @@ -1050,9 +1083,9 @@ export async function updatePromptById({ if (updates.status !== undefined) { patch.status = updates.status; } - if (updates.origin !== undefined) { - patch.origin = updates.origin; - } + // `origin` is never patchable via the PATCH body — mirrors `source`, which is + // already absent here. It is set once at create time (from the authenticated + // principal — see `upsertPrompts`) and never re-derived or overridden later. if (updates.intent !== undefined) { // The shared fallback strips intent when the column is known-absent. patch.intent = normalizeIntent(updates.intent); diff --git a/src/support/serenity/handlers/prompts-subworkspace.js b/src/support/serenity/handlers/prompts-subworkspace.js index 23d14e1e6..a08a46f9d 100644 --- a/src/support/serenity/handlers/prompts-subworkspace.js +++ b/src/support/serenity/handlers/prompts-subworkspace.js @@ -294,7 +294,7 @@ export async function handleUpdatePromptSubworkspace( const injectComputedType = makeTypeInjector(transport, workspaceId, classifyPromptType, log); const typed = await injectComputedType(projectId, { text: nextText, geoTargetId, tagIds: nextTagIds, - }); + }, { mode: 'update' }); try { await transport.renamePrompt(workspaceId, projectId, semrushPromptId, nextText); diff --git a/src/support/serenity/handlers/prompts.js b/src/support/serenity/handlers/prompts.js index 75e71f540..55b6f2399 100644 --- a/src/support/serenity/handlers/prompts.js +++ b/src/support/serenity/handlers/prompts.js @@ -19,7 +19,8 @@ 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 { resolveTypeValueInjection } from '../tag-tree.js'; +import { resolveTypeValueInjection, resolveOriginValueInjection } from '../tag-tree.js'; +import { ORIGIN_VALUE } from '../prompt-tags.js'; // TWIN FILE: the slice→project orchestration here is paralleled by the // subworkspace-mode handlers in prompts-subworkspace.js. The duplication is @@ -366,60 +367,115 @@ export async function createOnePrompt(transport, semrushWorkspaceId, projectId, } /** - * Builds the per-request `type` injector — the UNIFIED classification layer - * (serenity-docs#31). Given a pure `classifyPromptType(text, geoTargetId)` + * Builds the per-request `type` + `origin` injector — the UNIFIED + * classification layer (serenity-docs#31, extended by the-origin-dimension + * spec for `origin`). Given a pure `classifyPromptType(text, geoTargetId)` * closure (built by the controller from the brand name + region-clamped * aliases) that yields a BARE `type` value (`branded` / `non-branded`), it - * returns `injectComputedType(projectId, input)` which: - * - STRIPS every caller-supplied tag id that lives under the `type` root (the - * client may never set the value), and - * - APPENDS the pre-resolved upstream id of the server-computed value. The - * atomic `createPromptsByIds` 500s on an unresolved id, so it is resolved + * returns `injectComputedType(projectId, input, options)` which, for BOTH + * dimensions: + * - STRIPS every caller-supplied tag id that lives under the dimension's root + * (the client may never set either value directly), and + * - APPENDS a resolved upstream tag id in its place. The atomic + * `createPromptsByIds` 500s on an unresolved id, so ids are resolved * BEFORE the write. * The returned input carries the rewritten `tagIds`, so the caller's response - * echo reflects the computed type without a refetch (decision 5). + * echo reflects the computed values without a refetch (decision 5). * - * The strip set is every id under the `type` root, not a name prefix: a tag's - * dimension is its root, and a sub-category could legitimately be named - * `branded` without being a `type` value. + * The strip set for each dimension is every id under that dimension's root, + * not a name prefix: a tag's dimension is its root, and a sub-category could + * legitimately be named `branded` or `ai` without being a `type`/`origin` value. * - * Resolution ({@link resolveTypeValueInjection}, two tag-tree reads per distinct - * `type` value per project — the root level plus the `type` root's children) is - * memoized for the request, so a bulk create fans out over the distinct computed - * values rather than over the items. A non-function `classifyPromptType` - * (defensive) is a pass-through. + * `type` is always freshly classified from the write's own text. `origin` is + * asymmetric between create and update (`options.mode`, default `'create'`): + * - `'create'`: derives to {@link ORIGIN_VALUE.AI} — every prompt this layer + * creates is AI-generated. + * - `'update'`: re-injects whichever `origin` tag id is ALREADY in + * `input.tagIds` (the client round-trips the tags it read off the prior + * list call), rather than deriving a new one from anything in the update + * request. This is deliberate: `origin` records who authored the prompt + * ORIGINALLY, which an edit does not change. If none is found (a prompt + * tagged before this dimension existed), it falls back to `AI` rather than + * leave the prompt untagged — a strip-without-inject would make it invisible + * to origin-based filtering. * - * `resolveTypeValueInjection` resolves or throws, so the computed tag is always - * attached. It must never be dropped: `type` is the one dimension a client may - * not set, so a prompt written without it stays unclassified forever, and the - * caller sees a 2xx. Failing the write instead is free — the upstream bulk create - * is atomic and has not run yet. + * Resolution (two tag-tree reads per distinct value per project — the root + * level plus that dimension root's children) is memoized for the request per + * dimension, so a bulk create fans out over the distinct computed values + * rather than over the items. A non-function `classifyPromptType` (defensive) + * skips the `type` step; `origin` is always injected. + * + * Both resolvers resolve or throw, so the computed tags are always attached. + * They must never be dropped: neither dimension is client-settable, so a + * prompt written without one stays unclassified forever, and the caller sees a + * 2xx. Failing the write instead is free — the upstream bulk create is atomic + * and has not run yet. * * @param {object} transport - Serenity transport (Semrush proxy client). * @param {string} semrushWorkspaceId * @param {((text: string, geoTargetId: number) => string) | undefined} classifyPromptType * @param {object} [log] * @returns {(projectId: string, input: { text: string, geoTargetId: number, - * tagIds: string[] }) => + * tagIds: string[] }, options?: { mode?: 'create' | 'update' }) => * Promise<{ text: string, geoTargetId: number, tagIds: string[] }>} */ export function makeTypeInjector(transport, semrushWorkspaceId, classifyPromptType, log) { /** @type {Map>} */ - const cache = new Map(); - return async function injectComputedType(projectId, input) { - if (typeof classifyPromptType !== 'function') { - return input; + const typeCache = new Map(); + /** @type {Map>} */ + const originCache = new Map(); + + return async function injectComputedType(projectId, input, { mode = 'create' } = {}) { + let out = input; + + if (typeof classifyPromptType === 'function') { + const typeValue = classifyPromptType(input.text, input.geoTargetId); + const typeKey = `${projectId} ${typeValue}`; + let typePending = typeCache.get(typeKey); + if (!typePending) { + typePending = resolveTypeValueInjection( + transport, + semrushWorkspaceId, + projectId, + typeValue, + log, + ); + typeCache.set(typeKey, typePending); + } + const { computedId, typeTagIds } = await typePending; + const stripped = out.tagIds.filter((id) => !typeTagIds.includes(id)); + out = { ...out, tagIds: [...stripped, computedId] }; } - const typeValue = classifyPromptType(input.text, input.geoTargetId); - const key = `${projectId} ${typeValue}`; - let pending = cache.get(key); - if (!pending) { - pending = resolveTypeValueInjection(transport, semrushWorkspaceId, projectId, typeValue, log); - cache.set(key, pending); + + // `origin` resolution is the same tree lookup on both create and update — + // every id under the root, plus the resolved `AI` id — only what CREATE + // does with it differs from what UPDATE does. + const originKey = `${projectId} ${ORIGIN_VALUE.AI}`; + let originPending = originCache.get(originKey); + if (!originPending) { + originPending = resolveOriginValueInjection( + transport, + semrushWorkspaceId, + projectId, + ORIGIN_VALUE.AI, + log, + ); + originCache.set(originKey, originPending); } - const { computedId, typeTagIds } = await pending; - const stripped = input.tagIds.filter((id) => !typeTagIds.includes(id)); - return { ...input, tagIds: [...stripped, computedId] }; + const { computedId: aiId, originTagIds } = await originPending; + const stripped = out.tagIds.filter((id) => !originTagIds.includes(id)); + + // UPDATE re-injects whichever origin tag the caller already carried — it + // never derives a new one. CREATE always derives to AI. Falling back to AI + // when update finds none avoids a strip-without-inject (which would make + // the prompt invisible to origin-based filtering) for a prompt tagged + // before this dimension existed. + const nextOriginId = mode === 'update' + ? (out.tagIds.find((id) => originTagIds.includes(id)) ?? aiId) + : aiId; + out = { ...out, tagIds: [...stripped, nextOriginId] }; + + return out; }; } @@ -722,7 +778,7 @@ export async function handleUpdatePrompt( ); const typed = await injectComputedType(projectId, { text: nextText, geoTargetId, tagIds: nextTagIds, - }); + }, { mode: 'update' }); try { await transport.renamePrompt(semrushWorkspaceId, projectId, semrushPromptId, nextText); diff --git a/src/support/serenity/tag-tree.js b/src/support/serenity/tag-tree.js index 9712cb8a4..49ddef1a3 100644 --- a/src/support/serenity/tag-tree.js +++ b/src/support/serenity/tag-tree.js @@ -608,3 +608,46 @@ export async function resolveTypeValueInjection( typeTagIds: [...byName.values()], }; } + +/** + * Resolves the id-based injection of a server-computed `origin` value into a + * prompt write. Returns the wanted value's id plus EVERY id under the resolved + * authorship root, so the caller can strip any caller-supplied `origin` tag id + * (a client must never set the value itself on create; on update the caller + * re-injects the prompt's already-stored id instead of calling this with a + * freshly derived one — see `makeTypeInjector` in `handlers/prompts.js`). + * + * The root is resolved through {@link ensureDimensionRoots}, so during the + * `source` → `origin` rename this returns the tolerantly-resolved authorship + * root (whichever physical name it currently carries) — never a second one. + * + * @param {object} transport - Serenity transport (Semrush proxy client). + * @param {string} semrushWorkspaceId + * @param {string} projectId + * @param {string} wantValue - the bare `origin` value (`ai` / `human`). + * @param {object} [log] - logger. + * @returns {Promise<{ computedId: string, originTagIds: string[] }>} `computedId` + * is always resolved — {@link ensureChildren} throws rather than leave a hole. + */ +export async function resolveOriginValueInjection( + transport, + semrushWorkspaceId, + projectId, + wantValue, + log, +) { + const roots = await ensureDimensionRoots(transport, semrushWorkspaceId, projectId, log); + const originRootId = rootIdOf(roots, DIMENSION.ORIGIN); + const { byName } = await ensureChildren( + transport, + semrushWorkspaceId, + projectId, + originRootId, + [wantValue], + log, + ); + return { + computedId: /** @type {string} */ (byName.get(wantValue)), + originTagIds: [...byName.values()], + }; +} diff --git a/test/it/shared/tests/serenity.js b/test/it/shared/tests/serenity.js index 3c7712ab5..9d2e5b387 100644 --- a/test/it/shared/tests/serenity.js +++ b/test/it/shared/tests/serenity.js @@ -533,11 +533,12 @@ export default function serenityTests( 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 server-computes two tags and appends them to the supplied + // tagIds: a branded/non-branded `type` tag and an `origin` tag (`ai` on + // create — WP-O2b). So the created prompt carries the two supplied tags + // plus the computed type and origin tags. 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/prompts-storage.test.js b/test/support/prompts-storage.test.js index 8a051c7a4..376ce36ec 100644 --- a/test/support/prompts-storage.test.js +++ b/test/support/prompts-storage.test.js @@ -30,6 +30,7 @@ import { isMissingIntentColumnError, findPromptsBlockingRegionRemoval, getIntentsByPromptIds, + resolveOriginForWrite, } from '../../src/support/prompts-storage.js'; use(chaiAsPromised); @@ -899,7 +900,10 @@ describe('prompts-storage', () => { expect(result).to.not.be.null; expect(result.regions).to.deep.equal([]); expect(result.status).to.equal('active'); - expect(result.origin).to.equal('human'); + // No fallback: the column has zero NULLs in production, so a null/ + // undefined `origin` is returned as-is rather than silently coerced to + // 'human' (which would have mislabeled it). + expect(result.origin).to.equal(undefined); expect(result.source).to.equal('config'); expect(result.category).to.be.null; expect(result.topic).to.be.null; @@ -3190,6 +3194,79 @@ describe('prompts-storage', () => { }); }); + describe('resolveOriginForWrite (WP-O2b: origin derived from the authenticated principal)', () => { + it('honors a valid asserted value from a service principal', () => { + expect(resolveOriginForWrite('ai', true)).to.equal('ai'); + expect(resolveOriginForWrite('human', true)).to.equal('human'); + }); + + it('falls back to human for an unrecognized value from a service principal (never rejects)', () => { + expect(resolveOriginForWrite('bogus', true)).to.equal('human'); + expect(resolveOriginForWrite(undefined, true)).to.equal('human'); + }); + + it('always overrides to human for a user principal, regardless of the asserted value', () => { + expect(resolveOriginForWrite('ai', false)).to.equal('human'); + expect(resolveOriginForWrite('human', false)).to.equal('human'); + expect(resolveOriginForWrite(undefined, false)).to.equal('human'); + }); + }); + + describe('upsertPrompts - origin derived from the authenticated principal', () => { + function makeUpsertClient() { + return { + from: (table) => { + if (table === 'prompts') { + return { + select: () => ({ + eq: () => ({ + eq: () => thenable({ data: [], error: null }), + }), + }), + insert: () => ({ + select: () => thenable({ data: [{ prompt_id: 'new-1' }], error: null }), + }), + update: () => ({ eq: () => thenable({ error: null }) }), + }; + } + return makeChain({}); + }, + }; + } + + it('honors a service principal\'s asserted origin', async () => { + const result = await upsertPrompts({ + organizationId: ORG_ID, + brandUuid: BRAND_UUID, + prompts: [{ prompt: 'New prompt', regions: ['us'], origin: 'ai' }], + postgrestClient: makeUpsertClient(), + isServicePrincipal: true, + }); + expect(result.prompts[0].origin).to.equal('ai'); + }); + + it('overrides a user principal\'s asserted origin to human', async () => { + const result = await upsertPrompts({ + organizationId: ORG_ID, + brandUuid: BRAND_UUID, + prompts: [{ prompt: 'New prompt', regions: ['us'], origin: 'ai' }], + postgrestClient: makeUpsertClient(), + isServicePrincipal: false, + }); + expect(result.prompts[0].origin).to.equal('human'); + }); + + it('defaults to human when isServicePrincipal is omitted', async () => { + const result = await upsertPrompts({ + organizationId: ORG_ID, + brandUuid: BRAND_UUID, + prompts: [{ prompt: 'New prompt', regions: ['us'], origin: 'ai' }], + postgrestClient: makeUpsertClient(), + }); + expect(result.prompts[0].origin).to.equal('human'); + }); + }); + describe('bulkDeletePrompts', () => { it('throws when postgrestClient has no from', async () => { await expect( diff --git a/test/support/serenity/handlers/prompts-subworkspace.test.js b/test/support/serenity/handlers/prompts-subworkspace.test.js index 01c4d591a..5404cb296 100644 --- a/test/support/serenity/handlers/prompts-subworkspace.test.js +++ b/test/support/serenity/handlers/prompts-subworkspace.test.js @@ -154,7 +154,9 @@ describe('prompts-subworkspace handlers', () => { }, log); expect(result.created).to.have.length(1); expect(result.created[0]).to.include({ semrushPromptId: 'new-prompt', geoTargetId: 2840 }); - expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['p'], ['tag-1']); + // No classifier supplied, so the `type` step is skipped; the `origin` + // step always runs and appends the AI-authored origin id on create. + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WS, 'p-us-en', ['p'], ['tag-1', TAG_IDS.originAi]); expect(transport.publishProject).to.have.been.calledOnceWith(WS, 'p-us-en'); }); @@ -210,13 +212,13 @@ describe('prompts-subworkspace handlers', () => { }], }, log, classifyByBrandMention); expect(result.created[0].tagIds).to.deep.equal([ - TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, + TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originAi, ]); expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly( WS, 'p-us-en', ['is Acme good?'], - [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded], + [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originAi], ); }); @@ -311,10 +313,11 @@ describe('prompts-subworkspace handlers', () => { // The id is preserved — the edit is in place, never a re-create. expect(result.body.semrushPromptId).to.equal('old-id'); expect(transport.renamePrompt).to.have.been.calledOnceWithExactly(WS, 'p-us-en', 'old-id', 'new'); + // No caller-supplied origin id, so update falls back to the AI origin id. 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.originAi], replace: true }], ); expect(transport.deletePromptsByIds).to.not.have.been.called; expect(transport.createPromptsByIds).to.not.have.been.called; @@ -374,12 +377,12 @@ describe('prompts-subworkspace handlers', () => { }, log); expect(result.status).to.equal(200); expect(result.body.semrushPromptId).to.equal('old-id'); - expect(result.body.tagIds).to.deep.equal(['tag-cat-1']); + expect(result.body.tagIds).to.deep.equal(['tag-cat-1', TAG_IDS.originAi]); 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'], replace: true }], + [{ id: 'old-id', references: ['tag-cat-1', TAG_IDS.originAi], replace: true }], ); }); @@ -410,14 +413,14 @@ describe('prompts-subworkspace handlers', () => { }, log, classifyByBrandMention); expect(result.status).to.equal(200); expect(result.body.tagIds).to.deep.equal([ - TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, + TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originAi, ]); expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( WS, 'p-us-en', [{ id: 'old-id', - references: [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded], + references: [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originAi], replace: true, }], ); @@ -740,6 +743,6 @@ describe('prompts-subworkspace — defensive branch coverage', () => { languageCode: 'en', }, log); expect(result.status).to.equal(200); - expect(result.body.tagIds).to.deep.equal(['keep']); + expect(result.body.tagIds).to.deep.equal(['keep', TAG_IDS.originAi]); }); }); diff --git a/test/support/serenity/handlers/prompts.test.js b/test/support/serenity/handlers/prompts.test.js index 8a21d6003..9ff0dd2be 100644 --- a/test/support/serenity/handlers/prompts.test.js +++ b/test/support/serenity/handlers/prompts.test.js @@ -553,6 +553,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'hello' }], existing_count: 0, }), @@ -566,14 +567,16 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }, fakeLog()); expect(result.created).to.have.lengthOf(1); + // No classifier is supplied, so `type` is untouched; `origin` is always + // injected (AI on create) and appended to the echoed tagIds. expect(result.created[0]).to.deep.equal({ semrushPromptId: 'new-sem-id', geoTargetId: 2840, languageCode: 'en', text: 'hello', - tagIds: ['tag-cat-1', 'tag-child-1'], + tagIds: ['tag-cat-1', 'tag-child-1', TAG_IDS.originAi], }); - 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', TAG_IDS.originAi]); expect(transport.publishProject).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en'); }); @@ -643,6 +646,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 0, items: [], existing_count: 1, }), @@ -656,8 +660,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', TAG_IDS.originAi]); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep', TAG_IDS.originAi]); }); it('returns empty semrushPromptId (not the string "undefined") when createPromptsByIds returns an item with no id', async () => { @@ -666,6 +670,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ name: 'hello' }], }), @@ -692,6 +697,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }), publishProject: sinon.stub().resolves(), }; + transport.listProjectTags = makeListProjectTagsStub(); const tooLong = 'x'.repeat(201); const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { @@ -703,8 +709,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', TAG_IDS.originAi]); + expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', ['hello'], ['keep', TAG_IDS.originAi]); }); it('caps a bulk-create tagIds array at MAX_TAG_IDS (50), mirroring the list-read query cap', async () => { @@ -718,6 +724,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }), publishProject: sinon.stub().resolves(), }; + transport.listProjectTags = makeListProjectTagsStub(); const tooMany = Array.from({ length: 55 }, (_, i) => `tag-${i}`); const result = await handleCreatePrompts(transport, dataAccess, BRAND, WORKSPACE, { @@ -726,8 +733,10 @@ 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)); + // The MAX_TAG_IDS cap applies to CALLER-supplied ids (50); the server-injected + // origin id is additive, so the echoed set is 50 + 1. + expect(result.created[0].tagIds).to.have.lengthOf(51); + expect(result.created[0].tagIds).to.deep.equal([...tooMany.slice(0, 50), TAG_IDS.originAi]); }); it('skips a create row when tagIds sanitizes to empty (every entry malformed)', async () => { @@ -753,6 +762,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([usEn]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'good' }], }), @@ -790,6 +800,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { const dataAccess = makeDataAccess([project]); const err = Object.assign(new Error('rate limited'), { status: 429 }); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().rejects(err), publishProject: sinon.stub().resolves(), }; @@ -822,6 +833,7 @@ describe('handlers/prompts.js — handleCreatePrompts', () => { }); const dataAccess = makeDataAccess([project]); const transport = { + listProjectTags: makeListProjectTagsStub(), createPromptsByIds: sinon.stub().resolves({ page: 1, total: 1, items: [{ id: 'new-sem-id', name: 'ok' }], }), @@ -999,6 +1011,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { dataAccess.BrandSemrushProject.findBySlice.resolves(project); const transport = { + listProjectTags: makeListProjectTagsStub(), renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'next', is_updated: true }), updatePromptTagsByIds: sinon.stub().resolves(null), deletePromptsByIds: sinon.stub(), @@ -1025,13 +1038,13 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { geoTargetId: 2840, languageCode: 'en', text: 'next', - tagIds: ['tag-cat-1'], + tagIds: ['tag-cat-1', TAG_IDS.originAi], }); expect(transport.renamePrompt).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', 'sem-1', 'next'); expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( WORKSPACE, 'proj-us-en', - [{ id: 'sem-1', references: ['tag-cat-1'], replace: true }], + [{ id: 'sem-1', references: ['tag-cat-1', TAG_IDS.originAi], replace: true }], ); // Nothing is deleted or created anywhere on the edit path. expect(transport.deletePromptsByIds).to.have.callCount(0); @@ -1047,6 +1060,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { dataAccess.BrandSemrushProject.findBySlice.resolves(project); const transport = { + listProjectTags: makeListProjectTagsStub(), renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'next', is_updated: true }), updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), @@ -1064,11 +1078,11 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { fakeLog(), ); - expect(result.body.tagIds).to.deep.equal(['keep']); + expect(result.body.tagIds).to.deep.equal(['keep', TAG_IDS.originAi]); expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( WORKSPACE, 'proj-us-en', - [{ id: 'sem-1', references: ['keep'], replace: true }], + [{ id: 'sem-1', references: ['keep', TAG_IDS.originAi], replace: true }], ); }); @@ -1080,6 +1094,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { dataAccess.BrandSemrushProject.findBySlice.resolves(project); const transport = { + listProjectTags: makeListProjectTagsStub(), renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'next', is_updated: true }), updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), @@ -1101,11 +1116,11 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { fakeLog(), ); - expect(result.body.tagIds).to.deep.equal(['keep']); + expect(result.body.tagIds).to.deep.equal(['keep', TAG_IDS.originAi]); expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( WORKSPACE, 'proj-us-en', - [{ id: 'sem-1', references: ['keep'], replace: true }], + [{ id: 'sem-1', references: ['keep', TAG_IDS.originAi], replace: true }], ); }); @@ -1179,6 +1194,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { // stub lets us assert callCount(0) so a regression that brings the // walk back fails this test. listPromptsByTags: sinon.stub(), + listProjectTags: makeListProjectTagsStub(), renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'new text', is_updated: true }), updatePromptTagsByIds: sinon.stub().resolves(null), publishProject: sinon.stub().resolves(), @@ -1202,7 +1218,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { geoTargetId: 2840, languageCode: 'en', text: 'new text', - tagIds: ['tag-fresh'], + tagIds: ['tag-fresh', TAG_IDS.originAi], }); expect(transport.listPromptsByTags).to.have.callCount(0); expect(transport.renamePrompt).to.have.been.calledOnceWithExactly(WORKSPACE, 'proj-us-en', 'sem-1', 'new text'); @@ -1220,6 +1236,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { // must NOT trip the promptNotFound path. const err = new SerenityTransportError(404, 'not found'); const transport = { + listProjectTags: makeListProjectTagsStub(), renamePrompt: sinon.stub().rejects(err), updatePromptTagsByIds: sinon.stub(), }; @@ -1254,6 +1271,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const err = new SerenityTransportError(409, 'conflict'); const transport = { + listProjectTags: makeListProjectTagsStub(), renamePrompt: sinon.stub().rejects(err), updatePromptTagsByIds: sinon.stub(), publishProject: sinon.stub(), @@ -1283,6 +1301,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const err = Object.assign(new Error('upstream 503'), { status: 503 }); const transport = { + listProjectTags: makeListProjectTagsStub(), renamePrompt: sinon.stub().rejects(err), updatePromptTagsByIds: sinon.stub(), }; @@ -1313,6 +1332,7 @@ describe('handlers/prompts.js — handleUpdatePrompt', () => { const tagErr = Object.assign(new Error('tag write boom'), { status: 500 }); const transport = { + listProjectTags: makeListProjectTagsStub(), renamePrompt: sinon.stub().resolves({ id: 'sem-1', name: 'x', is_updated: true }), updatePromptTagsByIds: sinon.stub().rejects(tagErr), publishProject: sinon.stub().resolves(), @@ -1677,7 +1697,9 @@ describe('handlers/prompts.js — tag cache invalidation (Important #6)', () => }); // Step 1: populate cache via handleListTags with set A. - const transport = { listPromptsByTags }; + // listProjectTags feeds the origin-tag-tree resolution the create/update + // handlers now perform (harmless to the bulk-delete path, which never reads it). + const transport = { listPromptsByTags, listProjectTags: makeListProjectTagsStub() }; await handleListTags(transport, dataAccess, BRAND, WORKSPACE, { geoTargetId: 2840, languageCode: 'en', }, fakeLog()); @@ -1830,13 +1852,13 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }, fakeLog(), classifyByBrandMention); expect(result.created[0].tagIds).to.deep.equal([ - TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, + TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originAi, ]); expect(transport.createPromptsByIds).to.have.been.calledOnceWithExactly( WORKSPACE, 'proj-us-en', ['is Acme good?'], - [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded], + [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originAi], ); // The whole taxonomy already exists, so nothing is provisioned. expect(transport.createProjectTags).to.not.have.been.called; @@ -1862,7 +1884,7 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }, fakeLog(), classifyByBrandMention); expect(result.created[0].tagIds).to.deep.equal([ - TAG_IDS.categoryRunningShoes, TAG_IDS.typeNonBranded, + TAG_IDS.categoryRunningShoes, TAG_IDS.typeNonBranded, TAG_IDS.originAi, ]); }); @@ -1900,7 +1922,9 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }], }, fakeLog(), classifyByBrandMention); - expect(result.created[0].tagIds).to.deep.equal([decoyCategoryId, TAG_IDS.typeBranded]); + expect(result.created[0].tagIds).to.deep.equal([ + decoyCategoryId, TAG_IDS.typeBranded, TAG_IDS.originAi, + ]); }); // Projects that predate the taxonomy carry none of it; the first write @@ -1931,7 +1955,12 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) expect(createProjectTags.firstCall.args[3]).to.deep.equal({}); expect(createProjectTags.secondCall.args[2]).to.deep.equal(['branded']); expect(createProjectTags.secondCall.args[3]).to.deep.equal({ parentId: 'created::type' }); - expect(result.created[0].tagIds).to.deep.equal(['tag-cat-1', 'created:created::type:branded']); + // The `origin` value `ai` is then minted under the freshly-created origin root. + expect(createProjectTags.thirdCall.args[2]).to.deep.equal(['ai']); + expect(createProjectTags.thirdCall.args[3]).to.deep.equal({ parentId: 'created::origin' }); + expect(result.created[0].tagIds).to.deep.equal([ + 'tag-cat-1', 'created:created::type:branded', 'created:created::origin:ai', + ]); }); }); @@ -1955,7 +1984,7 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) expect(result.status).to.equal(200); expect(result.body.tagIds).to.deep.equal([ - TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, + TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originAi, ]); // The injector's output is the full replacement set the tag write sends. expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( @@ -1963,7 +1992,46 @@ 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.originAi], + replace: true, + }], + ); + }); + + // The create/update asymmetry (the whole point of `mode: 'update'`): an edit + // must NOT re-derive origin. A human-authored prompt keeps `human` on every + // subsequent edit — the injector re-injects whichever origin tag the caller + // round-tripped, never the AI default it applies on create. Without this, + // every edit would silently relabel a human prompt as AI-authored. + it('preserves an existing human origin on edit — never relabels it to ai', async () => { + const dataAccess = makeDataAccess([]); + dataAccess.BrandSemrushProject.findBySlice.resolves(project()); + const transport = { + listProjectTags: makeListProjectTagsStub(), + renamePrompt: sinon.stub().resolves({ id: 'old-id', name: 'now mentions Acme', is_updated: true }), + updatePromptTagsByIds: sinon.stub().resolves(null), + publishProject: sinon.stub().resolves(), + }; + + const result = await handleUpdatePrompt(transport, dataAccess, BRAND, WORKSPACE, 'old-id', { + text: 'now mentions Acme', + geoTargetId: 2840, + languageCode: 'en', + // The caller round-trips the prompt's stored human origin tag. + tagIds: [TAG_IDS.categoryRunningShoes, TAG_IDS.originHuman], + }, fakeLog(), classifyByBrandMention); + + expect(result.status).to.equal(200); + // origin stays human — re-injected from the caller's tags, not defaulted to ai. + expect(result.body.tagIds).to.deep.equal([ + TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originHuman, + ]); + expect(transport.updatePromptTagsByIds).to.have.been.calledOnceWithExactly( + WORKSPACE, + 'proj-us-en', + [{ + id: 'old-id', + references: [TAG_IDS.categoryRunningShoes, TAG_IDS.typeBranded, TAG_IDS.originHuman], replace: true, }], ); @@ -2001,32 +2069,40 @@ describe('handlers/prompts.js — unified type classification (serenity-docs#31) }; const inject = makeTypeInjector(transport, WORKSPACE, classifyByBrandMention, fakeLog()); - // Resolving one (project, type) key reads two levels: the roots, then the - // children of the `type` root. + // Resolving one (project, type) key reads two levels (the roots, then the + // children of the `type` root); the `origin` step then reads two more (the + // roots again, then the origin root's children) — 4 reads for the first call. const a = await inject('proj-1', { text: 'love Acme', geoTargetId: 2840, tagIds: ['x'] }); - expect(transport.listProjectTags).to.have.callCount(2); + expect(transport.listProjectTags).to.have.callCount(4); // Same project + same computed type => served from cache, no new reads. + // The `origin` cache key (project + AI) is likewise unchanged, so no reads. const b = await inject('proj-1', { text: 'Acme rocks', geoTargetId: 2840, tagIds: ['y'] }); - expect(transport.listProjectTags).to.have.callCount(2); - expect(a.tagIds).to.deep.equal(['x', TAG_IDS.typeBranded]); - expect(b.tagIds).to.deep.equal(['y', TAG_IDS.typeBranded]); + expect(transport.listProjectTags).to.have.callCount(4); + expect(a.tagIds).to.deep.equal(['x', TAG_IDS.typeBranded, TAG_IDS.originAi]); + expect(b.tagIds).to.deep.equal(['y', TAG_IDS.typeBranded, TAG_IDS.originAi]); - // A different computed type is a new cache key => one more resolution. + // A different computed type is a new cache key => one more type resolution + // (2 reads). Origin is still cached, so it adds none. const c = await inject('proj-1', { text: 'best running shoes', geoTargetId: 2840, tagIds: ['z'] }); - expect(transport.listProjectTags).to.have.callCount(4); - expect(c.tagIds).to.deep.equal(['z', TAG_IDS.typeNonBranded]); + expect(transport.listProjectTags).to.have.callCount(6); + expect(c.tagIds).to.deep.equal(['z', TAG_IDS.typeNonBranded, TAG_IDS.originAi]); expect(transport.createProjectTags).to.not.have.been.called; }); - it('passes the input through untouched when no classifier is supplied', async () => { - const transport = { listProjectTags: sinon.stub(), createProjectTags: sinon.stub() }; + it('skips the type step but still injects origin when no classifier is supplied', async () => { + const transport = { + listProjectTags: makeListProjectTagsStub(), + createProjectTags: sinon.stub(), + }; const inject = makeTypeInjector(transport, WORKSPACE, undefined, fakeLog()); const out = await inject('proj-1', { text: 'anything', geoTargetId: 2840, tagIds: ['x'] }); - expect(out.tagIds).to.deep.equal(['x']); - expect(transport.listProjectTags).to.not.have.been.called; + // No classifier => `type` is left untouched, but `origin` is always + // injected (AI on create), so the tag tree is still read for it. + expect(out.tagIds).to.deep.equal(['x', TAG_IDS.originAi]); + expect(transport.listProjectTags).to.have.been.called; }); }); });