Exclude prepend from Conductor testcase error line numbers - #4291
Exclude prepend from Conductor testcase error line numbers#4291Shrey5132 wants to merge 2 commits into
Conversation
runTestCaseConductor concatenates prepend/value/postpend/testcase into one file before sending it to the Conductor evaluator, but unlike evalCode.ts's Run-path fix for the same off-by-N bug (source-academy#4244), nothing subtracted the prepend's line count back out of a resulting error's reported line - so an error on the student's own first line was reported at `prepend line count + 1`. evalCodeConductorSaga's own prepend-offset correction doesn't reach this path: it's gated on an editor Run's actionType, and runTestCaseConductor calls evalCodeSaga with EVAL_SILENT instead. Shift the line back out locally in runTestCase.ts instead, using the same prepend value this saga already concatenated into the combined file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FYm9pttcp3nixtDLKiPh2k
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change corrects Conductor testcase error locations by removing non-empty prepend lines from positive line numbers. It preserves columns, location boundaries, and non-positive line values. Regression tests cover shifted and line 0 errors. ChangesConductor error location correction
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR makes a localized correction to testcase error line reporting, with validation checks passing; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FYm9pttcp3nixtDLKiPh2k
Summary
runTestCaseConductorconcatenatesprepend/student code/postpend/testcase into one combined file before sending it to the Conductor evaluator, but never subtracted the prepend's line count back out of a resulting error's reported line - so an error on the student's own first line was reported atprepend line count + 1.evalCode.ts'stoConductorSourceError/preludeLineOffset), but that fix doesn't reach this call site: it's gated on an editor Run'sactionType, andrunTestCaseConductorcallsevalCodeSagawithEVAL_SILENTinstead. Testcase/grading errors in assessments were still shifted.runTestCase.ts, using the sameprependvalue this saga already concatenates into the combined file. A location's line of0(toConductorSourceError's "no location info" fallback) is left untouched rather than shifted into a misleading line1.Test plan
0(unknown location) error is left unshifted.yarn test src/commons/sagas/WorkspaceSaga.test.ts- 69 passed.yarn eslinton changed files - clean.tsc --noEmit- clean.https://claude.ai/code/session_01FYm9pttcp3nixtDLKiPh2k