fix(validator): return submission evaluations in chronological order#256
Open
tryeverything24 wants to merge 1 commit into
Open
fix(validator): return submission evaluations in chronological order#256tryeverything24 wants to merge 1 commit into
tryeverything24 wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The submission-detail serializer (
_submission_to_schemainvalidator/src/eval_backend/api/routes.py) returned a submission'sevaluationslist in unordered (relationship / DB-row) order, whiletrainswere returned in chronological order. So the two collections on thesame
SubmissionOutcame back sorted inconsistently.The bug is a copy/wiring slip: the function already computes a chronologically
sorted
ordered_evaluationslist, but that list was only assigned to a locallatestvariable that is never used — the response was still built from theraw, unsorted
submission.evaluations:Fix
Serialize
evaluationsfromordered_evaluations(matching howtrainsisalready handled) and remove the dead
latestlocal. The current phase/messageshown on the submission is derived separately via
_latest_run, so nothing elserelied 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 aSubmissionOut) drive the submission timeline in the web UI. Returningevaluation 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.pywith a focused regression:test_submission_schema_orders_evaluations_chronologically— feedsevaluations 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 existingcorrect train ordering so the two paths stay consistent.
Full
validatorsuite: 39 passed (Postgres-backed tests included).