test: rewards & progression coverage (#970)#1003
Merged
Merged
Conversation
…ai#970) Drives the rewards coverage WDIO specs introduced in this PR. The mock exposes mockBehavior knobs (rewardsScenario, rewardsServiceError, rewardsLastSyncedAt) so each E2E case can flip the snapshot returned by GET /rewards/me without restarting the server. Scenarios cover the matrix taxonomy: default, activity_unlocked (12.1.1), integration_unlocked (12.1.2), plan_unlocked (12.1.3), high_usage (12.2.1/12.2.2), post_restart (12.2.3). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) Adds 14 cases across the unlock taxonomy (activity / integration / plan) and the progress-tracking surface (message-count proxy via featuresUsedCount, usage metrics, persistence semantics) by exercising normalizeRewardsSnapshot — the de-facto reducer for the rewards domain in this codebase. There is no Redux rewardsSlice; snapshot lives in Rewards.tsx component state. The file is placed at the path the issue plan called out so the matrix audit trail is unambiguous, and the test prose documents the no-slice architectural reality. Covers matrix rows 12.1.1, 12.1.2, 12.1.3, 12.2.1, 12.2.2, 12.2.3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds three cases that drive the Rewards page through the activity-, integration-, and plan-based unlock flavours via mockBehavior scenario priming. Each case resets the mock state, primes the relevant scenario, re-mounts the page (hash router unmount-remount fires a fresh /rewards/me fetch), and asserts the rendered summary line + achievement copy match the server-authoritative snapshot. Mac2 skipped — same rationale as whatsapp-flow / slack-flow / insights: no Appium label mapping for the bottom-tab Rewards icon yet. Linux tauri-driver is the source of truth for this spec. Covers matrix rows 12.1.1, 12.1.2, 12.1.3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…umansai#970) Adds three cases for matrix rows 12.2.1, 12.2.2, 12.2.3: - 12.2.1 message-count tracking — primes high_usage and asserts the server-authoritative unlock count + per-card achievement titles. - 12.2.2 usage metrics — asserts the metrics footer renders the streak + cumulative-tokens values. - 12.2.3 state persistence across restart — primes high_usage with a pre-restart lastSyncedAt, captures the durable counters from the rendered DOM, navigates away, flips to post_restart with a later lastSyncedAt, navigates back, and confirms (a) durable counters survive unchanged, (b) the admin request log shows >=2 GET /rewards/me hits, proving the simulated restart re-fetched. Restart simulation uses page unmount-remount because tauri-driver does not support cheap full-app restarts; this is the realistic equivalent for the Rewards page which fetches once on mount. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) Promotes the six §12 leaves from missing to covered with VU+WD layer designations and per-row notes that document the unlock taxonomy branches the new tests exercise. Adds a section header callout for the rewards-domain reality (frontend-only; no Rust counterpart; no Redux slice) so future agents do not waste cycles searching src/openhuman/ for a non-existent rewards/ module — answer to the issue's open question. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds comprehensive test coverage for Rewards & Progression feature (12.x matrix) with unit tests validating reward snapshot normalization logic, two e2e specs testing activity/integration/plan-based unlock flows and persistence across app restart, and a mock API endpoint providing configurable reward scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/test/e2e/specs/rewards-unlock-flow.spec.ts (1)
111-208: Add backend-effect assertions per scenario (not UI-only).These cases verify rendered text but never confirm the scenario-triggering
/rewards/mefetch actually happened after each mock flip. Adding a per-test request-log delta check will prevent stale-state false positives and make failures easier to triage.Based on learnings: Applies to
app/test/e2e/specs/*.spec.ts: Assert both UI outcomes and backend/mock effects in E2E tests when relevant; add failure diagnostics (request logs,dumpAccessibilityTree()) for faster debugging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/test/e2e/specs/rewards-unlock-flow.spec.ts` around lines 111 - 208, Tests setMockBehavior/resetMockBehavior then assert UI only, but never verify the scenario-triggering /rewards/me fetch occurred; update each spec (e.g., the tests using resetMockBehavior(), setMockBehavior(), waitForRewardsSnapshot()) to capture the request-log snapshot before flipping the mock, then after navigateToRewards()/waitForRewardsSnapshot() assert a new request for "/rewards/me" (or the mock endpoint) appears; on assertion failure also call diagnostic helpers like dumpAccessibilityTree() and include recent network/request logs to aid triage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/test/e2e/specs/rewards-progression-persistence.spec.ts`:
- Around line 210-220: The current assertion uses the global request log and can
be satisfied by earlier tests; change to a baseline+delta check by calling the
same browser.execute snippet twice: once before the restart/remount to capture
initialRewardsRequestCount and again after the restart to capture
rewardsRequestCount, then assert that (rewardsRequestCount -
initialRewardsRequestCount) is at least the expected number of new requests
(e.g., >= 1 or >= 2). Update the stepLog calls and the expect to use these two
variables so the test asserts the delta caused by this test's remount rather
than the cumulative log.
---
Nitpick comments:
In `@app/test/e2e/specs/rewards-unlock-flow.spec.ts`:
- Around line 111-208: Tests setMockBehavior/resetMockBehavior then assert UI
only, but never verify the scenario-triggering /rewards/me fetch occurred;
update each spec (e.g., the tests using resetMockBehavior(), setMockBehavior(),
waitForRewardsSnapshot()) to capture the request-log snapshot before flipping
the mock, then after navigateToRewards()/waitForRewardsSnapshot() assert a new
request for "/rewards/me" (or the mock endpoint) appears; on assertion failure
also call diagnostic helpers like dumpAccessibilityTree() and include recent
network/request logs to aid triage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68d31c59-c502-487f-bec1-2f1879427378
📒 Files selected for processing (5)
app/src/store/__tests__/rewardsSlice.test.tsapp/test/e2e/specs/rewards-progression-persistence.spec.tsapp/test/e2e/specs/rewards-unlock-flow.spec.tsdocs/TEST-COVERAGE-MATRIX.mdscripts/mock-api-core.mjs
Comment on lines
+210
to
+220
| const rewardsRequestCount = await browser.execute(async () => { | ||
| const apiBase = | ||
| (window as unknown as { __OPENHUMAN_API_BASE__?: string }).__OPENHUMAN_API_BASE__ ?? | ||
| 'http://127.0.0.1:18473'; | ||
| const res = await fetch(`${apiBase}/__admin/requests`); | ||
| const json = (await res.json()) as { data?: Array<{ method: string; url: string }> }; | ||
| const log = json.data ?? []; | ||
| return log.filter(r => r.method === 'GET' && /^\/rewards\/me/.test(r.url)).length; | ||
| }); | ||
| stepLog('rewards/me request count after restart simulation', { rewardsRequestCount }); | ||
| expect(rewardsRequestCount).toBeGreaterThanOrEqual(2); |
Contributor
There was a problem hiding this comment.
Request-count assertion is not isolated to this test run.
rewardsRequestCount >= 2 uses the global mock log since suite start, so earlier tests can satisfy it. This can falsely pass even if the remount re-fetch in this case regresses.
🔧 Proposed fix (baseline + delta assertion)
+async function getRewardsMeRequestCount(): Promise<number> {
+ return browser.execute(async () => {
+ const apiBase =
+ (window as unknown as { __OPENHUMAN_API_BASE__?: string }).__OPENHUMAN_API_BASE__ ??
+ 'http://127.0.0.1:18473';
+ const res = await fetch(`${apiBase}/__admin/requests`);
+ const json = (await res.json()) as { data?: Array<{ method: string; url: string }> };
+ const log = json.data ?? [];
+ return log.filter(r => r.method === 'GET' && /^\/rewards\/me/.test(r.url)).length;
+ });
+}
+
it('12.2.3 — state persists across a simulated restart (re-fetch on remount)', async () => {
+ const baselineRewardsRequestCount = await getRewardsMeRequestCount();
// Phase 1...
...
- const rewardsRequestCount = await browser.execute(async () => {
- ...
- });
+ const rewardsRequestCount = await getRewardsMeRequestCount();
stepLog('rewards/me request count after restart simulation', { rewardsRequestCount });
- expect(rewardsRequestCount).toBeGreaterThanOrEqual(2);
+ expect(rewardsRequestCount - baselineRewardsRequestCount).toBeGreaterThanOrEqual(2);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/test/e2e/specs/rewards-progression-persistence.spec.ts` around lines 210
- 220, The current assertion uses the global request log and can be satisfied by
earlier tests; change to a baseline+delta check by calling the same
browser.execute snippet twice: once before the restart/remount to capture
initialRewardsRequestCount and again after the restart to capture
rewardsRequestCount, then assert that (rewardsRequestCount -
initialRewardsRequestCount) is at least the expected number of new requests
(e.g., >= 1 or >= 2). Update the stepLog calls and the expect to use these two
variables so the test asserts the delta caused by this test's remount rather
than the cumulative log.
6 tasks
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
…#1003) Co-authored-by: Claude Opus 4.7 (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
2 participants
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.
Summary
rewards-unlock-flow.spec.ts+rewards-progression-persistence.spec.ts, 6 cases total) that drive the Rewards page through the full unlock taxonomy and a simulated app restart./rewards/memock endpoint inscripts/mock-api-core.mjswith mockBehavior knobs (rewardsScenario,rewardsServiceError,rewardsLastSyncedAt) so each E2E case can flip server state without restarting the mock.Problem
The test coverage matrix introduced in #773 marked all six §12 (Rewards & Progression) leaves missing, with the work tracked under #970:
The issue body also flagged an open question — does the rewards domain exist in Rust? Investigation answer: no. There is no
src/openhuman/rewards/module and no ReduxrewardsSliceinapp/src/store/; the snapshot is fetched per-mount viaapp/src/services/api/rewardsApi.tsand held inRewards.tsxcomponent state. Backend ownership lives intinyhumansai/backend(/rewards/me).Solution
Layer split (mirrors the matrix):
app/src/store/__tests__/rewardsSlice.test.ts, 14 cases): exercisesnormalizeRewardsSnapshot— the reducer-equivalent that turns the raw/rewards/mepayload into the canonical client snapshot every UI selector reads. Cases mirror the unlock taxonomy (activity / integration / plan) and the progress-tracking surface (message-count proxy viafeaturesUsedCount, usage metrics, persistence semantics including idempotent re-load and duplicate-id defense).app/test/e2e/specs/rewards-unlock-flow.spec.ts(3 cases — 12.1.1 / 12.1.2 / 12.1.3): primesmockBehavior.rewardsScenariofor each unlock branch, navigates to/rewards, and asserts the rendered summary line + per-card state matches.app/test/e2e/specs/rewards-progression-persistence.spec.ts(3 cases — 12.2.1 / 12.2.2 / 12.2.3): the persistence case primeshigh_usagewith a pre-restartlastSyncedAt, simulates a restart by unmounting + remounting the page (cheapest equivalent under tauri-driver), flips topost_restartwith a laterlastSyncedAt, and asserts both the durable counters survive AND the admin request log shows >=2 GET/rewards/mehits (proves the simulated restart actually re-fetched).Mock backend addition: the
/rewards/mehandler responds to six scenarios (default,activity_unlocked,integration_unlocked,plan_unlocked,high_usage,post_restart) plus arewardsServiceErrorfailure switch. Snapshot shape mirrors the existingRewardsSnapshottype so the same fixture set is usable by future component-level tests.Architectural note (also in the matrix and the Vitest header): the issue plan asked for
app/src/store/__tests__/rewardsSlice.test.ts. Rather than introduce a dead Redux slice purely to satisfy the path, the file lives at the requested path and exercises the de-facto rewards state layer. The test prose calls this out so reviewers can audit the choice without spelunking.Submission Checklist
app/src/store/__tests__/rewardsSlice.test.ts. Full suite: 952 passed (was 938).whatsapp-flow.spec.ts/slack-flow.spec.ts/insights-dashboard.spec.ts(introduced in test(foundation): coverage matrix + strategy + gap-fill batch (#773) #980).Impact
pnpm test:e2e:all:flows. Each new spec uses the existingtriggerAuthDeepLinkBypass+ onboarding helpers, so no new harness costs./rewards/meare unaffected. The matrix layout is unchanged for all rows except §12.Related
Rewardsbottom-tab icon, thesupportsExecuteScript()skip blocks in both new specs can be lifted (same pattern as the four §10 / §11 specs that already skip on Mac2).messageCountfield is exposed in the/rewards/mepayload, the proxy assertion in 12.2.1 should switch fromfeaturesUsedCountto that field directly.Summary by CodeRabbit
Tests
Documentation