[RichDom2185]

Description

Reduces duplication, improves abstraction, adds caching (so that each page is only fetched once, going back and forth between pages won’t refresh the content each time, resulting in instant loads).

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• Code quality improvements

How to test

Persistence (local storage) should still work. Page is cached. Index page (special handling) still works

Checklist

• I have tested this code
• I have updated the documentation

[coderabbitai]

Walkthrough

The change moves local-storage state helpers into a dedicated hook, adds SICP React Query keys and hooks, and migrates the JavaScript and Python SICP layouts to query-based fetching, persistence, parsing, loading, error handling, and scrolling.

Changes

SICP query migration

Layer / File(s)	Summary
Storage and shared helpers src/commons/hooks/useLocalStorageState.ts, src/commons/utils/Hooks.ts, src/commons/utils/LocalStorageHelper.ts, src/features/sicp/utils/SicpUtils.ts, src/new_routes/_layout.tsx	Local-storage helpers move to a dedicated hook module, the previous hook and helper exports are removed, and shared scrolling plus updated imports are introduced.
SICP query and persistence path src/queryKeys/sicp.ts, src/queryKeys/index.ts, src/pages/sicp/hooks/useSicpQuery.ts	SICP query factories and React Query hooks fetch JavaScript and Python sections, parse them with refs, map parse failures, and persist the active section.
SICP JavaScript layout integration src/new_routes/sicpjs/_layout.tsx	The JavaScript layout adopts query-provided data, loading state, errors, refs, redirects, and hash scrolling.
SICP Python layout integration src/new_routes/sicpy/_layout.tsx	The Python layout adopts query-provided data, loading state, errors, refs, redirects, scrolling, and outlet context.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main refactor and caching change in the SICP/SICPy layouts.
Description check	✅ Passed	The description covers summary, change type, testing, and checklist, with only minor missing details like issue reference and dependencies.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gemini-code-assist]

Code Review

This pull request refactors the SICP JS and Python section fetching to use React Query instead of manual fetch calls within useEffect hooks, centralizing query keys and state persistence. The review feedback highlights several critical issues: a bug in section persistence that prematurely resets the cached section to the default index during initial loading, a stale closure issue where globally cached JSX elements close over outdated refs (which can be resolved by moving the parsing logic to a local useMemo hook), and a performance concern regarding synchronous localStorage reads on every render that should be addressed with a lazy state initializer.

Important

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

[coveralls]

Coverage Report for CI Build 29308226642

Warning

No base build found for commit 4980a1f on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 42.244%

Details

• Patch coverage: 2 uncovered changes across 1 file (12 of 14 lines covered, 85.71%).

Uncovered Changes

File	Changed	Covered	%
src/commons/hooks/useLocalStorageState.ts	14	12	85.71%

Coverage Regressions

Requires a base build to compare against. How to fix this →

---

Coverage Stats

Relevant Lines:	14187
Covered Lines:	6442
Line Coverage:	45.41%
Relevant Branches:	7544
Covered Branches:	2738
Branch Coverage:	36.29%
Branches in Coverage %:	Yes
Coverage Strength:	28.05 hits per line

---

💛 - Coveralls

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (5)

src/commons/hooks/useLocalStorageState.ts (1)

32-32: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Use a lazy initializer for useState to avoid synchronous storage reads on every render.

readLocalStorage performs synchronous I/O and JSON parsing. Passing it directly to useState executes this logic on every single render, even though React discards the result after the initial mount. Wrap it in a function to evaluate it only once.

♻️ Proposed refactor

-  const [value, setValue] = useState<T>(readLocalStorage(key, defaultValue));
+  const [value, setValue] = useState<T>(() => readLocalStorage(key, defaultValue));

🤖 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 `@src/commons/hooks/useLocalStorageState.ts` at line 32, Update the useState
initialization in useLocalStorageState to pass a lazy initializer function that
calls readLocalStorage(key, defaultValue), ensuring the storage read and parsing
occur only during initial state creation.

src/queryKeys/sicp.ts (1)

8-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

sectionJs/sectionPy are near-identical; consider a shared fetch helper.

Both definitions differ only in base URL. A small fetchSicpSection(baseUrl, section, signal) helper would remove the duplication (and fix the res.json() vs await res.json() inconsistency in the same place).

♻️ Proposed refactor

+async function fetchSicpSection(baseUrl: string, section: string, signal: AbortSignal) {
+  const res = await fetch(baseUrl + encodeURIComponent(section) + extension, { signal });
+  if (!res.ok) {
+    throw new Error(res.statusText);
+  }
+  return res.json();
+}
+
 export const sicp = createQueryKeys('sicp', {
   sectionJs: (section: string) => ({
     queryKey: ['js', section],
-    queryFn: async ({ signal }) => {
-      const res = await fetch(baseUrlJs + section + extension, { signal });
-      if (!res.ok) {
-        throw new Error(res.statusText);
-      }
-      return res.json();
-    },
+    queryFn: ({ signal }) => fetchSicpSection(baseUrlJs, section, signal),
   }),
   sectionPy: (section: string) => ({
     queryKey: ['py', section],
-    queryFn: async ({ signal }) => {
-      const res = await fetch(baseUrlPy + section + extension, { signal });
-      if (!res.ok) {
-        throw new Error(res.statusText);
-      }
-      return await res.json();
-    },
+    queryFn: ({ signal }) => fetchSicpSection(baseUrlPy, section, signal),
   }),
 });

🤖 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 `@src/queryKeys/sicp.ts` around lines 8 - 29, Extract the duplicated request
logic from sicp.sectionJs and sicp.sectionPy into a shared fetchSicpSection
helper accepting baseUrl, section, and signal. Have both query functions
delegate to it, preserving the existing response validation while standardizing
JSON parsing through the helper.

src/new_routes/sicpjs/_layout.tsx (2)

74-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated hash-lookup logic between the JS and Python layouts has drifted. Both scroll effects resolve location.hash to a DOM element with near-identical code, but one uses document.querySelector(hash) and the other document.getElementById(hash.slice(1)) — the kind of copy-paste drift that produced the querySelector bug flagged on the JS layout. Extract a single shared helper (e.g. in src/features/sicp/utils/SicpUtils.ts, alongside scrollRefIntoView) that both layouts call.

• src/new_routes/sicpjs/_layout.tsx#L74-L83: replace the inline document.querySelector(hash) ?? refs.current[hash] lookup with a shared resolveHashElement(hash, refs) helper.
• src/new_routes/sicpy/_layout.tsx#L42-L51: replace the inline document.getElementById(hash.slice(1)) ?? refs.current[hash] lookup with the same shared helper, ensuring both layouts use the safer getElementById-based implementation.

🤖 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 `@src/new_routes/sicpjs/_layout.tsx` around lines 74 - 83, Extract the shared
hash-to-element resolution into a helper such as resolveHashElement alongside
scrollRefIntoView in src/features/sicp/utils/SicpUtils.ts, using
getElementById(hash.slice(1)) with the refs fallback. Update the effects in
src/new_routes/sicpjs/_layout.tsx lines 74-83 and
src/new_routes/sicpy/_layout.tsx lines 42-51 to call this helper, removing their
duplicated inline lookups.

---

93-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Error → SicpErrorType mapping ternary is duplicated verbatim across both layouts. Both files run the exact same error.message === 'Not Found' ? ... : error instanceof ParseJsonError ? ... : ... chain to pick a SicpErrorType. Extracting this into a shared helper (e.g. mapSicpError(error): SicpErrorType, colocated with getSicpError/SicpErrorType in SicpErrors.tsx) keeps both layouts in sync and centralizes the (already separately-flagged) statusText-based 404 detection fix.

• src/new_routes/sicpjs/_layout.tsx#L93-L102: replace the inline ternary with a call to a shared mapSicpError(error).
• src/new_routes/sicpy/_layout.tsx#L61-L70: replace the inline ternary with the same shared mapSicpError(error) call.

🤖 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 `@src/new_routes/sicpjs/_layout.tsx` around lines 93 - 102, Extract the
duplicated error-to-SicpErrorType mapping into a shared mapSicpError(error)
helper colocated with getSicpError and SicpErrorType in SicpErrors.tsx. Replace
the inline ternary in src/new_routes/sicpjs/_layout.tsx lines 93-102 and
src/new_routes/sicpy/_layout.tsx lines 61-70 with calls to mapSicpError(error),
preserving the existing mappings and centralizing 404 detection.

src/pages/sicp/hooks/useSicpQuery.ts (1)

34-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

useSicpJsSectionQuery and useSicPySectionQuery are duplicated near-verbatim.

Both hooks share the same shape (refs, useQuery with enabled/select/ParseJsonError wrapping, and a call to useSicpSectionPersistence), differing only by which queries.sicp.section* variant and constants are used. Given the PR's stated goal of reducing duplication, this could be collapsed into a single parameterized hook.

♻️ Proposed refactor

+function useSicpVariantSectionQuery(
+  section: string | undefined,
+  opts: {
+    queryOptions: ReturnType<typeof queries.sicp.sectionJs>;
+    index: string;
+    cacheKey: string;
+    errorLabel: string;
+  },
+) {
+  const refs = useRef<Record<string, HTMLElement | null>>({});
+  const { data, error, isPending, isFetching } = useQuery({
+    ...opts.queryOptions,
+    enabled: !!section && section !== opts.index,
+    select: json => {
+      try {
+        return parseArr(json, refs);
+      } catch (e) {
+        throw new ParseJsonError(opts.errorLabel, { cause: e });
+      }
+    },
+  });
+  useSicpSectionPersistence(section, data, error, { key: opts.cacheKey, defaultSection: opts.index });
+  return { data, error, isPending, isFetching, refs };
+}

🤖 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 `@src/pages/sicp/hooks/useSicpQuery.ts` around lines 34 - 79, Extract the
shared query and persistence logic from useSicpJsSectionQuery and
useSicPySectionQuery into one parameterized hook that accepts the section query,
cache key, and default-section constant. Keep each public hook as a thin wrapper
supplying its respective queries.sicp.sectionJs/sectionPy configuration and
SICP_INDEX/SICPY_INDEX values, while preserving refs, enabled behavior,
ParseJsonError handling, and the returned query state.

🤖 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 `@src/features/sicp/utils/SicpUtils.ts`:
- Around line 9-14: Update the scroll calculation around parentRef.current and
parent.scrollTo to use getBoundingClientRect() for both the target ref and
parent, deriving the target position relative to the parent’s current viewport
position and scroll state. Remove the offsetTop/window.scrollY-based relativeTop
calculation so scrolling remains correct regardless of intermediate positioning
or window scroll.

In `@src/new_routes/sicpjs/_layout.tsx`:
- Around line 79-83: Update the hash-anchor lookup in the effect using
location.hash to call document.getElementById with the leading “#” removed,
preventing selector parsing errors for IDs such as “#1.1” while preserving the
refs.current[hash] fallback and existing scrollRefIntoView flow.

In `@src/pages/sicp/hooks/useSicpQuery.ts`:
- Around line 10-32: Update useSicpSectionPersistence so the defaultSection
fallback is written only when error is present, not merely when data is
unavailable during loading. Preserve the existing section persistence for
successful data and default-section handling, and use the existing error
parameter in the conditional.

In `@src/queryKeys/sicp.ts`:
- Line 12: Update the fetch URL construction in the SICP JavaScript and Python
request paths to URI-encode the route-param `section` before concatenating it
with `baseUrlJs` or `baseUrlPy` and `extension`, preserving the existing fetch
behavior and signal handling.
- Around line 11-17: Update the query function’s HTTP error handling to throw an
error carrying res.status rather than using res.statusText as the message, then
change both SICP layout 404 checks to use error.status === 404. Preserve the
existing Not Found UI behavior for HTTP 404 responses.

---

Nitpick comments:
In `@src/commons/hooks/useLocalStorageState.ts`:
- Line 32: Update the useState initialization in useLocalStorageState to pass a
lazy initializer function that calls readLocalStorage(key, defaultValue),
ensuring the storage read and parsing occur only during initial state creation.

In `@src/new_routes/sicpjs/_layout.tsx`:
- Around line 74-83: Extract the shared hash-to-element resolution into a helper
such as resolveHashElement alongside scrollRefIntoView in
src/features/sicp/utils/SicpUtils.ts, using getElementById(hash.slice(1)) with
the refs fallback. Update the effects in src/new_routes/sicpjs/_layout.tsx lines
74-83 and src/new_routes/sicpy/_layout.tsx lines 42-51 to call this helper,
removing their duplicated inline lookups.
- Around line 93-102: Extract the duplicated error-to-SicpErrorType mapping into
a shared mapSicpError(error) helper colocated with getSicpError and
SicpErrorType in SicpErrors.tsx. Replace the inline ternary in
src/new_routes/sicpjs/_layout.tsx lines 93-102 and
src/new_routes/sicpy/_layout.tsx lines 61-70 with calls to mapSicpError(error),
preserving the existing mappings and centralizing 404 detection.

In `@src/pages/sicp/hooks/useSicpQuery.ts`:
- Around line 34-79: Extract the shared query and persistence logic from
useSicpJsSectionQuery and useSicPySectionQuery into one parameterized hook that
accepts the section query, cache key, and default-section constant. Keep each
public hook as a thin wrapper supplying its respective
queries.sicp.sectionJs/sectionPy configuration and SICP_INDEX/SICPY_INDEX
values, while preserving refs, enabled behavior, ParseJsonError handling, and
the returned query state.

In `@src/queryKeys/sicp.ts`:
- Around line 8-29: Extract the duplicated request logic from sicp.sectionJs and
sicp.sectionPy into a shared fetchSicpSection helper accepting baseUrl, section,
and signal. Have both query functions delegate to it, preserving the existing
response validation while standardizing JSON parsing through the helper.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 629178d0-6956-4952-9157-124c29832185

📥 Commits

Reviewing files that changed from the base of the PR and between 4980a1f and f030b3e.

📒 Files selected for processing (10)

• src/commons/hooks/useLocalStorageState.ts
• src/commons/utils/Hooks.ts
• src/commons/utils/LocalStorageHelper.ts
• src/features/sicp/utils/SicpUtils.ts
• src/new_routes/_layout.tsx
• src/new_routes/sicpjs/_layout.tsx
• src/new_routes/sicpy/_layout.tsx
• src/pages/sicp/hooks/useSicpQuery.ts
• src/queryKeys/index.ts
• src/queryKeys/sicp.ts

💤 Files with no reviewable changes (2)

• src/commons/utils/LocalStorageHelper.ts
• src/commons/utils/Hooks.ts

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fix unreliable scroll offset calculation.

Using offsetTop is fragile because it depends on the CSS position of intermediate ancestors (offsetParent), which may not be the parent. Additionally, mixing window.scrollY into the internal scroll target of a container (parent.scrollTo) is mathematically incorrect and will scroll to the wrong offset if the window has been scrolled.

Use getBoundingClientRect() to reliably compute the needed scroll offset relative to the current scroll position, regardless of CSS positioning or window scroll state.

🐛 Proposed fix

-  const parent = parentRef.current!;
-  const relativeTop = window.scrollY > parent.offsetTop ? window.scrollY : parent.offsetTop;
-  parent.scrollTo({
-    behavior: 'smooth',
-    top: ref.offsetTop - relativeTop,
-  });
+  const parent = parentRef.current!;
+  parent.scrollTo({
+    behavior: 'smooth',
+    top: parent.scrollTop + ref.getBoundingClientRect().top - parent.getBoundingClientRect().top,
+  });

📝 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 parent = parentRef.current!;
	const relativeTop = window.scrollY > parent.offsetTop ? window.scrollY : parent.offsetTop;
	parent.scrollTo({
	behavior: 'smooth',
	top: ref.offsetTop - relativeTop,
	});
	const parent = parentRef.current!;
	parent.scrollTo({
	behavior: 'smooth',
	top: parent.scrollTop + ref.getBoundingClientRect().top - parent.getBoundingClientRect().top,
	});

🤖 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 `@src/features/sicp/utils/SicpUtils.ts` around lines 9 - 14, Update the scroll
calculation around parentRef.current and parent.scrollTo to use
getBoundingClientRect() for both the target ref and parent, deriving the target
position relative to the parent’s current viewport position and scroll state.
Remove the offsetTop/window.scrollY-based relativeTop calculation so scrolling
remains correct regardless of intermediate positioning or window scroll.

[RichDom2185]

Out of scope. This is a simple refactor. Valid though

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== File outline ==\n'
ast-grep outline src/new_routes/sicpjs/_layout.tsx --view expanded || true

printf '\n== Relevant lines in src/new_routes/sicpjs/_layout.tsx ==\n'
sed -n '1,140p' src/new_routes/sicpjs/_layout.tsx

printf '\n== Search for similar hash lookup patterns ==\n'
rg -n "document\.(querySelector|getElementById)\(hash|refs\.current\[hash\]|location\.hash" src -S

printf '\n== Check for SICP anchor naming patterns ==\n'
rg -n "1\.1|2\.2\.3|#\d+\.\d+|\bsection\b" src/new_routes/sicpjs -S

Repository: source-academy/frontend

Length of output: 6808

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

path = Path("src/new_routes/sicpjs/_layout.tsx")
text = path.read_text()

# Extract the relevant effect block for exact context.
m = re.search(r"const hash = location\.hash;.*?scrollRefIntoView\(elem, parentRef\);\s*}, \[isLoading, location\.hash, refs\]\);", text, re.S)
print("EFFECT_BLOCK_FOUND=", bool(m))
if m:
    print(m.group(0))

# Simulate CSS selector validity risk for common SICP-style hashes.
tests = ["`#1.1`", "`#2.2.3`", "`#abc`", "`#a.b`", "`#1`", "`#1`\\2"]
print("\nselector validity probes:")
for sel in tests:
    # We can't invoke DOM here, but we can note CSS.escape requirements.
    # Simple heuristic: leading digit or unescaped dot in id-like fragment is problematic for querySelector.
    frag = sel[1:]
    problematic = bool(re.match(r"^\d", frag)) or "." in frag
    print(sel, "problematic=", problematic)
PY

Repository: source-academy/frontend

Length of output: 575

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== File outline ==\n'
ast-grep outline src/new_routes/sicpjs/_layout.tsx --view expanded || true

printf '\n== Relevant lines in src/new_routes/sicpjs/_layout.tsx ==\n'
sed -n '1,140p' src/new_routes/sicpjs/_layout.tsx

printf '\n== Similar lookup patterns ==\n'
rg -n "document\.(querySelector|getElementById)\(hash|refs\.current\[hash\]|location\.hash" src -S || true

printf '\n== Related SICP routes ==\n'
rg -n "SICP|sicpjs|sicpy" src -S || true

Repository: source-academy/frontend

Length of output: 14315

---

Use getElementById for hash anchors document.querySelector(hash) will throw on common SICP ids like #1.1 or #2.2.3. Use document.getElementById(hash.slice(1)) here and keep the refs.current[hash] fallback.

🤖 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 `@src/new_routes/sicpjs/_layout.tsx` around lines 79 - 83, Update the
hash-anchor lookup in the effect using location.hash to call
document.getElementById with the leading “#” removed, preventing selector
parsing errors for IDs such as “#1.1” while preserving the refs.current[hash]
fallback and existing scrollRefIntoView flow.

[RichDom2185]

Out of scope. This is a simple refactor

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Inspecting relevant files...\n'
git ls-files 'src/queryKeys/sicp.ts' 'src/**/_layout.tsx' | sed 's/^/- /'

printf '\n--- src/queryKeys/sicp.ts ---\n'
cat -n src/queryKeys/sicp.ts

printf '\n--- search for Not Found handling ---\n'
rg -n "Not Found|error\.message|statusText|res\.status" src --glob '!**/dist/**' --glob '!**/build/**' --glob '!**/node_modules/**'

printf '\n--- locate sicp layout files ---\n'
fd -a '_layout.tsx' src

Repository: source-academy/frontend

Length of output: 3386

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n--- src/new_routes/sicpjs/_layout.tsx (relevant lines) ---\n'
sed -n '1,140p' src/new_routes/sicpjs/_layout.tsx | cat -n

printf '\n--- src/new_routes/sicpy/_layout.tsx (relevant lines) ---\n'
sed -n '1,120p' src/new_routes/sicpy/_layout.tsx | cat -n

printf '\n--- search for section param usage / definitions ---\n'
rg -n "sectionJs\\(|sectionPy\\(|params\\.section|useParams|section:" src/new_routes src/queryKeys src -g '!**/dist/**' -g '!**/build/**' -g '!**/node_modules/**'

Repository: source-academy/frontend

Length of output: 15199

---

404 handling should use the HTTP status, not statusText
Response.statusText is empty for HTTP/2 responses, so throw new Error(res.statusText) makes the error.message === 'Not Found' branch in both SICP layouts miss real 404s. Attach res.status to the thrown error and switch the UI to error.status === 404.

🤖 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 `@src/queryKeys/sicp.ts` around lines 11 - 17, Update the query function’s HTTP
error handling to throw an error carrying res.status rather than using
res.statusText as the message, then change both SICP layout 404 checks to use
error.status === 404. Preserve the existing Not Found UI behavior for HTTP 404
responses.

[RichDom2185]

Valid, but out of scope for this refactor

[coderabbitai]

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Unescaped section interpolated directly into the fetch URL.

section comes from route params and is concatenated raw into baseUrlJs + section + extension / baseUrlPy + section + extension. Encode it to avoid malformed requests or unintended path segments (e.g. ../).

🛠️ Proposed fix

-      const res = await fetch(baseUrlJs + section + extension, { signal });
+      const res = await fetch(baseUrlJs + encodeURIComponent(section) + extension, { signal });

(apply similarly to the Py variant)

Also applies to: 22-22

🤖 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 `@src/queryKeys/sicp.ts` at line 12, Update the fetch URL construction in the
SICP JavaScript and Python request paths to URI-encode the route-param `section`
before concatenating it with `baseUrlJs` or `baseUrlPy` and `extension`,
preserving the existing fetch behavior and signal handling.

[RichDom2185]

Valid, but out of scope for this simple refactor