feat(miracle): add replay-driven LLM harness loop - #19
Open
gaoxiaobei wants to merge 28 commits into
Open
Annotations
10 errors and 2 warnings
|
Run blocking AI review
Process completed with exit code 1.
|
|
Run blocking AI review:
src/agentbench_frame/miracle/run_store.py#L119
The unchecked `config.agent` value is used as a path component. Absolute values or `..` segments can escape `--data-dir`%2C and quotes/newlines also produce malformed manually generated TOML%2C breaking process safety and the Results directory contract. Suggestion%3A Validate agent and run identifiers as single safe path components%2C reject absolute paths%2C separators%2C `.` and `..`%2C and serialize TOML strings with proper escaping rather than interpolation.
|
|
Run blocking AI review:
src/agentbench_frame/miracle/loop.py#L181
`total_steps` is computed by summing `MatchResult.rounds`. A Miracle round can contain multiple summon%2C move%2C attack%2C use%2C and end-round decisions%2C so the exported AgentBench step count materially underreports executed environment steps and makes cross-run accounting invalid. Suggestion%3A Count actual gameplay decisions from trace records%2C excluding card initialization and protocol-only frames%2C store the per-episode decision count%2C and sum that field for `total_steps`. Keep rounds as a separate metric.
|
|
Run blocking AI review:
skills/miracle-replay-reader/SKILL.md#L31
The replay Skill gives factually incorrect official scoring semantics%3A `score` is not remaining Miracle HP and 30000 is not merely a fixed win/loss marker. Official `ScoreListener` awards 1000 points per Miracle HP damaged plus enemy unit levels%2C so a score can exceed 30000; ties then add one point to camp 1. This Skill is included in every LLM request%2C so the error directly biases strategy analysis and result interpretation. Suggestion%3A Document the scoring formula from `StateSystem/Player.py` and `main.py` exactly%2C including unit-kill points%2C Miracle-damage points%2C destruction behavior%2C and the camp-1 tie-break increment.
|
|
Run blocking AI review:
src/agentbench_frame/miracle/run_store.py#L51
`charge_usage()` checks the token limit before updating counters. Because API usage has already been incurred when this method runs%2C an over-limit response is recorded as consuming zero tokens. Additionally%2C when an `LLMRequestError` carries over-limit usage%2C the second `charge_usage()` inside the exception handler raises a new uncaught `BudgetExceeded`%2C preventing Run finalization. Suggestion%3A Always record authoritative returned usage first%2C then report whether the resulting total exceeded the limit. Separate spent counters from pre-work admission checks%2C and ensure secondary budget exceptions in failure-accounting paths cannot escape finalization.
|
|
Run blocking AI review:
src/agentbench_frame/miracle/loop.py#L252
Candidate evaluation and IG run outside the iteration failure handler. A rollout or wall-budget exhaustion%2C match startup exception%2C trace error%2C or IG exception aborts `run_loop` without an iteration record%2C curves%2C or `summary.json`%2C violating the requirement to preserve incomplete and failed work. Suggestion%3A Wrap evaluation and IG as staged iteration work%2C persist per-episode and per-IG failures%2C and guarantee final curve/summary generation in a top-level `try/finally`. Budget exhaustion should stop further work but still finish the Run with an explicit exhausted status.
|
|
Run blocking AI review:
src/agentbench_frame/miracle/strategy_loader.py#L74
Validation calls `act()` only once and does not enforce the deterministic-policy assumption used by strict KL. A generated strategy using `random`%2C time%2C or process entropy is accepted%2C after which one sampled action is incorrectly interpreted as a delta distribution and differing samples are reported as infinite KL. Suggestion%3A Evaluate repeated fresh strategy instances on identical observations under recorded seeds and reject or explicitly mark nondeterministic policies as unsupported/missing. Isolated strategy processes should also receive deterministic RNG seeds and no entropy-bearing environment inputs.
|
|
Run blocking AI review:
src/agentbench_frame/miracle/decision_space.py#L84
The finite movement support does not match official legality%3A `_reachable()` omits the official `MAPBORDER()` obstacles and the enemy-adjacency obstruct zones from `get_obstructs_by_unit()`. It consequently labels some officially illegal moves as supported%2C which can turn an invalid action change into an `infinite` KL result instead of `missing`. Suggestion%3A Mirror the official path/reachability rules exactly%2C including map-border obstacles and obstruct-but-terminable cells%2C and add conformance tests comparing every enumerated move against the official legality implementation.
|
|
Run blocking AI review:
src/agentbench_frame/miracle/loop.py#L60
Replay evidence includes every `from_logic` observation regardless of `candidate_camp` and never pairs observations with the actual following `to_logic` action. With a small decision budget%2C a seat-1 evaluation can spend the entire budget on opponent observations while recording them as candidate decision reads%2C so the replay-driven context and accounting are scientifically misaligned. Suggestion%3A Filter observations to the episode's `candidate_camp`%2C align each observation with that camp's subsequent `to_logic` action in trace order%2C include both in evidence%2C and charge only the aligned candidate decisions actually included.
|
|
Run blocking AI review:
src/agentbench_frame/miracle/strategy_loader.py#L48
LLM-generated Python is executed with `exec_module()` inside the harness process. It can read the API-key environment variable%2C exfiltrate data%2C modify its supposedly immutable snapshots%2C terminate or hang the harness%2C and include secrets in exceptions that are then persisted. The thread-based decision timeout does not terminate running code and does not cover imports or IG calls. Suggestion%3A Load and execute every generated strategy in a killable isolated subprocess with an allowlisted environment that excludes API credentials%2C read-only strategy inputs%2C restricted filesystem/network access%2C and hard resource/time limits. Return actions over a serialized IPC boundary and sanitize all child errors before persistence.
|
|
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/miracle/match.py#L63
Replay filenames use only second-resolution time%2C tag%2C and seed. Concurrent or repeated same-tag matches started within one second target the same `.mrc` and trace files%2C causing truncation/interleaved official output and corrupted scientific artifacts. Suggestion%3A Add a collision-resistant component such as nanoseconds plus a random/UUID suffix%2C or reserve filenames atomically with exclusive creation before launching official logic.
|
background
wait
wait-all
cancel
parallel
Loading