Uh oh!
There was an error while loading. Please reload this page.
Frontend revamp (M1-M9 + deferred items + production hardening) - #62
Merged
Conversation
added 5 commits
April 19, 2026 14:42
Routes:
- PATCH/DELETE /api/calendar/assignments/{id} with whitelist, 404s, and
empty course_id -> NULL.
- GET /api/study-guide list/detail/regenerate already existed; tests now
cover cache hit/miss, exam keyword filter, and regenerate delete+insert.
- GET /api/admin/{roles,achievements,cosmetics} list endpoints for the
new Admin catalog tabs.
- GET /api/profile/username/check debounced availability probe with
invalid/taken/self reasons.
- GET /api/profile/{id}/cosmetics/catalog grouped by type with
per-item `owned` flag.
- GET /api/profile/{id}/achievements now joins achievement_triggers
and enriches each locked non-secret achievement with
{progress: {current, target}} via the new public get_user_stat().
- GET /api/social/rooms/{id}/messages accepts before/limit, returns
has_more, clamps limit to [1, 200], and serves ascending order.
Services:
- services/achievement_service.py exposes get_user_stat() as a public
wrapper so routes can compute progress without reaching into the
underscore-prefixed helper.
Tests:
- +34 new cases in test_calendar_routes, test_admin_routes,
test_profile_routes, plus new test_study_guide_routes and
test_social_messages. Full suite: 291 pass / 3 skip / 0 fail.
Docs:
- migration_cosmetics.sql documents the required `cosmetic-assets`
public Storage bucket for admin cosmetic asset uploads.Removes the legacy Next.js layout, jest harness, Dockerfile, and the old /signin, /privacy, /terms, /about, /careers, /flashcards pages. New app shell under src/app/(shell) with Sidebar + FloatingActions + global feedback flows. New screen components under components/screens for Dashboard, Learn, Tree, Study, Library, Calendar, Social, Achievements, Settings, Admin, plus a new public Profile page at /profile/[userId]. Onboarding and Auth live outside the shell. Milestone coverage: - M7 Study: /study now toggles between Study Guide (course -> exam cascading picker, recent-guides sidebar, per-topic cards, regenerate) and Flashcards (course-scoped generation, topic pills, 3D flip, 1/2/3 rating + Space/1/2/3 keyboard, "Generated using N library docs" chip). - M8 Profile/Settings/Admin/Achievements: public ProfileView, Settings tabs + CustomSelect + username availability + avatar upload (5 MB guard) + preview modal + cosmetics manager with Owned/Catalog toggle, Admin rewritten with Users/Roles/ Achievements/Cosmetics/Analytics tabs and RoleBadge assign/revoke inline, Achievements editable showcase (up to 5, drag-reorder) + progress bars + unlock toast via focus delta. - M9 Polish & a11y: useBodyScrollLock hook threaded through every full-viewport overlay, role="log"+aria-live on ChatPanel, skip-to- content link, prefers-reduced-motion CSS + one-shot d3 settle, KnowledgeGraph `comparison` prop + mastery bars in Social overview. Deferred items resolved: inline assignment edit, room-message pagination (before/limit + scroll preservation), KnowledgeGraph comparison overlay, Dashboard mobile tabs (already present - no change). Client-side housekeeping: local-mode shim handlers for every new endpoint, typed helpers in lib/api.ts, Link-based entry points to the public profile from Social directory, MemberRow, and StudyMatch cards.
- docs/frontend-audit/ captures the pre-revamp surface area: routes, components, state, API surface, auth, realtime, integrations, and per-feature notes, plus a rebuild checklist and a gap plan that drove the revamp roadmap. - CLAUDE.md: the Jest harness and src/__tests__ were removed on this branch, so the "npm test" instruction was misleading. Swap it for `npx tsc --noEmit` + manual smoke testing until a new harness is reintroduced.
…e gaps
Full-branch audit caught three real bugs:
1. M3 Learn screen fetched the knowledge graph but only rendered session
metadata in the right aside. Now renders <KnowledgeGraph> (with the
same api->data adapter Dashboard uses), highlights the node whose
name matches the current topic, and lets a click on a concept
deep-link to /learn?topic=<name>.
2. routes/social.py get_room_messages accepted `before` directly into a
PostgREST filter (`lt.{before}`) with no validation, so a request
like ?before=null or ?before=gt.2026-01-01 could distort the query.
Now parsed through datetime.fromisoformat first; invalid values 400.
New test case (+1 -> 7 passing) covers the rejection path.
3. lib/localData.ts was silently warning on /api/learn/action,
/api/learn/mode-switch, and /api/learn/sessions/{id}/resume in local
dev. Added stubs so local mode stays usable without the backend.
Also verified as false positives (no change needed): deleteAccount and
featured-achievements endpoints both exist; Settings username check
uses GET checkUsername not PATCH; Calendar saveEdit already calls
updateAssignment. Known alpha-mode design choice (many endpoints
accept user_id without require_self) is pervasive and out of scope
for an audit pass; leaving as-is.
Suite: 292 pass / 3 skip / 0 fail (backend), tsc clean (frontend).Deep audit turned up six deploy-fragile issues that passed local dev
and `next build` but would surface in a real HTTPS deploy:
1. Hydration mismatch on Dashboard greeting.
getGreetingPrefix(new Date()) was computed during render, so the
server's timezone would diverge from the client's and trigger a
React hydration error. Moved behind a useEffect.
2. Hydration mismatch on Dashboard random quote.
useMemo(() => QUOTES[Math.floor(Math.random()*...)]) picked a
different quote on server vs client. Moved to useState + useEffect
seeded after mount.
3. Session cookie missing Secure flag.
api/auth/session POST and DELETE set sapling_session without
`secure: true`; browsers drop the cookie on HTTPS origins, so
users appear logged-out after OAuth. Now set based on NODE_ENV.
4. Localhost fallback in Auth.tsx and auth/callback/page.tsx.
`process.env.NEXT_PUBLIC_API_URL ?? 'http://localhost:5000'` would
silently route production OAuth redirects to localhost. Replaced
with empty-string fallback so Next.js's /api/:path* rewrite takes
over when the env var is unset in the deploy environment.
5. UserContext direct fetches produced undefined URLs.
`${process.env.NEXT_PUBLIC_API_URL}/api/users` with an unset var
becomes the literal string `undefined/api/users`. Added the same
empty-fallback pattern so calls become relative and Next.js
rewrites them server-side.
6. Settings data-export fetch: same fix as #5.
Local verification: tsc --noEmit clean, `next build` clean (18
routes compiled, Middleware -> Proxy deprecation warning unchanged
and non-blocking).
False positives investigated and dismissed:
- page.tsx files without 'use client' are fine; Next.js lets server
page components render client component children.
- UserContext localStorage access is inside useEffect, not a
hydration risk.
- Supabase proxy client is lazy; only crashes if Social is opened
without NEXT_PUBLIC_SUPABASE_* set, which is the desired behavior.Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated frontend cookie handling and client API base derivation to prefer env-driven or same-origin URLs, moved client-only initializations to mount-time state, added OpenNext/Cloudflare dev init and config plus Next rewrites to proxy backend, removed Tailwind/global resets, added many agent skill docs and a cleanup script. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant NextJS
participant Backend
participant Cloudflare
User->>Browser: Click Sign in / OAuth callback
Browser->>NextJS: GET /api/auth/google or /api/auth/callback
Note right of NextJS: NEXT_PUBLIC_API_URL may be '' (same-origin) or external
NextJS->>Backend: Proxy /api/:path* rewrite (or passthrough /api/auth/session)
Backend-->>NextJS: Auth response + session token
NextJS-->>Browser: Set-Cookie: sapling_session (httpOnly, sameSite=lax, secure=NODE_ENV==='production')
Browser->>NextJS: Subsequent requests to /api/... (same-origin) or to BACKEND via rewrite
Cloudflare-->>NextJS: (dev init in dev only) OpenNext Cloudflare bindings active when running dev
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Deploying web with |
| Latest commit: | 933ddc0 |
| Status: | 🚫 Build failed. |
added 6 commits
April 19, 2026 15:29
The previous Cloudflare Pages deploy failed with: Error: Output directory "frontend/out" not found. Failed: build output directory not found Root cause: the framework preset on Cloudflare was "Next.js (Static HTML Export)", which sets the build command to `STATIC_EXPORT=true npm run build` and expects a `frontend/out/` directory from `next export`. Our app isn't exportable — it relies on middleware (auth guard), API routes (/api/auth/session), rewrites (/api/:path* -> BACKEND_URL), and dynamic routes (/profile/[userId]). Fix: add @opennextjs/cloudflare, which compiles Next.js into a Cloudflare Workers-compatible bundle (worker.js + assets/). Supports middleware and full SSR. Also add wrangler as a dev dep so `wrangler pages deploy` and `wrangler deploy` work locally. Tried @cloudflare/next-on-pages first, but its peer range is next@14.3–15.5 and we're on 16.1.6. Switched to @opennextjs/cloudflare (actively maintained, supports 16). Files: - frontend/package.json: adds cf:build, cf:preview, cf:deploy scripts and @opennextjs/cloudflare + wrangler dev deps. - frontend/wrangler.jsonc: Workers config (main=.open-next/worker.js, assets binding, nodejs_compat flag, compat date 2025-06-01). - frontend/open-next.config.ts: minimal OpenNext config. - frontend/next.config.ts: initOpenNextCloudflareForDev() so `next dev` works with Cloudflare bindings. `output: "standalone"` left in place (OpenNext does its own packaging, and keeping it lets `next build` produce a Docker-style server for any non-Cloudflare deploy). - frontend/.gitignore: ignore .open-next/ and .wrangler/. Local verification: `npm run cf:build` succeeds; produces `.open-next/worker.js`. `tsc --noEmit` clean. `next build` alone still clean (18 routes). Cloudflare Pages dashboard settings the user must update: - Build command: `npm run cf:build` (was STATIC_EXPORT=true npm run build) - Build output directory: `frontend/.open-next/assets` (was frontend/out) - OR migrate the project to Cloudflare Workers (recommended for SSR apps; `wrangler deploy` from frontend/ will work as-is).
The last deploy log showed Cloudflare Pages parsing wrangler.jsonc and rejecting it: A Wrangler configuration file was found but it does not appear to be valid. Did you mean to use wrangler.toml to configure Pages? If so, then make sure the file is valid and contains the `pages_build_output_dir` property. Pages wants a Pages-style config. But this repo can't target Pages regardless: * `next export` (Pages' only supported mode) doesn't work with middleware, /api/auth/session, the /api/:path* rewrite, or /profile/[userId] without generateStaticParams. * @cloudflare/next-on-pages (the Pages adapter) peer-deps on next@14.3–15.5; we're on 16.1.6. * @opennextjs/cloudflare (what we use) compiles for Workers. Switching the config to wrangler.toml with a clear header comment explaining Workers is the deploy target, and spelling out the two config shapes (Workers: main + [assets]; Pages: pages_build_output_dir) so future readers don't have to rediscover this. Also tidied .env.example: - NEXT_PUBLIC_API_URL now has an empty default (calls go same-origin via the Next rewrite, which avoids CORS). - BACKEND_URL added explicitly, used by the /api/:path* rewrite. `npm run cf:build` still produces .open-next/worker.js cleanly.
The last Pages deploy log (commit f7efe14) showed Cloudflare Pages still detecting wrangler.toml and rejecting it as invalid for Pages: A Wrangler configuration file was found but it does not appear to be valid. Did you mean to use wrangler.toml to configure Pages? If so, then make sure the file is valid and contains the `pages_build_output_dir` property. The file is correct for Workers (has `main` + `[assets]`), just not for Pages. Renaming to wrangler.workers.toml keeps the Workers config available without tripping Pages's auto-detection. Pages now skips directly to its "no out/ dir" failure (which is the real problem and needs a dashboard change — see PR body). Updated scripts: - cf:build unchanged (opennextjs-cloudflare build doesn't need the wrangler config). - cf:preview / cf:deploy now pass -c wrangler.workers.toml so wrangler finds the renamed file. No behavioral change when deploying on Workers — just stops the noise in the (failing) Pages build log.
Feature-parity audit of pre-revamp main@929658f against audit-fixes tip turned up three regressions. The backend routes still exist for all three; the revamp just didn't rewire them into the new UI. 1. Admin → Achievements → "Grant to user" form OLD /admin had adminGrantAchievement wired to a two-field form (user, achievement). The revamp's rewritten Admin screen kept the create/delete flows but dropped the grant flow. Added a "Grant to user" block at the bottom of the AchievementsTab left column with two CustomSelects (user, achievement) and a button that calls adminGrantAchievement(user_id, achievement_id). 2. Study flashcards → delete card OLD /flashcards had a delete button per card calling deleteFlashcard(user_id, card_id). The revamp dropped the delete action. Added a subtle "Delete card" button below the rating row in Study's FlashcardsMode; re-fetches on success. 3. Calendar → "View Google events" OLD Calendar had a "View upcoming Google events" button that called importGoogleEvents(user_id, days_ahead=60) and rendered the result. Revamp kept connect/sync/disconnect but dropped the import-preview. Added the button next to Sync + Disconnect (only when Google is connected) and a modal (GoogleEventsModal) that lists the 60-day upcoming events with title/time/location/link. Also preserved the OLD rating label "easy" (was changed to "good" in the M7 rewrite — reverted to match OLD UX). API helpers added back to lib/api.ts: - deleteFlashcard(userId, cardId) - importGoogleEvents(userId, daysAhead) - adminGrantAchievement(userId, achievementId) - GoogleEvent interface Local-mode stubs added for the three new routes so local dev doesn't warn. Verified: tsc clean, next build clean (18 routes), dev server reachable on all routes. Non-regressions investigated and intentionally not acted on: - submitJobApplication: careers pages deleted intentionally. - setFeaturedRole: backend exists but OLD Settings page didn't clearly wire a "pick your featured role" UI either. - exportToGoogleCalendar: bulk sync via /api/calendar/sync covers the 90% case; per-selection export is a niche workflow.
Deeper per-area audit (4 parallel passes covering the core loop,
content+collab, profile/admin, and auth/global) turned up three more
real regressions — backend endpoints unchanged, UI bindings lost:
1. Learn `?suggest=<concept>` no longer highlighted a node.
OLD learn/page.tsx:38 + :117–119 + :500 used suggestConcept from
searchParams to pre-highlight the Dashboard's "Learn next" pick.
NEW Learn only honored the current topic for highlight. Fixed in
Learn.tsx: highlightId now prefers ?suggest= (if the concept
exists in the graph) and falls back to topic, so deep links from
Dashboard work again.
2. Calendar table-view had no sort / ordering controls.
OLD AssignmentTable maintained sortKey/sortDirection state and
rendered a sort-by + direction picker. NEW had only CSV export
and bulk-select. Added a CustomSelect (due date / title / course
/ type) + asc/desc toggle button above the table; the table now
iterates over the sorted memo instead of the raw list.
3. Signed-in users weren't redirected away from /auth.
OLD middleware.ts:60–61 had a redirectIfSignedIn helper that
bounced an already-authenticated visitor to /dashboard when they
hit /signin. NEW middleware didn't handle /auth (the renamed
route) so a signed-in user could see the sign-in form again. Added
an explicit handler at the top of middleware() that verifies the
session cookie and redirects to /dashboard, plus '/auth' +
'/auth/' to the matcher so the middleware runs for that route.
False positives investigated and dismissed:
- Root layout missing FeedbackFlow/SessionFeedbackGlobal. These
live in (shell)/layout.tsx:21–22 which is correct — they're
only needed for signed-in shell routes, not /auth or /onboarding.
- RoleBadge icon = emoji instead of URL. The new Admin form
explicitly asks for an emoji ("Icon (emoji)"), and the existing
seed data uses NULL, so no legacy URL icons would regress.
- /flashcards removed from the protected-routes array. Intentional;
the route was retired with the M7 consolidation into /study.
- Social mention color-coding and initial-load skeleton — cosmetic
polish differences, not functional regressions.
Verified: tsc clean, next build clean (18 routes), npm run cf:build
clean. Dev server reachable and all routes 200.`npx skills add pbakaus/impeccable -y` dropped 17 skill definitions into .agents/skills/ (the tracked source of truth) and created symlink shims under .claude/skills/ (already ignored) and /skills/ (newly ignored). Also committing the skills-lock.json so installs are reproducible. Installed skills: adapt, animate, audit, bolder, clarify, colorize, critique, delight, distill, impeccable, layout, optimize, overdrive, polish, quieter, shape, typeset. Visual Mode itself isn't a standalone skill — it ships as (a) the Chrome Web Store extension at https://chromewebstore.google.com/detail/impeccable/bdkgmiklpdmaojlpflclinlofgjfpabf which must be installed manually, (b) a `/critique` sub-check, and (c) the `npx impeccable live|detect` CLI. Running `npx impeccable detect src/` against the revamped frontend surfaced 9 anti-patterns: 7 layout-transition (transition: width in Sidebar, Dashboard progress bars, Achievements progress, Social MasteryBar, Study flashcard progress, KnowledgeGraph comparison ring), 1 side-tab (MarkdownChat blockquote left-border, Calendar day-view left-border), 1 single-font (only Fraunces in layout.tsx). These are design-polish findings, not functional bugs — flagged for a separate polish pass rather than fixing inline with the skill install. See follow-up commits if the team wants to address them.
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
.agents/skills/bolder/SKILL.md (1)
84-89: Add explicit reduced-motion fallback guidance in the motion section.This section drives aggressive animation choices but doesn’t explicitly require
prefers-reduced-motionalternatives; adding that line would make execution safer.Suggested addition
### Motion & Animation - **Entrance choreography**: Staggered, dramatic page load animations with 50-100ms delays - **Scroll effects**: Parallax, reveal animations, scroll-triggered sequences - **Micro-interactions**: Satisfying hover effects, click feedback, state changes - **Transitions**: Smooth, noticeable transitions using ease-out-quart/quint/expo (not bounce or elastic—they cheapen the effect) +- **Reduced motion**: Provide non-spatial or minimal-motion alternatives for users with `prefers-reduced-motion: reduce`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/bolder/SKILL.md around lines 84 - 89, Update the "Motion & Animation" section to require a reduced-motion fallback by adding a bullet that references the CSS media query prefers-reduced-motion and outlines acceptable alternatives; specifically, beneath the existing bullets (Entrance choreography, Scroll effects, Micro-interactions, Transitions) add guidance such as: detect prefers-reduced-motion and provide reduced or no-animation variants (cut down stagger, disable parallax/reveal, use instant state changes or simple fades), and ensure keyboard focus/ARIA updates remain accessible. Target the "Motion & Animation" heading and the surrounding bullet list so reviewers can find the new guidance easily..agents/skills/quieter/SKILL.md (1)
75-80: Explicitly requireprefers-reduced-motionhandling in Motion Reduction.The section reduces motion conceptually, but adding a direct requirement for
prefers-reduced-motionwould make the instruction auditable and accessibility-safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/quieter/SKILL.md around lines 75 - 80, Update the "Motion Reduction" section in SKILL.md to explicitly require honoring the user's prefers-reduced-motion setting: add a line stating that implementations must detect and respect prefers-reduced-motion (via CSS `@media` (prefers-reduced-motion: reduce) and/or runtime checks in animation utilities) and fallback to no/very-minimal motion (per the existing "Remove animations entirely" guidance), and call out that easing and distance rules (e.g., "Refined easing" and shorter distances) must not apply when the user preference requests reduced motion..agents/skills/optimize/SKILL.md (1)
9-13: Add the standard/impeccablepreparation block for skill consistency.This skill is the outlier in the new skill set; adding the same mandatory prep/context protocol keeps behavior predictable across invocable skills.
🔧 Minimal consistency patch
Identify and fix performance issues to create faster, smoother user experiences. +## MANDATORY PREPARATION++Invoke /impeccable — it contains design principles, anti-patterns, and the **Context Gathering Protocol**. Follow the protocol before proceeding — if no design context exists yet, you MUST run /impeccable teach first.++---+ ## Assess Performance Issues🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/optimize/SKILL.md around lines 9 - 13, This skill is missing the standard "/impeccable" preparation block used by other skills; add an `/impeccable` prep block at the top of SKILL.md for the optimize skill (near the "Assess Performance Issues" heading) containing the usual fields (context/purpose, role instructions, constraints, and any tool/invocation hints) so the skill conforms to the standard prep protocol used by other skills and will be parsed consistently by the runtime..agents/skills/delight/SKILL.md (1)
120-126: Add language identifier to fenced code blocks.Multiple fenced code blocks in this file are missing language identifiers (lines 120, 129, 138, 240). While these are plaintext examples rather than code, adding language identifiers improves tooling support and accessibility.
📋 Proposed fix for plaintext examples
For the error message examples at line 120:
-```+```text "Error 404" "This page is playing hide and seek. (And winning)"For the empty state examples at line 129:
-```+```text "No projects" "Your canvas awaits. Create something amazing."For the playful labels at line 138:
-```+```text "Delete" "Send to void" (for playful brand)For the loading messages at line 240:
-```+```text Loading messages — write ones specific to your product, not generic AI filler:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/delight/SKILL.md around lines 120 - 126, The fenced code blocks in SKILL.md (the plaintext message examples such as the block containing "Error 404" / "This page is playing hide and seek. (And winning)", the block with "No projects" / "Your canvas awaits. Create something amazing.", the block with "Delete" / "Send to void", and the loading messages block) are missing language identifiers; edit each triple-backtick fence to include the "text" language identifier (e.g., change ``` to ```text) so tooling and accessibility can correctly treat these examples as plain text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/audit/SKILL.md:
- Around line 77-83: The score table's row order is inconsistent with the scan
definition: swap the "Theming" and "Responsive Design" rows in the markdown
table so that "Theming" is listed as `#3` and "Responsive Design" as `#4` to match
the scan section; update the table row entries for the Dimension and # columns
(the rows containing "Theming" and "Responsive Design") to reflect the correct
numbering and ordering.
In @.agents/skills/colorize/SKILL.md:
- Around line 97-100: Replace the awkward bullet text "Blobs/organic shapes" in
the list (the line that currently reads 'Blobs/organic shapes: Soft colored
shapes for visual interest') with a clearer phrasing such as "Blobs and organic
shapes: Soft colored shapes for visual interest" (or a hyphenated form "Blobs —
organic shapes") so the heading reads smoothly and matches the style of the
other bullets like "Illustrations" and "Shapes".
In @.agents/skills/critique/reference/heuristics-scoring.md:
- Around line 18-20: Add a blank line before each scoring table and keep one
after so MD058 is satisfied; specifically, for each occurrence where the bold
heading "**Scoring**:" is immediately followed by a table (e.g., the instance
shown and the ones at the listed offsets), insert an empty line between
"**Scoring**:" and the table and ensure there is a blank line after the table as
well.
In @.agents/skills/critique/reference/personas.md:
- Around line 168-176: The fenced template block for personas (the
triple-backtick section containing "### [Role] — \"[Name]\" ..." ) is missing a
language specifier which triggers MD040; update the fence opener from ``` to
```markdown so the block is explicitly marked as markdown (ensure only the
opener is changed and the closing ``` remains); no other content changes are
needed.
In @.agents/skills/impeccable/scripts/cleanup-deprecated.mjs:
- Around line 46-47: The code currently hardcodes root as '/' and uses new
URL(import.meta.url).pathname which breaks on Windows and with URL-encoded
filenames; replace the hardcoded root with path.parse(somePath).root (use
path.parse(file) .root) for cross-platform root detection in the loop that
references dir and root, and replace new URL(import.meta.url).pathname usage
with Node's fileURLToPath(import.meta.url) when deriving the current script file
path (used in the CLI execution check and any variable that builds paths from
import.meta.url). Ensure you import/require the path and url utilities
(path.parse and fileURLToPath) and use the resulting file path when computing
root and any CLI execution checks.
In @.agents/skills/optimize/SKILL.md:
- Line 16: Update the Core Web Vitals references to remove the deprecated FID:
replace the phrase "Core Web Vitals: LCP, FID/INP, CLS" with "Core Web Vitals:
LCP, INP, CLS" and change the section header "First Input Delay (FID < 100ms) /
INP (< 200ms)" to "Interaction to Next Paint (INP < 200ms)"; also scan the
SKILL.md document for any other occurrences of "FID" or "FID/INP" and replace
them with the current "INP" terminology to ensure consistency.
In @.agents/skills/overdrive/SKILL.md:
- Line 29: Fix the incomplete fragment "ask the user directly to clarify what
you cannot infer." in SKILL.md by replacing it with a complete sentence that
matches the surrounding guidance; for example: "Ask the user directly to clarify
anything you cannot infer, present these directions and get the user's pick
before writing any code, and explain trade-offs (browser support, performance
cost, complexity)." Ensure the replacement preserves the intent and punctuation
and appears where the fragment "ask the user directly to clarify what you cannot
infer." currently exists.
In @.agents/skills/polish/SKILL.md:
- Around line 31-34: There’s a contradiction between the "Review completeness"
guidance (the bullet allowing preserving known issues as TODOs) and the later
polish workflow rule that requires no TODOs; decide the policy (either allow
TODOs only when linked to tracked follow-ups or ban them entirely) and make the
language consistent: update the "Review completeness" bullet and the later
polish rule text to state the chosen policy, add the explicit exception syntax
(“TODO allowed only if linked to tracked follow-up with ticket/ID”), and add a
short example and a single TODO-preservation sentence in SKILL.md’s polish
workflow so both references (the Review completeness section and the no-TODOs
rule) match exactly.
In @.agents/skills/shape/SKILL.md:
- Around line 29-30: Capitalize the imperative sentence starters on the
specified lines in SKILL.md: change the lowercase "ask" at the start of the
sentence "ask the user directly to clarify what you cannot infer." (and the
other occurrence at line 94) to "Ask" so the imperative sentences match the
document's capitalization style and read consistently.
---
Nitpick comments:
In @.agents/skills/bolder/SKILL.md:
- Around line 84-89: Update the "Motion & Animation" section to require a
reduced-motion fallback by adding a bullet that references the CSS media query
prefers-reduced-motion and outlines acceptable alternatives; specifically,
beneath the existing bullets (Entrance choreography, Scroll effects,
Micro-interactions, Transitions) add guidance such as: detect
prefers-reduced-motion and provide reduced or no-animation variants (cut down
stagger, disable parallax/reveal, use instant state changes or simple fades),
and ensure keyboard focus/ARIA updates remain accessible. Target the "Motion &
Animation" heading and the surrounding bullet list so reviewers can find the new
guidance easily.
In @.agents/skills/delight/SKILL.md:
- Around line 120-126: The fenced code blocks in SKILL.md (the plaintext message
examples such as the block containing "Error 404" / "This page is playing hide
and seek. (And winning)", the block with "No projects" / "Your canvas awaits.
Create something amazing.", the block with "Delete" / "Send to void", and the
loading messages block) are missing language identifiers; edit each
triple-backtick fence to include the "text" language identifier (e.g., change
``` to ```text) so tooling and accessibility can correctly treat these examples
as plain text.
In @.agents/skills/optimize/SKILL.md:
- Around line 9-13: This skill is missing the standard "/impeccable" preparation
block used by other skills; add an `/impeccable` prep block at the top of
SKILL.md for the optimize skill (near the "Assess Performance Issues" heading)
containing the usual fields (context/purpose, role instructions, constraints,
and any tool/invocation hints) so the skill conforms to the standard prep
protocol used by other skills and will be parsed consistently by the runtime.
In @.agents/skills/quieter/SKILL.md:
- Around line 75-80: Update the "Motion Reduction" section in SKILL.md to
explicitly require honoring the user's prefers-reduced-motion setting: add a
line stating that implementations must detect and respect prefers-reduced-motion
(via CSS `@media` (prefers-reduced-motion: reduce) and/or runtime checks in
animation utilities) and fallback to no/very-minimal motion (per the existing
"Remove animations entirely" guidance), and call out that easing and distance
rules (e.g., "Refined easing" and shorter distances) must not apply when the
user preference requests reduced motion.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 66e4234d-8c6c-4306-a22a-4b592d663119
📒 Files selected for processing (39)
.agents/skills/adapt/SKILL.md.agents/skills/animate/SKILL.md.agents/skills/audit/SKILL.md.agents/skills/bolder/SKILL.md.agents/skills/clarify/SKILL.md.agents/skills/colorize/SKILL.md.agents/skills/critique/SKILL.md.agents/skills/critique/reference/cognitive-load.md.agents/skills/critique/reference/heuristics-scoring.md.agents/skills/critique/reference/personas.md.agents/skills/delight/SKILL.md.agents/skills/distill/SKILL.md.agents/skills/impeccable/SKILL.md.agents/skills/impeccable/reference/color-and-contrast.md.agents/skills/impeccable/reference/craft.md.agents/skills/impeccable/reference/extract.md.agents/skills/impeccable/reference/interaction-design.md.agents/skills/impeccable/reference/motion-design.md.agents/skills/impeccable/reference/responsive-design.md.agents/skills/impeccable/reference/spatial-design.md.agents/skills/impeccable/reference/typography.md.agents/skills/impeccable/reference/ux-writing.md.agents/skills/impeccable/scripts/cleanup-deprecated.mjs.agents/skills/layout/SKILL.md.agents/skills/optimize/SKILL.md.agents/skills/overdrive/SKILL.md.agents/skills/polish/SKILL.md.agents/skills/quieter/SKILL.md.agents/skills/shape/SKILL.md.agents/skills/typeset/SKILL.md.gitignorefrontend/src/components/screens/Admin.tsxfrontend/src/components/screens/Calendar.tsxfrontend/src/components/screens/Learn.tsxfrontend/src/components/screens/Study.tsxfrontend/src/lib/api.tsfrontend/src/lib/localData.tsfrontend/src/middleware.tsskills-lock.json
✅ Files skipped from review due to trivial changes (10)
- .gitignore
- .agents/skills/impeccable/reference/spatial-design.md
- .agents/skills/critique/reference/cognitive-load.md
- .agents/skills/impeccable/reference/color-and-contrast.md
- .agents/skills/impeccable/reference/responsive-design.md
- .agents/skills/impeccable/reference/interaction-design.md
- .agents/skills/impeccable/reference/ux-writing.md
- .agents/skills/impeccable/reference/typography.md
- .agents/skills/impeccable/reference/extract.md
- .agents/skills/impeccable/reference/craft.md
Comment on lines
+77
to
+83
| | # | Dimension | Score | Key Finding | | ||
| |---|-----------|-------|-------------| | ||
| | 1 | Accessibility | ? | [most critical a11y issue or "--"] | | ||
| | 2 | Performance | ? | | | ||
| | 3 | Responsive Design | ? | | | ||
| | 4 | Theming | ? | | | ||
| | 5 | Anti-Patterns | ? | | |
There was a problem hiding this comment.
Fix dimension order mismatch between the scan definition and score table.
In the scan section, Theming is #3 and Responsive Design is #4, but the table reverses them. This makes score mapping ambiguous.
Minimal correction
-| 3 | Responsive Design | ? | |-| 4 | Theming | ? | |+| 3 | Theming | ? | |+| 4 | Responsive Design | ? | |📝 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
| | # | Dimension | Score | Key Finding | | |
| |---|-----------|-------|-------------| | |
| | 1 | Accessibility | ? |[most critical a11y issue or "--"]| | |
| | 2 | Performance | ? || | |
| | 3 |Responsive Design| ? || | |
| | 4 |Theming| ? || | |
| | 5 | Anti-Patterns | ? || | |
| | # | Dimension | Score | Key Finding | | |
| |---|-----------|-------|-------------| | |
| | 1 | Accessibility | ? |[most critical a11y issue or "--"]| | |
| | 2 | Performance | ? || | |
| | 3 |Theming| ? || | |
| | 4 |Responsive Design| ? || | |
| | 5 | Anti-Patterns | ? || |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/audit/SKILL.md around lines 77 - 83, The score table's row
order is inconsistent with the scan definition: swap the "Theming" and
"Responsive Design" rows in the markdown table so that "Theming" is listed as `#3`
and "Responsive Design" as `#4` to match the scan section; update the table row
entries for the Dimension and # columns (the rows containing "Theming" and
"Responsive Design") to reflect the correct numbering and ordering.
Comment on lines
+97
to
+100
| - **Illustrations**: Add colored illustrations or icons | ||
| - **Shapes**: Geometric shapes in brand colors as background elements | ||
| - **Gradients**: Colorful gradient overlays or mesh backgrounds | ||
| - **Blobs/organic shapes**: Soft colored shapes for visual interest |
There was a problem hiding this comment.
Tighten heading phrasing for readability.
“Blobs/organic shapes” reads awkwardly; a hyphenated or “and” form is clearer.
✏️ Proposed copy tweak
-- **Blobs/organic shapes**: Soft colored shapes for visual interest+- **Blobs and organic shapes**: Soft colored shapes for visual interest📝 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
| -**Illustrations**: Add colored illustrations or icons | |
| -**Shapes**: Geometric shapes in brand colors as background elements | |
| -**Gradients**: Colorful gradient overlays or mesh backgrounds | |
| -**Blobs/organic shapes**: Soft colored shapes for visual interest | |
| -**Illustrations**: Add colored illustrations or icons | |
| -**Shapes**: Geometric shapes in brand colors as background elements | |
| -**Gradients**: Colorful gradient overlays or mesh backgrounds | |
| -**Blobs and organic shapes**: Soft colored shapes for visual interest |
🧰 Tools
🪛 LanguageTool
[grammar] ~100-~100: Use a hyphen to join words.
Context: ...grounds - Blobs/organic shapes: Soft colored shapes for visual interest ## B...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/colorize/SKILL.md around lines 97 - 100, Replace the awkward
bullet text "Blobs/organic shapes" in the list (the line that currently reads
'Blobs/organic shapes: Soft colored shapes for visual interest') with a clearer
phrasing such as "Blobs and organic shapes: Soft colored shapes for visual
interest" (or a hyphenated form "Blobs — organic shapes") so the heading reads
smoothly and matches the style of the other bullets like "Illustrations" and
"Shapes".
Comment on lines
+18
to
+20
| **Scoring**: | ||
| | Score | Criteria | | ||
| |-------|----------| |
There was a problem hiding this comment.
Add blank lines around scoring tables to satisfy markdownlint MD058.
The file consistently places tables immediately after **Scoring**: lines. Insert a blank line before each table (and keep one after), otherwise lint remains noisy.
🧹 Example pattern to apply throughout
**Scoring**:
+
| Score | Criteria |
|-------|----------|Also applies to: 38-40, 58-60, 78-80, 98-100, 118-120, 138-140, 158-160, 178-180, 198-200
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 19-19: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/critique/reference/heuristics-scoring.md around lines 18 -
20, Add a blank line before each scoring table and keep one after so MD058 is
satisfied; specifically, for each occurrence where the bold heading
"**Scoring**:" is immediately followed by a table (e.g., the instance shown and
the ones at the listed offsets), insert an empty line between "**Scoring**:" and
the table and ensure there is a blank line after the table as well.
Comment on lines
+168
to
+176
| ``` | ||
| ### [Role] — "[Name]" | ||
| **Profile**: [2-3 key characteristics derived from Design Context] | ||
| **Behaviors**: [3-4 specific behaviors based on the described audience] | ||
| **Red Flags**: [3-4 things that would alienate this specific user type] | ||
| ``` |
There was a problem hiding this comment.
Specify a language for the fenced template block (MD040).
Add a language (e.g., markdown) to the fence opener so markdownlint passes consistently.
✅ Minimal fix
-```+```markdown
### [Role] — "[Name]"
@@
-```+```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/critique/reference/personas.md around lines 168 - 176, The
fenced template block for personas (the triple-backtick section containing "###
[Role] — \"[Name]\" ..." ) is missing a language specifier which triggers MD040;
update the fence opener from ``` to ```markdown so the block is explicitly
marked as markdown (ensure only the opener is changed and the closing ```
remains); no other content changes are needed.
Comment on lines
+46
to
+47
| const { root } = { root: '/' }; | ||
| while (dir !== root) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Verify cross-platform-sensitive patterns in the cleanup script.
fd -i "cleanup-deprecated.mjs" --exec rg -n "root: '/'|new URL\(import\.meta\.url\)\.pathname|fileURLToPath\(|parse\(" {}
# Expected after fix:# - no matches for root: '/'# - no matches for new URL(import.meta.url).pathname# - matches for fileURLToPath( and parse(Repository: SaplingLearn/Sapling
Length of output: 46
🏁 Script executed:
# Find the cleanup-deprecated.mjs file
fd "cleanup-deprecated.mjs" -type fRepository: SaplingLearn/Sapling
Length of output: 234
🏁 Script executed:
# If found, check its existence and read the relevant sectionsif [ -f".agents/skills/impeccable/scripts/cleanup-deprecated.mjs" ];thenecho"=== File exists. Checking line count ===";
wc -l ".agents/skills/impeccable/scripts/cleanup-deprecated.mjs";echo"";echo"=== Lines 40-60 (around line 46-47) ===";
sed -n '40,60p'".agents/skills/impeccable/scripts/cleanup-deprecated.mjs"| cat -n;echo"";echo"=== Lines 195-205 (around line 200) ===";
sed -n '195,205p'".agents/skills/impeccable/scripts/cleanup-deprecated.mjs"| cat -n;elseecho"File not found at .agents/skills/impeccable/scripts/cleanup-deprecated.mjs";# Search for it anywhere in the repoecho"";echo"=== Searching for cleanup-deprecated.mjs anywhere ===";
find . -name "*cleanup-deprecated*" -type f 2>/dev/null;fiRepository: SaplingLearn/Sapling
Length of output: 1541
🏁 Script executed:
head -20 ".agents/skills/impeccable/scripts/cleanup-deprecated.mjs"| cat -nRepository: SaplingLearn/Sapling
Length of output: 988
🏁 Script executed:
sed -n '20,35p'".agents/skills/impeccable/scripts/cleanup-deprecated.mjs"| cat -nRepository: SaplingLearn/Sapling
Length of output: 875
Fix cross-platform path handling for root detection and CLI execution check.
Line 47 hardcodes the root as '/', which breaks on Windows where roots are drive letters like 'C:'. Line 201 uses new URL(import.meta.url).pathname, which is non-standard and fails with URL-encoded filenames. Use path.parse(...).root for dynamic root detection and fileURLToPath(import.meta.url) for idiomatic Node.js ESM path handling.
💡 Minimal patch
-import { join, resolve } from 'node:path';+import { join, parse, resolve } from 'node:path';+import { fileURLToPath } from 'node:url';
export function findProjectRoot(startDir = process.cwd()) {
let dir = resolve(startDir);
- const { root } = { root: '/' };+ const { root } = parse(dir);
while (dir !== root) {
@@
-if (process.argv[1] && resolve(process.argv[1]) === resolve(new URL(import.meta.url).pathname)) {+const isDirectRun =+ process.argv[1] &&+ resolve(process.argv[1]) === resolve(fileURLToPath(import.meta.url));++if (isDirectRun) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/impeccable/scripts/cleanup-deprecated.mjs around lines 46 -
47, The code currently hardcodes root as '/' and uses new
URL(import.meta.url).pathname which breaks on Windows and with URL-encoded
filenames; replace the hardcoded root with path.parse(somePath).root (use
path.parse(file) .root) for cross-platform root detection in the loop that
references dir and root, and replace new URL(import.meta.url).pathname usage
with Node's fileURLToPath(import.meta.url) when deriving the current script file
path (used in the CLI execution check and any variable that builds paths from
import.meta.url). Ensure you import/require the path and url utilities
(path.parse and fileURLToPath) and use the resulting file path when computing
root and any CLI execution checks.
| Understand current performance and identify problems: | ||
| 1. **Measure current state**: | ||
| - **Core Web Vitals**: LCP, FID/INP, CLS scores |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the current Core Web Vitals guidance on FID vs INP, and what thresholds are currently recommended for INP?
💡 Result:
The current Core Web Vitals guidance states that First Input Delay (FID) has been replaced by Interaction to Next Paint (INP) as the responsiveness metric. This change was announced in 2023 and took effect on March 12, 2024, when INP became a stable Core Web Vital and FID was deprecated. FID measured only the delay for the first user interaction, while INP provides a more comprehensive assessment by measuring the full lifecycle (input delay, event handling, and presentation) of all interactions during a page visit, reporting the worst-case at the 75th percentile. As of 2026, no changes to this replacement have occurred; INP remains the official metric. The recommended thresholds for INP, unchanged since its introduction, are: - Good: ≤ 200 ms - Needs Improvement: 200–500 ms - Poor: > 500 ms These apply at the 75th percentile of field data across mobile and desktop devices. Official sources confirm these thresholds remain current in 2026.
Citations:
- 1: https://web.dev/articles/inp
- 2: https://web.dev/defining-core-web-vitals-thresholds/
- 3: https://web.dev/articles/defining-core-web-vitals-thresholds?authuser=0
- 4: https://developers.google.com/search/docs/appearance/core-web-vitals
- 5: https://developers.google.com/search/blog/2023/05/introducing-inp
- 6: https://web.dev/blog/inp-cwv-launch?hl=en
- 7: https://web.dev/blog/inp-cwv-march-12
- 8: https://www.digitalapplied.com/blog/core-web-vitals-2026-inp-lcp-cls-optimization-guide
- 9: https://sitegrade.io/en/blog/core-web-vitals-2026-inp-update
- 10: https://apogeewatcher.com/blog/what-are-core-web-vitals-a-practical-guide-for-2026
🏁 Script executed:
# Check if the file exists and get its structure
fd -t f SKILL.md | head -20Repository: SaplingLearn/Sapling
Length of output: 46
🏁 Script executed:
# Read the relevant lines from the file
cat -n .agents/skills/optimize/SKILL.md | sed -n '10,25p; 200,215p; 230,245p'Repository: SaplingLearn/Sapling
Length of output: 1984
Remove FID from all Core Web Vitals references—it was deprecated in March 2024 and replaced by INP.
FID is no longer an official Core Web Vital. Lines 16 and 237 should list only LCP, INP, CLS instead of LCP, FID/INP, CLS. Line 207 should replace the section header "First Input Delay (FID < 100ms) / INP (< 200ms)" with "Interaction to Next Paint (INP < 200ms)" to avoid outdated guidance.
Also applies to: 207-207, 237-237
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/optimize/SKILL.md at line 16, Update the Core Web Vitals
references to remove the deprecated FID: replace the phrase "Core Web Vitals:
LCP, FID/INP, CLS" with "Core Web Vitals: LCP, INP, CLS" and change the section
header "First Input Delay (FID < 100ms) / INP (< 200ms)" to "Interaction to Next
Paint (INP < 200ms)"; also scan the SKILL.md document for any other occurrences
of "FID" or "FID/INP" and replace them with the current "INP" terminology to
ensure consistency.
| This skill has the highest potential to misfire. Do NOT jump straight into implementation. You MUST: | ||
| 1. **Think through 2-3 different directions** — consider different techniques, levels of ambition, and aesthetic approaches. For each direction, briefly describe what the result would look and feel like. | ||
| 2. **ask the user directly to clarify what you cannot infer.** to present these directions and get the user's pick before writing any code. Explain trade-offs (browser support, performance cost, complexity). |
There was a problem hiding this comment.
Fix grammatical error in instruction.
Line 29 contains a fragment: "ask the user directly to clarify what you cannot infer." This appears to be leftover from copy-pasting the "ask the user directly to clarify" pattern used in other skills. The sentence is incomplete.
📝 Proposed fix
-2. **ask the user directly to clarify what you cannot infer.** to present these directions and get the user's pick before writing any code. Explain trade-offs (browser support, performance cost, complexity).+2. Present these directions and ask the user to pick before writing any code. Explain trade-offs (browser support, performance cost, complexity).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/overdrive/SKILL.md at line 29, Fix the incomplete fragment
"ask the user directly to clarify what you cannot infer." in SKILL.md by
replacing it with a complete sentence that matches the surrounding guidance; for
example: "Ask the user directly to clarify anything you cannot infer, present
these directions and get the user's pick before writing any code, and explain
trade-offs (browser support, performance cost, complexity)." Ensure the
replacement preserves the intent and punctuation and appears where the fragment
"ask the user directly to clarify what you cannot infer." currently exists.
Comment on lines
+31
to
+34
| 1. **Review completeness**: | ||
| - Is it functionally complete? | ||
| - Are there known issues to preserve (mark with TODOs)? | ||
| - What's the quality bar? (MVP vs flagship feature?) |
There was a problem hiding this comment.
Resolve TODO policy contradiction in the polish workflow.
Line 33 allows preserving known issues as TODOs, while Line 192 requires no TODOs. Please clarify whether TODOs are allowed when linked to tracked follow-ups.
Also applies to: 192-192
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/polish/SKILL.md around lines 31 - 34, There’s a contradiction
between the "Review completeness" guidance (the bullet allowing preserving known
issues as TODOs) and the later polish workflow rule that requires no TODOs;
decide the policy (either allow TODOs only when linked to tracked follow-ups or
ban them entirely) and make the language consistent: update the "Review
completeness" bullet and the later polish rule text to state the chosen policy,
add the explicit exception syntax (“TODO allowed only if linked to tracked
follow-up with ticket/ID”), and add a short example and a single
TODO-preservation sentence in SKILL.md’s polish workflow so both references (the
Review completeness section and the no-TODOs rule) match exactly.
Comment on lines
+29
to
+30
| Ask these questions in conversation, adapting based on answers. Don't dump them all at once; have a natural dialogue. ask the user directly to clarify what you cannot infer. | ||
There was a problem hiding this comment.
Capitalize imperative sentences for consistency.
Line 29 and Line 94 start with lowercase “ask”, which reads like a typo in an otherwise polished guide.
✏️ Proposed copy-only fix
-Ask these questions in conversation, adapting based on answers. Don't dump them all at once; have a natural dialogue. ask the user directly to clarify what you cannot infer.+Ask these questions in conversation, adapting based on answers. Don't dump them all at once; have a natural dialogue. Ask the user directly to clarify what you cannot infer.
@@
-ask the user directly to clarify what you cannot infer. Get explicit confirmation of the brief before finishing. If the user disagrees with any part, revisit the relevant discovery questions.+Ask the user directly to clarify what you cannot infer. Get explicit confirmation of the brief before finishing. If the user disagrees with any part, revisit the relevant discovery questions.Also applies to: 94-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/shape/SKILL.md around lines 29 - 30, Capitalize the
imperative sentence starters on the specified lines in SKILL.md: change the
lowercase "ask" at the start of the sentence "ask the user directly to clarify
what you cannot infer." (and the other occurrence at line 94) to "Ask" so the
imperative sentences match the document's capitalization style and read
consistently.
Harsh design audit (multi-agent /critique) turned up 9 deterministic
anti-patterns and ~30 design issues. This pass addresses the P0+P1
findings — the foundation of the brand's visual language:
Typography foundation
- layout.tsx: dropped Fraunces / Geist / Inter; loaded
Playfair Display + Spectral + DM Sans + JetBrains Mono.
"Serif for soul, sans for function" per .impeccable.md.
- globals.css: --font-display is Playfair, new --font-serif is
Spectral, --font-sans is DM Sans. Stale data-type="humanist"
override removed from <html>.
- globals.css: new .body-serif + .h-sans utilities so screens can
pick up Spectral for long-form reading surfaces.
- Deleted the html.dark block and all dark-mode overrides — the
brand is light-mode-only; dark styles squatting here were a
liability.
Atmospheric backdrop (the brand's signature)
- New AtmosphericBackdrop canvas component renders 14 soft orbs
drifting at low opacity (max 0.10) in a warm palette (blues,
lilacs, ambers, teals — green reserved for UI branding).
- Mounted once in (shell)/layout.tsx, sits under all UI chrome.
- Honors prefers-reduced-motion with a single still frame.
Anti-pattern removals
- Deleted AIDisclaimerChip entirely + its usage in ChatPanel.
"AI-Powered pill" is on the brand's explicit no-fly list.
- Side-tab colored left-borders eliminated everywhere the
scanner flagged them:
· MarkdownChat blockquote -> italic body-serif, no border
· Calendar month/week cells -> colored dot prefix
· Calendar day view card -> colored dot, no border
· Sidebar active nav item -> background + weight only
- Hero-metric layouts (big number + small label + colored accent)
replaced with prose strips:
· Dashboard streak -> "You're on a {n}-day streak."
· Admin user counts -> single prose line with inline numbers
· Profile stats -> one-line metric strip (serif numbers)
StatCard helper removed from ProfileView.
- 5 progress bars converted from transition: width to
transform: scaleX to stop layout-thrash warnings
(KnowledgeGraph mastery ring, Dashboard course progress,
Achievements progress, Social MasteryBar, Study flashcard
progress).
Editorial layouts (kill bubble-panel disease)
- Library document grid (280px cards) -> editorial list: dot
prefix, serif title, Spectral summary line-clamped, inline meta.
- Social school directory grid (280px cards) -> roster list.
- Library category pill bar now hides pills for categories with
no documents, preventing the 7-pill chip spam at empty state.
Gradient text + color semantics
- NameColorRenderer no longer applies bg-clip-text gradients
(explicit brand anti-ref). Gradient cosmetics now render as
the first color stop only.
- ChatPanel assistant voice is Spectral (serif); user voice stays
sans. Loading "Thinking…" keeps the sans UI voice.
- Library collapsed 7 random-palette category colors to 3
learning-value semantic tiers: sage (core study material), ink
(structural/syllabus), rust (action/assignment).
Polish
- FeedbackFlow + SessionFeedbackFlow: emoji rating scales
(😞→🤩) replaced with serif numeric buttons and hover labels.
FeedbackFlow headline rewritten ("What's clicking?" vs. "How's
Sapling so far? Takes 10 seconds. No wrong answers.").
- Onboarding learning-style step: dingbats (◈ ✎ ◐ ◎ ◆) replaced
with real Icon components (tree / book / users / flask / sparkle).
- ErrorBoundary: "Something went wrong" replaced with calibrated
copy that parses common error shapes (network, timeout, 401),
Spectral subtext, plus a "Report this" mailto: fallback.
- FloatingActions "Report" button: accent-soft background so it's
visible as an invitation rather than muted chrome.
Deterministic scan: 9 -> 1 anti-patterns. Remaining finding is the
Sidebar collapse's transition: width — a known UX trade-off; a
transform-based rewrite would require restructuring the sidebar
layout and wasn't scoped for this pass.
tsc clean, next build clean (18 routes).There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/app/layout.tsx (1)
20-31: Trim the design-rationale block from the root layout.This is design documentation rather than non-obvious control flow, so it makes a hot file harder to scan. Move the rationale to the audit docs and keep the layout code-only.
As per coding guidelines,
**/*.{py,ts,tsx,js}:No docstrings or comments unless the logic is non-obvious.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/layout.tsx` around lines 20 - 31, Remove the long design-rationale comment block at the top of frontend/src/app/layout.tsx (the multi-line comment describing the type system) and move that content into the audit/design docs; keep the RootLayout (export default function RootLayout) file code-only and delete the comment so only implementation and non-obvious inline notes remain in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/app/globals.css`:
- Around line 14-15: The CSS custom properties --font-display and --font-serif
use the generic family keyword "Georgia" with an uppercase G which violates the
value-keyword-case lint rule; update both declarations (the --font-display and
--font-serif variable values) to use the lowercase generic family keyword
"georgia" so the declarations read with 'Playfair Display', 'Spectral', georgia,
serif.
- Around line 182-185: The keyframe names `fadeIn` and `slideUp` violate the
kebab-case naming rule; rename the keyframes to kebab-case (e.g., `fade-in` and
`slide-up`) and update all references to them in the CSS — specifically change
the `@keyframes` identifiers `fadeIn` -> `fade-in` and `slideUp` -> `slide-up`,
and update the animation usages in the `.fade-in` and `.slide-up` rules to use
the new kebab-case names.
In `@frontend/src/app/layout.tsx`:
- Around line 17-47: Replace the runtime <link> tags in layout.tsx's <head> with
next/font/google usage: import the four fonts (Playfair Display, Spectral, DM
Sans, JetBrains Mono) via next/font/google in the same file (or a fonts module),
configure the requested weights/italics and display/subset options, export the
font variables (e.g., playfair, spectral, dmSans, jetBrainsMono) and then apply
the combined className or individual font.variable values to the root element
(html/body) or component elements instead of the <link> tags so fonts are
self-hosted at build time and eliminate runtime requests and layout shift.
---
Nitpick comments:
In `@frontend/src/app/layout.tsx`:
- Around line 20-31: Remove the long design-rationale comment block at the top
of frontend/src/app/layout.tsx (the multi-line comment describing the type
system) and move that content into the audit/design docs; keep the RootLayout
(export default function RootLayout) file code-only and delete the comment so
only implementation and non-obvious inline notes remain in the file.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a99ab971-cfee-4174-aa6f-4ab93323f88c
📒 Files selected for processing (23)
frontend/src/app/(shell)/layout.tsxfrontend/src/app/globals.cssfrontend/src/app/layout.tsxfrontend/src/components/AIDisclaimerChip.tsxfrontend/src/components/AtmosphericBackdrop.tsxfrontend/src/components/ChatPanel.tsxfrontend/src/components/ErrorBoundary.tsxfrontend/src/components/FeedbackFlow.tsxfrontend/src/components/FloatingActions.tsxfrontend/src/components/KnowledgeGraph.tsxfrontend/src/components/MarkdownChat.tsxfrontend/src/components/NameColorRenderer.tsxfrontend/src/components/ProfileView.tsxfrontend/src/components/SessionFeedbackFlow.tsxfrontend/src/components/Sidebar.tsxfrontend/src/components/screens/Achievements.tsxfrontend/src/components/screens/Admin.tsxfrontend/src/components/screens/Calendar.tsxfrontend/src/components/screens/Dashboard.tsxfrontend/src/components/screens/Library.tsxfrontend/src/components/screens/Onboarding.tsxfrontend/src/components/screens/Social.tsxfrontend/src/components/screens/Study.tsx
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
added 12 commits
April 19, 2026 17:13
The user asked for the sidebar to become a top navbar — more minimal, less space, closer to the pre-revamp Navbar component. New component: TopNav.tsx - 56px sticky bar (pre-revamp was 60; shaved a few px). - Solid background (var(--bg-subtle)) — no backdrop-blur. The brand bans glassmorphism so the frosted-panel approach the pre-revamp used is off the table. - Left: Playfair "Sapling" wordmark + small "Alpha" mono badge, clickable to /dashboard. - Middle: 8 text-only links (Dashboard, Learn, Tree, Study, Library, Calendar, Social, Achievements). Active state is type-weight + text color only — no underline, pill, or side-tab border. - Right: Avatar button opens a dropdown with Settings, Admin (if admin), Sign out. The pre-revamp pattern. - Mobile (≤768px): hamburger on the far left opens a panel of all links anchored below. - Click-outside + Escape close the open menu. Click-through is closed on route change. Removed - components/Sidebar.tsx (dead code now). - lib/sidebar.tsx (SidebarProvider/useSidebar context — only Sidebar consumed it). - SidebarProvider wiring from root layout.tsx. (shell)/layout.tsx switched from flex-row (sidebar + main) to flex-column (TopNav on top, main below). The atmospheric backdrop and skip-link kept their positions. FloatingActions + global feedback flows untouched. tsc clean, next build clean (18 routes).
The revamp leaned on a warm "paper/ink" palette that drifted from the original brand. The user asked to bring back the pre-revamp color scheme and sprout logo. Pulled both straight from main@929658f. globals.css — swapped the semantic tokens to the pre-revamp values: --bg #f0f4f2 (brand mesh, faint green tint) --bg-panel #f8fbf8 --bg-subtle #e9efe9 --bg-soft #dfe8df --bg-topbar #dce6dc (new: distinct nav surface) --accent #1a5c2a (THE Sapling forest green) --accent-hover #144a21 --accent-soft rgba(26, 92, 42, 0.08) --accent-border rgba(26, 92, 42, 0.30) --border rgba(107, 114, 128, 0.18) --text #111827 (cooler, higher-contrast) --text-dim #4b5563 --text-muted #6b7280 Learning-state colors realigned to the pre-revamp brand: --warn amber #e8a33a (in-progress) --err coral #e85d4a (struggling / destructive) --info teal #2b8c96 (informational) The ink-* scale is retained as a gray ladder (for any component still consuming it), but re-keyed so ink-0 is the new brand mesh. public/sapling-icon.svg — restored the pre-revamp sprout SVG (stem + two leaves + tip circle) as a standalone asset, no deps. TopNav.tsx: - background switched from --bg-subtle to --bg-topbar so the nav reads as its own surface without needing a shadow. - Added <img src="/sapling-icon.svg" /> next to the wordmark. - Wordmark layout: icon + Playfair "Sapling" stacked over mono "Closed Alpha" (matches pre-revamp Navbar). tsc clean, next build clean, /sapling-icon.svg serves 200 from dev.
User asked to go back to the color scheme that was in place before commit 0166893. Restoring the warm neutrals: --bg #faf8f3 (warm cream) --bg-panel #ffffff --accent #3a6a2c (sap-600, muted forest green) --text #1a1814 (warm dark) --warn #b4562c (rust amber) --err #a83a3a --info #3e6f8a Kept from the previous commit: - The sprout logo at /sapling-icon.svg - The TopNav still renders it next to the wordmark So only the CSS tokens revert; the logo stays.
The user confirmed the target lockup: the pre-revamp sapling-word-icon.png (icon + "Sapling" serif baked together) with "CLOSED ALPHA" as a mono tagline underneath — exactly what shipped before. - Restored public/sapling-word-icon.png from main@929658f. - TopNav now renders that PNG in the top-left instead of hand-composing icon-SVG + Playfair-text separately. Single asset, guaranteed to match. - Desktop: PNG at 34px tall (natural 543×147 → 125×34) with the "Closed Alpha" mono tag left-aligned under the word. - Mobile: PNG at 28px tall, tagline hidden to save width. public/sapling-icon.svg is still there for favicons / any smaller-scale use; TopNav just doesn't reference it any more.
The user explicitly asked for the pre-revamp Navbar logo code to be dropped into the new TopNav as-is. Lifted lines 213–231 of main@929658f:frontend/src/components/Navbar.tsx: - <img src="/sapling-icon.svg"> at 32x32 with the pre-revamp's negative margins for a tight fit against the wordmark. - Wordmark in Spectral (inline fontFamily fallback chain), 700 weight, 20px desktop / 17px mobile, color #1a5c2a with a subtle 12px green text-shadow glow. - "Closed Alpha" tag at 9px, weight 600, letter-spacing 0.08em, uppercase, opacity 0.7. Dropped the sapling-word-icon.png approach; the icon.svg + inline styled text is what the pre-revamp shipped and it's what the user asked for.
Ported the icon set from the pre-revamp Sidebar (components/Icon.tsx names): Dashboard -> home Learn -> brain Tree -> tree Study -> bolt Library -> book Calendar -> cal Social -> users Achievements -> trophy Also added icons to the account-dropdown items: Settings -> cog Admin -> shield Desktop links: icon at 14px + label, small gap. Mobile panel: icon at 15px + label with more breathing room. Dropdown MenuItem takes an icon prop now.
Per user request, restore the pre-revamp placement: the daily quote
sits right under the greeting at the top of the page (as the TopBar
subtitle, italic), not in a labeled card lower down.
- subtitle now renders `"{quote}"` in italic instead of the previous
"Your knowledge is growing…" line.
- The dedicated card in leftPanel with the "Inspired learning" label
is removed.
While the greeting is animating in via Typewriter, the quote stays
as the stable subtitle text — same visual rhythm the pre-revamp had.Per user request, the bottom-of-sidebar "Recent activity" card is gone. Also removed the recentByNode useMemo that only fed that panel. The Tree and Learn screens still surface recent/ongoing work contextually (session list, last-studied in node detail), so nothing else breaks.
Graph card was minHeight 520 / inner 460. Reduced to minHeight 420 / inner 340 — closer to what the pre-revamp Dashboard rendered before it was over-sized during the M4 rebuild. The fullscreen button still works for users who want the full-window view.
Following the knowledge-graph shrink, the useState initial size was
still { w: 720, h: 440 } while the container's CSS height dropped to
340. That paints the graph at 440 on first render, ResizeObserver
fires a tick later, but in some HMR / slow-hydration cases the taller
version was still visible.
Synced initial h to 340 so there's no one-frame flash at the old
height. ResizeObserver still drives the actual runtime size.Grid template was minmax(0, 2.3fr) : minmax(300px, 1fr) which put the right column at roughly 30% of the row. User wants the stats/upcoming/ learn-next rail narrower. Switched to minmax(0, 4fr) : minmax(260px, 1fr) — right column renders at ~20% on wide desktops and degrades gracefully to 260px on smaller screens so the copy remains readable.
Reordered the left column so "My courses" is the first card (the primary jump-off action on the dashboard) and the knowledge graph sits under it as atmospheric context. Graph dimensions reduced again: minHeight 420 -> 340 (-80) inner height 340 -> 260 (-80) initial size state h synced to 260 Cumulative since before the shrink work: 520 -> 340 / 460 -> 260. The graph is now ~56% of its original footprint; fullscreen button is still there for users who want the big canvas.
- Downgrade framer-motion to ^11 (fixes Turbopack can't resolve motion-utils)
- Fix /auth/page.tsx to render Auth component instead of redirect("/")
- Fix CORS: client-side fetches use relative URLs through Next.js proxy
- Fix session route: fall back to backend verify when SESSION_SECRET missing
- Fix callback: await session before profile fetch, surface errors to /auth
- Add BACKEND_URL and NEXT_PUBLIC_API_URL to wrangler.toml for Cloudflare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>- Added backend/routes/newsletter.py with POST /api/newsletter/subscribe - Registered newsletter router in main.py at /api/newsletter - Added beta signup button below hero CTAs with glow animation - Added two-panel newsletter/beta modal with animated open/close - Added modal-backdrop and beta-glow CSS animations to globals.css - Uses relative URL (/api/newsletter/subscribe) to avoid CORS issues Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Conflict resolutions: - Navbar.tsx: kept deletion (replaced by Sidebar in (shell) layout) - page.tsx, globals.css, migration_newsletter.sql: kept audit-fixes - package-lock.json: regenerated against merged package.json CodeRabbit fixes: - globals.css: Georgia → georgia (value-keyword-case) - globals.css: fadeIn/slideUp keyframes → fade-in/slide-up (kebab-case) - globals.css: relabeled .landing-page block as pre-auth/sign-in theme, flagged for follow-up consolidation
Deploying with |
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs | frontend | aacac69 | May 01 2026, 03:11 AM |
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full frontend revamp from the audit/gap plan in
docs/frontend-audit/, broken into 5 logical commits, plus a Cloudflare-deploy hardening pass. Main was reset to the pre-revamp commit so the full scope is reviewable in a single PR.Scope
5 commits, 100 files, +21,480 / −28,882
0496327backend: new endpoints + testsbefore/limit/has_more, +34 new test cases399eae3frontend: revamp shell, screens, API client(shell)layout with Sidebar + FloatingActions + global flows. 12 new screen components. Removed legacy/signin,/privacy,/terms,/about,/careers,/flashcards, old Dockerfile, Jest harness, and old__tests__. New public profile page at/profile/[userId].1e1d69bdocs: audit + rebuild plandocs/frontend-audit/with per-feature specs, route inventory, component map, state model, API surface, rebuild checklist.CLAUDE.mdupdated to reflect that the Jest harness was removed.6f753ecaudit fixesbeforeparam on room messages validated as ISO 8601 (blocks PostgREST operator injection); local-mode handlers for/api/learn/action,/api/learn/mode-switch,/api/learn/sessions/{id}/resume.ebff15bproduction hardeningSecureflag in production; replaced?? 'http://localhost:5000'fallbacks with empty string so Next.js rewrite handles prod; UserContext + Settings no longer buildundefined/api/...when env var is unset.Milestones covered
/auth,/auth/callback, multi-step/onboarding.6f753ec).Courses / Stats & Moretabs.before/limit/has_more, school directory, study match, comparison view (mastery bars) when?suggest=<userId>./study. Guide: course → exam picker, recent sidebar, per-topic cards, regenerate. Flashcards: course filter, topic pills, 3D flip, Space/1/2/3 keyboard, "Generated using N library docs" chip./profile/[userId]. Settings: grouped nav,CustomSelect, debounced username availability, avatar upload with 5 MB client guard, preview modal, cosmetics manager with Owned/Catalog toggle. Admin: Users/Roles/Achievements/Cosmetics/Analytics tabs; inline RoleBadge assign/revoke; cosmetic asset upload to thecosmetic-assetsSupabase bucket (SQL snippet inbackend/db/migration_cosmetics.sql). Achievements: progress bars on locked cards, editable showcase (up to 5, drag reorder), unlock toast via localStorage delta + focus refetch, secrets stay hidden.useBodyScrollLockhook threaded through every full-viewport overlay,role="log"+aria-liveonChatPanel,role="radiogroup"onQuizPanel, skip-to-content link,prefers-reduced-motionCSS + one-shot d3 settle on the KnowledgeGraph.Deferred items shipped
/api/calendar/assignments/{id}+ Calendar UI save)comparisonprop + Social Overview pairing cardEnv vars required in Cloudflare dashboard
SESSION_SECRETBACKEND_URL/api/:path*rewriteNEXT_PUBLIC_API_URLBACKEND_URLis set so calls route through Next rewrite (no CORS)NEXT_PUBLIC_SUPABASE_URLNEXT_PUBLIC_SUPABASE_ANON_KEYNEXT_PUBLIC_LOCAL_MODEfalsein prodKnown design-level concerns (not fixed in this PR)
user_idin path/body withoutrequire_self. This is a pervasive alpha-mode pattern; a separate auth-hardening pass is warranted./api/usersis unauthenticated (called byUserContexton boot to populate the user switcher).services/auth_guard.pyhas a dev-mode fallback that acceptsuser_idfrom query params.Test plan
npx tsc --noEmitcleannpm run buildclean (18 routes compiled)pytestclean (293 pass, 3 skip, 0 fail)/auth→ Google →/auth/callback→ Dashboard, verifysapling_sessioncookie persists🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Infrastructure & Deployment
Chores
Documentation