|
| 1 | +# 📋 Summary |
| 2 | + |
| 3 | +Adds `[target].language` so the runner can evaluate `facebook/omnilingual-gaia2` — the |
| 4 | +published multilingual Gaia2 dataset — through the existing HuggingFace auto-download path. |
| 5 | + |
| 6 | +Before this change, running the dataset meant doing the data plumbing by hand: `hf download`, |
| 7 | +then `mt/scripts/data/parquet_to_json.py` to convert the parquet shards, then pointing |
| 8 | +`[target].dataset_root` at the converted tree. Three manual steps and a hardcoded language |
| 9 | +directory before anything ran. |
| 10 | + |
| 11 | +The gap turned out to be narrow. The dataset's parquet columns are `scenario_id` + `scenario` |
| 12 | +and its HF split is `test` — exactly what `download_hf_dataset()` already reads. The only |
| 13 | +mismatch was that the runner derived the HF *config* name from the split alone, while this |
| 14 | +dataset publishes 40 configs named `{language}_{split}` (10 languages × 4 capabilities). |
| 15 | + |
| 16 | +Now: |
| 17 | + |
| 18 | +```toml |
| 19 | +[target] |
| 20 | +dataset = "facebook/omnilingual-gaia2" |
| 21 | +language = "spa_Latn" |
| 22 | +``` |
| 23 | + |
| 24 | +Omitting `language` leaves every existing code path byte-for-byte unchanged, including the |
| 25 | +on-disk cache layout, so current users see no difference. |
| 26 | + |
| 27 | +# 🎯 Type of Change |
| 28 | + |
| 29 | +- [ ] 🐛 Bug fix (non-breaking change which fixes an issue) |
| 30 | +- [x] ✨ New feature (non-breaking change which adds functionality) |
| 31 | +- [ ] 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected) |
| 32 | +- [x] 📚 Documentation update |
| 33 | +- [ ] 🔧 Refactoring (no functional changes, no api changes) |
| 34 | +- [ ] ⚡ Performance improvement |
| 35 | +- [x] 🧪 Test addition or improvement |
| 36 | +- [ ] 🔨 Build/CI changes |
| 37 | + |
| 38 | +Also fixes one latent bug found along the way (see *Incidental fixes*). |
| 39 | + |
| 40 | +# 🏗️ Meta Agents Research Environments Components Affected |
| 41 | + |
| 42 | +- [ ] **Environment** (`are/simulation/environment.py`, `are/simulation/core/`, environment setup/configuration) |
| 43 | +- [ ] **UI/GUI** (`are/simulation/gui/`, web interface, client-side components) |
| 44 | +- [ ] **Scenarios** (`are/simulation/scenarios/`, scenario definitions, scenario runner) |
| 45 | +- [ ] **Agents** (`are/simulation/agents/`, agent implementations) |
| 46 | +- [x] **Documentation** (README, docs, comments) |
| 47 | +- [ ] **Build/CI** (GitHub Actions, Docker, dependencies) |
| 48 | + |
| 49 | +> Note: the template's component list predates `gaia2-cli/`. This PR is confined to |
| 50 | +> **`gaia2-cli/runner`** (`config.py`, `hf_dataset.py`, `cli.py`, tests, one example) plus |
| 51 | +> README updates. Nothing under `are/simulation/` is touched. |
| 52 | +
|
| 53 | +# 🔗 Related Issues |
| 54 | + |
| 55 | +<!-- Fill in if there is a tracking issue --> |
| 56 | + |
| 57 | +Follows #104, which added Omnilingual-GAIA2 translation and the multilingual judge prompts. |
| 58 | + |
| 59 | +# 🧪 Testing Strategy |
| 60 | + |
| 61 | +- [x] **Unit Tests**: Added/updated unit tests |
| 62 | +- [ ] **Integration Tests**: Tested integration with other components |
| 63 | +- [x] **Manual Testing**: Manually verified functionality |
| 64 | +- [ ] **Scenario Testing**: Ran specific scenarios to validate changes |
| 65 | +- [ ] **GUI Testing**: Tested web interface (if applicable) |
| 66 | +- [x] **Regression Testing**: Verified existing functionality still works |
| 67 | + |
| 68 | +## Test Details |
| 69 | + |
| 70 | +**Unit tests — 249 pass in `gaia2-cli/runner`** (was 225; +24 new). |
| 71 | + |
| 72 | +New `tests/test_hf_dataset.py` (12 tests) stubs `datasets.load_dataset` via `sys.modules`, so |
| 73 | +it needs no network. It covers: config name built as `spa_Latn_search`; files materialized to |
| 74 | +`<cache>/<language-keyed>/search/`; **the no-language path keeping the legacy cache dir name |
| 75 | +and bare config names** (backward-compat guard); split defaulting with and without a language; |
| 76 | +per-language cache isolation; the cache-hit short circuit; no partial directory left after a |
| 77 | +failed config *or* a `KeyboardInterrupt`; and the error listing available configs without |
| 78 | +masking the original failure. |
| 79 | + |
| 80 | +`tests/test_run_config.py` (+11) covers `language` round-tripping, `"all"` excluding `time`, |
| 81 | +rejection of `time`/`scenario`/`dataset_root`/malformed codes, the `--dry-run` output, and |
| 82 | +`run-dataset --language` forwarding and metadata. `tests/test_cli_judge_config.py` (+1) pins |
| 83 | +`language` into `run_config.json`. |
| 84 | + |
| 85 | +`ruff format --check .` and `ruff check` are clean repo-wide (428 files). |
| 86 | + |
| 87 | +**Manual — a full pass@3 run completed on a GPU node** with podman, OpenClaw, a local vLLM |
| 88 | +serving `Qwen/Qwen3.6-27B` as the agent (port 8000) and `gpt-oss-120b` as the judge with |
| 89 | +`reasoning_effort: low` (port 8001): |
| 90 | + |
| 91 | +- Cold download resolved the right config and materialized real data: |
| 92 | + ``` |
| 93 | + gaia2_runner.hf_dataset: Downloading HF dataset facebook/omnilingual-gaia2 (configs: ['spa_Latn_search']) |
| 94 | + gaia2_runner.hf_dataset: spa_Latn_search: downloading ... |
| 95 | + gaia2_runner.hf_dataset: spa_Latn_search: 160 scenarios |
| 96 | + ``` |
| 97 | + A second invocation logged `Using cached dataset at ...` instead of re-downloading. |
| 98 | +- **Cache layout** is as the design requires — bare split directory names under a |
| 99 | + language-keyed root, 160 scenarios each: |
| 100 | + `~/.cache/gaia2/hf_datasets/facebook_omnilingual-gaia2_spa_Latn/{execution,search}/`. |
| 101 | +- **pass@3 completed** across three runs, with the aggregation and per-split breakdown |
| 102 | + both working: |
| 103 | + ``` |
| 104 | + Per split: |
| 105 | + execution: 1/1 passed, 0/1 failed, 0/1 errors (100.0%) |
| 106 | + ... |
| 107 | + avg@3: 33.3% ± 47.1% |
| 108 | + pass@3: 100.0% ± 0.0% |
| 109 | + ``` |
| 110 | + The `execution` label confirms `_infer_result_split` still resolves the split correctly |
| 111 | + from the language-keyed cache root. |
| 112 | +- **`run_config.json` records the new field**, and notably carries no `time` split: |
| 113 | + ```json |
| 114 | + "dataset": "facebook/omnilingual-gaia2", |
| 115 | + "dataset_cache_dir": ".../hf_datasets/facebook_omnilingual-gaia2_spa_Latn", |
| 116 | + "language": "spa_Latn", |
| 117 | + "splits": ["execution"] |
| 118 | + ``` |
| 119 | +- Error path: a nonexistent language produced a `UsageError` naming the config it tried, |
| 120 | + listing all 40 available configs, and leaving no partial cache directory. |
| 121 | + |
| 122 | +**Not verified** — worth a reviewer's eye: |
| 123 | + |
| 124 | +- Only `spa_Latn` was exercised, on `execution` and `search`. The other nine languages are |
| 125 | + asserted to exist in the published repo's config list but were not run. |
| 126 | +- Scenario pass/fail rates were not the object here — a single scenario over three runs says |
| 127 | + nothing about model quality, only that the loop works end to end. |
| 128 | + |
| 129 | +# 🤖 AI Usage Declaration |
| 130 | + |
| 131 | +Written with Claude Code (Opus 5) throughout: codebase exploration, the implementation plan, |
| 132 | +the code and tests, and this description. Every claim above was verified by running the |
| 133 | +commands rather than asserted from the model's reading of the code — including four bugs the |
| 134 | +review caught that were confirmed by reading the affected lines before being fixed. Author |
| 135 | +reviewed and ran the end-to-end evaluation. |
| 136 | + |
| 137 | +# 📸 Screenshots/Recordings |
| 138 | + |
| 139 | +N/A — no UI changes. |
| 140 | + |
| 141 | +# ⚠️ Breaking Changes |
| 142 | + |
| 143 | +None. `language` is optional, and every branch it touches falls back to current behaviour when |
| 144 | +it is absent: |
| 145 | + |
| 146 | +- The cache directory suffix is appended **conditionally**, so existing populated caches under |
| 147 | + `~/.cache/gaia2/hf_datasets/` stay valid and no one re-downloads. |
| 148 | +- HF config names are unprefixed without a language, so non-omnilingual datasets are unaffected. |
| 149 | +- Split defaults still expand to all five `CANONICAL_SPLITS` (including `time`) with no language. |
| 150 | + |
| 151 | +A dedicated regression test pins each of these. |
| 152 | + |
| 153 | +## Incidental fixes |
| 154 | + |
| 155 | +- **Partial cache directories after a failed or interrupted download.** `split_dir.mkdir()` |
| 156 | + ran *before* `load_dataset`, and the cache checks test only "exists and is non-empty" — so a |
| 157 | + failed fetch left a directory that the next run accepted, silently resolving zero (or a |
| 158 | + truncated set of) scenarios for that split. The download is now wrapped so the directory is |
| 159 | + removed on failure, and the error names the config and lists what is available. The handler |
| 160 | + catches `BaseException`, not `Exception`, specifically so that a Ctrl-C mid-download also |
| 161 | + cleans up rather than leaving a half-populated split cached — `KeyboardInterrupt` is |
| 162 | + re-raised untouched. Pre-existing; not specific to this feature. |
| 163 | + |
| 164 | +## Known limitations, not addressed here |
| 165 | + |
| 166 | +Three pre-existing issues surfaced while testing. All are out of scope; each is worth its own |
| 167 | +issue. |
| 168 | + |
| 169 | +1. **`openai/`-prefixed model names are unreachable for the judge.** `judge/engine.py:60` |
| 170 | + prefixes `openai/` for `openai-compat` providers so litellm uses the OpenAI transport, and |
| 171 | + skips that when the name already starts with `openai/`. But litellm *strips* that prefix as |
| 172 | + provider routing — so a model genuinely served as `openai/gpt-oss-120b` goes out as |
| 173 | + `gpt-oss-120b` and 404s. The workaround is `model = "openai/openai/gpt-oss-120b"`, which is |
| 174 | + what the example config uses and what therefore lands verbatim in run metadata. A proper fix |
| 175 | + needs an explicit "this is the wire name" escape hatch. |
| 176 | +2. **Ctrl-C leaves orphaned containers.** Containers are started detached without `--rm` |
| 177 | + (`launcher.py:440`) and removed in a `finally` inside the worker thread |
| 178 | + (`runner.py:350`). `KeyboardInterrupt` reaches only the main thread, so those blocks never |
| 179 | + run and the containers keep driving their agent loops against the model server. Cleanup is |
| 180 | + manual: `podman rm -f $(podman ps -aq --filter name=gaia2-)`. A SIGINT handler that stops |
| 181 | + live containers would fix it. |
| 182 | +3. **`run-dataset` in HF mode globs the entire cache directory** rather than restricting to the |
| 183 | + selected splits, unlike `run-config`. With per-language cache dirs this stays |
| 184 | + language-correct, but `--language spa_Latn --splits search` will still pick up other splits |
| 185 | + previously cached for that language. |
| 186 | + |
| 187 | +# 📋 Checklist |
| 188 | + |
| 189 | +- [x] My code follows the project's style guidelines |
| 190 | +- [x] I have performed a self-review of my own code |
| 191 | +- [x] I have commented my code, particularly in hard-to-understand areas |
| 192 | +- [x] I have made corresponding changes to the documentation |
| 193 | +- [x] My changes generate no new warnings |
| 194 | +- [x] I have added tests that prove my fix is effective or that my feature works |
| 195 | +- [x] New and existing unit tests pass locally with my changes |
| 196 | +- [x] Any dependent changes have been merged and published |
0 commit comments