feat: post-celebration Astra review moment + Android release tsconfig fix (#382) - #398
Conversation
…se tsconfig fix (#382) - new top-priority 'review' engagement-prompt kind; TrueSheet review moment post streak-milestone/level-up celebrations; inline ReviewReminderCard removed - rate-gating copy deleted; dismiss=120d snooze, accept=one-shot; en+pt-BR - prepare-release-workspace: keep tsconfig extends inside the project root (SDK 56 @expo/cli "Failed to collapse" crash in Android Release) - level-up overlay: remove Continue button on both platforms Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR #398 — post-celebration Astra review moment + Android release tsconfig fix
Scope: PR #398 in thomasluizon/orbit-ui-mobile (frontend only — no orbit-api changes)
Recommendation: APPROVE
Summary
Three bundled, well-scoped changes: (1) a new mobile-only Astra-branded "review moment" sheet that reuses the existing shared engagement-prompt arbitration (adds a top-priority 'review' kind), replacing the old inline ReviewReminderCard; (2) a targeted fix to prepare-release-workspace.js keeping the tsconfig extends chain inside the isolated release workspace to unblock the Android release build; (3) removal of the redundant "Continue" button from the level-up overlay on both web and mobile. Traced the eligibility/arbitration logic (arm-time + display-time re-check, once-per-key guard, 120-day snooze, one-shot accept) end-to-end and it is internally consistent; no dead code, type-safety, or security issues found in the diff.
Findings
Critical
None.
High
None.
Medium
None.
Low / Info
- Two new locale strings (
reviewMoment.streakBody/levelBodyin bothen.jsonandpt-BR.json) use em dashes, whichDESIGN.md:185bans in user-facing copy. Not posting as a blocking finding per the signal gate — it's a pure style nit and the codebase already has many pre-existing em-dash instances elsewhere in the same locale files, so this isn't a regression specific to this PR's rigor.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED — level-up overlay button removal is symmetric on web+mobile; the review-moment surface is mobile-only by design (no web equivalent to a native app-store review flow), which is a legitimate platform-adapter exception per root CLAUDE.md, and web's engagement slot already always returned reviewReminder: false |
| i18n-syncer | IN SYNC — all 7 reviewMoment.* keys present in both en.json/pt-BR.json with matching placeholders; no orphaned reviewPrompt.* callsites left after the key rename |
| contract-aligner | N/A — no orbit-api changes and no packages/shared/src/types/*/endpoints.ts changes; the shared engagement-prompt store changes are client-only persisted state, not a network contract |
| security-reviewer | N/A — no orbit-api code changed |
Validation
Skipped per CI instructions — Build / Unit Tests / SonarCloud run as separate required checks on this PR.
Deferred — N/A dimensions & files not verdicted
- Dimension 13 (Backend hard rules): N/A — no
orbit-apifiles in this diff. - Dimension 14 (FEATURES.md parity): N/A — this is an internal engagement-mechanic swap (review reminder card → review moment sheet), not a new headline feature; consistent with the existing convention where the analogous referral-prompt and milestone-share-prompt mechanics also aren't tracked as their own
FEATURES.mdrows. - Contract-aligner / security-reviewer dimensions: gated off because the diff touches neither
orbit-apinor the shared type/endpoint contract — not "unverifiable," genuinely out of scope for this diff. - Every other changed file (mobile stores/hooks/components, web overlay, shared engagement-prompt-store/i18n) received a verdict during the dimension walk.
What's good
- Eligibility is checked twice by design (arm time in
use-habits.ts/_layout.tsx, redisplay time inreview-moment-sheet.tsx), and the "once-per-key" + snooze + one-shot-accept guards correctly self-heal (an armed-but-now-ineligible prompt gets cleared on next render) without any observed race. - The Android release fix carries a real WHY comment with an upstream URL, exactly per the comment policy, and ships with a dedicated regression test (
prepare-release-workspace.test.ts) asserting the rewrittenextendschain. - Thorough test coverage added for the new eligibility predicate, the arbiter priority ordering, and the sheet's visibility/dismissal paths.
Recommendation
Approve as-is. No action required before merge.
…days Launch-week velocity: an engaged new user now qualifies at their day-2 level-up instead of day 5+. 2-day minimum keeps the one-shot-per-120-days ask away from first contact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Code Review: PR #398 — feat: post-celebration Astra review moment + Android release tsconfig fix (#382)
Recommendation: REQUEST CHANGES
Summary
This PR bundles three changes: (1) a new mobile-only "Astra review moment" bottom sheet replacing the old inline ReviewReminderCard with a post-celebration Play Store review prompt (streak milestones 7/14/30/100/365 + any level-up, eligibility floor lowered to 10 completions / 2 active days), (2) removal of the redundant "Continue" button from the level-up overlay on both platforms, (3) an Android release-build tsconfig resolution fix. The engineering is solid — eligibility is checked consistently at arm time and display time through one shared pure function, arbitration priority is correct, tests are thorough, and the old mechanism's dead code is fully removed. Two issues surfaced below (both verified directly against the diff), one High and one Medium.
Findings
Critical: None
High
[HIGH] FEATURES.md not updated for the new review-moment feature
- location:
FEATURES.md(repo root) - issue: This PR introduces a materially new, named user-facing mechanism — a mobile-only Astra-branded review-moment sheet with its own eligibility rules and its own arbitration priority in the shared engagement-prompt store — fully replacing
ReviewReminderCard.FEATURES.mddocuments comparable/adjacent mechanisms at the same granularity (Milestone sharingrow 109,Celebration overlaysrow 184) but gets no new row for this one. - risk: Readers of
FEATURES.md, and any downstream copy derived from it, have no way to discover Orbit now prompts for a Play Store review, or under what conditions/platform. - fix: Add a row (e.g. in "Social & Growth" or "Gamification & Rewards"): description, Gating = Free, Platform = Mobile-only, mirroring the
Milestone sharingrow shape.
Medium
[MEDIUM] Em dashes in shipped review-moment copy violate the DESIGN.md ban
- location:
packages/shared/src/i18n/en.json:247-248,packages/shared/src/i18n/pt-BR.json:247-248 - issue:
reviewMoment.streakBodyandreviewMoment.levelBodycontain a literal em dash in both locales — e.g."You showed up {count} days in a row — that's real momentum..."and the pt-BR equivalent ("...seguidos — isso é ritmo de verdade..."). - risk: DESIGN.md explicitly bans em dashes in user-facing copy (
DESIGN.md:185: "No em dashes in copy. Use a comma, period, or hyphen."); this ships directly in production strings shown to users. - fix: Replace with a period or comma in both locales, e.g. "You showed up {count} days in a row. That's real momentum."
Low / Info: None (nothing else met the signal-gate bar)
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED — level-up overlay button removal is behaviorally identical on both platforms (tap-anywhere/backdrop-click, 6s auto-dismiss, back/escape all preserved); engagement-slot changes are parallel; the review-moment sheet's mobile-only scope is a legitimate pre-existing platform-adapter exception (web's engagement slot never supported a review slot type, by architecture) |
| i18n-syncer | IN SYNC — reviewMoment.* keys exist identically in en.json and pt-BR.json with matching interpolation placeholders; old reviewPrompt.* keys and references are fully removed repo-wide |
| contract-aligner | N/A — diff touches no packages/shared/src/types/*, endpoints.ts, or orbit-api DTOs |
| security-reviewer | N/A — diff touches no orbit-api code |
Validation
Not run in this session — CI runs Build / Unit Tests / SonarCloud as separate required checks for this PR (skipped per workflow instructions). PR body self-reports: shared 1384 / mobile 929 / web 2105 tests passing, lint/typecheck clean across all 3 workspaces.
Deferred
- Contract drift / backend hard rules: N/A — no shared Zod types/endpoints or orbit-api DTO changes; the new
EngagementPromptKindunion member is client-only Zustand shape, never serialized to the API. orbit-api repo not checked out in this job — not verifiable here regardless.
What's good
- Eligibility logic (
isReviewMomentEligible) is a single pure function reused at both arm time and display time — no duplicated logic, no drift risk. trackCompletion()runs before the arm-time eligibility check so the triggering completion counts toward its own milestone.- Old mechanism (
ReviewReminderCard,reviewPrompt.*keys,reviewReminderslot) is cleanly and completely removed — zero dead references anywhere. - The Android tsconfig fix carries a proper WHY comment with an upstream URL and a dedicated regression test.
- Level-up overlay dismissal behavior (tap, back/escape, auto-dismiss) is preserved identically on both platforms with matching new tests.
Recommendation
Add the FEATURES.md row and fix the two em-dash copy strings (en + pt-BR) before merge — both are small, mechanical fixes, no architectural rework needed.
1 participant
Closes #382
What
Three bundled changes:
1. Post-celebration Astra review moment (Android)
'review'kind in the shared engagement-prompt arbitration (review 3 > milestone-share 2 > referral 1) — reuses the single armed slot, once-per-key guard, and shared 14-day cooldown (markEngagementPromptedburns it when the sheet shows).use-habitsonSuccess, aftertrackCompletionso completion fix recursive parent completion prompts #20 counts for the same moment) + any level-up (armed in_layoutbeside the referral arming). Eligibility floor (onboarding + 10 completions + 2 distinct active days + not accepted + snooze lapsed — lowered from the pre-existing 20/5 for launch-week review velocity, product decision on top of Review prompt upgrade — post-celebration Astra-branded review moment (Android) #382) checked at arm time AND display time viaisReviewMomentEligible.ReviewMomentSheet) mirroringMilestoneSharePrompt— waits for celebrations to fully settle (+500ms), personalized copy referencing the actual streak/level, en + pt-BR.ReviewReminderCarddeleted;reviewReminderremoved from the shared engagement-slot rotation on both platforms.expo-store-review+ Play URL fallback unchanged.2. Android Release build fix (run 28720033241)
:app:createBundleReleaseJsAndAssetsdied withInvariant Violation: Failed to collapse— SDK 56@expo/cliresolves the tsconfigextendschain through Metro's file map; in the isolated.release-mobileworkspace the lookup of../../tsconfig.base.jsonmisses on Linux and the node_modules fallback (an upward-traversing specifier) crashes metro-file-map. The Sentry-upload failure was downstream fallout (no bundle/sourcemap existed).Fix:
prepare-release-workspace.jsnow copiestsconfig.base.jsoninto the copiedapps/mobile/and rewritesextendsto./tsconfig.base.json, keeping resolution inside the Metro project root (the same lookup mechanism that findstsconfig.jsonitself). Verified locally in a faithful isolated workspace: the exact CI command (expo export:embed --platform android --dev false) produces a complete 15MB Hermes bundle (6375 modules, all@/+@orbit/sharedaliases resolving). The pre-fix crash is Linux-specific and does not reproduce on Windows; final confirmation is the next manualAndroid Releasedispatch after merge.3. Level-up overlay: Continue button removed (both platforms)
The pill was redundant (tap-anywhere, 6s auto-dismiss, and back/escape all already dismiss) and miscentered. Removed on web + mobile; dismissal behavior covered by updated tests.
Platform parity
reviewReminder: false).Validation
trial-banner.tsx)🤖 Generated with Claude Code