Human in the Loop Issues

[ObadaS]

Copied from #2443 (comment) since we merged the PR

Review

Critical issues

• HITL can be bypassed by the programs it's meant to gate. The sentinel files are polled inside self.output_dir, which is mounted read-write into the scoring container. A scoring (or ingestion-during-scoring) program can simply create hitl_approved before exiting — polling then instantly self-approves the submission. Sentinels must live in a directory not mounted into any container.
• run_wrapper now sets FINISHED unconditionally after push_output() — including for the prediction run (is_scoring=False), where status was previously left at SCORING. A submission will be marked "Finished" between prediction and scoring. It's also redundant for the non-HITL scoring path, which already sets FINISHED inside start().
• tasks.py: apparent rebase leftover. The second block starting if (submission.phase.competition.queue): re-sets submission.queue, overrides run_args["execution_time_limit"], and saves — duplicating/conflicting with the effective_queue logic just above it (and potentially clobbering a participant-group queue). Looks like old code accidentally reintroduced; it should be removed.
• HITL rejection may fire on validly configured competitions. The submission.queue is None checks in _send_to_compute_worker and _run_submission run before effective_queue = submission.queue or submission.phase.competition.queue is applied. A competition with a private queue configured at competition level but whose submission hasn't had queue copied yet would be wrongly failed. Check the effective queue instead.
• Worker blocked for up to 24h. wait_for_human_validation() blocks the Celery task synchronously (3s poll, 24h max). With default concurrency the worker processes nothing else while waiting. The docs even say "The compute worker is enable to run another submission while waiting" — which is both a typo and, as written, not what the code does. Either document the blocking behavior clearly or rework (e.g., requeue/resume).

Bugs / risks

• HTTP server reachability: it binds 127.0.0.1:8765 inside the worker container. The documented ssh -L 8765:127.0.0.1:8765 operator@<compute-worker> tunnels to the host's loopback, which won't reach the container unless it runs with network_mode: host. Verify against the actual compose setup.
• Hardcoded port 8765, no allow_reuse_address: a crashed run or two concurrent HITL submissions on one host → Address already in use → the submission crashes. Use port 0 (log the chosen port) or make it configurable, and set allow_reuse_address = True.
• send_detailed_results finally block references websocket: if it isn't initialized to None before the try, a failure before assignment raises NameError inside finally. Verify initialization.
• Fragile frontend detection: is_hitl_failure() matches status_details.indexOf('Human in the Loop'), but the message in _send_to_compute_worker says "Human-in-the-Loop (HITL)" — that failure won't match. Use a consistent marker or a dedicated status/flag rather than string-matching a human message.
• Sentinel files pollute results: hitl_approved sits in output_dir and will be included in the pushed output archive.
• validate_hitl_configuration() runs after prepare() — image pulled and data downloaded before rejecting. Cheap to move before prepare().
• Duplicated checks: the HITL/public-worker rejection exists in both _send_to_compute_worker and _run_submission with different messages. Keep one.
• Timeout message: missing space — "...without validation(submission 42)".
• start() unpacking: the nested if self.human_in_the_loop branches for task_results unpacking are brittle; a comment above still says "2 or 3 gathered tasks" which is now wrong. Consider tracking task names/indices instead of positional unpacking.

Style / conventions

• Mixed %-style and f-string logging in the same block; pick one (repo mostly uses f-strings).
• AWAITING_VALIDATION is ordered before SCORING in the worker's list and after it in the Django model — cosmetic, but confusing.
• Unrelated noise: removed # 20 minutes comment, added blank lines in watch_detailed_results, doc whitespace tweak in the setup page.
• Docs have typos ("exemple", "reviwed" in commit, "the check the scoring file", "is enable to run") and an incomplete sentence ("Each competition exposes the following option:" with nothing following before the image).

Test coverage

None added. At minimum: unit tests for the queue-gating logic in tasks.py (HITL + no queue → Failed; HITL + private queue → human_in_the_loop=True in run_args), serializer round-trip of the new field, and worker-side tests for validate_hitl_configuration and approve/reject/timeout paths (sentinel polling is easy to test with tmpdirs and a short timeout).