feat: add aso-reviews endpoint for the ASO UI CWV feedback loop

[tathagat2241]

Summary

Adds a customer-facing review-capture endpoint so the ASO UI can record CWV feedback into the same Learning Agent loop as the Backoffice (SITES-39002). Reuses the existing feedback_event store and daily S3 export — the only differentiator is source = aso_ui.

Changes

• Shared core: refactored createBackofficeReview into captureReview(context, { source, allowedRejectionCategories }). Two thin wrappers now call it:
  • createBackofficeReview — source=backoffice, all rejection categories.
  • createAsoReview — source=aso_ui, categories limited to bad_recommendation / other (no product_bug from customers).
• Route: POST /sites/:siteId/opportunities/:opportunityId/suggestions/:suggestionId/aso-reviews, source bound by the route (never trusted from the body, FR-10). Classified in facs-capabilities.js (LLMO + ASO) and required-capabilities.js.
• OpenAPI: new aso-reviews path (no editedFix/stateTransition; rejectionCategory enum [bad_recommendation, other]; reuses the SuggestionReview response).
• Feedback is pure signal — it does not change suggestion status (state_transition null).

Reuse (no changes needed)

feedback_event already allows source=aso_ui; the export gate (previous_fix present AND not product_bug) and daily cadence are unchanged — ASO rows with a code patch export, patch-less ones stay in Postgres.

Testing

New ASO controller tests (source stamping, product_bug→400, source-mismatch→400); updated the routes-list + expected-functions guard tests. 479 controller + 3133 route-governance tests pass; docs:lint valid; lint clean.

Jira: https://jira.corp.adobe.com/browse/SITES-39002

Please ensure your pull request adheres to the following guidelines:

• 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.
• 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:

• 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.
• 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:

• make sure you update the API specification and the examples to reflect the changes.

If the PR is introducing a new audit type:

• make sure you update the API specification with the type, schema of the audit result and an example

Related Issues

Thanks for contributing!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[absarasw]

Structurally clean batch. The createBackofficeReview → shared captureReview(context, { source, allowedRejectionCategories }) refactor is exactly the right shape — no duplication, both routes share the same auth/validation/scrubbing/insert plumbing, and the ASO wrapper cleanly restricts to bad_recommendation/other (no product_bug from customers). FACS + required-capabilities entries added for both LLMO + ASO products (aso/can_edit). Tests cover source stamping, category restriction, and the FR-10 source-mismatch 400. 479 controller + 3133 route-governance tests pass.

Findings

One Important cross-repo observation, one Minor OpenAPI doc gap, and one carryover from the previous round.

Important — captureReview is silent on every non-2xx exit except the DB insert error path

Grepped the function: only two log lines emit — feedback_capture.scrub_hit_total (on success + scrub hits) and Failed to record review for suggestion ${id} (on the Postgres error path). Every other exit is silent from a monitoring perspective:

Exit	Currently logged?
400 — missing/invalid event_id	❌
400 — invalid verdict / rejection_category / state_transition	❌
400 — body-supplied source mismatch (the FR-10 abuse case)	❌
400 — non-string detail_markdown / guidance_markdown / feedback_subject_id	❌
400 — over-long feedback_subject_id	❌
403 — cross-tenant	❌
404 — site/suggestion not found	❌
413 — detail_markdown >8 KB	❌
413 — guidance_markdown >64 KB	❌
503 — feedback store unavailable	❌
500 — Postgres insert failed	✅
200/201 with scrub hits	✅

Why this matters now. With the Backoffice review path we could tolerate silent 4xx — ESE audience, tight blast radius, immediate feedback loop. Opening a customer-facing endpoint (aso_ui) changes the picture:

• Customer hits the 8 KB detail_markdown cap — 413 is returned; the customer sees an unexplained "Failed to submit feedback" toast; we never learn the cap is a problem, or how often it happens.
• Client bug sends malformed data at scale — every affected customer gets a generic toast; we don't discover the drift until someone escalates.
• FR-10 violation (body-supplied source: 'backoffice' from a curl-fu request against /aso-reviews) — this is the exact abuse case FR-10 was designed to protect against, and there's no signal it happens.
• The ASO UI has no client-side error-reporting infrastructure (grepped: no Sentry / DataDog RUM / Bugsnag / Rollbar — only omega for product analytics, which doesn't fire on failure). Server-side logs are the only observability surface. Silent 4xx = truly silent.

Suggested fix. Add a context.log?.warn?.(...) at every non-2xx exit with the source and the reason. Same log format the scrub-hit line already uses — Splunk can then slice by source (aso_ui vs backoffice) and reason. Applies retroactively to Backoffice too since it's the same shared captureReview (pre-existing, not new to this PR, but the customer-facing rollout makes it actionable).

Sketch:

if (!isValidUUID(eventId)) {
  context.log?.warn?.(`feedback_capture.rejected source=${source} suggestion=${suggestionId} reason=invalid_event_id`);
  return badRequest('event_id is required and must be a UUID');
}
// ... and analogous lines at every other 4xx/413/503 exit

No new dependency, no schema change, no UI change needed for coverage. Fits in the same PR or a follow-up.

Minor — OpenAPI 413 doc only mentions detail_markdown

The aso-reviews route documents 413: description: detail_markdown exceeds the 8 KB limit, but the controller also returns 413 for guidance_markdown exceeds the 64 KB limit. Broaden the description or add a second 413 example. Cosmetic.

Cross-repo carryover — gist tarball pin (Mihir's X4)

package.json still pins @adobe/spacecat-shared-data-access to https://gist.github.com/tathagat2241/…/*.tgz. Not touched this PR, not new — same follow-up bump PR we've been flagging since Round 1. Fold the repin into whichever PR bumps to the newly-published shared version.

Assessment

Ready to merge? With the observability fix — highly recommend. It's the smallest change with the biggest reduction in production blind spots, especially given the ASO UI has no client-side error reporting to fill the gap.

[github-actions]

This PR will trigger a minor release when merged.

[tathagat2241]

Important (observability):

Fixed in the shared captureReview — added a rejected(reason, response) helper that emits one context.log.warn('feedback_capture.rejected source=… suggestion=… reason=…') at every non-2xx exit (400/403/404/413/503), same format as the scrub-hit line. Splunk can slice by source (aso_ui vs backoffice) and reason, and since it's the shared function it covers the backoffice path retroactively too. The FR-10 source_mismatch case is now observable. Added tests asserting the warn fires (with the right source/reason) and that a 201 emits none.

Minor (OpenAPI 413):

Broadened the 413 description on both aso-reviews and backoffice-reviews to cover guidance_markdown (64 KB) in addition to detail_markdown (8 KB). docs:lint passes.

Carryover (gist pin):

Agreed — leaving the spacecat-shared-data-access repin for the version-bump follow-up PR as flagged; not touching it here.