feat/discovery

shouldHoldAmbientForTour resets on every tour re-engagement — rolling, not one-way

domains/cross-domain/src/feature-discovery.decisions.ts:250

Every time the user relaunches the tour via the hub, lastSeenAt updates and the 20h cooldown resets. A user who relaunches the tour daily would never see the ambient coachmark on the contratos page. The old isFirstRunTourPending was one-way (false forever after first engagement); the new function is a rolling window. This may be intentional (don't surface ambient during active tour sessions), but it is undocumented — the comment says 'surfaces on a later visit' without mentioning that repeated relaunch extends the hold indefinitely.

inTourFlow guard may suppress the 'Siguiente paso' toast on bookmarked TOUR_RETURN_PARAM URLs

apps/platform/src/components/discovery/DescubreMenu.tsx:110

The double-toast guard reads searchParams.has(TOUR_RETURN_PARAM). A user who navigates to the CFE credentials page mid-tour and bookmarks that URL would have TOUR_RETURN_PARAM in the URL on a fresh load with no active tour — the toast would be silently suppressed. Very narrow edge case (TOUR_RETURN_PARAM is meant to be consumed by the tour flow), and prevRef logic further limits the scope, but the guard is slightly wider than 'is a tour actually running'.

shouldHoldAmbientForTour not named decide* per canonical naming

domains/cross-domain/src/feature-discovery.decisions.ts:250

Pure decision functions in the .decisions.ts layer should follow the decide{Operation} naming convention (canonical-form.md). The new function is named shouldHoldAmbientForTour; per convention it should be decideHoldAmbientForTour. Sibling isFirstRunTourPending has the same deviation and predates this change — but that does not justify extending the pattern. Low-risk (purely naming), but the canonical form should be respected for new additions.

Wire rename firstRunTourPending → holdAmbientForTour — all refs updated but no grep record in PR

packages/api/src/schemas/discovery.schemas.ts:75

canonical-form.md requires a workspace grep before renaming wire-level fields. All references are correctly updated (handler, component, shell, schema) with no orphans. The concern is process: the PR description does not document that a grep was performed. This is a process note, not a live bug — flagging so the convention is explicitly followed next time.

Shell integration test does not assert holdAmbientForTour output

domains/cross-domain/src/__tests__/feature-discovery.integration.test.ts

The five getDiscoverySpotlightShell integration tests assert spotlightKey but none assert holdAmbientForTour. The shared-now fix (time-consistency between decideSpotlight and shouldHoldAmbientForTour) is validated only by unit tests. A targeted test (profile with product_tour exposure within/outside cooldown) would pin the shell wiring and catch future regressions at the injection boundary.

shouldHoldAmbientForTour exact boundary (== cooldownHours) not covered

domains/cross-domain/src/__tests__/feature-discovery.decisions.test.ts:529

The implementation uses strict less-than (< cooldownHours * 3600 * 1000), so at exactly AMBIENT_AFTER_TOUR_COOLDOWN_HOURS the result is false (released). Tests prove at +1h past but not at the exact boundary. Minor for a 20h window but the strict vs non-strict edge is undocumented by tests.

shouldHoldAmbientForTour and isFirstRunTourPending independently re-implement the 'never engaged' check

domains/cross-domain/src/feature-discovery.decisions.ts:253

Both functions search for the product_tour exposure and return true when none is found, but using different logic paths. If the 'never engaged' definition changes (e.g., a new field), only isFirstRunTourPending would be updated. shouldHoldAmbientForTour could call isFirstRunTourPending(exposures) in the no-tour-row branch to make the dependency explicit.

inTourFlow searchParams checks could use a shared TOUR_PARAMS array from tour-catalog

apps/platform/src/components/discovery/DescubreMenu.tsx:110

Three separate searchParams.has() calls on TOUR_PARAM / TOUR_RETURN_PARAM / SUBTOUR_PARAM. These are co-defined in tour-catalog.ts. Extracting TOUR_PARAMS = [TOUR_PARAM, TOUR_RETURN_PARAM, SUBTOUR_PARAM] as const there and using .some(p => searchParams.has(p)) would make exhaustiveness structural — if a fourth tour param is added to the catalog, this guard would be an easy miss otherwise.