fix(core): display in-progress content attempts in chapter history tab

[syed-tp]

• In-progress attempts were hidden from history because hasAttempts was only set when completed.
• Now hasAttempts tracks any attempt record, while completed badges remain reserved for finished items.

[claude]

Claude finished @syed-tp's task in 1m 45s —— View job

---

Claude finished reviewing this PR

• Read AGENTS.md and core docs (ai_context.md, architecture.md)
• Fetch PR diff and understand changes
• Review OpenSpec / SDK boundary / architecture rules
• Review data layer & general code quality
• Post final review summary

---

PR Review Summary

✅ What's good

• Correctly identifies and documents the root cause (hasAttempts was conflated with "completed") and fixes it at the source in CurriculumParser.
• Test file updated to reflect the new expected hasAttempts/progressStatus semantics for in-progress attempts.
• No SDK boundary, design-token, or Material/Cupertino widget violations — change is confined to data-layer logic.
• This is a bug fix, so OpenSpec is not required per the exemption rules.

🔴 CRITICAL — Must fix before merge

1. hasAttempts semantics change breaks the "completed" badge for in-progress tests/assessments.
   packages/courses/lib/widgets/chapter_content_item.dart:24-28 computes:

   final isCompleted =
       lesson.progressStatus == LessonProgressStatus.completed ||
           (lesson.hasAttempts &&
               (lesson.type == LessonType.test || lesson.type == LessonType.assessment));

   This treats hasAttempts == true as a proxy for "completed" for test/assessment lessons. That was safe only because — prior to this PR — hasAttempts was set exclusively for completed attempts (the very behavior this PR removes). After this change, CurriculumParser now sets hasAttempts = true for in-progress attempts too (packages/core/lib/data/sources/curriculum_parser.dart:53-58), so an in-progress test/assessment will now incorrectly render the green checkmark "completed" badge (AppSemantics.progressValue(value: 1.0, label: examCompletedLabel, ...) at chapter_content_item.dart:124-130), directly contradicting this PR's own stated goal that "completed badges remain reserved for finished items."
   This isn't covered by any test (lesson_list_item_test.dart only exercises notStarted/hasAttempts: false cases).
   Suggested fix: base isCompleted solely on progressStatus == LessonProgressStatus.completed (drop the hasAttempts OR-clause), or explicitly special-case in-progress vs. completed rather than reusing hasAttempts.

   Fix this →

🟡 WARNING — Should fix

1. Untested behavior change in course_repository.dart. packages/courses/lib/repositories/course_repository.dart:908-922 — previously, video/live-stream lessons never had remote attempts data applied at all; now they do whenever attemptsById[dto.id] exists, and only fall back to the "reset to notStarted" behavior when absent. This is a reasonable-looking change but is a distinct behavioral shift beyond what the PR description covers ("hasAttempts tracks any attempt record...") and isn't exercised by course_repository_test.dart (no test covers _applyContentStatuses/refreshContentStatuses for video/stream lessons with a matching remote attempt). Please add a test or clarify in the PR description whether this is intentional.

🔵 SUGGESTION — Nice to have

1. Consider adding a widget test for ChapterContentItem/LessonListItem that directly asserts badge visibility for hasAttempts: true + progressStatus: inProgress across lesson types (video, test, assessment) — this is exactly the seam where the CRITICAL issue above slipped through, and a targeted test would catch future regressions here too.

Verdict

CHANGES REQUESTED

[syed-tp]

@claude review

[claude]

Claude finished @syed-tp's task in 54s —— View job

---

Claude finished reviewing this PR

• Read AGENTS.md and core docs (ai_context.md, architecture.md)
• Fetch PR diff and understand current state of changes
• Check if prior CRITICAL feedback (isCompleted badge regression) has been addressed
• Review data layer & general code quality
• Post final review summary

---

PR Review Summary

This PR has been updated since the last review round, and both previously-flagged issues have been fixed and are now covered by tests.

✅ What's good

• CRITICAL issue resolved: packages/courses/lib/widgets/chapter_content_item.dart:24 now computes isCompleted solely from lesson.progressStatus == LessonProgressStatus.completed, dropping the stale hasAttempts-based OR-clause. In-progress tests/assessments no longer render the "completed" checkmark badge. Regression tests were added in both chapter_content_item_test.dart (lines 62–86, 88–112) and lesson_list_item_test.dart (lines 70–100) covering in-progress-with-hasAttempts=true vs. truly-completed cases.
• WARNING resolved: packages/courses/lib/repositories/course_repository.dart:912-930 — the video/live-stream attempt-application behavior change is now directly tested via the new applyContentStatusesForTest (@visibleForTesting) helper and the enrichContentStatuses correctly applies attempts to video, stream, and non-video lessons test (course_repository_test.dart:133-229), which verifies video/exam lessons pick up remote attempt data and non-video/stream lessons without a matching remote attempt reset to notStarted.
• packages/core/lib/data/sources/curriculum_parser.dart:23-58 cleanly separates "has an attempt at all" (attemptedMap) from "has a completed attempt" (completedAttempts), and correctly derives inProgress vs notStarted vs completed progress status. Matching test updates in curriculum_parser_test.dart verify hasAttempts=true/progressStatus=inProgress for the in-progress lesson and completed for the finished one.
• No SDK boundary, design-token, or Material/Cupertino widget violations — changes remain confined to data-layer logic and a single derived boolean in the widget layer.
• This is a bug fix, so OpenSpec is not required per the exemption rules.

🔴 CRITICAL — Must fix before merge

None.

🟡 WARNING — Should fix

None.

🔵 SUGGESTION — Nice to have

1. applyContentStatusesForTest (course_repository.dart:895-904) is a production-code, @visibleForTesting pass-through added purely to expose the private _applyContentStatuses for unit testing. This is a reasonable and common pattern for testing private business logic, but consider whether an existing public entry point (e.g. driving the test through refreshContentStatuses/streamFilteredContents) could achieve the same coverage without adding a test-only method to the public API surface. Not blocking.

Verdict

APPROVED