Skip to content

fix(benchmark): harden Toolathlon job lifecycle and result validation - #1656

Merged
Yunnglin merged 3 commits into
modelscope:mainfrom
git-jxj:fix/toolathlon-job-lifecycle
Aug 31, 2026
Merged

fix(benchmark): harden Toolathlon job lifecycle and result validation#1656
Yunnglin merged 3 commits into
modelscope:mainfrom
git-jxj:fix/toolathlon-job-lifecycle

Conversation

@git-jxj

@git-jxj git-jxj commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Hardens the Toolathlon private-mode client:

  • Retry transient status, task archive, and result download failures without cancelling the remote job.
  • Keep failed archive names pending locally in case the server no longer returns them.
  • Recognize the official timeout state, task_names response field, and average_success_rate score.
  • Handle relay exit and process cleanup without masking the original failure.
  • Pass the local API key through the relay environment instead of process arguments.
  • Reject missing or invalid result scores instead of treating them as zero.

The scoring and lifecycle changes move the benchmark evaluation version to v1.1.

@git-jxj
git-jxj marked this pull request as ready for review August 28, 2026 03:02
@Yunnglin

Copy link
Copy Markdown
Collaborator

Thanks for the thorough lifecycle hardening and regression coverage. The clean-relay-exit race fix, moving the API key out of argv, and reaping the relay process are all useful improvements.

I compared this implementation with the current official Toolathlon client/server code and found a few protocol and lifecycle differences that should be addressed before merge:

  1. Transient polling/download errors should not cancel the remote evaluation.

    The official client treats incremental download and status-poll failures as recoverable: it logs them and continues polling. In this PR, any exception from _download_completed_tasks(), _get_job_status(), or result downloading reaches the outer handler and immediately calls _cancel_job_safely().

    Consequently, a temporary network error, one HTTP 500, or a task archive download failure can cancel a multi-hour remote evaluation. Please keep polling/retrying transient service and download failures, and reserve cancellation for explicit local timeout, user interruption, relay failure that cannot recover, or another genuinely fatal condition.

  2. Please include the official timeout terminal state.

    The official status poller handles failed, timeout, and cancelled as terminal failures. _FAILED_JOB_STATES currently omits timeout, so a remotely timed-out job may be misreported as an unexpected relay exit and cancelled again, or continue polling until the local timeout. Please add timeout and cover it with a regression test that verifies no second cancellation occurs.

  3. Please align completed-task parsing with the official response schema.

    The official /get_completed_tasks endpoint returns:

    {"task_names": [...]}

    but _download_completed_tasks() currently reads only completed_tasks or tasks. This can silently disable incremental task-result downloads. Please support task_names and update the mock flow to exercise the official field.

  4. Please recognize the official aggregate score field and version the scoring change.

    The official eval_stats.json generator publishes the aggregate score as average_success_rate. _extract_accuracy() does not currently recognize it and instead depends on eval_res_all.jsonl as a fallback. Please support average_success_rate.

    Also, changing missing/invalid score handling from a silent 0.0 to an exception changes evaluation semantics. Per EvalScope's evaluation-versioning policy, Toolathlon should explicitly declare evaluation_version='v1.1'.

With these changes, the PR would preserve the useful hardening while remaining compatible with the official Toolathlon protocol and its retry behavior.

@git-jxj

git-jxj commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

Updated in fee18a0. Transient poll and download errors now retry without cancelling the job. Also added timeout, task_names, average_success_rate, and v1.1 handling. Failed archive downloads remain pending locally so they can be retried if the server no longer returns the task name.

@Yunnglin Yunnglin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The lifecycle hardening, protocol compatibility updates, score validation, and generated Toolathlon documentation are consistent and well covered. Thanks for addressing the review feedback.

@Yunnglin
Yunnglin merged commit a74a07d into modelscope:main Aug 31, 2026
3 checks passed
@git-jxj
git-jxj deleted the fix/toolathlon-job-lifecycle branch September 3, 2026 02:13
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.

2 participants