ralph: #5 — Design Bootstrap Test Cases — create YAML case files for 10+ universal coding tasks#52
ralph: #5 — Design Bootstrap Test Cases — create YAML case files for 10+ universal coding tasks#52jharris1679 wants to merge 1 commit intomainfrom
Conversation
WalkthroughThis PR adds 13 new bootstrap test case configurations in YAML format, each containing sample code and metadata for coding exercises. Cases span JavaScript (Node.js) and Python, covering security vulnerabilities, performance issues, code quality problems, error handling, concurrency bugs, and testing scenarios. Total of 1,145 lines added across files in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![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
🤖 Fix all issues with AI agents
In `@cases/bootstrap/api-documentation.yaml`:
- Line 16: The Run command in the api-documentation exercise currently uses an
inappropriate build step ("Run: npm run build"); update the Run field in
cases/bootstrap/api-documentation.yaml to a command that actually verifies or
serves the documentation (for example start the server with node api_server.js,
run a docs generator like npm run docs or openapi-generator-cli, or a script
that launches the documentation preview). Edit the Run entry so it invokes the
verification/preview workflow (e.g. node api_server.js or the project's docs
generation script) instead of a generic build command to make the exercise
runnable for students.
In `@cases/bootstrap/component-extraction.yaml`:
- Around line 22-27: There's a typo in the YAML tags list: replace the incorrect
tag value "dpry" with the correct "dry" under the tags key so the tags array
contains "javascript", "refactoring", "code-quality", and "dry"; update the
entry "dpry" to "dry" in the tags block to fix the typo.
In `@cases/bootstrap/error-handling.yaml`:
- Around line 40-50: The fetchUserData function is incorrect because https.get
is callback-based and the current function returns the ClientRequest instead of
the parsed body; wrap the https.get flow in a Promise (or convert to
async/await) inside fetchUserData, accumulate chunks from res.on('data'),
resolve with JSON.parse(body) on res.on('end'), and reject the Promise on
request 'error' or JSON parse errors so callers can try/catch; update references
to fetchUserData and the https.get/res.on handlers accordingly.
- Around line 29-36: The code imports fs.promises but calls fs.readFileSync in
getUserProfile, causing a TypeError; replace the import const fs =
require('fs').promises; with the base module const fs = require('fs'); so
fs.readFileSync is available (or alternatively switch getUserProfile to use
fs.promises.readFile and make it async), ensuring the function name
getUserProfile and the read call (fs.readFileSync / fs.promises.readFile) are
updated consistently.
In `@cases/bootstrap/memory-leak.yaml`:
- Line 68: The main-module guard uses __filename === require.main.path which is
incorrect because require.main.path is a directory and the condition is always
false; update the demo guard in the file to use the standard Node idiom (e.g.,
check require.main === module or compare require.main.filename to __filename) so
the demo block (the code guarded by the current if using __filename and
require.main.path) actually executes when the file is run directly; replace the
incorrect condition with one of these correct checks and ensure any null-safe
access to require.main is handled.
🧹 Nitpick comments (2)
cases/bootstrap/input-validation.yaml (1)
34-47: Plaintext password stored in user object — potentially misleading for a security-focused exercise.
registerUserdestructurespasswordand includes it verbatim in the returned user object (line 41). For an exercise categorized under "security," this sets a bad example. Consider either hashing the password in the scaffold (e.g., usingcrypto.createHash) or adding a prompt bullet point about not storing plaintext passwords.cases/bootstrap/race-condition.yaml (1)
30-31: Unusedimport timeadds noise to the exercise.
timeis imported but never used in the code. Remove it to keep the exercise code focused.Proposed fix
import threading - import time from typing import List
| - Follow API documentation best practices | ||
| - Include authentication requirements | ||
|
|
||
| Run: npm run build |
There was a problem hiding this comment.
Run command npm run build is inappropriate for a documentation exercise.
This exercise asks students to add API documentation (JSDoc, OpenAPI, etc.). The run command should reflect how to verify the documentation or start the server, not build it. Consider something like node api_server.js or a documentation generation command.
🤖 Prompt for AI Agents
In `@cases/bootstrap/api-documentation.yaml` at line 16, The Run command in the
api-documentation exercise currently uses an inappropriate build step ("Run: npm
run build"); update the Run field in cases/bootstrap/api-documentation.yaml to a
command that actually verifies or serves the documentation (for example start
the server with node api_server.js, run a docs generator like npm run docs or
openapi-generator-cli, or a script that launches the documentation preview).
Edit the Run entry so it invokes the verification/preview workflow (e.g. node
api_server.js or the project's docs generation script) instead of a generic
build command to make the exercise runnable for students.
| tags: | ||
| - javascript | ||
| - refactoring | ||
| - code-quality | ||
| - dpry | ||
|
|
There was a problem hiding this comment.
Typo in tag: dpry should be dry.
tags:
- javascript
- refactoring
- code-quality
- - dpry
+ - dry📝 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.
| tags: | |
| - javascript | |
| - refactoring | |
| - code-quality | |
| - dpry | |
| tags: | |
| - javascript | |
| - refactoring | |
| - code-quality | |
| - dry |
🤖 Prompt for AI Agents
In `@cases/bootstrap/component-extraction.yaml` around lines 22 - 27, There's a
typo in the YAML tags list: replace the incorrect tag value "dpry" with the
correct "dry" under the tags key so the tags array contains "javascript",
"refactoring", "code-quality", and "dry"; update the entry "dpry" to "dry" in
the tags block to fix the typo.
| const fs = require('fs').promises; | ||
| const https = require('https'); | ||
|
|
||
| // Function to read user profile | ||
| function getUserProfile(userId) { | ||
| const filePath = `/data/users/${userId}.json`; | ||
| const data = fs.readFileSync(filePath, 'utf-8'); | ||
| return JSON.parse(data); |
There was a problem hiding this comment.
fs.readFileSync is not available on fs.promises.
Line 29 imports require('fs').promises, but line 35 calls fs.readFileSync, which only exists on the base fs module. This will throw TypeError: fs.readFileSync is not a function — an unintentional bug unrelated to the exercise's error-handling goal.
Since the exercise focuses on adding try-catch around synchronous I/O, import the base fs module instead:
Proposed fix
- const fs = require('fs').promises;
+ const fs = require('fs');📝 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.
| const fs = require('fs').promises; | |
| const https = require('https'); | |
| // Function to read user profile | |
| function getUserProfile(userId) { | |
| const filePath = `/data/users/${userId}.json`; | |
| const data = fs.readFileSync(filePath, 'utf-8'); | |
| return JSON.parse(data); | |
| const fs = require('fs'); | |
| const https = require('https'); | |
| // Function to read user profile | |
| function getUserProfile(userId) { | |
| const filePath = `/data/users/${userId}.json`; | |
| const data = fs.readFileSync(filePath, 'utf-8'); | |
| return JSON.parse(data); |
🤖 Prompt for AI Agents
In `@cases/bootstrap/error-handling.yaml` around lines 29 - 36, The code imports
fs.promises but calls fs.readFileSync in getUserProfile, causing a TypeError;
replace the import const fs = require('fs').promises; with the base module const
fs = require('fs'); so fs.readFileSync is available (or alternatively switch
getUserProfile to use fs.promises.readFile and make it async), ensuring the
function name getUserProfile and the read call (fs.readFileSync /
fs.promises.readFile) are updated consistently.
| function fetchUserData(userId) { | ||
| const url = `https://api.example.com/users/${userId}`; | ||
| const data = https.get(url, (res) => { | ||
| let body = ''; | ||
| res.on('data', (chunk) => body += chunk); | ||
| res.on('end', () => { | ||
| return JSON.parse(body); | ||
| }); | ||
| }); | ||
| return data; | ||
| } |
There was a problem hiding this comment.
fetchUserData is fundamentally broken beyond the error-handling scope.
https.get is callback-based — return data on line 49 returns the ClientRequest object, not the parsed response body. A student adding try-catch alone cannot make this function work correctly; it needs to be restructured as a Promise or use async/await.
For an "easy" difficulty exercise focused on error handling, consider either simplifying this to a synchronous example or explicitly noting in the prompt that the async pattern also needs fixing. Alternatively, bump the difficulty to "medium."
🤖 Prompt for AI Agents
In `@cases/bootstrap/error-handling.yaml` around lines 40 - 50, The fetchUserData
function is incorrect because https.get is callback-based and the current
function returns the ClientRequest instead of the parsed body; wrap the
https.get flow in a Promise (or convert to async/await) inside fetchUserData,
accumulate chunks from res.on('data'), resolve with JSON.parse(body) on
res.on('end'), and reject the Promise on request 'error' or JSON parse errors so
callers can try/catch; update references to fetchUserData and the
https.get/res.on handlers accordingly.
| } | ||
| } | ||
|
|
||
| if (__filename === require.main.path) { |
There was a problem hiding this comment.
Bug: incorrect main-module guard — demo block will never execute.
require.main.path returns the directory of the main module, not its filename. This condition will always be false, so the demo block is dead code.
Use the standard Node.js idiom instead:
- if (__filename === require.main.path) {
+ if (require.main === module) {📝 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.
| if (__filename === require.main.path) { | |
| if (require.main === module) { |
🤖 Prompt for AI Agents
In `@cases/bootstrap/memory-leak.yaml` at line 68, The main-module guard uses
__filename === require.main.path which is incorrect because require.main.path is
a directory and the condition is always false; update the demo guard in the file
to use the standard Node idiom (e.g., check require.main === module or compare
require.main.filename to __filename) so the demo block (the code guarded by the
current if using __filename and require.main.path) actually executes when the
file is run directly; replace the incorrect condition with one of these correct
checks and ensure any null-safe access to require.main is handled.
1 participant
Issue
Closes #5
Status: ✓ verified
Build, tests, and lint all pass locally.
Summary
Automated implementation by Ralph (rlmkit + MiniMax M2.5).
Review the changes carefully — this was generated by a local model.
Summary by CodeRabbit
Release Notes
New Features