feat: Add AntWar2 - #2
Open
NemotionalDamage wants to merge 3 commits into
Open
Annotations
10 errors and 3 warnings
|
Run blocking AI review:
src/agentbench_frame/population/store.py#L164
`import_player_zip` never reads%2C extracts%2C validates%2C or copies `zip_path`. It constructs an `ExternalStrategy` without a script or command%2C causing the reference hold bot to be saved instead of the imported ladder player and silently benchmarking the wrong strategy. Suggestion%3A Securely unpack the archive into a temporary directory%2C reject traversal/symlink entries%2C validate a declared entry point%2C copy the actual bot artifacts into the population%2C and fail rather than substituting the reference bot.
|
|
Run blocking AI review:
src/agentbench_frame/strategy/external_strategy.py#L111
`ExternalStrategy.timeout` is never enforced%3A `_recv` performs blocking pipe reads and an unresponsive bot can hang a worker indefinitely. The bot command is also loaded from strategy metadata and launched directly with the host environment and filesystem access%2C allowing an untrusted ladder strategy to read CI secrets or modify the checkout. Suggestion%3A Run external bots in an isolated process/container with a scrubbed environment%2C restricted filesystem/network and resource limits; enforce per-request deadlines and terminate the entire process group on timeout or protocol violations.
|
|
Run blocking AI review:
src/agentbench_frame/entries/rl_entry.py#L195
RL cost accounting double-counts every environment interaction by copying the accumulated interaction count into a second GPU entry. It also records total wall-clock time as `gpu_seconds` whenever Torch is installed%2C even when training runs on CPU%2C so the required cost summary is materially inaccurate. Suggestion%3A Set interactions to zero on the GPU-only cost entry%2C record GPU time only when CUDA is actually used with an appropriate timer%2C and retain wall time solely in `wall_seconds`.
|
|
Run blocking AI review:
src/agentbench_frame/entries/eval_entry.py#L113
Headline evaluation fields are added only after `run.finish()` has already built and written `summary.json`. The same pattern occurs in iteration%2C and RL adds `torch_available` and `smoke_mode` after finishing. Returned in-memory dictionaries contain these fields%2C but persisted run artifacts omit rankings%2C best-strategy history%2C and mode metadata needed by CI and later analysis. Suggestion%3A Attach result fields to the Run before calling `finish`%2C extend `finish` to accept final metrics%2C or atomically rewrite the finalized summary after merging results in all three entry points.
|
|
Run blocking AI review:
src/agentbench_frame/entries/eval_entry.py#L69
Evaluation opponent selection can silently evaluate the wrong population. In compare mode%2C any nonempty visible population replaces the explicitly requested `strategy_ids`%2C making results depend on prior contents of the working directory; in single mode the target is not removed from split-derived opponents%2C so its self-play result can be included in the reported average. Suggestion%3A Make explicit strategy IDs authoritative or define an explicit intersection/selection mode%2C always remove the target from single-mode opponents%2C and record exact strategy versions or hashes used.
|
|
Run blocking AI review:
src/agentbench_frame/entries/rl_entry.py#L137
Even after fixing the wrapper crash%2C the advertised PPO configuration is not honored%3A `ppo_epochs` is never used%2C every critic transition is trained against `returns.mean()` rather than its corresponding return%2C and the supplied seed never seeds Python%2C NumPy%2C Torch%2C policy initialization%2C or action sampling. Identical recorded configurations therefore produce different runs and the value update is scientifically incorrect. Suggestion%3A Seed all RNGs before constructing the policy%2C derive deterministic episode seeds%2C implement the requested number of PPO epochs%2C and pair each value prediction with its own return/advantage target.
|
|
Run blocking AI review:
src/agentbench_frame/entries/rl_entry.py#L185
The non-smoke RL path passes the `RLStrategy` wrapper to `_ppo_episode`%2C although that function calls `.parameters()` and `.forward()` and requires the underlying Torch policy. Training raises `AttributeError` before completing the first episode. Suggestion%3A Pass `strategy.policy` to `_ppo_episode` and add a Torch-enabled non-smoke integration test.
|
|
Run blocking AI review:
src/agentbench_frame/arena/payoff.py#L99
`PayoffMatrix.add_strategy` ignores its `strategy` argument and calls `update` with the original mapping. If the new ID is not already duplicated into `strategies`%2C every pair involving it is skipped and the advertised incremental evaluation reports zero games. Suggestion%3A Create an evaluation mapping containing `sid%3A strategy` before calling `update`%2C and test adding a strategy that is not already present in the caller's mapping.
|
|
Run blocking AI review:
src/agentbench_frame/env/antwar2_env.py#L286
Actions returned by `get_legal_actions()` cannot be passed back to `step()`. The method flattens each bundle to a list of integers%2C while `_to_operations` only recognizes an outer list containing operation lists; a selected legal action such as `[11%2C 2%2C 9]` is therefore parsed as no operations and becomes a hold. Suggestion%3A Use one round-trippable action representation for both APIs%2C such as nested operation-token lists%2C or teach `_to_operations` to parse the exact flattened encoding including multi-operation bundles.
|
|
Run blocking AI review:
src/agentbench_frame/strategy/base.py#L104
Strategy manifests are written before subclass artifacts update `meta.extra`. Consequently saved rule strategies lose their policy and reload as greedy%2C RL strategies do not record/load `policy.pt` and reload with fresh random weights%2C and external strategies do not record their generated script. This silently changes evaluated strategies and invalidates population results. Suggestion%3A Save artifacts before serializing the final manifest%2C preferably into a temporary directory followed by an atomic rename%2C and add round-trip tests for every strategy kind.
|
|
Complete job
Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24: actions/checkout@v4. For more information see: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
|
|
Run blocking AI review:
src/agentbench_frame/population/store.py#L78
Explicit retrieval of the current population version fails. Registration snapshots only the previous version%2C but `get(strategy_id%2C version=current_version)` always looks under `versions/<version>`%2C which does not exist until a later registration occurs. Suggestion%3A Return the root strategy when the requested version equals the current manifest version%2C or snapshot every newly registered version and make the root a latest-version pointer.
|
|
Run blocking AI review:
src/agentbench_frame/cli.py#L141
When neither `tomllib` nor `tomli` is installed%2C the CI data checker treats substring presence as TOML/schema validation. Commented fields%2C fields in the wrong table%2C empty values%2C and malformed TOML can all pass%2C allowing invalid run data into the dataset. Suggestion%3A Declare `tomli` for supported pre-3.11 Python versions or fail the check with a clear dependency error; do not substitute textual substring checks for parsing.
|
background
wait
wait-all
cancel
parallel
Loading