Week 12: Two Small Pull Requests

Both pieces of the autograding groundwork from last week's "pipeline nobody could
feed" post finally went up for review. Getting them out the door taught me more
about the packaging rule I set for myself than the underlying feature did, and
along the way I found a gap in my own merged code that rhymes uncomfortably with
the one I found last week.

The rule I made myself follow

Somewhere back in this phase I decided to open small pull requests one at a time
and hold dependent branches locally until the parent merges, instead of stacking
them or folding a dependency into one oversized diff. I have rejected my own
compare pages twice for showing more than one commit, which is a strange thing to
do to yourself but a useful one.

The test case editor is the clearest reason why. Its branch holds a cherry pick
of the testbench model's commit plus the editor's own commit, because the editor
needs the model's table to exist to run its tests at all. Opening it now would
show two commits on the compare page, and a reviewer would have to work out which
one is actually new. So it stays local until the model merges, at which point I
rebase, the borrowed commit drops out, and what is left is one clean diff over
the editor's own files.

Which one first, and why it wasn't obvious

That left a smaller decision: open the testbench model or the runner first.

The runner's spec is deliberately built so it does not need the model to exist.
It signs its double with Struct.new(:data) instead of pulling in the real
Testbench factory, so the runner is green against master on its own. Nothing
about it forces the model to land first.

I opened the model first anyway, for three reasons that all point the same way.
The test case editor is stacked on the model, not the runner, so unblocking the
editor means merging the model first regardless. The model is also the smaller,
easier diff: one migration, one class, one spec, no external HTTP contract to
explain to a reviewer. And both branches touch db/schema.rb, which is exactly
the kind of file you do not want two open pull requests editing at once, since
whichever merges second has to rebase past a version bump it did not cause.

What actually lives in the Testbench model

One row per assignment, JSON in a jsonb column, validated in three layers that
mirror the shape of the data itself: suite, then group, then signal.

def data_is_runnable
  return errors.add(:data, "must be a testbench object") unless data.is_a?(Hash)

  errors.add(:data, "type must be comb or seq") unless TYPES.include?(data["type"])
  return errors.add(:data, "must define a group") if groups.empty?

  groups.each { |group| validate_group(group) }
end

Each group needs a case count and at least one input and one output. Each signal
needs a label to bind to a circuit pin, a positive bit width, and exactly as many
values as its group's case count. That last check is why validation lives in
Ruby instead of a database constraint. A jsonb column schema check can enforce
shape, but it cannot easily say "this array's length must equal that other
field's value," and that cross-field relationship is most of what makes a
testbench actually runnable rather than merely well formed.

assignment_id is unique at both levels, a database index and a model
validation, because a race on create could otherwise slip a second row past a
Ruby only check. TYPES is closed to comb and seq, the two suite types the
simulator can actually execute, so a suite claiming verilog fails at save time
instead of failing later, mid grade, in front of a student.

The runner, and the decision that already happened

Most of the runner's design was decided last week, before a line of it existed.
This week was writing it down.

def self.call(project:, testbench:)
  new(project, testbench).call
end

One entrypoint. It posts the project's circuit JSON and the testbench's suite
JSON to a sidecar service over HTTP, with timeouts split into connect, write,
and read rather than one blanket number, because a hung connection should fail
fast while an actual simulation run legitimately needs more time than the
handshake. Every way this can go wrong, a bad status, an unreachable host, a
body that is not JSON, a response with no groups, collapses into one
Autograder::Runner::RunnerError, so the grading job that calls this later only
ever needs to rescue one thing.

Result = Struct.new(:passed, :total, :groups) do
  def score
    total.zero? ? 0.0 : passed.fdiv(total)
  end
end

The raw groups array rides along inside the result even though only score
matters right now, because the results page a few pull requests from now needs
to show which specific case failed, not just the fraction. The endpoint's host
comes from SIMULATOR_RUNNER_URL, following the same undocumented environment
variable pattern the codebase already uses for YOSYS_PATH, so anyone who has
configured one external tool location here already knows how to configure this
one.

What "next" turned out to mean

Last week's post ended by promising the editor would land next. It has not, and
writing the runner's pull request description is what changed my mind.

I went back to reread the roster sync pull request from two weeks ago, the one
that calls the platform's Names and Roles service, to check whether it needed
anything from this new access token machinery. It does. So does grade passback.
Both of those already assume an Authorization: Bearer header carrying a token
scoped to the right service, and until this week nothing in the codebase could
produce one. I had built two pull requests that read from and write to platform
services that require an OAuth exchange neither of them performs.

This is the same shape of gap as last week's missing test case editor: a feature
description satisfied on paper by code that quietly assumes a piece nobody has
built yet. The difference is I caught this one myself, by rereading my own
merged work with the new piece in mind, rather than having a mentor's evaluation
catch it for me.

Where the project stands

The testbench model and the runner are both open for review, in that order.
Roster sync and grade passback are merged but cannot actually authenticate
against a platform yet. The test case editor is finished and waiting locally for
the model to merge.

Next week: the access token exchange that both of those merged pull requests
have been silently depending on.