← all branches

feat/discovery

blockedviewing older commit
cc41079 · incrementalPR #305reviewed 2026-07-20 16:25 UTC5H · 5M · 3L
The branch
Purpose
Feature Discovery v1 for existing Batu platform users — contextual coachmarks, Descubre hub checklist, first-run guided tour. Flag-gated (feature-discovery-spotlights, OFF by default).
Goal
Land ambient spotlights + Descubre hub + first-run linear tour with FCIS backend, RLS-scoped DB table, and PostHog analytics. Nothing reaches users until the flag is manually flipped.
Sub-goals
  • SG-1: Ambient spotlight — server-decided, paced ≤1/rolling-day, retire-on-dismiss
  • SG-2: Descubre hub — header badge, gap checklist, ?descubre deep-link pull flow
  • SG-3: First-run linear product tour — 5 steps, guided walkthrough, Contratos page
  • SG-4: FCIS backend — DiscoveryFCIS namespace, discovery_state table (migration 0062), JSend API handlers
  • SG-5: i18n + analytics — es/en coachmark/tour copy, PostHog events, E2E anchor smoke
The changes (whole branch)
What
FeatureHighlight.tsx deleted (was a navigate+pulse component). Pull behavior (?descubre=<key>) absorbed into FeatureSpotlight as a 'pulled' branch that renders the full coachmark card (title/body/CTA) purely from the URL param — no server query, no eligibility latch. This fixes the old pull→pull jam where both cards vanished and nothing remounted.
Why
The navigate+pulse approach (FeatureHighlight) was a stepping stone that lacked copy and CTA. The full coachmark card is the intended UX; rendering it param-pure (no server round-trip) is what makes pull→pull a clean remount instead of a race.
Areas
apps/platform+2438−52domains/core+498−0domains/cross-domain+1604−0packages/analytics+92−0packages/api+103−0packages/database+13485−1packages/secrets+152−12e2e/platform+61−0
Blast
65 files, +18474/-65 net across the branch. Entirely additive: one new DB table, new API contract, new client components. No changes to existing domain entities or API contracts.
flag-gated: feature-discovery-spotlights (OFF by default, deploy precondition) migration 0062 discovery_state required 2-row organizations.segment reclassify script (manual, not auto-run)
➖ci· gh pr checks requires elevated token➖coderabbit· no .coderabbit.yaml in repo✅typecheck· 7 files changed, all in apps/platform + packages/analytics; layout.tsx import removed cleanly, no dangling references found

Findings · 13

correctness3

high

Pull coachmark calls report('seen') unconditionally — burns ambient 20h budget

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

The anchor-locate effect in SpotlightCoachmark calls report('seen') on every successful anchor find, regardless of the `pulled` prop. The cross-domain spec (feature-discovery.decisions.ts:422) explicitly marks pulls as budget- AND retirement-exempt. The old FeatureHighlight made zero recordOutcome calls. Fix: guard with `if (!pulled) { report('seen'); }` inside the locate effect.

medium

Pull coachmark increments retirement counter — permanently suppresses ambient for power hub users

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

Same root cause as the budget bug. RETIRE_AFTER_UNCLICKED_SHOWS=3 means a user who clicks the same hub row three times without hitting CTA permanently retires that key from the ambient schedule. Spec lines 303, 371 confirm pull is retirement-exempt. Same one-line fix as above.

low

Comment claims pull overrides tour gate, but code suppresses pull when tour is active

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

`if (tourActive) return null` at line 135 executes before the pull branch at line 141, so an active tour also suppresses pull cards. The comment at line 140 states 'PULL takes precedence... over the tour gate' — this is misleading. Either update the comment or move the pull branch above the tourActive guard.

conventions1

low

Stale test file comment names deleted FeatureHighlight

apps/platform/src/components/discovery/__tests__/highlight-key.test.ts:5

Covered above in tests lens. One-line comment fix: replace 'FeatureHighlight only ever pulses' with accurate description of the pull branch in FeatureSpotlight.

tests6

high

No test: pull branch renders SpotlightCoachmark when gate open + valid ?descubre key

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

The primary new contract (FeatureSpotlight renders SpotlightCoachmark with pulled=true for ?descubre=<validKey> when enabled) is entirely untested. Three conditions guard this branch (pulledKey && enabled && page && orgId) — each a distinct silent failure mode.

high

No test: pull branch suppressed when gate is closed (flag off / mobile / no org)

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

A bookmarked or shared ?descubre URL must be suppressed on mobile / without the flag. No test verifies FeatureSpotlight correctly skips the pull branch when enabled=false.

high

No test: pulled=true dismiss calls router.replace to strip ?descubre param

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

stripPull() is the mechanism preventing a reload from re-opening the dismissed card. If router.replace is not called on dismiss, the card reappears on reload — a UX regression. The useCallback dep array also has a stale-closure risk (pathname). Testable with a mocked router.

high

No test: pulled=true CTA click calls router.replace to strip ?descubre param

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

Same as dismiss gap — stripPull must fire on CTA click across all SPOTLIGHT_CTA kinds (navigate, acknowledge, click-anchor). Untested.

medium

No component test: invalid ?descubre falls through to ambient path

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

resolveHighlightKey is well-unit-tested. But the downstream: FeatureSpotlight correctly falls through to ambient when pulledKey is null — is not tested at component level.

medium

Stale test comment references deleted FeatureHighlight and claims no render tests needed

apps/platform/src/components/discovery/__tests__/highlight-key.test.ts:8

'FeatureHighlight only ever pulses a control for a key this function blesses... The rest of the cue is a self-clearing effect with no branching worth a render test.' Both claims are now false: FeatureHighlight is deleted and the pull branch (stripPull, gate check) does have branches worth testing.

improvement3

medium

No origin tag on discovery.spotlight_shown — can't distinguish ambient vs pull in analytics

packages/analytics/src/events.ts

Removing DiscoveryHighlightShownEvent merged pull and ambient into the same event. Adding origin:'pull'|'ambient' to SpotlightEventProperties (or DiscoverySpotlightShownEvent specifically) restores the signal needed to measure hub adoption rates separately from passive discovery. One-line change in events.ts + one prop in the track() call.

medium

Double useSearchParams subscription in parent + child — minor redundancy

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

FeatureSpotlight reads useSearchParams() to extract pulledKey (line 76); SpotlightCoachmark reads it again to build the stripped URL in stripPull (line 185). React dedupes the subscription, so no perf cost. Alternative: use window.location.search in stripPull to eliminate the second hook call entirely.

low

stripPull is a no-op on ambient path — minor asymmetry

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

Both onDismiss and onCta call stripPull() which immediately returns when pulled=false. Alternative: inline the stripPull call only in the pulled branch. Acceptable as-is.

History · 36 commits

  1. c1fe7b7needs attentionincremental3H · 4M · 3L2026-07-27 06:02
  2. 414db56safeincremental0H · 0M · 2L2026-07-23 00:52
  3. ea68968needs attentionincremental0H · 4M · 7L2026-07-22 23:01
  4. 4bdb2c9needs attentionincremental1H · 3M · 5L2026-07-21 23:22
  5. 6e3f255safeincremental0H · 0M · 0L2026-07-20 16:46
  6. cc41079blockedincremental5H · 5M · 3L2026-07-20 16:25current
  7. 2451ee0needs attentionincremental0H · 5M · 6L2026-07-20 16:09
  8. ab7c103needs attentionincremental0H · 1M · 6L2026-07-20 15:52
  9. 7eff611needs attentionincremental0H · 1M · 2L2026-07-18 00:41
  10. 246c67csafeincremental0H · 0M · 0L2026-07-17 23:58
  11. 7521b17safeincremental0H · 0M · 0L2026-07-17 23:35
  12. 1847ec8needs attentionincremental0H · 2M · 4L2026-07-17 23:28
  13. 5d3175cneeds attentionincremental3H · 6M · 4L2026-07-17 19:31
  14. d3f875dneeds attentionincremental1H · 2M · 4L2026-07-17 18:01
  15. 47d25faneeds attentionincremental1H · 1M · 2L2026-07-17 00:46
  16. 440d838needs attentionincremental4H · 9M · 9L2026-07-17 00:27
  17. 7466f97needs attentionincremental0H · 1M · 5L2026-07-16 23:20
  18. b686d58needs attentionincremental0H · 2M · 2L2026-07-16 14:24
  19. 9ea1446blockedincremental2H · 7M · 10L2026-07-16 13:44
  20. 6f39bb9needs attentionincremental0H · 2M · 7L2026-07-14 21:53
  21. 2eadf2aneeds attentionincremental0H · 1M · 2L2026-07-14 20:12
  22. 97bd08fneeds attentionincremental0H · 2M · 5L2026-07-14 19:40
  23. a48e3ffneeds attentionincremental5H · 8M · 6L2026-07-14 18:22
  24. 69473b7needs attentionincremental2H · 5M · 3L2026-07-14 01:34
  25. 92e4235needs attentionincremental2H · 1M · 3L2026-07-14 01:04
  26. 6be8018safeincremental0H · 0M · 3L2026-07-14 00:15
  27. 090b4daneeds attentionincremental1H · 3M · 5L2026-07-13 23:50
  28. fa7683bneeds attentionincremental1H · 4M · 3L2026-07-13 18:55
  29. d64cd72needs attentionincremental2H · 3M · 2L2026-07-13 16:27
  30. c1f337bneeds attentionincremental3H · 7M · 14L2026-07-13 13:30
  31. 46e32b2safeincremental0H · 0M · 3L2026-07-11 02:54
  32. 3b30949needs attentionincremental0H · 2M · 3L2026-07-11 02:39
  33. 9f8dbdfneeds attentionincremental3H · 5M · 5L2026-07-11 02:32
  34. 23191eeneeds attentionincremental5H · 12M · 7L2026-07-11 02:18
  35. fa9b88fneeds attentionincremental0H · 1M · 2L2026-07-11 01:30
  36. 6a8bf42blockedfull8H · 11M · 8L2026-07-11 01:08