fix(validator): skip synchronous eval for submissions with no artifact (#143)

[joaovictor91123]

Re-opens #156, which was auto-closed (silently) by the submission-eval lifecycle when the branch was pushed — the exact #143 behaviour this PR fixes. Rebased onto current main; test-router/test-validator are green.

Fixes #143

Summary

Infra/bugfix PRs that ship no submissions/final_model/best_theta.npy were being evaluated, failing with missing_checkpoint, and marked failed — the state that closes them before a maintainer can merge (#143). My own #117 was auto-closed exactly this way.

The async paths already enforce "don't evaluate a submission with no artifact": the pipeline enqueue in /submit and queue.py both gate on submission.submission_artifact_id is not None.

# routes.py — async enqueue, already gated
if submission.submission_artifact_id is not None and (
    not settings.sync_eval_on_submit or settings.uses_train_pipeline
):
    enqueue_submission_pipeline_job(...)

But the synchronous branch right below it was missed:

# routes.py — sync eval, NOT gated on the artifact
if settings.sync_eval_on_submit and not settings.uses_train_pipeline:
    session.refresh(submission)
    evaluate_submission(session, submission, runtime_settings)   # runs with no checkpoint

evaluate_submission then hits eval_runner.py "submission … does not have a checkpoint to evaluate" → missing_checkpoint → submission failed.

What Changed

• validator/src/eval_backend/api/routes.py: extracted _should_sync_eval(settings, submission) and added the same submission_artifact_id is not None clause, so the synchronous path matches the two async paths. The decision still reads settings (as the async enqueue gate does); only the artifact guard is added.
• validator/tests/test_submit_sync_eval_gate.py (new, offline — no Postgres/GPU): the pure _should_sync_eval decision is unit-tested — no-artifact → skip, artifact present → eval, sync disabled → skip, train pipeline → defer to async.
• docs/JOURNAL.md: replication-log entry per repository protocol (AGENTS.md §6).

What This Deliberately Does NOT Change

eval_runner's missing_checkpoint failure is left intact. That path is the correct behaviour when a submission genuinely has an uploaded artifact row whose checkpoint file is missing/corrupt. The fix is narrowly about not starting an eval for a submission that has no artifact at all — mirroring the existing async gate, not weakening the safety net.

Why the artifact_id gate is the right signal (matching #143's proposal)

An infra/bugfix PR never uploads a bundle, so its submission's submission_artifact_id stays None. #143 proposes exactly this: "only queue eval when submission_artifact_id is set." This applies that rule to the one code path that violated it.

Notes

• Verified locally: the new test_no_artifact_is_never_sync_evaluated fails on pre-fix main (the sync branch returns True for a checkpoint-less submission) and passes after; the other three pass both ways. The test needs no database, so it runs in CI regardless of Postgres availability (the DB-backed validator tests skip without it).
• Two honest caveats, both outside this diff: a code fix cannot reopen already-closed PRs, and it only takes effect once the maintainer deploys the validator — which is why Infra/bugfix PRs auto-closed by validator before maintainer can merge #143 asks for a manual merge/deploy. Until then, this PR (like other infra PRs) may itself be caught by the very bug it fixes.
• Scope is limited to the sync-eval gate; registration, the async queue, eval execution, and result publishing are untouched.