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 SemanticValueVisibilityMapper class → ImageEnrichmentMapper in spacecat-shared-tokowaka-client
• Renames mapper file, test file, and fixtures folder accordingly
• Updates mapper-registry import + registration
• Updates audit.model.js enum: SEMANTIC_VALUE_VISIBILITY: 'semantic-value-visibility' → IMAGE_ENRICHMENT: 'image-enrichment'
• Updates opportunityType value in 5 fixture JSONs and matching test expectations

Cross-repo PRs (must merge in this order)

1. feat: rename semantic-value-visibility opportunity to image-enrichment #1570 ← this PR
2. feat: rename semantic-value-visibility opportunity to image-enrichment spacecat-audit-worker#2447 (depends on this — uses new enum + folder structure)
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

• npm test — all unit tests pass (2540 tests across tokowaka-client + data-access)
• CI green: full Test job passed across all 18 packages
• 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

[dzehnder]

Hey @yevheniia-severinovska,

The rename is clean and the cross-repo merge plan is well-documented. One in-scope safety improvement worth considering before merge.

Strengths

• Tight, focused diff: enum key, mapper class name, opportunityType field, fixture folder, log strings, JSDoc, and consuming test paths all move in lockstep (no half-renames).
• Naming alignment with multimedia-enrichment (video transcripts) is the right forward call.
• Mapper class export is internal: packages/spacecat-shared-tokowaka-client/src/index.js only re-exports FastlyKVClient and calculateForwardedHost, so renaming SemanticValueVisibilityMapper to ImageEnrichmentMapper does not break the npm public API.
• Test enforces the new literal: image-enrichment-mapper.test.js:46 asserts getOpportunityType() returns 'image-enrichment', so a future revert would fail loudly.
• Fixture rename is consistent: only opportunityType changed; other payload fields (semanticHtml, imageUrl, transformRules) correctly preserved.
• No leftover references in this repo: a tree walk at PR head finds zero remaining semantic-value-visibility / SemanticValueVisibility strings.

Issues

Important (Should Fix)

packages/spacecat-shared-data-access/src/models/audit/audit.model.js:98 - removing SEMANTIC_VALUE_VISIBILITY from the published AUDIT_TYPES enum is a breaking change to a public export. No symbolic consumer was found across adobe/* (consumers reference the string literal directly), but AUDIT_TYPES is part of the @adobe/spacecat-shared-data-access contract. Given the cross-repo merge order has this PR landing first, the lower-risk path is to keep both keys for one minor cycle:

IMAGE_ENRICHMENT: 'image-enrichment',
/** @deprecated use IMAGE_ENRICHMENT - remove after mystique#1704 + audit-worker#2447 in prod */
SEMANTIC_VALUE_VISIBILITY: 'image-enrichment',

Delete the alias in a follow-up PR once the producer side is fully cut over.

Minor (Nice to Have)

packages/spacecat-shared-tokowaka-client/src/mappers/mapper-registry.js:48 - in-flight messages with the old opportunityType during the rollout window. Between this PR + audit-worker PR 2447 deploying and mystique PR 1704 shipping, mystique will keep emitting opportunityType: 'semantic-value-visibility'. getMapper returns null for unknown types (no fallback), so those messages no-op at the mapping layer. There are two views on whether this needs mitigation:

• One view: register ImageEnrichmentMapper under both keys for one release as a safety net. Four-line change, removes the cross-repo merge ordering as a hard correctness constraint.
• Other view: the null-mapper return is the documented contract (mapper-registry.test.js:124-142 already asserts this), so unknown types are handled by design. No change needed.

Worth a sentence in the PR description either way - which path are you taking, and what is the expected behavior of in-flight semantic-value-visibility opportunities during the deployment window?

Existing persisted records. Audits and opportunities written before this rename retain auditType: 'semantic-value-visibility' / type: 'semantic-value-visibility'. With no migration, the platform carries two values for the same logical concept indefinitely. This appears consistent with the spacecat convention (no aliases on prior renames either), so not a blocker - but a one-line note in the data-access README listing legacy values would help downstream analytics consumers.

Recommendations

• If you take the alias path, add a // TODO(remove after mystique PR 1704 + audit-worker PR 2447 in prod) comment so the cleanup PR is greppable.

Out of scope, worth tracking

Coralogix/Splunk dashboards or saved DataPrime queries pinned to semantic-value-visibility will silently lose signal once new records start writing the new value. A grep of observability assets before the audit-worker companion lands is a small but useful step.

Assessment

Ready to merge? With fixes.
Reasoning: The rename is mechanically clean and well-scoped. The one in-scope improvement worth landing here is keeping SEMANTIC_VALUE_VISIBILITY as an aliased enum key for one release - it costs almost nothing and converts the strict 4-PR merge order from a hard constraint into a soft cleanup.

Next Steps

1. Add the deprecation alias in audit.model.js (one line).
2. Decide on the mapper-registry approach and document in the PR description.

[dzehnder]

Important: removing SEMANTIC_VALUE_VISIBILITY is a breaking change to the published AUDIT_TYPES enum.

No symbolic consumer was found across adobe/* (consumers use the string literal directly), but AUDIT_TYPES is part of the @adobe/spacecat-shared-data-access contract. Since this PR lands first in the 4-PR cross-repo sequence, the lower-risk path is to keep both keys for one minor cycle:

IMAGE_ENRICHMENT: 'image-enrichment',
/** @deprecated use IMAGE_ENRICHMENT - remove after mystique PR 1704 + audit-worker PR 2447 in prod */
SEMANTIC_VALUE_VISIBILITY: 'image-enrichment',

Delete the alias in a follow-up PR once the producer side is fully cut over.

[github-actions]

This PR will trigger a minor release when merged.

[yevheniia-severinovska]

Addressed in c87909f: kept SEMANTIC_VALUE_VISIBILITY enum key as deprecated alias mapping to 'image-enrichment'. Skipping mapper-registry double-key — null-mapper return is documented behavior, in-flight messages no-op rather than crash. Will remove the alias in a follow-up PR after mystique #1704 + audit-worker #2447 reach prod.

[yevheniia-severinovska]

Round 2 — addressed all in-scope items:

• ✅ SEMANTIC_VALUE_VISIBILITY enum kept as deprecated alias (commit c87909f)
• ✅ Mapper-registry registers ImageEnrichmentMapper under semantic-value-visibility alias too (commit 2bccd5c)

Both aliases will be removed in a follow-up PR after mystique #1704 + audit-worker #2447 reach prod. Skipped README legacy-values note (no AUDIT_TYPES section currently exists in data-access README; happy to add a new section if desired).

Re-requesting review.

[dzehnder]

Hey @yevheniia-severinovska,

Both prior findings are cleanly addressed. Ready to merge.

Strengths

• Prior Important resolved: audit.model.js:99 keeps SEMANTIC_VALUE_VISIBILITY as a deprecated alias mapping to 'image-enrichment', with a @deprecated comment naming the removal trigger ("after producer cutover"). Test mirror at audit.model.test.js:224 matches exactly. Producers using the enum constant transparently emit the new value.
• Prior Minor resolved: mapper-registry.js:55-58 registers the same ImageEnrichmentMapper instance under both 'image-enrichment' and 'semantic-value-visibility' keys, with a comment naming the cutover dependency. Symmetric resolution: enum-key consumers and string-literal producers both land on the new mapper.
• Cleanup story is concrete: both aliases are time-boxed via comments naming the dependent PRs. Co-locating the removal trigger with the code that has to change is the right pattern - someone closing those PRs has a grep target.

Recommendations

• The mapper-registry alias copies the same instance under two keys. With the current BaseOpportunityMapper shape (stateless, log-only), this is fine. A one-line note in the deprecated comment - "alias relies on mapper instances being stateless" - would protect a future contributor who adds per-key state from being surprised by shared mutation. Not blocking.
• Before removing the aliases in the follow-up cleanup PR, a quick Coralogix check on the semantic-value-visibility literal in the days leading up to removal would catch any unexpected legacy producers (archived configs, replay queues). Cheap insurance for the cleanup PR, not this one.

Assessment

Ready to merge? Yes.
Reasoning: The 9-line incremental diff does exactly what the deprecation strategy requires - dual-key aliases with named cutover triggers, and nothing more.