Fix active workout video page

[9thLevelSoftware]

Summary

• Restore the active-workout HUD video page between Metrics and Stats.
• Forward ExerciseRepository and enableVideoPlayback from WorkoutTab into WorkoutHud.
• Add a source-level guard so the active HUD keeps the 3-page video wiring.

Fixes #643

Verification

• Red check: :shared:testAndroidHostTest --tests com.devil.phoenixproject.presentation.WorkoutHudVideoWiringTest failed with the HUD video wiring reverted (2 failing assertions).
• Green targeted: JAVA_HOME=/opt/homebrew/opt/openjdk@17 ANDROID_HOME=/opt/homebrew/share/android-commandlinetools ./gradlew :shared:testAndroidHostTest --tests 'com.devil.phoenixproject.presentation.WorkoutHudVideoWiringTest' -Pskip.supabase.check=true
• Green full shared gate: JAVA_HOME=/opt/homebrew/opt/openjdk@17 ANDROID_HOME=/opt/homebrew/share/android-commandlinetools ./gradlew :shared:allTests -Pskip.supabase.check=true

[gemini-code-assist]

Code Review

This pull request introduces an instruction page with video playback capabilities into the active-workout HUD, expanding the pager from two to three pages. It forwards the necessary repository and settings from WorkoutTab and adds a source-level guard test. The review feedback highlights two main areas of improvement: first, keying the remember states in InstructionPage on both currentExerciseIndex and exerciseId to prevent potential UI state mismatches; second, avoiding the practice of reading and parsing physical source files from the filesystem within unit tests to prevent environment-dependent fragility.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[kilo-code-bot]

Code Review Roast 🔥

Verdict: No Issues Found | Recommendation: Merge

The previous test-guard finding is fixed in af2e804: the assertion now independently checks both var videoEntity by remember(currentExerciseIndex, exerciseId) and var isLoading by remember(currentExerciseIndex, exerciseId) declarations, so either state holder regressing to incomplete keys fails the guard.

🏆 Best part: One focused diff, one targeted fix, zero new sins introduced. The runtime wiring from the prior commit is now actually defended by the test.

💀 Worst part: Nothing new to roast. I came with a spatula and the kitchen is empty.

📊 Overall: A bouncer who now checks both IDs. Acceptable.

Files Reviewed (1 file)

• shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/WorkoutHudVideoWiringTest.kt - 0 issues

Correctness / Safety Findings

No correctness or safety findings.

Ponytail Review

Ponytail: Lean already. Ship.
Ponytail net: 0 lines.

Suggested Minimal Patch

No patch needed.

Final Merge Guidance

Can merge as-is. The previously flagged test-coverage gap is closed; no further issues on the incremental diff.

Previous Review Summaries (2 snapshots, latest commit 47afe3b)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 47afe3b)

Verdict: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
🚨 critical	0
⚠️ warning	0
💡 suggestion	1
🤏 nitpick	0

Issue Details (click to expand)

File	Line	Roast
shared/.../WorkoutHudVideoWiringTest.kt	38	The new guard only proves one remember call has the fixed keys, so the second state holder can regress while the test smiles and waves.

---

🏆 Best part: The runtime fix is annoyingly reasonable now: selected-exercise fallback, matching remember keys, and actual logging instead of the previous cone of silence.

💀 Worst part: The new regression guard checks one shoelace and declares the whole marathon-ready. Tests that only catch half the bug are how gremlins get tenure.

📊 Overall: The code path is fixed; the guard just needs to stop being a cardboard security camera.

Files Reviewed (2 files)

• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/WorkoutHud.kt - 0 issues
• shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/WorkoutHudVideoWiringTest.kt - 1 issue

Correctness / Safety Findings

minor: shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/WorkoutHudVideoWiringTest.kt:L38: The source guard checks only that one remember(currentExerciseIndex, exerciseId) occurrence exists, so isLoading can lose the fixed keys without failing the test. Require exact checks for both videoEntity and isLoading.

Ponytail Review

Ponytail: Lean already. Ship.
Ponytail net: 0 lines.

Suggested Minimal Patch

Tighten the existing assertion in WorkoutHudVideoWiringTest.kt to check both state holders use remember(currentExerciseIndex, exerciseId).

Final Merge Guidance

Can merge after the test guard is tightened; the runtime fix itself looks clean.

---

Fix these issues in Kilo Cloud

Previous review (commit 9052077)

Verdict: 1 Issue Found | Recommendation: Approve with optional nitpick

Overview

Severity	Count
🚨 critical	0
⚠️ warning	0
💡 suggestion	0
🤏 nitpick	1

Issue Details (click to expand)

File	Line	Roast
shared/.../screen/WorkoutHud.kt	728	catch (_: Exception) with a shrug comment — the sibling loader in WorkoutTab.kt:1759 actually logs via kermit. Future-you is going to be reading tea leaves.

---

🏆 Best part: Re-adding the 3-page pager wiring with proper parameter forwarding and a source-level guard test that explicitly documents the contract — that's an actual regression break-glass, not vibes.

💀 Worst part: The try { … } catch (_: Exception) { /* stays null */ } block swallowing every failure mode without a single Logger.e. In a workout HUD, a silent black video panel will absolutely become a "video doesn't work" support ticket that no one can debug.

📊 Overall: A focused, mostly surgical fix that puts back what was lost. It's like watching someone reinstall a wheel they stole off their own car — competent, just don't lose it again.

Correctness / Safety Findings

No correctness or safety findings.

Ponytail Review

shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/WorkoutHud.kt:L727-733: reuse: Same currentExerciseIndex-keyed videoEntity by remember + LaunchedEffect + getVideos(...).firstOrNull() pattern already lives in WorkoutTab.kt:1741-1762. Extract a tiny rememberExerciseVideo(exerciseRepository, exerciseId) composable and call it from both sites so future video-loading tweaks happen in one place.

Ponytail net: ~-10 lines (plus the silent catch becomes logged in one spot).

Suggested Minimal Patch

No patch is required. The single nitpick about logging on video-load failure is the only actionable item and is non-blocking.

Final Merge Guidance

Can merge as-is. Logging the swallowed exception is optional cleanup; the PR resolves issue #643 with the right files, the right pager wiring, and a deliberately strict source-level guard test.

---

Fix these issues in Kilo Cloud

---

_{Reviewed by minimax-m3 · Input: 27.4K · Output: 3.3K · Cached: 181.1K}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9052077e75

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".