feat/discovery

Stale scroll/resize listeners when gate disables mid-pulse

apps/platform/src/components/discovery/FeatureHighlight.tsx:135

The scroll/resize effect depends on `[pulse !== null]`. If `enabled` flips false mid-pulse (e.g., user resizes below 1024px during the 2600ms ring), the locate effect's cleanup cancels `pulseTimer` before it fires, so `setPulse(null)` never runs. `pulse` stays non-null and the scroll/resize effect's listener keeps calling `setPulse(...)` on every scroll/resize until unmount. The render guard (`pulse.key !== key` where `key` is now null) suppresses the portal, so there's no visual glitch — but the listeners are leaked. Fix: also clear `pulse` in the locate effect's cleanup, or include `key` in the scroll/resize effect's deps.

?descubre param not stripped when gate disables mid-pulse

apps/platform/src/components/discovery/FeatureHighlight.tsx:113

When `enabled` flips false mid-pulse, the locate effect cleanup cancels `pulseTimer` — so `stripRef.current()` (inside the timer callback) is never called. The `?descubre` param lingers in the URL. If the user resizes back to desktop, `key` becomes non-null again and the pulse re-triggers — the exact retry behaviour the comment says to prevent. The param strip should also fire from the locate effect cleanup path.

Test PAGES array manually mirrors the SpotlightPage union

apps/platform/src/components/discovery/__tests__/spotlight-page.test.ts:14

TypeScript will catch a value that is NOT in the union, but not a member that is MISSING from the array. Derive from the catalog or use a Record keyed by SpotlightPage to make exhaustiveness compile-time-checkable.

GuidedTour/RpuSubTour still have inline gate logic — drift risk

apps/platform/src/components/discovery/GuidedTour.tsx

GuidedTour and RpuSubTour compute their own enabled flag (isPreviewDeployment() && isDesktop && !!orgId) rather than consuming useDiscoveryGate. If a new condition is added to the gate (e.g., a role check), those two surfaces will silently diverge and activate when they shouldn't. Follow-up: migrate them to useDiscoveryGate.

orgId correctly threaded through the refactor — no cross-org leakage

apps/platform/src/components/discovery/FeatureSpotlight.tsx

orgId from useDiscoveryGate is still present in the TanStack Query key and API call params in FeatureSpotlight. The refactor did not introduce any cross-org data leakage.

Boolean expression `[pulse !== null]` as effect dependency — intent unclear

apps/platform/src/components/discovery/FeatureHighlight.tsx:135

Passing a computed boolean to the deps array is non-idiomatic and the eslint-disable comment is too terse. When `pulse` transitions from one non-null Pulse to another (pull→pull), `pulse !== null` stays `true` — the effect does NOT re-subscribe. The correctness argument is that `anchorRef.current` is updated in the locate effect before the listener fires, so reads are correct. But this is a subtle invariant. A comment explaining the intent, or using `[pulse?.key]` as the dep (which would re-subscribe on key change and is self-documenting), would make this clearer.

useSyncExternalStore subscribe may return undefined — not aligned with sibling pattern

apps/platform/src/components/discovery/use-discovery-gate.ts:31

`posthog?.onFeatureFlags?.(() => onStoreChange())` returns `undefined` when posthog is null. The returned cleanup `() => unsubscribe?.()` is technically valid (no-op via optional chaining), but GuidedTour.tsx and RpuSubTour.tsx use the explicit `?? (() => {})` guard pattern to return a stable no-op. Align for consistency: `return unsubscribe ?? (() => {})`.

useEffect with no dependency array should have an explanatory comment

apps/platform/src/components/discovery/FeatureHighlight.tsx:68

The effect that updates `stripRef.current` and `pageRef.current` runs after every render intentionally (to keep refs current without adding to the locate-effect deps). A brief comment would prevent a future reader from 'fixing' it by adding deps and introducing stale closures.

useDiscoveryGate has no unit test

apps/platform/src/components/discovery/use-discovery-gate.ts

useDiscoveryGate is the single gate for both FeatureHighlight and FeatureSpotlight — if it drifts, both surfaces misfire silently. The boolean result `(flagEnabled || isPreviewDeployment()) && isDesktop && !!orgId` is the highest-value pure predicate added in this diff. Suggested: extract `computeGate(flagEnabled, isDesktop, orgId, isPreview)` as a pure function and test all 8 on/off combinations; leave the hook as thin useSyncExternalStore wiring.

pull→pull guard (pulse.key !== key) has no render test

apps/platform/src/components/discovery/FeatureHighlight.tsx:139

The key correctness invariant — a DOMRect measured for a superseded key never renders the ring at the wrong control — is enforced by a single line. It motivated the entire Pulse struct rewrite. A focused render test (jsdom + RTL, mocking useDiscoveryGate + useSearchParams) simulating a pull→pull key change would pin this regression path. Without it, a future refactor of the useState/useEffect interplay could silently re-introduce the pull→pull flicker.

PAGES array in spotlight-page.test.ts is not exhaustive — new pages silently skipped

apps/platform/src/components/discovery/__tests__/spotlight-page.test.ts:15

The array is typed `SpotlightPage[]` but TS won't catch a missing union member — only a wrong one. If a fourth SpotlightPage is added, the round-trip test silently skips it. Use `as const satisfies readonly SpotlightPage[]` or key by a Record to enforce exhaustiveness at compile time.

pathForPage silent fallback to '/bills/contratos' is untested

apps/platform/src/components/discovery/__tests__/spotlight-page.test.ts

`pathForPage` falls back to `'/bills/contratos'` for unknown keys. The current type signature makes it hard to pass an unknown value, but the fallback exists. A test making the contract explicit (or asserting it throws) prevents silent wrong-navigation if the fallback is ever changed.

matchMedia creates two separate objects in subscribe vs getSnapshot

apps/platform/src/components/discovery/use-discovery-gate.ts:44

The `subscribe` callback calls `window.matchMedia(DESKTOP_QUERY)` and attaches a listener to it. `getSnapshot` calls `window.matchMedia(DESKTOP_QUERY)` again, creating a second object. Modern browsers return the same underlying `MediaQueryList` for identical query strings, but this relies on implementation detail. The standard pattern is to hoist to module scope: `const desktopMq = typeof window !== 'undefined' ? window.matchMedia(DESKTOP_QUERY) : null` so both share the same object and the SSR guard is written once.

isPreviewDeployment() called on every render — hoist to module scope

apps/platform/src/components/discovery/use-discovery-gate.ts:56

`isPreviewDeployment()` reads `process.env.NEXT_PUBLIC_VERCEL_ENV`, which is inlined at build time — it's effectively a constant after compilation. Hoisting it as `const IS_PREVIEW = isPreviewDeployment()` at module scope makes the build-time nature explicit and avoids the trivial function-call overhead on every render of every mounted gate consumer.

Two matchMedia subscriptions when both surfaces are mounted simultaneously

apps/platform/src/components/discovery/use-discovery-gate.ts:44

FeatureSpotlight and FeatureHighlight both mount useDiscoveryGate in the dashboard layout, creating two separate matchMedia subscriptions for the same `(min-width: 1024px)` query. A module-level singleton subscribe/getSnapshot (the standard useSyncExternalStore pattern) would deduplicate. Negligible for two callers, but worth noting as discovery surfaces grow.