Skip to content

Commit 1435d52

Browse files
authored
feat: capture guidance_markdown + feedback_subject_id on backoffice reviews (#2798)
## Summary Extends the Backoffice **review-capture** endpoint for the Learning Agent feedback loop (SITES-43974) to persist two new fields, and consumes the companion `@adobe/spacecat-shared-data-access` release so they round-trip on read. - `guidanceMarkdown` — AI-generated issue context (title + description). Validated as a string, 64 KB cap (413 on overflow), secret-scrubbed + HTML-sanitised via `redactFeedbackContent`, stored as `guidance_markdown`. - `feedbackSubjectId` — SpaceCat-internal per-issue grouping key (e.g. a CWV issue id). Validated as a string ≤ 200 chars, stored as `feedback_subject_id`. Lets the Backoffice group per-issue "previous feedback" when issues share one `suggestionId`. ⚠️ Deploy order: depends on mysticat-data-service #790 (the `feedback_event` columns) being migrated first. ## Changes - `src/controllers/suggestions.js` (`createBackofficeReview`): accept + validate + scrub + store `guidanceMarkdown` and `feedbackSubjectId` on the inserted row. - `src/support/feedback-redaction.js`: secret-scrub `guidanceMarkdown` alongside the other content fields (defence-in-depth — guidance can quote customer HTML/config even though it's AI-generated). - OpenAPI: request body (`site-opportunities.yaml`) documents both fields; `SuggestionReview` response (`schemas.yaml`) adds `feedbackSubjectId` + `guidanceMarkdown`, typed as a `[string, 'null']` union (OAS 3.1) so the schema matches the runtime (null for whole-suggestion reviews / absent guidance). - `package.json` / lockfile: bump `@adobe/spacecat-shared-data-access` to the released version so `toReviewView` returns the new fields on `?include=reviews` (and repin off the gist tarball to the published registry version). - Tests: capture happy-paths + validation (400 non-string, 413 oversize) for both fields; redaction test for `guidanceMarkdown`; read-response test asserting `?include=reviews` returns `feedbackSubjectId` and `?include=reviews,patches` returns `guidanceMarkdown`. ## Testing Controller + redaction unit tests pass; `npm run docs:lint` valid; lint clean. Full suite runs in CI. Please ensure your pull request adheres to the following guidelines: - [X] make sure to link the related issues in this description. Or if there's no issue created, make sure you describe here the problem you're solving. - [X] when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes If the PR is changing the API specification: - [X] make sure you add a "Not implemented yet" note the endpoint description, if the implementation is not ready yet. Ideally, return a 501 status code with a message explaining the feature is not implemented yet. - [X] make sure you add at least one example of the request and response. If the PR is changing the API implementation or an entity exposed through the API: - [X] make sure you update the API specification and the examples to reflect the changes. If the PR is introducing a new audit type: - [X] make sure you update the API specification with the type, schema of the audit result and an example ## Related Issues Jira: https://jira.corp.adobe.com/browse/SITES-43974 Thanks for contributing!
1 parent ece06d8 commit 1435d52

8 files changed

Lines changed: 129 additions & 11 deletions

File tree

docs/openapi/schemas.yaml

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12188,12 +12188,23 @@ SuggestionReview:
1218812188
tier:
1218912189
type: string
1219012190
enum: [paid, free]
12191+
feedbackSubjectId:
12192+
type: [string, 'null']
12193+
description: >-
12194+
SpaceCat-internal id of the sub-item this review is about within the
12195+
suggestion (e.g. a CWV issue id), used to group per-issue feedback. Null
12196+
for whole-suggestion reviews.
1219112197
previousFix:
1219212198
type: object
12193-
description: Patch snapshot at review time (only when ?include=...,patches).
12199+
description: AI-generated patch snapshot at review time (only when ?include=...,patches).
1219412200
editedFix:
1219512201
type: object
1219612202
description: ESE-edited patch (only when ?include=...,patches).
12203+
guidanceMarkdown:
12204+
type: [string, 'null']
12205+
description: >-
12206+
AI-generated issue context (title + description) for the reviewed issue
12207+
(only when ?include=...,patches). Null when not set.
1219712208
required:
1219812209
- eventId
1219912210
- eventTime

docs/openapi/site-opportunities.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,6 +1264,21 @@ site-opportunity-suggestion-backoffice-reviews:
12641264
type: string
12651265
maxLength: 8192
12661266
description: Optional ESE rationale (markdown). Required on reject by the UI; sanitised + 8 KB cap server-side.
1267+
guidanceMarkdown:
1268+
type: string
1269+
maxLength: 65536
1270+
description: >-
1271+
AI-generated issue context (title + description) for the reviewed issue.
1272+
Gives the Learning Agent "what the issue was" alongside the generated
1273+
patch (previousFix) and the review. Sanitised + 64 KB cap server-side.
1274+
feedbackSubjectId:
1275+
type: string
1276+
maxLength: 200
1277+
description: >-
1278+
Optional SpaceCat-internal id of the sub-item this review is about
1279+
within the suggestion (e.g. a CWV issue id), so the Backoffice can group
1280+
per-issue "previous feedback" when issues share one suggestionId.
1281+
Independent of the Fix-entity issue id. Not exported to S3.
12671282
rejectionCategory:
12681283
type: string
12691284
enum: [product_bug, bad_recommendation, other]

package-lock.json

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@
8888
"@adobe/spacecat-shared-brand-client": "1.4.0",
8989
"@adobe/spacecat-shared-cloudflare-client": "1.2.1",
9090
"@adobe/spacecat-shared-content-client": "1.8.24",
91-
"@adobe/spacecat-shared-data-access": "4.8.0",
91+
"@adobe/spacecat-shared-data-access": "4.9.0",
9292
"@adobe/spacecat-shared-drs-client": "1.13.0",
9393
"@adobe/spacecat-shared-gpt-client": "1.6.23",
9494
"@adobe/spacecat-shared-http-utils": "1.34.0",

src/controllers/suggestions.js

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2872,8 +2872,8 @@ function SuggestionsController(ctx, sqs, env) {
28722872

28732873
const body = isNonEmptyObject(context.data) ? context.data : {};
28742874
const {
2875-
eventId, verdict, detailMarkdown, rejectionCategory,
2876-
stateTransition, previousFix, editedFix,
2875+
eventId, verdict, detailMarkdown, guidanceMarkdown, rejectionCategory,
2876+
stateTransition, previousFix, editedFix, feedbackSubjectId,
28772877
} = body;
28782878

28792879
// FR-09: event_id is MANDATORY and client-supplied (no server fallback).
@@ -2902,6 +2902,24 @@ function SuggestionsController(ctx, sqs, env) {
29022902
return createResponse({ message: 'detail_markdown exceeds the 8 KB limit' }, 413);
29032903
}
29042904
}
2905+
// guidance_markdown is the AI-generated issue context (title + description).
2906+
// Larger cap than detail_markdown (64 KB) because issue descriptions +
2907+
// implementation guidance run long.
2908+
if (guidanceMarkdown != null) {
2909+
if (typeof guidanceMarkdown !== 'string') {
2910+
return badRequest('guidance_markdown must be a string');
2911+
}
2912+
if (Buffer.byteLength(guidanceMarkdown, 'utf8') > 65536) {
2913+
return createResponse({ message: 'guidance_markdown exceeds the 64 KB limit' }, 413);
2914+
}
2915+
}
2916+
// feedback_subject_id is an opaque grouping id (e.g. a CWV issue id) — a short
2917+
// string, not free text. Bounded to guard against abuse.
2918+
if (feedbackSubjectId != null) {
2919+
if (typeof feedbackSubjectId !== 'string' || feedbackSubjectId.length > 200) {
2920+
return badRequest('feedback_subject_id must be a string of at most 200 characters');
2921+
}
2922+
}
29052923

29062924
const site = await Site.findById(siteId);
29072925
if (!site) {
@@ -2939,10 +2957,13 @@ function SuggestionsController(ctx, sqs, env) {
29392957

29402958
const {
29412959
detailMarkdown: cleanMarkdown,
2960+
guidanceMarkdown: cleanGuidance,
29422961
previousFix: cleanPreviousFix,
29432962
editedFix: cleanEditedFix,
29442963
scrubHits,
2945-
} = redactFeedbackContent({ detailMarkdown, previousFix, editedFix });
2964+
} = redactFeedbackContent({
2965+
detailMarkdown, guidanceMarkdown, previousFix, editedFix,
2966+
});
29462967

29472968
const scrubbed = Object.entries(scrubHits);
29482969
if (scrubbed.length > 0) {
@@ -2959,6 +2980,8 @@ function SuggestionsController(ctx, sqs, env) {
29592980
signal,
29602981
reviewer_id: reviewerId,
29612982
detail_markdown: cleanMarkdown ?? null,
2983+
guidance_markdown: cleanGuidance ?? null,
2984+
feedback_subject_id: feedbackSubjectId ?? null,
29622985
previous_fix: cleanPreviousFix ?? null,
29632986
edited_fix: cleanEditedFix ?? null,
29642987
state_transition: hasText(stateTransition) ? stateTransition : null,

src/support/feedback-redaction.js

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,21 +129,34 @@ export function scrubDeep(value, hits) {
129129
*
130130
* @param {object} input
131131
* @param {string} [input.detailMarkdown]
132+
* @param {string} [input.guidanceMarkdown]
132133
* @param {*} [input.previousFix]
133134
* @param {*} [input.editedFix]
134-
* @returns {{ detailMarkdown: (string|undefined), previousFix: *, editedFix: *,
135+
* @returns {{ detailMarkdown: (string|undefined),
136+
* guidanceMarkdown: (string|undefined), previousFix: *, editedFix: *,
135137
* scrubHits: Record<string, number> }}
136138
*/
137-
export function redactFeedbackContent({ detailMarkdown, previousFix, editedFix } = {}) {
139+
export function redactFeedbackContent({
140+
detailMarkdown, guidanceMarkdown, previousFix, editedFix,
141+
} = {}) {
138142
const scrubHits = {};
139143

140144
let cleanMarkdown = detailMarkdown;
141145
if (typeof detailMarkdown === 'string') {
142146
cleanMarkdown = scrubString(sanitizeMarkdown(detailMarkdown), scrubHits);
143147
}
144148

149+
// guidance_markdown is AI-generated issue context, but we secret-scrub it too
150+
// (defence in depth — it can quote customer HTML/config). Sanitised like the
151+
// reviewer's rationale.
152+
let cleanGuidance = guidanceMarkdown;
153+
if (typeof guidanceMarkdown === 'string') {
154+
cleanGuidance = scrubString(sanitizeMarkdown(guidanceMarkdown), scrubHits);
155+
}
156+
145157
return {
146158
detailMarkdown: cleanMarkdown,
159+
guidanceMarkdown: cleanGuidance,
147160
previousFix: previousFix === undefined ? undefined : scrubDeep(previousFix, scrubHits),
148161
editedFix: editedFix === undefined ? undefined : scrubDeep(editedFix, scrubHits),
149162
scrubHits,

test/controllers/suggestions-reviews.test.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,58 @@ describe('Suggestions Controller - backoffice reviews', () => {
249249
expect(response.status).to.equal(413);
250250
});
251251

252+
it('captures guidance_markdown (issue context) alongside the generated patch', async () => {
253+
const context = makeContext({
254+
postgrest: { onInsert: () => ({ data: { event_id: EVENT_ID, signal: 'negative', tier: 'free' }, error: null }) },
255+
});
256+
context.data.guidanceMarkdown = '# Slow hero image\n\nLCP element is a 1.2 MB PNG.';
257+
const response = await controller.createBackofficeReview(context);
258+
expect(response.status).to.equal(201);
259+
const inserted = context.dataAccess.services.postgrestClient.capturedInserts[0];
260+
expect(inserted.guidance_markdown).to.equal('# Slow hero image\n\nLCP element is a 1.2 MB PNG.');
261+
// the generated patch is stored too (the LA maps the review to it)
262+
expect(inserted.previous_fix).to.deep.equal({ patch: 'before' });
263+
});
264+
265+
it('rejects oversize guidance_markdown with 413 (64 KB cap)', async () => {
266+
const context = makeContext();
267+
context.data.guidanceMarkdown = 'x'.repeat(65537);
268+
const response = await controller.createBackofficeReview(context);
269+
expect(response.status).to.equal(413);
270+
});
271+
272+
it('rejects a non-string guidance_markdown with 400', async () => {
273+
const context = makeContext();
274+
context.data.guidanceMarkdown = { not: 'a string' };
275+
const response = await controller.createBackofficeReview(context);
276+
expect(response.status).to.equal(400);
277+
});
278+
279+
it('stores feedback_subject_id (per-issue grouping key) on the row', async () => {
280+
const context = makeContext({
281+
postgrest: { onInsert: () => ({ data: { event_id: EVENT_ID, signal: 'negative', tier: 'free' }, error: null }) },
282+
});
283+
context.data.feedbackSubjectId = 'issue-42';
284+
const response = await controller.createBackofficeReview(context);
285+
expect(response.status).to.equal(201);
286+
const inserted = context.dataAccess.services.postgrestClient.capturedInserts[0];
287+
expect(inserted.feedback_subject_id).to.equal('issue-42');
288+
});
289+
290+
it('rejects a non-string feedback_subject_id with 400', async () => {
291+
const context = makeContext();
292+
context.data.feedbackSubjectId = { not: 'a string' };
293+
const response = await controller.createBackofficeReview(context);
294+
expect(response.status).to.equal(400);
295+
});
296+
297+
it('rejects an over-long feedback_subject_id with 400', async () => {
298+
const context = makeContext();
299+
context.data.feedbackSubjectId = 'x'.repeat(201);
300+
const response = await controller.createBackofficeReview(context);
301+
expect(response.status).to.equal(400);
302+
});
303+
252304
it('returns 404 when the site is not found', async () => {
253305
mockDataAccess.Site.findById.resolves(null);
254306
const response = await controller.createBackofficeReview(makeContext());

test/support/feedback-redaction.test.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,16 @@ describe('feedback-redaction', () => {
129129
it('sanitises + scrubs markdown and scrubs both patches, aggregating hits', () => {
130130
const result = redactFeedbackContent({
131131
detailMarkdown: '<script>x</script> key AKIAABCDEFGHIJKLMNOP me@adobe.com',
132+
guidanceMarkdown: '<style>y</style> issue at foo.corp.adobe.com',
132133
previousFix: { code: 'Bearer abcdefghijklmnop' },
133134
editedFix: { code: `ghp_${'z'.repeat(25)}` },
134135
});
135136
expect(result.detailMarkdown).to.not.contain('<script>');
136137
expect(result.detailMarkdown).to.contain('[[REDACTED:aws_access_key]]');
137138
expect(result.detailMarkdown).to.contain('[[REDACTED:adobe_email]]');
139+
// guidance_markdown is sanitised + scrubbed the same way (defence in depth)
140+
expect(result.guidanceMarkdown).to.not.contain('<style>');
141+
expect(result.guidanceMarkdown).to.contain('[[REDACTED:adobe_internal_host]]');
138142
expect(result.previousFix.code).to.contain('[[REDACTED:bearer_token]]');
139143
expect(result.editedFix.code).to.contain('[[REDACTED:github_pat]]');
140144
expect(result.scrubHits.aws_access_key).to.be.greaterThan(0);

0 commit comments

Comments
 (0)