feat: rename semantic-value-visibility opportunity to image-enrichment

[yevheniia-severinovska]

Summary

Renames the audit/opportunity type from semantic-value-visibility to image-enrichment to align with multimedia-enrichment (video transcripts) naming convention. Decision per Slack alignment with Dirk + Yaashi.

Changes in this repo

• Renames src/semantic-value-visibility/ → src/image-enrichment/ (handler, guidance-handler, constants, opportunity-data-mapper)
• Renames test/audits/semantic-value-visibility/ and test/fixtures/semantic-value-visibility/ folders
• Updates HANDLERS map keys in src/index.js: 'semantic-value-visibility' → 'image-enrichment', 'guidance:semantic-value-visibility' → 'guidance:image-enrichment'
• Keeps old keys as deprecated aliases for one release window (per review feedback) — tolerates in-flight SQS messages from producers that haven't redeployed yet
• Updates AUDIT_TYPE, GUIDANCE_TYPE, OPPORTUNITY_TYPE constants
• Updates log prefixes, docstrings, test descriptions

Deploy ordering note

This PR can land independently. The deprecated HANDLERS aliases tolerate semantic-value-visibility messages from producers (jobs-dispatcher cron, mystique) that have not yet deployed. Aliases removed in a follow-up PR after producer cutover (mystique PR 1704 + jobs-dispatcher) reaches prod.

Cross-repo PRs

1. feat: rename semantic-value-visibility opportunity to image-enrichment spacecat-shared#1570 (provides new enum)
2. feat: rename semantic-value-visibility opportunity to image-enrichment #2447 ← this PR
3. https://git.corp.adobe.com/experience-platform/mystique/pull/1704 (mystique — renames OpportunityTypeEnum + agent folder + crew class)
4. adobe/project-elmo-ui#1643 (UI route alias)

Test plan

• Unit tests pass for test/audits/image-enrichment/** (24 tests)
• CI green: full build job passed (4m19s including coverage gate)
• End-to-end demo on kashi.com verified locally via mysticat: mystique → SQS → bridge → DDB persists type: image-enrichment → elmo UI renders correctly

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[dzehnder]

Hey @yevheniia-severinovska,

The rename is mechanically clean and well-tested. One in-scope safety improvement to the HANDLERS dispatch map is worth landing before merge.

Strengths

• Surgical 268-line rename across 9 files: import identifiers, HANDLERS map keys, log prefixes, test descriptions, esmock paths, fixture path/value, and the three constants in constants.js all move in lockstep.
• Constants centralized in src/image-enrichment/constants.js:13-15 (AUDIT_TYPE, GUIDANCE_TYPE, OPPORTUNITY_TYPE) - the wire contract has a single source of truth.
• Wire-contract assertion at test/audits/image-enrichment/handler.test.js:86 pins the literal 'guidance:image-enrichment' (not the constant), so a future typo in constants.js would still be caught.
• Fixture stub coverage: stale-opportunity branch at guidance-handler.test.js:319 correctly returns 'image-enrichment' from getType(), exercising the OPPORTUNITY_TYPE comparison at guidance-handler.js:71.
• No leftover references at PR head: zero semantic-value-visibility / semanticValueVisibility mentions in non-CHANGELOG files.
• semanticHtml untrusted-LLM-content safety annotation at guidance-handler.js:48 and the Array.isArray(suggestions) validation are preserved verbatim - the rename does not weaken any trust boundary.
• Naming aligns image-enrichment with the existing multimedia-enrichment (video) pattern: clean two-token taxonomy (modality + intent) that future audit types can slot into.

Issues

Important (Should Fix)

src/index.js:220-221 - HANDLERS map should keep the old keys as aliases for one release. Removing 'semantic-value-visibility' and 'guidance:semantic-value-visibility' means any in-flight SQS message with the old type hits the if (!handler) branch in src/index.js#run, logs a warn, and returns notFound(). SQS treats that as success, the message is acked, and the audit/guidance is silently dropped. Producers of these messages include scheduled crons in spacecat-jobs-dispatcher and in-flight mystique guidance responses - none of which are guaranteed to redeploy in lock-step with this PR.

Fix: add aliases for one release.

'image-enrichment': imageEnrichment,
'guidance:image-enrichment': imageEnrichmentGuidance,
// Deprecated aliases - remove after jobs-dispatcher cron + mystique PR 1704 reach prod
'semantic-value-visibility': imageEnrichment,
'guidance:semantic-value-visibility': imageEnrichmentGuidance,

Four in-scope lines. Converts a strict 4-PR merge ordering into a tolerant rollout, decouples deploy graphs of independently-deployed services, and survives any single-repo rollback. Removal is a one-line follow-up PR after the producers cut over.

Minor (Nice to Have)

No integration-level test asserts HANDLERS dispatcher registration. Both unit-test files import the handler module directly and bypass src/index.js#run. A future merge conflict that drops one of the HANDLERS map entries would not be caught by this audit's own tests. Optional follow-up: a single test that posts {type: 'image-enrichment'} and {type: 'guidance:image-enrichment'} to run and asserts dispatch lands in the right handler. Locks in the rename against wiring drift at very low cost.

Recommendations

• Add a one-line note to the PR description spelling out the deploy ordering constraint and the in-flight-message risk, so the on-call merger does not have to reconstruct it from the cross-repo merge list.
• If you take the alias path, add a // TODO(remove after jobs-dispatcher + mystique PR 1704 in prod) comment so the cleanup PR is greppable. Schedule the cleanup PR for ~2 weeks out.

Out of scope, worth tracking

• spacecat-jobs-dispatcher cron entry for auditType: 'semantic-value-visibility': if not renamed and deployed in lock-step, scheduled audits drop silently until updated. Highest-leverage external coupling.
• Outbound message type change to mystique: sendToMystique now emits type: 'guidance:image-enrichment' (verified at handler.test.js:86). Per the documented merge order, mystique PR 1704 deploys after this PR, so messages sent during the window will be unrecognized on the mystique side. Mystique-side handling, but reinforces the case for the inbound alias above (symmetric tolerance on both sides of the boundary).
• Coralogix dashboards/alerts and saved DataPrime queries pinned to log prefix [semantic-value-visibility] will silently lose signal once this deploys.
• Historical DDB opportunity records with type: 'semantic-value-visibility' are not migrated. UI must continue to handle both for the lifetime of those records, or a backfill must be scheduled.

Assessment

Ready to merge? With fixes.
Reasoning: The rename mechanics in this repo are clean and well-covered. The one in-scope safety improvement worth landing here is the 4-line HANDLERS alias - it costs almost nothing and decouples this repo's deploy from the jobs-dispatcher and mystique deploys. Without it, the merge is still safe only if the four-repo deploy is truly lock-step.

Next Steps

1. Add the HANDLERS aliases in src/index.js (4 lines + comment).
2. Add a one-line deploy ordering note to the PR description.
3. Track the jobs-dispatcher and Coralogix items separately (not in this PR).

[dzehnder]

Important: keep the old keys as aliases for one release.

Removing 'semantic-value-visibility' and 'guidance:semantic-value-visibility' from HANDLERS means any in-flight SQS message with the old type hits the if (!handler) branch in src/index.js#run, logs a warn, and returns notFound(). SQS treats that as success - the message is acked and the audit/guidance is silently dropped. Producers (scheduled crons in spacecat-jobs-dispatcher, in-flight mystique guidance responses) are not guaranteed to redeploy in lock-step with this PR.

Fix:

'image-enrichment': imageEnrichment,
'guidance:image-enrichment': imageEnrichmentGuidance,
// Deprecated aliases - remove after jobs-dispatcher cron + mystique PR 1704 reach prod
'semantic-value-visibility': imageEnrichment,
'guidance:semantic-value-visibility': imageEnrichmentGuidance,

Four in-scope lines. Converts a strict 4-PR merge ordering into a tolerant rollout.

[github-actions]

This PR will trigger a minor release when merged.

[yevheniia-severinovska]

Addressed in a640c36: kept old 'semantic-value-visibility' and 'guidance:semantic-value-visibility' HANDLERS keys as deprecated aliases pointing to the new handlers. In-flight SQS messages tolerated through rollout. Will remove the aliases in a follow-up PR after jobs-dispatcher + mystique #1704 reach prod.

[yevheniia-severinovska]

Round 2 — addressed all in-scope items:

• ✅ HANDLERS map keeps semantic-value-visibility and guidance:semantic-value-visibility as deprecated aliases (commit a640c36)
• ✅ PR description updated with deploy-ordering note + cross-repo coupling explanation

Skipped the integration test for HANDLERS dispatcher — the existing test setup requires @adobe/spacecat-shared-drs-client package that doesn't install cleanly in my local node_modules (CI handles it). Happy to add it if you want me to push despite local validation gap.

Re-requesting review.

[dzehnder]

Hey @yevheniia-severinovska,

Prior Important is cleanly addressed. Ready to merge.

Strengths

• Prior Important resolved: src/index.js:229-230 adds 'semantic-value-visibility' and 'guidance:semantic-value-visibility' as aliases to the same imageEnrichment / imageEnrichmentGuidance handler instances. The // @deprecated remove after jobs-dispatcher + mystique PR 1704 in prod comment names a concrete cleanup trigger that is greppable.
• Symmetric with the alias landing in the spacecat-shared companion: together they convert a strict 4-PR ordering into a tolerant rollout window. The deploy ordering note added to the PR description makes the strategy reviewable without spelunking through commits.
• 3-line change with no new code paths or behavior changes - just routes legacy SQS message types to the same handlers during the cross-repo rollout window.

Recommendations

• Worth lifting as a reusable pattern for future downstream-coupled renames in the spacecat ecosystem: every dual-key handler should name the specific upstream PR(s) whose merge unblocks removal, so the alias never outlives its reason. Not blocking - this PR already does it.
• Author offered to add the integration-level HANDLERS dispatcher test if pushed; the original Minor framing was "not blocking," and the local-env friction (@adobe/spacecat-shared-drs-client install) is a reasonable defer. Skipping is fine.

Assessment

Ready to merge? Yes.
Reasoning: The Important is cleanly resolved with the minimum viable diff. CI is green. Pushback on the integration test is reasonable given the original Minor framing and local validation gap.