Skip to content

feat(brand-claims): trigger prerequisite audits before an on-demand claims run (LLMO-7263) #3208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
akshaymagapu wants to merge 4 commits into
base: main
Choose a base branch
from
+42 −16
Open

feat(brand-claims): trigger prerequisite audits before an on-demand claims run (LLMO-7263) #3208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
70f2618
feat(brand-claims): trigger prerequisite audits before an on-demand c…
akshaymagapu Sep 4, 2026
25ee3d7
test(brand-claims): update LlmoController request test for the prereq…
akshaymagapu Sep 4, 2026
790765e
feat(brand-claims): name the requester in the on-demand claims Slack …
akshaymagapu Sep 7, 2026
d839768
Revert "feat(brand-claims): name the requester in the on-demand claim…
akshaymagapu Sep 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/controllers/llmo/brand-claims.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ const WEEK_RE = /^\d{4}-W\d{2}$/;
// bypassable). Kept in step with project-elmo-ui's BRAND_CLAIMS_REQUEST_COOLDOWN_MS.
const BRAND_CLAIMS_AUDIT_TYPE = 'brand-claims';
const BRAND_CLAIMS_REQUEST_COOLDOWN_MS = 7 * 24 * 60 * 60 * 1000;
// Prerequisite audits fired before an on-demand claims run (LLMO-7263) so their
// fresh outputs (off-site brand presence, Wikipedia facts) are available to
// mystique's claims extraction. They share the audit queue with the claims
// trigger; the audit-worker delays the claims ready-signal (onDemand) to let
// these land first.
const BRAND_CLAIMS_PREREQUISITE_AUDIT_TYPES = ['offsite-brand-presence', 'wikipedia-analysis'];
// `model` is interpolated into the S3 key, so constrain it to alphanumerics,
// dots, hyphens, underscores — no `/` — to prevent using HeadObject as an
// object-existence probe across arbitrary key paths.
Expand Down Expand Up @@ -227,6 +233,17 @@ export async function handleRequestBrandClaims(context, site) {
}

try {
// Fire the prerequisite audits first so mystique has fresh inputs when it runs
// claims; the audit-worker delays the claims ready-signal (onDemand) to let these
// complete. All three land on the same audit queue.
for (const type of BRAND_CLAIMS_PREREQUISITE_AUDIT_TYPES) {
// eslint-disable-next-line no-await-in-loop

@MysticatBot MysticatBot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (blocking): The three sendMessage calls are sequential inside one try/catch. If the first prerequisite succeeds but a later send fails, the catch returns 500 - but the error log says "failed to enqueue audit" without naming which messages already landed on the queue. An operator investigating a partial failure has no signal about what was sent.

Track which messages were sent before the failure:

let enqueuedTypes = [];
try {
  for (const type of BRAND_CLAIMS_PREREQUISITE_AUDIT_TYPES) {
    await sqs.sendMessage(queueUrl, {
      type, siteId: site.getId(),
      auditContext: { trigger: 'on-demand-brand-claims' },
    });
    enqueuedTypes.push(type);
  }
  await sqs.sendMessage(queueUrl, { /* claims trigger */ });
  enqueuedTypes.push('brand-claims');
} catch (sqsError) {
  log.error(`Brand Claims on-demand: failed after enqueuing [${enqueuedTypes.join(', ')}] for site ${site.getId()}: ${sqsError.message}`);
  return internalServerError('Brand Claims on-demand is temporarily unavailable');
}

The orphaned prerequisite audit is harmless (idempotent, no onDemand flag), so the external contract does not need to change - only the error log needs to reflect what actually happened.

await sqs.sendMessage(queueUrl, {
type,
siteId: site.getId(),
auditContext: { trigger: 'on-demand-brand-claims' },
});
}
await sqs.sendMessage(queueUrl, {
type: 'brand-claims',
siteId: site.getId(),
Expand All @@ -239,7 +256,7 @@ export async function handleRequestBrandClaims(context, site) {
log.error(`Brand Claims on-demand: failed to enqueue audit for site ${site.getId()}: ${sqsError.message}`);
return internalServerError('Brand Claims on-demand is temporarily unavailable');
}
log.info(`Brand Claims on-demand: triggered brand-claims audit for site ${site.getId()}`);
log.info(`Brand Claims on-demand: triggered ${BRAND_CLAIMS_PREREQUISITE_AUDIT_TYPES.join(', ')} + brand-claims audits for site ${site.getId()}`);

// Dedicated channel for on-demand Brand Claims request alerts (LLMO-7263),
// set in Vault, so these can be routed/muted independently of other LLMO alerts.
Expand Down
32 changes: 19 additions & 13 deletions test/controllers/llmo/brand-claims.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,16 +392,22 @@ describe('handleRequestBrandClaims (on-demand, LLMO-7263)', () => {

afterEach(() => sandbox.restore());

it('triggers the brand-claims audit with onDemand and notifies Slack (202)', async () => {
it('triggers the prerequisite audits then the brand-claims audit (onDemand) and notifies Slack (202)', async () => {
const result = await handleRequestBrandClaims(context, site);
expect(result.status).to.equal(202);
expect(sqsSend).to.have.been.calledOnce;
const [queueUrl, msg] = sqsSend.getCall(0).args;
expect(queueUrl).to.equal('audit-q');
expect(msg.type).to.equal('brand-claims');
expect(msg.siteId).to.equal('site-1');
expect(msg.onDemand).to.equal(true);
expect(msg.auditContext).to.deep.equal({ trigger: 'on-demand-brand-claims' });
// offsite-brand-presence + wikipedia-analysis fire before brand-claims.
expect(sqsSend).to.have.been.calledThrice;
const types = sqsSend.getCalls().map((c) => c.args[1].type);
expect(types).to.deep.equal(['offsite-brand-presence', 'wikipedia-analysis', 'brand-claims']);
sqsSend.getCalls().forEach((c) => {
expect(c.args[0]).to.equal('audit-q');
expect(c.args[1].siteId).to.equal('site-1');
expect(c.args[1].auditContext).to.deep.equal({ trigger: 'on-demand-brand-claims' });
});
// Only the claims trigger carries onDemand; the prerequisite audits are plain triggers.
expect(sqsSend.getCall(0).args[1].onDemand).to.equal(undefined);
expect(sqsSend.getCall(1).args[1].onDemand).to.equal(undefined);
expect(sqsSend.getCall(2).args[1].onDemand).to.equal(true);
expect(postSlackMessage).to.have.been.calledOnce;
});

Expand All @@ -423,14 +429,14 @@ describe('handleRequestBrandClaims (on-demand, LLMO-7263)', () => {
postSlackMessage.rejects(new Error('slack down'));
const result = await handleRequestBrandClaims(context, site);
expect(result.status).to.equal(202);
expect(sqsSend).to.have.been.calledOnce;
expect(sqsSend).to.have.been.calledThrice;
});

it('skips Slack when not configured but still triggers the audit', async () => {
context.env.SLACK_BOT_TOKEN = undefined;
const result = await handleRequestBrandClaims(context, site);
expect(result.status).to.equal(202);
expect(sqsSend).to.have.been.calledOnce;
expect(sqsSend).to.have.been.calledThrice;
expect(postSlackMessage).to.not.have.been.called;
});

Expand All @@ -453,7 +459,7 @@ describe('handleRequestBrandClaims (on-demand, LLMO-7263)', () => {
getLatestAudit.resolves({ getAuditedAt: () => ranAt });
const result = await handleRequestBrandClaims(context, site);
expect(result.status).to.equal(202);
expect(sqsSend).to.have.been.calledOnce;
expect(sqsSend).to.have.been.calledThrice;
});

it('proceeds (202) at the 7-day boundary (cooldown uses strict <)', async () => {
Expand All @@ -463,14 +469,14 @@ describe('handleRequestBrandClaims (on-demand, LLMO-7263)', () => {
getLatestAudit.resolves({ getAuditedAt: () => ranAt });
const result = await handleRequestBrandClaims(context, site);
expect(result.status).to.equal(202);
expect(sqsSend).to.have.been.calledOnce;
expect(sqsSend).to.have.been.calledThrice;
});

it('fails open (202) when the cooldown lookup throws', async () => {
getLatestAudit.rejects(new Error('db down'));
const result = await handleRequestBrandClaims(context, site);
expect(result.status).to.equal(202);
expect(sqsSend).to.have.been.calledOnce;
expect(sqsSend).to.have.been.calledThrice;
expect(context.log.warn).to.have.been.called;
});
});
7 changes: 5 additions & 2 deletions test/controllers/llmo/llmo.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4542,10 +4542,13 @@ describe('LlmoController', () => {
};
});

it('validates LLMO access, triggers the audit, and returns 202', async () => {
it('validates LLMO access, triggers the prerequisite + claims audits, and returns 202', async () => {
const result = await controller.requestBrandClaims(reqCtx);
expect(result.status).to.equal(202);
expect(reqCtx.sqs.sendMessage).to.have.been.calledOnce;
// Prerequisite audits (offsite-brand-presence, wikipedia-analysis) fire before brand-claims.
expect(reqCtx.sqs.sendMessage).to.have.been.calledThrice;
const types = reqCtx.sqs.sendMessage.getCalls().map((c) => c.args[1].type);
expect(types).to.deep.equal(['offsite-brand-presence', 'wikipedia-analysis', 'brand-claims']);
});

it('returns 403 when LLMO access validation fails', async () => {
Expand Down
Loading