Skip to content

fix(validator): return submission evaluations in chronological order#256

Open
tryeverything24 wants to merge 1 commit into
mini-router:mainfrom
tryeverything24:fix-submission-eval-order
Open

fix(validator): return submission evaluations in chronological order#256
tryeverything24 wants to merge 1 commit into
mini-router:mainfrom
tryeverything24:fix-submission-eval-order

Conversation

@tryeverything24

Copy link
Copy Markdown

What

The submission-detail serializer (_submission_to_schema in
validator/src/eval_backend/api/routes.py) returned a submission's
evaluations list in unordered (relationship / DB-row) order, while
trains were returned in chronological order. So the two collections on the
same SubmissionOut came back sorted inconsistently.

The bug is a copy/wiring slip: the function already computes a chronologically
sorted ordered_evaluations list, but that list was only assigned to a local
latest variable that is never used — the response was still built from the
raw, unsorted submission.evaluations:

ordered_evaluations = sorted(
    submission.evaluations,
    key=lambda run: (run.created_at or _utcnow(), run.id),
)
ordered_trains = sorted(
    submission.trains,
    key=lambda run: (run.created_at or _utcnow(), run.id),
)
latest = ordered_evaluations[-1] if ordered_evaluations else None   # dead
evaluations = [
    EvaluationOut(...)
    for run in submission.evaluations                                # unsorted
]
trains = [TrainOut(...) for run in ordered_trains]                   # sorted

Fix

Serialize evaluations from ordered_evaluations (matching how trains is
already handled) and remove the dead latest local. The current phase/message
shown on the submission is derived separately via _latest_run, so nothing else
relied on that variable.

Net diff is small: one line switched to the sorted list, one dead line removed.

Why it matters

GET /api/submissions/{id} (and the create/webhook responses that embed a
SubmissionOut) drive the submission timeline in the web UI. Returning
evaluation runs in a nondeterministic, storage-dependent order makes the run
history render out of sequence, and makes "latest run" logic on the client
unreliable. Ordering is now deterministic and consistent with trains.

Tests

Adds validator/tests/test_submission_schema.py with a focused regression:

  • test_submission_schema_orders_evaluations_chronologically — feeds
    evaluations in reverse-chronological order and asserts the serialized output
    is chronological. Fails on main ([2, 1] != [1, 2]), passes with the fix.
  • test_submission_schema_orders_trains_chronologically — pins the existing
    correct train ordering so the two paths stay consistent.

Full validator suite: 39 passed (Postgres-backed tests included).

The submission detail serializer computed a chronologically sorted

"ordered_evaluations" list but only used it for an unused local variable,

building the response from the unordered relationship instead. Trains were

already ordered, so evaluations and trains came back in inconsistent order.

Serialize evaluations from the sorted list (matching trains) and drop the

dead variable. Adds a regression test covering both orderings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant