feat(assignments): add Testbench model for autograded test suites - #7836
feat(assignments): add Testbench model for autograded test suites#7836magic-peach wants to merge 2 commits into
Conversation
Stores the simulator's testbench JSON per assignment and validates it is runnable before the autograder feeds it to the simulator. Groundwork for the headless runner and GradingResult PRs that follow.
WalkthroughAdds a Merge Risk: 🔵 Low · up to The PR adds persisted testbench data and validates its structure, but signal labels can still accept non-string values that may fail when the simulator uses them. The change is mergeable with explicit owner awareness and follow-up to tighten the testbench contract. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: dabde971-c705-457a-9bcb-d3a874ac19b4
📒 Files selected for processing (6)
app/models/assignment.rbapp/models/testbench.rbdb/migrate/20260814120000_create_testbenches.rbdb/schema.rbspec/factories/testbenches.rbspec/models/testbench_spec.rb
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
| end | ||
|
|
||
| def validate_signal(signal, cases) | ||
| return errors.add(:data, "every signal needs a label") unless signal.is_a?(Hash) && signal["label"].present? |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
printf '%s\n' '--- applicable repository guidance ---'
find /tmp/coderabbit-repo-knowledge/circuitverse-circuitverse-08827a34 -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- testbench model ---'
cat -n app/models/testbench.rb | sed -n '1,120p'
printf '%s\n' '--- focused diff ---'
git diff -- app/models/testbench.rbRepository: CircuitVerse/CircuitVerse
Length of output: 2569
🏁 Script executed:
printf '%s\n' '--- Rails version and focused test files ---'
rg -n '^\s*gem ["'\'']rails|rails \(' Gemfile Gemfile.lock 2>/dev/null || true
fd -i 'testbench' .
printf '%s\n' '--- label consumers and validation references ---'
rg -n 'bitWidth|signal\[["'\'']label["'\'']\]|\.label|signal.*label|every signal needs a label' app test spec 2>/dev/null | head -160Repository: CircuitVerse/CircuitVerse
Length of output: 7827
🏁 Script executed:
printf '%s\n' '--- model specs ---'
cat -n spec/models/testbench_spec.rb | sed -n '1,130p'
printf '%s\n' '--- controller and simulator entry points ---'
cat -n app/controllers/testbench_controller.rb | sed -n '1,180p'
cat -n simulator/src/testbench.js | sed -n '1,220p'
printf '%s\n' '--- simulator binding references ---'
rg -n -C 3 'label|inputs|outputs|bitWidth' simulator/src/testbench simulator/src/testbench.jsRepository: CircuitVerse/CircuitVerse
Length of output: 50381
🌐 Web query:
Rails 8.1 ActiveSupport Object#present? String#blank? Numeric#blank? official API documentation
💡 Result:
In Ruby on Rails 8.1, the methods present? and blank? are provided by ActiveSupport as core extensions to the Object class, making them available across virtually all objects [1][2][3]. Object#present? The present? method returns true if an object is not blank [4][3]. It is defined as the negation of blank? [4][1]: def present?!blank? end String#blank? For strings, the blank? method returns true if the string is empty or contains only whitespace characters (including Unicode whitespace) [5][6][7]. It is implemented as: def blank? empty? || BLANK_RE.match?(self) # (with rescue handling for encoding) end Numeric#blank? In Rails 8.1, the Numeric class defines blank? to always return false, as numbers are considered to be present [8]: def blank? false end These methods are standard throughout the Rails framework for checking the presence of data, such as parameters or model attributes [4][9]. Official documentation can be found in the Ruby on Rails API guides and class references [4][9][5].
Citations:
- 1: https://github.com/rails/rails/blob/v8.1.3.1/activesupport/lib/active_support/core_ext/object/blank.rb
- 2: https://api.rubyonrails.org/v8.1/files/activesupport/lib/active_support/core_ext/object/blank_rb.html
- 3: https://api.rubyonrails.org/v8.1/classes/Object.html
- 4: https://api.rubyonrails.org/v8.1.2.1/classes/Object.html
- 5: https://api.rubyonrails.org/v8.1.2.1/classes/String.html
- 6: https://api.rubyonrails.org/classes/String.html
- 7: https://api.rubyonrails.org/v8.1.0/classes/String.html
- 8: https://github.com/rails/rails/blob/v8.1.1/activesupport/lib/active_support/core_ext/object/blank.rb
- 9: https://guides.rubyonrails.org/v8.1/active_support_core_extensions.html
Require signal labels to be strings.
present? rejects whitespace-only strings, but it accepts non-string JSON values such as 1. The simulator later calls .trim() on each label, so a numeric label can pass model validation and raise a JavaScript TypeError. Require label.is_a?(String) && label.present?, and add a regression example for a numeric label.
1 participant
Fixes part of #7412
Describe the changes you have made in this PR -
This pr adds a Testbench model which is basically one JSONB-backed row per assignment that holds the test suite an instructor authors and the autograder later feeds to the simulator. This is the data layer everything else in the autograding phase builds on but nothing runs a testbench yet, this PR just makes sure a saved one is actually runnable before it's allowed to exist.
Screenshots of the UI changes (If any) -
none
Code Understanding and AI Usage
**Did you use AI generated code (ChatGPT, Claude, Copilot, etc.) in any part of this PR? **
If you used AI assistance:
Explain your implementation approach:
Describe in your own words:
What problem does your code solve?
Before the pr runner or the grading job can do anything, there has to be somewhere to store the instructor's test suite and a guarantee that what's stored is well-formed enough to run. Without validation at the model layer, a malformed suite (missing a group, an output with fewer values than test cases, a bad bit width) would only surface when the simulator chokes on it mid-run which reads to the student as their circuit being wrong, not the test data being broken. This PR pushes that check as early as possible in order to save time.
What alternative approaches did you consider?
Storing the suite as a column on Assignment itself: simpler schema, but confuses "what the assignment is" with "how it's graded," and would make the has-one relationship to a runner-facing testbench harder to reason about once the editor and versioning land.
Validating the suite shape only at the point of use (in the runner): rejected, because that pushes bad data into production and only fails at run time, per-submission, instead of at authoring time.
Why did you choose this specific implementation?
assignment_id is unique, and it's enforced in two places: the database (a unique index) and the model (a validation).
Validation happens in three steps that mirrors how a testbench is actually structured , So when something's wrong, the error points to the exact broken signal.
What are the key functions/components and what do they do?
Testbench : belongs to Assignment
#data_is_runnable : the top-level validation: checks data is a Hash, its type is one the simulator supports, and it has at least one group.
#validate_group(group) : checks each group has a positive case count (n) and at least one input and one output signal.
#validate_signal(signal, cases) : checks each signal has a label so the simulator can bind it to the right pin, a positive bitWidth, and exactly as many values as the group's case count.
Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.
Summary by CodeRabbit