French-flag redesign: warm palette, wide-screen shell, and scenario roadmap - #43
Merged
Conversation
Adds failing tests (feature not yet implemented) covering: - roadmap step status derivation + non-regressing auto-advance (utils/roadmapStepStatus.ts) - Scenario.steps localStorage persistence + getScenarioSteps accessor (services/scenarioService.ts) - conditional "currentStepIndex" response schema field (services/geminiService.ts) - ScenarioSetup roadmap editor (add/remove/edit/reorder steps) UI contract - ORB_STATE_COLORS export pinning the mic orb to a red-family palette - index.css @theme tablet/desktop breakpoint tokens Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011uZQPdbxtsHWKe8Ur5PacX
- index.css: Tailwind v4 @theme block with tablet(760px)/desktop(1200px) breakpoints and a blue/white/red/navy/cream palette; swap dark slate body theme for the light cream backdrop. - utils/roadmapStepStatus.ts: getRoadmapStepStatus/advanceRoadmapStep pure helpers for the scenario roadmap sidebar (never-regress auto-advance).
Export ORB_STATE_COLORS from components/Orb.tsx as the single source of truth for per-state mic colors: muted red-pink IDLE, vivid red RECORDING, neutral navy PROCESSING, blue PLAYING. Mic now reads as the dominantly red control per the French-flag redesign.
…chema - types.ts: ScenarioStep, Scenario.steps, VoiceResponse.currentStepIndex - scenarioService.ts: getScenarioSteps() defensive accessor, seedRoadmapStepsFromSummary() heuristic seed for the roadmap editor, and a roadmap-tracking instruction block appended to the role-play system instruction when steps are present. - geminiService.ts: RoadmapSingleCharacterSchema/ROADMAP_RESPONSE_SCHEMA — a separate conditional schema branch (mirrors the isTefQuestioning precedent) so "currentStepIndex" is only ever present in the response schema when activeScenario.steps is non-empty; extracted and forwarded on VoiceResponse the same way isRepeat/conceptLabels are.
Add controlled roadmapSteps/onRoadmapStepsChange props and render an editable ordered step list (reorder via move-up/down, inline edit, remove, add) in the AI-summary-confirmed branch, per wireframe 3d/4d. Drag handle (⋮⋮) is decorative affordance; move-up/down buttons are the functional reorder mechanism. handleStartPractice now builds Scenario.steps from the editor state.
Restructure App.tsx into the 3-region shell from wireframe 2a: TopBar
(brand + settings + mobile hamburger menu), NavRail (Free Talk / Role
Play / TEF Ad / TEF Questions mode switcher, CSS-only tablet/desktop
responsive collapse), center conversation content (no longer capped at
max-w-2xl on wide screens), and a right-side ScenarioRoadmap sidebar
(desktop static column, tablet edge-tab + right drawer, mobile chip +
bottom sheet) shown only for active role-play scenarios with steps.
The footer (exit/start control + mic orb + speed) is now a single
persistent element pinned to the viewport bottom at every breakpoint,
built by restyling the existing compact Controls+Orb bar rather than
rebuilding — mic orb no longer moves to a centered landing position.
Nav mode switching reuses existing handlers verbatim (handleOpenScenarioSetup,
handleOpenTefAdSetup, setTefQuestioningMode('setup'), handleExitScenario,
handleExitTefAd, handleExitTefQuestioning) via a new handleNavSelect —
no forked exit/entry logic. Recolored App.tsx and Controls.tsx to the
French-flag light palette from index.css.
New components: components/NavRail.tsx, components/TopBar.tsx,
components/ScenarioRoadmap.tsx.
Test fix: __tests__/scenarioDescriptionRecordingAbortDiscard.test.tsx
queried `getByRole('button', { name: /Role Play/i })` unscoped, which
is now ambiguous because the persistent nav rail also exposes a
"Role Play" control by design. Added a data-testid to the test's own
PracticeModeSheet mock and scoped the query to it (mock has no other
distinguishing markup). Also hardened TopBar's mobile menu and the
roadmap drawer to only mount their contents while open, rather than
relying on vaul's internal presence handling, since this test's vaul
mock renders Drawer.Root children unconditionally.
ScenarioSetup is touched in this pass for the roadmap editor (and its own wireframe screens 3c/3d are shown fully re-skinned in the approved design), so the whole modal — inputs, saved-scenario list, transcript picker, character cards, action buttons — moves from dark slate to the parle-* light tokens for visual consistency with the new app shell. Left the yellow API-key warning banner unchanged: it's a documented cross-cutting pattern (AGENTS.md) shared with components pass 2 will still be re-skinning, so changing it only here would fragment it.
Re-skin the practice-mode bottom sheet (wireframe 3b/4b) to the French-flag light blue/white/red palette established in pass 1: white sheet on a parle-navy scrim, parle-navy text, parle-blue mode cards, and the "past topic suggestions" row folded from violet into the blue accent family.
Re-skin AdPersuasionSetup and AdQuestioningSetup (wireframe 3e/4e, 3g/4g) to the light blue/white/red palette: white modal cards, parle-navy text, parle-blue dropzone/CTAs, parle-red error states. The missing-Gemini-key warning banner keeps its amber/yellow semantics (AGENTS.md) but switches to light-mode-appropriate amber-50/amber-400/amber-900 instead of the old dark-theme bg-yellow-900/border-yellow-600 pair; "green" success states (Ad Analyzed, Start Conversation) fold into the blue accent family since green has no place in the French-flag 3-color system. PersuasionTimer/QuestioningTimer (TEF topbar chips) move off slate/amber onto parle tokens, using two shades of red to preserve the low/critical urgency gradient without reintroducing amber outside the credentials banner. Also fixes two dark-slate remnants in App.tsx that pass 1 missed: the landing-view status text colors and the TEF "Time's Up" overlay (now white card on a parle-navy scrim, matching every other modal). Updates one test (adPersuasionCredentials.test.ts) whose regex asserted the old bg-yellow-/border-yellow- class names — purely a color-class string update, the assertion still verifies the warning banner renders.
Re-skin the post-exercise summary family (wireframe 3i/4i) to the light palette: TefAdSummary, TefQuestioningSummary, ScenarioReviewSummary (the role-play equivalent), TefReviewPanel and ScenarioStandardizationReviewPanel (embedded review content), and TefTopicSuggestionsList (shared topic-card renderer) all move from slate cards to white cards on a parle-navy scrim. Per the wireframe's wf-pill convention, "met"/positive states map to blue and "unmet"/negative states map to red — not green/red — so all green accents (checkmarks, "What Went Well", vocabulary corrections) fold into the blue family, and TefAdSummary's Done button changes from amber to blue to match TefQuestioningSummary (amber stays reserved for the credentials warning banner per AGENTS.md). The violet "Topics You Could Have Mentioned" heading also folds into blue, consistent with the rest of the scope. Review-panel error states (not the credentials banner) map to red as a destructive/failure state rather than amber. No test changes needed — the review-panel and summary tests query by role and text, not class strings.
Re-skin TefTopicHistorySheet (wireframe 3j/4j), TefRecentAdsCarousel, AdThumbnail, and ApiKeySetup (Settings, wireframe 3k/4k) to the light blue/white/red palette: white cards on a parle-navy scrim, parle-navy text/borders, parle-blue primary actions. Green "most recent"/"Start" accents fold into blue; the persuasion/questioning exercise-type badges in topic history now differentiate via blue vs. red (previously blue vs. emerald) since the palette only carries two content accent colors. ImageLightbox keeps its full-bleed dark backdrop (a photo viewer stays dark regardless of app theme, and AGENTS.md/task guidance explicitly allows dark overlay scrims to remain) but swaps generic Tailwind slate for the app's own parle-navy-900 dark token so the chrome reads as part of the same design system rather than a leftover default. No test changes needed here — tefTopicHistorySheet.test.tsx and tefRecentAdsCarousel.test.tsx query by role/text, not class strings.
Re-skin the chat surfaces that render inside the new light shell's center column: ConversationHistory (chat bubbles), ConversationHint, and PracticeGuidePanel. Bubbles now match the wireframe's literal treatment — white agent bubbles / light-blue user bubbles with navy borders and text, rather than the old dark-slate-with-white-text messenger look. The "Audio unavailable" inline warning moves from yellow to red (a failure indicator, not the reserved credentials banner). PracticeGuidePanel's violet accent folds into the blue family per the 3-color palette.
Matches the light-mode amber treatment already applied to AdPersuasionSetup/AdQuestioningSetup in the redesign pass, since it was left out of that pass's scope.
- Disable inactive modes in TopBar's mobile menu to match NavRail's guardrail against stacking a setup flow on a running session (review S1) - Recolor the landing view's status-text warnings (missing API key, connection error) from dark-theme yellow/red-400 to the light-theme amber/parle-red tokens used everywhere else in the redesign
Add an AGENTS.md pattern section for the scenario roadmap's conditional response-schema branch (RoadmapSingleCharacterSchema / currentStepIndex) and the never-regress auto-advance rule in advanceRoadmapStep(), modeled on the existing TEF Ad Questioning schema-selection section. Notes the sentence-split seeding heuristic (seedRoadmapStepsFromSummary) as an intentional, user-editable simplification rather than a bug, adds corresponding review-agent notes, and a Version History entry. Update README.md: describe the roadmap capability under Scenario role-play, note the French-flag design tokens/breakpoints under Tech stack, and add NavRail/TopBar/ScenarioRoadmap to the project layout table.
Pins down the contract for replacing the sentence-split heuristic with a real AI call: ScenarioSummarySchema must require a 2-8 entry `steps` array, processScenarioDescriptionOpenAI must surface it in its JSON result (and fall back to `steps: []` on error), and App.tsx must prefer those AI steps over seedRoadmapStepsFromSummary. Fails before the implementation exists.
Extend the existing OpenAI scenario-planning call (structured output, already producing summary + characters) to also return a `steps: string[]` field (2-8 entries) describing the roadmap beats — no extra request, same latency/cost as before. App.tsx now prefers these AI-generated steps when seeding the roadmap editor, falling back to the sentence-split heuristic (seedRoadmapStepsFromSummary) only when the AI call fails or returns no usable steps (e.g. a non-JSON legacy response).
Replaces the now-outdated "sentence-split seeding is intentional, don't swap in an AI call" guidance with the actual current behavior: the AI call is now the primary source, and the heuristic is a documented fallback that must not be removed. Updates the matching review-agent note and adds a Version History entry.
Covers: multi-character roadmap schema/auto-advance, functional drag-and-drop reorder in the roadmap editor, the nav rail's new "Past topic suggestions" entry, and quick-start regeneration behavior for saved scenarios without a roadmap yet.
createMultiCharacterSchema (and its multi-character system instruction) never carried the roadmap currentStepIndex field — only the single-character schema branch had it. Since characters.length > 1 is checked before the roadmap-steps check in schema selection, any multi-character scenario with roadmap steps (e.g. a bakery visit with a Baker + Cashier, the exact example used throughout this feature's own design mockups) silently dropped the field, so the roadmap sidebar never advanced past step 1. createMultiCharacterSchema now conditionally extends its base shape with currentStepIndex when the scenario has non-empty steps, mirroring the existing conditional-schema precedent (isTefQuestioning / RoadmapSingleCharacterSchema). generateMultiCharacterSystemInstruction now also appends the roadmap instruction section.
The unicode "▶" character renders off-center within its circular badge across common fonts (inconsistent glyph metrics) — flex centering alone can't fix that. Replaced it with a small inline SVG triangle, which centers pixel-perfectly regardless of font.
The ⋮⋮ handle had no event handlers — dragging it did nothing; only the up/down buttons actually reordered steps. Each step row is now a native HTML5 drag source/drop target (draggable + dragstart/dragover/drop), so dragging a row and dropping it on another row's position reorders the list. The up/down buttons are unchanged. feat: quick-starting a saved scenario without a roadmap now regenerates one Clicking "Start" on a saved scenario with no `steps` (e.g. every scenario saved before the roadmap feature existed) used to jump straight into practice, permanently skipping the roadmap editor for it — the only way to get a roadmap onto it was recreating the scenario from scratch. handleQuickStart now branches: scenarios that already have steps keep the unchanged fast path (onStartPractice directly); scenarios without steps instead call the new onRegenerateRoadmap callback. The roadmap editor's local "Start Practice" also gains a regeneratingScenario prop so it reuses that scenario's existing id/createdAt on save (an update in place) instead of generating a new scenario (which would have produced a duplicate entry).
…rail extras in App.tsx Three App.tsx changes bundled together since they touch overlapping regions of the same handlers/JSX shell: 1. Multi-character response branch now calls advanceRoadmapStep the same way the single-character branch already did, consuming the currentStepIndex field added to the multi-character schema (see the geminiService.ts commit) — completes the fix for the roadmap being stuck on step 1 for multi-character scenarios. 2. handleRegenerateRoadmapForScenario (+ shared processScenarioDescriptionAndPopulate helper extracted from handleSubmitScenarioDescription) implements the app-side half of quick-start roadmap regeneration for saved scenarios without steps. 3. App shell restructure: the nav rail and roadmap sidebar are now both full-height siblings of a center column containing <main> + the footer, instead of the nav rail being nested in a row above the footer. Previously <main> centered its content across (row width - nav rail width), while the footer centered across the full row width, producing a real pixel offset between the two — verified via getBoundingClientRect() before/after (was offset, now 0px, widths and positions identical). The nav rail also gains an onOpenTopicHistory prop, rendering a "Past topics" entry (with a divider) below the four mode items — mirrors the same divider/entry pattern already used in PracticeModeSheet's bottom sheet, and opens the same TefTopicHistorySheet.
Pins down the fix for a real race condition found in live usage: clicking "Start" on a saved scenario twice in a row (e.g. navigating back and retrying) fired two concurrent AI roadmap-planning requests with no cancellation, and whichever settled last silently overwrote the other's data — independent of which one the user actually meant to keep.
Accepts an optional signal, passed to the LangChain invoke() call via RunnableConfig.signal so an aborted request's underlying HTTP call is actually cancelled rather than left running to completion. An abort-like error (isAbortLikeError) is re-thrown instead of being swallowed into the generic fallback response, so callers can tell an intentional cancel apart from a real failure.
…ositioning
Two App.tsx changes bundled together (both touch the roadmap sidebar's
tablet drawer / scenario-setup regions):
1. Race-condition fix: processScenarioDescriptionAndPopulate now aborts
any in-flight scenario-planning request (and bumps a request token)
before starting a new one, via a new cancelScenarioPlanningRequest
helper. Both the success and error paths check that token before
touching state, so a stale response can never win regardless of
network settle order. Also called from handleCloseScenarioSetup, so
an abandoned request can't write stale state after the modal closes.
2. Tablet roadmap drawer positioning fix: previously pinned edge-to-edge
top-to-bottom (`inset-y-0`) regardless of content height, and its
drag handle stayed at the top (a leftover from the bottom-sheet
variant) despite the drawer sliding in from the right. Now:
- Vertically centered near the edge tab by default, using
`inset-y-0 + my-auto + h-fit` (margin-based centering, not
`transform`, since vaul already drives the slide animation via an
inline `transform` on the same element — verified empirically that
centering-via-margin and vaul's positioning don't conflict, while
centering-via-translate would have been silently clobbered).
- Capped at `max-h-[90dvh]` so long roadmaps still get an internal
scrollbar and effectively span the height, rather than overflowing
the viewport uncapped.
- The drag handle moved to the left edge (the side facing back into
the main content) with a vertical orientation, matching the
drawer's horizontal slide direction — dragging from the top
incorrectly implied a vertical gesture. Handle sizing needed `!`
(important) overrides since vaul injects its own
`[data-vaul-handle]` size rule at runtime, which otherwise wins the
specificity tie against a plain Tailwind utility class.
- Verified via getBoundingClientRect()/getComputedStyle() in a real
browser: short content centers correctly (equal top/bottom margin),
long content caps at 90dvh and stays scrollable, and the mobile
bottom-sheet variant (handle on top, pinned to bottom) is
unaffected.
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
parle | 65d4333 | Commit Preview URL Branch Preview URL |
Jul 12 2026, 06:07 PM |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
parle-personal | 65d4333 | Commit Preview URL Branch Preview URL |
Jul 12 2026, 06:08 PM |
WalkthroughAdds editable scenario roadmaps with AI-reported progress, monotonic advancement, persistence, regeneration, and cancellation-safe planning. It also introduces responsive navigation, roadmap presentation, a unified parle visual theme, and extensive tests for roadmap, schema, planning, and UI behavior. ChangesScenario roadmap flow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ScenarioSetup
participant App
participant OpenAI
participant Gemini
participant ScenarioRoadmap
User->>ScenarioSetup: edit or submit scenario
ScenarioSetup->>App: provide roadmap configuration
App->>OpenAI: request summary and steps
OpenAI-->>App: return structured roadmap
App->>Gemini: start roadmap-aware session
Gemini-->>App: return currentStepIndex
App->>ScenarioRoadmap: update monotonic progress
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
components/ScenarioRoadmap.tsx (1)
27-55: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExpose roadmap status to assistive technologies.
The done/current/upcoming state is conveyed only visually; the markers are
aria-hidden, so screen readers receive identical step text for every item. Addaria-current="step"for the current item and a visually hidden status label for all three states.Proposed accessibility fix
<li key={step.id} className={`flex items-start gap-2.5 rounded-lg px-2.5 py-2 text-sm transition-colors ${ isCurrent ? 'bg-parle-blue-100 border border-parle-blue-500 text-parle-navy-900 font-medium' : 'text-parle-navy-700' }`} + aria-current={isCurrent ? 'step' : undefined} > + <span className="sr-only"> + {isDone ? 'Completed: ' : isCurrent ? 'Current: ' : 'Upcoming: '} + </span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/ScenarioRoadmap.tsx` around lines 27 - 55, Add accessible roadmap status in the list-item rendering around the existing isDone and isCurrent state logic: set aria-current="step" only on the current item, and add a visually hidden label identifying each step as done, current, or upcoming. Keep the decorative marker aria-hidden and preserve the existing visual styling and step text.services/geminiService.ts (1)
226-240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated response-schema selection logic into a shared helper.
These two IIFEs in
createChatSession()andsendVoiceMessage()implement identical branching logic (multi-character → no-scenario →isTefQuestioning→ roadmap → default). This is the exact class of duplication that already caused the bug this PR fixes elsewhere (the multi-character schema silently missingcurrentStepIndexbecause the roadmap branch wasn't added to every code path). Any future schema branch added to only one copy will silently regress the other.♻️ Proposed extraction of shared schema-selection logic
+// Shared response-schema resolution, used by both createChatSession() and +// sendVoiceMessage() to avoid the two call sites drifting out of sync when a +// new schema branch (e.g. roadmap) is added. +function resolveResponseSchema() { + if (activeScenario && activeScenario.characters && activeScenario.characters.length > 1) { + return createGeminiMultiCharacterSchema(activeScenario); + } + if (!activeScenario) { + return FREE_CONVERSATION_RESPONSE_SCHEMA; + } + if (activeScenario.isTefQuestioning) { + return TEF_QUESTIONING_RESPONSE_SCHEMA; + } + if (activeScenario.steps && activeScenario.steps.length > 0) { + return ROADMAP_RESPONSE_SCHEMA; + } + return SINGLE_CHARACTER_RESPONSE_SCHEMA; +}Then in
createChatSession():- const responseSchema = (() => { - if (activeScenario && activeScenario.characters && activeScenario.characters.length > 1) { - return createGeminiMultiCharacterSchema(activeScenario); - } - if (!activeScenario) { - return FREE_CONVERSATION_RESPONSE_SCHEMA; - } - if (activeScenario.isTefQuestioning) { - return TEF_QUESTIONING_RESPONSE_SCHEMA; - } - if (activeScenario.steps && activeScenario.steps.length > 0) { - return ROADMAP_RESPONSE_SCHEMA; - } - return SINGLE_CHARACTER_RESPONSE_SCHEMA; - })(); + const responseSchema = resolveResponseSchema();And in
sendVoiceMessage():- const responseSchemaForThisRequest = (() => { - if (activeScenario && activeScenario.characters && activeScenario.characters.length > 1) { - return createGeminiMultiCharacterSchema(activeScenario); - } - if (!activeScenario) { - return FREE_CONVERSATION_RESPONSE_SCHEMA; - } - if (activeScenario.isTefQuestioning) { - return TEF_QUESTIONING_RESPONSE_SCHEMA; - } - if (activeScenario.steps && activeScenario.steps.length > 0) { - return ROADMAP_RESPONSE_SCHEMA; - } - return SINGLE_CHARACTER_RESPONSE_SCHEMA; - })(); + const responseSchemaForThisRequest = resolveResponseSchema();Also applies to: 734-748
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/geminiService.ts` around lines 226 - 240, Extract the duplicated response-schema branching from createChatSession() and sendVoiceMessage() into one shared helper, using the existing ordering: multi-character, no scenario, TEF questioning, roadmap, then single-character fallback. Replace both IIFEs with calls to that helper so future schema branches are maintained in one place.services/scenarioService.ts (1)
183-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication: reuse
getScenarioStepsinstead of re-derivingsteps.
getScenarioSteps(scenario)(lines 11-13) already encapsulates thescenario?.steps ?? []normalization this file exports specifically to avoid ad-hoc null-checking elsewhere.generateRoadmapInstructionSectionre-implements the same pattern inline instead of calling it.♻️ Proposed refactor
function generateRoadmapInstructionSection(scenario: Scenario): string { - const steps = scenario.steps ?? []; + const steps = getScenarioSteps(scenario); if (steps.length === 0) return '';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/scenarioService.ts` around lines 183 - 186, Update generateRoadmapInstructionSection to obtain steps through the existing getScenarioSteps(scenario) helper instead of directly normalizing scenario.steps. Preserve the current empty-steps early return and subsequent behavior.__tests__/roadmapStepStatus.test.ts (1)
108-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a
stepsLength === 0test case foradvanceRoadmapStep.
getRoadmapStepStatusexplicitly tests thestepsLength <= 0boundary (lines 92-95), butadvanceRoadmapStephas no equivalent case. Since clamping into[0, stepsLength - 1]is ill-defined whenstepsLengthis 0, pinning the expected behavior here would guard against a future implementation regression (e.g., returning-1orNaN).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/roadmapStepStatus.test.ts` around lines 108 - 151, Add a focused `stepsLength === 0` test to the `advanceRoadmapStep` suite, covering representative inputs and asserting the intended finite boundary behavior without returning `-1` or `NaN`. Keep the existing clamping and progression tests unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@App.tsx`:
- Around line 1449-1459: Ensure closing the scenario setup always clears
isProcessingScenario even when it invalidates an in-flight request. Update
handleCloseScenarioSetup to reset the flag, or make the finally block in
processScenarioDescriptionAndPopulate clear it unconditionally while preserving
stale-request protection for other state updates.
In `@components/Orb.tsx`:
- Around line 11-12: Update the IDLE entry in ORB_STATE_COLORS to use a darker
red that provides stronger contrast against the white microphone icon, while
leaving the other orb state colors unchanged.
In `@components/ScenarioSetup.tsx`:
- Around line 186-193: Validate sourceIndex in handleRoadmapStepDrop before
calling splice, rejecting values below zero or at least roadmapSteps.length in
addition to NaN and targetIndex. Return without changing the roadmap for invalid
external drag data, while preserving the existing reorder behavior for valid
indices.
In `@index.css`:
- Line 39: Update the font-family declaration in the visible CSS rule to remove
quotes around the Inter font name, while preserving the sans-serif fallback.
---
Nitpick comments:
In `@__tests__/roadmapStepStatus.test.ts`:
- Around line 108-151: Add a focused `stepsLength === 0` test to the
`advanceRoadmapStep` suite, covering representative inputs and asserting the
intended finite boundary behavior without returning `-1` or `NaN`. Keep the
existing clamping and progression tests unchanged.
In `@components/ScenarioRoadmap.tsx`:
- Around line 27-55: Add accessible roadmap status in the list-item rendering
around the existing isDone and isCurrent state logic: set aria-current="step"
only on the current item, and add a visually hidden label identifying each step
as done, current, or upcoming. Keep the decorative marker aria-hidden and
preserve the existing visual styling and step text.
In `@services/geminiService.ts`:
- Around line 226-240: Extract the duplicated response-schema branching from
createChatSession() and sendVoiceMessage() into one shared helper, using the
existing ordering: multi-character, no scenario, TEF questioning, roadmap, then
single-character fallback. Replace both IIFEs with calls to that helper so
future schema branches are maintained in one place.
In `@services/scenarioService.ts`:
- Around line 183-186: Update generateRoadmapInstructionSection to obtain steps
through the existing getScenarioSteps(scenario) helper instead of directly
normalizing scenario.steps. Preserve the current empty-steps early return and
subsequent behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f023c4d8-22a8-4a8b-a3d3-8d2478d4a9ec
📒 Files selected for processing (51)
AGENTS.mdApp.tsxREADME.md__tests__/NavRail.topicHistory.test.tsx__tests__/ScenarioSetup.quickStartRegenerate.test.tsx__tests__/ScenarioSetup.roadmapEditor.dragReorder.test.tsx__tests__/ScenarioSetup.roadmapEditor.test.tsx__tests__/adPersuasionCredentials.test.ts__tests__/openaiService.roadmapSteps.test.ts__tests__/openaiService.scenarioDescriptionAbort.test.ts__tests__/orbStateColors.test.ts__tests__/responsiveBreakpoints.test.ts__tests__/roadmapMultiCharacterAutoAdvance.source.test.ts__tests__/roadmapMultiCharacterSchema.test.ts__tests__/roadmapSchemaSelection.test.ts__tests__/roadmapStepStatus.test.ts__tests__/scenarioDescriptionAbort.source.test.ts__tests__/scenarioDescriptionAiRoadmapSteps.source.test.ts__tests__/scenarioDescriptionRecordingAbortDiscard.test.tsx__tests__/scenarioService.roadmapSteps.test.tscomponents/AdPersuasionSetup.tsxcomponents/AdQuestioningSetup.tsxcomponents/AdThumbnail.tsxcomponents/ApiKeySetup.tsxcomponents/Controls.tsxcomponents/ConversationHint.tsxcomponents/ConversationHistory.tsxcomponents/ImageLightbox.tsxcomponents/NavRail.tsxcomponents/Orb.tsxcomponents/PersuasionTimer.tsxcomponents/PracticeGuidePanel.tsxcomponents/PracticeModeSheet.tsxcomponents/QuestioningTimer.tsxcomponents/ScenarioReviewSummary.tsxcomponents/ScenarioRoadmap.tsxcomponents/ScenarioSetup.tsxcomponents/ScenarioStandardizationReviewPanel.tsxcomponents/TefAdSummary.tsxcomponents/TefQuestioningSummary.tsxcomponents/TefRecentAdsCarousel.tsxcomponents/TefReviewPanel.tsxcomponents/TefTopicHistorySheet.tsxcomponents/TefTopicSuggestionsList.tsxcomponents/TopBar.tsxindex.cssservices/geminiService.tsservices/openaiService.tsservices/scenarioService.tstypes.tsutils/roadmapStepStatus.ts
Comment on lines
1449
to
1459
| } catch (error) { | ||
| if (requestId !== scenarioPlanningRequestIdRef.current) return; // stale/aborted — ignore | ||
| console.error('Error processing scenario:', error); | ||
| setAiSummary('I understand your scenario. Ready to begin when you are!'); | ||
| setScenarioCharacters([]); // Fallback to single-character | ||
| setRoadmapSteps(['']); | ||
| setScenarioCharacters(fallbackCharacters); | ||
| } finally { | ||
| setIsProcessingScenario(false); | ||
| if (requestId === scenarioPlanningRequestIdRef.current) { | ||
| setIsProcessingScenario(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'isProcessingScenario' components/ScenarioSetup.tsxRepository: CodeWithOz/parle
Length of output: 1427
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the scenario setup open/close flow and processing-state handling in App.tsx.
ast-grep outline App.tsx --view expanded | sed -n '1,260p'
printf '\n---- relevant references ----\n'
rg -n -C 4 'handle(Open|Close)ScenarioSetup|cancelScenarioPlanningRequest|setIsProcessingScenario|isProcessingScenario|ScenarioSetup' App.tsxRepository: CodeWithOz/parle
Length of output: 6916
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether ScenarioSetup exposes close/cancel controls that remain usable while processing.
ast-grep outline components/ScenarioSetup.tsx --view expanded | sed -n '1,260p'
printf '\n---- close/cancel controls ----\n'
rg -n -C 4 'onClose|onCancel|Close|Cancel|handleClose|handleCancel|dismiss|back|nav' components/ScenarioSetup.tsxRepository: CodeWithOz/parle
Length of output: 20650
isProcessingScenario can stay true after closing the setup mid-request (App.tsx:1179-1459). handleCloseScenarioSetup() bumps the request token, so the guarded finally in processScenarioDescriptionAndPopulate() skips setIsProcessingScenario(false) for the aborted request. Reset the flag in the close handler or clear it unconditionally in finally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@App.tsx` around lines 1449 - 1459, Ensure closing the scenario setup always
clears isProcessingScenario even when it invalidates an in-flight request.
Update handleCloseScenarioSetup to reset the flag, or make the finally block in
processScenarioDescriptionAndPopulate clear it unconditionally while preserving
stale-request protection for other state updates.
Comment on lines
+11
to
+12
| export const ORB_STATE_COLORS: Record<'IDLE' | 'RECORDING' | 'PROCESSING' | 'PLAYING', string> = { | ||
| IDLE: '#d9827e', // muted red-pink — recognizably red, low saturation |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Increase idle orb contrast.
#d9827e provides only about 2.8:1 contrast against the white microphone icon, making the idle control harder to perceive. Use a darker idle red or a dark idle icon color.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/Orb.tsx` around lines 11 - 12, Update the IDLE entry in
ORB_STATE_COLORS to use a darker red that provides stronger contrast against the
white microphone icon, while leaving the other orb state colors unchanged.
Comment on lines
+186
to
+193
| const handleRoadmapStepDrop = (targetIndex: number) => (e: React.DragEvent<HTMLDivElement>) => { | ||
| e.preventDefault(); | ||
| const sourceIndex = parseInt(e.dataTransfer.getData('text/plain'), 10); | ||
| if (Number.isNaN(sourceIndex) || sourceIndex === targetIndex) return; | ||
| const next = roadmapSteps.slice(); | ||
| const [moved] = next.splice(sourceIndex, 1); | ||
| next.splice(targetIndex, 0, moved); | ||
| onRoadmapStepsChange(next); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate the drag source index before mutating the roadmap.
An external drop can supply -1 or an out-of-range value. splice() then moves the wrong item or inserts undefined; subsequently, handleStartPractice() crashes on text.trim().
Proposed fix
const handleRoadmapStepDrop = (targetIndex: number) => (e: React.DragEvent<HTMLDivElement>) => {
e.preventDefault();
- const sourceIndex = parseInt(e.dataTransfer.getData('text/plain'), 10);
- if (Number.isNaN(sourceIndex) || sourceIndex === targetIndex) return;
+ const sourceIndexValue = e.dataTransfer.getData('text/plain');
+ const sourceIndex = Number(sourceIndexValue);
+ if (
+ sourceIndexValue === '' ||
+ !Number.isInteger(sourceIndex) ||
+ sourceIndex < 0 ||
+ sourceIndex >= roadmapSteps.length ||
+ sourceIndex === targetIndex
+ ) return;
const next = roadmapSteps.slice();Based on learnings: preserve documented intentional patterns when the fix retains their benefits.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleRoadmapStepDrop = (targetIndex: number) => (e: React.DragEvent<HTMLDivElement>) => { | |
| e.preventDefault(); | |
| const sourceIndex = parseInt(e.dataTransfer.getData('text/plain'), 10); | |
| if (Number.isNaN(sourceIndex) || sourceIndex === targetIndex) return; | |
| const next = roadmapSteps.slice(); | |
| const [moved] = next.splice(sourceIndex, 1); | |
| next.splice(targetIndex, 0, moved); | |
| onRoadmapStepsChange(next); | |
| const handleRoadmapStepDrop = (targetIndex: number) => (e: React.DragEvent<HTMLDivElement>) => { | |
| e.preventDefault(); | |
| const sourceIndexValue = e.dataTransfer.getData('text/plain'); | |
| const sourceIndex = Number(sourceIndexValue); | |
| if ( | |
| sourceIndexValue === '' || | |
| !Number.isInteger(sourceIndex) || | |
| sourceIndex < 0 || | |
| sourceIndex >= roadmapSteps.length || | |
| sourceIndex === targetIndex | |
| ) return; | |
| const next = roadmapSteps.slice(); | |
| const [moved] = next.splice(sourceIndex, 1); | |
| next.splice(targetIndex, 0, moved); | |
| onRoadmapStepsChange(next); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/ScenarioSetup.tsx` around lines 186 - 193, Validate sourceIndex in
handleRoadmapStepDrop before calling splice, rejecting values below zero or at
least roadmapSteps.length in addition to NaN and targetIndex. Return without
changing the roadmap for invalid external drag data, while preserving the
existing reorder behavior for valid indices.
Source: Learnings
| } | ||
|
|
||
| body { | ||
| font-family: 'Inter', sans-serif; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove quotes around the font-family name per Stylelint.
Static analysis flags font-family-name-quotes on this line.
🔧 Proposed fix
- font-family: 'Inter', sans-serif;
+ font-family: Inter, sans-serif;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| font-family: 'Inter', sans-serif; | |
| font-family: Inter, sans-serif; |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 39-39: Expected no quotes around "Inter" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@index.css` at line 39, Update the font-family declaration in the visible CSS
rule to remove quotes around the Inter font name, while preserving the
sans-serif fallback.
Source: Linters/SAST tools
cancelScenarioPlanningRequest() bumps the request token to invalidate any in-flight roadmap-generation request, but that also means the request's own `finally` block (guarded by that same token) skips clearing isProcessingScenario. Left uncleared, reopening the setup flow would show "Processing..." indefinitely with nothing actually in flight. handleCloseScenarioSetup now clears it directly. Code review fix.
#d9827e (the original muted red-pink) measured ~2.8:1 contrast against the white mic icon, below WCAG's 3:1 minimum for UI components. #ce5f5a improves this to ~3.9:1 while staying at 54% saturation — still clearly less saturated than RECORDING's 62%, preserving the muted-vs-vivid distinction. Code review fix.
Native HTML5 drag data isn't guaranteed to come from this list (e.g. dragging text/a link onto a row from elsewhere), so sourceIndex wasn't guaranteed to be a valid array index. A negative value would splice from the wrong end; an out-of-range value would splice in `undefined` as a bogus step. Now rejected alongside the existing NaN/self-drop checks. Code review fix.
Done/current/upcoming was conveyed only by color and a decorative (aria-hidden) marker. Added aria-current="step" on the current item and a visually hidden status label per step. Code review nitpick.
Locks in existing (already-correct) behavior: the clampIndex guard returns 0 rather than NaN/-1 when there are no valid indices to clamp into. No production code change. Code review nitpick.
createChatSession() and sendVoiceMessage() each had an identical schema-selection IIFE (multi-character, no scenario, TEF questioning, roadmap, single-character fallback). Extracted to selectResponseSchema() so future schema branches are maintained in one place instead of two. Behavior unchanged — same branching order, same schemas. Code review nitpick.
Was duplicating the same scenario.steps ?? [] normalization that getScenarioSteps() already centralizes. Behavior unchanged. Code review nitpick.
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
2 participants
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
Implements the approved Claude Design mockups for Parle: a French-flag-inspired
(blue/white/red) redesign replacing the dark theme, a wide-screen app shell
(nav rail + pinned footer + responsive roadmap sidebar), and the feature that
started the whole redesign — an editable, AI-tracked "scenario roadmap" for
role-play practice.
tablet(760px)/desktop(1200px) breakpoints, appliedapp-wide (every screen re-skinned: Free Talk, Practice Mode picker, both TEF
flows, summaries, topic history, settings)
footer permanently pinned to the bottom — mic reads dominantly red (muted
idle → vivid recording)
auto-advances from AI inference during conversation (single- and
multi-character scenarios), never regressing a step. Steps are
AI-generated as part of the existing scenario-planning call (no extra
request), with a heuristic fallback only when that call fails.
"Start" — generated and saved back onto the same scenario, not duplicated
sidebar (static column → edge-tab drawer → bottom sheet) across
desktop/tablet/mobile
via
AbortSignal+ a request-token guard, so a stale response can neveroverwrite a newer one
(margin-based centering, compatible with vaul's own slide animation),
capped/scrollable for long content, with its drag handle moved to the left
edge to match the horizontal slide direction
Process
Built via the repo's
/devworkflow (TDD → build → review → browserverification → docs), run manually across several passes since the custom
subagent types aren't registered in this tool session — each phase was
filled by a general-purpose agent given that role's exact instructions.
Manual code review (Opus, CodeRabbit was unavailable) found 0
Critical/Warning issues. All fixes from two rounds of live-usage testing are
included (multi-character roadmap not advancing, non-functional drag handles,
footer/sidebar misalignment, marker centering, and the two issues above).
Test plan
npm test— 601 passing (94 new across this branch)npm run build— cleansidebar/drawer/sheet, drag-and-drop reorder, quick-start regeneration
(with and without an existing roadmap), nav rail additions, tablet
drawer centering/scroll/handle position — all via mocked
OpenAI/seeded localStorage since no live API key is available in this
environment
Summary by CodeRabbit
New Features
Style
Bug Fixes
Tests