Repository navigation
fix(gamification): show streak to free users (read from profile, not Pro-gated endpoint) - #248
Conversation
…Pro-gated endpoint) The home header flame chip and the streak page's "Maior" (longest) sourced the streak from useStreakInfo -> GET /api/gamification/streak, which is Pro-gated (and client-disabled for free users). Free users always saw 0, even with a real streak. Current streak was already free via /api/profile; longest was not exposed there. - Header reads profile.currentStreak; remove the Pro-gated useStreakInfo from the Today screen (web + mobile). - Streak page "Maior" reads profile.longestStreak (web + mobile). - Add longestStreak to the shared profile schema + test factory. - Update web profile/streak tests for the new field + source. Partial pull-forward of #186/#196 (streak is free per the upgrade matrix); the broader freeze/XP rebalance remains in those issues. Paired with orbit-api fix/streak-free-tier (ProfileResponse.LongestStreak). 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
|
There was a problem hiding this comment.
Code Review: PR #248
Scope: PR #248 — fix(gamification): show streak to free users (read from profile, not Pro-gated endpoint)
Recommendation: NEEDS WORK
Summary
This PR correctly fixes the root cause: both the Today header and the Streak page's "Maior" (longest) card were reading from the Pro-gated GET /api/gamification/streak endpoint, causing free users to see 0. The fix moves both reads to profile.currentStreak / profile.longestStreak (sourced from the ungated GET /api/profile), which is the right architectural decision. The shared type addition and test updates are clean. However, the web Profile page still reads its streak display from the Pro-gated useStreakInfo hook — the exact same bug just fixed on the Today screen — and the new longestStreak field is required/non-nullable in the Zod schema against an API whose paired PR may not be deployed yet.
Findings
High
[HIGH] Web Profile page still reads currentStreak from the Pro-gated hook — same bug, missed surface
- dimension: Correctness (#1) + Parity (#9)
- location:
apps/web/app/(app)/profile/page.tsx:43-44 - issue:
profile/page.tsxstill hasconst { data: streakInfo } = useStreakInfo(profile?.hasProAccess ?? false)andconst streak = streakInfo?.currentStreak ?? 0. A free user's streak shows as0in the Profile header bar and stat tiles — the same regression this PR fixes on Today and Streak. Mobile'sapps/mobile/app/(tabs)/profile.tsx:207already readsconst streak = profile?.currentStreak ?? 0(no Pro gate), so web is behind mobile. - risk: Free users continue to see
0on the Profile screen despite the fix. The PR's stated goal is only partially achieved. Mobile profile shows the correct value; web profile shows0for free users. - fix: In
apps/web/app/(app)/profile/page.tsx, remove theuseStreakInfocall and replacestreakwithprofile?.currentStreak ?? 0, mirroring mobile line 207. - reference: CLAUDE.md rule 1 (root cause over workarounds), CLAUDE.md cross-platform parity (MANDATORY)
[HIGH] longestStreak added as required (non-nullable) before the API deploy is confirmed
- dimension: Contract drift + backward-compat guard (#11)
- location:
packages/shared/src/types/profile.ts:47 - issue:
longestStreak: z.number()is required and non-nullable. The PR body says the client "falls back to0gracefully until then", but that is not what happens. Zod parse ofProfileResponsewill throw (ZodError) iflongestStreakis absent from the server response. The?? 0at call sites only runs whenprofileitself is nullish — not when the Zod parse fails. AnyuseProfilecall hitting a pre-orbit-api#213 API build will fail to parse the profile response entirely — not just show0for streak. - risk: Until orbit-api#213 is live, all platform clients lose their profile data on every page that calls
useProfile. This is a silent breakage window between frontend and API deploy. - fix: Use
longestStreak: z.number().optional().default(0)to make the Zod parse tolerant until the API ships the field, then tighten it once orbit-api#213 is confirmed deployed. - reference: CLAUDE.md "Backward compatibility (append-only + deploy-order)"
Medium
[MEDIUM] useStreakInfo export on web has reduced caller surface — candidate for un-export
- dimension: Dead / stale code (#2)
- location:
apps/web/hooks/use-gamification.ts:90 - issue: After applying the High fix to
profile/page.tsx, the only remaining caller of the exporteduseStreakInfowill be insideuseStreakFreezeinternally. The export would no longer be needed from outside the file. - fix: After the High fix, grep for direct
useStreakInfoimports outsideuse-gamification.ts; if none, remove theexportkeyword and delete the now-orphaned test imports. - reference: CLAUDE.md rule 2 (delete unused code immediately)
[MEDIUM] No test coverage for the free-user streak case on the web Profile page
- dimension: Correctness (#1)
- location:
apps/web/__tests__/app/profile-page.test.tsx:46 - issue: The profile-page test mocks
useStreakInfobut never asserts that a free user (hasProAccess: false) sees a real streak value. The regression went undetected here. - fix: Add a test case: render with
profile.hasProAccess = false, profile.currentStreak = 5and assert the streak tile shows5. - reference: CLAUDE.md "Every new feature needs tests. Tests asserting behavior, not implementation details."
Low / Info
[INFO] longestStreak field naming and type convention match the two existing sibling fields in gamification types — consistent with gamificationProfileSchema and streakInfoSchema. No action required.
[INFO] Deploy ordering intent is correctly documented in the PR body — the concern raised in the High finding is about Zod strictness, not missing awareness.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PARTIAL — Today + Streak screens are PAIRED; web Profile page is behind mobile |
| i18n-syncer | IN SYNC — no new user-facing strings; all keys present in both locales |
| contract-aligner | MATCH (conditional) — Zod convention aligns with established pattern; contingent on orbit-api#213 delivering public int LongestStreak { get; init; } on ProfileResponse |
| security-reviewer | N/A — no orbit-api changes in this diff |
Validation
| Check | Result | Details |
|---|---|---|
| Lint | N/A | PR already merged; CI green per PR body |
| Type check | N/A | PR body: "type-check clean" |
| Tests | N/A | PR body: shared 990 · web 90 · mobile 20 — all passing |
| Build (api) | N/A | No orbit-api changes in this diff |
What's good
- Root cause correctly identified and fixed at the right level — removing the Pro gate rather than patching around it.
- Today header and Streak page updated symmetrically on both platforms.
longestStreakadded to the right schema (profileSchema) so it flows through the existinguseProfilepath with no new hook.- Test factory and inline test fixtures updated everywhere the profile object is constructed, keeping tests green.
- PR body is honest about the deploy dependency and the known stale-streak limitation for free users.
- Mobile profile was already reading from
profile.currentStreakdirectly — the PR validates that pattern.
Recommendation
Two items before this fix is complete:
- Fix the web Profile page (
apps/web/app/(app)/profile/page.tsx:43-44): replaceuseStreakInfowithprofile?.currentStreak ?? 0and add a free-user test case. - Make
longestStreakparse-tolerant (z.number().optional().default(0)) until orbit-api#213 is confirmed deployed, or confirm the API was deployed first and the breakage window has already closed.
🤖 Generated with Claude Code
…o claude-review.yml - WORKFLOW.md: add /execute as the driven shortcut for the single-slice path (mirrors how /feature is presented for the medium/large path). - claude-review.yml: extend the contract category into a backward-compat guard that flags removed/renamed/newly-required fields breaking already-shipped mobile clients (mirrors /pr-review Phase 5). Relates to #226. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Missed surface in the streak free-tier fix: apps/web/app/(app)/profile/page.tsx still read the streak from the Pro-gated useStreakInfo, so free users saw 0 on the Profile screen (mobile already reads profile.currentStreak). Source it from profile.currentStreak to match, plus a free-user regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — addressed in
Side note: this review posted as a comment rather than approve/request-changes because it ran via the |
…ps nits (#226) - claude-review.yml: replace the duplicated inline rubric with a thin wrapper that runs the /pr-review skill (single source of truth) and posts exactly one decisive review -- --approve or --request-changes, never a bare comment. Skips Phase 6 /validate and the cross-repo checks that need the un-checked-out orbit-api. - rubric.md: add a "Signal gate" -- high-signal only (Critical/High/actionable Medium); never post style nits; outcome is deterministic (NEEDS WORK iff any Critical/High survives). - SKILL.md: reference the gate in Phase 3; Phase 7 posting is now decisive and caller-aware (CI wrapper posts the decision; local self-review falls back to --comment since GitHub blocks self-approval). Implements #226. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|



What
A free user with a real streak saw
0in the Today header flame, the streak page's "Maior" (longest), and the Profile page. Root cause: those surfaces read the streak fromuseStreakInfo→GET /api/gamification/streak, which is Pro-gated (if (!user.HasProAccess) → 403) and client-disabled for free users (enabled: hasProAccess), so they fell back to?? 0. "Atual" was already correct because it reads the ungated/api/profile.Fix (web + mobile, parity)
profile.currentStreak; removed the Pro-gateduseStreakInfo.profile.longestStreak.longestStreakto the sharedprofileSchema+ test factory.Why
Per #186 / #196, streak (current + longest) is a free feature; only the freeze system stays Pro. This is the "streak numbers free" slice — the broader freeze/XP rebalance + upgrade matrix remain in #186/#196.
Tests
shared 990 · web 92 (streak/today/profile/profile-page) · mobile 20 (today screen) · type-check clean.
Notes
ProfileResponse.LongestStreak(deploy API first; client falls back to0gracefully — clients don't Zod-parse the profile, theyJSON.parse … as T).0.Also bundled — CI review tooling (implements #226)
Separate commits, unrelated to the streak fix:
WORKFLOW.md: add/executeas the driven shortcut for the single-slice path (mirrors/feature).claude-review.ymlis now a thin decisive wrapper around the/pr-reviewskill — it runs the skill (single source of truth) and posts exactly one--approve/--request-changes, never a bare comment. The duplicated inline rubric is retired./pr-reviewskill hardened: a new Signal gate inrubric.md(high-signal only — no style nits; deterministicNEEDS WORKiff any Critical/High), referenced inSKILL.mdPhase 3, with decisive + caller-aware posting in Phase 7 (CI posts the decision; local self-review falls back to--commentsince GitHub blocks self-approval).Paired PR
Backend: thomasluizon/orbit-api#213
🤖 Generated with Claude Code