feat: implement StudentSync v2 MVP domains - #9
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughAdded new academic, projects, and profile sub-stores to user profiles, extended domain config and plugin loading for those domains, and introduced tool plugins for study sessions, project tracking, life mode/interests, and activity summaries. ChangesStudentSync v2 — Operating System for Students
Sequence Diagram(s)sequenceDiagram
participant DomainConfig
participant getLifeDomainPlugins
participant setLifeModePlugin
participant logStudySessionPlugin
participant createProjectPlugin
participant ensureDomains
participant persist
DomainConfig->>getLifeDomainPlugins: enable academic, projects, profile
getLifeDomainPlugins->>setLifeModePlugin: return profile tools
getLifeDomainPlugins->>logStudySessionPlugin: return academic tools
getLifeDomainPlugins->>createProjectPlugin: return project tools
setLifeModePlugin->>ensureDomains: initialize profile store
logStudySessionPlugin->>ensureDomains: initialize academic store
createProjectPlugin->>ensureDomains: initialize projects store
setLifeModePlugin->>persist: persist(context.store)
logStudySessionPlugin->>persist: persist(context.store)
createProjectPlugin->>persist: persist(context.store)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.0)src/core/plugins/domains/profile.tsFile contains syntax errors that prevent linting: Line 8: expected 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 |
PR Summary by QodoAdd StudentSync v2 MVP domains (academic, projects, profile) Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1. Missing getOptionalArray export
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| import type { ToolPlugin } from '../ToolRegistry.js'; | ||
| import type { StudySession, KnowledgeNode } from '../../store.js'; | ||
| import { getString, getOptionalString, getOptionalNumber, getOptionalArray } from '../builtin/_args.js'; | ||
| import { ensureDomains, generateId, persist, todayISO } from './_helpers.js'; |
There was a problem hiding this comment.
1. Missing getoptionalarray export 🐞 Bug ≡ Correctness
The new academic/profile/projects domain plugins import getOptionalArray from src/core/plugins/builtin/_args.js, but that module does not export it, so the process will fail during build or at runtime with an ESM named-export error when loading these plugins.
Agent Prompt
## Issue description
Several new domain plugins import `getOptionalArray` from `src/core/plugins/builtin/_args.js`, but `_args.ts` does not export such a symbol. This causes a hard failure (TypeScript compile error and/or Node ESM `does not provide an export named` error) when these modules are imported.
## Issue Context
`_args.ts` currently provides `getOptionalStringArray()` which matches the usage here (arrays of strings).
## Fix Focus Areas
- src/core/plugins/domains/academic.ts[1-70]
- src/core/plugins/domains/profile.ts[1-42]
- src/core/plugins/domains/projects.ts[1-62]
- src/core/plugins/builtin/_args.ts[1-37]
## Suggested fix
- Replace `getOptionalArray` imports/usages with `getOptionalStringArray`:
- `subtopics`, `add`, `remove`, and `links` should be parsed via `getOptionalStringArray(args, key) ?? []` (or left `undefined` when appropriate).
- Alternatively (if you truly need a generic array parser), implement and export `getOptionalArray` from `_args.ts` and update callers to validate element types (since current callers assume `string[]`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/core/plugins/domains/index.ts (1)
3-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport the shared
DomainConfigtype instead of mirroring it here.This loader still appears to carry its own
DomainConfigdeclaration alongsidesrc/core/schema.ts. Adding three more domain keys means the config contract now has to stay in sync in two places. Import the shared type fromschema.tsso plugin loading cannot silently drift from the config schema.Also applies to: 74-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/plugins/domains/index.ts` around lines 3 - 20, The domain plugin loader is duplicating the DomainConfig contract instead of using the shared type from schema.ts, which can drift as new domain keys are added. Update the domain loader in domains/index.ts to import and use the shared DomainConfig type from src/core/schema.ts, and remove any locally mirrored declaration so the plugin loading contract stays aligned with the schema.src/core/store.ts (1)
174-228: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftConsolidate these persisted-domain types into one source of truth.
StudySession,ProjectEntry, andActivitySessionare also declared insrc/core/plugins/domains/academic.ts,src/core/plugins/domains/projects.ts, andsrc/core/plugins/domains/profile.ts. Keeping separate copies for the same stored shapes makes the persistence contract easy to drift. Move them to a shared types module or import them from here instead of redefining them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/store.ts` around lines 174 - 228, Consolidate the persisted domain shapes into a single source of truth by removing the duplicate definitions of StudySession, ProjectEntry, and ActivitySession from the domain plugin files and importing them from the shared store types instead. Update the affected domain modules and any related store interfaces in src/core/store.ts so AcademicStore, ProjectsStore, and ProfileStore reference the shared types directly, keeping the persisted contract defined in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/plugins/domains/_helpers.ts`:
- Around line 30-32: The profile initialization in ensureDomains only fills
interests and activitySessions when profile.profile already exists, but it does
not backfill a missing mode on partial profiles. Update the ensureDomains helper
in _helpers.ts to also default profile.profile.mode to the same exploration
value whenever it is absent, so this behavior matches normalizeUserProfile and
generate_morning_brief no longer sees undefined.
In `@src/core/plugins/domains/academic.ts`:
- Around line 28-36: The academic plugin `level` field is being used as both a
0-100 mastery value and a spaced-repetition interval bucket, causing
inconsistent scheduling. Update `log_study_session`, `update_knowledge_graph`,
and `get_spaced_repetition_schedule` to use a single consistent meaning, or
split the data into separate fields such as `mastery` and `reviewInterval`. Make
sure the writer in `update_knowledge_graph` and the session tracking in
`log_study_session` normalize to the same scale that
`get_spaced_repetition_schedule` reads.
In `@src/core/plugins/domains/profile.ts`:
- Line 8: The profile domain descriptions in the plugin module contain malformed
string literals because the apostrophe is written with an extra backslash,
causing the parser to see an unterminated string. Update the affected
description fields in the profile domain definition so the apostrophes are
escaped correctly or the strings use double quotes, and apply the same fix
consistently in all affected entries within the profile plugin.
In `@src/core/store.ts`:
- Around line 334-349: Deep-normalize the returned sub-stores in store.ts
instead of only checking the top-level containers. Update the AcademicStore,
ProjectsStore, and ProfileStore construction so nested records like
knowledgeGraph entries, project items, interests, and activity sessions are
individually sanitized into the shapes expected by academic.ts and projects.ts,
avoiding raw null/invalid nested fields from being passed through. Use the
existing store-building logic around AcademicStore, ProjectsStore, and
ProfileStore to locate and apply the normalization before returning these
objects.
---
Nitpick comments:
In `@src/core/plugins/domains/index.ts`:
- Around line 3-20: The domain plugin loader is duplicating the DomainConfig
contract instead of using the shared type from schema.ts, which can drift as new
domain keys are added. Update the domain loader in domains/index.ts to import
and use the shared DomainConfig type from src/core/schema.ts, and remove any
locally mirrored declaration so the plugin loading contract stays aligned with
the schema.
In `@src/core/store.ts`:
- Around line 174-228: Consolidate the persisted domain shapes into a single
source of truth by removing the duplicate definitions of StudySession,
ProjectEntry, and ActivitySession from the domain plugin files and importing
them from the shared store types instead. Update the affected domain modules and
any related store interfaces in src/core/store.ts so AcademicStore,
ProjectsStore, and ProfileStore reference the shared types directly, keeping the
persisted contract defined in one place.
🪄 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
Run ID: b9e47daf-c330-48a7-a056-14aa26f449f0
📒 Files selected for processing (8)
DESIGN_CHOICES.mdsrc/core/plugins/domains/_helpers.tssrc/core/plugins/domains/academic.tssrc/core/plugins/domains/index.tssrc/core/plugins/domains/profile.tssrc/core/plugins/domains/projects.tssrc/core/schema.tssrc/core/store.ts
| if (!profile.profile) profile.profile = { mode: 'Exploration Mode', interests: [], activitySessions: [] }; | ||
| if (!profile.profile.interests) profile.profile.interests = []; | ||
| if (!profile.profile.activitySessions) profile.profile.activitySessions = []; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Backfill mode when the profile store already exists.
ensureDomains only assigns the default mode when profile.profile is missing entirely. If an older or partial profile already has { interests, activitySessions } but no mode, generate_morning_brief still returns undefined after this helper runs. Default profile.profile.mode here as well so this path matches normalizeUserProfile.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/plugins/domains/_helpers.ts` around lines 30 - 32, The profile
initialization in ensureDomains only fills interests and activitySessions when
profile.profile already exists, but it does not backfill a missing mode on
partial profiles. Update the ensureDomains helper in _helpers.ts to also default
profile.profile.mode to the same exploration value whenever it is absent, so
this behavior matches normalizeUserProfile and generate_morning_brief no longer
sees undefined.
| if (!kg[session.topic]) { | ||
| kg[session.topic] = { level: 1, lastReviewed: todayISO(), subtopics: [] }; | ||
| } else { | ||
| kg[session.topic].level += 0.5; // Arbitrary increment for tracking mastery | ||
| kg[session.topic].lastReviewed = todayISO(); | ||
| } | ||
|
|
||
| persist(context.store); | ||
| return { success: true, session, currentMasteryLevel: kg[session.topic].level }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Inconsistent level semantics across academic plugins.
The level field carries three conflicting meanings:
log_study_sessionseeds new topics atlevel: 1and increments by0.5per session (an open-ended "study count" scale).update_knowledge_graphoverwriteslevelwithmasteryLeveldocumented as0-100.get_spaced_repetition_scheduleinterpretslevelvia thresholds (>10 → 30d,>5 → 14d, …).
So a user setting masteryLevel: 100 is treated as "review in 30 days", while study-session increments take ~20 sessions to reach the same bucket. The mastery (0-100) scale and the interval-threshold scale are incompatible and will produce surprising review schedules. Consider separating mastery (0-100) from the spaced-repetition level/interval, or normalizing both writers to one scale.
Also applies to: 63-66, 90-94
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/plugins/domains/academic.ts` around lines 28 - 36, The academic
plugin `level` field is being used as both a 0-100 mastery value and a
spaced-repetition interval bucket, causing inconsistent scheduling. Update
`log_study_session`, `update_knowledge_graph`, and
`get_spaced_repetition_schedule` to use a single consistent meaning, or split
the data into separate fields such as `mastery` and `reviewInterval`. Make sure
the writer in `update_knowledge_graph` and the session tracking in
`log_study_session` normalize to the same scale that
`get_spaced_repetition_schedule` reads.
|
|
||
| export const setLifeModePlugin: ToolPlugin = { | ||
| name: 'set_life_mode', | ||
| description: 'Set the user\\'s current life mode (e.g., Exploration Mode, Exam Mode, Placement Mode).', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Syntax error: escaped single quote terminates the string literal.
These descriptions use \\' (escaped backslash followed by an unescaped quote) inside single-quoted strings, so the string closes prematurely and the file fails to parse — Biome reports "unterminated string literal" on lines 8, 24, and 89. This breaks the entire module.
Use a single backslash escape (\') or switch to double quotes.
🐛 Proposed fix (line 8 shown; apply same to 24 and 89)
- description: 'Set the user\\'s current life mode (e.g., Exploration Mode, Exam Mode, Placement Mode).',
+ description: "Set the user's current life mode (e.g., Exploration Mode, Exam Mode, Placement Mode).",Also applies to: 24-24, 89-89
🧰 Tools
🪛 Biome (2.5.0)
[error] 8-8: expected , but instead found s
(parse)
[error] 8-8: expected , but instead found current
(parse)
[error] 8-8: expected , but instead found life
(parse)
[error] 8-8: expected , but instead found mode
(parse)
[error] 8-8: expected , but instead found .
(parse)
[error] 8-8: expected , but instead found g
(parse)
[error] 8-8: expected , but instead found .
(parse)
[error] 8-8: expected , but instead found Mode
(parse)
[error] 8-8: expected , but instead found Mode
(parse)
[error] 8-8: expected , but instead found Mode
(parse)
[error] 8-8: Expected a function body but instead found '.'.
(parse)
[error] 8-8: unterminated string literal
(parse)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/plugins/domains/profile.ts` at line 8, The profile domain
descriptions in the plugin module contain malformed string literals because the
apostrophe is written with an extra backslash, causing the parser to see an
unterminated string. Update the affected description fields in the profile
domain definition so the apostrophes are escaped correctly or the strings use
double quotes, and apply the same fix consistently in all affected entries
within the profile plugin.
Source: Linters/SAST tools
|
|
||
| const academic: AcademicStore = { | ||
| studySessions: Array.isArray(user.academic?.studySessions) ? user.academic!.studySessions : [], | ||
| knowledgeGraph: user.academic?.knowledgeGraph && typeof user.academic.knowledgeGraph === 'object' ? user.academic.knowledgeGraph : {}, | ||
| }; | ||
|
|
||
| const projects: ProjectsStore = { | ||
| projects: Array.isArray(user.projects?.projects) ? user.projects!.projects : [], | ||
| }; | ||
|
|
||
| const profileStore: ProfileStore = { | ||
| mode: user.profile?.mode || 'Exploration Mode', | ||
| interests: Array.isArray(user.profile?.interests) ? user.profile!.interests : [], | ||
| activitySessions: Array.isArray(user.profile?.activitySessions) ? user.profile!.activitySessions : [], | ||
| }; | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Deep-normalize the new sub-stores before returning them.
These branches only validate the top-level container. Downstream tools assume nested entries are already well-formed — for example, academic.ts dereferences node.lastReviewed/node.subtopics, and projects.ts does project.notes.push(...) plus ...project.links. A persisted payload like { knowledgeGraph: { os: null } } or { projects: [{ notes: null, links: null }] } will survive normalization here and then crash the new plugins at runtime. Normalize each nested record here rather than passing raw objects through.
Also applies to: 367-370
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/store.ts` around lines 334 - 349, Deep-normalize the returned
sub-stores in store.ts instead of only checking the top-level containers. Update
the AcademicStore, ProjectsStore, and ProfileStore construction so nested
records like knowledgeGraph entries, project items, interests, and activity
sessions are individually sanitized into the shapes expected by academic.ts and
projects.ts, avoiding raw null/invalid nested fields from being passed through.
Use the existing store-building logic around AcademicStore, ProjectsStore, and
ProfileStore to locate and apply the normalization before returning these
objects.
Implemented the 12 MVP features for "StudentSync v2" as an operating system for students.
StoredUserProfileto include structured tracking foracademic,projects, andprofiledomains.src/core/plugins/domains/:profile.ts: Life modes, Activity sessions API, Morning/Evening reflection briefs.academic.ts: Study sessions, Knowledge graph tracker, Spaced repetition scheduler.projects.ts: Project vault and note tracker.schema.ts.PR created automatically by Jules for task 5161031251550577040 started by @Ayush-Vish
Summary by CodeRabbit
New Features
Bug Fixes