Skip to content

feat: adaptive exam input, graph canvas, voice, confetti#160

Closed
beenycool wants to merge 2 commits into
refactor/studentos-upstash-ratelimitfrom
feat/adaptive-input-voice-graph-confetti
Closed

feat: adaptive exam input, graph canvas, voice, confetti#160
beenycool wants to merge 2 commits into
refactor/studentos-upstash-ratelimitfrom
feat/adaptive-input-voice-graph-confetti

Conversation

@beenycool
Copy link
Copy Markdown
Owner

@beenycool beenycool commented Apr 5, 2026

Summary

Stacked on refactor/studentos-upstash-ratelimit — merge that PR first (or rebase this branch onto main after it lands).

Changes

  • AdaptiveInput and GraphCanvas improvements (including graph drawing types).
  • Exam page: inline former useExamLogic behavior; expanded adaptive answering flow.
  • /api/weakness-paper route with rate limiting (uses async checkRateLimit from the base PR).
  • VoiceDictationButton in study techniques; confetti on daily completion and Pomodoro breaks.
  • UI polish: PDF viewer sticky toolbar, coach-voice feedback styling, coach shell / sidebar a11y, button/card micro-interactions.

Dependencies

Adds canvas-confetti and @types/canvas-confetti (not included in the base PR).

Made with Cursor

Summary by CodeRabbit

Release Notes

  • New Features

    • Added celebration confetti animations for session completions
    • Added voice dictation support for text input in study techniques
    • Added weakness paper generation from exam mistakes
    • Added adjustable PDF panel sizing in exams
  • Improvements

    • Fixed mobile navigation toggle behavior
    • Enhanced accessibility with proper navigation indicators
    • Improved button and card visual feedback with press effects
    • Optimized memory list filtering performance
    • Made PDF viewer tabs sticky during scrolling

…ening

- Remove monolithic studentOS.ts; add getSupabase helper and update modules
- Add optional Upstash Redis-backed rate limiting with in-memory fallback
- Await checkRateLimit in Hack Club and OpenRouter routes; cap Gemini inline base64 size
- Align coach pages and SessionDialog with shared studentOS row/types
- Migrate exam layout to react-resizable-panels v4 (Group/Separator)
- Fix TypeScript build issues (DOMPurify config typing, MathKeyboard labels, duplicate button attrs)

Made-with: Cursor
…tion UX

- Expand AdaptiveInput (MCQ, graph drawing types) and refactor GraphCanvas
- Inline exam logic into exam page; remove useExamLogic hook
- Add weakness-paper API, VoiceDictationButton, and confetti on milestones
- Coach shell and sidebar a11y; coach-voice styling for feedback blocks
- PDF viewer sticky controls; study technique modal and UI polish

Made-with: Cursor
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
aimarkerv2 Error Error Apr 5, 2026 5:14pm

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 5, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: be00b1b0-df0a-4eb5-b4c8-3f86e6e19159

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/adaptive-input-voice-graph-confetti

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@beenycool
Copy link
Copy Markdown
Owner Author

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several new features and UI enhancements, including a custom weakness paper generation API, voice dictation support, and celebratory confetti effects. Key technical changes include refactoring the AdaptiveInput and GraphCanvas components to TypeScript and optimizing graph rendering with a static layer cache. Feedback focuses on a regression in the Button component's default type, accessibility concerns regarding removed focus indicators on summary elements, and potential UX issues with global card animations. Additionally, improvements were suggested for error handling in the new API route and correcting a likely typo in the AI model identifier.

Comment on lines +43 to 50
({ className, variant, size, asChild = false, ...props }, ref) => {
const Comp = asChild ? Slot : "button";
return (
<Comp
className={cn(buttonVariants({ variant, size, className }))}
ref={ref}
{...(!asChild ? { type: type ?? "button" } : {})}
{...props}
/>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Removing the default type="button" for the Button component is a regression. In HTML, the default type for a <button> element is "submit". If this component is used inside a <form>, it will trigger a submission by default unless explicitly overridden. The previous implementation correctly defaulted to "button" to avoid this common pitfall, which is especially important since many call sites in this PR have also had their explicit type="button" removed.

    ({ className, variant, size, asChild = false, type, ...props }, ref) => {
        const Comp = asChild ? Slot : "button";
        return (
            <Comp
                className={cn(buttonVariants({ variant, size, className }))}
                ref={ref}
                {...(!asChild ? { type: type ?? "button" } : {})}
                {...props}
            />
        );

Comment thread app/(exam)/exam/page.tsx
Comment on lines +771 to +778
const togglePdfEmphasis = useCallback(() => {
setPdfEmphasized((prev) => {
const next = !prev;
queueMicrotask(() => {
pdfPanelRef.current?.resize(next ? '56%' : '50%');
});
return next;
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Triggering side effects like pdfPanelRef.current?.resize inside a state updater function is not recommended as state updaters should be pure. While queueMicrotask defers the execution, it is cleaner to handle this in a useEffect hook that synchronizes the panel size with the pdfEmphasized state to ensure predictable rendering behavior.

{hasChecklist ? (
<details className="group">
<summary className="list-none w-full py-2 bg-primary/5 text-primary border border-primary/10 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/10 transition-colors cursor-pointer select-none focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2">
<summary className="list-none w-full py-2 bg-primary/5 text-primary border border-primary/10 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/10 transition-colors cursor-pointer select-none">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The removal of focus-visible utility classes from the summary element negatively impacts accessibility. Since this is a standard HTML element and not the Button component (which now handles focus styles via variants), it will no longer show a focus ring when navigated via keyboard. Please restore the focus indicators to maintain WCAG compliance.

Suggested change
<summary className="list-none w-full py-2 bg-primary/5 text-primary border border-primary/10 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/10 transition-colors cursor-pointer select-none">
<summary className="list-none w-full py-2 bg-primary/5 text-primary border border-primary/10 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/10 transition-colors cursor-pointer select-none focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2">

Authorization: `Bearer ${apiKey}`,
},
body: JSON.stringify({
model: 'qwen/qwen3-32b',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The model name qwen/qwen3-32b appears to be a typo. As of current releases, the latest stable version is Qwen 2.5. Please verify if you intended to use qwen/qwen2.5-32b or a similar existing model name to avoid 404/400 errors from the AI provider.

Suggested change
model: 'qwen/qwen3-32b',
model: 'qwen/qwen2.5-32b',

ref={ref}
className={cn(
"rounded-lg border bg-card text-card-foreground shadow-sm",
"rounded-lg border bg-card text-card-foreground shadow-sm transition-transform duration-150 active:scale-[0.99]",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Adding active:scale-[0.99] to the base Card component may lead to poor UX. Cards are frequently used as static containers for content like text or forms. Having the entire container shrink on click can be distracting and feel 'mushy' when the user is trying to interact with elements inside the card (like selecting text or clicking an input). It is generally better to apply such micro-interactions only to explicitly interactive elements.

Comment on lines +146 to +147
const message = e instanceof Error ? e.message : 'Internal error';
return NextResponse.json({ error: message }, { status: 500 });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Returning e.message directly to the client in a 500 error response can potentially leak sensitive implementation details or stack traces. It is safer to log the full error on the server and return a generic error message to the user.

Suggested change
const message = e instanceof Error ? e.message : 'Internal error';
return NextResponse.json({ error: message }, { status: 500 });
return NextResponse.json({ error: 'Internal server error' }, { status: 500 });

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several features and improvements, including a new AI-powered weakness paper generator, a voice dictation component, celebration effects using confetti, and performance optimizations via memoization and canvas layering. Feedback highlights a regression in the Button component where the removal of the default type="button" may cause unintended form submissions. Additionally, it is recommended to avoid exposing raw external API error messages to the client in the weakness paper route and to parameterize the hardcoded language in the voice dictation component for better reusability.

Comment on lines +43 to 49
({ className, variant, size, asChild = false, ...props }, ref) => {
const Comp = asChild ? Slot : "button";
return (
<Comp
className={cn(buttonVariants({ variant, size, className }))}
ref={ref}
{...(!asChild ? { type: type ?? "button" } : {})}
{...props}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The removal of the default type="button" logic is a significant regression. In HTML, the default type for a <button> element inside a <form> is "submit". By removing this default, any Button component used within a form that does not explicitly set a type will now trigger form submission on click. This often leads to unintended side effects like page reloads or premature validation.

    ({ className, variant, size, asChild = false, type, ...props }, ref) => {
        const Comp = asChild ? Slot : "button";
        return (
            <Comp
                className={cn(buttonVariants({ variant, size, className }))}
                ref={ref}
                {...(!asChild ? { type: type ?? "button" } : {})}
                {...props}
            />
        );

Comment on lines +131 to +133
if (!hcRes.ok) {
const errText = await hcRes.text();
return NextResponse.json({ error: `AI error: ${hcRes.status} ${errText}` }, { status: 502 });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Directly returning errText from an external API call to the client can be problematic. It might expose internal implementation details or leak sensitive information from the upstream provider. Additionally, if the upstream service returns a large HTML error page, it could bloat the response. It's safer to log the full error on the server and return a generic, user-friendly error message.

Suggested change
if (!hcRes.ok) {
const errText = await hcRes.text();
return NextResponse.json({ error: `AI error: ${hcRes.status} ${errText}` }, { status: 502 });
if (!hcRes.ok) {
const errText = await hcRes.text();
console.error(`AI service error (${hcRes.status}):`, errText);
return NextResponse.json({ error: 'The AI service failed to generate the paper. Please try again later.' }, { status: 502 });
}

if (!SR) return;

const rec = new SR();
rec.lang = 'en-GB';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The language for speech recognition is hardcoded to en-GB. While appropriate for the current context, it limits the component's reusability. Consider making the language a prop with a default value, or deriving it from the application's locale settings if internationalization is planned.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
app/components/PDFViewer.tsx (1)

134-136: ⚠️ Potential issue | 🟡 Minor

Missing redrawAnnotations in useEffect dependency array.

The effect calls redrawAnnotations(), but the function is not in the dependency array. Since redrawAnnotations is recreated on every render and captures pageNumber, annotations, and annotationMode, the linter would flag this.

Consider wrapping redrawAnnotations in useCallback with appropriate dependencies, or inlining the logic.

♻️ Option 1: Wrap in useCallback
+    const redrawAnnotations = useCallback(() => {
-    const redrawAnnotations = () => {
         if (!annotationCanvasRef.current) return;
         // ... existing logic
-    };
+    }, [pageNumber, annotations, annotationMode]);

     useEffect(() => {
         redrawAnnotations();
-    }, [pageNumber, annotations, scale]);
+    }, [redrawAnnotations, scale]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/components/PDFViewer.tsx` around lines 134 - 136, redrawAnnotations is
recreated each render but used inside the useEffect at PDFViewer which causes a
missing-dependency lint issue; wrap redrawAnnotations in useCallback (including
its dependencies: pageNumber, annotations, annotationMode, scale if used) so it
has stable identity, then update the useEffect dependency array to include
redrawAnnotations (or alternatively inline the redraw logic into the effect).
Ensure the function signature and calls (redrawAnnotations, useEffect that
depends on [pageNumber, annotations, scale]) are updated consistently so the
linter no longer complains.
app/components/inputs/GraphCanvas.tsx (1)

41-46: ⚠️ Potential issue | 🟠 Major

Render the axes from the configured origin.

toCanvasX() / toCanvasY() honor xMin/xMax/yMin/yMax, but this block still hard-codes the Y axis to the left edge and the X axis to the bottom edge. Any graph that crosses zero now renders with the visible axes in the wrong place, even though points and labels are transformed with the configured bounds.

Proposed fix
-        ctx.moveTo(padding, padding);
-        ctx.lineTo(padding, height - padding);
-        ctx.moveTo(padding, height - padding);
-        ctx.lineTo(width - padding, height - padding);
+        const yAxisX = xMin <= 0 && 0 <= xMax ? toCanvasX(0) : padding;
+        const xAxisY = yMin <= 0 && 0 <= yMax ? toCanvasY(0) : height - padding;
+
+        ctx.moveTo(yAxisX, padding);
+        ctx.lineTo(yAxisX, height - padding);
+        ctx.moveTo(padding, xAxisY);
+        ctx.lineTo(width - padding, xAxisY);

Also applies to: 123-126

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/components/inputs/GraphCanvas.tsx` around lines 41 - 46, The axes are
being rendered at the canvas edges instead of at the configured origin; update
the GraphCanvas rendering to compute the canvas pixel positions for the origin
using the existing helpers (toCanvasX(0) and toCanvasY(0) or equivalent
transform functions that use xMin/xMax/yMin/yMax) and draw the vertical (Y) axis
at the computed canvasXOrigin and the horizontal (X) axis at canvasYOrigin
rather than the left/bottom edges; also move tick/label placement logic to use
those origin coordinates and clamp them inside the canvas bounds. Apply the same
change to the other axis-rendering block referenced (the second occurrence
around the 123-126 area) so both axes and their ticks/labels respect the
configured origin.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/`(coach)/memory/page.tsx:
- Line 142: Replace the string literal fallback 'preferences' with the shared
constant to avoid drift: when computing const cat = item.category ||
'preferences'; use MEMORY_CATEGORIES.PREFERENCES instead of the hardcoded string
so the fallback aligns with the enum/constant; update the expression that
defines cat (referencing item.category and MEMORY_CATEGORIES.PREFERENCES) so
grouping/labels/icons remain consistent.

In `@app/`(exam)/exam/page.tsx:
- Around line 1074-1083: The Group/Panel usage is using react-resizable-panels
v3 props but the app has v4; change Group's direction prop to orientation,
remove autoSaveId and instead call the v4 useDefaultLayout hook at the component
level to supply persistence, and convert numeric size props on Panel
(defaultSize, minSize, maxSize) from numbers to percentage strings (e.g.
"50%","22%","72%"); update the Panel with ref pdfPanelRef and all other Panel
instances inside this Group to use orientation and percentage strings and wire
layout persistence via useDefaultLayout rather than autoSaveId.

In `@app/api/weakness-paper/route.ts`:
- Around line 131-134: The handler currently returns raw upstream/internal error
details (errText from hcRes and e.message) to clients; instead, in the hcRes
error branch (the if (!hcRes.ok) that reads errText) and in the catch block that
references e, log the full diagnostics server-side (using your existing logger
or console.error) and return a generic client-safe NextResponse.json error
(e.g., "Upstream service error" with status 502 for hcRes failures, and a
generic "Internal server error" with status 500 for exceptions). Keep the
logging of hcRes.status, errText, and the caught error object for diagnostics,
but remove them from the JSON response sent to the client.
- Around line 115-129: The fetch call to HACKCLUB_API_URL (creating hcRes) has
no timeout and can hang; wrap the request in an AbortController with a
configurable timeout (e.g., 5–15s), pass controller.signal to fetch, and clear
the timeout on completion; in the route handler catch an AbortError (or check
controller.signal.aborted) and return a controlled timeout response (HTTP 504 or
similar) instead of letting the request hang. Update the fetch invocation where
hcRes is created and ensure the timeout is cleaned up after the response is
handled.

In `@app/components/AdaptiveInput.tsx`:
- Around line 46-57: normalizeGraphValue currently creates a fresh
GraphDrawingValue object on every render so memo(GraphCanvas) never sees a
stable value; wrap the normalizeGraphValue(...) call in a useMemo in the
AdaptiveInput component and pass that memoized result to the GraphCanvas value
prop (e.g. const memoizedGraph = useMemo(() => normalizeGraphValue(propsValue),
[propsValue, /* or a stable dep like JSON.stringify(propsValue) or specific
nested deps like propsValue?.points */] )). This keeps the same reference for
unchanged drawings and lets memo(GraphCanvas) avoid unnecessary re-renders;
reference normalizeGraphValue, GraphDrawingValue, GraphCanvas and the value prop
when applying the change.
- Around line 77-93: The component keeps a redundant local state
(figureBackground / setFigureBackground) that can get out of sync with the
upstream graphFigure prop; remove the local state and effect and use graphFigure
directly everywhere (delete the useState and the useEffect that sets
figureBackground), and update any onClearBackground handlers so they clear the
upstream source of truth (call the parent setter / passed callback that sets
graphFigure to null instead of setFigureBackground) so clearing truly updates
graphFigure.

In `@app/components/study/TechniqueModal.tsx`:
- Around line 319-322: The "Generate New Prompts" Button in TechniqueModal
should be disabled while generation is in progress to prevent concurrent AI
requests; update the Button (where onGenerate is passed) to set disabled based
on a boolean like isGenerating (or isLoading) and ensure onGenerate sets that
flag true at start and false on all exit paths (success/error/finally) inside
the TechniqueModal component (or the existing generate handler) so the UI cannot
trigger multiple concurrent calls; reference the onGenerate handler and the
TechniqueModal component to locate where to add the isGenerating state and the
disabled prop.

In `@app/components/ui/card.tsx`:
- Around line 11-14: The card component is applying an interactive style token
("active:scale-[0.99]") via the cn call on the container's className which
implies pressability for a non-interactive div; either remove the
"active:scale-[0.99]" from the className string in the cn call to keep the card
purely presentational, or make the component truly interactive by rendering a
semantic interactive element (e.g., <button> or an anchor) or wrapping content
in a Link and ensure any onClick-handling card exposes accessibility affordances
(add role="button", tabIndex={0} and keyboard handlers for Enter/Space) while
keeping the rest of the cn/className usage intact so styles remain consistent.

In `@app/components/UIComponents.tsx`:
- Line 387: The interactive <summary> elements (e.g., the one with className
"list-none w-full py-2 bg-primary/5 ...") lost explicit keyboard focus styling;
restore visible focus by adding explicit focus-visible utility classes to those
controls (for example add focus-visible:outline-2,
focus-visible:outline-offset-2 and a focus-visible:outline color class such as
focus-visible:outline-primary or equivalent to the element's className). Update
the same fix for the other affected interactive elements referenced in the
comment (the other <summary> instances and similar controls) so keyboard users
receive a clear focus ring.

In `@app/components/VoiceDictationButton.tsx`:
- Around line 83-91: The onerror handler in VoiceDictationButton currently
swallows errors; update rec.onerror to accept the error event (e.g., event) and
log useful details (like event.error or event.type) before calling
setActive(false) and clearing recRef.current; keep rec.onend unchanged. Use
console.error or the component's logger so microphone/permission/network issues
are visible during debugging.
- Around line 70-73: VoiceDictationButton currently hardcodes the
SpeechRecognition language to 'en-GB' by setting rec.lang = 'en-GB'; make this
configurable by adding an optional prop (e.g., lang?: string) on the
VoiceDictationButton component with a sensible default ('en-GB'), then use that
prop to set rec.lang (falling back to the default if undefined). Update the
component props type, pass the prop through to wherever the SR instance is
created (reference SR and the function or useEffect that constructs rec), and
ensure any callers are unaffected by providing the default when no lang prop is
supplied.

In `@app/lib/confetti.ts`:
- Around line 14-17: Wrap the body of burstConfetti in a try/catch so dynamic
import failures or confetti invocation errors don’t become unhandled rejections:
inside the exported async function burstConfetti(overrides?: Partial<Options>)
catch any thrown error from (await import('canvas-confetti')).default or
confetti(...) and handle it (e.g., console.debug/processLogger or silently
return) so callers using void burstConfetti(...) won’t produce unhandled promise
rejections; reference the existing defaults and overrides merge (confetti({
...defaults, ...overrides })) when placing the try block.

---

Outside diff comments:
In `@app/components/inputs/GraphCanvas.tsx`:
- Around line 41-46: The axes are being rendered at the canvas edges instead of
at the configured origin; update the GraphCanvas rendering to compute the canvas
pixel positions for the origin using the existing helpers (toCanvasX(0) and
toCanvasY(0) or equivalent transform functions that use xMin/xMax/yMin/yMax) and
draw the vertical (Y) axis at the computed canvasXOrigin and the horizontal (X)
axis at canvasYOrigin rather than the left/bottom edges; also move tick/label
placement logic to use those origin coordinates and clamp them inside the canvas
bounds. Apply the same change to the other axis-rendering block referenced (the
second occurrence around the 123-126 area) so both axes and their ticks/labels
respect the configured origin.

In `@app/components/PDFViewer.tsx`:
- Around line 134-136: redrawAnnotations is recreated each render but used
inside the useEffect at PDFViewer which causes a missing-dependency lint issue;
wrap redrawAnnotations in useCallback (including its dependencies: pageNumber,
annotations, annotationMode, scale if used) so it has stable identity, then
update the useEffect dependency array to include redrawAnnotations (or
alternatively inline the redraw logic into the effect). Ensure the function
signature and calls (redrawAnnotations, useEffect that depends on [pageNumber,
annotations, scale]) are updated consistently so the linter no longer complains.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 6e64b410-6596-4cff-8409-386e35646c89

📥 Commits

Reviewing files that changed from the base of the PR and between b4f6046 and 9fc1691.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (22)
  • app/(coach)/daily/page.tsx
  • app/(coach)/memory/page.tsx
  • app/(exam)/exam/page.tsx
  • app/api/weakness-paper/route.ts
  • app/components/AdaptiveInput.tsx
  • app/components/PDFViewer.tsx
  • app/components/UIComponents.tsx
  • app/components/VoiceDictationButton.tsx
  • app/components/coach/CoachShell.js
  • app/components/inputs/GraphCanvas.tsx
  • app/components/inputs/graphDrawingTypes.ts
  • app/components/layout/AppSidebar.tsx
  • app/components/study/PomodoroContent.tsx
  • app/components/study/TechniqueModal.tsx
  • app/components/ui/button.tsx
  • app/components/ui/card.tsx
  • app/globals.css
  • app/hooks/useExamLogic.js
  • app/lib/confetti.ts
  • app/test-component/page.tsx
  • package.json
  • tsconfig.tsbuildinfo
💤 Files with no reviewable changes (1)
  • app/hooks/useExamLogic.js

);
return filtered.reduce(
(acc, item) => {
const cat = item.category || 'preferences';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Use the category constant instead of a string literal fallback.

Line 142 hardcodes 'preferences'; this can silently diverge from MEMORY_CATEGORIES.PREFERENCES. Prefer the shared constant to keep grouping/labels/icons aligned.

Suggested diff
-                const cat = item.category || 'preferences';
+                const cat = item.category || MEMORY_CATEGORIES.PREFERENCES;
📝 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 cat = item.category || 'preferences';
const cat = item.category || MEMORY_CATEGORIES.PREFERENCES;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/`(coach)/memory/page.tsx at line 142, Replace the string literal fallback
'preferences' with the shared constant to avoid drift: when computing const cat
= item.category || 'preferences'; use MEMORY_CATEGORIES.PREFERENCES instead of
the hardcoded string so the fallback aligns with the enum/constant; update the
expression that defines cat (referencing item.category and
MEMORY_CATEGORIES.PREFERENCES) so grouping/labels/icons remain consistent.

Comment thread app/(exam)/exam/page.tsx
Comment on lines +1074 to +1083
<Group direction="horizontal" autoSaveId="exam-panels">
{/* PDF Viewer */}
<Panel defaultSize={50} minSize={20} className="relative z-10 hidden md:block">
<Panel
panelRef={pdfPanelRef}
id="exam-pdf"
defaultSize={50}
minSize={22}
maxSize={72}
className="relative z-10 hidden md:block"
>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find and read package.json to check react-resizable-panels version
fd package.json -type f | head -5 | xargs -I {} sh -c 'echo "File: {}"; cat {}'

Repository: beenycool/aimarkerv2

Length of output: 234


🏁 Script executed:

# Read the target file at lines 1074-1083
sed -n '1074,1083p' app/\(exam\)/exam/page.tsx

Repository: beenycool/aimarkerv2

Length of output: 529


🏁 Script executed:

# Check imports at the top of the file to see what's being imported from react-resizable-panels
head -100 app/\(exam\)/exam/page.tsx | grep -A 5 "react-resizable-panels"

Repository: beenycool/aimarkerv2

Length of output: 393


🏁 Script executed:

# Find package.json files and check react-resizable-panels version
find . -name "package.json" -exec cat {} \; | grep -A 2 -B 2 "react-resizable-panels"

Repository: beenycool/aimarkerv2

Length of output: 221


🌐 Web query:

react-resizable-panels v4 API documentation direction orientation prop

💡 Result:

In react-resizable-panels v4, the Group component (formerly PanelGroup in v3) uses the orientation prop to specify the resizable direction. There is no direction prop in v4; it was renamed from v3 to better align with ARIA standards. The orientation prop: - Type: "horizontal" | "vertical" - Default: "horizontal" - Description: Specifies the resizable orientation ("horizontal" or "vertical"); defaults to "horizontal". Example usage: import { Group, Panel, Separator } from "react-resizable-panels"; Content 1 Content 2 This is confirmed across official documentation sources including the official site, README, and v4 release notes.

Citations:


🌐 Web query:

react-resizable-panels v4 vs v3 autoSaveId useDefaultLayout API changes

💡 Result:

In react-resizable-panels v4 (released Dec 2025), the autoSaveId prop from v3 (on PanelGroup) has been replaced by the useDefaultLayout hook for persistent layouts. The v3 PanelGroup autoSaveId="unique-group-id" automatically saved/restored layouts to localStorage. In v4, use the hook to achieve the same: import { Group, Panel, Separator, useDefaultLayout } from "react-resizable-panels"; const { defaultLayout, onLayoutChanged } = useDefaultLayout({ id: "unique-group-id" // Replaces autoSaveId; uses localStorage by default }); left right Key changes: - PanelGroup -> Group (direction -> orientation="horizontal") - PanelResizeHandle -> Separator - autoSaveId -> useDefaultLayout({ id }) - Saves per panel id combination if ids provided on Panels (e.g. ); falls back otherwise - v4.8.0+ (Mar 2026): useDefaultLayout auto-migrates v3 legacy localStorage formats - Supports custom storage: useDefaultLayout({ id, storage: sessionStorage }) - Group also accepts direct defaultLayout prop + onLayoutChanged for manual persistence - defaultLayout is uncontrolled (initial only); use imperative refs (useGroupRef) for programmatic control Other major v4 changes: pixel/%/rem sizes (defaultSize="200px"), improved SSR, no more onCollapse/onExpand (use onResize), disableGlobalCursorStyles -> . Official migration guide: GitHub CHANGELOG.md#migrating-from-version-3-to-4 and releases/tag/4.0.0. Docs: react-resizable-panels.vercel.app.

Citations:


🌐 Web query:

react-resizable-panels v4 numeric sizes pixels vs percentages defaultSize minSize

💡 Result:

In react-resizable-panels v4, Panel props like defaultSize and minSize accept flexible units for size constraints. Numeric values (e.g. defaultSize={200}, minSize={20}) are assumed to be pixels. Strings without units (e.g. defaultSize="200") are assumed to be percentages (0-100%). Explicit units can be used as strings: "200px" for pixels, "20%" for percentages, or other CSS units like "1rem", "10vh", "5vw", "em". Examples from docs and code: - defaultSize={30} treats as 30px - defaultSize="30" treats as 30% - defaultSize="200px" is 200px - defaultSize={"200px"} is 200px (string) This applies to defaultSize, minSize, maxSize, collapsedSize. Official documentation confirms: "Numeric values are assumed to be pixels. Strings without explicit units are assumed to be percentages (0%..100%). Percentages may also be specified as strings ending with '%' (e.g. '33%'). Pixels may also be specified as strings ending with the unit 'px'." Note: Percentage-based sizes may cause slight layout shift during server-rendering.

Citations:


Replace v3 props with v4 equivalents: use orientation instead of direction, replace autoSaveId with useDefaultLayout, and convert numeric sizes to percentage strings.

The installed version is react-resizable-panels v4.6.4. In v4, direction was renamed to orientation, autoSaveId was removed in favor of the useDefaultLayout hook for persistence, and numeric sizes are interpreted as pixels—not percentages. This means defaultSize={50}, minSize={22}, and maxSize={72} equate to 50px, 22px, and 72px, rendering the PDF pane as a thumbnail. Update the Group and both Panel components:

Required changes
-                    <Group direction="horizontal" autoSaveId="exam-panels">
+                    <Group orientation="horizontal" {...layoutProps}>

Convert all Panel size props to percentage strings:

-                            defaultSize={50}
-                            minSize={22}
-                            maxSize={72}
+                            defaultSize="50%"
+                            minSize="22%"
+                            maxSize="72%"

Add persistence via useDefaultLayout hook at the component level:

+const { defaultLayout, onLayoutChanged } = useDefaultLayout({
+  id: "exam-panels",
+});
+const layoutProps = {
+  defaultLayout,
+  onLayoutChanged,
+};

Apply the same fixes to all other Panel components in the Group.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/`(exam)/exam/page.tsx around lines 1074 - 1083, The Group/Panel usage is
using react-resizable-panels v3 props but the app has v4; change Group's
direction prop to orientation, remove autoSaveId and instead call the v4
useDefaultLayout hook at the component level to supply persistence, and convert
numeric size props on Panel (defaultSize, minSize, maxSize) from numbers to
percentage strings (e.g. "50%","22%","72%"); update the Panel with ref
pdfPanelRef and all other Panel instances inside this Group to use orientation
and percentage strings and wire layout persistence via useDefaultLayout rather
than autoSaveId.

Comment on lines +115 to +129
const hcRes = await fetch(HACKCLUB_API_URL, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${apiKey}`,
},
body: JSON.stringify({
model: 'qwen/qwen3-32b',
temperature: 0.35,
messages: [
{ role: 'system', content: system },
{ role: 'user', content: userMsg },
],
}),
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add an explicit timeout to the upstream AI request.

This external call currently waits indefinitely until platform timeout, which can degrade route reliability under upstream stalls. Bound it with an abort timeout and return a controlled timeout response.

Suggested fix
-        const hcRes = await fetch(HACKCLUB_API_URL, {
-            method: 'POST',
-            headers: {
-                'Content-Type': 'application/json',
-                Authorization: `Bearer ${apiKey}`,
-            },
-            body: JSON.stringify({
-                model: 'qwen/qwen3-32b',
-                temperature: 0.35,
-                messages: [
-                    { role: 'system', content: system },
-                    { role: 'user', content: userMsg },
-                ],
-            }),
-        });
+        const controller = new AbortController();
+        const timeout = setTimeout(() => controller.abort(), 15_000);
+        let hcRes: Response;
+        try {
+            hcRes = await fetch(HACKCLUB_API_URL, {
+                method: 'POST',
+                headers: {
+                    'Content-Type': 'application/json',
+                    Authorization: `Bearer ${apiKey}`,
+                },
+                body: JSON.stringify({
+                    model: 'qwen/qwen3-32b',
+                    temperature: 0.35,
+                    messages: [
+                        { role: 'system', content: system },
+                        { role: 'user', content: userMsg },
+                    ],
+                }),
+                signal: controller.signal,
+            });
+        } catch (err) {
+            if ((err as Error)?.name === 'AbortError') {
+                return NextResponse.json({ error: 'AI service timed out. Please try again.' }, { status: 504 });
+            }
+            throw err;
+        } finally {
+            clearTimeout(timeout);
+        }
📝 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 hcRes = await fetch(HACKCLUB_API_URL, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${apiKey}`,
},
body: JSON.stringify({
model: 'qwen/qwen3-32b',
temperature: 0.35,
messages: [
{ role: 'system', content: system },
{ role: 'user', content: userMsg },
],
}),
});
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), 15_000);
let hcRes: Response;
try {
hcRes = await fetch(HACKCLUB_API_URL, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${apiKey}`,
},
body: JSON.stringify({
model: 'qwen/qwen3-32b',
temperature: 0.35,
messages: [
{ role: 'system', content: system },
{ role: 'user', content: userMsg },
],
}),
signal: controller.signal,
});
} catch (err) {
if ((err as Error)?.name === 'AbortError') {
return NextResponse.json({ error: 'AI service timed out. Please try again.' }, { status: 504 });
}
throw err;
} finally {
clearTimeout(timeout);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/weakness-paper/route.ts` around lines 115 - 129, The fetch call to
HACKCLUB_API_URL (creating hcRes) has no timeout and can hang; wrap the request
in an AbortController with a configurable timeout (e.g., 5–15s), pass
controller.signal to fetch, and clear the timeout on completion; in the route
handler catch an AbortError (or check controller.signal.aborted) and return a
controlled timeout response (HTTP 504 or similar) instead of letting the request
hang. Update the fetch invocation where hcRes is created and ensure the timeout
is cleaned up after the response is handled.

Comment on lines +131 to +134
if (!hcRes.ok) {
const errText = await hcRes.text();
return NextResponse.json({ error: `AI error: ${hcRes.status} ${errText}` }, { status: 502 });
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Stop returning raw upstream/internal error details to clients.

errText and e.message can expose internals. Log detailed diagnostics server-side, but return generic client-safe messages.

Suggested fix
         if (!hcRes.ok) {
             const errText = await hcRes.text();
-            return NextResponse.json({ error: `AI error: ${hcRes.status} ${errText}` }, { status: 502 });
+            console.error('weakness-paper upstream error', {
+                status: hcRes.status,
+                body: errText.slice(0, 500),
+            });
+            return NextResponse.json({ error: 'AI service request failed. Please try again.' }, { status: 502 });
         }
@@
     } catch (e) {
         console.error('weakness-paper route:', e);
-        const message = e instanceof Error ? e.message : 'Internal error';
-        return NextResponse.json({ error: message }, { status: 500 });
+        return NextResponse.json({ error: 'Internal error' }, { status: 500 });
     }
 }

Also applies to: 145-147

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/weakness-paper/route.ts` around lines 131 - 134, The handler
currently returns raw upstream/internal error details (errText from hcRes and
e.message) to clients; instead, in the hcRes error branch (the if (!hcRes.ok)
that reads errText) and in the catch block that references e, log the full
diagnostics server-side (using your existing logger or console.error) and return
a generic client-safe NextResponse.json error (e.g., "Upstream service error"
with status 502 for hcRes failures, and a generic "Internal server error" with
status 500 for exceptions). Keep the logging of hcRes.status, errText, and the
caught error object for diagnostics, but remove them from the JSON response sent
to the client.

Comment on lines +46 to 57
function normalizeGraphValue(v: AdaptiveInputValue): GraphDrawingValue {
if (v && typeof v === 'object' && !Array.isArray(v) && 'points' in v) {
const g = v as GraphDrawingValue;
return {
points: g.points || [],
lines: g.lines || [],
labels: g.labels || [],
paths: g.paths || [],
};
}
return { ...emptyGraph };
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Memoize the normalized graph value before passing it to GraphCanvas.

This helper allocates a fresh GraphDrawingValue object on every render, so memo(GraphCanvas) never receives a stable value prop. In app/(exam)/exam/page.tsx, timeElapsed ticks every second, which means the graph input rerenders once per timer tick even when the drawing is unchanged.

Proposed fix
-import React, { useState, useId, useEffect, memo, useCallback } from 'react';
+import React, { useState, useId, useEffect, memo, useCallback, useMemo } from 'react';
...
+        const normalizedGraphValue = useMemo(() => normalizeGraphValue(value), [value]);
+
         if (type === 'graph_drawing') {
             return (
                 <div className="bg-card p-4 rounded-xl border border-border">
@@
                     <GraphCanvas
                         config={graphConfig}
-                        value={normalizeGraphValue(value)}
+                        value={normalizedGraphValue}
                         onChange={(g) => onChange(g)}
                         backgroundImage={figureBackground}
                         onClearBackground={() => setFigureBackground(null)}

Also applies to: 230-233

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/components/AdaptiveInput.tsx` around lines 46 - 57, normalizeGraphValue
currently creates a fresh GraphDrawingValue object on every render so
memo(GraphCanvas) never sees a stable value; wrap the normalizeGraphValue(...)
call in a useMemo in the AdaptiveInput component and pass that memoized result
to the GraphCanvas value prop (e.g. const memoizedGraph = useMemo(() =>
normalizeGraphValue(propsValue), [propsValue, /* or a stable dep like
JSON.stringify(propsValue) or specific nested deps like propsValue?.points */]
)). This keeps the same reference for unchanged drawings and lets
memo(GraphCanvas) avoid unnecessary re-renders; reference normalizeGraphValue,
GraphDrawingValue, GraphCanvas and the value prop when applying the change.

Comment on lines 11 to 14
className={cn(
"rounded-lg border bg-card text-card-foreground shadow-sm",
"rounded-lg border bg-card text-card-foreground shadow-sm transition-transform duration-150 active:scale-[0.99]",
className
)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider if active:scale is appropriate for a container element.

Cards are typically non-interactive containers (<div>). Adding active:scale-[0.99] provides visual press feedback, which implies interactivity. However:

  1. If the card itself should be clickable, consider making it a <button> or wrapping content in a link for keyboard accessibility
  2. If only elements inside the card are interactive, the whole-card scale effect may confuse users about what's clickable

If cards are intentionally made clickable via onClick, ensure they also have role="button", tabIndex={0}, and keyboard event handlers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/components/ui/card.tsx` around lines 11 - 14, The card component is
applying an interactive style token ("active:scale-[0.99]") via the cn call on
the container's className which implies pressability for a non-interactive div;
either remove the "active:scale-[0.99]" from the className string in the cn call
to keep the card purely presentational, or make the component truly interactive
by rendering a semantic interactive element (e.g., <button> or an anchor) or
wrapping content in a Link and ensure any onClick-handling card exposes
accessibility affordances (add role="button", tabIndex={0} and keyboard handlers
for Enter/Space) while keeping the rest of the cn/className usage intact so
styles remain consistent.

{hasChecklist ? (
<details className="group">
<summary className="list-none w-full py-2 bg-primary/5 text-primary border border-primary/10 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/10 transition-colors cursor-pointer select-none focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2">
<summary className="list-none w-full py-2 bg-primary/5 text-primary border border-primary/10 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/10 transition-colors cursor-pointer select-none">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Re-add explicit focus-visible styles on interactive controls.

These controls lost explicit focus treatment; please preserve visible keyboard focus indicators for consistent accessibility.

Suggested fix
-<summary className="list-none w-full py-2 bg-primary/5 text-primary border border-primary/10 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/10 transition-colors cursor-pointer select-none">
+<summary className="list-none w-full py-2 bg-primary/5 text-primary border border-primary/10 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/10 transition-colors cursor-pointer select-none focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2">

-<button type="button" onClick={onExplain} disabled={explaining} className="w-full py-2 bg-primary/10 text-primary border border-primary/20 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/20 transition-colors">
+<button type="button" onClick={onExplain} disabled={explaining} className="w-full py-2 bg-primary/10 text-primary border border-primary/20 rounded-lg text-sm font-semibold flex items-center justify-center gap-2 hover:bg-primary/20 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2">

-<button type="button" aria-label="Send follow-up question" title="Send follow-up question" onClick={handleSend} disabled={sendingFollowUp || !followUpText.trim()} className="bg-primary text-primary-foreground p-2 rounded-lg hover:bg-primary/90 disabled:opacity-50">
+<button type="button" aria-label="Send follow-up question" title="Send follow-up question" onClick={handleSend} disabled={sendingFollowUp || !followUpText.trim()} className="bg-primary text-primary-foreground p-2 rounded-lg hover:bg-primary/90 disabled:opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2">

-<button type="button" onClick={onRetry} className="w-full mt-2 py-3 border border-border rounded-lg font-semibold flex items-center justify-center gap-2 text-sm text-foreground hover:bg-muted/30 transition-colors">
+<button type="button" onClick={onRetry} className="w-full mt-2 py-3 border border-border rounded-lg font-semibold flex items-center justify-center gap-2 text-sm text-foreground hover:bg-muted/30 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2">

-<button type="button" onClick={onNext} className="w-full mt-2 py-3 bg-primary hover:bg-primary/90 text-primary-foreground rounded-lg font-semibold flex items-center justify-center gap-2 transition-colors">
+<button type="button" onClick={onNext} className="w-full mt-2 py-3 bg-primary hover:bg-primary/90 text-primary-foreground rounded-lg font-semibold flex items-center justify-center gap-2 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2">

Also applies to: 398-399, 437-437, 444-444, 449-449

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/components/UIComponents.tsx` at line 387, The interactive <summary>
elements (e.g., the one with className "list-none w-full py-2 bg-primary/5 ...")
lost explicit keyboard focus styling; restore visible focus by adding explicit
focus-visible utility classes to those controls (for example add
focus-visible:outline-2, focus-visible:outline-offset-2 and a
focus-visible:outline color class such as focus-visible:outline-primary or
equivalent to the element's className). Update the same fix for the other
affected interactive elements referenced in the comment (the other <summary>
instances and similar controls) so keyboard users receive a clear focus ring.

Comment on lines +70 to +73
const rec = new SR();
rec.lang = 'en-GB';
rec.interimResults = false;
rec.continuous = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider making the language configurable.

The language is hardcoded to 'en-GB'. For international users or different English variants, consider accepting an optional lang prop with a sensible default.

♻️ Suggested enhancement
 export interface VoiceDictationButtonProps {
     onAppend: (text: string) => void;
     className?: string;
     disabled?: boolean;
+    lang?: string;
 }

-export function VoiceDictationButton({ onAppend, className, disabled }: VoiceDictationButtonProps) {
+export function VoiceDictationButton({ onAppend, className, disabled, lang = 'en-GB' }: VoiceDictationButtonProps) {
     // ...
         const rec = new SR();
-        rec.lang = 'en-GB';
+        rec.lang = lang;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/components/VoiceDictationButton.tsx` around lines 70 - 73,
VoiceDictationButton currently hardcodes the SpeechRecognition language to
'en-GB' by setting rec.lang = 'en-GB'; make this configurable by adding an
optional prop (e.g., lang?: string) on the VoiceDictationButton component with a
sensible default ('en-GB'), then use that prop to set rec.lang (falling back to
the default if undefined). Update the component props type, pass the prop
through to wherever the SR instance is created (reference SR and the function or
useEffect that constructs rec), and ensure any callers are unaffected by
providing the default when no lang prop is supplied.

Comment on lines +83 to +91
rec.onerror = () => {
setActive(false);
recRef.current = null;
};

rec.onend = () => {
setActive(false);
recRef.current = null;
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider logging speech recognition errors for debugging.

The onerror handler silently fails. During development or for troubleshooting, logging the error type can help diagnose issues (e.g., microphone permissions, network errors for cloud recognition).

-        rec.onerror = () => {
+        rec.onerror = (e) => {
+            console.warn('SpeechRecognition error:', e);
             setActive(false);
             recRef.current = null;
         };
📝 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
rec.onerror = () => {
setActive(false);
recRef.current = null;
};
rec.onend = () => {
setActive(false);
recRef.current = null;
};
rec.onerror = (e) => {
console.warn('SpeechRecognition error:', e);
setActive(false);
recRef.current = null;
};
rec.onend = () => {
setActive(false);
recRef.current = null;
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/components/VoiceDictationButton.tsx` around lines 83 - 91, The onerror
handler in VoiceDictationButton currently swallows errors; update rec.onerror to
accept the error event (e.g., event) and log useful details (like event.error or
event.type) before calling setActive(false) and clearing recRef.current; keep
rec.onend unchanged. Use console.error or the component's logger so
microphone/permission/network issues are visible during debugging.

Comment thread app/lib/confetti.ts
Comment on lines +14 to +17
export async function burstConfetti(overrides?: Partial<Options>): Promise<void> {
if (typeof window === 'undefined') return;
const confetti = (await import('canvas-confetti')).default;
await confetti({ ...defaults, ...overrides });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify fire-and-forget call sites that would surface unhandled rejections if burstConfetti throws.
rg -nP '\bvoid\s+burstConfetti\s*\(' --type=ts --type=tsx
rg -nP '\bburstConfetti\s*\(' --type=ts --type=tsx

Repository: beenycool/aimarkerv2

Length of output: 119


🏁 Script executed:

#!/bin/bash
# Search for burstConfetti call sites using basic grep first
rg -n 'burstConfetti' --type ts

# Also check the full confetti.ts file to understand implementation
cat -n app/lib/confetti.ts

Repository: beenycool/aimarkerv2

Length of output: 1435


Handle confetti failures inside the helper to avoid unhandled rejections.

All three call sites use the fire-and-forget pattern (void burstConfetti(...)), meaning any errors from the dynamic import or confetti invocation would become unhandled rejections. Wrap the implementation in try/catch to handle failures gracefully.

Suggested fix
 export async function burstConfetti(overrides?: Partial<Options>): Promise<void> {
     if (typeof window === 'undefined') return;
-    const confetti = (await import('canvas-confetti')).default;
-    await confetti({ ...defaults, ...overrides });
+    try {
+        const confetti = (await import('canvas-confetti')).default;
+        await confetti({ ...defaults, ...overrides });
+    } catch (error) {
+        console.warn('Confetti unavailable:', error);
+    }
 }
📝 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
export async function burstConfetti(overrides?: Partial<Options>): Promise<void> {
if (typeof window === 'undefined') return;
const confetti = (await import('canvas-confetti')).default;
await confetti({ ...defaults, ...overrides });
export async function burstConfetti(overrides?: Partial<Options>): Promise<void> {
if (typeof window === 'undefined') return;
try {
const confetti = (await import('canvas-confetti')).default;
await confetti({ ...defaults, ...overrides });
} catch (error) {
console.warn('Confetti unavailable:', error);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/confetti.ts` around lines 14 - 17, Wrap the body of burstConfetti in
a try/catch so dynamic import failures or confetti invocation errors don’t
become unhandled rejections: inside the exported async function
burstConfetti(overrides?: Partial<Options>) catch any thrown error from (await
import('canvas-confetti')).default or confetti(...) and handle it (e.g.,
console.debug/processLogger or silently return) so callers using void
burstConfetti(...) won’t produce unhandled promise rejections; reference the
existing defaults and overrides merge (confetti({ ...defaults, ...overrides }))
when placing the try block.

@beenycool beenycool force-pushed the refactor/studentos-upstash-ratelimit branch from b4f6046 to 10f8295 Compare April 6, 2026 11:00
@beenycool beenycool deleted the branch refactor/studentos-upstash-ratelimit April 6, 2026 11:00
@beenycool beenycool closed this Apr 6, 2026
beenycool added a commit that referenced this pull request Apr 6, 2026
beenycool added a commit that referenced this pull request Apr 6, 2026
beenycool added a commit that referenced this pull request Apr 6, 2026
* feat: adaptive exam input, graph canvas, voice dictation, and celebration UX

- Expand AdaptiveInput (MCQ, graph drawing types) and refactor GraphCanvas
- Inline exam logic into exam page; remove useExamLogic hook
- Add weakness-paper API, VoiceDictationButton, and confetti on milestones
- Coach shell and sidebar a11y; coach-voice styling for feedback blocks
- PDF viewer sticky controls; study technique modal and UI polish

Made-with: Cursor

* Apply reviewer suggestions for PR #160

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant