feat(upgrade): 3-column redesign + yearly-only matrix + retrospective-bug fix (#7) - #327
Merged
Conversation
… CTA (#7) Chooser = 3 cards (Free / Pro-Yearly hero / Pro-Monthly); feature list = one 2-col Free-vs-Pro matrix (kills the 3x repetition). New cell state pro:'yearly'|true|false fixes the retrospective bug (now checkmark+Yearly pill, was proEnabled:false). Matrix reflects #186 free-tier (streaks/XP/freeze free). Trial-state CTA: Subscribe to keep Pro / Upgrade to Pro (trial is a backend grant). Mobile: stacked cards + accordion matrix + sticky CTA. web+mobile, both locales. Co-Authored-By: Claude Opus 4.8 (1M context) <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 #327 — feat(upgrade): 3-column redesign + yearly-only matrix + retrospective-bug fix
Recommendation: APPROVE
Summary
A well-executed redesign of the upgrade/paywall screens on both web and mobile. The 3-column Free/Pro-Yearly hero/Pro-Monthly chooser replaces the old radio-card selector cleanly. The AI Retrospective retrospective-bug fix is correct: pro: 'yearly' (the new UpgradeProState union) is the right model — the old proEnabled: false while still listing it in UPGRADE_YEARLY_EXTRA_FEATURES was genuinely inconsistent. Convert-state messaging (trial vs free) lands on both platforms in parity. i18n and backward-compat gates passed cleanly.
Findings
Critical — None
High — None
Medium
[MEDIUM] Scratchpad audit file committed to the branch
- dimension: Dead / stale code (#2)
- location:
CUsersthomaAppDataLocalTempclaudeC--...scratchpadsoft_delete_audit.txt(tracked in this PR diff) - issue: A Windows temp-path scratchpad file (
soft_delete_audit.txt, 70 lines of internal audit notes) was accidentally committed. No runtime effect but pollutes the commit history. - risk: Ships into
mainand stays in git history unless explicitly removed. - fix:
git rm "CUsersthomaAppDataLocalTemp..."and add the Windows temp prefix to.gitignore. Can be done in a quick follow-up commit or the next squash. - reference: CLAUDE.md rule 2 — "Delete unused code immediately."
Low / Info
[LOW] any in the three new mobile test files
- dimension: Type safety (#6)
- location:
apps/mobile/__tests__/components/upgrade/plan-comparison-cards.test.tsx:11-12,17,27,pricing-footer.test.tsx:12,32-33,plan-selection.test.tsx:29 - issue:
let tree: anyand(node: any)inreact-test-renderertest helpers. Pre-existing pattern inplan-card.test.tsxtoo, but these are new files. - fix: Type
treeasimport type { ReactTestRenderer } from 'react-test-renderer'; narrow node props as{ children: unknown }. - reference: CLAUDE.md rule 3.
[INFO] freeHeading = "Your Pro trial ended" may mislead permanently-free users
- dimension: Correctness (#1)
- location:
packages/shared/src/i18n/en.jsonupgrade.convert.freeHeading, bothpricing-section.tsxfiles - issue:
trialActive ? trialHeading : freeHeading—freeHeadingfires wheneverisTrialActive === false, which includes users who never had a trial (trialEndsAt: null). The PR body states the trial is a backend signup grant (all users get one), so this is safe if that invariant holds server-side. - fix: No action if the backend always sets
trialEndsAtat signup. If that invariant might not hold, addprofile?.trialEndsAt ? freeHeading : genericHeadingas a guard. - reference: CLAUDE.md rule 1 — documents the server-side assumption.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED — all substantive mirrors updated; three accepted platform-adapter differences (PricingFooter mobile sticky bar, page-level test web-only, lucide mock mobile-only) |
| i18n-syncer | IN SYNC — 27+ new keys in both locales, all removed keys purged, zero orphaned callsites |
| contract-aligner | N/A — no API types or Zod wire-format schemas changed; upgrade.ts changes are display-layer only |
| security-reviewer | N/A — no orbit-api code touched |
Backward-compat guard
No ⚠️ breaks old mobile clients findings.
TRIAL_EXPIRED_FEATURE_KEYSremoval: was never consumed by either compiled app bundle (bothtrial-expired-modal.tsxhad inline local arrays). Zero runtime impact.freeEnabled→free,proEnabled→pro,iconKeymoved to category: TypeScript interface renames on a static in-memory data structure. No Zod schemas, no API wire format. Producer and consumer updated together in this PR. Old APK bundles carry their own compiled snapshot.
Validation
N/A — run commands require user approval in this CI session. PR body reports tsc + lint clean, 3338 tests passing (shared 1032 · web 1664 · mobile 642).
What's good
- Retrospective bug fix is correct:
UpgradeProState = 'yearly' | true | falsewith JSDoc is the right model. Both the data and every rendering path on web and mobile are updated together. iconKeyrefactor: moving from per-row to per-category is the right DRY lift — icons belong to the category, not individual rows.- i18n discipline: 27+ keys added to both locales in the same commit. The shared test validates this.
- Checkout extraction:
checkoutLoading/checkoutError/CTA out ofPricingSectionintoPricingFooter(mobile) /PlanCard(web) is a clean separation of concerns. - Test coverage: 5 new test files covering accordion expand/collapse, yearly-pill assertion, trial-vs-free CTA switching, and pricing footer states.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
PR Review — #327 feat(upgrade): 3-column redesign + yearly-only matrix + retrospective-bug fix
Decision: APPROVE
| Severity | Count |
|---|---|
| Critical (incl. |
0 |
| High | 0 |
| Medium | 0 |
| Low / Info | 1 |
Findings
[Low] Dead disabled prop on PricingFooter — apps/mobile/app/upgrade.tsx:237-248
PricingFooter is only rendered when isOnline is true (outer gate: !showBilling && plans && isOnline), so disabled={!isOnline} can never be truthy while the component is mounted. Zero behavioral impact — offline state is handled by unmounting the footer and showing OfflineUnavailableState instead. Either drop isOnline from the render condition to make the disabled state reachable, or remove the disabled prop if hiding the footer offline is the intent.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED — all mirrors present; mobile-only PricingFooter vs web per-card CTAs is a valid platform adapter |
| i18n-syncer | IN SYNC — all 23 new keys in both en.json and pt-BR.json; removed keys pruned from both |
| contract-aligner | N/A — no orbit-api changes; packages/shared/src/types/* untouched |
What's good
UpgradeProState = 'yearly' | true | falseis the correct root-cause fix for the retrospective display bug — explicit discriminant rather than inferring from a missingproEnabled.FreeCell/ProCelldecomposition (both platforms) cleanly gates onrow.type === 'text'before falling to boolean check/X rendering.- Tests target changed behaviors (accordion, yearly-only pill, trial vs. free CTA copy) — not implementation details.
- i18n is spotless: 23 new keys, all removed keys cleaned from both locales.
useTrialExpired/useTrialUrgentcorrectly removed from the upgrade page; they remain available toprofile/page.tsxandtrial-banner.tsx.
thomasluizon
added a commit
that referenced
this pull request
Jun 27, 2026
…340) #327 redesigned the upgrade paywall from a radiogroup plan-selector into the 3-column PricingSection (a PlanCard grid). The post-deploy prod smoke failed on the now-absent getByRole('radiogroup'). The 'paywall-checkout' testid still exists (the yearly hero card CTA), so drop only the obsolete radiogroup line. Prod is healthy — the other 6 smoke tests (auth, create/log habit, Astra) pass. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrade/paywall redesign (web + mobile) — research-backed
Fixes the retrospective-display bug and the "repeats itself too much" problem at once.
Structural fix (data model)
upgrade.tsmatrix cell gains a 3rd Pro statepro: 'yearly' | true | false. AI Retrospective is nowpro:'yearly'— renders ✓ + an inlineYearlypill instead of the oldproEnabled:falsebug (which showed Pro as not having it). The matrix is the single source of truth —proFeaturesshrunk to 4 marquee keys; the trial-expired 3rd copy removed.Layout (the repetition fix)
Chooser = 3 plan cards (Free · Pro-Yearly hero, center, raised+glow+badge+the only filled CTA · Pro-Monthly anchor). Feature list = ONE 2-column Free-vs-Pro matrix below — monthly & yearly are the same Pro minus retrospective, so the matrix never forks. Grouped categories, positive marking, value-text quantities, "Not included" text (a11y), sticky header.
Entitlements + trial
Matrix reflects the #186 free-tier rebalance (streaks, XP/level, streak-freeze = free; retrospective = yearly-only). Assumes
gamification_free_tieris enabled — flip the flag when the rebalance launches so the entitlement matches the matrix. Trial CTA is convert-not-start (the trial is a backend signup grant): "Subscribe to keep Pro" (active) / "Upgrade to Pro" (expired), with the honest "no card was taken" line. No countdowns.Mobile
Stacked selectable cards (yearly hero preselected), full matrix behind a "Compare all features" accordion, single sticky bottom CTA.
Validation
shared 1032 · web 1664 · mobile 642 · tsc + lint clean · i18n parity (1726 keys each).
🤖 Generated with Claude Code