[khashyap0803]

Round 4 of the audit, branched from master at e68d911 (after #4 was merged). Same rules as before: one commit per fix, a JVM unit test for each fix, explicit git add paths, no amend or force-push. Test names avoid " : * ? < > |.

This PR leaves the security-rules PR (#3) alone. It does not touch firestore.rules, database.rules.json, firebase-rules-tests/ or the docs that PR changes.

Contents: 5 fixes, one commit each, in the order you gave them. For each one I reverted just the fix's logic (or, for fix 5, put master's files back), re-ran its tests, and confirmed they fail.

A correction to round 3. Nothing in the app calls AnalyticsService, and nothing calls FlashcardService.getAllCardsNeedingReview. No screen shows analytics today. In #4 I described the Analytics bug as something a student saw ("Analytics showed 0 completed"). That was wrong: it was service output with no screen behind it. Fixes 1 and 4 below still correct that service, but no student sees the difference until a screen uses it.

1. Changes

#	Where (this branch)	Bug	What a student experienced	Fix	Test
1	services/FirestoreManager.kt:62, :136, :368, :985; callers: ui/practice/PracticeContentActivity.kt:105, :145, :199; PracticeFragment.kt:107; services/ProjectSuggestionService.kt:58; DailyTipService.kt:41, :49, :92; FlashcardService.kt:236; AnalyticsService.kt:111, :410	Every caller of getStagesWithProgress() turned a failed read into an empty list, and an empty list means "no path yet".	On a network or permission error: Practice flashcards said "Complete your assessment first". Interview prep gave generic questions. Projects gave beginner projects. The daily tip was generic and was stored as the whole day's tip.	FirestoreManager.readPath() returns PathRead: Path, NoPath or Failed. Every caller switches on it. NoPath keeps its existing behaviour (the assessment prompt, generic interview topics, beginner projects). Failed shows "Couldn't load your progress. Check your connection and try again." Practice flashcards, interview prep and projects show it in the screen's empty state, and pull-to-refresh retries. Quick Practice shows it as a toast, and tapping the button again retries. Daily tips: on a failed read the student gets today's generic tip, and it is not stored, so the next visit asks again. Analytics and projects report the message through onError. getAllCardsNeedingReview and getQuickStats now return a failure. getQuickStats now returns Result<LearningAnalytics>; nothing calls it.	FailedPathReadTest (8)
2	services/FirestoreManager.kt:59, :125, :376, :995; UserProgressSyncService.kt:92; StageGeneratorService.kt:35, :365, :609	A retake reset the stage progress in one write, then saved the stages in another.	If the reset succeeded and the save failed, the student kept the old path with its progress wiped.	replacePathForRetake puts all of it in one WriteBatch, planned by retakeWrites, so either all of it happens or none of it does: Reset the stage fields of the progress document. Delete the old stages the new path doesn't reuse. A set without merge overwrites a reused id. Save the new stages. The reset now writes only currentStage, completedStages, stageStars and lastUpdated, so it needs no read first and never touches XP or the streak. That makes resetStageProgressInCloud unused, so it is removed. Any failure still shows "Couldn't start your retake. Please try again." with Try again.	RetakeReplaceTest (4)
3	ui/learningpath/LearningPathFragment.kt:143, :283, :294, :374; res/layout/fragment_learning_path.xml; res/values/strings.xml	When the saved stages (or the check for a report) could not be read, the screen showed a toast.	An empty screen, and no way to retry without leaving.	It now shows the existing failed state, worded for loading: "We couldn't load your learning path" / "Check your connection and try again. Your progress is safe.", with Retry. The build failure keeps its own wording. PathFailure (LOAD, BUILD) carries the two wordings. A confirmed-missing report still goes to the assessment.	PathFailureTest (3)
4	services/AnalyticsService.kt:42, :50, :124, :416	The streak, and with it the learning pace and the completion estimate, came from users/{u}/progress/current. Nothing writes that document.	The streak was always 0 and the pace always "slow" (no screen shows them, see above).	Reads streak and bestStreak from users/{u}/data/progress, the document Home's Study Streak card reads and saveStreak (PR #2) keeps. It goes through UserProgressSyncService.loadProgressResult(), and uses Home's default of 1 when there is no document yet. A failed read gives "Couldn't load your progress…", not a streak of 0. How the streak is written is unchanged. XP still comes from where it did (see table 2).	AnalyticsStreakTest (4)
5	ui/challenge/Challenge1Activity.kt:96, :146; Challenge2Activity.kt:74, :118; Challenge3Activity.kt:76, :134; services/PreReleaseEventService.kt (6 lines)	12 Log.i/Log.w calls, which R8 keeps in release builds, included the participant's roll number. I checked every Log.w, Log.e and Log.i in the app, multi-line ones included. None include a username or email, and no exception message passed to them is built from one.	Roll numbers were readable in release device logs.	Each message is kept, with the roll number removed. Log.d and Log.v are untouched, and no logging was added.	ReleaseLogIdentifierTest (2)

Fix 2: the batch size

• Largest path: 100 stages. The model is asked for 40 stages, but the parser kept every stage object it was given. Even {} parsed as a stage, so in principle one 8192-token reply could have produced thousands. parseStagesFromResponse now keeps at most MAX_STAGES = 100.
• Largest batch: 201 writes. That is 1 reset + 100 deletes + 100 saves, for a 100-stage old path whose ids the new path reuses none of. Firestore's limit is 500. A normal 40-stage retake with ids 1…40 on both sides is 41 writes, because reused ids are overwritten rather than deleted.
• Paths saved before this change aren't capped. The batch is checked against 500 before commit. A stored path would need more than 399 stage documents whose ids the new path doesn't reuse to go over. In that case nothing is written and the student sees "Couldn't start your retake".

On the tests

• What the tests cover. The tests call the real decision functions (pathReadOf, retakeWrites, streakFrom, paceFor, PathFailure) and the parser.
  • The service-level checks in FailedPathReadTest run the real getTodaysTip, generateProjectSuggestions, getAllCardsNeedingReview, generateAnalytics and getQuickStats against a FirestoreManager built without its constructor. That object has no SharedPreferences, so every read throws inside its own try and comes back as a failed Result, the same path a network or permission error takes.
  • The Activity and Fragment branches need Robolectric to exercise, so I checked them by reading the code. That covers the practice screens' empty state, the Learning Path failed state and the retake screen. So for fix 3, PathFailureTest checks the wording that showPathFailed uses, not the branch that calls it.
• Two tests read source files: PathFailureTest reads strings.xml, and ReleaseLogIdentifierTest scans src/main/java. Both resolve paths from the module directory, which is where Gradle runs unit tests.
• Existing tests changed:
  • RetakeReplaceTest is rewritten, because round 3's three-write seam it tested is gone.
  • DailyTipFallbackTest reads .tips from generateTips()'s new return type.
• No device verification. Nothing here was run on a phone or emulator.

2. Found but not fixed

Sev	Where	Issue	Suggested fix
Medium	services/UserProgressSyncService.kt loadProgressFromCloud; ui/home/HomeFragment.kt:140	Same problem as fix 1, but on Home, which doesn't go through getStagesWithProgress(). loadProgressFromCloud returns null on a failed read, and Home treats null as a new student: 0 XP, streak 1.	Use loadProgressResult() and show the connection message on Failed.
Low	services/AnalyticsService.kt:132, :421	Analytics XP still comes from getProgress(), which reads the never-written users/{u}/progress/current. generateAnalytics falls back to the sum of completed stages' rewards, and getQuickStats to 0. Fix 4 already reads the right document.	Take totalXP from the same read.
Low	AnalyticsService, FlashcardService.getAllCardsNeedingReview	No callers, so fixes 1 and 4 can't be seen in the app, and there is no screen to put a retry on.	Wire them to a screen, or remove them.
Low	Streak source (Home and Analytics)	The stored streak only changes when Learning Path is opened. After days away, both show the old streak until then.	Recompute it on read from lastVisitDate.
Low	services/FirestoreManager.kt:558	deleteAllPersonalizedStages deletes from the wrong path, and after fix 2 nothing calls it.	Remove it.
Low	services/GeminiQuizService.kt parseQuizResponse, GeminiChallengeService.kt (Cleaned response)	Log.e writes up to 500 characters (or all) of the model's reply to release logs. That is not an identifier, but it is model text.	Log.d, or log only the length.
Low	Earlier rounds	Rotation and back-out show "…cancelled": LoginActivity.kt:518, :589; QuizActivity.kt:183. Catch-alls swallow cancellation: GeminiQuizService.kt:204, StageContentService.kt:151, others in FirestoreManager. !! in LoginActivity. Unused GEMINI_API_KEY field. The test harness's C:/ path.	As in #4.

Out of scope as you asked, so not started: individual contestant sign-in, a server-side username lookup, the event accounts, partly generated quizzes, and rotation state on the retake screen.

3. Verification

./gradlew :app:testDebugUnitTest :app:compileReleaseKotlin
BUILD SUCCESSFUL in 59s
51 actionable tasks: 51 executed

Run on this branch's head after ./gradlew clean. JUnit XML totals: 205 tests, 0 failures, 0 errors, 0 skipped across 37 classes. That is master's 187, plus 17 in four new classes, plus 1 more in the rewritten RetakeReplaceTest (3 → 4):

Test class	Tests
FailedPathReadTest (new)	8
RetakeReplaceTest (rewritten)	4
PathFailureTest (new)	3
AnalyticsStreakTest (new)	4
ReleaseLogIdentifierTest (new)	2

Kotlin warnings: 20, the same as master (10 deprecation warnings, each reported for both the debug and release compiles). None are new. compileReleaseKotlin passes.

Dependencies came from the repositories in settings.gradle.kts, with no mirror; they were already cached.

---

Generated by Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.