(3/n) Introduce declarative YAML task definitions - #81
Conversation
geoalgo
left a comment
There was a problem hiding this comment.
LGTM overall, I have left some comments.
| args = list(sys.argv[1:] if argv is None else argv) | ||
| if args[:1] == ["tasks"]: | ||
| from judgearena.tasks.cli import run_task_command | ||
|
|
||
| run_task_command(args[1:]) | ||
| return | ||
|
|
||
| try: | ||
| cfg = build_run_config(argv) | ||
| cfg = build_run_config(args) |
There was a problem hiding this comment.
Can you add a comment here on the rationale? Why doing run_task_command only when "tasks" is passed as CLI?
It is not obvious to understand what this code does.
There was a problem hiding this comment.
Hi David,
This function basically changes the execution flow for the main entry point. If judgearena tasks if provided we can execute arbitrary command like --list to see all the available tasks (or validate any) - if not provided simply execute the workflow. I think we can discard this if you think we dont need such functionality but for completeness I did implement it anyways.
There was a problem hiding this comment.
Ok I would suggest something like this to improve readability:
is_task_cli = args[:1] == ["tasks"]
if is_task_cli:
# handle the case where we run `judgearena tasks ARGS`
from judgearena.tasks.cli import run_task_command
run_task_command(args[1:])
else:
# generic case for other commands
cfg = build_run_config(args)which is more readable (but still a bit hacky).
| self._adapters = adapters or AdapterCatalog() | ||
| self._tasks: dict[str, ResolvedTaskSpec] | None = None | ||
|
|
||
| def get(self, task_id: str) -> ResolvedTaskSpec: |
There was a problem hiding this comment.
I think it would be better to avoid a registry that basically reimplement most dictionary functions.
We can get that by having:
load_tasks(...) -> dict[str, ResolvedTaskSpec]
and just dealing with the dictionary in downstream code.
We save 100 LOC and also the need to check an API which is really just a dict interface.
There was a problem hiding this comment.
I agree, this is an over-complication that is added and can easily be avoided. If we ever need such functionality we can change it without a problem in the future. I will change it
| return merged | ||
|
|
||
|
|
||
| class TaskLoader: |
There was a problem hiding this comment.
See my other comment, we just need one function to load all tasks which returns a dict all of those can be in the same file.
There was a problem hiding this comment.
I thing merging these two into a single registry would make the registry larger, as function wise, registry.py handles resolving the tasks by calling the loader. However as registry is the only script that calls TaskLoader, it is also OK to put them all in.
However, compared to TaskRegistry which is basically a dict, I feel like TaskLoader class is more graceful. Instead of providing the functions like load,..., discover, it just provides a single object loader which just handles all the things we want to do.
Happy to convert it to functions or merge it with registry if you think that is more cleaner and readable approach
| from judgearena.tasks.schema import HuggingFaceDatasetSource, ResolvedTaskSpec | ||
|
|
||
|
|
||
| def download_task_sources(task: ResolvedTaskSpec, local_dir: Path) -> None: |
There was a problem hiding this comment.
should we keep only one from download_all and this one?
There was a problem hiding this comment.
download_task_sources is an adapter from newer version of reading revision from tasks.yaml and dowloading the specified task. IT is basically used in download_hf which then download_all calls as:
if is_arena_hard_dataset(dataset):
download_arena_hard(dataset=dataset, local_tables_path=local_path_tables)
else:
download_hf(name=dataset, local_path=local_path_tables)So I think at this point we should not consodliate it in the current PR. As we add more tasks and complete the entire stacked PRs where we also done with adding the arena as tasks (up to #86 ), than a single function can download the entire available tasks, as well as specific ones (also another advantage of our classed approach, now we wont be needing if statements as a class will handle the downloading by itself).
My suggestion is to let it stay like this at the current PR, and let the other PR's in the stack handles this change after we added all the tasks.
There was a problem hiding this comment.
Makes sense, perhaps we just add support later to define a custom download function in case its needed when we cant rely on just dowload_hf.
0ca3154 to
14bb329
Compare
Define validated YAML task specifications with discovery commands and package AlpacaEval as the first task.
Use registered definitions for runner, baseline, prompt, and judge defaults while retaining fallbacks for unmigrated tasks.
Use task-declared source revisions and field mappings for instruction and pre-generated output tables.
Store compact task versions and resolved YAML hashes in run metadata without embedding the full definition.
- cli.py: comment the reason tasks subcommands are routed before build_run_config - paths.py: remove the dead duplicate download_hf (shadowed by utils/io.py) and its now-unused imports - utils/io.py: use if/else and comment the packaged-task vs legacy dataset branches
Replace the TaskRegistry class and its dict-facade API (get/find/list/ validate_all) with a cached load_tasks() -> dict[str, ResolvedTaskSpec] and dict access at call sites. Drop the ValidationReport, TaskSummary, and UnknownTaskError wrappers; the CLI now formats the unknown-task message and counts with len(). get_packaged_task stays as the thin load_tasks().get() helper used across the codebase. Addresses review comments #3 and #6.
Route the 'tasks validate' confirmation through logger.info (status, to stderr) and configure logging on the tasks CLI path, keeping print for list/show which emit results to stdout.
14bb329 to
5297408
Compare
Summary
Task-specific configuration is currently distributed across Python constants, dataset loaders, baseline mappings, prompt
registries, and runner conditions. Adding or changing a task therefore requires modifying several unrelated modules.
This PR introduces declarative YAML task definitions, inspired by the task packaging approach used by lm-evaluation-
harness.
AlpacaEval is migrated as the first packaged task. Existing benchmarks continue using their current fallback behavior
until they are migrated in later PRs.
Task definitions
A task YAML describes the stable contract of a benchmark:
The main sections are:
The protocol does not contain implementation code. It references registered components such as pairwise,
judgearena_tables, pairwise_preference, and pairwise_win_rate.
Main components
The new task package contains three main responsibilities:
The schema defines the supported dataset sources, baseline strategies, judge settings, and scoring configuration.
Unknown fields are rejected.
The loader reads YAML safely, resolves optional _base.yaml inheritance, and calculates hashes for the source and
resolved definitions.
The registry discovers packaged tasks by ID and verifies that every referenced runner, dataset adapter, prompt,
parser, and scorer exists.
The CLI allows users to inspect and validate task definitions before running them.
Runtime behavior
When a task is executed, JudgeArena resolves it through the registry:
Task ID
-> load and validate task YAML
-> resolve dataset source and field mappings
-> apply task protocol defaults
-> select the registered runner
-> generate, judge, and score
Task definitions provide benchmark defaults, while users can still configure experiment-specific values.
The effective precedence is:
CLI flags -> run config YAML -> task-defined defaults -> framework defaults
For example:
Additional changes
result.