feat(data-access): guidance + feedback_subject_id + code-patch export gate for feedback

[tathagat2241]

Summary

Feedback-event model changes for the Learning Agent loop (SITES-43974).

• toJsonlRow now emits guidance_markdown.
• Opt-out (OPT_OUT_STRIPPED_FIELDS) now strips only the human-authored fields (detail_markdown, edited_fix). The AI-generated guidance_markdown + previous_fix and all metadata always ship.
• shouldExport now requires a code patch (previous_fix present) and not product_bug. Text-guidance-only feedback stays in Postgres and is not exported (reserved for a future text-guidance corpus).
• toReviewView returns feedbackSubjectId (base view, for per-issue grouping) and guidanceMarkdown (with patches).
• .d.ts updated.

Testing

• 28 unit tests pass; lint clean; 100% coverage maintained.

Downstream

bump @adobe/spacecat-shared-data-access after release: spacecat-jobs-dispatcher (export gate + guidance in JSONL) and spacecat-api-service (read view returns the new fields).

Please ensure your pull request adheres to the following guidelines:

• make sure to link the related issues in this description
• when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

Related Issues

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

Thanks for contributing!

[ramboz]

Summary

The new review-view fields and code-patch export gate are well tested, but the opt-out boundary now exports customer-derived patch and guidance content for organizations with training_opt_in=false.

Blocker

B1. Preserve the canonical opt-out boundary. Keep previous_fix and guidance_markdown stripped unless the product/privacy policy and authoritative database contract are revised together.

Verdict

Ready to merge: No
Reasoning: B1 can export customer-derived content contrary to the current opt-out contract.

For a deeper dedicated security pass, consider invoking /adobe-security-foundations followed by /adobe-security-lang.

[absarasw]

LGTM. The three-way rebalance across toJsonlRow / shouldExport / toReviewView / OPT_OUT_STRIPPED_FIELDS is clean and each choice matches its downstream contract.

• OPT_OUT_STRIPPED_FIELDS = ['detail_markdown', 'edited_fix'] — strips only human-authored fields, so opted-out orgs still contribute the AI half of the corpus (guidance_markdown + previous_fix). This is the right posture and it's now single-sourced here.
• toJsonlRow includes guidance_markdown at top level, deliberately omits feedback_subject_id — matches the migration's COMMENT ON COLUMN in #790 ("NOT exported to S3"). The Learning Agent gets the AI issue context but not the Backoffice grouping key. ✓
• shouldExport gates on previous_fix != null — text-guidance-only reviews stay in Postgres for the future text corpus, patch-carrying reviews export today. Correct.
• toReviewView puts feedbackSubjectId in the base view (the Backoffice filter needs it on every read, not gated behind ?include=patches) and guidanceMarkdown behind includePatches (64 KB, heavy — don't load unless needed). Right trade-off.

28 tests, coverage maintained, .d.ts in sync.

Minor observation: includePatches is now a slight misnomer since it also gates guidanceMarkdown — but renaming would ripple through every caller for a naming-only win. Not worth it.

Downstream sequencing noted in the PR body — the api-service + jobs-dispatcher bumps that consume these changes are the follow-up (cross-repo comment forthcoming).

[absarasw]

Cross-repo seam check for the 4-PR batch

Posting the cross-repo view here since #1809 is the shared library — the linchpin for the other three PRs. Reviewed all four side-by-side and walked each seam (per the CR-1 lesson from the previous round). Per-PR reviews on each PR are separate; this is the seam-level pass.

PRs in the batch:

• mysticat-data-service#790 — feedback_event columns
• feat(data-access): guidance + feedback_subject_id + code-patch export gate for feedback #1809 (this PR) — model, toJsonlRow, toReviewView, opt-out, shouldExport
• spacecat-api-service#2798 — capture handler
• experience-success-studio-backoffice#338 — reviewer UI

✅ Seams that agree

Seam	Both sides agree
DB columns ↔ API insert	guidance_markdown + feedback_subject_id (snake) in #790 exactly match the controller row-object keys in #2798
API insert ↔ UI POST body	camelCase in UI (#338) → controller extracts + snake-cases into the row (#2798) — every field flows through
DB semantics ↔ JSONL contract	Migration COMMENT ON COLUMN in #790 ("feedback_subject_id NOT exported to S3") matches toJsonlRow in #1809 deliberately omitting the field
Opt-out single-sourced	Only #1809 defines OPT_OUT_STRIPPED_FIELDS. #2798 doesn't re-declare a strip list; #338 doesn't strip client-side
Text-only skip is a single decision	UI omits previousFix when the patch is empty (#338), shared shouldExport gates on previous_fix != null (#1809), exporter (jobs-dispatcher, downstream) uses shouldExport — one gate, three consistent implementations
REJECTION_CATEGORIES / VERDICT	Shared enums in #1809, UI re-declares in reviewConstants.js with an explicit comment that the shared package is Node/PostgREST and can't be bundled into App Builder. Documented drift, not accidental
OpenAPI schemas ↔ controller	maxLength (65536 / 200), types, and enum sets in #2798's OpenAPI match the controller's own validation exactly
Build order	#790 → #1809 (publish) → api-service bump → #2798 → jobs-dispatcher bump → #338. Nullable columns + optional fields make out-of-order merges safe, but full function needs the bumps

⚠️ Two cross-repo follow-ups needed

Both are already flagged in the PR bodies of #1809 and #338 as "post-release bumps" — surfacing here so they're tracked separately from the PRs.

X-1 — Shared dep bump is required to close the end-to-end loop.
After this PR publishes:

• spacecat-api-service must bump @adobe/spacecat-shared-data-access — so getReviewsForSuggestion → toReviewView returns feedbackSubjectId and guidanceMarkdown. Without this, the Backoffice per-issue filter in chore(deps): update dynamo-db-local #338 matches nothing (its "known limitation") and the previous-feedback table shows "No feedback yet" even after captures.
• spacecat-jobs-dispatcher must bump too — so buildJsonl uses the new shouldExport (text-guidance-only skip) and includes guidance_markdown at top level. Without this, the JSONL is still on the old contract and the LA gets patches without issue context.

X-2 — Gist-tarball pin carryover on spacecat-api-service (Mihir's X4).
@adobe/spacecat-shared-data-access in spacecat-api-service's package.json is still pinned to the https://gist.github.com/tathagat2241/…/*.tgz URL from the previous round. #2798 doesn't touch package.json (nor should it), but the X-1 bump PR is the natural place to repin from the gist to the published registry version — kills the supply-chain concern permanently.

Suggested merge + rollout order

1. fix: Test fix #790 merges (nullable ADD COLUMN — safe on live table).
2. feat(data-access): guidance + feedback_subject_id + code-patch export gate for feedback #1809 merges + publishes to npm.
3. Bump PR on spacecat-api-service (repin off the gist, bump to the new registry version).
4. #2798 merges (or rebase off the bump PR — either order is safe).
5. Bump PR on spacecat-jobs-dispatcher.
6. chore(deps): update dynamo-db-local #338 merges — the per-issue filter is populated end-to-end.

cc @tathagat2241

[tathagat2241]

@absarasw Thanks for the thorough read. Agreed includePatches is a slight misnomer now that it also gates guidanceMarkdown; not worth rippling through every caller for a naming-only change — leaving as-is.

[github-actions]

This PR will trigger a minor release when merged.

Resolved all the comments.

[solaris007]

🎉 This PR is included in version @adobe/spacecat-shared-data-access-v4.9.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀