Request datastore validation after each full cron submission

every_eval_ever/cron/README.md

@@ -240,6 +240,15 @@ the pull request for the adapter. A refresh the Hub refuses is reported in the
 run report and the step summary rather than failing the run, because by then
 the records are published.
 
+A submission that lands completely ends by posting `/eee validate changed` as
+a fresh comment on the pull request, because the datastore's validator runs on
+that command rather than on push — records nobody comments on are records
+nobody validated. It is posted last, after the records and the description, so
+what it validates is the finished submission, and never after a partial one: a
+retry that completes the submission asks instead. A comment the Hub refuses is
+reported like a refused description rewrite, with a note asking a human to
+post the command by hand.
+
 ## Setup
 
 1. Nothing, if the token in the next step can create datasets: the first run

every_eval_ever/cron/__main__.py

@@ -609,9 +609,13 @@ def _finish(
             if submission is not None:
                 pull_request = submission.pull_request
                 committed_paths = submission.committed_paths
-                if submission.description_note:
-                    notes.append(submission.description_note)
-                    outcome.messages.append(submission.description_note)
+                for note in (
+                    submission.description_note,
+                    submission.validation_note,
+                ):
+                    if note:
+                        notes.append(note)
+                        outcome.messages.append(note)
 
     report = outcome.to_manifest()
     report['raw_reference'] = raw_reference

every_eval_ever/cron/submit.py

@@ -32,6 +32,11 @@
 #: How an instance-level sidecar is named after its aggregate. The two are
 #: one record and are committed together.
 SAMPLES_SUFFIX = '_samples.jsonl'
+#: The comment that asks the datastore's validation bot to check what a pull
+#: request now carries. Validation on the Hub side runs on request, not on
+#: push: a pull request nobody comments on is a pull request nobody
+#: validated (see evaleval/EEE_datastore discussion 168).
+VALIDATION_COMMAND = '/eee validate changed'
 
 
 class SubmissionError(RuntimeError):
@@ -113,6 +118,11 @@ class Submission:
     #: Why the pull request body still describes an earlier run, when it does.
     #: Not a failure: the records are published either way.
     description_note: str | None = None
+    #: Why the datastore's validator was not asked to check this pull
+    #: request, when it was not. Not a failure either, but worth a human
+    #: reading: an unvalidated pull request sits unreviewed until somebody
+    #: posts the command by hand.
+    validation_note: str | None = None
 
 
 def _discussion_number(discussion: Any) -> int | None:
@@ -328,6 +338,23 @@ def update_description(
                 f'{type(exc).__name__}: {exc}'
             ) from exc
 
+    def request_validation(self, pull_request: PullRequest) -> None:
+        """Post :data:`VALIDATION_COMMAND` on a pull request, as a new
+        comment, which is what makes the datastore validate it."""
+        try:
+            self.api.comment_discussion(
+                repo_id=self.repo_id,
+                repo_type='dataset',
+                discussion_num=pull_request.number,
+                comment=VALIDATION_COMMAND,
+            )
+        except Exception as exc:  # noqa: BLE001 - re-raised with context
+            raise SubmissionError(
+                f'could not request validation on pull request '
+                f'{pull_request.number} on {self.repo_id}: '
+                f'{type(exc).__name__}: {exc}'
+            ) from exc
+
     def pull_request_status(self, number: int) -> str:
         """Return ``open``, ``merged`` or ``closed`` for a pull request.
 
@@ -560,14 +587,16 @@ def _upload_batches(
                 )
             except Exception as exc:  # noqa: BLE001 - re-raised with context
                 landed = self._paths_on_ref(pull_request, batch)
-                unresolved: list[str] = []
                 if landed:
+                    # The Hub accepted the commit and only the reply was
+                    # lost, so this batch is on the pull request and the
+                    # upload carries on. Stopping here instead would end a
+                    # run whose every batch landed as a failure nothing
+                    # retries, because its records are all accounted for.
                     committed.extend(landed)
-                    hint = (
-                        ' The failing batch itself reached the pull request '
-                        'despite the error and is counted as committed.'
-                    )
-                elif landed is None:
+                    continue
+                unresolved: list[str] = []
+                if landed is None:
                     unresolved = [operation.path_in_repo for operation in batch]
                     hint = (
                         ' Whether the failing batch landed could not be '
@@ -654,6 +683,10 @@ def publish(
         body a reviewer reads describes the run that last added to it rather
         than whichever run opened it.
 
+        A submission that lands completely ends by requesting validation
+        (see :meth:`request_validation`); a partial one leaves that to the
+        retry that completes it.
+
         An opening commit that errored after landing is adopted rather than
         repeated, and what it left on the ref decides whether its batch counts
         as published.
@@ -719,10 +752,25 @@ def publish(
                 # The records are in. A stale body is worth reporting and not
                 # worth failing a run that published everything it meant to.
                 note = f'{exc}; the body still describes an earlier run'
+        validation_note = None
+        # Last, once the pull request holds everything this run meant to
+        # publish, so the validator reads the finished submission. A partial
+        # submission never reaches here, which is deliberate: asking for
+        # validation of half an upload wastes the reviewer the command
+        # summons.
+        try:
+            self.request_validation(pull_request)
+        except SubmissionError as exc:
+            # The records are in, same bargain as the description: report
+            # that nobody asked for validation rather than fail the run.
+            validation_note = (
+                f'{exc}; post `{VALIDATION_COMMAND}` on it manually'
+            )
         return Submission(
             pull_request=pull_request,
             committed_paths=tuple(committed),
             description_note=note,
+            validation_note=validation_note,
         )
 
 
@@ -841,6 +889,7 @@ def pull_request_description(
     'Submission',
     'SubmissionError',
     'SAMPLES_SUFFIX',
+    'VALIDATION_COMMAND',
     'marker',
     'pull_request_description',
     'pull_request_title',

tests/test_cron_store_and_submit.py

@@ -61,6 +61,8 @@ def __init__(
         self.details_error: Exception | None = None
         self.edit_comment_error: Exception | None = None
         self.edited_comments: list[tuple[int, str]] = []
+        self.comment_error: Exception | None = None
+        self.posted_comments: list[tuple[int, str]] = []
         self.list_files_error: Exception | None = None
         self.whoami_error: Exception | None = None
         self.repo_info_error: Exception | None = None
@@ -152,6 +154,12 @@ def edit_discussion_comment(
                 return type('Comment', (), {'content': new_content})()
         raise EntryNotFoundError(f'discussion {discussion_num} not found')
 
+    def comment_discussion(self, *, discussion_num, comment, **kwargs):
+        if self.comment_error is not None:
+            raise self.comment_error
+        self.posted_comments.append((discussion_num, comment))
+        return type('Comment', (), {'content': comment})()
+
     def list_repo_files(self, repo_id=None, **kwargs):
         if self.list_files_error is not None:
             raise self.list_files_error
@@ -1106,15 +1114,17 @@ def fail_on_second(**kwargs):
     )
 
 
-def test_a_batch_that_landed_despite_the_error_is_counted_committed(
+def test_a_batch_that_landed_despite_the_error_does_not_stop_the_upload(
     tmp_path,
 ) -> None:
     """The ambiguous timeout: the Hub accepted the commit, the client saw an
-    error. Reporting only the earlier batches would make the caller's ledger
-    forget this one, and the retry would republish it under fresh UUID paths.
-    The pull request ref is the arbiter of what actually landed."""
+    error. The pull request ref is the arbiter of what actually landed, and a
+    batch that is on it is a success, so the upload carries on. Stopping
+    instead turned a run whose final batch landed this way into a failure
+    with every record accounted for, which no retry ever completed and so
+    nothing ever validated."""
     tree = _upload_tree(tmp_path, 7)
-    hub = FakeHub()
+    hub = FakeHub(discussions=[cron_pr(12)])
     sub = submit.DatastoreSubmitter(hub, batch_size=3)
     pull_request = submit.PullRequest(12, 'https://x/12', 'refs/pr/12', 'x')
 
@@ -1128,20 +1138,19 @@ def land_then_time_out(**kwargs):
 
     hub.create_commit = land_then_time_out
 
-    with pytest.raises(submit.PartialSubmissionError) as caught:
-        sub.publish(
-            'hle',
-            pull_request=pull_request,
-            operations=submit.upload_operations(tree),
-            description='',
-            message='hle 2026-08-10',
-        )
+    submission = sub.publish(
+        'hle',
+        pull_request=pull_request,
+        operations=submit.upload_operations(tree),
+        description='body',
+        message='hle 2026-08-10',
+    )
 
-    # Both the clean first batch and the ambiguous second one are reported;
-    # only the never-attempted third is left for the retry.
-    assert len(caught.value.committed_paths) == 6
-    assert caught.value.unresolved_paths == ()
-    assert 'reached the pull request despite the error' in str(caught.value)
+    # The clean first batch and the two ambiguous ones all count; nothing is
+    # left for a retry, and the finished submission is validated.
+    assert len(submission.committed_paths) == 7
+    assert hub.posted_comments == [(12, submit.VALIDATION_COMMAND)]
+    assert submission.validation_note is None
 
 
 def test_an_unanswerable_reconciliation_claims_nothing(tmp_path) -> None:
@@ -1373,6 +1382,94 @@ def test_a_description_that_cannot_be_refreshed_does_not_fail_the_run(
     assert 'describes an earlier run' in submission.description_note
 
 
+# --- validation is asked for once everything is in ------------------------
+
+
+def test_a_full_submission_into_a_reused_pull_request_asks_for_validation(
+    tmp_path,
+) -> None:
+    """The datastore validates on request, so a run that published records
+    has to post the command or nothing checks them."""
+    tree = _upload_tree(tmp_path, 2)
+    hub = FakeHub(discussions=[cron_pr(12)])
+    sub = submit.DatastoreSubmitter(hub)
+    pull_request = submit.PullRequest(12, 'https://x/12', 'refs/pr/12', 'x')
+
+    submission = sub.publish(
+        'hle',
+        pull_request=pull_request,
+        operations=submit.upload_operations(tree),
+        description='body',
+        message='hle 2026-08-10',
+    )
+
+    assert hub.posted_comments == [(12, submit.VALIDATION_COMMAND)]
+    assert submission.validation_note is None
+
+
+def test_a_newly_opened_pull_request_asks_for_validation(tmp_path) -> None:
+    tree = _upload_tree(tmp_path, 2)
+    hub = FakeHub()
+    sub = submit.DatastoreSubmitter(hub)
+
+    submission = sub.publish(
+        'hle',
+        pull_request=None,
+        operations=submit.upload_operations(tree),
+        description='body',
+        message='hle 2026-08-10',
+    )
+
+    number = submission.pull_request.number
+    assert hub.posted_comments == [(number, submit.VALIDATION_COMMAND)]
+    assert submission.validation_note is None
+
+
+def test_a_validation_request_that_fails_does_not_fail_the_run(
+    tmp_path,
+) -> None:
+    """The records are published either way; the note tells a human to post
+    the command by hand."""
+    tree = _upload_tree(tmp_path, 2)
+    hub = FakeHub(discussions=[cron_pr(12)])
+    hub.comment_error = RuntimeError('403 Forbidden')
+    sub = submit.DatastoreSubmitter(hub)
+    pull_request = submit.PullRequest(12, 'https://x/12', 'refs/pr/12', 'x')
+
+    submission = sub.publish(
+        'hle',
+        pull_request=pull_request,
+        operations=submit.upload_operations(tree),
+        description='body',
+        message='hle 2026-08-10',
+    )
+
+    assert len(submission.committed_paths) == 2
+    assert '403 Forbidden' in submission.validation_note
+    assert submit.VALIDATION_COMMAND in submission.validation_note
+
+
+def test_a_partial_submission_asks_for_no_validation(tmp_path) -> None:
+    """Validation of half an upload wastes the reviewer it summons; the
+    retry that completes the submission asks instead."""
+    tree = _upload_tree(tmp_path, 2)
+    hub = FakeHub(discussions=[cron_pr(12)])
+    hub.commit_error = RuntimeError('504 Gateway Timeout')
+    sub = submit.DatastoreSubmitter(hub)
+    pull_request = submit.PullRequest(12, 'https://x/12', 'refs/pr/12', 'x')
+
+    with pytest.raises(submit.PartialSubmissionError):
+        sub.publish(
+            'hle',
+            pull_request=pull_request,
+            operations=submit.upload_operations(tree),
+            description='body',
+            message='hle 2026-08-10',
+        )
+
+    assert hub.posted_comments == []
+
+
 # --- what happened to the last pull request -------------------------------