Implement problem data for JavaScript levels #4
Conversation
Add problem data structure with 8 problems across 3 difficulty levels: Level 1 (3 problems): - Variable redeclaration error - Infinite loop detection - Date.getMonth() misuse Level 2 (3 problems): - Missing class method - Set/Map incorrect usage - String vs number type confusion Level 3 (2 problems): - Object.entries with dot notation issue - Static vs instance method confusion with Array.find Also implemented getProblems() function to dynamically load problems using Vite's import.meta.glob for efficient problem management.
WalkthroughA problem-loading system is implemented via Vite glob imports with caching and filtering utilities in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
app/problems/index.ts (2)
41-50: Add validation for loaded problem data.The initialization function assumes all JSON files contain valid Problem objects. Consider adding basic validation or error handling to catch malformed data at load time rather than runtime.
function initializeProblemsCache() { if (cacheInitialized) return; for (const path in problemModules) { - const problem = problemModules[path].default; - problemsCache.push(problem); + try { + const problem = problemModules[path].default; + // Basic validation + if (!problem.id || !problem.codeLanguage || !problem.level || !Array.isArray(problem.issues)) { + console.warn(`Invalid problem structure in ${path}:`, problem); + continue; + } + problemsCache.push(problem); + } catch (error) { + console.error(`Failed to load problem from ${path}:`, error); + } } cacheInitialized = true; }
58-66: Consider validating the level parameter.The function accepts any number for
level, but the problem set defines three levels (1, 2, 3). Invalid level values will silently return empty arrays, which might mask caller errors.Add bounds checking or document the valid range:
export function getProblems(lang: CodeLanguageOrAll, level: number): Problem[] { initializeProblemsCache(); + + // Validate level is within expected bounds + if (level < 1 || level > 3) { + console.warn(`Invalid level ${level}, expected 1-3`); + return []; + } return problemsCache.filter((problem) => { const langMatch = lang === 'all' || problem.codeLanguage === lang; const levelMatch = problem.level === level; return langMatch && levelMatch; }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/problems/index.ts(1 hunks)app/problems/javascript/level1/js-l1-001.json(1 hunks)app/problems/javascript/level1/js-l1-002.json(1 hunks)app/problems/javascript/level1/js-l1-003.json(1 hunks)app/problems/javascript/level2/js-l2-001.json(1 hunks)app/problems/javascript/level2/js-l2-002.json(1 hunks)app/problems/javascript/level2/js-l2-003.json(1 hunks)app/problems/javascript/level3/js-l3-001.json(1 hunks)app/problems/javascript/level3/js-l3-002.json(1 hunks)
🔇 Additional comments (8)
app/problems/javascript/level1/js-l1-001.json (1)
1-26: LGTM!The problem definition is well-structured with accurate line references and clear bilingual descriptions. The issue correctly identifies the variable redeclaration bug.
app/problems/javascript/level1/js-l1-002.json (1)
1-27: LGTM!The problem definition accurately captures the infinite loop bug. The line references span the entire while loop, which is appropriate for this issue context.
app/problems/javascript/level1/js-l1-003.json (1)
1-25: LGTM!The problem clearly documents the getMonth() offset bug. Line references appropriately span both the buggy assignment and its usage, making the issue teaching-effective.
app/problems/javascript/level2/js-l2-002.json (1)
1-37: LGTM!Both issues are well-defined with accurate line references. The critical bug (Set indexing) and design concern (misleading function name) are both appropriately documented.
app/problems/javascript/level2/js-l2-003.json (1)
1-41: LGTM!Both issues are accurately referenced and well-documented. The type coercion problem and validation concern are appropriately categorized by severity.
app/problems/index.ts (3)
72-75: Good use of shallow copy to protect cache.The spread operator prevents external mutation of the cache array. Note that the Problem objects themselves remain mutable. If strict immutability is required, consider documenting that callers should not modify returned objects, or investigate using a type-level readonly constraint.
83-95: LGTM!The score calculation logic correctly implements the combo multiplier system with clear thresholds and proper integer rounding.
28-32: Hardcoded glob pattern is correct and no changes needed.Verification confirms the glob pattern
./javascript/level*/*.jsonis intentionally JavaScript-only, which matches the actual directory structure. No other language directories exist. The code is well-designed: theProblemtype already includes acodeLanguagefield typed asCodeLanguage, and thegetProblems()function properly filters by language. This creates a forward-compatible architecture where adding other languages (php, ruby, java, dart) only requires creating corresponding directories—no code changes would be needed. The current implementation is correct as-is.
| "issues": [ | ||
| { | ||
| "id": "js-l2-001-1", | ||
| "lines": [17], | ||
| "type": "bug", | ||
| "severity": "critical", | ||
| "score": 4, | ||
| "description": { | ||
| "ja": "subtractメソッドが定義されていないため、実行時にTypeErrorが発生します", | ||
| "en": "The 'subtract' method is not defined, causing a TypeError at runtime" | ||
| } | ||
| } | ||
| ] |
There was a problem hiding this comment.
Incorrect line reference for the missing method issue.
The issue describes that the subtract method is not defined, but the line reference points to line 17 (getValue() return statement). The actual problem occurs at line 22 where subtract() is called. Update the line reference to [22] to match the issue description.
{
"id": "js-l2-001-1",
- "lines": [17],
+ "lines": [22],
"type": "bug",
"severity": "critical",📝 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.
| "issues": [ | |
| { | |
| "id": "js-l2-001-1", | |
| "lines": [17], | |
| "type": "bug", | |
| "severity": "critical", | |
| "score": 4, | |
| "description": { | |
| "ja": "subtractメソッドが定義されていないため、実行時にTypeErrorが発生します", | |
| "en": "The 'subtract' method is not defined, causing a TypeError at runtime" | |
| } | |
| } | |
| ] | |
| "issues": [ | |
| { | |
| "id": "js-l2-001-1", | |
| "lines": [22], | |
| "type": "bug", | |
| "severity": "critical", | |
| "score": 4, | |
| "description": { | |
| "ja": "subtractメソッドが定義されていないため、実行時にTypeErrorが発生します", | |
| "en": "The 'subtract' method is not defined, causing a TypeError at runtime" | |
| } | |
| } | |
| ] |
🤖 Prompt for AI Agents
In app/problems/javascript/level2/js-l2-001.json around lines 24 to 36, the
issue entry "js-l2-001-1" incorrectly references line [17]; update its "lines"
array to [22] so the reported location matches the actual call to subtract() at
line 22. Edit the JSON so the "lines" field for that issue is [22] (no other
changes).
| { | ||
| "id": "js-l3-001-1", | ||
| "lines": [7], | ||
| "type": "bug", | ||
| "severity": "critical", | ||
| "score": 4, | ||
| "description": { | ||
| "ja": "ドット記法でkeyを使うと文字列'key'がプロパティ名になります。ブラケット記法[key]を使う必要があります", | ||
| "en": "Using dot notation with 'key' creates a property named 'key'. Should use bracket notation [key]" | ||
| } |
There was a problem hiding this comment.
Incorrect line reference for dot notation bug.
Issue js-l3-001-1 references line [7] (the closing brace }), but the actual bug is at line [6] where dot notation is used (currentSettings.key = value;). Update the line reference to match the buggy code line.
{
"id": "js-l3-001-1",
- "lines": [7],
+ "lines": [6],
"type": "bug",📝 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.
| { | |
| "id": "js-l3-001-1", | |
| "lines": [7], | |
| "type": "bug", | |
| "severity": "critical", | |
| "score": 4, | |
| "description": { | |
| "ja": "ドット記法でkeyを使うと文字列'key'がプロパティ名になります。ブラケット記法[key]を使う必要があります", | |
| "en": "Using dot notation with 'key' creates a property named 'key'. Should use bracket notation [key]" | |
| } | |
| { | |
| "id": "js-l3-001-1", | |
| "lines": [6], | |
| "type": "bug", | |
| "severity": "critical", | |
| "score": 4, | |
| "description": { | |
| "ja": "ドット記法でkeyを使うと文字列'key'がプロパティ名になります。ブラケット記法[key]を使う必要があります", | |
| "en": "Using dot notation with 'key' creates a property named 'key'. Should use bracket notation [key]" | |
| } |
🤖 Prompt for AI Agents
In app/problems/javascript/level3/js-l3-001.json around lines 22 to 31, the rule
js-l3-001-1 incorrectly points to line [7] (closing brace) instead of the actual
buggy line; update the "lines" array for id "js-l3-001-1" from [7] to [6] so the
reference points to the line with the dot-notation usage (`currentSettings.key =
value;`).
| { | ||
| "id": "js-l3-001-2", | ||
| "lines": [4, 5, 6, 7, 8], | ||
| "type": "design", | ||
| "severity": "minor", | ||
| "score": 2, | ||
| "description": { | ||
| "ja": "for...ofループを使えばより簡潔に書けます: for (const [key, value] of entries)", | ||
| "en": "Can be written more concisely using for...of loop: for (const [key, value] of entries)" | ||
| } | ||
| } |
There was a problem hiding this comment.
Incorrect line reference for design refactor issue.
Issue js-l3-001-2 references lines [4, 5, 6, 7, 8], but this includes the empty line at index 8. The for loop spans lines [3, 4, 5, 6, 7]. Update to accurately capture the loop block.
{
"id": "js-l3-001-2",
- "lines": [4, 5, 6, 7, 8],
+ "lines": [3, 4, 5, 6, 7],
"type": "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.
| { | |
| "id": "js-l3-001-2", | |
| "lines": [4, 5, 6, 7, 8], | |
| "type": "design", | |
| "severity": "minor", | |
| "score": 2, | |
| "description": { | |
| "ja": "for...ofループを使えばより簡潔に書けます: for (const [key, value] of entries)", | |
| "en": "Can be written more concisely using for...of loop: for (const [key, value] of entries)" | |
| } | |
| } | |
| { | |
| "id": "js-l3-001-2", | |
| "lines": [3, 4, 5, 6, 7], | |
| "type": "design", | |
| "severity": "minor", | |
| "score": 2, | |
| "description": { | |
| "ja": "for...ofループを使えばより簡潔に書けます: for (const [key, value] of entries)", | |
| "en": "Can be written more concisely using for...of loop: for (const [key, value] of entries)" | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/problems/javascript/level3/js-l3-001.json around lines 33 to 43, the
issue entry js-l3-001-2 incorrectly lists the affected lines as [4, 5, 6, 7, 8]
(including an empty line); update the "lines" array to accurately reflect the
for loop block as [3, 4, 5, 6, 7] so the refactor suggestion references the
correct code span.
| "issues": [ | ||
| { | ||
| "id": "js-l3-002-1", | ||
| "lines": [12], | ||
| "type": "bug", | ||
| "severity": "critical", | ||
| "score": 4, | ||
| "description": { | ||
| "ja": "インスタンスメソッド内でthis.find()を呼んでいますが、thisはUserインスタンスで配列ではありません", | ||
| "en": "Calling this.find() in instance method, but 'this' is a User instance, not an array" | ||
| } | ||
| }, |
There was a problem hiding this comment.
Incorrect line reference for this.find() bug.
Issue js-l3-002-1 references line [12] (the closing brace of findByName), but the actual bug is at line [11] where this.find() is incorrectly called. Update to reference the correct line with the buggy code.
{
"id": "js-l3-002-1",
- "lines": [12],
+ "lines": [11],
"type": "bug",🤖 Prompt for AI Agents
In app/problems/javascript/level3/js-l3-002.json around lines 29 to 40, the
issue js-l3-002-1 incorrectly points to line 12 (the closing brace) instead of
the actual buggy call at line 11; update the "lines" array for that issue from
[12] to [11] so the reported location points to the this.find() call inside
findByName.
| { | ||
| "id": "js-l3-002-2", | ||
| "lines": [22], | ||
| "type": "bug", | ||
| "severity": "critical", | ||
| "score": 4, | ||
| "description": { | ||
| "ja": "findByNameはインスタンスメソッドですが、配列を受け取るように設計されていません。静的メソッドにすべきです", | ||
| "en": "findByName is an instance method but not designed to receive an array. Should be a static method" | ||
| } | ||
| }, |
There was a problem hiding this comment.
Out-of-bounds line reference.
Issue js-l3-002-2 references line [22], but the code array only contains 22 elements (indices 0-21). This line reference is out of bounds. The issue likely intends to reference the findByName method definition at lines [10, 11]. Update to the correct line range.
{
"id": "js-l3-002-2",
- "lines": [22],
+ "lines": [10, 11],
"type": "bug",📝 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.
| { | |
| "id": "js-l3-002-2", | |
| "lines": [22], | |
| "type": "bug", | |
| "severity": "critical", | |
| "score": 4, | |
| "description": { | |
| "ja": "findByNameはインスタンスメソッドですが、配列を受け取るように設計されていません。静的メソッドにすべきです", | |
| "en": "findByName is an instance method but not designed to receive an array. Should be a static method" | |
| } | |
| }, | |
| { | |
| "id": "js-l3-002-2", | |
| "lines": [10, 11], | |
| "type": "bug", | |
| "severity": "critical", | |
| "score": 4, | |
| "description": { | |
| "ja": "findByNameはインスタンスメソッドですが、配列を受け取るように設計されていません。静的メソッドにすべきです", | |
| "en": "findByName is an instance method but not designed to receive an array. Should be a static method" | |
| } | |
| }, |
🤖 Prompt for AI Agents
In app/problems/javascript/level3/js-l3-002.json around lines 41 to 51, the
issue entry "js-l3-002-2" incorrectly references line [22] (out of bounds);
update its "lines" field to point to the actual findByName method definition at
[10,11] so the JSON accurately references the method location.
Add problem data structure with 8 problems across 3 difficulty levels:
Level 1 (3 problems):
Level 2 (3 problems):
Level 3 (2 problems):
Also implemented getProblems() function to dynamically load problems using Vite's import.meta.glob for efficient problem management.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.