fix(it3): address CodeRabbit review — double-submit guard, bounded loop, type annotation#72
Merged
Merged
Conversation
…op, type annotation - WorkoutSession.tsx: add isFinishing guard to handleFinalizarTreino; button disabled while in-flight to prevent duplicate saves/XP on rapid clicks - workout-session.spec.ts: scope iniciarButton to 'Treino E2E' row via filter; add MAX_EXERCISES=20 guard to while loop for deterministic failure on regression - workout-feedback-flow.test.ts: type-annotate fallback shape as WorkoutFeedbackOutput - CHANGELOG.md: consolidate two [Unreleased] sections into one - CURRENT-STATE.md: fix version header to reflect completed state Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Changelog & Docs CHANGELOG.md, docs/CURRENT-STATE.md |
Replaced Unreleased heading/date; removed Unreleased Sentry production wiring section; updated branch/version to main (v0.6.0 tagged) and CI/E2E status. |
Workout Session UI src/components/WorkoutSession.tsx |
Added isFinishing state; handleFinalizarTreino short-circuits when finishing, sets/resets isFinishing, and disables the "Finalizar Treino" button while in-flight. |
E2E Tests tests/e2e/specs/workout-session.spec.ts |
Scoped iniciar button selection to the div.rounded-lg row containing Treino E2E; added MAX_EXERCISES = 20 loop guard and error on exceeding it. |
Test Type Safety src/ai/flows/workout-feedback-flow.test.ts |
Added a type-only import WorkoutFeedbackOutput and annotated the fallback fixture to enforce the expected contract at compile time. |
Tooling Config .coderabbit.yaml |
Added repository configuration: language en-US, auto-review mode enabled (drafts disabled), review profile defaults and TypeScript docstring generation disabled. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- feat(it3): AI workout feedback + E2E coverage for workout session and enrollment #71 — Direct follow-up to PR
#71that originally introduced the workout finish flow and related E2E/tests; this PR applies fixes to the same areas.
Poem
🐰 I bounced through changelogs and tests tonight,
Guarded the finish with a soft, fluffy bite,
Scoped the buttons, capped the loop's spree,
Typed the fallback for certainty,
Hoppity—v0.6.0 lands just right! ✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically summarizes the main changes: adding a double-submit guard, implementing a bounded loop, and adding type annotation—all key fixes from the CodeRabbit review. |
| Description check | ✅ Passed | The description covers all required template sections: a clear summary of changes, proper categorization as a 'fix', and a test plan checklist aligned with the repository standards. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/coderabbit-it3-followup
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/WorkoutSession.tsx (1)
135-172:⚠️ Potential issue | 🟠 Major
isFinishingcan remain stuck ifonFinishfails.
onFinish(Line 152) is outside the currenttry/finally, so a thrown/rejected call skips Line 171 and leaves the UI permanently locked in this session.Proposed fix
const handleFinalizarTreino = async () => { if (isFinishing) return; setIsFinishing(true); - const endTime = new Date(); - const duracaoMinutos = Math.round((endTime.getTime() - startTime.getTime()) / 60000); - - const historico: Omit<HistoricoTreino, 'id' | 'alunoId'> = { - treinoId: treino.id, - dataExecucao: startTime.toISOString(), - duracaoMinutos, - exercicios: exerciciosEmSessao.map((ex) => ({ - exercicioId: ex.exercicioOriginal.id, - nomeExercicio: ex.exercicioOriginal.nomeExercicio, - seriesExecutadas: ex.seriesExecutadas, - })), - }; - - await onFinish(historico); - setCompleted(true); - - const completedExercises = exerciciosEmSessao - .filter((ex) => ex.seriesExecutadas.some((s) => s.concluido)) - .map((ex) => ex.exercicioOriginal.nomeExercicio); - - setIsLoadingFeedback(true); try { + const endTime = new Date(); + const duracaoMinutos = Math.round((endTime.getTime() - startTime.getTime()) / 60000); + + const historico: Omit<HistoricoTreino, 'id' | 'alunoId'> = { + treinoId: treino.id, + dataExecucao: startTime.toISOString(), + duracaoMinutos, + exercicios: exerciciosEmSessao.map((ex) => ({ + exercicioId: ex.exercicioOriginal.id, + nomeExercicio: ex.exercicioOriginal.nomeExercicio, + seriesExecutadas: ex.seriesExecutadas, + })), + }; + + await onFinish(historico); + setCompleted(true); + + const completedExercises = exerciciosEmSessao + .filter((ex) => ex.seriesExecutadas.some((s) => s.concluido)) + .map((ex) => ex.exercicioOriginal.nomeExercicio); + + setIsLoadingFeedback(true); + try { const result = await generateWorkoutFeedback({ goal: treino.objetivo, completedExercises, totalExercises: exerciciosEmSessao.length, }); setFeedback(result); - } catch { - setFeedback({ title: 'Treino Concluído!', message: 'Continue assim!' }); - } finally { - setIsLoadingFeedback(false); + } catch { + setFeedback({ title: 'Treino Concluído!', message: 'Continue assim!' }); + } finally { + setIsLoadingFeedback(false); + } + } finally { setIsFinishing(false); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/WorkoutSession.tsx` around lines 135 - 172, The onFinish call inside handleFinalizarTreino can throw and leave isFinishing true; wrap the await onFinish(...) so setIsFinishing(false) always runs (e.g., move onFinish into a try/catch/finally or add a surrounding try/finally) and only call setCompleted(true) and start the feedback flow (setIsLoadingFeedback and generateWorkoutFeedback) after onFinish succeeds; ensure any error from onFinish is handled (log or set fallback state) so the UI is never permanently locked by isFinishing remaining true.
🧹 Nitpick comments (1)
CHANGELOG.md (1)
8-8: Consider fully normalizing to a single[Unreleased]section.This header cleanup is good, but there are still multiple later
[Unreleased]blocks. Converting older ones to versioned entries (or merging intentfully) will keep release history clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` at line 8, The changelog contains multiple "[Unreleased]" headings; keep a single canonical "[Unreleased]" section (the one at the top) and locate any other occurrences of the marker ("[Unreleased]") elsewhere in CHANGELOG.md, then either convert those later "[Unreleased]" blocks into proper versioned entries with their release version/date or merge their contents into the single top "[Unreleased]" section so history remains consistent and unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/CURRENT-STATE.md`:
- Line 5: The document's high-level status line "Version: 0.6.0 (It3 complete —
all quality gates green, 17/17 E2E passing)" is inconsistent with later lines
that state CI/merge is pending; update the file so all status statements match:
either change the header status to indicate CI/merge is pending or update the
later lines (the lines referencing CI/merge pending) to reflect completion,
ensuring the phrases "It3 complete", "all quality gates green", and "CI/merge
pending" are reconciled across the file so the document presents one clear,
consistent state.
---
Outside diff comments:
In `@src/components/WorkoutSession.tsx`:
- Around line 135-172: The onFinish call inside handleFinalizarTreino can throw
and leave isFinishing true; wrap the await onFinish(...) so
setIsFinishing(false) always runs (e.g., move onFinish into a try/catch/finally
or add a surrounding try/finally) and only call setCompleted(true) and start the
feedback flow (setIsLoadingFeedback and generateWorkoutFeedback) after onFinish
succeeds; ensure any error from onFinish is handled (log or set fallback state)
so the UI is never permanently locked by isFinishing remaining true.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Line 8: The changelog contains multiple "[Unreleased]" headings; keep a single
canonical "[Unreleased]" section (the one at the top) and locate any other
occurrences of the marker ("[Unreleased]") elsewhere in CHANGELOG.md, then
either convert those later "[Unreleased]" blocks into proper versioned entries
with their release version/date or merge their contents into the single top
"[Unreleased]" section so history remains consistent and unambiguous.
🪄 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: b92ba349-950d-42e2-8971-d50ecf29df41
📒 Files selected for processing (5)
CHANGELOG.mddocs/CURRENT-STATE.mdsrc/ai/flows/workout-feedback-flow.test.tssrc/components/WorkoutSession.tsxtests/e2e/specs/workout-session.spec.ts
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…peScript TypeScript types serve as documentation; JSDoc coverage metrics are Python-specific and not applicable to this Next.js/React codebase. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WorkoutSession.tsx: addisFinishingguard tohandleFinalizarTreino; button disabled while in-flight to prevent duplicate saves/XP on rapid clicks (CodeRabbit critical)workout-session.spec.ts: scopeiniciarButtonto'Treino E2E'row via.filter(); addMAX_EXERCISES=20guard towhileloop for deterministic failure on regressionworkout-feedback-flow.test.ts: type-annotate fallback shape asWorkoutFeedbackOutputso compiler validates the contractCHANGELOG.md: consolidate dois[Unreleased]sections into oneCURRENT-STATE.md: fix version header inconsistencyTest plan
npm run typecheck→ 0 errorsnpm run lint→ 0 errorsnpm test→ 22/22 passingtreinoRowlocator🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Documentation
Chores