Add local student dashboard + Code Lab runner#156
Open
NischaySabharwal1 wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds a local-first dev server and local frontend pages: new dashboard and phase pages, updated lesson UI with a Code Lab using local APIs (/api/lesson-meta, /api/run, /api/test, /api/list, /api/rubric, /api/review), and progress driven by window.AIFSProgress; includes styles and documentation for local development. ChangesLocal-First Student Dashboard
🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
site/app.js (2)
61-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPhase completion is still derived from static metadata, not user progress.
computeStats()andrenderPhases()still rely onPHASES[*].statusfor phase completion/status class, which can contradict user progress-derived lesson counts and show misleading completion UI.Suggested fix
- var completePhases = 0; - for (var p = 0; p < PHASES.length; p++) { - if (PHASES[p].status === 'complete') completePhases++; - } + var completePhases = 0; + for (var p = 0; p < PHASES.length; p++) { + var phaseDone = 0; + var phaseTotal = PHASES[p].lessons.length; + for (var q = 0; q < phaseTotal; q++) { + var lesson = PHASES[p].lessons[q]; + if (hasProgress && lesson.url) { + var phaseLp = window.AIFSProgress.extractPath(lesson.url); + if (phaseLp && window.AIFSProgress.isLessonComplete(phaseLp)) phaseDone++; + } + } + if (phaseTotal > 0 && phaseDone === phaseTotal) completePhases++; + }- var statusClass = p.status.replace(/ /g, '-'); + var statusClass = done === total ? 'complete' : (done > 0 ? 'in-progress' : 'planned');Also applies to: 122-122
🤖 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 `@site/app.js` around lines 61 - 64, computeStats() and renderPhases() are still using PHASES[p].status to determine completed phases (see the completePhases loop), causing UI to disagree with user progress; change them to derive phase status from actual user progress/lesson completion counts instead of static PHASES[].status. Specifically, in computeStats() replace the loop that counts completePhases from PHASES with logic that inspects the user's progress/lessonsCompleted data for each phase (e.g., map each phase id to number of completed lessons and consider a phase complete when its completedLessons === totalLessons), store that computed status/counts on the stats object, and then update renderPhases() to read the phase status/counts from the computed stats (not PHASES[p].status) so the UI classes reflect real progress. Ensure you update any references to completePhases, PHASES[p].status, and phase completion checks to use the new computed values.
233-233:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse encoded lesson paths consistently in modal links.
At Line 233,
lessonPathis interpolated unencoded, while Line 224 encodes it. Keep both encoded to avoid query-string breakage for edge-case paths.Suggested fix
- actionHtml = '<a href="lesson.html?path=' + lessonPath + '" class="modal-lesson-read">' + (userComplete ? 'Review' : 'Read') + '</a>'; + actionHtml = '<a href="lesson.html?path=' + encodeURIComponent(lessonPath) + '" class="modal-lesson-read">' + (userComplete ? 'Review' : 'Read') + '</a>';🤖 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 `@site/app.js` at line 233, The modal link is using an unencoded lessonPath when building actionHtml, causing inconsistent encoding versus the earlier encoded usage; update the construction of actionHtml to use the same encoded value (e.g., the encoded lessonPath variable or call encodeURIComponent(lessonPath)) so both places consistently encode query parameters (refer to actionHtml and lessonPath where the link is built).
🧹 Nitpick comments (1)
site/lesson.html (1)
2113-2134: ⚡ Quick winTest endpoint only receives the currently selected file's edits.
When a user edits multiple files across different selections,
testCurrentonly sends the currently visible file in theeditsobject. If tests depend on changes across multiple files, they will fail unexpectedly because other modified files retain their original content in the temp copy.Consider tracking all modified files across the session:
Proposed enhancement: Track all edits
var originalByFile = {}; + var editedByFile = {}; function loadFile(filePath) { setOut('loading', 'Fetching ' + filePath + '…'); + // Save current edits before switching + var prevFile = sel.value; + if (prevFile && ed.value !== originalByFile[prevFile]) { + editedByFile[prevFile] = ed.value; + } fetch(fileUrl(filePath)) .then(function (r) { if (!r.ok) throw new Error('file-missing'); return r.text(); }) .then(function (txt) { if (typeof originalByFile[filePath] !== 'string') originalByFile[filePath] = txt; - ed.value = txt; + ed.value = editedByFile[filePath] || txt; setOut('', ''); }) // ... function testCurrent() { var filePath = sel.value; setOut('testing', 'Running tests…'); - var edits = {}; - edits[filePath] = ed.value; + // Include all modified files + editedByFile[filePath] = ed.value; + var edits = Object.assign({}, editedByFile);🤖 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 `@site/lesson.html` around lines 2113 - 2134, testCurrent currently builds a local edits map containing only the active file (edits[filePath] = ed.value) so tests miss changes made to other files; persist a session-wide edits map (e.g., modifiedEdits) and update it whenever the editor content or selection changes (handlers in the file-selection/change logic should set modifiedEdits[filePath] = ed.value or delete if unchanged), then have testCurrent send the full modifiedEdits instead of a single-file map in its POST body; ensure any save/reset logic also updates modifiedEdits so the server receives all modified files for testing.
🤖 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 `@site/dashboard.js`:
- Around line 123-132: The reset handler currently calls
window.AIFSProgress.reset() then renderKpis(PHASES) and renderPhaseCards(PHASES)
but does not update the Continue CTA target; modify the click handler for
resetBtn to call the full render() (or explicitly recompute/update the Continue
button target) after reset so the `#continueBtn` state/target is recalculated;
update the resetBtn listener that references window.AIFSProgress.reset(),
renderKpis, and renderPhaseCards to invoke render() (or the function that builds
the Continue CTA) as the final step.
In `@site/FRONTEND.md`:
- Around line 13-18: The doc snippet contradicts its preamble: it says "From the
repo root" but then runs "cd ai-engineering-from-scratch", which will fail for
users already at root; in the code block that contains "cd
ai-engineering-from-scratch" and "node site/local-server.mjs" remove the "cd
ai-engineering-from-scratch" line (or alternatively change the preamble to "From
the parent directory") so the sequence is consistent and simply runs "node
site/local-server.mjs" from the repo root (refer to the command lines "cd
ai-engineering-from-scratch" and "node site/local-server.mjs" in the snippet).
In `@site/header.js`:
- Around line 47-50: The local-host detection in header.js using variables host
and proto only checks for '127.0.0.1' and 'localhost' so add '0.0.0.0' and the
IPv6 loopback '::1' to the condition that calls paint(0); update the if that
currently reads (proto === 'file:' || host === '127.0.0.1' || host ===
'localhost') to also test host === '0.0.0.0' and host === '::1' (and if your
code may encounter bracketed IPv6 addresses, normalize host by trimming
surrounding brackets before comparison) so local-first behavior covers common
local dev hosts.
In `@site/local-server.mjs`:
- Around line 135-138: ensureTmpDir currently creates temp directories that are
never removed; update the code to remove those dirs after use by changing the
callers (/api/run and /api/test handlers) to wrap their execution in try/finally
and call a safe removal (e.g. await fsp.rm(tmpDir, { recursive: true, force:
true }) or fs.promises.rm) in the finally block; ensure the tmpDir returned from
ensureTmpDir is passed to the handlers' finally, and handle any removal errors
silently/log them so cleanup won't throw and will always run.
In `@site/phase.js`:
- Line 118: The code in phase.js uses escapeAttr(lessonPath) but escapeAttr is
defined only in lesson.html, causing a ReferenceError; fix by extracting the
escapeAttr implementation into a shared utility module (e.g., utils/escape.js)
and import or include that before phase.js so escapeAttr is available to
phase.html, or alternatively add an equivalent local helper function named
escapeAttr at the top of phase.js and use it for the line that constructs the
toggle button (the html += '<button ... data-path="' + escapeAttr(lessonPath) +
'">...' expression); ensure the helper matches the original escaping semantics
to avoid XSS regressions.
---
Outside diff comments:
In `@site/app.js`:
- Around line 61-64: computeStats() and renderPhases() are still using
PHASES[p].status to determine completed phases (see the completePhases loop),
causing UI to disagree with user progress; change them to derive phase status
from actual user progress/lesson completion counts instead of static
PHASES[].status. Specifically, in computeStats() replace the loop that counts
completePhases from PHASES with logic that inspects the user's
progress/lessonsCompleted data for each phase (e.g., map each phase id to number
of completed lessons and consider a phase complete when its completedLessons ===
totalLessons), store that computed status/counts on the stats object, and then
update renderPhases() to read the phase status/counts from the computed stats
(not PHASES[p].status) so the UI classes reflect real progress. Ensure you
update any references to completePhases, PHASES[p].status, and phase completion
checks to use the new computed values.
- Line 233: The modal link is using an unencoded lessonPath when building
actionHtml, causing inconsistent encoding versus the earlier encoded usage;
update the construction of actionHtml to use the same encoded value (e.g., the
encoded lessonPath variable or call encodeURIComponent(lessonPath)) so both
places consistently encode query parameters (refer to actionHtml and lessonPath
where the link is built).
---
Nitpick comments:
In `@site/lesson.html`:
- Around line 2113-2134: testCurrent currently builds a local edits map
containing only the active file (edits[filePath] = ed.value) so tests miss
changes made to other files; persist a session-wide edits map (e.g.,
modifiedEdits) and update it whenever the editor content or selection changes
(handlers in the file-selection/change logic should set modifiedEdits[filePath]
= ed.value or delete if unchanged), then have testCurrent send the full
modifiedEdits instead of a single-file map in its POST body; ensure any
save/reset logic also updates modifiedEdits so the server receives all modified
files for testing.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 073cb36d-4633-4f15-a18e-901c5335b25e
📒 Files selected for processing (12)
README.mdsite/FRONTEND.mdsite/app.jssite/dashboard.htmlsite/dashboard.jssite/header.jssite/index.htmlsite/lesson.htmlsite/local-server.mjssite/phase.htmlsite/phase.jssite/style.css
Comment on lines
+123
to
+132
| var resetBtn = document.getElementById('resetBtn'); | ||
| if (resetBtn) { | ||
| resetBtn.addEventListener('click', function () { | ||
| if (!window.AIFSProgress) return; | ||
| var ok = window.confirm('Clear all your local progress (quiz answers and completed lessons)? This cannot be undone.'); | ||
| if (!ok) return; | ||
| window.AIFSProgress.reset(); | ||
| renderKpis(PHASES); | ||
| renderPhaseCards(PHASES); | ||
| }); |
There was a problem hiding this comment.
Reset flow leaves the Continue button target stale.
After window.AIFSProgress.reset(), KPIs/cards rerender, but #continueBtn is not recomputed. Re-run full render() (or recompute target) so CTA matches new state.
Suggested fix
- window.AIFSProgress.reset();
- renderKpis(PHASES);
- renderPhaseCards(PHASES);
+ window.AIFSProgress.reset();
+ render();🤖 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 `@site/dashboard.js` around lines 123 - 132, The reset handler currently calls
window.AIFSProgress.reset() then renderKpis(PHASES) and renderPhaseCards(PHASES)
but does not update the Continue CTA target; modify the click handler for
resetBtn to call the full render() (or explicitly recompute/update the Continue
button target) after reset so the `#continueBtn` state/target is recalculated;
update the resetBtn listener that references window.AIFSProgress.reset(),
renderKpis, and renderPhaseCards to invoke render() (or the function that builds
the Continue CTA) as the final step.
Comment on lines
+13
to
+18
| From the repo root: | ||
|
|
||
| ```bash | ||
| cd ai-engineering-from-scratch | ||
| node site/local-server.mjs | ||
| ``` |
There was a problem hiding this comment.
Quick-start command conflicts with stated working directory.
Line 13 says “From the repo root,” but Line 16 then cds into the repo again, which fails for users already at root.
Suggested doc fix
## Quick start
From the repo root:
```bash
-cd ai-engineering-from-scratch
node site/local-server.mjs</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
From the repo root:
🤖 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 `@site/FRONTEND.md` around lines 13 - 18, The doc snippet contradicts its
preamble: it says "From the repo root" but then runs "cd
ai-engineering-from-scratch", which will fail for users already at root; in the
code block that contains "cd ai-engineering-from-scratch" and "node
site/local-server.mjs" remove the "cd ai-engineering-from-scratch" line (or
alternatively change the preamble to "From the parent directory") so the
sequence is consistent and simply runs "node site/local-server.mjs" from the
repo root (refer to the command lines "cd ai-engineering-from-scratch" and "node
site/local-server.mjs" in the snippet).
Comment on lines
+47
to
+50
| var host = String(window.location && window.location.hostname || ''); | ||
| var proto = String(window.location && window.location.protocol || ''); | ||
| if (proto === 'file:' || host === '127.0.0.1' || host === 'localhost') { | ||
| paint(0); |
There was a problem hiding this comment.
Local-host detection should include 0.0.0.0 and ::1.
Current guard misses common local dev hosts, so “local-first” can still make network calls in those setups.
Suggested fix
- if (proto === 'file:' || host === '127.0.0.1' || host === 'localhost') {
+ if (
+ proto === 'file:' ||
+ host === '127.0.0.1' ||
+ host === 'localhost' ||
+ host === '0.0.0.0' ||
+ host === '::1'
+ ) {📝 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
| var host = String(window.location && window.location.hostname || ''); | |
| var proto = String(window.location && window.location.protocol || ''); | |
| if (proto === 'file:' || host === '127.0.0.1' || host === 'localhost') { | |
| paint(0); | |
| var host = String(window.location && window.location.hostname || ''); | |
| var proto = String(window.location && window.location.protocol || ''); | |
| if ( | |
| proto === 'file:' || | |
| host === '127.0.0.1' || | |
| host === 'localhost' || | |
| host === '0.0.0.0' || | |
| host === '::1' | |
| ) { | |
| paint(0); |
🤖 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 `@site/header.js` around lines 47 - 50, The local-host detection in header.js
using variables host and proto only checks for '127.0.0.1' and 'localhost' so
add '0.0.0.0' and the IPv6 loopback '::1' to the condition that calls paint(0);
update the if that currently reads (proto === 'file:' || host === '127.0.0.1' ||
host === 'localhost') to also test host === '0.0.0.0' and host === '::1' (and if
your code may encounter bracketed IPv6 addresses, normalize host by trimming
surrounding brackets before comparison) so local-first behavior covers common
local dev hosts.
Comment on lines
+135
to
+138
| async function ensureTmpDir(prefix) { | ||
| const dir = await fsp.mkdtemp(path.join(os.tmpdir(), prefix)); | ||
| return dir; | ||
| } |
There was a problem hiding this comment.
Temporary directories are never cleaned up, causing disk space leaks.
ensureTmpDir creates directories that are used by /api/run and /api/test but never deleted. Over repeated usage, this will accumulate orphaned temp directories in the system's temp folder.
Proposed fix: Add cleanup after execution
+async function removeDirRecursive(dir) {
+ try {
+ await fsp.rm(dir, { recursive: true, force: true });
+ } catch {}
+}
+
async function ensureTmpDir(prefix) {
const dir = await fsp.mkdtemp(path.join(os.tmpdir(), prefix));
return dir;
}Then in /api/run and /api/test handlers, wrap the execution in a try/finally:
const tmp = await ensureTmpDir('aifs-run-');
+ try {
const dstLesson = path.join(tmp, 'lesson');
// ... existing code ...
return sendJson(res, 200, { ... });
+ } finally {
+ removeDirRecursive(tmp);
+ }🤖 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 `@site/local-server.mjs` around lines 135 - 138, ensureTmpDir currently creates
temp directories that are never removed; update the code to remove those dirs
after use by changing the callers (/api/run and /api/test handlers) to wrap
their execution in try/finally and call a safe removal (e.g. await
fsp.rm(tmpDir, { recursive: true, force: true }) or fs.promises.rm) in the
finally block; ensure the tmpDir returned from ensureTmpDir is passed to the
handlers' finally, and handle any removal errors silently/log them so cleanup
won't throw and will always run.
| html += '<div class="lesson-tile-actions">'; | ||
| if (lessonPath) html += '<a class="btn btn-primary" href="lesson.html?path=' + encodeURIComponent(lessonPath) + '">Open<\/a>'; | ||
| if (hasProgress && lessonPath) { | ||
| html += '<button class="btn lesson-toggle" type="button" data-path="' + escapeAttr(lessonPath) + '">' + (userComplete ? 'Unmark' : 'Mark done') + '<\/button>'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all escapeAttr definitions/usages with context.
rg -n -C3 '\bescapeAttr\b'
# Confirm which scripts are loaded by phase.html and their order.
rg -n -C2 '<script .*src=' site/phase.htmlRepository: rohitg00/ai-engineering-from-scratch
Length of output: 3735
Fix escapeAttr usage on the phase page
site/phase.js calls escapeAttr(lessonPath) at line 118, but escapeAttr is defined only in site/lesson.html; the scripts loaded by site/phase.html (data.js, progress.js, header.js, cmdpalette.js, then phase.js) don’t define it. This can throw a ReferenceError when rendering the lesson toggle—move escapeAttr to a shared utility loaded by phase.html, or add an equivalent local helper to phase.js.
🤖 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 `@site/phase.js` at line 118, The code in phase.js uses escapeAttr(lessonPath)
but escapeAttr is defined only in lesson.html, causing a ReferenceError; fix by
extracting the escapeAttr implementation into a shared utility module (e.g.,
utils/escape.js) and import or include that before phase.js so escapeAttr is
available to phase.html, or alternatively add an equivalent local helper
function named escapeAttr at the top of phase.js and use it for the line that
constructs the toggle button (the html += '<button ... data-path="' +
escapeAttr(lessonPath) + '">...' expression); ensure the helper matches the
original escaping semantics to avoid XSS regressions.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
site/app.js (2)
45-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a single completion source for phase metrics and styling.
completeLessons/ per-phasedonenow come fromAIFSProgress, butcompletePhasesand the row status badge still come from staticPHASES[*].status. A fresh profile can therefore show0 / Nlessons completed while phases still render as complete. Derive phase completion and styling from the same per-lesson progress data to keep the dashboard coherent.Also applies to: 105-128
🤖 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 `@site/app.js` around lines 45 - 69, computeStats currently mixes sources: it counts completeLessons from AIFSProgress but computes completePhases and UI badges from PHASES[*].status, causing inconsistent displays. Change computeStats (and the row badge rendering logic referenced around the 105-128 area) so completePhases is derived by iterating each PHASES[p].lessons and treating a phase as complete only when every lesson's URL path returns true from window.AIFSProgress.isLessonComplete(after using window.AIFSProgress.extractPath) (fall back to PHASES status only if AIFSProgress is unavailable), and use that same per-lesson result to decide per-phase "done" styling/badges instead of reading PHASES[p].status directly.
224-237:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the static lesson status as the fallback.
The unconditional
else statusClass = 'planned'also runs whenwindow.AIFSProgressis unavailable, so the modal loses the curriculum's built-in status metadata and every unfinished lesson looks planned. Only overridel.statuswhen local progress is actually available.Suggested fix
var statusClass = l.status.replace(/ /g, '-'); - if (userComplete) statusClass = 'complete'; - else statusClass = 'planned'; + if (hasProgress) { + statusClass = userComplete ? 'complete' : 'planned'; + }🤖 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 `@site/app.js` around lines 224 - 237, The code unconditionally overwrites the lesson's original status with 'planned' even when no local progress exists; change the logic in the loop around hasProgress/userComplete/statusClass so that you initialize statusClass from l.status (statusClass = l.status.replace(/ /g, '-')), then only override it to 'complete' when userComplete is true and only set it to 'planned' when hasProgress is true and userComplete is false (i.e., if (!userComplete && hasProgress) statusClass = 'planned'); ensure you reference the existing variables userComplete, hasProgress, statusClass and l.status when making this conditional change.
🤖 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 `@README.md`:
- Around line 136-137: Remove the duplicated "Option C — find your level
*(recommended)*." heading by keeping only the updated line that reads "Inside
Claude, Cursor, Codex, OpenClaw, Hermes, or any agent with the curriculum skills
installed" and deleting the earlier/duplicate line that mentions "SkillKit
installed"; ensure the README's Getting started flow contains the single
corrected Option C heading.
---
Outside diff comments:
In `@site/app.js`:
- Around line 45-69: computeStats currently mixes sources: it counts
completeLessons from AIFSProgress but computes completePhases and UI badges from
PHASES[*].status, causing inconsistent displays. Change computeStats (and the
row badge rendering logic referenced around the 105-128 area) so completePhases
is derived by iterating each PHASES[p].lessons and treating a phase as complete
only when every lesson's URL path returns true from
window.AIFSProgress.isLessonComplete(after using
window.AIFSProgress.extractPath) (fall back to PHASES status only if
AIFSProgress is unavailable), and use that same per-lesson result to decide
per-phase "done" styling/badges instead of reading PHASES[p].status directly.
- Around line 224-237: The code unconditionally overwrites the lesson's original
status with 'planned' even when no local progress exists; change the logic in
the loop around hasProgress/userComplete/statusClass so that you initialize
statusClass from l.status (statusClass = l.status.replace(/ /g, '-')), then only
override it to 'complete' when userComplete is true and only set it to 'planned'
when hasProgress is true and userComplete is false (i.e., if (!userComplete &&
hasProgress) statusClass = 'planned'); ensure you reference the existing
variables userComplete, hasProgress, statusClass and l.status when making this
conditional change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bb48cf26-26df-4251-ae41-9e47f81704c1
📒 Files selected for processing (5)
README.mdsite/app.jssite/index.htmlsite/lesson.htmlsite/style.css
Comment on lines
+136
to
137
| **Option C — find your level *(recommended)*.** Skip ahead intelligently. Inside Claude, Cursor, Codex, OpenClaw, Hermes, or any agent with SkillKit installed: | ||
| **Option C — find your level *(recommended)*.** Skip ahead intelligently. Inside Claude, Cursor, Codex, OpenClaw, Hermes, or any agent with the curriculum skills installed: |
There was a problem hiding this comment.
Remove duplicated Option C heading line.
Line 136 and Line 137 both define the same “Option C” heading; this renders duplicate content in the Getting started flow. Keep only one line (prefer the updated “curriculum skills installed” wording).
🤖 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 `@README.md` around lines 136 - 137, Remove the duplicated "Option C — find
your level *(recommended)*." heading by keeping only the updated line that reads
"Inside Claude, Cursor, Codex, OpenClaw, Hermes, or any agent with the
curriculum skills installed" and deleting the earlier/duplicate line that
mentions "SkillKit installed"; ensure the README's Getting started flow contains
the single corrected Option C heading.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Kind of change
Checklist
LESSON_TEMPLATE.mdstructure[Name](phases/...)), not bare textdocs/en.mdclaimsPhase / lesson
Notes for reviewer