scratchpad and admin interface fix. - #65
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSuccessful login now performs a full reload. The course workspace adds a Monaco scratchpad with persisted code, stdin, execution output, and language support. Chat code blocks can run code or open in the scratchpad. Backend services execute scratchpad submissions through Judge0. ChangesScratchpad execution
Authentication reload
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Student
participant CourseWorkspace
participant Scratchpad
participant scratchpadExecute
participant ScratchpadRoute
participant Judge0
Student->>CourseWorkspace: Open scratchpad or run assistant code
CourseWorkspace->>Scratchpad: Load code and language
Scratchpad->>scratchpadExecute: Submit code, language ID, and stdin
scratchpadExecute->>ScratchpadRoute: POST /scratchpad/execute
ScratchpadRoute->>Judge0: Execute free-run submission
Judge0-->>ScratchpadRoute: Return output and status
ScratchpadRoute-->>scratchpadExecute: Return execution result
scratchpadExecute-->>Scratchpad: Display status and output
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
🚀 Admin Preview Deployment Successful!
|
🚀 Preview Deployment Successful!
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (8)
web/src/routes/_authenticated.course.tsx (1)
385-385: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the formatting of
handleSelectLesson.The
navigate(call sits on the same line as the opening brace. The body is then misaligned.♻️ Proposed change
- function handleSelectLesson(id: string) { navigate({ + function handleSelectLesson(id: string) { + navigate({ to: "/course", search: { problemId: id }, });🤖 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 `@web/src/routes/_authenticated.course.tsx` at line 385, Reformat the handleSelectLesson function so navigate({ starts on a new, properly indented line after the opening brace, with the rest of the function body consistently aligned.server/routers.py (1)
43-50: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider validating
language_idagainst an allowlist.The route forwards any integer to Judge0. The frontend uses a fixed set of seven language IDs. An allowlist keeps the server contract aligned with the client and rejects invalid IDs before the upstream call.
Note on the Ruff
B008hint at Line 48:Depends(...)in the argument default is the required FastAPI pattern, and the existingjudge0_executionroute uses the same form. No change is needed.🤖 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 `@server/routers.py` around lines 43 - 50, Validate body.language_id in scratchpad_execution against the frontend’s fixed seven-language allowlist before calling services.run_scratchpad, and reject unsupported IDs through the route’s existing request-validation mechanism. Keep the Judge0 call unchanged for allowed IDs and leave the Depends(get_current_user_with_token) parameter pattern intact.Source: Linters/SAST tools
server/services/__init__.py (1)
270-273: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffConsider reusing a shared
httpx.AsyncClient.
_judge0_submitcreates a client for every call. That repeats the TLS handshake on each scratchpad run. A module-level client, or one attached to the application lifespan, reuses connections. The graded path at Lines 348 and 383 has the same pattern, so a shared client benefits both.🤖 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 `@server/services/__init__.py` around lines 270 - 273, Update _judge0_submit and the graded paths around lines 348 and 383 to reuse a shared httpx.AsyncClient instead of creating a new client per request. Initialize the client at module or application-lifespan scope and ensure it is properly closed during shutdown, while preserving the existing request payloads, headers, timeout, and response handling.web/src/components/student/Scratchpad.tsx (1)
59-63: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDebounce the
localStoragewrite.
handleChangewrites tolocalStorageon every keystroke.localStorage.setItemis synchronous and blocks the main thread. With a large snippet this causes typing lag in the editor. The course route already uses a 5-second interval save for the main editor at Lines 154-167; apply a similar approach here.🤖 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 `@web/src/components/student/Scratchpad.tsx` around lines 59 - 63, Debounce the localStorage persistence in handleChange instead of calling localStorage.setItem on every keystroke. Track the latest scratchpad value and save it through a 5-second interval or equivalent delayed mechanism, while keeping setValue immediate and ensuring pending timers or intervals are cleaned up appropriately.web/src/lib/api.ts (2)
47-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winModel the client-side error shape in the result type.
Scratchpad.tsxandChatBox.tsxboth store{ error: true, stderr }in the same result state. That shape is not part ofScratchpadRunResult. Both consumers hold the state asany, so the mismatch is invisible to the compiler. Add the field to the shared type and let the consumers type their state.♻️ Proposed change
export interface ScratchpadRunResult { + error?: boolean; stdout?: string; stderr?: string; compile_output?: string; time?: string; memory?: number; status?: { id: number; description: string }; status_id?: number; }🤖 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 `@web/src/lib/api.ts` around lines 47 - 55, Extend ScratchpadRunResult with an optional boolean error field to represent the { error: true, stderr } client-side result shape, then replace the any result-state annotations in Scratchpad.tsx and ChatBox.tsx with ScratchpadRunResult so both consumers use the shared type.
62-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared token and header logic.
Lines 62-63 and Line 74 repeat the block from
sendChatMessageat Lines 28-29 and Line 41. Extract a small helper so the bearer-token contract stays in one place.♻️ Proposed helper
async function authHeaders(): Promise<Record<string, string>> { const tokenRes = await authClient.convex.token(); const token = tokenRes.data?.token; return token ? { Authorization: `Bearer ${token}` } : {}; }🤖 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 `@web/src/lib/api.ts` around lines 62 - 75, Extract the repeated token retrieval and Authorization header construction from sendChatMessage and the scratchpad request into a shared async authHeaders helper. Update both call sites to await this helper and pass its result as the request headers, preserving the existing behavior when no token is available.web/src/components/student/problem/ChatBox.tsx (1)
97-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winJudge0 language IDs and the run-and-shape-error logic are duplicated across the scratchpad surfaces. The shared root cause is that no module owns the language catalogue or the execution result handling, so each component defines its own copy. The copies already diverge:
web/src/routes/_authenticated.course.tsxLines 270-279 includessql: 82, while the two new maps do not. Extract one language module and one run helper, then import both.
web/src/components/student/problem/ChatBox.tsx#L97-L105: remove the localLANGUAGE_IDSobject and import the shared language catalogue. Moving it out of the component body also stops the map from being recreated on every render.web/src/components/student/Scratchpad.tsx#L11-L19: replace the localLANGUAGESmap with the shared catalogue, keeping the display labels as part of that shared definition.web/src/components/student/Scratchpad.tsx#L65-L84: replace the body with a call to a shared run helper that wrapsscratchpadExecuteand returns the normalized error shape.web/src/components/student/problem/ChatBox.tsx#L107-L122: replace the body with the same shared run helper.🤖 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 `@web/src/components/student/problem/ChatBox.tsx` around lines 97 - 105, Extract a shared language catalogue and run helper: in web/src/components/student/problem/ChatBox.tsx:97-105 remove LANGUAGE_IDS and import the catalogue; in web/src/components/student/Scratchpad.tsx:11-19 replace LANGUAGES with that catalogue, preserving display labels and including sql. In web/src/components/student/Scratchpad.tsx:65-84 and web/src/components/student/problem/ChatBox.tsx:107-122 replace each local execution/error-shaping body with the shared helper wrapping scratchpadExecute and returning the normalized error shape.server/model/scratchpad.py (1)
5-7: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd size limits to
codeandstdin.The model accepts a payload of any size. Each accepted request forwards the full body to Judge0. Add
max_lengthconstraints to bound the request body and the upstream load.♻️ Proposed change
-from pydantic import BaseModel +from pydantic import BaseModel, Field from typing import Optional class ScratchpadCode(BaseModel): - code: str + code: str = Field(..., max_length=50_000) language_id: Optional[int] = 71 - stdin: Optional[str] = None + stdin: Optional[str] = Field(default=None, max_length=10_000)🤖 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 `@server/model/scratchpad.py` around lines 5 - 7, Update the scratchpad request model fields code and stdin to enforce appropriate max_length constraints, limiting both the submitted source and standard input before forwarding them to Judge0 while preserving their existing types and defaults.
🤖 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 `@admin/src/components/Login.tsx`:
- Line 183: In the login redirect flow containing window.location.assign,
replace the navigation call with window.location.replace("/") so the application
reloads without adding the login page to browser history.
In `@server/model/scratchpad.py`:
- Line 6: Update the language_id field in the scratchpad model to prevent
explicit null values from reaching run_scratchpad and _judge0_submit: use a
non-optional defaulted field or coerce None to 71 before constructing the Judge0
payload, while preserving the default when the key is omitted.
In `@server/services/__init__.py`:
- Around line 282-292: Update the Judge0 exception handling around the three
raises in the request flow: chain each re-raised exception with “from e” to
satisfy B904, map upstream 4xx authentication and rate-limit responses
(including 401, 403, and 429) to HTTP 502, and return fixed non-sensitive
messages for 5xx and auth failures. Keep upstream status, response text, and
parsed details in logs only; do not expose Judge0 message/error content or
status codes to clients.
In `@web/src/components/student/problem/ChatBox.tsx`:
- Around line 69-79: Update normalizeCodeLanguage and the code-block rendering
logic to distinguish supported runner languages from unknown fence languages:
move LANGUAGE_IDS to module scope, and render neither the Run button nor the
Scratchpad button unless the normalized language has a runner ID. Preserve
existing behavior for recognized languages while preventing unknown languages
such as bash, json, sql, or text from falling back to Python.
- Around line 191-193: Update the output rendering in ChatBox’s result display
so non-empty stdout does not suppress stderr or compile_output; render the
available stdout and error output together, matching Scratchpad.tsx’s behavior
while retaining the "(empty output)" fallback when no output exists.
- Around line 244-270: Update the ReactMarkdown components configuration in
ChatBox to override the pre renderer and return its children directly,
preventing fenced code blocks from wrapping the CodeBlock div in an invalid pre
element. Keep the existing code renderer and CodeBlock behavior unchanged.
In `@web/src/components/student/Scratchpad.tsx`:
- Around line 40-53: Update the openWith method in useImperativeHandle to
persist the loaded code to exemplai_scratchpad, matching the storage behavior of
handleChange and handleReset, while preserving the existing language, editor,
value, and result updates.
In `@web/src/routes/_authenticated.course.tsx`:
- Around line 356-362: Update handleOpenScratchpad to store the pending code and
snippetLanguage in state instead of using setTimeout. Apply that pending snippet
after Scratchpad mounts via an effect keyed to the pending snippet (or
isScratchpadVisible and the pending snippet), invoking
scratchpadRef.current?.openWith only when the ref is available, so the snippet
is not lost.
- Around line 487-510: Reset editorRef.current when CodeEditor is unmounted as
isScratchpadVisible becomes true, using the existing visibility state and effect
flow near the course component. Ensure autosave, handleSave,
handleSendErrorToChat, and ChatBox cannot access the disposed Monaco instance
while the scratchpad is open.
---
Nitpick comments:
In `@server/model/scratchpad.py`:
- Around line 5-7: Update the scratchpad request model fields code and stdin to
enforce appropriate max_length constraints, limiting both the submitted source
and standard input before forwarding them to Judge0 while preserving their
existing types and defaults.
In `@server/routers.py`:
- Around line 43-50: Validate body.language_id in scratchpad_execution against
the frontend’s fixed seven-language allowlist before calling
services.run_scratchpad, and reject unsupported IDs through the route’s existing
request-validation mechanism. Keep the Judge0 call unchanged for allowed IDs and
leave the Depends(get_current_user_with_token) parameter pattern intact.
In `@server/services/__init__.py`:
- Around line 270-273: Update _judge0_submit and the graded paths around lines
348 and 383 to reuse a shared httpx.AsyncClient instead of creating a new client
per request. Initialize the client at module or application-lifespan scope and
ensure it is properly closed during shutdown, while preserving the existing
request payloads, headers, timeout, and response handling.
In `@web/src/components/student/problem/ChatBox.tsx`:
- Around line 97-105: Extract a shared language catalogue and run helper: in
web/src/components/student/problem/ChatBox.tsx:97-105 remove LANGUAGE_IDS and
import the catalogue; in web/src/components/student/Scratchpad.tsx:11-19 replace
LANGUAGES with that catalogue, preserving display labels and including sql. In
web/src/components/student/Scratchpad.tsx:65-84 and
web/src/components/student/problem/ChatBox.tsx:107-122 replace each local
execution/error-shaping body with the shared helper wrapping scratchpadExecute
and returning the normalized error shape.
In `@web/src/components/student/Scratchpad.tsx`:
- Around line 59-63: Debounce the localStorage persistence in handleChange
instead of calling localStorage.setItem on every keystroke. Track the latest
scratchpad value and save it through a 5-second interval or equivalent delayed
mechanism, while keeping setValue immediate and ensuring pending timers or
intervals are cleaned up appropriately.
In `@web/src/lib/api.ts`:
- Around line 47-55: Extend ScratchpadRunResult with an optional boolean error
field to represent the { error: true, stderr } client-side result shape, then
replace the any result-state annotations in Scratchpad.tsx and ChatBox.tsx with
ScratchpadRunResult so both consumers use the shared type.
- Around line 62-75: Extract the repeated token retrieval and Authorization
header construction from sendChatMessage and the scratchpad request into a
shared async authHeaders helper. Update both call sites to await this helper and
pass its result as the request headers, preserving the existing behavior when no
token is available.
In `@web/src/routes/_authenticated.course.tsx`:
- Line 385: Reformat the handleSelectLesson function so navigate({ starts on a
new, properly indented line after the opening brace, with the rest of the
function body consistently aligned.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 789ed393-82ed-4834-898e-7b408a6a92b6
📒 Files selected for processing (9)
admin/src/components/Login.tsxserver/model/scratchpad.pyserver/routers.pyserver/services/__init__.pyweb/src/components/student/Scratchpad.tsxweb/src/components/student/SidePane.tsxweb/src/components/student/problem/ChatBox.tsxweb/src/lib/api.tsweb/src/routes/_authenticated.course.tsx
| // with a live session. A client-side navigate would leave Convex | ||
| // caching a missing token and the first data fetch failing with an | ||
| // unauthorized error that Solid's createResource never retries. | ||
| window.location.assign("/"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve history replacement during the hard reload.
window.location.assign("/") adds a new browser history entry. Use window.location.replace("/") to reload the application without preserving the login page in history.
Proposed fix
- window.location.assign("/");
+ window.location.replace("/");📝 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.
| window.location.assign("/"); | |
| window.location.replace("/"); |
🤖 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 `@admin/src/components/Login.tsx` at line 183, In the login redirect flow
containing window.location.assign, replace the navigation call with
window.location.replace("/") so the application reloads without adding the login
page to browser history.
|
|
||
| class ScratchpadCode(BaseModel): | ||
| code: str | ||
| language_id: Optional[int] = 71 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Explicit null for language_id bypasses the default.
Pydantic applies the 71 default only when the key is absent. If a client sends "language_id": null, the field is None. server/routers.py Line 50 passes that None into run_scratchpad, and _judge0_submit puts language_id: None in the Judge0 payload. Judge0 rejects that submission. Use a non-optional field with a default, or coerce None to the default.
🐛 Proposed fix
class ScratchpadCode(BaseModel):
code: str
- language_id: Optional[int] = 71
+ language_id: int = 71
stdin: Optional[str] = None📝 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.
| language_id: Optional[int] = 71 | |
| class ScratchpadCode(BaseModel): | |
| code: str | |
| language_id: int = 71 | |
| stdin: Optional[str] = None |
🤖 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 `@server/model/scratchpad.py` at line 6, Update the language_id field in the
scratchpad model to prevent explicit null values from reaching run_scratchpad
and _judge0_submit: use a non-optional defaulted field or coerce None to 71
before constructing the Judge0 payload, while preserving the default when the
key is omitted.
| except httpx.HTTPStatusError as e: | ||
| log.error(f"Judge0 error response {e.response.status_code}: {e.response.text}") | ||
| try: | ||
| err_data = e.response.json() | ||
| detail = err_data.get("message") or err_data.get("error") or str(e) | ||
| except Exception: | ||
| detail = f"Execution service returned error: {e.response.text}" | ||
| raise HTTPException( | ||
| status_code=e.response.status_code, | ||
| detail=detail | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not return the upstream Judge0 status code to the client.
Line 290 re-raises e.response.status_code verbatim. If Judge0 rejects the request because of a bad X-Auth-Token or an expired RapidAPI key, the API returns 401 or 403 to the browser. The client cannot distinguish that from its own session expiring. A Judge0 429 also becomes an API 429, which conflicts with the SlowAPI limit on the route. Map upstream 4xx auth and rate-limit failures to 502 Bad Gateway, and keep the upstream detail in the log only.
Line 286 also puts the upstream message/error text directly in the response detail. Judge0 error bodies can include configuration details. Return a fixed message for 5xx and auth failures.
Also add from e on the three raises to satisfy Ruff B904 at Lines 278, 289, and 295.
🛡️ Proposed fix
except httpx.TimeoutException as e:
log.error(f"Judge0 request timed out: {e}")
raise HTTPException(
status_code=status.HTTP_504_GATEWAY_TIMEOUT,
detail="Code execution request timed out."
- )
+ ) from e
except httpx.HTTPStatusError as e:
log.error(f"Judge0 error response {e.response.status_code}: {e.response.text}")
- try:
- err_data = e.response.json()
- detail = err_data.get("message") or err_data.get("error") or str(e)
- except Exception:
- detail = f"Execution service returned error: {e.response.text}"
- raise HTTPException(
- status_code=e.response.status_code,
- detail=detail
- )
+ upstream = e.response.status_code
+ if upstream in (400, 422):
+ # Client-caused: surface the upstream validation message.
+ try:
+ err_data = e.response.json()
+ detail = err_data.get("message") or err_data.get("error") or "Invalid submission."
+ except ValueError:
+ detail = "Invalid submission."
+ raise HTTPException(status_code=upstream, detail=detail) from e
+ raise HTTPException(
+ status_code=status.HTTP_502_BAD_GATEWAY,
+ detail="The code execution service rejected the request."
+ ) from e
except httpx.RequestError as e:
log.error(f"Judge0 connection error: {e}")
raise HTTPException(
status_code=status.HTTP_502_BAD_GATEWAY,
detail="Failed to connect to the code execution service."
- )
+ ) from e🧰 Tools
🪛 Ruff (0.16.1)
[warning] 287-287: Do not catch blind exception: Exception
(BLE001)
[warning] 289-292: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 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 `@server/services/__init__.py` around lines 282 - 292, Update the Judge0
exception handling around the three raises in the request flow: chain each
re-raised exception with “from e” to satisfy B904, map upstream 4xx
authentication and rate-limit responses (including 401, 403, and 429) to HTTP
502, and return fixed non-sensitive messages for 5xx and auth failures. Keep
upstream status, response text, and parsed details in logs only; do not expose
Judge0 message/error content or status codes to clients.
Source: Linters/SAST tools
| // Normalise a markdown fence language tag to a monaco/language key. | ||
| function normalizeCodeLanguage(lang?: string): string { | ||
| if (!lang) return "python"; | ||
| const key = lang.toLowerCase(); | ||
| if (key === "python3" || key === "py") return "python"; | ||
| if (key === "js" || key === "node") return "javascript"; | ||
| if (key === "ts") return "typescript"; | ||
| if (key === "c++" || key === "cc" || key === "cpp") return "cpp"; | ||
| if (key === "golang") return "go"; | ||
| return key; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Unknown fence languages fall back to Python and produce a confusing error.
normalizeCodeLanguage returns the input unchanged for any language it does not map. Line 112 then applies LANGUAGE_IDS[lang] ?? 71. A bash, json, sql, or text block therefore renders a Run button, executes as Python 3, and shows a Python SyntaxError.
Assistant replies frequently contain shell and JSON blocks. Hide the Run button when the language has no runner ID.
🐛 Proposed fix
Move LANGUAGE_IDS to module scope, then gate the button:
+ const runnerId = LANGUAGE_IDS[lang];
...
- <button
- type="button"
- onClick={handleRun}
- disabled={isRunning}
+ {runnerId !== undefined && (
+ <button
+ type="button"
+ onClick={handleRun}
+ disabled={isRunning}
className="inline-flex items-center gap-1 rounded-md bg-lagoon px-2 py-0.5 text-[10px] font-semibold text-white hover:bg-lagoon-deep disabled:opacity-50 transition-colors cursor-pointer"
title="Run this snippet"
>
{isRunning ? (
<Loader2 className="size-3 animate-spin" />
) : (
<Play className="size-3 fill-current" />
)}
<span>Run</span>
</button>
+ )}Apply the same gate to the Scratchpad button, since Scratchpad.openWith also falls back to Python for unknown languages.
Also applies to: 107-112
🤖 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 `@web/src/components/student/problem/ChatBox.tsx` around lines 69 - 79, Update
normalizeCodeLanguage and the code-block rendering logic to distinguish
supported runner languages from unknown fence languages: move LANGUAGE_IDS to
module scope, and render neither the Run button nor the Scratchpad button unless
the normalized language has a runner ID. Preserve existing behavior for
recognized languages while preventing unknown languages such as bash, json, sql,
or text from falling back to Python.
| <pre className="max-h-32 overflow-auto whitespace-pre-wrap px-3 pb-2 font-mono text-[11px] text-zinc-300"> | ||
| {result.stdout ? result.stdout : hasError ? result.stderr || result.compile_output : "(empty output)"} | ||
| </pre> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Error output is hidden when the run produces both stdout and stderr.
Line 192 shows result.stdout when it is non-empty and never shows stderr or compile_output in that case. A script that prints output and then raises an exception shows only the printed output. The student does not see the error. Scratchpad.tsx Lines 200-216 renders both. Match that behavior.
🐛 Proposed fix
- <pre className="max-h-32 overflow-auto whitespace-pre-wrap px-3 pb-2 font-mono text-[11px] text-zinc-300">
- {result.stdout ? result.stdout : hasError ? result.stderr || result.compile_output : "(empty output)"}
- </pre>
+ <pre className="max-h-32 overflow-auto whitespace-pre-wrap px-3 pb-2 font-mono text-[11px]">
+ {result.stdout && (
+ <span className="text-zinc-300">{result.stdout}</span>
+ )}
+ {hasError && (
+ <span className="text-rose-400">
+ {result.stderr || result.compile_output}
+ </span>
+ )}
+ {!result.stdout && !hasError && (
+ <span className="text-zinc-500">(empty output)</span>
+ )}
+ </pre>📝 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.
| <pre className="max-h-32 overflow-auto whitespace-pre-wrap px-3 pb-2 font-mono text-[11px] text-zinc-300"> | |
| {result.stdout ? result.stdout : hasError ? result.stderr || result.compile_output : "(empty output)"} | |
| </pre> | |
| <pre className="max-h-32 overflow-auto whitespace-pre-wrap px-3 pb-2 font-mono text-[11px]"> | |
| {result.stdout && ( | |
| <span className="text-zinc-300">{result.stdout}</span> | |
| )} | |
| {hasError && ( | |
| <span className="text-rose-400"> | |
| {result.stderr || result.compile_output} | |
| </span> | |
| )} | |
| {!result.stdout && !hasError && ( | |
| <span className="text-zinc-500">(empty output)</span> | |
| )} | |
| </pre> |
🤖 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 `@web/src/components/student/problem/ChatBox.tsx` around lines 191 - 193,
Update the output rendering in ChatBox’s result display so non-empty stdout does
not suppress stderr or compile_output; render the available stdout and error
output together, matching Scratchpad.tsx’s behavior while retaining the "(empty
output)" fallback when no output exists.
| useImperativeHandle(ref, () => ({ | ||
| openWith(code: string, lang?: string) { | ||
| let key: string = lang ?? ""; | ||
| key = key.toLowerCase(); | ||
| if (key === "python3" || key === "py") key = "python"; | ||
| if (!LANGUAGES[key]) key = DEFAULT_LANG; | ||
| setLanguage(key); | ||
| setValue(code); | ||
| setResult(null); | ||
| if (editorRef.current) { | ||
| editorRef.current.setValue(code); | ||
| } | ||
| }, | ||
| })); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
openWith does not persist the loaded snippet.
handleChange at Line 62 and handleReset at Line 90 write to exemplai_scratchpad. openWith does not. If a user loads a snippet from chat and reloads the page without editing, the previous saved content returns instead of the snippet. Persist the code here as well.
🐛 Proposed fix
setLanguage(key);
setValue(code);
setResult(null);
+ localStorage.setItem("exemplai_scratchpad", code);
if (editorRef.current) {
editorRef.current.setValue(code);
}📝 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.
| useImperativeHandle(ref, () => ({ | |
| openWith(code: string, lang?: string) { | |
| let key: string = lang ?? ""; | |
| key = key.toLowerCase(); | |
| if (key === "python3" || key === "py") key = "python"; | |
| if (!LANGUAGES[key]) key = DEFAULT_LANG; | |
| setLanguage(key); | |
| setValue(code); | |
| setResult(null); | |
| if (editorRef.current) { | |
| editorRef.current.setValue(code); | |
| } | |
| }, | |
| })); | |
| useImperativeHandle(ref, () => ({ | |
| openWith(code: string, lang?: string) { | |
| let key: string = lang ?? ""; | |
| key = key.toLowerCase(); | |
| if (key === "python3" || key === "py") key = "python"; | |
| if (!LANGUAGES[key]) key = DEFAULT_LANG; | |
| setLanguage(key); | |
| setValue(code); | |
| setResult(null); | |
| localStorage.setItem("exemplai_scratchpad", code); | |
| if (editorRef.current) { | |
| editorRef.current.setValue(code); | |
| } | |
| }, | |
| })); |
🤖 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 `@web/src/components/student/Scratchpad.tsx` around lines 40 - 53, Update the
openWith method in useImperativeHandle to persist the loaded code to
exemplai_scratchpad, matching the storage behavior of handleChange and
handleReset, while preserving the existing language, editor, value, and result
updates.
| function handleOpenScratchpad(code: string, snippetLanguage?: string) { | ||
| setIsScratchpadVisible(true); | ||
| // Defer so the Scratchpad mounts before we try to preload it. | ||
| setTimeout(() => { | ||
| scratchpadRef.current?.openWith(code, snippetLanguage); | ||
| }, 0); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
setTimeout(..., 0) does not reliably wait for the Scratchpad mount.
setIsScratchpadVisible(true) schedules a render. React 19 may commit that render after the zero-delay macrotask runs. If scratchpadRef.current is still null, the optional call silently does nothing and the snippet never loads into the scratchpad. The failure is intermittent and depends on scheduling.
Pass the pending snippet through state instead, so the value is applied when Scratchpad mounts.
🐛 Proposed fix
function handleOpenScratchpad(code: string, snippetLanguage?: string) {
+ setPendingSnippet({ key: (pendingSnippet?.key ?? 0) + 1, code, language: snippetLanguage });
setIsScratchpadVisible(true);
- // Defer so the Scratchpad mounts before we try to preload it.
- setTimeout(() => {
- scratchpadRef.current?.openWith(code, snippetLanguage);
- }, 0);
}Then apply the snippet inside Scratchpad with an effect keyed on the snippet key. If you prefer to keep the ref contract, replace the timer with a useEffect that fires after the commit:
useEffect(() => {
if (isScratchpadVisible && pendingSnippet) {
scratchpadRef.current?.openWith(pendingSnippet.code, pendingSnippet.language);
}
}, [isScratchpadVisible, pendingSnippet]);🤖 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 `@web/src/routes/_authenticated.course.tsx` around lines 356 - 362, Update
handleOpenScratchpad to store the pending code and snippetLanguage in state
instead of using setTimeout. Apply that pending snippet after Scratchpad mounts
via an effect keyed to the pending snippet (or isScratchpadVisible and the
pending snippet), invoking scratchpadRef.current?.openWith only when the ref is
available, so the snippet is not lost.
| {isScratchpadVisible ? ( | ||
| <Scratchpad ref={scratchpadRef} /> | ||
| ) : ( | ||
| <CodeEditor | ||
| onMount={handleEditorMount} | ||
| language={language} | ||
| value={currentCode} | ||
| onChange={handleCodeChange} | ||
| fontSize={fontSize} | ||
| isRunning={isRunning} | ||
| isSubmitting={isSubmitting} | ||
| executionResult={executionResult} | ||
| isConsoleOpen={isConsoleOpen} | ||
| setIsConsoleOpen={setIsConsoleOpen} | ||
| onRun={() => handleExecute("run")} | ||
| onSubmit={() => handleExecute("submit")} | ||
| onSendErrorToChat={handleSendErrorToChat} | ||
| isSaved={isSaved} | ||
| onSave={handleSave} | ||
| testCases={activeQuestion?.testCases || []} | ||
| isCompleted={isCompleted} | ||
| onNextLesson={handleNextLesson} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unmounting CodeEditor leaves editorRef pointing at a disposed Monaco editor.
When isScratchpadVisible becomes true, CodeEditor unmounts and @monaco-editor/react disposes the underlying Monaco instance. editorRef.current still holds that disposed instance because nothing resets it. Several code paths keep using it while the scratchpad is open:
- The autosave interval at Lines 157-164 calls
editorRef.current.getValue()every 5 seconds. handleSaveat Line 235 callsgetValue().handleSendErrorToChatat Line 328 callsgetValue().ChatBoxreadseditorRef.currentfor editor context on every message send.
A disposed Monaco editor has a null model, so getValue() throws. Reset the ref when the editor unmounts.
🐛 Proposed fix
{isScratchpadVisible ? (
<Scratchpad ref={scratchpadRef} />
) : (
<CodeEditor
- onMount={handleEditorMount}
+ onMount={handleEditorMount}
+ key="course-editor"Add an unmount cleanup so the ref never holds a disposed editor:
useEffect(() => {
if (isScratchpadVisible) {
editorRef.current = null;
}
}, [isScratchpadVisible]);Alternatively, keep CodeEditor mounted and hide it with a hidden class so the editor state and the ref stay valid.
🤖 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 `@web/src/routes/_authenticated.course.tsx` around lines 487 - 510, Reset
editorRef.current when CodeEditor is unmounted as isScratchpadVisible becomes
true, using the existing visibility state and effect flow near the course
component. Ensure autosave, handleSave, handleSendErrorToChat, and ChatBox
cannot access the disposed Monaco instance while the scratchpad is open.
🚀 Admin Preview Deployment Successful!
|
🚀 Preview Deployment Successful!
|
77ccb4b to
b148aad
Compare
🚀 Admin Preview Deployment Successful!
|
🚀 Admin Preview Deployment Successful!
|
🚀 Preview Deployment Successful!
|
🚀 Preview Deployment Successful!
|
b148aad to
bfc291a
Compare
🚀 Admin Preview Deployment Successful!
|
🚀 Admin Preview Deployment Successful!
|
🚀 Preview Deployment Successful!
|
🚀 Preview Deployment Successful!
|
The Convex client and its setAuth token fetch run once at module load, while the user is unauthenticated, so it caches a missing token. A client-side navigate after sign-in doesn't re-run setAuth, causing the first protected data fetch to fail with an unauthorized error that Solid's createResource never retries. Force a full reload to boot the app with a live session.
bfc291a to
cde398a
Compare
🚀 Admin Preview Deployment Successful!
|
🚀 Admin Preview Deployment Successful!
|
🚀 Preview Deployment Successful!
|
🚀 Preview Deployment Successful!
|
This branch was successfully deployed
2 participants
This PR introduces scratchpad for student to freely writing the code that are generated example (either using the interface to insert code or manually typing it).
In addition, admin interface being unauthenticated after is also fixed. Now it will properly refresh the page to fetch new authenticated token and discards no-token cached from earlier when admins are greeted with authentication screen.
todo
Stack created with GitHub Stacks CLI • Give Feedback 💬
Summary by CodeRabbit
New Features
Bug Fixes