core&ui&judge: allow run multiple pretests in single request#1098
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request changes pretest input handling from a single string to arrays across the codebase. The public type RecordPayload.input is widened to Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code graph analysis (1)packages/hydrojudge/src/judge/run.ts (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (7)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/common/types.ts(1 hunks)packages/hydrojudge/src/judge/run.ts(2 hunks)packages/hydrojudge/src/task.ts(2 hunks)packages/hydrooj/src/handler/problem.ts(2 hunks)packages/hydrooj/src/model/record.ts(2 hunks)packages/ui-default/components/scratchpad/ScratchpadToolbarContainer.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/hydrooj/src/handler/problem.ts (2)
framework/framework/validator.ts (2)
Types(29-66)Types(91-193)framework/framework/error.ts (1)
ValidationError(54-63)
packages/hydrojudge/src/judge/run.ts (7)
packages/common/subtask.ts (1)
NormalizedCase(134-137)packages/hydrojudge/src/judge/interface.ts (1)
Context(6-6)packages/hydrooj/src/model/record.ts (1)
judge(82-130)packages/hydrojudge/src/judge/objective.ts (1)
judge(7-81)packages/hydrojudge/src/judge/hack.ts (1)
judge(8-86)packages/hydrojudge/src/judge/generate.ts (1)
judge(13-146)packages/hydrojudge/src/flow.ts (1)
runFlow(76-136)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (8)
packages/ui-default/components/scratchpad/ScratchpadToolbarContainer.jsx (1)
38-49: LGTM! Input correctly wrapped in array for new API.The change correctly adapts the UI to send
inputas an array, aligning with the updated backend contract. The UI currently supports single pretest input but the backend is now ready for multiple inputs when the UI is enhanced.packages/common/types.ts (1)
109-110: LGTM! Type widened to support both string and array inputs.The union type
string | string[]maintains backward compatibility while enabling the new multiple pretest input feature. This is consistent with the gradual migration pattern across the codebase.packages/hydrooj/src/model/record.ts (2)
137-137: LGTM! Args type updated for array-based input.
170-173: LGTM! Pretest path correctly handles array input with empty array default.The default of
args.input || []ensures a safe fallback when no input is provided, consistent with the new array-based input handling.packages/hydrooj/src/handler/problem.ts (2)
486-488: LGTM! Parameter type and validation correctly updated for array input.The parameter declaration, default value, and method signature are all correctly updated to handle array-based pretest input. The
Types.ArrayOf(Types.String)decorator will properly validate and transform incoming data.
501-501: LGTM! Validation correctly checks for non-empty array.The validation
!input.lengthproperly ensures at least one pretest input is provided whenpretestis true.packages/hydrojudge/src/judge/run.ts (2)
8-53: LGTM! Per-case judging implementation is well-structured.The curried
judgeCasefunction correctly:
- Executes each test case with its own input
- Handles time/memory limit detection with 2x allowance for debugging
- Captures exit codes and signals for runtime errors
- Triggers analysis on first WA/RE for debugging assistance
- Returns properly structured case results
55-78: LGTM! Judge function correctly orchestrates per-case flow.The implementation properly:
- Creates a single subtask containing all input cases with uniform scoring
- Maps each input string from
ctx.inputto a normalized case structure- Delegates compilation and execution to
runFlowwith thejudgeCasehandlerThe
output: ''for each case is intentional since pretest mode doesn't validate output against expected results—it just runs the code and reports the outcome.
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| subtaskId: 1, | ||
| status, | ||
| time: Math.floor(time * 1000000) / 1000000, | ||
| score: 1, |
There was a problem hiding this comment.
Bug: Score always 1 regardless of pass/fail status
The judgeCase function unconditionally returns score: 1 regardless of whether the test case passed or failed. Since the subtask uses type: 'sum', all case scores are summed together, meaning failed test cases (TLE, MLE, runtime error) still contribute to the total score. Looking at other judges like default.ts and all checkers, the score is conditionally set to 0 for non-ACCEPTED statuses. The score here likely needs to be conditional on status === STATUS.STATUS_ACCEPTED.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.
Note
Enable submitting multiple pretest inputs at once, with per-case execution in judge and end-to-end schema/handler/UI updates.
RecordPayload.inputto bestring | string[].packages/hydrojudge/src/judge/run.tsto handle multiple inputs viarunFlow; buildsubtasksfromctx.input, execute per-case, run analysis once, and truncate messages.packages/hydrojudge/src/judge/generate.tsto read per-case stdin fromctx.input[i-1].JudgeTask.inputasstring[]inpackages/hydrojudge/src/task.ts.problem.submitnow acceptsinput: string[]and validates non-empty for pretest; pass array torecord.add.manage.scriptpretest wrapsinputas array.RecordModel.addstores pretestinputasstring[].input: [pretestInput]incomponents/scratchpad/ScratchpadToolbarContainer.jsx.Written by Cursor Bugbot for commit e78b7b5. This will update automatically on new commits. Configure here.