Week 15: What the Simulator Actually Binds

#7746 merged clean, rebase and all, closing out last week's story. That
freed the week for the thing I actually promised: picking the test case
editor back up. Except the editor cannot be evaluated on its own terms until
its two dependencies, the testbench model (#7836) and the runner (#7906),
are actually mergeable, and both came back from review this week with
findings that needed fixing before either could move.

A label that is not a string

CodeRabbit's first flag on the testbench model was on the signal validator:

def validate_signal(signal, cases)
  return errors.add(:data, "every signal needs a label") unless signal.is_a?(Hash) && signal["label"].present?
  ...

present? rejects nil, empty strings, and whitespace-only strings. It does
not reject 1. A JSON body with "label": 1 sails through this check, gets
stored, and later reaches the simulator, which calls .trim() on every
signal's label while binding pins to the circuit. Numbers do not have a
.trim() method in JavaScript. The failure is not "this test case behaves
oddly," it is a TypeError that kills the run.

The fix is one clause:

return errors.add(:data, "every signal needs a label") unless signal.is_a?(Hash) && signal["label"].is_a?(String) && signal["label"].present?

Small, but it is the kind of small that only shows up if you trace the value
forward into the file that actually consumes it, rather than stopping at "the
model validates its own shape."

The binding a human reviewer caught that a linter would not

The second finding was not from CodeRabbit. A reviewer left this on the same
file:

nothing checks that later groups use the same input/output labels as
groups[0]. the simulator binds pins from groups[0] only
(testbench.js:818-823) and then does inputs[label].state and
group.outputs.find(...).results.push per group, so a suite that passes
this validator can throw a TypeError mid-run.

I went and read testbench.js before touching anything, because the claim is
specific enough to verify rather than trust. It is exactly right:

function bindIO(data, scope) {
    const inputs = {};
    data.groups[0].inputs.forEach((dataInput) => {
        inputs[dataInput.label.trim()] = scope.Input.find(...);
    });
    ...
}

Only data.groups[0] ever gets read here. Every other group is bound
against whatever bindIO built from the first one. If group two declares an
input labeled c that group one never had, inputs['c'] is undefined, and
setInputValues dies trying to set .state on it. The model's own
validation, one group at a time, had no way to catch this, because the bug is
not in any single group, it is in the relationship between groups.

The fix compares each group after the first against the first's shape:

def validate_group(group, reference)
  ...
  next unless reference

  shape = ->(list) { list.map { |s| [s["label"], s["bitWidth"]] } }
  next if shape.call(signals) == shape.call(reference[side])

  errors.add(:data, "#{side} must match the first group's signals")
end

Values that are the right length and the wrong alphabet

Same reviewer, same file, a second finding: values were only length checked
against the case count, never checked for actually being binary digits. The
simulator feeds inputs through parseInt(v, 2) and compares outputs against
dec2bin(value, bitWidth) as strings. [0, 1] as integers, "2" on a one
bit pin, all of it passes the old check and then fails every single case at
run time, silently, which reads to a student as "I got every case wrong"
rather than "the test data itself is malformed."

def valid_values?(values, cases, width)
  values.is_a?(Array) && values.size == cases &&
    values.all? { |v| v.is_a?(String) && width.is_a?(Integer) && v.match?(/\A[01]{#{width}}\z/) }
end

Exact width, exact alphabet, exact type. I added a regression case for a
numeric label, one for wrong-width values, and one for a second group with a
mismatched pin set, then reverted each fix in turn to confirm its matching
test actually fails without it. All three did. A test that cannot fail is
not a test, it is a comment that happens to run.

The runner's two findings were a different flavor entirely

#7906, the runner, got two findings that had nothing to do with the shape
of any data structure:

with this fallback a prod box missing SIMULATOR_RUNNER_URL silently posts
student circuits to its own localhost and every run just shows as
"unreachable".

def endpoint
  "#{ENV.fetch('SIMULATOR_RUNNER_URL', DEFAULT_URL)}/run"
end

The default was meant for local development convenience and quietly doubled
as a production misconfiguration that fails silent instead of loud. Dropping
the fallback entirely, ENV.fetch('SIMULATOR_RUNNER_URL') with no second
argument, means a missing variable raises immediately instead of routing
every grading attempt to a service that was never there.

The second finding was that the request carries no authentication at all,
so anything that can reach the runner can make it simulate arbitrary
circuits. I added a bearer token, ENV.fetch('SIMULATOR_RUNNER_TOKEN'), sent
on every request. The runner service on the other end still needs to
actually check it, which is a different pull request's job, but the caller
no longer has an excuse not to send one.

Two different ways of being right

The testbench findings came from reading JavaScript nobody asked the model
to know about. The runner findings came from reading the word "fallback" and
asking what happens when the thing it falls back to is wrong. Neither is
something a type checker or a schema validator would have caught on its
own; both needed someone to actually trace a value from where it is written
to where it is read. That is becoming the throughline of this whole review
cycle: static analysis catches shapes, a reader who follows the data catches
contracts.

Where the project stands

Both the testbench model and the runner have their review fixes pushed, all
cases verified failing without their fix and passing with it. The test case
editor is still sitting locally, unstacked, waiting for these two to actually
merge before its own compare page can show one clean commit instead of three
borrowed ones.

Next week: the editor's own review round, which turns out to have found the
same class of bug in the client side JSON that this week found in the model,
plus one unrelated bug that only became reachable because of a setting this
phase never had before.