Release v 1.0.3#151
Conversation
…ystem # Conflicts: # src/app/learning-plans/new.tsx # src/components/bottom-nav.tsx
Refresh the app design system across core screens
…ayova-mvp into redesign-learnplan-page
Generate higher-quality theory, practice, and praxis session content. Make Praxis advance without per-answer feedback and analyze at the end. Add theory-card queue behavior and fallback coverage tests. CodeRabbit review was attempted but blocked by rate limit.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
… scroll indicator
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(app)/home.tsx (1)
425-444: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSync
dayKeyintoselectedDayKeyon mount.HomeScreenstill initializes fromtoday, so/home?dayKey=...lands on the wrong day unless the search param is applied insrc/app/(app)/home.tsx.🤖 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 `@src/app/`(app)/home.tsx around lines 425 - 444, HomeScreen is still initializing selectedDayKey from today instead of the incoming dayKey query, so deep links can open the wrong day. Update the HomeScreen state setup and its mount-time effect logic in the selectedDayKey flow to read the route/search param and sync it immediately, using the existing getDayKey, useCurrentLocalDay, and setSelectedDayKey logic in HomeScreen.
🧹 Nitpick comments (1)
src/app/learning-plans/[planId]/success.tsx (1)
1-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared success-screen logic to remove duplication.
This file is nearly identical to
src/app/entry/success.tsx(sameStack.Screen/StatusBarsetup, samefinish()redirect pattern, sameuseMemoshape aroundbuildDateTimeLabel). Consider extracting a small shared hook (e.g.useFinishRedirect(dayKey)) or a wrapper component that takestitle/detailLabel/date-time params, to avoid maintaining two copies of this navigation/formatting logic.🤖 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 `@src/app/learning-plans/`[planId]/success.tsx around lines 1 - 43, This success screen duplicates the same navigation and date-label setup used in the entry success screen, so extract the shared logic into a reusable hook or wrapper component. Move the common `useMemo`/`buildDateTimeLabel` handling and the `finish()` redirect pattern from `LearningPlanSuccessScreen` into a shared helper (for example a `useFinishRedirect` hook or generic success-screen wrapper), then pass only the screen-specific `title`, `detailLabel`, and params into each screen.
🤖 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 `@convex/learningPlanAi.ts`:
- Around line 1430-1447: The multiple-choice mapping in learningPlanAi.ts can
persist a guessed correct answer when no choice is marked isCorrect, which later
breaks grading in submitAnswer. Update the logic in the multipleChoice branch to
validate that exactly one choice has isCorrect before building the returned
item, and if none is found, throw so withGeneratedTextRetry regenerates instead
of defaulting correctChoiceId to the first option. Use the existing item.choices
mapping and correctChoice lookup to locate the fix.
In `@src/app/`(app)/home.tsx:
- Around line 390-395: The fallback in getLearningCardSummary is hardcoded
placeholder copy and should be replaced with a generic user-facing message.
Update the final return path in getLearningCardSummary so that when a DayEntry
has no notes and no kind, it uses neutral fallback text consistent with the
other branches rather than the math-equation sentence, keeping the logic around
formatGermanUiText, entry.notes, and entry.kind unchanged.
---
Outside diff comments:
In `@src/app/`(app)/home.tsx:
- Around line 425-444: HomeScreen is still initializing selectedDayKey from
today instead of the incoming dayKey query, so deep links can open the wrong
day. Update the HomeScreen state setup and its mount-time effect logic in the
selectedDayKey flow to read the route/search param and sync it immediately,
using the existing getDayKey, useCurrentLocalDay, and setSelectedDayKey logic in
HomeScreen.
---
Nitpick comments:
In `@src/app/learning-plans/`[planId]/success.tsx:
- Around line 1-43: This success screen duplicates the same navigation and
date-label setup used in the entry success screen, so extract the shared logic
into a reusable hook or wrapper component. Move the common
`useMemo`/`buildDateTimeLabel` handling and the `finish()` redirect pattern from
`LearningPlanSuccessScreen` into a shared helper (for example a
`useFinishRedirect` hook or generic success-screen wrapper), then pass only the
screen-specific `title`, `detailLabel`, and params into each screen.
🪄 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: 5736748c-203b-4c30-aa4c-6daedee4b1b4
⛔ Files ignored due to path filters (8)
assets/onboarding/dayova-y.pngis excluded by!**/*.pngassets/onboarding/intro-path.svgis excluded by!**/*.svgassets/onboarding/intro-tasks.svgis excluded by!**/*.svgassets/onboarding/intro-upload.svgis excluded by!**/*.svgconvex/_generated/ai/ai-files.state.jsonis excluded by!**/_generated/**convex/_generated/ai/guidelines.mdis excluded by!**/_generated/**convex/_generated/api.d.tsis excluded by!**/_generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (50)
convex/learningPlanAi.tsconvex/learningPlanAiScheduling.test.tsconvex/learningPlanPlanningHints.tsconvex/learningPlans.test.tsconvex/learningPlans.tsconvex/learningSessionContent.test.tsconvex/learningSessionContent.tsconvex/learningTimes.test.tsconvex/learningTimes.tsconvex/users.tsdocs/contexts/auth/CONTEXT.mddocs/contexts/product/CONTEXT.mdmetro.config.jspackage.jsonsrc/app/(app)/home.tsxsrc/app/(app)/learning-plans.tsxsrc/app/(auth)/index.tsxsrc/app/(auth)/login.tsxsrc/app/(auth)/onboarding.tsxsrc/app/(auth)/register.tsxsrc/app/_layout.tsxsrc/app/entry/new.tsxsrc/app/entry/success.tsxsrc/app/learning-plans/[planId]/index.tsxsrc/app/learning-plans/[planId]/review.tsxsrc/app/learning-plans/[planId]/sessions/[sessionId]/index.tsxsrc/app/learning-plans/[planId]/success.tsxsrc/app/learning-plans/new.tsxsrc/app/learning-times/edit.tsxsrc/app/learning-times/index.tsxsrc/components/onboarding/onboarding-flow.test.tssrc/components/onboarding/onboarding-flow.tssrc/components/onboarding/onboarding-ui.tsxsrc/components/ui/date-time-picker-sheet.tsxsrc/components/ui/icon.tsxsrc/components/ui/notched-action-card.tsxsrc/components/ui/success-confirmation-screen.tsxsrc/context/AuthContext.tsxsrc/context/OnboardingContext.tsxsrc/features/auth/dayova-auth-flow.tsxsrc/features/learning-plans/learning-plan-ui.tsxsrc/features/learning-plans/replan-recovery.test.tssrc/features/learning-plans/replan-recovery.tssrc/features/learning-plans/theory-card-queue.test.tssrc/features/learning-plans/theory-card-queue.tssrc/lib/date-time-label.tssrc/lib/navigation-actions.test.tssrc/lib/navigation-actions.tssrc/lib/navigation.tssrc/types/svg.d.ts
✅ Files skipped from review due to trivial changes (3)
- src/app/(auth)/login.tsx
- convex/learningPlanPlanningHints.ts
- docs/contexts/auth/CONTEXT.md
🚧 Files skipped from review as they are similar to previous changes (8)
- docs/contexts/product/CONTEXT.md
- src/app/_layout.tsx
- src/components/ui/notched-action-card.tsx
- src/app/learning-plans/new.tsx
- src/app/learning-plans/[planId]/index.tsx
- src/app/learning-plans/[planId]/sessions/[sessionId]/index.tsx
- src/app/(app)/learning-plans.tsx
- src/app/learning-times/index.tsx
| if (item.kind === "multipleChoice") { | ||
| const choices = item.choices.map((choice, choiceIndex) => ({ | ||
| id: `choice-${choiceIndex + 1}`, | ||
| text: normalizeAiGeneratedGermanText(choice.text), | ||
| isCorrect: choice.isCorrect, | ||
| })); | ||
| const correctChoice = choices.find((choice) => choice.isCorrect); | ||
|
|
||
| return { | ||
| kind: "multipleChoice" as const, | ||
| title: title || `Aufgabe ${index + 1}`, | ||
| prompt, | ||
| explanation, | ||
| idealAnswer, | ||
| choices: choices.map(({ id, text }) => ({ id, text })), | ||
| correctChoiceId: correctChoice?.id ?? choices[0]?.id ?? "choice-1", | ||
| evaluationKeywords, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Multiple-choice items with no AI-marked correct option get an arbitrary "correct" answer.
When the model returns a multipleChoice item where no choice has isCorrect: true, correctChoiceId silently defaults to the first choice. Downstream submitAnswer grades multiple-choice by exact selectedChoiceId match, so a student picking the actually-right option would be marked wrong. Consider throwing inside the retry callback so withGeneratedTextRetry regenerates, rather than persisting a guessed correct answer.
🐛 Proposed guard
if (item.kind === "multipleChoice") {
const choices = item.choices.map((choice, choiceIndex) => ({
id: `choice-${choiceIndex + 1}`,
text: normalizeAiGeneratedGermanText(choice.text),
isCorrect: choice.isCorrect,
}));
const correctChoice = choices.find((choice) => choice.isCorrect);
+ if (!correctChoice) {
+ throw new Error(
+ "Multiple-choice task has no correct choice.",
+ );
+ }
return {
kind: "multipleChoice" as const,
title: title || `Aufgabe ${index + 1}`,
prompt,
explanation,
idealAnswer,
choices: choices.map(({ id, text }) => ({ id, text })),
- correctChoiceId: correctChoice?.id ?? choices[0]?.id ?? "choice-1",
+ correctChoiceId: correctChoice.id,
evaluationKeywords,
};
}📝 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.
| if (item.kind === "multipleChoice") { | |
| const choices = item.choices.map((choice, choiceIndex) => ({ | |
| id: `choice-${choiceIndex + 1}`, | |
| text: normalizeAiGeneratedGermanText(choice.text), | |
| isCorrect: choice.isCorrect, | |
| })); | |
| const correctChoice = choices.find((choice) => choice.isCorrect); | |
| return { | |
| kind: "multipleChoice" as const, | |
| title: title || `Aufgabe ${index + 1}`, | |
| prompt, | |
| explanation, | |
| idealAnswer, | |
| choices: choices.map(({ id, text }) => ({ id, text })), | |
| correctChoiceId: correctChoice?.id ?? choices[0]?.id ?? "choice-1", | |
| evaluationKeywords, | |
| }; | |
| if (item.kind === "multipleChoice") { | |
| const choices = item.choices.map((choice, choiceIndex) => ({ | |
| id: `choice-${choiceIndex + 1}`, | |
| text: normalizeAiGeneratedGermanText(choice.text), | |
| isCorrect: choice.isCorrect, | |
| })); | |
| const correctChoice = choices.find((choice) => choice.isCorrect); | |
| if (!correctChoice) { | |
| throw new Error( | |
| "Multiple-choice task has no correct choice.", | |
| ); | |
| } | |
| return { | |
| kind: "multipleChoice" as const, | |
| title: title || `Aufgabe ${index + 1}`, | |
| prompt, | |
| explanation, | |
| idealAnswer, | |
| choices: choices.map(({ id, text }) => ({ id, text })), | |
| correctChoiceId: correctChoice.id, | |
| evaluationKeywords, | |
| }; |
🤖 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 `@convex/learningPlanAi.ts` around lines 1430 - 1447, The multiple-choice
mapping in learningPlanAi.ts can persist a guessed correct answer when no choice
is marked isCorrect, which later breaks grading in submitAnswer. Update the
logic in the multipleChoice branch to validate that exactly one choice has
isCorrect before building the returned item, and if none is found, throw so
withGeneratedTextRetry regenerates instead of defaulting correctChoiceId to the
first option. Use the existing item.choices mapping and correctChoice lookup to
locate the fix.
| const getLearningCardSummary = (entry: DayEntry | null) => { | ||
| if (!entry) return "Keine offenen Aufgaben oder Lernblöcke für heute."; | ||
| if (entry.notes?.trim()) return formatGermanUiText(entry.notes.trim()); | ||
| if (entry.kind?.trim()) return formatGermanUiText(entry.kind.trim()); | ||
| return "Sicheres Auflösen komplexerer Gleichungen mit Klammern und Variablen auf beiden Seiten."; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Suspicious hardcoded fallback text in getLearningCardSummary.
When an entry exists but has no notes and no kind, the function falls back to a very specific, unrelated sentence about solving math equations with brackets/variables. This reads like leftover placeholder/mock copy rather than an intentional generic fallback, and will be shown to real users for any entry lacking notes/kind (e.g., freshly created exam entries without notes).
🐛 Proposed fix
const getLearningCardSummary = (entry: DayEntry | null) => {
if (!entry) return "Keine offenen Aufgaben oder Lernblöcke für heute.";
if (entry.notes?.trim()) return formatGermanUiText(entry.notes.trim());
if (entry.kind?.trim()) return formatGermanUiText(entry.kind.trim());
- return "Sicheres Auflösen komplexerer Gleichungen mit Klammern und Variablen auf beiden Seiten.";
+ return "Keine weiteren Details vorhanden.";
};📝 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.
| const getLearningCardSummary = (entry: DayEntry | null) => { | |
| if (!entry) return "Keine offenen Aufgaben oder Lernblöcke für heute."; | |
| if (entry.notes?.trim()) return formatGermanUiText(entry.notes.trim()); | |
| if (entry.kind?.trim()) return formatGermanUiText(entry.kind.trim()); | |
| return "Sicheres Auflösen komplexerer Gleichungen mit Klammern und Variablen auf beiden Seiten."; | |
| }; | |
| const getLearningCardSummary = (entry: DayEntry | null) => { | |
| if (!entry) return "Keine offenen Aufgaben oder Lernblöcke für heute."; | |
| if (entry.notes?.trim()) return formatGermanUiText(entry.notes.trim()); | |
| if (entry.kind?.trim()) return formatGermanUiText(entry.kind.trim()); | |
| return "Keine weiteren Details vorhanden."; | |
| }; |
🤖 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 `@src/app/`(app)/home.tsx around lines 390 - 395, The fallback in
getLearningCardSummary is hardcoded placeholder copy and should be replaced with
a generic user-facing message. Update the final return path in
getLearningCardSummary so that when a DayEntry has no notes and no kind, it uses
neutral fallback text consistent with the other branches rather than the
math-equation sentence, keeping the logic around formatGermanUiText,
entry.notes, and entry.kind unchanged.
# Conflicts: # app.json # docs/contexts/product/CONTEXT.md # package.json # pnpm-lock.yaml # src/app/_layout.tsx # src/app/entry/new.tsx # src/app/learning-plans/new.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/app/learning-plans/`[planId]/generating.tsx:
- Around line 87-92: The analytics call inside the promise `.then()` chain in
`generating.tsx` should be isolated from the plan-generation success flow so a
`capture("study_plan_generated", ...)` failure does not trigger the outer
`.catch()`. Update the `then((result) => ...)` block to wrap `capture` in its
own try-catch, keeping the successful generation path separate from error
handling. Use the existing `capture` call and the surrounding promise chain in
the generation logic to locate the fix.
🪄 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: c3e836d4-716b-4a9c-a72f-7615276ed4b1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.env.exampleconvex/schema.tsconvex/users.tsdocs/contexts/integrations/CONTEXT.mddocs/contexts/platform/CONTEXT.mddocs/contexts/product/CONTEXT.mdpackage.jsonsrc/app/_layout.tsxsrc/app/entry/new.tsxsrc/app/learning-plans/[planId]/generating.tsxsrc/app/learning-plans/new.tsxsrc/components/analytics-identity.tsxsrc/context/AuthContext.tsxsrc/lib/analytics.tssrc/lib/runtime-config.ts
✅ Files skipped from review due to trivial changes (3)
- docs/contexts/platform/CONTEXT.md
- package.json
- docs/contexts/integrations/CONTEXT.md
🚧 Files skipped from review as they are similar to previous changes (5)
- convex/schema.ts
- src/app/_layout.tsx
- src/context/AuthContext.tsx
- src/app/learning-plans/new.tsx
- src/app/entry/new.tsx
| .then((result) => { | ||
| capture("study_plan_generated", { | ||
| learning_plan_id: planId, | ||
| session_count: result.sessionCount, | ||
| }); | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Isolate analytics capture from the error-handling chain.
If capture throws, the error propagates to .catch(), which sets errorMessage to "Der Lernplan konnte nicht erstellt werden." even though the plan was generated successfully. This also resets didStartRef, allowing an unnecessary retry. Wrap the capture call in a try-catch to prevent analytics errors from affecting the user-facing flow.
🛡️ Proposed fix
.then((result) => {
+ try {
capture("study_plan_generated", {
learning_plan_id: planId,
session_count: result.sessionCount,
});
+ } catch {
+ // Analytics errors must not affect the generation flow
+ }
})📝 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.
| .then((result) => { | |
| capture("study_plan_generated", { | |
| learning_plan_id: planId, | |
| session_count: result.sessionCount, | |
| }); | |
| }) | |
| .then((result) => { | |
| try { | |
| capture("study_plan_generated", { | |
| learning_plan_id: planId, | |
| session_count: result.sessionCount, | |
| }); | |
| } catch { | |
| // Analytics errors must not affect the generation flow | |
| } | |
| }) |
🤖 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 `@src/app/learning-plans/`[planId]/generating.tsx around lines 87 - 92, The
analytics call inside the promise `.then()` chain in `generating.tsx` should be
isolated from the plan-generation success flow so a
`capture("study_plan_generated", ...)` failure does not trigger the outer
`.catch()`. Update the `then((result) => ...)` block to wrap `capture` in its
own try-catch, keeping the successful generation path separate from error
handling. Use the existing `capture` call and the surrounding promise chain in
the generation logic to locate the fix.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/app/`(app)/home.tsx:
- Around line 508-510: The dashboard loaded check in home.tsx is too permissive
because Object.values(entriesByDayResults).every(...) returns true for an empty
object, which can fire dashboard_viewed before auth or data are ready. Update
the isDashboardDataLoaded logic in the relevant effect to require both user and
isConvexAuthenticated, and also ensure entriesByDayResults contains a non-empty
set of resolved results before treating the dashboard as loaded. Use the
existing home.tsx effect and isDashboardDataLoaded symbol to locate and tighten
this gate.
🪄 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: 72df2896-8b68-4cd3-902a-3c191ba0a765
📒 Files selected for processing (2)
src/app/(app)/home.tsxsrc/lib/analytics.ts
| const isDashboardDataLoaded = Object.values(entriesByDayResults).every( | ||
| (result) => result !== undefined, | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '480,660p' src/app/'(app)'/home.tsxRepository: Dayova/dayova-mvp
Length of output: 6085
Gate dashboard_viewed on auth and non-empty query results
Object.values(entriesByDayResults).every(...) is true for {}, so the effect can emit dashboard_viewed with 0 entries before Convex auth resolves and then never emit again. Require user && isConvexAuthenticated and a non-empty result set before marking the dashboard loaded.
🤖 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 `@src/app/`(app)/home.tsx around lines 508 - 510, The dashboard loaded check in
home.tsx is too permissive because Object.values(entriesByDayResults).every(...)
returns true for an empty object, which can fire dashboard_viewed before auth or
data are ready. Update the isDashboardDataLoaded logic in the relevant effect to
require both user and isConvexAuthenticated, and also ensure entriesByDayResults
contains a non-empty set of resolved results before treating the dashboard as
loaded. Use the existing home.tsx effect and isDashboardDataLoaded symbol to
locate and tighten this gate.
Open work:
-[ ] Release versioning system: Complete new build if there's a new tag on GitHub
Done, not tested work:
-[ ] New learning system (#133, #131, #132, #141, #150)
-[ ] Analytics system (#144, #76, #77, #78, #79, #80, #81, #82, #83, #84)
-[ ] Fix of #128 (#143)
-[ ] New onboarding (#82, #153)
Tested work:
-[x] New design system (#147, #145, #127)
-[x] Flower fix (#139)
Nothing yet
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests