feat(web,mobile,shared): FEATURES.md inventory + feature-guide restructure (#378) - #386
Conversation
…cture (#378) Add FEATURES.md at the repo root: one authoritative, code-derived, gating- and platform-aware inventory of every user-facing Orbit feature. Marketing-toned headline (Astra -> MCP -> social -> core tracker), then per-domain tables (Feature | Description | Gating | Platform | Locale notes). Counts verified against orbit-api source: 61 IAiTools, 94 McpServerTools across 15 classes. Restructure the in-app FeatureGuideDrawer 6 -> 9 tabs on both platforms (kept structurally identical): Astra, Connect, Social, Habits, Goals, Calendar, Rewards, Settings, Notifications. Rename chat -> astra and chatSection -> astraSection atomically across both components and both locales; surface the previously-absent MCP (Connect) and social layers and promote gamification/widget/referral/retrospective out of Settings into Rewards/Social. Correct gating copy to match PayGate: gamification is Free (not Pro), retrospective is Yearly-Pro. All onboarding.featureGuide.* keys authored in en.json and pt-BR.json (pt-BR fully translated); 163 keys each, identical sets. 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.
/pr-review — PR #386
feat(web,mobile,shared): FEATURES.md inventory + feature-guide restructure (#378)
Scope: FEATURES.md (new), apps/web/components/onboarding/feature-guide-drawer.tsx, apps/mobile/components/onboarding/feature-guide-drawer.tsx, apps/web/__tests__/.../feature-guide-drawer.test.tsx, packages/shared/src/i18n/{en,pt-BR}.json. Frontend/docs-only, no orbit-api changes.
Verdict: APPROVE
No Critical or High findings survive. Two Medium items below are non-blocking, worth a follow-up.
Findings
[MEDIUM] Missing mobile test coverage for a substantially reworked component
· dimension: 9 (Parity) / Testing (root CLAUDE.md)
· location: apps/mobile/components/onboarding/feature-guide-drawer.tsx
· issue: The drawer's tab structure changed 6→9 tabs (net-new astra, connect, social sections; chat removed) on both platforms. apps/web/__tests__/components/onboarding/feature-guide-drawer.test.tsx was updated with new assertions for the added tabs, but no equivalent test file exists for the mobile component (apps/mobile/__tests__/components/onboarding/feature-guide-drawer.test.tsx does not exist — pre-existing gap, not created by this PR, but this PR is the natural point to close it since it's the PR reworking the component's tab logic).
· risk: A future regression in the mobile tab-switching logic (e.g. activeSection state, sectionItems lookup) has zero automated coverage, while the identical web logic is covered by 15+ assertions.
· fix: Add a mobile RTL test mirroring the updated web suite (renders all 9 tabs, default section is astra, tab-click switches sections, aria-selected/active-chip equivalent).
· reference: CLAUDE.md Testing — "Every new feature needs tests"; root CLAUDE.md cross-platform parity mandate.
[MEDIUM] MCP tool-class row counts don't sum to the stated total
· dimension: 1 (Correctness)
· location: FEATURES.md:76-90 vs FEATURES.md:69
· issue: Line 69 and the PR description both state "94 [McpServerTool] methods across 15 tool classes." The 15 per-class rows in the table (23+12+8+8+8+6+5+4+4+3+3+3+2+2+2) sum to 93, not 94.
· risk: FEATURES.md is positioned as the single code-derived SoT that downstream docs (Play listing, landing page, pre-launch QA matrix) will copy numbers from verbatim — an internal arithmetic mismatch undermines that guarantee even though it's docs-only (no runtime/build impact).
· fix: Re-check the actual per-class method count in orbit-api/src/Orbit.Api/Mcp/Tools/*.cs and correct either the 94 total or the row that's undercounted by one.
· reference: PR body's own "Ground truth (verified against orbit-api source)" claim; not independently verifiable against orbit-api in this CI job (sibling repo not checked out) — flagging the internal sum mismatch only, which is verifiable from the diff alone.
Verified clean (no findings)
- i18n (dimension 10):
i18n-syncercross-check —en.json/pt-BR.jsonfully in sync foronboarding.featureGuide.*(142 leaf keys each),chat/chatSectioncleanly removed with zero orphaned callsites inapps/weborapps/mobile,widgetTitle/DescandretrospectiveTitle/Descconsistently relocated intorewardsSectionin both locale files, brand words (Orbit,Astra) correctly left untranslated in pt-BR. (Note: PR body claims 163 keys — the independently-counted 142 is the verified figure; informational only, doesn't affect merge.) - Parity (dimension 9) — component logic:
SectionKeyunion,tabsarray, andsectionItemsmap are structurally and behaviorally identical between web and mobile (same 9 sections, same key names, same ordering).FeatureGuideDrawerProps(onOpenChangeon web vsonCloseon mobile) differs, but that divergence pre-dates this PR (unchanged lines in both diffs) and is out of scope here. - Comment policy (dimension 4): No new comments in either component diff.
- Type safety (dimension 6): No
any/as any/as unknown as Xintroduced. - Dead code (dimension 2): Old
chat/chatSectionbranch fully removed from both components, both locale files, and the web test file; no orphaned references found. - console.log (dimension 7): None.
- DESIGN.md (dimension 8): N/A — diff doesn't touch rendered UI markup/tokens, only tab data and copy.
FEATURES.mdis a repo-root doc, not app UI copy, so the em-dash ban doesn't gate it; the em dash appearing inpt-BR.json'saiMemoryDescpredates this PR (unchanged from the removed line). - Contract drift (dimension 11): N/A — no
packages/shared/src/types/*or endpoint changes; not verifiable againstorbit-apiin this CI job regardless (sibling repo not checked out). - Security (dimension 12): N/A — no auth, input handling, or trust-boundary code touched.
Validation (per PR description, not independently re-run — CI's Build/Unit Tests/SonarCloud checks cover this)
tsc --noEmit clean, eslint clean, shared vitest 1378 passed, mobile vitest 913 passed, web feature-guide-drawer test 14 passed.
…w rubric Every PR that adds/changes/removes a user-facing feature, gating, platform availability, or Astra/MCP tools must update FEATURES.md in the same PR — missing update is High, mirroring the web-mobile parity bar. orbit-api diffs emit a cross-repo flag instead of verifying. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…w rubric Mirror of the orbit-ui-mobile rubric addition (thomasluizon/orbit-ui-mobile#386): API diffs that change the user-facing feature surface (gating, Astra/MCP tools) must flag the required FEATURES.md update in the paired frontend PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review: PR #386 — pr-review rubric: FEATURES.md parity dimension
Scope: Reviewing the diff since the last bot review (commit 5fd9fa5d, already APPROVEd). The new head (8d7363b3) adds exactly one commit on top: wiring a new rubric dimension #14 ("FEATURES.md parity") into .claude/skills/pr-review/rubric.md and .claude/skills/pr-review/SKILL.md. No apps/, packages/, or orbit-api files changed in this increment.
Recommendation: APPROVE
Summary
This increment is a self-referential, docs-only change to the review tooling itself: it adds "14. FEATURES.md parity" to the shared rubric and threads the gate/dimension-list references through SKILL.md Phase 3. The new section is internally consistent (correct numbering, correct severity-ladder usage — High for a missing update/stale claim, Medium for a stale in-app guide, matching the bar the rubric already sets for "missing parity"), correctly gated ("only when the diff changes the user-facing feature surface"), and correctly handles the cross-repo case (when reviewing an orbit-api-only PR, FEATURES.md isn't in that checkout, so the rule degrades to "flag that the paired frontend PR needs the update" rather than trying to verify it). No Critical or High findings survive.
Findings
Critical
None.
High
None.
Medium
None.
Low / Info
None posted (signal gate: Low/Info nits are dropped on a PR review). One informational, non-blocking note: .claude/skills/audit-code-quality/SKILL.md (lines ~89-108) hardcodes a per-slice "Rubric dimensions in focus" table and a sentence explicitly listing which dimensions are "diff-only" and dropped for a whole-repo audit (currently names #11 and security). Dimension 14 is diff-only by the same logic but isn't mentioned there. Net effect is already correct — #14 simply never appears in any slice's dimension list — but the explanatory sentence is now slightly stale. Not concretely harmful enough to block; a one-line follow-up would close the gap if the author wants full self-consistency.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A — no apps/web/** or apps/mobile/** file in this increment |
| i18n-syncer | N/A — no user-facing strings or i18n/*.json in this increment |
| contract-aligner | N/A — no packages/shared/src/types/* / endpoints.ts; also not verifiable in CI (sibling repo not checked out) |
| security-reviewer | N/A — no orbit-api code changed |
Validation
Skipped per task instructions — Build/Unit Tests/SonarCloud run as separate required CI checks for this job.
What's good
- The new dimension follows the existing template exactly (Reference line, gate statement, severity-ladder-consistent bullets), so it reads as native to the file rather than bolted on.
- Correctly reuses the established "High = missing parity" and "Medium = degraded but not missing" severity calibration instead of inventing a new scale.
- Explicitly handles the cross-repo blind spot (file not in checkout when reviewing
orbit-api) rather than leaving it to guesswork — consistent with howcontract-aligner's sibling-repo gap is already handled elsewhere in the skill.
Recommendation
Merge as-is. Optional, non-blocking follow-up: add dimension 14 to the "diff-only dimensions" sentence in .claude/skills/audit-code-quality/SKILL.md for full self-consistency, but this does not need to hold up this PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review: PR #386 (orbit-ui-mobile)
Recommendation: APPROVE
Summary
Frontend-only PR (no orbit-api changes) that (1) adds a new root-level FEATURES.md feature inventory and restructures the in-app FeatureGuideDrawer from 6→9 tabs (Astra, Connect, Social, Habits, Goals, Calendar, Rewards, Settings, Notifications) identically on web and mobile, atomically renaming chat/chatSection → astra/astraSection, (2) adds a FEATURES.md parity dimension to this repo's own pr-review rubric, and (3) a one-line CSS spacing fix on the web support page. The restructure is clean: both platform components are line-for-line structurally identical, i18n is fully synced (163/163 keys, byte-identical key sets, genuine translations), no stale references to the old chat/chatSection keys remain anywhere, and the web test suite was updated to match the new 9-tab structure.
Findings
Critical
None.
High
None.
Medium
None.
Low / Info
- [Info] Three logically-distinct commits bundled into one PR — feat (FEATURES.md + drawer restructure), chore (rubric dimension 14 addition), and an unrelated fix (support-page spacing). Root CLAUDE.md's "one feature or fix per PR" convention would normally split these three; not blocking since each is small, low-risk, and self-contained.
- [Info] Mobile has no dedicated test file for
FeatureGuideDrawer— confirmed via parity-checker this is a pre-existing gap, not introduced or worsened by this PR.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED |
| i18n-syncer | IN SYNC |
| contract-aligner | N/A — no orbit-api changes, no shared types/endpoints touched |
| security-reviewer | N/A — no orbit-api code changed |
parity-checker independently confirmed the tab list, sectionItems record (all 9 sections, every titleKey/descKey pair, same order), and default active section ('astra') are byte-identical between the web and mobile drawer components. It also confirmed the web support-page spacing tweak needs no mobile mirror (the <aside> is a desktop-only helper panel; mobile's SupportForm already inlines the same description text above the fields — a pre-existing, legitimate layout divergence).
i18n-syncer independently confirmed all 163 onboarding.featureGuide.* leaf keys exist in both en.json and pt-BR.json with genuine translations, zero stale references to the removed keys anywhere in apps/web or apps/mobile, and brand words correctly left untranslated in pt-BR.
Validation
| Check | Result |
|---|---|
| Lint | N/A — no node_modules installed in this review environment; PR body reports eslint pass |
| Type check | N/A — same reason; PR body reports tsc --noEmit pass across shared/web/mobile |
| Tests | N/A — same reason; PR body reports 1378 shared + 913 mobile + 14 web tests passing |
| Build (api) | N/A — no orbit-api changes in this diff |
Manual review found no syntax/type/logic issues. Self-reported PR numbers (163/163 i18n keys, 9/9 tabs, byte-identical structures) were independently re-derived and confirmed in this review (jq key-set hash match, line-by-line component diff).
Deferred
- Ground-truth tool-count verification (61 IAiTool, 94 McpServerTool across 15 classes, PayGateService.cs gating claims in FEATURES.md) — not verifiable in this environment; the orbit-api sibling repo is not checked out alongside orbit-ui-mobile here. Recommend a follow-up spot-check against orbit-api source.
- Contract drift / backward-compat (#11), Backend hard rules (#13) — N/A, no orbit-api or shared-type changes.
- Security (#12) — N/A for API-side categories; frontend categories reviewed manually, no findings.
- All other changed files were read in full and given a verdict — nothing left un-reviewed.
What's good
- The chat → astra rename is genuinely atomic: zero orphaned references left anywhere.
- Web and mobile drawer components are structurally identical down to key order.
- The new i18n content is real, idiomatic Portuguese, not machine-copied English.
- The web test file was proactively expanded (6→11 test cases) to cover every new tab.
- FEATURES.md cites its own sources inline and documents which counts need re-verification.
Recommendation
Approve as-is. Optional non-blocking follow-up: a human with both repos checked out should spot-check the FEATURES.md tool-count claims (61/94/15) against current orbit-api source before treating it as fully authoritative for downstream consumers.
🤖 Generated with /pr-review
|
* chore: prune superseded root docs — description.md + mangled planning orphan Removes two dead root-level docs superseded by README.md; the second is a path-collapsed junk filename. Docs-only; no code references them. Refs thomasluizon/orbit-ui-mobile#380 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(review): add FEATURES.md parity dimension (#14) to the pr-review rubric Mirror of the orbit-ui-mobile rubric addition (thomasluizon/orbit-ui-mobile#386): API diffs that change the user-facing feature surface (gating, Astra/MCP tools) must flag the required FEATURES.md update in the paired frontend PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
/pr-review — PR #386
feat(web,mobile,shared): FEATURES.md inventory + feature-guide restructure (#378)
Recommendation: APPROVE
| Severity | Count |
|---|---|
| Critical (incl. |
0 |
| High | 0 |
| Medium | 0 |
| Low / Info | 1 |
Summary
Adds FEATURES.md (net-new, code-derived feature inventory) and restructures FeatureGuideDrawer from 6→9 tabs (Astra, Connect, Social, Habits, Goals, Calendar, Rewards, Settings, Notifications), identically on web and mobile, fully localized en/pt-BR. Frontend/docs-only, no orbit-api touched.
Verified directly (not just via diff-reading):
- Parity:
apps/web/components/onboarding/feature-guide-drawer.tsxandapps/mobile/components/onboarding/feature-guide-drawer.tsxhave identicalSectionKeyunions, tab order, andsectionItemscontent pointers. - i18n: full
onboarding.featureGuide.*block in bothen.jsonandpt-BR.json— exact key parity, zero drift, no orphaned old keys left anywhere in the repo. - FEATURES.md accuracy (frontend-checkable claims): confirmed
/insights,/u/[slug],/delete-account,r/[code],/exploreare web-only routes; the Android widget module has no web counterpart; the accountability-pair route-shape difference (dedicated mobile screen vs. embedded web component) is real. Backend-sourced claims (AI/MCP tool counts, gating services) are not verifiable in this environment sinceorbit-apiis not checked out here.
Findings
Low/Info — apps/web/app/(app)/support/page.tsx carries an unrelated paddingTop tweak (from commit 61e1c980) not described in the PR title/body. The change itself is correct and low-risk (desktop-only sidebar, no mobile mirror needed) — a process note, not a blocker.
Since last review
No new feature-diff commits since the prior APPROVE (5fd9fa5d) — the only subsequent commit is a merge from main. No new findings to raise.
Validation
Could not execute lint/typecheck/tests in this review sandbox. Per CI adaptations for this workflow, Build / Unit Tests / SonarCloud run as separate required checks and are not re-run here.
Deferred
- Mobile has no unit test file for
FeatureGuideDrawer(pre-existing gap, not introduced by this PR). - Backend-sourced
FEATURES.mdnumbers not independently verifiable (sibling repo not present in this environment).
1 participant
Two connected, frontend-only deliverables (no orbit-api changes). Closes #378.
Issue checkboxes
FEATURES.mdexists, is exhaustive, marks plan gating + platform per feature. Net-new at the repo root: marketing-toned headline (Astra → MCP → social → core tracker), then 14 per-domain tables with columnsFeature | Description | Gating | Platform | Locale notes. Astra and MCP are category+count rows. Marks/insightsweb-only,/u/[slug]web-only, widget Android-only, Retrospective Yearly-Pro, and the no-leaderboards fact.FeatureGuideDrawerrestructured 6 → 9 tabs on both platforms (kept structurally identical): Astra, Connect, Social, Habits, Goals, Calendar, Rewards, Settings, Notifications.chat→astra/chatSection→astraSectionrenamed atomically across both components + both locales; MCP (Connect) and social layers surfaced; gamification/widget/referral/retrospective promoted out of Settings into Rewards/Social. Both locales fully authored (pt-BR translated, not stubbed).Ground truth (verified against orbit-api source)
IAiToolregistrations (ServiceCollectionExtensions.AiServices.cs).[McpServerTool]methods across 15 tool classes (Mcp/Tools/*.cs) — the plan's 79 was stale; re-verified and corrected.PayGateService.cs/AppConstants.cs. Copy corrected where the old strings misstated gating: gamification/XP is Free (feature flag, not PayGate), retrospective is Yearly-Pro,/insightsis Pro + web-only.i18n
onboarding.featureGuide.*= 163 keys in en.json, 163 in pt-BR.json, identical sets (0 drift). No stalechatSection/featureGuide.chatreferences remain.Validation
tsc --noEmit: shared / web / mobile all pass.eslint(changed files, both platforms): pass.packages/sharedvitest: 1378 passed (incl.i18n.test.tsen/pt-BR parity).apps/mobilevitest: 913 passed.apps/webfeature-guide-drawer test: 14 passed (updated for the 9-tab structure).Deferred
Visual QA of the 9-tab guide on both platforms (dev-server / emulator walkthrough) is deferred to the user — this was a content-only change with no browser E2E in scope.
🤖 Generated with Claude Code