learning: Add spaced repetition and active recall via conversation #204
Conversation
…torage API and sync with local state, replacing `learning-state.json`. if storage.json differs from localStorage, storage.json should win 100% of the time, effectively wiping the browser's local cache to match the file
Summary of ChangesHello @vineet-reddy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the learning platform by integrating a conversational spaced repetition system. This system leverages the FSRS algorithm to intelligently schedule concept reviews, moving away from manual flashcards towards natural, AI-driven dialogues. The changes include new API endpoints for review conversations and data persistence, a dedicated UI component for review sessions, and modifications to the existing Socratic dialogue to incorporate past learning memories and proactively address concepts due for review. This aims to improve long-term retention and provide a more engaging and personalized learning experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an impressive pull request that introduces a major new feature: a conversational spaced repetition system using the FSRS algorithm. The implementation is comprehensive, covering new API endpoints, UI components, data persistence logic, and detailed documentation. The code is generally well-structured and demonstrates a deep understanding of the learning principles being implemented. My review focuses on improving data persistence robustness, resolving a critical inconsistency in the FSRS logic, and enhancing type safety and server-side practices. Overall, this is a fantastic addition to the project.
| export async function POST(req: NextRequest) { | ||
| try { | ||
| const changes = await req.json(); | ||
|
|
||
| ensureDirectory(); | ||
|
|
||
| // Read existing data | ||
| let currentData = {}; | ||
| if (fs.existsSync(STORAGE_FILE)) { | ||
| try { | ||
| const content = fs.readFileSync(STORAGE_FILE, 'utf-8'); | ||
| currentData = JSON.parse(content); | ||
| } catch (e) { | ||
| // Ignore parse errors, start fresh | ||
| } | ||
| } | ||
|
|
||
| // Merge changes | ||
| const newData = { ...currentData, ...changes }; | ||
|
|
||
| // Write back | ||
| fs.writeFileSync(STORAGE_FILE, JSON.stringify(newData, null, 2)); | ||
|
|
||
| return NextResponse.json({ success: true }); | ||
| } catch (error) { | ||
| console.error('Failed to write storage:', error); | ||
| return NextResponse.json({ error: 'Failed to write storage' }, { status: 500 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation of reading the entire file, modifying it in memory, and writing it back is susceptible to race conditions. If two requests arrive concurrently, the last one to write will overwrite the changes from the other, leading to data loss. For a production environment, this should be addressed with a file-locking mechanism or by migrating to a more robust storage solution like SQLite that handles concurrency.
| } catch (e) { | ||
| // Ignore parse errors, start fresh | ||
| } |
There was a problem hiding this comment.
Silently ignoring JSON parsing errors when reading the storage file can lead to unintentional data loss. If the file becomes corrupted, the application will treat it as empty and overwrite it with new data, losing all previous state. It would be safer to log the corruption error and return a 500 status to prevent data loss and make the issue visible.
console.error(`Failed to parse storage file: ${e}`);
throw new Error('Storage file is corrupted');| export function updateFSRSParameters( | ||
| states: Map<string, ConceptState>, | ||
| conceptId: string, | ||
| understanding: number // 0-1, where 1 is perfect recall | ||
| ): Map<string, ConceptState> { | ||
| const state = getConceptState(states, conceptId); | ||
|
|
||
| // FSRS-like parameter update | ||
| // Higher understanding increases stability, lowers difficulty | ||
| // Lower understanding decreases stability, increases difficulty | ||
|
|
||
| // Stability update (simplified FSRS) | ||
| // On good recall (>0.7): multiply stability by 1.5-2.5 based on understanding | ||
| // On poor recall (<0.3): reset to near-initial values | ||
| // In between: small adjustments | ||
| let newStability: number; | ||
| let newDifficulty: number; | ||
|
|
||
| if (understanding >= 0.7) { | ||
| // Good recall - increase stability | ||
| const multiplier = 1.5 + understanding; // 2.2-2.5x for good recall | ||
| newStability = Math.min(state.stability * multiplier, 365); // Cap at 1 year | ||
| newDifficulty = state.difficulty * 0.9; // Concept getting easier | ||
| } else if (understanding <= 0.3) { | ||
| // Poor recall - reset stability | ||
| newStability = 1; // Back to 1 day | ||
| newDifficulty = Math.min(state.difficulty + 0.1, 1); // Concept is harder than thought | ||
| } else { | ||
| // Medium recall - small adjustments | ||
| newStability = state.stability * (0.8 + understanding * 0.4); // 0.92-1.08x | ||
| newDifficulty = state.difficulty; // No change | ||
| } | ||
|
|
||
| const updatedState: ConceptState = { | ||
| ...state, | ||
| stability: newStability, | ||
| difficulty: Math.max(0.1, Math.min(1, newDifficulty)), // Clamp 0.1-1 | ||
| lastReview: Date.now(), | ||
| }; | ||
|
|
||
| const newStates = new Map(states); | ||
| newStates.set(conceptId, updatedState); | ||
| return newStates; | ||
| } |
There was a problem hiding this comment.
There is a critical logic duplication and inconsistency between updateFSRSParameters in this file and calculateNextInterval in lib/spaced-repetition.ts. This function is the one currently being used, but the function in spaced-repetition.ts is what's referenced in the documentation. This discrepancy can lead to confusion and bugs.
The FSRS update logic should be consolidated into a single function within lib/spaced-repetition.ts that returns both the new stability and difficulty. This file should then import and use that single source of truth.
| console.log('\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━'); | ||
| console.log('🔄 NEW REVIEW DIALOGUE REQUEST'); | ||
| console.log('━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━'); | ||
| console.log('📌 Library ID:', libraryId); | ||
| console.log('📌 Conversation turns:', conversationHistory.length); | ||
| console.log('📌 Due concepts:', dueConcepts?.length || 0); | ||
|
|
||
| if (dueConcepts?.length > 0) { | ||
| console.log('📌 Concepts to review:'); | ||
| dueConcepts.forEach((c: DueConceptForReview, i: number) => { | ||
| console.log(` ${i + 1}. ${c.conceptName} (${Math.round(c.retrievability * 100)}% retained, ${c.memories.length} memories)`); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The file contains numerous console.log statements for debugging. While useful during development, they can add significant noise to production logs and may expose sensitive information. It's better to use a dedicated logging library or make logging conditional based on the environment (process.env.NODE_ENV).
| if (memories && memories.length > 0) { | ||
| memoryContext = ` | ||
| **YOUR NOTES FROM PREVIOUS CONVERSATIONS WITH THIS STUDENT:** | ||
| ${memories.map(m => `- [${new Date(m.timestamp).toLocaleDateString()}] ${m.content} (understanding: ${Math.round(m.understanding * 100)}%)`).join('\n')} |
There was a problem hiding this comment.
Using new Date(m.timestamp).toLocaleDateString() on the server can lead to inconsistent date formatting, as it depends on the server's locale. This might present dates incorrectly to users in different timezones. It's more robust to send the timestamp or an ISO 8601 string to the client and let the user's browser handle the formatting according to their locale.
`${new Date(m.timestamp).toISOString().split('T')[0]}] ${m.content} (understanding: ${Math.round(m.understanding * 100)}%)`| <ReactMarkdown | ||
| remarkPlugins={[remarkGfm]} | ||
| components={{ | ||
| code({ node, inline, className, children, ...props }: any) { |
There was a problem hiding this comment.
The props for the code component in ReactMarkdown are typed as any. To improve type safety and maintainability, you should use the specific types provided by React and the syntax highlighter library. This avoids potential runtime errors and improves developer experience.
| code({ node, inline, className, children, ...props }: any) { | |
| code({ node, inline, className, children, ...props }: { node: any; inline?: boolean; className?: string; children: React.ReactNode }) { |
1 participant
Implements conversational spaced repetition system for the learning module using FSRS algorithm.
See implementation details