Skip to content

Commit 10900e3

Browse files
khashyap0803claude
andcommitted
Merge pull request #5 from khashyap0803/claude/audit-2026-10-r4
Audit round 4: a failed progress read is told apart from having no learning path, a retake resets progress and replaces the stages in one WriteBatch, Learning Path shows Retry when loading fails, Analytics reads the streak Home uses, and release logs no longer contain roll numbers. Verified locally: 205 unit tests, 0 failures; release build tested on the emulator (Learning Path, flashcards, project challenges). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 parents e68d911 + c5815e3 commit 10900e3

23 files changed

Lines changed: 618 additions & 203 deletions

‎app/src/main/java/com/example/embeddedsystemscareerguide/services/AnalyticsService.kt‎

Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,37 @@ class AnalyticsService(private val context: Context) {
3131
instance ?: AnalyticsService(context.applicationContext).also { instance = it }
3232
}
3333
}
34+
35+
/**
36+
* The streak as Home's Study Streak card shows it: from the progress
37+
* document (users/{username}/data/progress) that LearningPathFragment's
38+
* saveStreak keeps, with the same defaults when there is none yet. Null
39+
* when the read failed. Analytics used to read users/{username}/progress/
40+
* current, which nothing writes, so the streak and pace were always 0.
41+
*/
42+
internal fun streakFrom(load: UserProgressSyncService.ProgressLoad): Streak? = when (load) {
43+
is UserProgressSyncService.ProgressLoad.Loaded -> Streak(load.progress.streak, load.progress.bestStreak)
44+
UserProgressSyncService.ProgressLoad.Absent ->
45+
UserProgressSyncService.UserProgress().let { Streak(it.streak, it.bestStreak) }
46+
is UserProgressSyncService.ProgressLoad.Failed -> null
47+
}
48+
49+
/** Learning pace from the current streak. */
50+
internal fun paceFor(streak: Int): String = when {
51+
streak >= 5 -> "fast"
52+
streak >= 3 -> "steady"
53+
else -> "slow"
54+
}
3455
}
3556

3657
private val geminiService = OllamaService.getInstance(context)
3758
private val firestoreManager = FirestoreManager.getInstance(context)
59+
private val progressSyncService = UserProgressSyncService(context)
3860
private val gson = Gson()
3961

62+
/** The streak figures analytics uses. */
63+
internal data class Streak(val current: Int, val best: Int)
64+
4065
/**
4166
* Learning analytics summary
4267
*/
@@ -83,11 +108,21 @@ class AnalyticsService(private val context: Context) {
83108
callback.onProgress("Analyzing your progress...")
84109

85110
// Collect data
86-
val stages = firestoreManager.getStagesWithProgress().getOrNull() ?: emptyList()
111+
val stages = when (val read = firestoreManager.readPath()) {
112+
is PathRead.Path -> read.stages
113+
PathRead.NoPath -> {
114+
callback.onError("No learning path found. Please complete the assessment first.")
115+
return@withContext
116+
}
117+
is PathRead.Failed -> {
118+
Log.w(TAG, "Could not read the learning path", read.cause)
119+
callback.onError(FirestoreManager.PROGRESS_LOAD_FAILED_MESSAGE)
120+
return@withContext
121+
}
122+
}
87123
val progress = firestoreManager.getProgress().getOrNull()
88-
89-
if (stages.isEmpty()) {
90-
callback.onError("No learning path found. Please complete the assessment first.")
124+
val streak = streakFrom(progressSyncService.loadProgressResult()) ?: run {
125+
callback.onError(FirestoreManager.PROGRESS_LOAD_FAILED_MESSAGE)
91126
return@withContext
92127
}
93128

@@ -108,19 +143,19 @@ class AnalyticsService(private val context: Context) {
108143
val (strongTopics, weakTopics) = categorizeTopics(stages)
109144

110145
// Estimate completion time
111-
val estimatedDays = estimateCompletionDays(stages, completedStages, progress)
146+
val estimatedDays = estimateCompletionDays(stages, completedStages, streak.current)
112147

113148
// Determine learning pace
114-
val learningPace = determinePace(progress)
149+
val learningPace = paceFor(streak.current)
115150

116151
val analytics = LearningAnalytics(
117152
totalXP = totalXP,
118153
totalStages = totalStages,
119154
completedStages = completedStages,
120155
averageStars = averageStars,
121156
totalTimeMinutes = 0, // Would need to track this separately
122-
currentStreak = progress?.currentStreak ?: 0,
123-
longestStreak = progress?.longestStreak ?: 0,
157+
currentStreak = streak.current,
158+
longestStreak = streak.best,
124159
strongTopics = strongTopics,
125160
weakTopics = weakTopics,
126161
progressPercentage = progressPercentage,
@@ -196,31 +231,20 @@ class AnalyticsService(private val context: Context) {
196231
private fun estimateCompletionDays(
197232
stages: List<PersonalizedStage>,
198233
completedStages: Int,
199-
progress: UserProgress?
234+
currentStreak: Int
200235
): Int {
201236
val remainingStages = stages.size - completedStages
202237

203238
// Assume average of 1-2 stages per day based on pace
204239
val stagesPerDay = when {
205240
completedStages == 0 -> 1
206-
progress?.currentStreak ?: 0 >= 7 -> 2
241+
currentStreak >= 7 -> 2
207242
else -> 1
208243
}
209244

210245
return (remainingStages / stagesPerDay.coerceAtLeast(1)).coerceAtLeast(1)
211246
}
212247

213-
/**
214-
* Determine learning pace based on activity
215-
*/
216-
private fun determinePace(progress: UserProgress?): String {
217-
return when {
218-
progress?.currentStreak ?: 0 >= 5 -> "fast"
219-
progress?.currentStreak ?: 0 >= 3 -> "steady"
220-
else -> "slow"
221-
}
222-
}
223-
224248
/**
225249
* Generate AI-powered recommendations
226250
*/
@@ -379,12 +403,18 @@ class AnalyticsService(private val context: Context) {
379403
}
380404

381405
/**
382-
* Get quick stats for dashboard display
406+
* Get quick stats for dashboard display. A failed read is a failure, not zeros.
383407
*/
384-
suspend fun getQuickStats(): LearningAnalytics = withContext(Dispatchers.IO) {
408+
suspend fun getQuickStats(): Result<LearningAnalytics> = withContext(Dispatchers.IO) {
385409
try {
386-
val stages = firestoreManager.getStagesWithProgress().getOrNull() ?: emptyList()
410+
val stages = when (val read = firestoreManager.readPath()) {
411+
is PathRead.Path -> read.stages
412+
PathRead.NoPath -> emptyList()
413+
is PathRead.Failed -> return@withContext Result.failure(read.cause)
414+
}
387415
val progress = firestoreManager.getProgress().getOrNull()
416+
val streak = streakFrom(progressSyncService.loadProgressResult())
417+
?: return@withContext Result.failure(IllegalStateException(FirestoreManager.PROGRESS_LOAD_FAILED_MESSAGE))
388418

389419
val completedStages = stages.count { it.isCompleted }
390420
val totalStages = stages.size
@@ -393,18 +423,18 @@ class AnalyticsService(private val context: Context) {
393423
stages.sumOf { it.starsEarned }.toFloat() / completedStages
394424
} else 0f
395425

396-
LearningAnalytics(
426+
Result.success(LearningAnalytics(
397427
totalXP = totalXP,
398428
totalStages = totalStages,
399429
completedStages = completedStages,
400430
averageStars = averageStars,
401-
currentStreak = progress?.currentStreak ?: 0,
402-
longestStreak = progress?.longestStreak ?: 0,
431+
currentStreak = streak.current,
432+
longestStreak = streak.best,
403433
progressPercentage = (completedStages * 100) / totalStages.coerceAtLeast(1)
404-
)
434+
))
405435
} catch (e: Exception) {
406436
Log.e(TAG, "Error getting quick stats", e)
407-
LearningAnalytics()
437+
Result.failure(e)
408438
}
409439
}
410440
}

‎app/src/main/java/com/example/embeddedsystemscareerguide/services/DailyTipService.kt‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import android.content.Context
44
import android.util.Log
55
import com.google.gson.Gson
66
import com.google.gson.JsonObject
7+
import kotlinx.coroutines.CancellationException
78
import kotlinx.coroutines.Dispatchers
89
import kotlinx.coroutines.withContext
910

@@ -36,10 +37,16 @@ class DailyTipService(private val context: Context) {
3637
private val firestoreManager = FirestoreManager.getInstance(context)
3738
private val gson = Gson()
3839

40+
/** Tips from [generateTips], and whether [getTodaysTip] may store them as the day's tips. */
41+
internal data class Tips(val tips: List<DailyTip>, val store: Boolean)
42+
3943
/**
4044
* Get today's tip, generating new ones if needed
4145
*/
42-
suspend fun getTodaysTip(): Result<DailyTip> = withContext(Dispatchers.IO) {
46+
suspend fun getTodaysTip(): Result<DailyTip> = getTodaysTip(save = { firestoreManager.saveDailyTip(it) })
47+
48+
/** [getTodaysTip] with the write passed in. */
49+
internal suspend fun getTodaysTip(save: suspend (DailyTip) -> Unit): Result<DailyTip> = withContext(Dispatchers.IO) {
4350
try {
4451
val todayDate = getTodayDateString()
4552

@@ -55,17 +62,19 @@ class DailyTipService(private val context: Context) {
5562
Log.d(TAG, "No tip for today, generating new tips")
5663
val newTips = generateTips()
5764

58-
if (newTips.isNotEmpty()) {
65+
if (newTips.tips.isNotEmpty()) {
5966
// Save all generated tips
60-
newTips.forEach { tip ->
61-
firestoreManager.saveDailyTip(tip)
67+
if (newTips.store) {
68+
newTips.tips.forEach { tip ->
69+
save(tip)
70+
}
6271
}
63-
return@withContext Result.success(newTips.first())
72+
return@withContext Result.success(newTips.tips.first())
6473
}
6574

6675
// Fallback tip
6776
val fallbackTip = createFallbackTip(todayDate)
68-
firestoreManager.saveDailyTip(fallbackTip)
77+
save(fallbackTip)
6978
Result.success(fallbackTip)
7079

7180
} catch (e: Exception) {
@@ -77,10 +86,19 @@ class DailyTipService(private val context: Context) {
7786
/**
7887
* Generate tips based on user's current progress
7988
*/
80-
internal suspend fun generateTips(): List<DailyTip> {
89+
internal suspend fun generateTips(): Tips {
8190
try {
8291
// Get user context for personalization
83-
val stages = firestoreManager.getStagesWithProgress().getOrNull() ?: emptyList()
92+
val stages = when (val read = firestoreManager.readPath()) {
93+
is PathRead.Path -> read.stages
94+
PathRead.NoPath -> emptyList()
95+
is PathRead.Failed -> {
96+
// A stored tip is served all day without asking again, and this one
97+
// knows nothing about where the student is - so show it, don't store it.
98+
Log.w(TAG, "Could not read the learning path; using a generic tip for now", read.cause)
99+
return Tips(createFallbackTips(), store = false)
100+
}
101+
}
84102
val currentStage = FirestoreManager.currentStageOf(stages)
85103
val completedTopics = stages.filter { it.isCompleted }.map { it.title }
86104

@@ -94,15 +112,17 @@ class DailyTipService(private val context: Context) {
94112

95113
if (result.isFailure) {
96114
Log.e(TAG, "API call failed: ${result.exceptionOrNull()?.message}")
97-
return createFallbackTips()
115+
return Tips(createFallbackTips(), store = true)
98116
}
99117

100118
val responseText = result.getOrNull() ?: ""
101-
return parseTipsFromResponse(responseText)
119+
return Tips(parseTipsFromResponse(responseText), store = true)
102120

121+
} catch (e: CancellationException) {
122+
throw e
103123
} catch (e: Exception) {
104124
Log.e(TAG, "Error generating tips", e)
105-
return createFallbackTips()
125+
return Tips(createFallbackTips(), store = true)
106126
}
107127
}
108128

‎app/src/main/java/com/example/embeddedsystemscareerguide/services/FirestoreManager.kt‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ class FirestoreManager private constructor(private val context: Context) {
5454
const val COLLECTION_PROJECTS = "projects"
5555
const val COLLECTION_INTERVIEW_PREP = "interview_prep"
5656
const val COLLECTION_DAILY_TIPS = "daily_tips"
57+
58+
/** Firestore's limit on the writes in one WriteBatch. */
59+
internal const val MAX_BATCH_WRITES = 500
60+
61+
/** Shown wherever the student's path or progress could not be read. */
62+
const val PROGRESS_LOAD_FAILED_MESSAGE = "Couldn't load your progress. Check your connection and try again."
5763

5864
// Singleton instance
5965
@Volatile
@@ -105,6 +111,37 @@ class FirestoreManager private constructor(private val context: Context) {
105111
internal fun currentStageOf(stages: List<PersonalizedStage>): PersonalizedStage? =
106112
stages.firstOrNull { !it.isCompleted } ?: stages.lastOrNull()
107113

114+
/**
115+
* Keeps a failed read apart from an empty path. Callers used to turn a failure
116+
* into an empty list, so a network or permission error told the student to
117+
* take the assessment.
118+
*/
119+
/**
120+
* Every write a retake makes, for one WriteBatch: reset the stage progress,
121+
* delete the old stages the new path doesn't reuse, save the new stages. A set
122+
* without merge replaces the whole document, so an old stage whose id comes
123+
* back is overwritten rather than deleted first.
124+
*/
125+
internal fun retakeWrites(
126+
oldStageIds: List<String>,
127+
newStages: List<PersonalizedStage>,
128+
now: Long
129+
): List<RetakeWrite> {
130+
val newIds = newStages.map { it.id.toString() }.toSet()
131+
return listOf<RetakeWrite>(RetakeWrite.ResetProgress(UserProgressSyncService.stageResetFields(now))) +
132+
oldStageIds.filter { it !in newIds }.map { RetakeWrite.DeleteStage(it) } +
133+
newStages.map { RetakeWrite.SaveStage(it) }
134+
}
135+
136+
internal fun pathReadOf(read: Result<List<PersonalizedStage>>): PathRead {
137+
val stages = read.getOrElse {
138+
// getPersonalizedStages hands a cancellation back as a failed Result.
139+
if (it is CancellationException) throw it
140+
return PathRead.Failed(it)
141+
}
142+
return if (stages.isEmpty()) PathRead.NoPath else PathRead.Path(stages)
143+
}
144+
108145
/**
109146
* [getStagesWithProgress] with its two reads passed in. A missing progress
110147
* document (nothing completed yet) is an empty map; a failed read is a failure,
@@ -327,6 +364,45 @@ class FirestoreManager private constructor(private val context: Context) {
327364
}
328365
)
329366

367+
/** [getStagesWithProgress], with "no path yet" and "couldn't read" kept apart. */
368+
internal suspend fun readPath(): PathRead = pathReadOf(getStagesWithProgress())
369+
370+
/**
371+
* A retake's path swap as one WriteBatch ([retakeWrites]), so Firestore applies
372+
* all of it or none of it. The reset and the save used to be separate writes,
373+
* and a failed save after a successful reset left the old path with its
374+
* progress wiped.
375+
*/
376+
suspend fun replacePathForRetake(stages: List<PersonalizedStage>): Result<Unit> = withContext(Dispatchers.IO) {
377+
try {
378+
val username = getCurrentUsername()
379+
val stagesRef = getStagesDocRef(username).collection(COLLECTION_PERSONALIZED_STAGES)
380+
val progressRef = getUserDocRef(username).collection(COLLECTION_DATA).document("progress")
381+
val oldIds = stagesRef.get().await().documents.map { it.id }
382+
val writes = retakeWrites(oldIds, stages, System.currentTimeMillis())
383+
check(writes.size <= MAX_BATCH_WRITES) {
384+
"A retake needs ${writes.size} writes; one batch holds $MAX_BATCH_WRITES"
385+
}
386+
387+
val batch = firestore.batch()
388+
writes.forEach { write ->
389+
when (write) {
390+
is RetakeWrite.ResetProgress -> batch.set(progressRef, write.fields, SetOptions.merge())
391+
is RetakeWrite.DeleteStage -> batch.delete(stagesRef.document(write.stageId))
392+
is RetakeWrite.SaveStage -> batch.set(stagesRef.document(write.stage.id.toString()), write.stage)
393+
}
394+
}
395+
batch.commit().await()
396+
Log.d(TAG, "Retake path saved: ${stages.size} stages in ${writes.size} writes")
397+
Result.success(Unit)
398+
} catch (e: CancellationException) {
399+
throw e
400+
} catch (e: Exception) {
401+
Log.e(TAG, "Error replacing the path for a retake", e)
402+
Result.failure(e)
403+
}
404+
}
405+
330406
/**
331407
* Check if user has personalized stages
332408
*/
@@ -905,6 +981,23 @@ data class UserProfile(
905981
val assessmentScore: Int = 0
906982
)
907983

984+
/** What [FirestoreManager.readPath] found. */
985+
internal sealed class PathRead {
986+
/** The student's path, with completion from the progress document. Never empty. */
987+
data class Path(val stages: List<PersonalizedStage>) : PathRead()
988+
/** The read succeeded and there is no path yet: the student takes the assessment. */
989+
object NoPath : PathRead()
990+
/** The read failed. Not a missing path: never send the student to the assessment for it. */
991+
data class Failed(val cause: Throwable) : PathRead()
992+
}
993+
994+
/** One write in [FirestoreManager.retakeWrites]. */
995+
internal sealed class RetakeWrite {
996+
data class ResetProgress(val fields: Map<String, Any>) : RetakeWrite()
997+
data class DeleteStage(val stageId: String) : RetakeWrite()
998+
data class SaveStage(val stage: PersonalizedStage) : RetakeWrite()
999+
}
1000+
9081001
/**
9091002
* Personalized learning stage
9101003
*/

‎app/src/main/java/com/example/embeddedsystemscareerguide/services/FlashcardService.kt‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,11 @@ class FlashcardService(private val context: Context) {
233233
suspend fun getAllCardsNeedingReview(): Result<Map<Int, List<Flashcard>>> =
234234
withContext(Dispatchers.IO) {
235235
try {
236-
val stages = firestoreManager.getStagesWithProgress().getOrNull() ?: emptyList()
236+
val stages = when (val read = firestoreManager.readPath()) {
237+
is PathRead.Path -> read.stages
238+
PathRead.NoPath -> emptyList()
239+
is PathRead.Failed -> return@withContext Result.failure(read.cause)
240+
}
237241
val reviewMap = mutableMapOf<Int, List<Flashcard>>()
238242

239243
stages.filter { it.isCompleted }.forEach { stage ->

0 commit comments

Comments
 (0)