Conversation
📝 WalkthroughWalkthroughThis pull request establishes a complete feature roadmap and initial implementation for a Duolingo-inspired language learning app. It includes foundational engineering guidelines, design system setup, home screen refactor, authentication flow, learning content system, navigation, lesson UI, audio integration with Vision Agents backend, and analytics instrumentation. ChangesLanguage Learning App Feature Roadmap
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
prompts/AGENTS.md (1)
86-87:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComplete or remove the dangling architecture directive.
The sentence ends with a colon but no structure follows, leaving a required guideline undefined.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prompts/AGENTS.md` around lines 86 - 87, The line "Use this structure unless there is a strong reason to change it:" ends with a colon but has no structure; either complete it by appending the intended structure/example (e.g., a bullet list or template showing the required fields/format) or remove the colon and rephrase to a complete sentence (e.g., "Use this structure unless there is a strong reason to change it.") so the directive in prompts/AGENTS.md is no longer dangling.
🧹 Nitpick comments (3)
prompts/11-lesson-ui.md (1)
5-5: ⚡ Quick winReduce required data expansion for the first iteration.
Requiring “at least 5 more lessons” may over-scope the initial implementation; consider “add only enough lessons to support the required UI states” for faster, simpler delivery.
Based on learnings: "Build feature by feature... build the smallest useful version first, and refactor only when repetition or complexity appears".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prompts/11-lesson-ui.md` at line 5, Update the lesson data expansion rule in the "Allow users to select/open any lesson" requirement so it no longer mandates adding "at least 5 more lessons"; instead change the instruction text to say "add only enough lessons to support the required UI states" (or equivalent minimal expansion). Locate the sentence in the prompt that currently requires "at least 5 more lessons" and replace it with the smaller-scope phrasing so UI implementation can create only the minimal number of placeholder lessons needed for selection/opening and state coverage.app/onboarding.tsx (2)
6-6: ⚡ Quick winConsider consolidating image imports for consistency.
Line 6 uses
require()directly formascotWelcome, while Line 26 uses theimagesconstant formascotLogo. For maintainability, consider addingmascotWelcometo theimagesconstant and importing it from there.♻️ Suggested approach
Add to
constants/images:mascotWelcome: require("../assets/images/mascot-welcome.png")Then update this file:
-const mascotWelcome = require("../assets/images/mascot-welcome.png"); - export default function OnboardingScreen() {<Image - source={mascotWelcome} + source={images.mascotWelcome} resizeMode="contain"Also applies to: 26-26
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/onboarding.tsx` at line 6, Consolidate image imports by adding mascotWelcome to the shared images constant (e.g., add mascotWelcome: require("../assets/images/mascot-welcome.png") in constants/images) and then replace the local require in onboarding.tsx by importing mascotWelcome from the images object like mascotLogo is used; update any references in onboarding.tsx to use images.mascotWelcome (or the destructured mascotWelcome) so all image assets are consistently loaded from the images constant.
40-40: 💤 Low valueHardcoded color value instead of design token.
Line 40 uses
#626b85directly. Consider extracting this to a design token in your Tailwind theme (e.g.,text-lingua-text-secondaryor similar) for consistency with the rest of the color system.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/onboarding.tsx` at line 40, Replace the hardcoded color hex in the Text element's className in app/onboarding.tsx (the JSX Text with className "mt-[10px] font-poppins text-[11px] leading-[16px] text-[`#626b85`]") with a design-token class (e.g., text-lingua-text-secondary) and add that token to the Tailwind theme colors in your tailwind.config.js (map the token to the `#626b85` value) so the component uses the new token consistently across the app.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/onboarding.tsx`:
- Line 66: The "Get Started" Link in app/onboarding.tsx currently navigates to
"/" but should go to the Sign Up screen; update the Link component's href prop
(the Link usage that wraps the "Get Started" button) to point to the Sign Up
route (e.g., "/signup" or the app's signup path) so clicking Get Started
navigates to the Sign Up screen instead of home.
- Around line 30-31: Replace the hardcoded app name "lingua" in the onboarding
UI with the required name "muolingo": update the Text element (the JSX element
rendering the app title in app/onboarding.tsx, i.e., the Text node with
className "font-poppins-bold text-[14px] leading-[18px]
text-lingua-text-primary") so its displayed string is "muolingo" instead of
"lingua".
In `@prompts/05-clerk.md`:
- Line 7: Fix the typo in the route text by replacing the malformed route string
"(/onboarding(" with the correct closing-parenthesis form "(/onboarding)";
update the sentence that reads "If not authenticated, show onboarding route
(/onboarding(" so it reads "If not authenticated, show onboarding route
(/onboarding)" to ensure the route path is correctly closed.
In `@prompts/10-home-ui.md`:
- Line 3: The prompt text is ambiguous and repetitive ("exactly ... exactly
done"); replace that clause with one concise constraint sentence that specifies
fidelity to the provided design for spacing and layout, while keeping the two
functional requirements (display logged-in user info from Clerk and the selected
language from Zustand + AsyncStorage). Update the prompt in
prompts/10-home-ui.md to remove duplicate "exactly" phrasing, state a single
clear constraint about following the attached design's spacing/layout, and
explicitly retain the requirements to show Clerk user info and the
Zustand+AsyncStorage language selection for implementers.
In `@prompts/18-more-posthog.md`:
- Line 19: Update the custom-events numbering so the sequence is consecutive:
change the "4. lesson_abandoned — fires when the user exits a lesson before
lesson_completed fires (back navigation, screen unmount before completion)."
entry number from 4 to 3 (the event identifier is lesson_abandoned) so the list
reads 1, 2, 3 rather than skipping to 4.
- Around line 20-25: The spec mismatches the property name for the
lesson-abandon event: change either the reference to duration_seconds to use
time_into_lesson_seconds or vice versa so both uses are identical; update the
event sending code that builds the lesson_abandoned PostHog properties (look for
the usage that references lesson_id, last_question_index and the mount-captured
start-time ref) to emit a single consistent property name (e.g.,
time_into_lesson_seconds) and ensure the ref-based duration calculation assigns
to that same key.
---
Outside diff comments:
In `@prompts/AGENTS.md`:
- Around line 86-87: The line "Use this structure unless there is a strong
reason to change it:" ends with a colon but has no structure; either complete it
by appending the intended structure/example (e.g., a bullet list or template
showing the required fields/format) or remove the colon and rephrase to a
complete sentence (e.g., "Use this structure unless there is a strong reason to
change it.") so the directive in prompts/AGENTS.md is no longer dangling.
---
Nitpick comments:
In `@app/onboarding.tsx`:
- Line 6: Consolidate image imports by adding mascotWelcome to the shared images
constant (e.g., add mascotWelcome:
require("../assets/images/mascot-welcome.png") in constants/images) and then
replace the local require in onboarding.tsx by importing mascotWelcome from the
images object like mascotLogo is used; update any references in onboarding.tsx
to use images.mascotWelcome (or the destructured mascotWelcome) so all image
assets are consistently loaded from the images constant.
- Line 40: Replace the hardcoded color hex in the Text element's className in
app/onboarding.tsx (the JSX Text with className "mt-[10px] font-poppins
text-[11px] leading-[16px] text-[`#626b85`]") with a design-token class (e.g.,
text-lingua-text-secondary) and add that token to the Tailwind theme colors in
your tailwind.config.js (map the token to the `#626b85` value) so the component
uses the new token consistently across the app.
In `@prompts/11-lesson-ui.md`:
- Line 5: Update the lesson data expansion rule in the "Allow users to
select/open any lesson" requirement so it no longer mandates adding "at least 5
more lessons"; instead change the instruction text to say "add only enough
lessons to support the required UI states" (or equivalent minimal expansion).
Locate the sentence in the prompt that currently requires "at least 5 more
lessons" and replace it with the smaller-scope phrasing so UI implementation can
create only the minimal number of placeholder lessons needed for
selection/opening and state coverage.
🪄 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 Plus
Run ID: 6a18e83a-7d0c-4229-a30e-42bfb32be34f
📒 Files selected for processing (21)
app/index.tsxapp/onboarding.tsxprompts/01-nativewind.mdprompts/02-design-theme.mdprompts/03-onboarding-ui.mdprompts/04-authentication-ui.mdprompts/05-clerk.mdprompts/06-content-system.mdprompts/07-language-ui.mdprompts/08-zustand.mdprompts/09-bottom-tab-nav.mdprompts/10-home-ui.mdprompts/11-lesson-ui.mdprompts/12-audio-lesson-ui.mdprompts/13-stream-integration.mdprompts/14-vision-agents.mdprompts/15-connection-to-ui.mdprompts/16-ai-teacher-improvements.mdprompts/17-live-captions.mdprompts/18-more-posthog.mdprompts/AGENTS.md
| <Text className="font-poppins-bold text-[14px] leading-[18px] text-lingua-text-primary"> | ||
| lingua |
There was a problem hiding this comment.
App name doesn't match the prompt specification.
The code uses "lingua" but prompts/03-onboarding-ui.md explicitly specifies the app name should be "muolingo" (replacing "lingua").
📝 Proposed fix
- <Text className="font-poppins-bold text-[14px] leading-[18px] text-lingua-text-primary">
- lingua
+ <Text className="font-poppins-bold text-[14px] leading-[18px] text-lingua-text-primary">
+ muolingo📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Text className="font-poppins-bold text-[14px] leading-[18px] text-lingua-text-primary"> | |
| lingua | |
| <Text className="font-poppins-bold text-[14px] leading-[18px] text-lingua-text-primary"> | |
| muolingo |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/onboarding.tsx` around lines 30 - 31, Replace the hardcoded app name
"lingua" in the onboarding UI with the required name "muolingo": update the Text
element (the JSX element rendering the app title in app/onboarding.tsx, i.e.,
the Text node with className "font-poppins-bold text-[14px] leading-[18px]
text-lingua-text-primary") so its displayed string is "muolingo" instead of
"lingua".
|
|
||
| <View className="flex-1" /> | ||
|
|
||
| <Link href="/" asChild> |
There was a problem hiding this comment.
"Get Started" navigates to the wrong route.
The button links to the home route (/) but prompts/04-authentication-ui.md Line 5 explicitly states: "Update onboarding so pressing Get Started navigates to the Sign Up screen."
🔗 Proposed fix
- <Link href="/" asChild>
+ <Link href="/sign-up" asChild>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Link href="/" asChild> | |
| <Link href="/sign-up" asChild> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/onboarding.tsx` at line 66, The "Get Started" Link in app/onboarding.tsx
currently navigates to "/" but should go to the Sign Up screen; update the Link
component's href prop (the Link usage that wraps the "Get Started" button) to
point to the Sign Up route (e.g., "/signup" or the app's signup path) so
clicking Get Started navigates to the Sign Up screen instead of home.
|
|
||
| Keep the existing UI and navigation flow intact. Implement email-based Sign Up, Sign In, social auth where supported, and verification code handling through Clerk. | ||
|
|
||
| After successful verification/authentication, navigate to the home route (/). If not authenticated, show onboarding route (/onboarding(. If authenticated, show home route (/). |
There was a problem hiding this comment.
Fix typo in route path.
Missing closing parenthesis: (/onboarding( should be (/onboarding).
🐛 Proposed fix
-After successful verification/authentication, navigate to the home route (/). If not authenticated, show onboarding route (/onboarding(. If authenticated, show home route (/).
+After successful verification/authentication, navigate to the home route (/). If not authenticated, show onboarding route (/onboarding). If authenticated, show home route (/). 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| After successful verification/authentication, navigate to the home route (/). If not authenticated, show onboarding route (/onboarding(. If authenticated, show home route (/). | |
| After successful verification/authentication, navigate to the home route (/). If not authenticated, show onboarding route (/onboarding). If authenticated, show home route (/). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prompts/05-clerk.md` at line 7, Fix the typo in the route text by replacing
the malformed route string "(/onboarding(" with the correct closing-parenthesis
form "(/onboarding)"; update the sentence that reads "If not authenticated, show
onboarding route (/onboarding(" so it reads "If not authenticated, show
onboarding route (/onboarding)" to ensure the route path is correctly closed.
| @@ -0,0 +1,9 @@ | |||
| Read AGENTS.md first and follow it strictly. | |||
|
|
|||
| Implement the Home screen UI exactly as shown in the attached design with spacing structure and such neatly exactly done. Display the logged-in user information from Clerk and the selected language from Zustand + AsyncStorage. | |||
There was a problem hiding this comment.
Clarify the implementation directive wording.
Line 3 is repetitive/ambiguous (exactly ... exactly done), which makes execution less deterministic. Suggest tightening it to one clear constraint sentence.
Proposed edit
-Implement the Home screen UI exactly as shown in the attached design with spacing structure and such neatly exactly done. Display the logged-in user information from Clerk and the selected language from Zustand + AsyncStorage.
+Implement the Home screen UI to match the attached design closely, including spacing and layout. Display logged-in user information from Clerk and the selected language from Zustand + AsyncStorage.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Implement the Home screen UI exactly as shown in the attached design with spacing structure and such neatly exactly done. Display the logged-in user information from Clerk and the selected language from Zustand + AsyncStorage. | |
| Implement the Home screen UI to match the attached design closely, including spacing and layout. Display logged-in user information from Clerk and the selected language from Zustand + AsyncStorage. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prompts/10-home-ui.md` at line 3, The prompt text is ambiguous and repetitive
("exactly ... exactly done"); replace that clause with one concise constraint
sentence that specifies fidelity to the provided design for spacing and layout,
while keeping the two functional requirements (display logged-in user info from
Clerk and the selected language from Zustand + AsyncStorage). Update the prompt
in prompts/10-home-ui.md to remove duplicate "exactly" phrasing, state a single
clear constraint about following the attached design's spacing/layout, and
explicitly retain the requirements to show Clerk user info and the
Zustand+AsyncStorage language selection for implementers.
| 2. lesson_started — fires when the lesson screen mounts and the user begins the lesson. | ||
| Properties: { lesson_id: string, language: string, lesson_number: number } | ||
|
|
||
| 4. lesson_abandoned — fires when the user exits a lesson before lesson_completed fires (back navigation, screen unmount before completion). |
There was a problem hiding this comment.
Fix event numbering typo (4. should be 3.).
The custom-events list skips from 2 to 4, which can confuse implementation/checklist tracking.
Suggested fix
-4. lesson_abandoned — fires when the user exits a lesson before lesson_completed fires (back navigation, screen unmount before completion).
+3. lesson_abandoned — fires when the user exits a lesson before lesson_completed fires (back navigation, screen unmount before completion).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prompts/18-more-posthog.md` at line 19, Update the custom-events numbering so
the sequence is consecutive: change the "4. lesson_abandoned — fires when the
user exits a lesson before lesson_completed fires (back navigation, screen
unmount before completion)." entry number from 4 to 3 (the event identifier is
lesson_abandoned) so the list reads 1, 2, 3 rather than skipping to 4.
| Properties: { lesson_id: string, time_into_lesson_seconds: number, last_question_index: number } | ||
|
|
||
|
|
||
| Implementation rules: | ||
| - Track lesson start time with a ref captured on mount so duration_seconds is accurate. | ||
| - Do not modify any UI. |
There was a problem hiding this comment.
Align lesson_abandoned duration property naming across spec lines.
Line 20 defines time_into_lesson_seconds, but Line 24 references duration_seconds. This mismatch can fragment PostHog event schema and break downstream queries.
Suggested fix
-Properties: { lesson_id: string, time_into_lesson_seconds: number, last_question_index: number }
+Properties: { lesson_id: string, duration_seconds: number, last_question_index: number }
-Track lesson start time with a ref captured on mount so duration_seconds is accurate.
+Track lesson start time with a ref captured on mount so duration_seconds is accurate.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Properties: { lesson_id: string, time_into_lesson_seconds: number, last_question_index: number } | |
| Implementation rules: | |
| - Track lesson start time with a ref captured on mount so duration_seconds is accurate. | |
| - Do not modify any UI. | |
| Properties: { lesson_id: string, duration_seconds: number, last_question_index: number } | |
| Implementation rules: | |
| - Track lesson start time with a ref captured on mount so duration_seconds is accurate. | |
| - Do not modify any UI. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prompts/18-more-posthog.md` around lines 20 - 25, The spec mismatches the
property name for the lesson-abandon event: change either the reference to
duration_seconds to use time_into_lesson_seconds or vice versa so both uses are
identical; update the event sending code that builds the lesson_abandoned
PostHog properties (look for the usage that references lesson_id,
last_question_index and the mount-captured start-time ref) to emit a single
consistent property name (e.g., time_into_lesson_seconds) and ensure the
ref-based duration calculation assigns to that same key.
There was a problem hiding this comment.
This implementation hasn't been added yet — currently at prompt03. Will apply the consistent naming (time_into_lesson_seconds) when the PostHog lesson_abandoned event is implemented in a later PR.
There was a problem hiding this comment.
@Retsomm, understood — thanks for the context! Since the fix is planned for a later PR, would you like me to open a GitHub issue to track the naming consistency (time_into_lesson_seconds vs duration_seconds in prompts/18-more-posthog.md) so it's not lost when you reach that implementation step?
There was a problem hiding this comment.
Yes please, that would be helpful!
There was a problem hiding this comment.
🧩 Analysis chain
@Retsomm, I've created a GitHub issue to track this — you can find it linked above once the script runs. It captures the time_into_lesson_seconds vs duration_seconds inconsistency with a reference back to this PR comment, so it'll be easy to pick up when you reach that implementation step. 🐇
🏁 Script executed:
#!/bin/bash
gh issue create \
--repo Retsomm/react-native-lingua \
--title "Fix lesson_abandoned event property name inconsistency: time_into_lesson_seconds vs duration_seconds" \
--body "## Summary
In \`prompts/18-more-posthog.md\`, the \`lesson_abandoned\` event has a property name mismatch:
- **Line 20** defines the property as \`time_into_lesson_seconds\` in the event schema.
- **Line 24** references it as \`duration_seconds\` in the implementation rule about the mount-time ref.
This inconsistency should be resolved before the PostHog \`lesson_abandoned\` event is implemented (currently planned after prompt 03).
## Recommended fix
Use \`time_into_lesson_seconds\` consistently in both the event property definition and the implementation rule, so the emitted event and the ref-based duration calculation share the same key.
## References
- PR: https://github.com/Retsomm/react-native-lingua/pull/1
- Comment: https://github.com/Retsomm/react-native-lingua/pull/1#discussion_r3256438596
/cc `@Retsomm`" \
--assignee Retsomm
Length of output: 56
@Retsomm, the issue has been created: #2 - Fix lesson_abandoned event property name inconsistency: time_into_lesson_seconds vs duration_seconds
It's assigned to you and links back to this PR comment for easy reference when you get to the PostHog implementation. 🐇 ✨
Summary by CodeRabbit