From 9adb9b6fdb7ac016b7d511450c8b0cb2165cf628 Mon Sep 17 00:00:00 2001 From: Tathagat Sharma Date: Mon, 13 Jul 2026 14:28:06 +0530 Subject: [PATCH 1/3] feat: capture guidance_markdown + feedback_subject_id on backoffice reviews --- docs/openapi/schemas.yaml | 13 ++++- docs/openapi/site-opportunities.yaml | 15 ++++++ src/controllers/suggestions.js | 29 +++++++++-- src/support/feedback-redaction.js | 17 ++++++- test/controllers/suggestions-reviews.test.js | 52 ++++++++++++++++++++ test/support/feedback-redaction.test.js | 4 ++ 6 files changed, 124 insertions(+), 6 deletions(-) diff --git a/docs/openapi/schemas.yaml b/docs/openapi/schemas.yaml index 20c263296f..57214e2b05 100644 --- a/docs/openapi/schemas.yaml +++ b/docs/openapi/schemas.yaml @@ -12005,12 +12005,23 @@ SuggestionReview: tier: type: string enum: [paid, free] + feedbackSubjectId: + type: string + description: >- + SpaceCat-internal id of the sub-item this review is about within the + suggestion (e.g. a CWV issue id), used to group per-issue feedback. Null + for whole-suggestion reviews. previousFix: type: object - description: Patch snapshot at review time (only when ?include=...,patches). + description: AI-generated patch snapshot at review time (only when ?include=...,patches). editedFix: type: object description: ESE-edited patch (only when ?include=...,patches). + guidanceMarkdown: + type: string + description: >- + AI-generated issue context (title + description) for the reviewed issue + (only when ?include=...,patches). Null when not set. required: - eventId - eventTime diff --git a/docs/openapi/site-opportunities.yaml b/docs/openapi/site-opportunities.yaml index cf39fa5469..26ab5e3a8e 100644 --- a/docs/openapi/site-opportunities.yaml +++ b/docs/openapi/site-opportunities.yaml @@ -1264,6 +1264,21 @@ site-opportunity-suggestion-backoffice-reviews: type: string maxLength: 8192 description: Optional ESE rationale (markdown). Required on reject by the UI; sanitised + 8 KB cap server-side. + guidanceMarkdown: + type: string + maxLength: 65536 + description: >- + AI-generated issue context (title + description) for the reviewed issue. + Gives the Learning Agent "what the issue was" alongside the generated + patch (previousFix) and the review. Sanitised + 64 KB cap server-side. + feedbackSubjectId: + type: string + maxLength: 200 + description: >- + Optional SpaceCat-internal id of the sub-item this review is about + within the suggestion (e.g. a CWV issue id), so the Backoffice can group + per-issue "previous feedback" when issues share one suggestionId. + Independent of the Fix-entity issue id. Not exported to S3. rejectionCategory: type: string enum: [product_bug, bad_recommendation, other] diff --git a/src/controllers/suggestions.js b/src/controllers/suggestions.js index df30c8faf0..78576511d6 100644 --- a/src/controllers/suggestions.js +++ b/src/controllers/suggestions.js @@ -2872,8 +2872,8 @@ function SuggestionsController(ctx, sqs, env) { const body = isNonEmptyObject(context.data) ? context.data : {}; const { - eventId, verdict, detailMarkdown, rejectionCategory, - stateTransition, previousFix, editedFix, + eventId, verdict, detailMarkdown, guidanceMarkdown, rejectionCategory, + stateTransition, previousFix, editedFix, feedbackSubjectId, } = body; // FR-09: event_id is MANDATORY and client-supplied (no server fallback). @@ -2902,6 +2902,24 @@ function SuggestionsController(ctx, sqs, env) { return createResponse({ message: 'detail_markdown exceeds the 8 KB limit' }, 413); } } + // guidance_markdown is the AI-generated issue context (title + description). + // Larger cap than detail_markdown (64 KB) because issue descriptions + + // implementation guidance run long. + if (guidanceMarkdown != null) { + if (typeof guidanceMarkdown !== 'string') { + return badRequest('guidance_markdown must be a string'); + } + if (Buffer.byteLength(guidanceMarkdown, 'utf8') > 65536) { + return createResponse({ message: 'guidance_markdown exceeds the 64 KB limit' }, 413); + } + } + // feedback_subject_id is an opaque grouping id (e.g. a CWV issue id) — a short + // string, not free text. Bounded to guard against abuse. + if (feedbackSubjectId != null) { + if (typeof feedbackSubjectId !== 'string' || feedbackSubjectId.length > 200) { + return badRequest('feedback_subject_id must be a string of at most 200 characters'); + } + } const site = await Site.findById(siteId); if (!site) { @@ -2939,10 +2957,13 @@ function SuggestionsController(ctx, sqs, env) { const { detailMarkdown: cleanMarkdown, + guidanceMarkdown: cleanGuidance, previousFix: cleanPreviousFix, editedFix: cleanEditedFix, scrubHits, - } = redactFeedbackContent({ detailMarkdown, previousFix, editedFix }); + } = redactFeedbackContent({ + detailMarkdown, guidanceMarkdown, previousFix, editedFix, + }); const scrubbed = Object.entries(scrubHits); if (scrubbed.length > 0) { @@ -2959,6 +2980,8 @@ function SuggestionsController(ctx, sqs, env) { signal, reviewer_id: reviewerId, detail_markdown: cleanMarkdown ?? null, + guidance_markdown: cleanGuidance ?? null, + feedback_subject_id: feedbackSubjectId ?? null, previous_fix: cleanPreviousFix ?? null, edited_fix: cleanEditedFix ?? null, state_transition: hasText(stateTransition) ? stateTransition : null, diff --git a/src/support/feedback-redaction.js b/src/support/feedback-redaction.js index 5ba45a08dc..f79e9a3999 100644 --- a/src/support/feedback-redaction.js +++ b/src/support/feedback-redaction.js @@ -129,12 +129,16 @@ export function scrubDeep(value, hits) { * * @param {object} input * @param {string} [input.detailMarkdown] + * @param {string} [input.guidanceMarkdown] * @param {*} [input.previousFix] * @param {*} [input.editedFix] - * @returns {{ detailMarkdown: (string|undefined), previousFix: *, editedFix: *, + * @returns {{ detailMarkdown: (string|undefined), + * guidanceMarkdown: (string|undefined), previousFix: *, editedFix: *, * scrubHits: Record }} */ -export function redactFeedbackContent({ detailMarkdown, previousFix, editedFix } = {}) { +export function redactFeedbackContent({ + detailMarkdown, guidanceMarkdown, previousFix, editedFix, +} = {}) { const scrubHits = {}; let cleanMarkdown = detailMarkdown; @@ -142,8 +146,17 @@ export function redactFeedbackContent({ detailMarkdown, previousFix, editedFix } cleanMarkdown = scrubString(sanitizeMarkdown(detailMarkdown), scrubHits); } + // guidance_markdown is AI-generated issue context, but we secret-scrub it too + // (defence in depth — it can quote customer HTML/config). Sanitised like the + // reviewer's rationale. + let cleanGuidance = guidanceMarkdown; + if (typeof guidanceMarkdown === 'string') { + cleanGuidance = scrubString(sanitizeMarkdown(guidanceMarkdown), scrubHits); + } + return { detailMarkdown: cleanMarkdown, + guidanceMarkdown: cleanGuidance, previousFix: previousFix === undefined ? undefined : scrubDeep(previousFix, scrubHits), editedFix: editedFix === undefined ? undefined : scrubDeep(editedFix, scrubHits), scrubHits, diff --git a/test/controllers/suggestions-reviews.test.js b/test/controllers/suggestions-reviews.test.js index 3f026d4208..7a2ca10a5d 100644 --- a/test/controllers/suggestions-reviews.test.js +++ b/test/controllers/suggestions-reviews.test.js @@ -249,6 +249,58 @@ describe('Suggestions Controller - backoffice reviews', () => { expect(response.status).to.equal(413); }); + it('captures guidance_markdown (issue context) alongside the generated patch', async () => { + const context = makeContext({ + postgrest: { onInsert: () => ({ data: { event_id: EVENT_ID, signal: 'negative', tier: 'free' }, error: null }) }, + }); + context.data.guidanceMarkdown = '# Slow hero image\n\nLCP element is a 1.2 MB PNG.'; + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(201); + const inserted = context.dataAccess.services.postgrestClient.capturedInserts[0]; + expect(inserted.guidance_markdown).to.equal('# Slow hero image\n\nLCP element is a 1.2 MB PNG.'); + // the generated patch is stored too (the LA maps the review to it) + expect(inserted.previous_fix).to.deep.equal({ patch: 'before' }); + }); + + it('rejects oversize guidance_markdown with 413 (64 KB cap)', async () => { + const context = makeContext(); + context.data.guidanceMarkdown = 'x'.repeat(65537); + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(413); + }); + + it('rejects a non-string guidance_markdown with 400', async () => { + const context = makeContext(); + context.data.guidanceMarkdown = { not: 'a string' }; + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(400); + }); + + it('stores feedback_subject_id (per-issue grouping key) on the row', async () => { + const context = makeContext({ + postgrest: { onInsert: () => ({ data: { event_id: EVENT_ID, signal: 'negative', tier: 'free' }, error: null }) }, + }); + context.data.feedbackSubjectId = 'issue-42'; + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(201); + const inserted = context.dataAccess.services.postgrestClient.capturedInserts[0]; + expect(inserted.feedback_subject_id).to.equal('issue-42'); + }); + + it('rejects a non-string feedback_subject_id with 400', async () => { + const context = makeContext(); + context.data.feedbackSubjectId = { not: 'a string' }; + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(400); + }); + + it('rejects an over-long feedback_subject_id with 400', async () => { + const context = makeContext(); + context.data.feedbackSubjectId = 'x'.repeat(201); + const response = await controller.createBackofficeReview(context); + expect(response.status).to.equal(400); + }); + it('returns 404 when the site is not found', async () => { mockDataAccess.Site.findById.resolves(null); const response = await controller.createBackofficeReview(makeContext()); diff --git a/test/support/feedback-redaction.test.js b/test/support/feedback-redaction.test.js index 9f1b566f9c..838f0547c1 100644 --- a/test/support/feedback-redaction.test.js +++ b/test/support/feedback-redaction.test.js @@ -129,12 +129,16 @@ describe('feedback-redaction', () => { it('sanitises + scrubs markdown and scrubs both patches, aggregating hits', () => { const result = redactFeedbackContent({ detailMarkdown: ' key AKIAABCDEFGHIJKLMNOP me@adobe.com', + guidanceMarkdown: ' issue at foo.corp.adobe.com', previousFix: { code: 'Bearer abcdefghijklmnop' }, editedFix: { code: `ghp_${'z'.repeat(25)}` }, }); expect(result.detailMarkdown).to.not.contain('