From a162fc773fe4d6e38840f67cd0d35e1e5f04c23c Mon Sep 17 00:00:00 2001 From: Aling33 <3489986145@qq.com> Date: Wed, 14 Oct 2026 06:14:22 +0800 Subject: [PATCH 01/19] feat: import OSWorld LibreOffice tasks as computer-use environments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #24. scripts/import_osworld.py imports 16 OSWorld LibreOffice Calc and Writer tasks as computer-use environments (task.md, setup.sh, verifier.py, variants.json, metadata.json, assets/). OSWorld is Apache-2.0; the imported configs, instructions and files keep its license. The verifier ports each task's own evaluator into a state check over the saved file — compare_table rules or a reference-text compare, never a screenshot. The shared _checks.py reads workbooks (inline strings, merges, number formats) and documents with the standard library only, and cross-checks rule-by-rule against OSWorld's evaluator: 1967 checks, 0 mismatches. Rules the starting file already satisfies become guards, not progress checks, so an untouched or vanished file scores 0 while a partial solution earns the share of its rules it has done (OSWorld's evaluator was all-or-nothing). deepgym check now covers computer-use folders: it demanded a reference_solution.* no desktop env ships and then replayed a reference a local session can't run. It accepts the folder, and reports a skipped reference_passes when there is no episode to replay or no desktop (the same skips deepgym qa already makes); lint_env reads a computer-use verifier the way the loader prepends it. tests/office_fixtures.py builds .xlsx/.docx fixtures with the standard library; tests/test_import_osworld.py covers the importer, the ported evaluator and the guards split (32 tests). --- deepgym/CHANGELOG.md | 4 + deepgym/scripts/import_osworld.py | 536 ++++++++++++++++ .../src/deepgym/envs/computer_use/_checks.py | 469 +++++++++++++- deepgym/src/deepgym/factory/check.py | 33 +- deepgym/tests/office_fixtures.py | 109 ++++ deepgym/tests/test_factory.py | 42 ++ deepgym/tests/test_import_osworld.py | 573 ++++++++++++++++++ 7 files changed, 1759 insertions(+), 7 deletions(-) create mode 100644 deepgym/scripts/import_osworld.py create mode 100644 deepgym/tests/office_fixtures.py create mode 100644 deepgym/tests/test_import_osworld.py diff --git a/deepgym/CHANGELOG.md b/deepgym/CHANGELOG.md index 9a0f1c76..5ed57bb7 100644 --- a/deepgym/CHANGELOG.md +++ b/deepgym/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- **OSWorld computer-use tasks:** `scripts/import_osworld.py` imports 16 of OSWorld's (Apache-2.0) LibreOffice Calc and Writer tasks as computer-use environments, one folder per task. The agent gets the task's starting file open in Calc or Writer and saves it; `verifier.py` scores the file it saved — with the task's own `compare_table` rules, ported into the shared `_checks.py` (workbook and document readers, the rule engine, pandas' rounding) and cross-checked rule-by-rule against OSWorld's own evaluator, or its reference text for the Writer tasks — never a screenshot. Each rule the starting file already satisfies is a guard rather than a progress check, so an untouched file or a vanished one scores 0 while a partial solution earns the fraction of its rules it has done (the evaluator was all-or-nothing). The importer downloads the task configs and files over the GitHub contents API and the hf-mirror host when the usual hosts are blocked. `deepgym check` now runs on a computer-use folder: it demanded a `reference_solution.*` every desktop env lacks and replayed a reference a local session can't run; a computer-use folder now passes the files check and reports a skipped `reference_passes` instead — no episode to replay, or no desktop (the skips `deepgym qa` already makes; `--mode daytona` replays the built-ins' episodes) — and `lint_env` reads a computer-use verifier the way the loader prepends it. + ## 0.5.0 (2026-10-14) - **Sessions: no verdict only for failures that are ours for certain.** Once the agent has acted, a failure is its judged 0 (`policy_sandbox_failure`) unless it is ours for certain: a network, provider or auth failure (HTTP 401 and 403 now count as infrastructure in `is_infrastructure_failure`), a verifier that couldn't run (its upload or launch failed) or timed out before it read the agent's output, an interrupt, or E2B or Modal retiring the sandbox at the end of its one-hour lifetime (new `SandboxExpiredError`, before a `SandboxDiedError`) unless the agent's own actions took at least half the session's time. A failure the rule can't read is the agent's 0. The rule is `deepgym.session.failure_result`, which verl's `deepgym_agent` loop and Harbor's agent phase also use; before, a 401 uploading verifier code or a verl final verifier that timed out before reading anything scored 0, and a Harbor agent that replaced its work directory with a link, grew it past the archive cap, made packing outrun its deadline, killed its sandbox or spent most of the provider lifetime in its script got no verdict; in Harbor any failure once the agent's script has started is now its 0 unless it is ours for certain. Session verifiers now get the consulted mark single runs use, embedded in the verifier's own script rather than on a command line: a step or final verifier that times out after reading the agent's output (a FIFO it left, say) is a judged 0 with `solution_timeout` (for a step, its reward, and the episode goes on), and so is a verifier that is killed, or gives no verdict, after reading it (`solution_error`), unless the agent's code failed to launch. Each verifier also prints a start line first, so a timeout whose output the provider lost is a judged 0 rather than passing for one that never read anything. Listing the agent's folder, or starting a program in it or on one of its files, now counts as reading its output, and the mark is written to the stdout file itself, so capturing `sys.stdout` can't hide it, and cut from what the verifier printed exactly, so a line it fell inside reads whole. Daytona enforces command deadlines in the sandbox, so a timed-out command keeps what it printed. A FIFO the agent leaves where the harness writes is refused at once instead of hanging the write, the local backend shows output that isn't UTF-8 instead of ending the episode, and E2B, Daytona and Docker/Apple uploads to a stopped sandbox raise `SandboxDiedError`. `deepgym_solution.run_solution` prints the consulted mark before it releases the solution, so a solution that dies the moment it starts can't pass for one that never ran. diff --git a/deepgym/scripts/import_osworld.py b/deepgym/scripts/import_osworld.py new file mode 100644 index 00000000..d9e0f183 --- /dev/null +++ b/deepgym/scripts/import_osworld.py @@ -0,0 +1,536 @@ +#!/usr/bin/env python3 +"""Import OSWorld tasks into DeepGym computer-use environments. + +Downloads the task configs (plus the files the tasks start from and their reference +files) for a subset of OSWorld's LibreOffice tasks and writes one DeepGym +computer-use environment folder per task: + + - task.md the task's instruction, plus where the file is and how to save + - setup.sh decodes the starting file and opens it in LibreOffice Calc/Writer + - verifier.py scores the saved file with the task's own compare_table rules + - variants.json the rules (progress and guards), the reference data, result path + - metadata.json type, domain, tags and step budget + - assets/.b64 the starting file, staged into the session work directory + +The shared ``_checks.py`` (the readers and rule engine the verifier needs) is copied +next to the environments, and ``registry.json`` indexes them all, so the imported +tasks load through ``load_environment()`` like any other environment. Each verifier +scores the file the task saves, never the screen. + +OSWorld is Apache-2.0 (https://github.com/xlang-ai/OSWorld); the imported task +configs, instructions and files keep that license. + +Usage: + python scripts/import_osworld.py +""" + +from __future__ import annotations + +import base64 +import importlib.resources +import importlib.util +import json +import re +import shutil +import tempfile +import urllib.error +import urllib.parse +import urllib.request +from pathlib import Path +from types import ModuleType +from typing import NamedTuple + +_OSWORLD_REPO = 'xlang-ai/OSWorld' +_TASK_URL = f'https://raw.githubusercontent.com/{_OSWORLD_REPO}/main/evaluation_examples/examples' +_TASK_API_URL = ( + f'https://api.github.com/repos/{_OSWORLD_REPO}/contents/evaluation_examples/examples' +) + +# (task id, OSWorld domain, environment name): the LibreOffice tasks whose evaluator +# scores the file the task saves, so a DeepGym verifier can check them with no live +# desktop. Chrome tasks are excluded: the desktop is locked down and offline. +TASKS = ( + ('42e0a640-4f19-4b28-973d-729602b5a4a7', 'libreoffice_calc', 'Sum revenue and expenses'), + ('abed40dc-063f-4598-8ba5-9fe749c0615d', 'libreoffice_calc', 'Unique names in order'), + ('357ef137-7eeb-4c80-a3bb-0951f26a8aff', 'libreoffice_calc', 'Total pay from hours and rate'), + ('1273e544-688f-496b-8d89-3e0f40aa0606', 'libreoffice_calc', 'Copy a column to a new sheet'), + ('1e8df695-bd1b-45b3-b557-e7d599cf7597', 'libreoffice_calc', 'Add a profit column'), + ('4de54231-e4b5-49e3-b2ba-61a0bec721c0', 'libreoffice_calc', 'Fill a ramp and join values'), + ('4172ea6e-6b77-4edb-a9cc-c0014bd1603b', 'libreoffice_calc', 'Loan maturity dates'), + ('37608790-6147-45d0-9f20-1137bb35703d', 'libreoffice_calc', 'Split names into columns'), + ('04d9aeaf-7bed-4024-bedb-e10e6f00eb7f', 'libreoffice_calc', 'Balance sheet changes'), + ('26a8440e-c166-4c50-aef4-bfb77314b46b', 'libreoffice_calc', 'Monthly sales totals'), + ('035f41ba-6653-43ab-aa63-c86d449d62e5', 'libreoffice_calc', 'Gross profit column'), + ('1d17d234-e39d-4ed7-b46f-4417922a4e7c', 'libreoffice_calc', 'Merged-cell summary sheet'), + ('0cecd4f3-74de-457b-ba94-29ad6b5dafb6', 'libreoffice_calc', 'Rename, copy, reorder sheets'), + ('adf5e2c3-64c7-4644-b7b6-d2f0167927e7', 'libreoffice_writer', 'Add a citation and reference'), + ('d53ff5ee-3b1a-431e-b2be-30ed2673079b', 'libreoffice_writer', 'Lowercase the document'), + ('e528b65e-1107-4b8c-8988-490e4fece599', 'libreoffice_writer', 'Capitalize every word'), +) + +_RULE_TYPES = frozenset({'sheet_name', 'sheet_data', 'check_cell'}) +"""compare_table rule types the ported engine in _checks.py implements.""" + +_METHODS = frozenset({'eq', 'ne', 'le', 'lt', 'ge', 'gt'}) +"""Comparison methods check_cell props may use; besides these, 'approx:'.""" + +_FILE_NAME = re.compile(r'[\w.\- ]+') +"""What a staged file name may look like: shell-quotable, no separators.""" + + +class Task(NamedTuple): + """An OSWorld task, reduced to the parts an environment folder needs.""" + + task_id: str + domain: str + title: str + instruction: str + func: str + """The evaluator: 'compare_table' or 'compare_docx_files'.""" + file_name: str + """The staged file, at ~/Desktop/ in the session.""" + file_url: str + gold_url: str | None + """The evaluator's reference file, or None for a task compare_table checks alone.""" + rules: list[dict] + """compare_table rules; empty for compare_docx_files.""" + + +def download(url: str, *, accept: str | None = None) -> bytes: + """Download *url* with a plain GET.""" + request = urllib.request.Request(url, headers={'User-Agent': 'deepgym-import-osworld'}) + if accept: + request.add_header('Accept', accept) + with urllib.request.urlopen(request, timeout=120) as response: + return response.read() + + +def download_first(urls: list[str], *, accept: str | None = None) -> bytes: + """Download the first of *urls* that answers; later ones cover blocked hosts. + + Raises: + RuntimeError: If none of the URLs could be downloaded. + """ + error: Exception | None = None + for url in urls: + try: + return download(url, accept=accept) + except (OSError, urllib.error.URLError) as exc: + error = exc + raise RuntimeError(f'could not download {urls[0]}: {error}') + + +def fetch_task(task_id: str, domain: str) -> dict: + """Download one OSWorld task config. + + The GitHub contents API covers networks that block raw.githubusercontent.com; + the Accept header is what makes it return the file itself, not its listing. + """ + path = f'{domain}/{task_id}.json' + raw = download_first( + [f'{_TASK_URL}/{path}', f'{_TASK_API_URL}/{path}'], + accept='application/vnd.github.raw', + ) + return json.loads(raw) + + +def fetch_file(url: str, dest: Path) -> None: + """Download one task file; the hf-mirror host covers huggingface.co being blocked.""" + urls = [url] + if urllib.parse.urlsplit(url).hostname == 'huggingface.co': + urls.append(url.replace('huggingface.co', 'hf-mirror.com')) + dest.write_bytes(download_first(urls)) + + +def validate_rules(rules: list[dict], task_id: str) -> list[dict]: + """Check every rule is one the ported engine implements. + + An unsupported rule fails here, while importing, rather than quietly failing + the generated verifier: such a task needs a port of its own. + + Raises: + ValueError: If a rule (or the way it names a sheet, cell or comparison) + is not implemented by ``_checks.py``. + """ + for rule in rules: + kind = rule.get('type') + if kind not in _RULE_TYPES: + raise ValueError(f'{task_id}: compare_table rule {kind!r} is not ported yet') + if kind == 'sheet_data': + for key in ('sheet_idx0', 'sheet_idx1'): + if key not in rule: + raise ValueError(f'{task_id}: sheet_data rule without {key!r}') + _check_sheet_idx(rule[key], task_id) + elif kind == 'check_cell': + _check_sheet_idx(rule.get('sheet_idx'), task_id) + props = rule.get('props') or {} + if not props: + raise ValueError(f'{task_id}: check_cell rule without props') + for prop, spec in props.items(): + if prop not in ('value', 'merge'): + raise ValueError(f'{task_id}: check_cell prop {prop!r} is not ported yet') + method = str(spec.get('method', '')) + if method in _METHODS: + continue + if method.startswith('approx:'): + try: + float(method.split(':', 1)[1]) + except ValueError as exc: + raise ValueError(f'{task_id}: bad {method!r}') from exc + continue + raise ValueError(f'{task_id}: comparison {method!r} is not ported yet') + return rules + + +def _check_sheet_idx(idx: object, task_id: str) -> None: + """A sheet index is an int or 'RI'/'RN'/'EI'/'EN' followed by a number or name.""" + if isinstance(idx, int) and not isinstance(idx, bool): + return + if isinstance(idx, str) and idx.startswith(('RI', 'RN', 'EI', 'EN')): + return + raise ValueError(f'{task_id}: sheet index {idx!r} is not ported yet') + + +def parse_task(task_id: str, domain: str, title: str, task: dict) -> Task: + """Reduce an OSWorld task config to the parts an environment needs. + + Raises: + ValueError: If the task does not consist of one downloaded file, opened + once and scored from the file it saves; such a task needs more than + this importer ports. + """ + if task.get('id') != task_id: + raise ValueError(f'task id {task.get("id")!r} does not match {task_id!r}') + instruction = str(task.get('instruction', '')).strip() + if not instruction: + raise ValueError(f'{task_id}: no instruction') + + steps: dict[str, list[dict]] = {} + for step in task.get('config', []): + steps.setdefault(str(step.get('type')), []).append(step) + if len(steps.get('download', [])) != 1 or len(steps.get('open', [])) != 1: + raise ValueError(f'{task_id}: needs exactly one download and one open step') + files = steps['download'][0].get('parameters', {}).get('files', []) + if len(files) != 1: + raise ValueError(f'{task_id}: needs exactly one downloaded file') + file_name = Path(files[0]['path']).name + if not _FILE_NAME.fullmatch(file_name): + raise ValueError(f'{task_id}: file name {file_name!r} is not stageable') + opened = Path(steps['open'][0].get('parameters', {}).get('path', '')).name + if opened != file_name: + raise ValueError(f'{task_id}: opens {opened!r}, not the downloaded {file_name!r}') + + evaluator = task.get('evaluator', {}) + func = str(evaluator.get('func', '')) + result = evaluator.get('result') or {} + result_name = Path(str(result.get('path', ''))).name + if result.get('type') != 'vm_file' or result_name != file_name: + raise ValueError(f'{task_id}: result {result_name!r} is not the saved {file_name!r}') + expected = evaluator.get('expected') or None + if expected is not None and expected.get('type') != 'cloud_file': + raise ValueError(f'{task_id}: expected is not a cloud_file') + gold_url = str(expected['path']) if expected else None + + if func == 'compare_table': + options = evaluator.get('options') or {} + rules = options.get('rules') if isinstance(options, dict) else None + if not isinstance(rules, list) or not rules: + raise ValueError(f'{task_id}: compare_table without a rules list') + return Task( + task_id, domain, title, instruction, func, file_name, + files[0]['url'], gold_url, validate_rules(rules, task_id), + ) # fmt: skip + if func == 'compare_docx_files': + if gold_url is None: + raise ValueError(f'{task_id}: compare_docx_files needs a reference file') + return Task( + task_id, domain, title, instruction, func, file_name, files[0]['url'], gold_url, [] + ) + raise ValueError(f'{task_id}: evaluator {func!r} is not ported yet') + + +def build_task_md(task: Task) -> str: + """The prompt: the instruction, where the file is, and how to save it. + + Raises: + ValueError: If the instruction contains '$'. task.md is substituted as a + string.Template at load time, so a stray $ would break the environment. + """ + app = 'LibreOffice Calc' if task.func == 'compare_table' else 'LibreOffice Writer' + text = ( + f'{task.instruction}\n' + f'\n' + f'The file is open in {app}, and it is also at ~/Desktop/{task.file_name}. ' + f'Save your changes to the same file with Ctrl+S (keep the current format). ' + f'Use the `done` action when finished.\n' + ) + if '$' in text: + raise ValueError(f'{task.task_id}: the instruction contains $, which task.md cannot hold') + return text + + +def build_setup(task: Task) -> str: + """The setup script: decode the starting file and open it in LibreOffice. + + The document window is awaited like OSWorld's own open step does (by title), + with a plain wait as the fallback for images without wmctrl. + """ + component = '--calc' if task.func == 'compare_table' else '--writer' + return _SETUP.replace('', task.file_name).replace('', component) + + +# and are replaced; literal braces and $s stay untouched. +_SETUP = """set -euo pipefail +soffice_app="" +for candidate in soffice libreoffice; do + if command -v "$candidate" >/dev/null 2>&1; then soffice_app=$candidate; break; fi +done +if [ -z "$soffice_app" ]; then + echo "LibreOffice is not installed in this image; it is required by this environment." >&2 + exit 1 +fi +file="$HOME/Desktop/" +mkdir -p "$(dirname "$file")" +base64 -d "assets/.b64" > "$file" +(setsid nohup "$soffice_app" --norestore "$file" >/tmp/deepgym_soffice.log 2>&1 &) +if command -v wmctrl >/dev/null 2>&1; then + for _ in $(seq 60); do + if wmctrl -l 2>/dev/null | grep -qF ''; then break; fi + sleep 1 + done +else + sleep 10 +fi +""" + + +def build_verifier(task: Task) -> str: + """The verifier: OSWorld's evaluator ported to a state check on the saved file. + + Every compare_table rule is one progress check (the original evaluator was + all-or-nothing, so a partial solution earns partial reward), and a workbook + that cannot be read fails the guard: an unsaved or missing file scores 0. + """ + if task.func == 'compare_docx_files': + return ( + f'"""Scores the file OSWorld task {task.task_id} saves, against the text its\n' + f'compare_docx_files evaluator reads.\n' + f'\n' + f'Generated by scripts/import_osworld.py; OSWorld is Apache-2.0. The loader\n' + f'prepends _checks.py and PARAMS: read_docx_text normalizes the saved document\n' + f'the way python-docx does for the original evaluator.\n' + f'"""\n' + _TEXT_VERIFIER_BODY + ) + return ( + f'"""Scores the file OSWorld task {task.task_id} saves, with the task\'s own\n' + f'compare_table rules; each rule is one check.\n' + f'\n' + f'Generated by scripts/import_osworld.py; OSWorld is Apache-2.0. The loader\n' + f'prepends _checks.py and PARAMS, so the rules run against the saved file, never\n' + f'the screen. Rules the starting file already satisfies are guards, not progress.\n' + f'"""\n' + _CALC_VERIFIER_BODY + ) + + +_CALC_VERIFIER_BODY = """ +import os +from pathlib import Path + +workbook = Path(os.environ['DEEPGYM_AGENT_HOME']) / PARAMS['result'] +try: + sheets = read_sheets(str(workbook)) +except ValueError: + sheets = None + +report( + progress=office_rule_checks(PARAMS['progress'], sheets, PARAMS['expected']), + guards={ + 'workbook_saved': sheets is not None, + **office_rule_checks(PARAMS['guards'], sheets, PARAMS['expected']), + }, +) +""" + +_TEXT_VERIFIER_BODY = """ +import os +from pathlib import Path + +document = Path(os.environ['DEEPGYM_AGENT_HOME']) / PARAMS['result'] +try: + text = read_docx_text(str(document)) +except ValueError: + text = None + +report( + progress={'matches_reference': text == PARAMS['expected_text']}, + guards={'document_saved': text is not None}, +) +""" + + +def build_variants(task: Task, source: Path, gold: Path | None, checks: ModuleType) -> str: + """The task's one variant: the result path, its rules and its reference data. + + The reference data is read from the gold file with the very readers the verifier + will use, and lives here because variants.json is a spec file: the loader keeps it + out of the session and only prepends it to the verifier. + + Each rule is progress or a guard, decided here by evaluating it against the + untouched starting file: a rule the file already satisfies is not something the + task asks for, so it becomes a guard (keep it holding) instead of a progress + check that saving nothing would collect. + + Raises: + ValueError: If the reference file fails one of the task's own rules, or + nothing in it requires work: every rule already holds in the starting + file, or (for a text task) the reference text is the starting text. + """ + variant: dict = {'result': f'Desktop/{task.file_name}'} + if task.func == 'compare_table': + expected = ( + None if gold is None else [list(sheet) for sheet in checks.read_sheets(str(gold))] + ) + named = [ + {**rule, 'name': f'rule{index}_{rule.get("type", "?")}'} + for index, rule in enumerate(task.rules) + ] + if expected is not None: + reference = checks.office_rule_checks(named, expected, expected) + failed = [name for name, ok in reference.items() if not ok] + if failed: + raise ValueError(f'{task.task_id}: the reference file fails {failed}') + untouched = checks.office_rule_checks(named, checks.read_sheets(str(source)), expected) + variant['progress'] = [rule for rule in named if not untouched[rule['name']]] + variant['guards'] = [rule for rule in named if untouched[rule['name']]] + if not variant['progress']: + raise ValueError(f'{task.task_id}: needs no work; saving nothing would score') + variant['expected'] = expected + else: + expected_text = checks.read_docx_text(str(gold)) + if expected_text == checks.read_docx_text(str(source)): + raise ValueError(f'{task.task_id}: needs no work; saving nothing would score') + variant['expected_text'] = expected_text + return json.dumps([variant], indent=2, ensure_ascii=False) + '\n' + + +def _tags(task: Task) -> list[str]: + app = 'calc' if task.func == 'compare_table' else 'writer' + return ['osworld', 'libreoffice', app, 'desktop', 'gui'] + + +def build_metadata(task: Task) -> str: + """metadata.json: the fields deepgym check validates.""" + metadata = { + 'id': task.task_id, + 'name': task.title, + 'type': 'computer-use', + 'difficulty': 'medium', + 'domain': task.domain.replace('_', '-'), + 'tags': _tags(task), + 'max_steps': 30, + } + return json.dumps(metadata, indent=2) + '\n' + + +def build_registry_entry(task: Task) -> dict: + """The registry.json entry: how the environment is found and listed.""" + return { + 'id': task.task_id, + 'name': task.title, + 'path': f'environments/osworld/{task.task_id}', + 'type': 'computer-use', + 'benchmark': 'osworld', + 'variants': 1, + 'difficulty': 'medium', + 'domain': task.domain.replace('_', '-'), + 'family': 'computer-use', + 'tags': _tags(task), + } + + +def build_environment( + task: Task, source: Path, gold: Path | None, output_dir: Path, checks: ModuleType +) -> dict: + """Write one environment folder and return its registry entry.""" + env_dir = output_dir / task.task_id + (env_dir / 'assets').mkdir(parents=True, exist_ok=True) + (env_dir / 'task.md').write_text(build_task_md(task), encoding='utf-8') + (env_dir / 'setup.sh').write_text(build_setup(task), encoding='utf-8') + (env_dir / 'verifier.py').write_text(build_verifier(task), encoding='utf-8') + (env_dir / 'variants.json').write_text( + build_variants(task, source, gold, checks), encoding='utf-8' + ) + (env_dir / 'metadata.json').write_text(build_metadata(task), encoding='utf-8') + asset = env_dir / 'assets' / f'{task.file_name}.b64' + asset.write_text(base64.encodebytes(source.read_bytes()).decode('ascii'), encoding='utf-8') + return build_registry_entry(task) + + +def checks_path() -> Path: + """The shared _checks.py: from this checkout, else from the installed package. + + Raises: + FileNotFoundError: If neither has it. + """ + checkout = ( + Path(__file__).resolve().parent.parent + / 'src' + / 'deepgym' + / 'envs' + / 'computer_use' + / '_checks.py' + ) + if checkout.is_file(): + return checkout + where = importlib.resources.files('deepgym') / 'envs' / 'computer_use' / '_checks.py' + installed = Path(str(where)) + if not installed.is_file(): + raise FileNotFoundError('no _checks.py next to this script or in the installed package') + return installed + + +def load_checks_module() -> ModuleType: + """Import _checks.py, so the importer reads reference files exactly as the verifier will.""" + path = checks_path() + spec = importlib.util.spec_from_file_location('deepgym_computer_use_checks', path) + if spec is None or spec.loader is None: + raise ImportError(f'cannot load {path}') + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def install_shared(output_dir: Path) -> Path: + """Copy _checks.py next to the environments, where the loader reads it from.""" + target = output_dir / '_checks.py' + shutil.copyfile(checks_path(), target) + return target + + +def main() -> None: + output_dir = Path.home() / '.deepgym' / 'environments' / 'osworld' + output_dir.mkdir(parents=True, exist_ok=True) + install_shared(output_dir) + checks = load_checks_module() + + entries = [] + for task_id, domain, title in TASKS: + print(f'{task_id}: importing the {domain} task...') + task = parse_task(task_id, domain, title, fetch_task(task_id, domain)) + with tempfile.TemporaryDirectory() as tmp: + source = Path(tmp) / task.file_name + fetch_file(task.file_url, source) + gold = None + if task.gold_url is not None: + gold = Path(tmp) / Path(task.gold_url).name + fetch_file(task.gold_url, gold) + entries.append(build_environment(task, source, gold, output_dir, checks)) + + (output_dir / 'registry.json').write_text( + json.dumps(entries, indent=2, ensure_ascii=False) + '\n', encoding='utf-8' + ) + print(f'Generated {len(entries)} OSWorld environments in {output_dir}') + + +if __name__ == '__main__': + main() diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index 26ac4f35..e96559d0 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -1,7 +1,18 @@ -"""Shared helpers for computer-use verifiers.""" +"""Shared helpers for computer-use verifiers. + +Besides the app-state helpers, this module carries the readers and rule engine +for office files: `read_sheets` and `read_docx_text` read .xlsx and .docx with +the standard library only, and `office_rule_checks` evaluates the compare_table +rules of an imported OSWorld task against the workbook the agent saved. +""" + +from __future__ import annotations import json +import re +import zipfile from pathlib import Path +from xml.etree import ElementTree as ET def load_state(path: str = 'state.json') -> dict: @@ -38,3 +49,459 @@ def report(progress: dict[str, bool], guards: dict[str, bool]) -> None: } ) ) + + +# Office files {{{ # +# +# The readers follow what OSWorld's own evaluators see through pandas, openpyxl +# and python-docx: cached formula values (not formula text), empty cells as +# None, numbers int when integral (dates stay raw serials, so both sides of a +# comparison encode the same way), and docx text as python-docx reads it. +# office_rule_checks() evaluates compare_table's rules with the same semantics, +# including DataFrame.round's rounding (numpy's, not Python's, half-to-even). + +_XLSX_NS = '{http://schemas.openxmlformats.org/spreadsheetml/2006/main}' +_REL_NS = '{http://schemas.openxmlformats.org/officeDocument/2006/relationships}' +_PKG_REL_NS = '{http://schemas.openxmlformats.org/package/2006/relationships}' +_W_NS = '{http://schemas.openxmlformats.org/wordprocessingml/2006/main}' + +_COORD_RE = re.compile(r'([A-Z]+)(\d+)') + + +def _coord_to_cell(coord: str) -> tuple[int, int]: + """'C7' -> (row 7, column 3), both 1-based.""" + match = _COORD_RE.fullmatch(coord) + if not match: + raise ValueError(f'not a cell coordinate: {coord!r}') + col = 0 + for ch in match.group(1): + col = col * 26 + (ord(ch) - ord('A') + 1) + return int(match.group(2)), col + + +def _number(text: str) -> int | float | None: + """A numeric cell's value: float when fractional, int when integral (pandas-like).""" + try: + value = float(text) + except ValueError: + return None + if value.is_integer() and abs(value) < 1e15: + return int(value) + return value + + +def _cell_value(cell: ET.Element, shared: list[str]) -> object: + """The value of one element, as pandas reads it (dates stay serials).""" + kind = cell.get('t') + if kind == 'inlineStr': + node = cell.find(f'{_XLSX_NS}is') + if node is None: + return None + return ''.join(t.text or '' for t in node.iter(f'{_XLSX_NS}t')) + value = cell.find(f'{_XLSX_NS}v') + if value is None or value.text is None: + return None + if kind == 's': + try: + return shared[int(value.text)] + except (ValueError, IndexError): + return None + if kind == 'str': + return value.text + if kind == 'b': + return value.text == '1' + if kind == 'e': + return value.text + return _number(value.text) + + +def _merge_ranges(root: ET.Element) -> list[str]: + merges = root.find(f'{_XLSX_NS}mergeCells') + if merges is None: + return [] + return [cell.get('ref', '') for cell in merges.findall(f'{_XLSX_NS}mergeCell')] + + +def _trim(grid: list[list[object]]) -> list[list[object]]: + """Drop trailing rows and columns whose every cell is None (pandas reads no further).""" + while grid and all(value is None for value in grid[-1]): + grid.pop() + while grid and all((row[-1] if row else None) is None for row in grid): + for row in grid: + row.pop() + return grid + + +def _sheet_grid(root: ET.Element, shared: list[str]) -> list[list[object]]: + """Rows of cell values over the sheet's used range; missing cells are None.""" + cells: dict[tuple[int, int], object] = {} + max_row = 0 + max_col = 0 + data = root.find(f'{_XLSX_NS}sheetData') + if data is None: + return [] + for row in data.findall(f'{_XLSX_NS}row'): + for cell in row.findall(f'{_XLSX_NS}c'): + ref = cell.get('r') + if not ref: + continue + r, c = _coord_to_cell(ref) + cells[(r, c)] = _cell_value(cell, shared) + max_row = max(max_row, r) + max_col = max(max_col, c) + grid: list[list[object]] = [] + for r in range(1, max_row + 1): + grid.append([cells.get((r, c)) for c in range(1, max_col + 1)]) + return _trim(grid) + + +def _sheet_parts(archive: zipfile.ZipFile) -> list[tuple[str, str]]: + """(sheet name, part name) for every sheet, in workbook order.""" + workbook = ET.fromstring(archive.read('xl/workbook.xml')) + rels = ET.fromstring(archive.read('xl/_rels/workbook.xml.rels')) + targets = { + rel.get('Id'): rel.get('Target') for rel in rels.findall(f'{_PKG_REL_NS}Relationship') + } + sheets = [] + node = workbook.find(f'{_XLSX_NS}sheets') + for sheet in [] if node is None else node: + target = targets.get(sheet.get(f'{_REL_NS}id'), '') + if target.startswith('/'): + target = target[1:] + elif not target.startswith('xl/'): + target = f'xl/{target}' + sheets.append((sheet.get('name', ''), target)) + return sheets + + +def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: + """Every sheet of *path*: (name, values grid, merged ranges in A1 notation). + + Args: + path: Path to an .xlsx workbook. + + Returns: + Sheets in workbook order. A grid's cells are strings, numbers (int when + integral), booleans, error text, or None for an empty cell. + + Raises: + ValueError: If the file is missing or not a readable workbook. + """ + try: + with zipfile.ZipFile(path) as archive: + shared_node = None + if 'xl/sharedStrings.xml' in archive.namelist(): + shared_node = ET.fromstring(archive.read('xl/sharedStrings.xml')) + shared = [ + ''.join(t.text or '' for t in si.iter(f'{_XLSX_NS}t')) + for si in (shared_node if shared_node is not None else []) + ] + sheets = [] + for name, part in _sheet_parts(archive): + root = ET.fromstring(archive.read(part)) + sheets.append((name, _sheet_grid(root, shared), _merge_ranges(root))) + return sheets + except (OSError, zipfile.BadZipFile, KeyError, ET.ParseError) as exc: + raise ValueError(f'{path} is not a readable workbook: {exc}') from exc + + +def _run_text(run: ET.Element) -> str: + """python-docx's `Run.text`: direct t/tab/br/cr/noBreakHyphen/ptab children.""" + parts = [] + for node in run: + tag = node.tag + if tag == f'{_W_NS}t': + parts.append(node.text or '') + elif tag == f'{_W_NS}tab': + parts.append('\t') + elif tag == f'{_W_NS}cr': + parts.append('\n') + elif tag == f'{_W_NS}br': + parts.append('\n' if node.get(f'{_W_NS}type', 'textWrapping') == 'textWrapping' else '') + elif tag == f'{_W_NS}noBreakHyphen': + parts.append('-') + elif tag == f'{_W_NS}ptab': + parts.append('\t') + return ''.join(parts) + + +def read_docx_text(path: str) -> str: + """The document's text as compare_docx_files reads it: python-docx's + `paragraph.text` for every body paragraph (direct runs and hyperlink runs), in + order, whitespace collapsed to single spaces, stripped. + + Args: + path: Path to a .docx document. + + Returns: + The normalized text. + + Raises: + ValueError: If the file is missing or not a readable document. + """ + try: + with zipfile.ZipFile(path) as archive: + root = ET.fromstring(archive.read('word/document.xml')) + except (OSError, zipfile.BadZipFile, KeyError, ET.ParseError) as exc: + raise ValueError(f'{path} is not a readable document: {exc}') from exc + body = root.find(f'{_W_NS}body') + paragraphs = [] + if body is not None: + for para in body: + if para.tag != f'{_W_NS}p': + continue + text_parts = [] + for child in para: + if child.tag == f'{_W_NS}r': + text_parts.append(_run_text(child)) + elif child.tag == f'{_W_NS}hyperlink': + for run in child: + if run.tag == f'{_W_NS}r': + text_parts.append(_run_text(run)) + paragraphs.append(''.join(text_parts)) + return re.sub(r'\s+', ' ', '\n'.join(paragraphs)).strip() + + +def _np_round(value: float, precision: int) -> float: + """Round like pandas' DataFrame.round, i.e. like numpy: round the value scaled by + 10**precision to nearest-even, then scale back. + + Python's `round(value, precision)` can disagree at exact halves, e.g. + round(-0.00005, 4) == -0.0001 while numpy rounds it to -0.0. + """ + scale = 10**precision + return round(value * scale) / scale + + +def _is_number(value: object) -> bool: + return not isinstance(value, bool) and isinstance(value, (int, float)) + + +def _same_value(a: object, b: object) -> bool: + """Elementwise equality as pandas uses it: None only equals None; numbers compare + across int/float (and bool, as Python's `==` does); other types must match.""" + if a is None or b is None: + return a is None and b is None + if isinstance(a, (int, float)) and isinstance(b, (int, float)): + return float(a) == float(b) + if isinstance(a, str) and isinstance(b, str): + return a == b + return False + + +def _round_grid(grid: list[list[object]], precision: int) -> list[list[object]]: + """DataFrame.round(precision)'s effect on a grid: round all-numeric columns' data. + + Row 0 is the header and becomes the frame's column labels, so it never takes + part in a column's dtype and is never rounded. pandas leaves object columns (a + column becomes object as soon as one data cell is not a plain number), bools + and datetimes untouched, so a stray text cell turns off rounding for the whole + column on that side. + """ + width = max((len(row) for row in grid), default=0) + numeric = [] + for column in range(width): + values = [row[column] for row in grid[1:] if column < len(row) and row[column] is not None] + numeric.append(bool(values) and all(_is_number(value) for value in values)) + if not any(numeric): + return grid + rounded = [list(grid[0])] if grid else [] + for row in grid[1:]: + rounded.append( + [ + _np_round(value, precision) + if column < len(row) and numeric[column] and _is_number(value) + else value + for column, value in enumerate(row) + ] + ) + return rounded + + +def sheets_match( + agent: list[list[object]], expected: list[list[object]], precision: int = 4 +) -> bool: + """Whether two grids are the same sheet, as compare_table's sheet_data sees it. + + Mirrors `pd.read_excel(a).round(precision).equals(pd.read_excel(b).round(precision))`: + each side is rounded by its own column dtypes first, then shapes and every value + must match. + """ + if len(agent) != len(expected): + return False + for row_a, row_e in zip(agent, expected): + if len(row_a) != len(row_e): + return False + agent = _round_grid(agent, precision) + expected = _round_grid(expected, precision) + for row_a, row_e in zip(agent, expected): + for value_a, value_e in zip(row_a, row_e): + if not _same_value(value_a, value_e): + return False + return True + + +def is_merged(merges: list[str], coordinate: str) -> bool: + """True when *coordinate* sits in a merged range but is not its top-left anchor. + + This is what openpyxl reports for a MergedCell, which compare_table's check_cell + 'merge' prop matches against. + """ + r, c = _coord_to_cell(coordinate) + for ref in merges: + if ':' not in ref: + continue + start, end = ref.split(':', 1) + r1, c1 = _coord_to_cell(start) + r2, c2 = _coord_to_cell(end) + if r1 <= r <= r2 and c1 <= c <= c2 and not (r == r1 and c == c1): + return True + return False + + +def cell_at(sheet_rows: list[list[object]], coordinate: str) -> object: + """The value at *coordinate* ('E3'), or None outside the sheet.""" + r, c = _coord_to_cell(coordinate) + if 1 <= r <= len(sheet_rows) and 1 <= c <= len(sheet_rows[r - 1]): + return sheet_rows[r - 1][c - 1] + return None + + +def build_sheet(rows: list[list[object]]) -> list[list[object]]: + """Trim a grid built from a literal the way read_sheets trims a file's.""" + return _trim([list(row) for row in rows]) + + +def _parse_sheet_idx(idx: object, names_r: list[str], names_e: list[str]) -> tuple[str, str]: + """OSWorld's `_parse_sheet_idx`: int/RI/RN select the *result* book (the file the + agent saved), EI/EN the *expected* one. Returns (side, sheet name); an index that + cannot be resolved yields name '' which then fails to load, as in OSWorld.""" + if isinstance(idx, int): + if not names_r or idx >= len(names_r): + return ('r', '') + return ('r', names_r[idx]) + if isinstance(idx, str) and idx.startswith('RI'): + try: + return ('r', names_r[int(idx[2:])]) + except Exception: + return ('r', '') + if isinstance(idx, str) and idx.startswith('RN'): + return ('r', idx[2:]) + if isinstance(idx, str) and idx.startswith('EI'): + try: + return ('e', names_e[int(idx[2:])]) + except Exception: + return ('e', '') + if isinstance(idx, str) and idx.startswith('EN'): + return ('e', idx[2:]) + raise ValueError(f'unrecognized sheet index: {idx!r}') + + +def _match_rule(value: object, rule: dict) -> bool: + """OSWorld's `_match_value_to_rule` for the comparison methods the importer + copies over. A malformed rule or unknown method fails the rule, never passes it.""" + import operator as op + + method = str(rule.get('method', '')) + if method in {'eq', 'ne', 'le', 'lt', 'ge', 'gt'}: + try: + return bool(getattr(op, method)(value, rule['ref'])) + except (KeyError, TypeError): + return False + if method.startswith('approx'): + try: + return abs(float(value) - rule['ref']) <= float(method.split(':', 1)[1]) + except (KeyError, TypeError, ValueError): + return False + return False + + +def _rule_verdict( + rule: dict, + result: list[tuple[str, list[list[object]], list[str]]] | None, + expected: list[tuple[str, list[list[object]], list[str]]] | None, + names_r: list[str], + names_e: list[str], +) -> bool: + """One compare_table rule against the agent's workbook (and the reference). + + A rule that cannot be resolved — a missing sheet, index or cell — fails; + no rule of any shape raises.""" + kind = rule.get('type') + if kind == 'sheet_name': + return expected is not None and names_r == names_e + if kind == 'sheet_data': + + def grid_of(idx: object) -> list[list[object]] | None: + try: + side, name = _parse_sheet_idx(idx, names_r, names_e) + except ValueError: + return None + sheets = result if side == 'r' else expected + match = [sheet for sheet in sheets or [] if sheet[0] == name] + return match[0][1] if match else None + + grid1 = grid_of(rule.get('sheet_idx0')) + if grid1 is None: + return False + grid2 = grid_of(rule.get('sheet_idx1')) + if grid2 is None: + return False + return sheets_match(grid1, grid2, rule.get('precision', 4)) + if kind == 'check_cell': + try: + side, name = _parse_sheet_idx(rule.get('sheet_idx'), names_r, names_e) + except ValueError: + return False + sheets = result if side == 'r' else expected + match = [sheet for sheet in sheets or [] if sheet[0] == name] + if not match or not rule.get('props'): + return False + _, rows, merges = match[0] + for prop, rule_value in rule['props'].items(): + try: + if prop == 'value': + value = cell_at(rows, rule['coordinate']) + elif prop == 'merge': + value = is_merged(merges, rule['coordinate']) + else: + return False + except (KeyError, ValueError): + return False + if not _match_rule(value, rule_value): + return False + return True + return False + + +def office_rule_checks( + rules: list[dict], + result: list[tuple[str, list[list[object]], list[str]]] | None, + expected: list[tuple[str, list[list[object]], list[str]]] | None, +) -> dict[str, bool]: + """Evaluate an imported task's compare_table rules against the agent's workbook. + + Args: + rules: The task's rules, in order. + result: `read_sheets()` of the file the agent saved, or None if it could not + be read. + expected: `read_sheets()` of the task's reference workbook, or None when the + task has no reference file. + + Returns: + {rule name, e.g. 'rule0_sheet_data': passed}; a rule whose sheets, indices + or cells cannot be resolved fails. A rule's 'name', when it has one, is + used as its key. + """ + names_r = [sheet[0] for sheet in result or []] + names_e = [sheet[0] for sheet in expected or []] + checks = {} + for index, rule in enumerate(rules): + name = str(rule.get('name') or f'rule{index}_{rule.get("type", "?")}') + while name in checks: # two rules must never share one verdict slot + name += '_' + checks[name] = _rule_verdict(rule, result, expected, names_r, names_e) + return checks + + +# }}} Office files # diff --git a/deepgym/src/deepgym/factory/check.py b/deepgym/src/deepgym/factory/check.py index c5882eed..2ba4a2bd 100644 --- a/deepgym/src/deepgym/factory/check.py +++ b/deepgym/src/deepgym/factory/check.py @@ -59,6 +59,9 @@ def check_env(env_dir: Path, dg: DeepGym) -> list[QACheck]: Returns: files, metadata and verifier_source; then, if those pass, reference_passes, deterministic, empty_fails, stub_fails (single-turn) and protocol; then approval. + A desktop env whose reference can't run here (it ships no episode to replay, + or the backend gives no desktop) reports a single skipped reference_passes + instead of the run checks. """ if (env_dir / 'task.toml').is_file(): return [_harbor_check(env_dir), approval_check(env_dir)] @@ -120,8 +123,9 @@ def _files_check(env_dir: Path, metadata: EnvMetadata | None) -> QACheck: if metadata is not None and metadata.type == 'multi-turn': if not (env_dir / REFERENCE_ACTIONS).is_file(): problems.append(f'{REFERENCE_ACTIONS} is missing (the episode that must score 1.0)') - elif not any((env_dir / name).is_file() for name in REFERENCE_FILES): - problems.append(f'{" or ".join(REFERENCE_FILES)} is missing (it must score 1.0)') + elif metadata is None or metadata.type != 'computer-use': + if not any((env_dir / name).is_file() for name in REFERENCE_FILES): + problems.append(f'{" or ".join(REFERENCE_FILES)} is missing (it must score 1.0)') root = env_dir.resolve() for rel in metadata.support_files if metadata else []: path = (env_dir / rel).resolve() @@ -129,7 +133,10 @@ def _files_check(env_dir: Path, metadata: EnvMetadata | None) -> QACheck: problems.append(f'support file {rel!r} is not a file inside the folder') if problems: return QACheck(name='files', status='fail', details='; '.join(problems)) - return QACheck(name='files', status='pass', details='task, verifier, reference and metadata') + details = 'task, verifier, reference and metadata' + if metadata is not None and metadata.type == 'computer-use': + details = 'task, verifier and metadata (a desktop env may ship a reference episode)' + return QACheck(name='files', status='pass', details=details) def _verifier_check(env_dir: Path) -> QACheck: @@ -162,7 +169,7 @@ def lint_env(env_dir: Path) -> list[str]: for rel in (metadata.support_files if metadata else []) if (env_dir / rel).is_file() } - prepended = metadata is not None and metadata.type == 'multi-turn' + prepended = metadata is not None and metadata.type in ('multi-turn', 'computer-use') try: return lint_verifier(path.read_text(encoding='utf-8'), support, prepended=prepended) except SyntaxError as exc: @@ -170,7 +177,12 @@ def lint_env(env_dir: Path) -> list[str]: def _run_checks(env_dir: Path, dg: DeepGym) -> list[QACheck]: - """Run the reference, empty and stub solutions (or episodes) and judge them.""" + """Run the reference, empty and stub solutions (or episodes) and judge them. + + A desktop env whose reference can't run here — one with no episode to replay, or + one off the Daytona backend — reports a single skipped ``reference_passes``: no + local session can drive a desktop. + """ try: env = load_environment_dir(env_dir) except (ValueError, OSError) as exc: @@ -182,7 +194,16 @@ def _run_checks(env_dir: Path, dg: DeepGym) -> list[QACheck]: except SyntaxError as exc: details = f'verifier.py with its support files prepended: line {exc.lineno}: {exc.msg}' return [QACheck(name='load', status='fail', details=details)] - results = episode_results(env, env.reference_actions or [], dg, _RERUNS) + if env.reference_actions is None: + details = ( + f'this env ships no {REFERENCE_ACTIONS}: ' + 'an agent has to drive it, not a script to replay' + ) + return [QACheck(name='reference_passes', status='skip', details=details)] + if env.desktop and dg.backend != 'daytona': + details = 'a computer-use env needs a desktop: run with --mode daytona' + return [QACheck(name='reference_passes', status='skip', details=details)] + results = episode_results(env, env.reference_actions, dg, _RERUNS) return [*run_verdicts(results, runs), protocol_check(results)] results = single_turn_results(env, env.reference_solution or '', dg, _RERUNS) return [*single_turn_verdicts(results, _RERUNS), protocol_check(results)] diff --git a/deepgym/tests/office_fixtures.py b/deepgym/tests/office_fixtures.py new file mode 100644 index 00000000..71df6abd --- /dev/null +++ b/deepgym/tests/office_fixtures.py @@ -0,0 +1,109 @@ +"""Hand-built .xlsx and .docx fixtures for the office reader tests. + +Stdlib only, like the readers themselves: these builders write just the parts +`_checks.py` reads, so no office library is needed to exercise it. +""" + +import re +import zipfile +from pathlib import Path + +_CELL = re.compile(r'([A-Z]+)(\d+)') + +_XLSX_NS = 'http://schemas.openxmlformats.org/spreadsheetml/2006/main' +_REL_NS = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships' +_PKG_REL_NS = 'http://schemas.openxmlformats.org/package/2006/relationships' +_W_NS = 'http://schemas.openxmlformats.org/wordprocessingml/2006/main' + + +class Inline(str): + """A string cell stored inline (t="inlineStr") instead of in sharedStrings.""" + + +def make_xlsx(path: Path, sheets: list[tuple[str, dict[str, object], list[str]]]) -> Path: + """Write a minimal workbook: one entry per sheet, (name, cells, merged ranges). + + A cell value may be a number, a bool, a str (shared string) or an Inline (an + inline string); a cell left out is empty. Cells are 1-based references like 'C7'. + """ + shared: dict[str, int] = {} + parts = [] + for index, (name, cells, merges) in enumerate(sheets, start=1): + parts.append((name, _sheet_xml(cells, merges, shared), index)) + members = { + 'xl/workbook.xml': _workbook_xml([name for name, _, _ in parts]), + 'xl/_rels/workbook.xml.rels': _rels_xml(len(parts)), + } + members.update({f'xl/worksheets/sheet{index}.xml': xml for _, xml, index in parts}) + if shared: + members['xl/sharedStrings.xml'] = _shared_xml(shared) + with zipfile.ZipFile(path, 'w') as archive: + for member, xml in members.items(): + archive.writestr(member, xml) + return path + + +def _cell_xml(ref: str, value: object, shared: dict[str, int]) -> str: + if isinstance(value, bool): + return f'{int(value)}' + if isinstance(value, (int, float)): + return f'{value!r}' + if isinstance(value, Inline): + return f'{value}' + if isinstance(value, str): + return f'{shared.setdefault(value, len(shared))}' + raise TypeError(f'cannot write {value!r}') + + +def _sheet_xml(cells: dict[str, object], merges: list[str], shared: dict[str, int]) -> str: + rows: dict[int, list[str]] = {} + for ref, value in cells.items(): + rows.setdefault(int(_CELL.fullmatch(ref).group(2)), []).append( + _cell_xml(ref, value, shared) + ) + body = ''.join( + f'{"".join(row)}' for number, row in sorted(rows.items()) + ) + merge = ( + f'' + + ''.join(f'' for ref in merges) + + '' + if merges + else '' + ) + return f'{body}{merge}' + + +def _shared_xml(shared: dict[str, int]) -> str: + items = sorted(shared, key=shared.get) + body = ''.join(f'{text}' for text in items) + count = len(items) + return f'{body}' + + +def _workbook_xml(names: list[str]) -> str: + sheets = ''.join( + f'' + for index, name in enumerate(names, start=1) + ) + return f'{sheets}' + + +def _rels_xml(count: int) -> str: + rels = ''.join( + f'' + for index in range(1, count + 1) + ) + return f'{rels}' + + +def make_docx(path: Path, paragraphs: list[str]) -> Path: + """Write a minimal document; a paragraph starting with '{para}' + for para in paragraphs + ) + document = f'{body}' + with zipfile.ZipFile(path, 'w') as archive: + archive.writestr('word/document.xml', document) + return path diff --git a/deepgym/tests/test_factory.py b/deepgym/tests/test_factory.py index cbdfbb57..ee8df478 100644 --- a/deepgym/tests/test_factory.py +++ b/deepgym/tests/test_factory.py @@ -468,6 +468,48 @@ def test_check_rejects_invalid_reference_actions(tmp_path: Path, local_dg: DeepG assert check.details.startswith('reference_actions.json: ') +def test_check_skips_the_reference_a_desktop_env_cannot_run(local_dg: DeepGym) -> None: + """A computer-use folder's reference episode drives a real desktop, so a local + session can't replay it: the run checks are a skip, not a failure.""" + builtin = Path(__file__).resolve().parent.parent / 'src' / 'deepgym' / 'envs' + folder = builtin / 'computer_use' / 'signup_form' + + checks = {check.name: check for check in check_env(folder, local_dg)} + + assert checks['files'].status == 'pass' + assert checks['metadata'].status == 'pass' + assert checks['verifier_source'].status == 'pass' + assert checks['reference_passes'].status == 'skip' + assert 'desktop' in checks['reference_passes'].details + + +def test_check_skips_a_desktop_env_with_no_reference_episode( + tmp_path: Path, local_dg: DeepGym +) -> None: + """A desktop env an agent drives ships no episode to replay: the run checks are a + skip, not a failure over a missing reference_actions.json.""" + shared = Path(__file__).resolve().parent.parent / 'src' / 'deepgym' / 'envs' / 'computer_use' + root = tmp_path / 'computer_use' + folder = root / 'calc_task' + folder.mkdir(parents=True) + (root / '_checks.py').write_text( + (shared / '_checks.py').read_text(encoding='utf-8'), encoding='utf-8' + ) + shutil.copytree(shared / '_shared', root / '_shared') + (folder / 'task.md').write_text('Do the thing.', encoding='utf-8') + (folder / 'verifier.py').write_text('report(1.0, {})\n', encoding='utf-8') + (folder / 'variants.json').write_text('[{}]', encoding='utf-8') + (folder / 'metadata.json').write_text( + '{"name": "calc_task", "type": "computer-use"}', encoding='utf-8' + ) + + checks = {check.name: check for check in check_env(folder, local_dg)} + + assert checks['files'].status == 'pass' + assert checks['reference_passes'].status == 'skip' + assert 'an agent has to drive it' in checks['reference_passes'].details + + def test_check_skips_image_based_repo_patch_folders(tmp_path: Path, local_dg: DeepGym) -> None: (tmp_path / 'test_patch.diff').write_text('', encoding='utf-8') diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py new file mode 100644 index 00000000..6fec4d58 --- /dev/null +++ b/deepgym/tests/test_import_osworld.py @@ -0,0 +1,573 @@ +"""OSWorld importer: the generated envs score the file the task saves, not the screen. + +Runs offline: the task configs and the office files here are built by hand, and the +importer's own functions are the ones under test. +""" + +import base64 +import copy +import json +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from deepgym import ComputerUseEnvironment +from deepgym.registry import load_environment_dir +from tests.office_fixtures import make_docx, make_xlsx + +scripts_dir = Path(__file__).resolve().parent.parent / 'scripts' +if str(scripts_dir) not in sys.path: + sys.path.insert(0, str(scripts_dir)) + +from import_osworld import ( # noqa: E402 + _TASK_API_URL, + _TASK_URL, + TASKS, + Task, + build_environment, + build_registry_entry, + build_setup, + build_task_md, + fetch_file, + fetch_task, + install_shared, + load_checks_module, + parse_task, +) + +TASK_ID = '00000000-0000-4000-8000-000000000024' +CALC = 'libreoffice_calc' +WRITER = 'libreoffice_writer' +FILE = 'budget.xlsx' +DOC = 'report.docx' +DESKTOP = '/home/user/Desktop' +TITLE = 'A task title' + +_START = { + 'A1': 'Item', + 'B1': 'Cost', + 'A2': 'Rent', + 'B2': 1200, + 'A3': 'Food', + 'B3': 300, + 'A4': 'Total', +} +_GOLD = {**_START, 'B4': 1500} +_MERGED = ['A5:B5'] +"""The gold file's merge; is_merged() is openpyxl's MergedCell rule, so B5, not A5.""" + +_DOC_TEXT = ['Presentation Instruction', 'Describe some customs in China.'] +_DOC_GOLD = ['presentation instruction', 'describe some customs in china.'] + + +def _cell(sheet_idx: object, coordinate: str, prop: str, spec: dict) -> dict: + return { + 'type': 'check_cell', + 'sheet_idx': sheet_idx, + 'coordinate': coordinate, + 'props': {prop: spec}, + } + + +def _rules() -> list[dict]: + """Two rules the starting file fails and one it already holds.""" + return [ + _cell(0, 'A2', 'value', {'method': 'eq', 'ref': 'Rent'}), + _cell('RNSheet1', 'B4', 'value', {'method': 'eq', 'ref': 1500}), + _cell(0, 'B5', 'merge', {'method': 'eq', 'ref': True}), + ] + + +def _task_config( + file_name: str = FILE, + *, + func: str = 'compare_table', + rules: list[dict] | None = None, + instruction: str = 'Fill in the total and merge the closing row.', + expected: bool = True, + task_id: str = TASK_ID, +) -> dict: + """An OSWorld task config shaped like the LibreOffice ones the importer reads.""" + remote = f'https://example.invalid/{file_name}' + download = { + 'type': 'download', + 'parameters': {'files': [{'path': f'{DESKTOP}/{file_name}', 'url': remote}]}, + } + evaluator: dict = { + 'func': func, + 'result': {'type': 'vm_file', 'path': f'{DESKTOP}/{file_name}'}, + } + if expected: + evaluator['expected'] = { + 'type': 'cloud_file', + 'path': f'https://example.invalid/gold/{file_name}', + } + if func == 'compare_table': + evaluator['options'] = {'rules': _rules() if rules is None else rules} + return { + 'id': task_id, + 'instruction': instruction, + 'config': [download, {'type': 'open', 'parameters': {'path': f'{DESKTOP}/{file_name}'}}], + 'evaluator': evaluator, + } + + +def _parse(config: dict, *, domain: str = CALC) -> Task: + return parse_task(config['id'], domain, TITLE, config) + + +def _import( + tmp_path: Path, config: dict, source: Path, gold: Path | None, *, domain: str = CALC +) -> tuple[ComputerUseEnvironment, Path]: + """Import one task from the local *source* and *gold* files, and load it back.""" + task = parse_task(config['id'], domain, TITLE, config) + out = tmp_path / 'environments' / 'osworld' + out.mkdir(parents=True, exist_ok=True) + install_shared(out) + build_environment(task, source, gold, out, load_checks_module()) + return load_environment_dir(out / task.task_id), out + + +def _calc_env(tmp_path: Path, *, rules: list[dict] | None = None): + source = make_xlsx(tmp_path / 'source.xlsx', [('Sheet1', _START, [])]) + gold = make_xlsx(tmp_path / 'gold.xlsx', [('Sheet1', _GOLD, _MERGED)]) + return _import(tmp_path, _task_config(rules=rules), source, gold) + + +def _doc_env(tmp_path: Path, *, gold_text=None): + source = make_docx(tmp_path / 'source.docx', _DOC_TEXT) + gold = make_docx(tmp_path / 'gold.docx', _DOC_GOLD if gold_text is None else gold_text) + config = _task_config(DOC, func='compare_docx_files') + return _import(tmp_path, config, source, gold, domain=WRITER) + + +def _run_verifier(env: ComputerUseEnvironment, home: Path, work: Path) -> dict: + """Run *env*'s verifier as Session.score does: its own HOME, the agent's by name.""" + for rel, text in env.files.items(): + (work / rel).parent.mkdir(parents=True, exist_ok=True) + (work / rel).write_text(text) + (work / '_final_verifier.py').write_text(env.final_verifier_code) + proc = subprocess.run( + [sys.executable, '-I', '_final_verifier.py'], + capture_output=True, + text=True, + cwd=work, + env={'HOME': str(work), 'DEEPGYM_AGENT_HOME': str(home)}, + timeout=60, + ) + assert proc.returncode == 0, proc.stderr + return json.loads(proc.stdout.strip().splitlines()[-1]) + + +def _save(tmp_path: Path, home: Path, cells: dict, merges: list[str], *, name: str = FILE) -> None: + """Save a workbook to ~/Desktop/ of the agent's home, where the task saves.""" + saved = make_xlsx(tmp_path / 'state.xlsx', [('Sheet1', cells, merges)]) + (home / 'Desktop').mkdir(parents=True, exist_ok=True) + shutil.copyfile(saved, home / 'Desktop' / name) + + +@pytest.fixture() +def work(tmp_path: Path) -> Path: + path = tmp_path / 'work' + path.mkdir() + return path + + +# --- The generated folder, as the loader and deepgym check see it. --- + + +def test_imported_env_loads_as_computer_use(tmp_path: Path) -> None: + env, out = _calc_env(tmp_path) + env_dir = out / TASK_ID + + assert isinstance(env, ComputerUseEnvironment) + assert sorted(path.name for path in env_dir.iterdir()) == [ + 'assets', + 'metadata.json', + 'setup.sh', + 'task.md', + 'variants.json', + 'verifier.py', + ] + # Spec files stay out of the session; the starting file is staged instead. + assert set(env.files) == {f'assets/{FILE}.b64'} + assert not any('checks' in rel for rel in env.files) + assert 'def report(' in env.final_verifier_code + asset = (env_dir / 'assets' / f'{FILE}.b64').read_text() + assert base64.b64decode(asset) == (tmp_path / 'source.xlsx').read_bytes() + assert f'assets/{FILE}.b64' in env.setup_script + assert env.task.startswith('Fill in the total and merge the closing row.') + assert '$' not in env.task + + +def test_metadata_and_registry_entry_describe_a_desktop_task(tmp_path: Path) -> None: + _, out = _calc_env(tmp_path) + metadata = json.loads((out / TASK_ID / 'metadata.json').read_text()) + assert metadata == { + 'id': TASK_ID, + 'name': TITLE, + 'type': 'computer-use', + 'difficulty': 'medium', + 'domain': 'libreoffice-calc', + 'tags': ['osworld', 'libreoffice', 'calc', 'desktop', 'gui'], + 'max_steps': 30, + } + assert build_registry_entry(_parse(_task_config())) == { + 'id': TASK_ID, + 'name': TITLE, + 'path': f'environments/osworld/{TASK_ID}', + 'type': 'computer-use', + 'benchmark': 'osworld', + 'variants': 1, + 'difficulty': 'medium', + 'domain': 'libreoffice-calc', + 'family': 'computer-use', + 'tags': ['osworld', 'libreoffice', 'calc', 'desktop', 'gui'], + } + + +def test_task_md_names_the_file_the_app_and_the_done_action() -> None: + text = build_task_md(_parse(_task_config())) + assert text.startswith('Fill in the total and merge the closing row.') + assert f'~/Desktop/{FILE}' in text + assert 'LibreOffice Calc' in text and 'Ctrl+S' in text and '`done`' in text + + writer = _parse(_task_config(DOC, func='compare_docx_files'), domain=WRITER) + assert 'LibreOffice Writer' in build_task_md(writer) + + # task.md is substituted as a string.Template at load time. + loud = _parse(_task_config(instruction='Show the $ total.')) + with pytest.raises(ValueError, match='contains'): + build_task_md(loud) + + +def test_setup_script_decodes_the_staged_file() -> None: + script = build_setup(_parse(_task_config())) + assert f'assets/{FILE}.b64' in script + assert f'"$HOME/Desktop/{FILE}"' in script + assert '--calc' in script and '--writer' not in script + assert 'sudo' not in script + + writer = _parse(_task_config(DOC, func='compare_docx_files'), domain=WRITER) + assert '--writer' in build_setup(writer) + + +# --- Scoring: the saved file, never the screen. --- + + +def test_scores_untouched_half_done_and_finished_files(tmp_path: Path, work: Path) -> None: + env, _ = _calc_env(tmp_path) + home = tmp_path / 'home' + for label, cells, merges, score in ( + ('untouched', _START, [], 0.0), + ('total only', {**_START, 'B4': 1500}, [], 0.5), + ('finished', _GOLD, _MERGED, 1.0), + ): + _save(tmp_path, home, cells, merges) + result = _run_verifier(env, home, work) + assert result['score'] == score, label + assert result['passed'] is (score == 1.0), label + assert {case['id'] for case in result['cases']} == { + 'workbook_saved', + 'rule0_check_cell', + 'rule1_check_cell', + 'rule2_check_cell', + }, label + + # Nothing saved: every check fails and the guard says why. + result = _run_verifier(env, tmp_path / 'empty-home', work) + assert result['score'] == 0.0 + assert 'workbook_saved' in result['details'] + + +def test_rules_the_starting_file_holds_become_guards(tmp_path: Path, work: Path) -> None: + """Saving the starting file untouched must earn nothing, so a rule it already + holds is a guard, and only the rules that need work are progress.""" + env, out = _calc_env(tmp_path) + variant = json.loads((out / TASK_ID / 'variants.json').read_text())[0] + + assert variant['result'] == f'Desktop/{FILE}' + assert [rule['coordinate'] for rule in variant['progress']] == ['B4', 'B5'] + assert [rule['coordinate'] for rule in variant['guards']] == ['A2'] + assert [rule['name'] for rule in variant['progress']] == [ + 'rule1_check_cell', + 'rule2_check_cell', + ] + + home = tmp_path / 'home' + _save(tmp_path, home, _START, []) + assert _run_verifier(env, home, work)['score'] == 0.0 + + +def test_a_sheet_rule_is_one_all_or_nothing_check(tmp_path: Path, work: Path) -> None: + """compare_table compares a whole sheet, so fixing one cell of it earns nothing; + and values within the rule's precision match, as pandas' rounding makes them.""" + head = {'A1': 'Name', 'B1': 'Amount', 'A2': 'a', 'A3': 'b', 'A4': 'c'} + start = {**head, 'B2': 1.0, 'B3': 2.5, 'B4': 3.0} + done = {**head, 'B2': 1.5, 'B3': 2.5, 'B4': 4.0} + source = make_xlsx(tmp_path / 'source.xlsx', [('Sheet1', start, [])]) + gold = make_xlsx(tmp_path / 'gold.xlsx', [('Sheet1', done, [])]) + rules = [{'type': 'sheet_data', 'sheet_idx0': 0, 'sheet_idx1': 'EI0'}] + env, _ = _import(tmp_path, _task_config(rules=rules), source, gold) + + home = tmp_path / 'home' + for label, cells, score in ( + ('untouched', start, 0.0), + ('one of the two cells fixed', {**start, 'B4': 4.0}, 0.0), + ('within the precision', {**start, 'B2': 1.50001, 'B4': 4.0}, 1.0), + ('past the precision', {**start, 'B2': 1.5002, 'B4': 4.0}, 0.0), + ('finished', done, 1.0), + ): + _save(tmp_path, home, cells, []) + assert _run_verifier(env, home, work)['score'] == score, label + + +def test_text_tasks_compare_the_saved_text(tmp_path: Path, work: Path) -> None: + env, out = _doc_env(tmp_path) + variant = json.loads((out / TASK_ID / 'variants.json').read_text())[0] + assert set(variant) == {'result', 'expected_text'} + assert variant['result'] == f'Desktop/{DOC}' + assert variant['expected_text'] == 'presentation instruction describe some customs in china.' + + home = tmp_path / 'home' + for label, text, score in ( + ('untouched', _DOC_TEXT, 0.0), + ('halfway', ['presentation instruction', 'Describe some customs in China.'], 0.0), + ('finished', _DOC_GOLD, 1.0), + ): + (home / 'Desktop').mkdir(parents=True, exist_ok=True) + make_docx(home / 'Desktop' / DOC, text) + assert _run_verifier(env, home, work)['score'] == score, label + + result = _run_verifier(env, tmp_path / 'empty-home', work) + assert result['score'] == 0.0 and 'document_saved' in result['details'] + + +# --- Imports that would produce a task the starting state already solves. --- + + +def test_a_task_the_starting_file_already_solves_is_rejected(tmp_path: Path) -> None: + source = make_xlsx(tmp_path / 'source.xlsx', [('Sheet1', _START, [])]) + gold = make_xlsx(tmp_path / 'gold.xlsx', [('Sheet1', _GOLD, _MERGED)]) + with pytest.raises(ValueError, match='needs no work'): + _import(tmp_path, _task_config(rules=[_rules()[0]]), source, gold) + + with pytest.raises(ValueError, match='needs no work'): + _doc_env(tmp_path, gold_text=_DOC_TEXT) + + +def test_a_reference_that_fails_a_rule_is_rejected(tmp_path: Path) -> None: + source = make_xlsx(tmp_path / 'source.xlsx', [('Sheet1', _START, [])]) + gold = make_xlsx(tmp_path / 'gold.xlsx', [('Sheet1', {**_START, 'B4': 1400}, _MERGED)]) + with pytest.raises(ValueError, match='reference file fails'): + _import(tmp_path, _task_config(), source, gold) + + +# --- Task configs the importer does not port. --- + + +def _updated(config: dict, path: tuple, value: object) -> dict: + """A deep copy of *config* with the value at the nested *path* replaced.""" + clone = copy.deepcopy(config) + node = clone + for key in path[:-1]: + node = node[key] + node[path[-1]] = value + return clone + + +def _rule(config: dict, rule: dict) -> dict: + return _updated(config, ('evaluator', 'options', 'rules'), [rule]) + + +def _cell_rule(method: str, *, sheet_idx: object = 0, **props: object) -> dict: + return { + 'type': 'check_cell', + 'sheet_idx': sheet_idx, + 'coordinate': 'A1', + 'props': props or {'value': {'method': method, 'ref': 1}}, + } + + +def _other_id(config: dict) -> dict: + return _updated(config, ('id',), 'another') + + +def _no_instruction(config: dict) -> dict: + return _updated(config, ('instruction',), ' ') + + +def _no_open(config: dict) -> dict: + return _updated(config, ('config',), config['config'][:1]) + + +def _two_downloads(config: dict) -> dict: + return _updated(config, ('config',), [config['config'][0], *config['config']]) + + +def _two_files(config: dict) -> dict: + other = {'path': f'{DESKTOP}/other.xlsx', 'url': 'https://example.invalid/other.xlsx'} + files = config['config'][0]['parameters']['files'] + return _updated(config, ('config', 0, 'parameters', 'files'), [*files, other]) + + +def _opens_another(config: dict) -> dict: + return _updated(config, ('config', 1, 'parameters', 'path'), f'{DESKTOP}/other.xlsx') + + +def _local_result(config: dict) -> dict: + return _updated(config, ('evaluator', 'result', 'type'), 'local_file') + + +def _result_names_another(config: dict) -> dict: + return _updated(config, ('evaluator', 'result', 'path'), f'{DESKTOP}/other.xlsx') + + +def _pptx_evaluator(config: dict) -> dict: + return _updated(config, ('evaluator', 'func'), 'compare_pptx_files') + + +def _no_rules(config: dict) -> dict: + return _updated(config, ('evaluator', 'options'), {}) + + +def _chart_rule(config: dict) -> dict: + return _rule(config, {'type': 'compare_chart'}) + + +def _half_sheet_data(config: dict) -> dict: + return _rule(config, {'type': 'sheet_data', 'sheet_idx0': 0}) + + +def _bare_cell_rule(config: dict) -> dict: + return _rule(config, {'type': 'check_cell', 'sheet_idx': 0, 'coordinate': 'A1'}) + + +def _style_prop(config: dict) -> dict: + return _rule(config, _cell_rule('eq', style={'method': 'eq', 'ref': 1})) + + +def _contains(config: dict) -> dict: + return _rule(config, _cell_rule('contains')) + + +def _bad_tolerance(config: dict) -> dict: + return _rule(config, _cell_rule('approx:tight')) + + +def _bad_index(config: dict) -> dict: + return _rule(config, _cell_rule('eq', sheet_idx='XX0')) + + +def _unstaged_name(config: dict) -> dict: + name = 'a;b.xlsx' + file = {'path': f'{DESKTOP}/{name}', 'url': 'https://example.invalid/x'} + patched = _updated(config, ('config', 0, 'parameters', 'files'), [file]) + return _updated(patched, ('config', 1, 'parameters', 'path'), f'{DESKTOP}/{name}') + + +_REJECTED = [ + ('no instruction', _no_instruction, 'no instruction'), + ('no open step', _no_open, 'one download and one open'), + ('two downloads', _two_downloads, 'one download and one open'), + ('two files in one download', _two_files, 'one downloaded file'), + ('opening another file than the one staged', _opens_another, 'not the downloaded'), + ('a result that is not the saved file', _local_result, 'is not the saved'), + ('a result that names another file', _result_names_another, 'is not the saved'), + ('an evaluator that is not ported', _pptx_evaluator, 'is not ported yet'), + ('compare_table without a rules list', _no_rules, 'without a rules list'), + ('a rule that is not ported', _chart_rule, 'is not ported yet'), + ('sheet_data without its second index', _half_sheet_data, "without 'sheet_idx1'"), + ('check_cell without props', _bare_cell_rule, 'without props'), + ('a prop that is not ported', _style_prop, 'is not ported yet'), + ('a comparison that is not ported', _contains, 'is not ported yet'), + ('a tolerance that is not a number', _bad_tolerance, 'bad'), + ('a sheet index that is not ported', _bad_index, 'is not ported yet'), + ('a file name that cannot be staged', _unstaged_name, 'is not stageable'), +] + + +@pytest.mark.parametrize( + ('label', 'mutate', 'match'), _REJECTED, ids=[case[0] for case in _REJECTED] +) +def test_a_task_the_importer_does_not_port_is_rejected(label: str, mutate, match: str) -> None: + with pytest.raises(ValueError, match=match): + _parse(mutate(_task_config())) + + +def test_a_config_that_is_not_the_task_asked_for_is_rejected() -> None: + with pytest.raises(ValueError, match='does not match'): + parse_task(TASK_ID, CALC, TITLE, _other_id(_task_config())) + + +def test_a_text_task_without_a_usable_reference_is_rejected() -> None: + with pytest.raises(ValueError, match='needs a reference file'): + _parse(_task_config(DOC, func='compare_docx_files', expected=False), domain=WRITER) + + odd = _task_config(DOC, func='compare_docx_files') + odd['evaluator']['expected'] = {'type': 'vm_file', 'path': f'{DESKTOP}/{DOC}'} + with pytest.raises(ValueError, match='is not a cloud_file'): + _parse(odd, domain=WRITER) + + +def test_the_imported_tasks_are_unique_libreoffice_tasks() -> None: + ids = [task_id for task_id, _, _ in TASKS] + assert len(set(ids)) == len(ids) + assert {domain for _, domain, _ in TASKS} == {CALC, WRITER} + titles = [title for _, _, title in TASKS] + assert len(set(titles)) == len(titles) and all(titles) + + +# --- Downloads: the fallbacks that make a blocked network work. --- + + +def test_fetch_task_falls_back_to_the_contents_api(monkeypatch: pytest.MonkeyPatch) -> None: + asked: list[str] = [] + + def fake(url: str, *, accept: str | None = None) -> bytes: + asked.append(url) + if url.startswith(_TASK_URL): + raise OSError('blocked') + return b'{"id": "abc"}' + + monkeypatch.setattr('import_osworld.download', fake) + assert fetch_task('abc', CALC) == {'id': 'abc'} + assert asked == [f'{_TASK_URL}/{CALC}/abc.json', f'{_TASK_API_URL}/{CALC}/abc.json'] + + def refuse(url: str, *, accept: str | None = None) -> bytes: + raise OSError('blocked') + + monkeypatch.setattr('import_osworld.download', refuse) + with pytest.raises(RuntimeError, match='raw.githubusercontent.com'): + fetch_task('abc', CALC) + + +def test_fetch_file_mirrors_huggingface_and_leaves_other_hosts_alone( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + asked: list[str] = [] + + def fake(url: str, *, accept: str | None = None) -> bytes: + asked.append(url) + if 'hf-mirror' not in url: + raise OSError('blocked') + return b'file bytes' + + monkeypatch.setattr('import_osworld.download', fake) + host = 'https://huggingface.co/datasets/x/y/resolve/main/a.xlsx' + fetch_file(host, tmp_path / 'a.xlsx') + assert asked == [host, host.replace('huggingface.co', 'hf-mirror.com')] + assert (tmp_path / 'a.xlsx').read_bytes() == b'file bytes' + + asked.clear() + + def any_host(url: str, *, accept: str | None = None) -> bytes: + asked.append(url) + return b'file bytes' + + monkeypatch.setattr('import_osworld.download', any_host) + fetch_file('https://example.com/a.xlsx', tmp_path / 'b.xlsx') + assert asked == ['https://example.com/a.xlsx'] From b3a62a06b248e29cad47ddae0f573367bbbfda0a Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 14:39:44 +0530 Subject: [PATCH 02/19] fix(osworld): bound the office readers, follow the docx main-part relationship and compare dates as dates --- deepgym/scripts/import_osworld.py | 1 - .../src/deepgym/envs/computer_use/_checks.py | 105 +++++++++++++++--- deepgym/tests/office_fixtures.py | 24 +++- deepgym/tests/test_import_osworld.py | 58 +++++++++- 4 files changed, 170 insertions(+), 18 deletions(-) diff --git a/deepgym/scripts/import_osworld.py b/deepgym/scripts/import_osworld.py index d9e0f183..6691bb1d 100644 --- a/deepgym/scripts/import_osworld.py +++ b/deepgym/scripts/import_osworld.py @@ -45,7 +45,6 @@ _TASK_API_URL = ( f'https://api.github.com/repos/{_OSWORLD_REPO}/contents/evaluation_examples/examples' ) - # (task id, OSWorld domain, environment name): the LibreOffice tasks whose evaluator # scores the file the task saves, so a DeepGym verifier can check them with no live # desktop. Chrome tasks are excluded: the desktop is locked down and offline. diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index e96559d0..88b790d4 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -55,8 +55,9 @@ def report(progress: dict[str, bool], guards: dict[str, bool]) -> None: # # The readers follow what OSWorld's own evaluators see through pandas, openpyxl # and python-docx: cached formula values (not formula text), empty cells as -# None, numbers int when integral (dates stay raw serials, so both sides of a -# comparison encode the same way), and docx text as python-docx reads it. +# None, numbers int when integral, a number shown as a date marked as one (openpyxl +# reads it as a datetime; check_cell reads the raw serial), and docx text as +# python-docx reads it. # office_rule_checks() evaluates compare_table's rules with the same semantics, # including DataFrame.round's rounding (numpy's, not Python's, half-to-even). @@ -67,6 +68,18 @@ def report(progress: dict[str, bool], guards: dict[str, bool]) -> None: _COORD_RE = re.compile(r'([A-Z]+)(\d+)') +_MAX_PART_BYTES = 32 << 20 +"""Largest unpacked XML part read from a saved file; a bigger one is unreadable.""" +_MAX_CELLS = 1_000_000 +"""Largest used range (rows x columns) read from a sheet; a bigger one is unreadable.""" + + +def _read_part(archive: zipfile.ZipFile, name: str) -> ET.Element: + """Parse one XML part, refusing one that unpacks past :data:`_MAX_PART_BYTES`.""" + if archive.getinfo(name).file_size > _MAX_PART_BYTES: + raise ValueError(f'{name} is larger than {_MAX_PART_BYTES} bytes') + return ET.fromstring(archive.read(name)) + def _coord_to_cell(coord: str) -> tuple[int, int]: """'C7' -> (row 7, column 3), both 1-based.""" @@ -91,7 +104,7 @@ def _number(text: str) -> int | float | None: def _cell_value(cell: ET.Element, shared: list[str]) -> object: - """The value of one element, as pandas reads it (dates stay serials).""" + """The value of one element, as pandas reads it (a date is its serial here).""" kind = cell.get('t') if kind == 'inlineStr': node = cell.find(f'{_XLSX_NS}is') @@ -115,6 +128,38 @@ def _cell_value(cell: ET.Element, shared: list[str]) -> object: return _number(value.text) +_DATE_FORMAT_IDS = frozenset({14, 15, 16, 17, 18, 19, 20, 21, 22, 45, 46, 47}) +"""Built-in number formats that show a date or time.""" +_FORMAT_NOISE = re.compile(r'"[^"]*"|\\.|_.|\*.|\[[^\]]*\]') +"""Quoted text, escapes, padding and [colour/locale/elapsed] sections of a format code.""" + + +def _date_styles(archive: zipfile.ZipFile) -> set[int]: + """Indexes of the cell styles whose number format shows a date or time. + + openpyxl reads a number in such a cell as a datetime, so pandas never equates it + with a plain number; :func:`_sheet_grid` marks those cells the same way. + """ + if 'xl/styles.xml' not in archive.namelist(): + return set() + root = _read_part(archive, 'xl/styles.xml') + custom = { + int(fmt.get('numFmtId', '-1')): fmt.get('formatCode', '') + for fmt in root.iter(f'{_XLSX_NS}numFmt') + } + xfs = root.find(f'{_XLSX_NS}cellXfs') + dated = set() + for index, xf in enumerate([] if xfs is None else xfs.findall(f'{_XLSX_NS}xf')): + fmt_id = int(xf.get('numFmtId', '0')) + code = custom.get(fmt_id) + if code is None: + if fmt_id in _DATE_FORMAT_IDS: + dated.add(index) + elif re.search(r'[dmyhs]', _FORMAT_NOISE.sub('', code.split(';')[0]), re.IGNORECASE): + dated.add(index) + return dated + + def _merge_ranges(root: ET.Element) -> list[str]: merges = root.find(f'{_XLSX_NS}mergeCells') if merges is None: @@ -132,8 +177,16 @@ def _trim(grid: list[list[object]]) -> list[list[object]]: return grid -def _sheet_grid(root: ET.Element, shared: list[str]) -> list[list[object]]: - """Rows of cell values over the sheet's used range; missing cells are None.""" +def _sheet_grid(root: ET.Element, shared: list[str], dated: set[int]) -> list[list[object]]: + """Rows of cell values over the sheet's used range; missing cells are None. + + The used range ends at the last non-empty row and column, as after :func:`_trim`. + A number in a date-formatted cell (style index in *dated*) becomes + ``{'date': serial}``, so it only equals the same serial shown as a date. + + Raises: + ValueError: If the used range holds more than :data:`_MAX_CELLS` cells. + """ cells: dict[tuple[int, int], object] = {} max_row = 0 max_col = 0 @@ -145,10 +198,17 @@ def _sheet_grid(root: ET.Element, shared: list[str]) -> list[list[object]]: ref = cell.get('r') if not ref: continue + value = _cell_value(cell, shared) + if value is None: + continue + if _is_number(value) and int(cell.get('s', '0')) in dated: + value = {'date': value} r, c = _coord_to_cell(ref) - cells[(r, c)] = _cell_value(cell, shared) + cells[(r, c)] = value max_row = max(max_row, r) max_col = max(max_col, c) + if max_row * max_col > _MAX_CELLS: + raise ValueError(f'used range of {max_row} x {max_col} cells is too large') grid: list[list[object]] = [] for r in range(1, max_row + 1): grid.append([cells.get((r, c)) for c in range(1, max_col + 1)]) @@ -157,8 +217,8 @@ def _sheet_grid(root: ET.Element, shared: list[str]) -> list[list[object]]: def _sheet_parts(archive: zipfile.ZipFile) -> list[tuple[str, str]]: """(sheet name, part name) for every sheet, in workbook order.""" - workbook = ET.fromstring(archive.read('xl/workbook.xml')) - rels = ET.fromstring(archive.read('xl/_rels/workbook.xml.rels')) + workbook = _read_part(archive, 'xl/workbook.xml') + rels = _read_part(archive, 'xl/_rels/workbook.xml.rels') targets = { rel.get('Id'): rel.get('Target') for rel in rels.findall(f'{_PKG_REL_NS}Relationship') } @@ -182,7 +242,8 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: Returns: Sheets in workbook order. A grid's cells are strings, numbers (int when - integral), booleans, error text, or None for an empty cell. + integral), ``{'date': serial}`` for a number shown as a date or time, + booleans, error text, or None for an empty cell. Raises: ValueError: If the file is missing or not a readable workbook. @@ -191,15 +252,16 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: with zipfile.ZipFile(path) as archive: shared_node = None if 'xl/sharedStrings.xml' in archive.namelist(): - shared_node = ET.fromstring(archive.read('xl/sharedStrings.xml')) + shared_node = _read_part(archive, 'xl/sharedStrings.xml') shared = [ ''.join(t.text or '' for t in si.iter(f'{_XLSX_NS}t')) for si in (shared_node if shared_node is not None else []) ] + dated = _date_styles(archive) sheets = [] for name, part in _sheet_parts(archive): - root = ET.fromstring(archive.read(part)) - sheets.append((name, _sheet_grid(root, shared), _merge_ranges(root))) + root = _read_part(archive, part) + sheets.append((name, _sheet_grid(root, shared, dated), _merge_ranges(root))) return sheets except (OSError, zipfile.BadZipFile, KeyError, ET.ParseError) as exc: raise ValueError(f'{path} is not a readable workbook: {exc}') from exc @@ -241,7 +303,17 @@ def read_docx_text(path: str) -> str: """ try: with zipfile.ZipFile(path) as archive: - root = ET.fromstring(archive.read('word/document.xml')) + # The main part is the one the package's officeDocument relationship names, + # as python-docx resolves it, not whatever sits at word/document.xml. + rels = _read_part(archive, '_rels/.rels') + main = [ + rel.get('Target', '').lstrip('/') + for rel in rels.findall(f'{_PKG_REL_NS}Relationship') + if rel.get('Type', '').endswith('/relationships/officeDocument') + ] + if len(main) != 1: + raise ValueError(f'{len(main)} main document parts') + root = _read_part(archive, main[0]) except (OSError, zipfile.BadZipFile, KeyError, ET.ParseError) as exc: raise ValueError(f'{path} is not a readable document: {exc}') from exc body = root.find(f'{_W_NS}body') @@ -279,9 +351,12 @@ def _is_number(value: object) -> bool: def _same_value(a: object, b: object) -> bool: """Elementwise equality as pandas uses it: None only equals None; numbers compare - across int/float (and bool, as Python's `==` does); other types must match.""" + across int/float (and bool, as Python's `==` does); other types must match, and + a date only equals the same date.""" if a is None or b is None: return a is None and b is None + if isinstance(a, dict) or isinstance(b, dict): + return a == b if isinstance(a, (int, float)) and isinstance(b, (int, float)): return float(a) == float(b) if isinstance(a, str) and isinstance(b, str): @@ -462,6 +537,8 @@ def grid_of(idx: object) -> list[list[object]] | None: try: if prop == 'value': value = cell_at(rows, rule['coordinate']) + if isinstance(value, dict): # check_cell reads a date's raw serial + value = value['date'] elif prop == 'merge': value = is_merged(merges, rule['coordinate']) else: diff --git a/deepgym/tests/office_fixtures.py b/deepgym/tests/office_fixtures.py index 71df6abd..4ba34d43 100644 --- a/deepgym/tests/office_fixtures.py +++ b/deepgym/tests/office_fixtures.py @@ -20,11 +20,15 @@ class Inline(str): """A string cell stored inline (t="inlineStr") instead of in sharedStrings.""" +class Date(int): + """A serial number in a cell formatted as a date (built-in number format 14).""" + + def make_xlsx(path: Path, sheets: list[tuple[str, dict[str, object], list[str]]]) -> Path: """Write a minimal workbook: one entry per sheet, (name, cells, merged ranges). - A cell value may be a number, a bool, a str (shared string) or an Inline (an - inline string); a cell left out is empty. Cells are 1-based references like 'C7'. + A cell value may be a number, a bool, a Date, a str (shared string) or an Inline + (an inline string); a cell left out is empty. Cells are 1-based references like 'C7'. """ shared: dict[str, int] = {} parts = [] @@ -33,6 +37,7 @@ def make_xlsx(path: Path, sheets: list[tuple[str, dict[str, object], list[str]]] members = { 'xl/workbook.xml': _workbook_xml([name for name, _, _ in parts]), 'xl/_rels/workbook.xml.rels': _rels_xml(len(parts)), + 'xl/styles.xml': _STYLES_XML, } members.update({f'xl/worksheets/sheet{index}.xml': xml for _, xml, index in parts}) if shared: @@ -46,6 +51,8 @@ def make_xlsx(path: Path, sheets: list[tuple[str, dict[str, object], list[str]]] def _cell_xml(ref: str, value: object, shared: dict[str, int]) -> str: if isinstance(value, bool): return f'{int(value)}' + if isinstance(value, Date): + return f'{int(value)}' if isinstance(value, (int, float)): return f'{value!r}' if isinstance(value, Inline): @@ -74,6 +81,13 @@ def _sheet_xml(cells: dict[str, object], merges: list[str], shared: dict[str, in return f'{body}{merge}' +_STYLES_XML = ( + f'' + '' +) +"""Style 0 is General, style 1 a date.""" + + def _shared_xml(shared: dict[str, int]) -> str: items = sorted(shared, key=shared.get) body = ''.join(f'{text}' for text in items) @@ -104,6 +118,12 @@ def make_docx(path: Path, paragraphs: list[str]) -> Path: for para in paragraphs ) document = f'{body}' + main = f'{_REL_NS}/officeDocument' + rels = ( + f'' + f'' + ) with zipfile.ZipFile(path, 'w') as archive: + archive.writestr('_rels/.rels', rels) archive.writestr('word/document.xml', document) return path diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 6fec4d58..201f3d8c 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -10,13 +10,14 @@ import shutil import subprocess import sys +import zipfile from pathlib import Path import pytest from deepgym import ComputerUseEnvironment from deepgym.registry import load_environment_dir -from tests.office_fixtures import make_docx, make_xlsx +from tests.office_fixtures import Date, make_docx, make_xlsx scripts_dir = Path(__file__).resolve().parent.parent / 'scripts' if str(scripts_dir) not in sys.path: @@ -302,6 +303,61 @@ def test_rules_the_starting_file_holds_become_guards(tmp_path: Path, work: Path) assert _run_verifier(env, home, work)['score'] == 0.0 +def test_a_date_only_matches_a_cell_shown_as_that_date(tmp_path: Path, work: Path) -> None: + """pandas reads a date-formatted cell as a datetime, so the right serial typed as + a plain number does not match the reference's date.""" + head = {'A1': 'Start', 'B1': 'Due'} + start = {**head, 'A2': Date(40437)} + done = {**start, 'B2': Date(40557)} + source = make_xlsx(tmp_path / 'source.xlsx', [('Sheet1', start, [])]) + gold = make_xlsx(tmp_path / 'gold.xlsx', [('Sheet1', done, [])]) + rules = [{'type': 'sheet_data', 'sheet_idx0': 0, 'sheet_idx1': 'EI0'}] + env, _ = _import(tmp_path, _task_config(rules=rules), source, gold) + + home = tmp_path / 'home' + for label, cells, score in ( + ('the serial as a plain number', {**start, 'B2': 40557}, 0.0), + ('the date', done, 1.0), + ): + _save(tmp_path, home, cells, []) + assert _run_verifier(env, home, work)['score'] == score, label + + +def test_a_saved_file_too_large_to_read_scores_zero(tmp_path: Path, work: Path) -> None: + """One cell at the far corner of a sheet would make a dense grid of a billion + cells: the workbook counts as unreadable (a scored 0), not a verifier crash.""" + env, _ = _calc_env(tmp_path) + home = tmp_path / 'home' + _save(tmp_path, home, {**_GOLD, 'AMJ1048576': 'x'}, _MERGED) + + result = _run_verifier(env, home, work) + + assert result['score'] == 0.0 + assert 'workbook_saved' in result['details'] + + +def test_the_main_document_part_is_the_one_the_package_names(tmp_path: Path) -> None: + """python-docx reads the part the officeDocument relationship names, so a stray + word/document.xml holding the expected text is not what gets scored.""" + checks = load_checks_module() + doc = make_docx(tmp_path / 'real.docx', _DOC_TEXT) + with zipfile.ZipFile(doc) as archive: + real = archive.read('word/document.xml') + forged = tmp_path / 'forged.docx' + main = 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/officeDocument' + with zipfile.ZipFile(forged, 'w') as archive: + archive.writestr( + '_rels/.rels', + '' + f'', + ) + archive.writestr('word/real.xml', real) + with zipfile.ZipFile(make_docx(tmp_path / 'gold.docx', _DOC_GOLD)) as gold: + archive.writestr('word/document.xml', gold.read('word/document.xml')) + + assert checks.read_docx_text(str(forged)) == checks.read_docx_text(str(doc)) + + def test_a_sheet_rule_is_one_all_or_nothing_check(tmp_path: Path, work: Path) -> None: """compare_table compares a whole sheet, so fixing one cell of it earns nothing; and values within the rule's precision match, as pandas' rounding makes them.""" From dcdf42ba125e0d2220b08358cb1168445f1fb487 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 14:39:53 +0530 Subject: [PATCH 03/19] fix(osworld): a rule that holds once an empty sheet is added is a guard, not progress --- deepgym/scripts/import_osworld.py | 24 ++++++++++++++++++++---- deepgym/tests/test_import_osworld.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/deepgym/scripts/import_osworld.py b/deepgym/scripts/import_osworld.py index 6691bb1d..f16cef1c 100644 --- a/deepgym/scripts/import_osworld.py +++ b/deepgym/scripts/import_osworld.py @@ -377,7 +377,9 @@ def build_variants(task: Task, source: Path, gold: Path | None, checks: ModuleTy Each rule is progress or a guard, decided here by evaluating it against the untouched starting file: a rule the file already satisfies is not something the task asks for, so it becomes a guard (keep it holding) instead of a progress - check that saving nothing would collect. + check that saving nothing would collect. So does a rule the starting file holds + once the sheets the rules name are added empty ("D1 is not merged"): adding a + blank sheet is not progress either. Raises: ValueError: If the reference file fails one of the task's own rules, or @@ -398,9 +400,23 @@ def build_variants(task: Task, source: Path, gold: Path | None, checks: ModuleTy failed = [name for name, ok in reference.items() if not ok] if failed: raise ValueError(f'{task.task_id}: the reference file fails {failed}') - untouched = checks.office_rule_checks(named, checks.read_sheets(str(source)), expected) - variant['progress'] = [rule for rule in named if not untouched[rule['name']]] - variant['guards'] = [rule for rule in named if untouched[rule['name']]] + start = checks.read_sheets(str(source)) + names = {sheet[0] for sheet in start} + new_sheets = { + str(rule[key])[2:] + for rule in named + for key in ('sheet_idx', 'sheet_idx0', 'sheet_idx1') + if str(rule.get(key, '')).startswith('RN') and str(rule[key])[2:] not in names + } + blank = start + [(name, [], []) for name in sorted(new_sheets)] + held = { + name + for sheets in (start, blank) + for name, ok in checks.office_rule_checks(named, sheets, expected).items() + if ok + } + variant['progress'] = [rule for rule in named if rule['name'] not in held] + variant['guards'] = [rule for rule in named if rule['name'] in held] if not variant['progress']: raise ValueError(f'{task.task_id}: needs no work; saving nothing would score') variant['expected'] = expected diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 201f3d8c..5edf26fc 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -303,6 +303,34 @@ def test_rules_the_starting_file_holds_become_guards(tmp_path: Path, work: Path) assert _run_verifier(env, home, work)['score'] == 0.0 +def test_a_rule_an_empty_new_sheet_holds_is_a_guard(tmp_path: Path, work: Path) -> None: + """'D1 is not merged' on a sheet the task adds holds as soon as an empty sheet is + added, so it is a guard: adding a blank Sheet2 earns nothing.""" + source = make_xlsx(tmp_path / 'source.xlsx', [('Sheet1', _START, [])]) + summary = {'A1': 'Summary'} + gold = make_xlsx( + tmp_path / 'gold.xlsx', [('Sheet1', _START, []), ('Sheet2', summary, ['A1:B1'])] + ) + rules = [ + _cell('RNSheet2', 'A1', 'value', {'method': 'eq', 'ref': 'Summary'}), + _cell('RNSheet2', 'B1', 'merge', {'method': 'eq', 'ref': True}), + _cell('RNSheet2', 'C1', 'merge', {'method': 'eq', 'ref': False}), + ] + env, out = _import(tmp_path, _task_config(rules=rules), source, gold) + variant = json.loads((out / TASK_ID / 'variants.json').read_text())[0] + assert [rule['coordinate'] for rule in variant['progress']] == ['A1', 'B1'] + assert [rule['coordinate'] for rule in variant['guards']] == ['C1'] + + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + for label, sheets, score in ( + ('blank Sheet2', [('Sheet1', _START, []), ('Sheet2', {}, [])], 0.0), + ('finished', [('Sheet1', _START, []), ('Sheet2', summary, ['A1:B1'])], 1.0), + ): + make_xlsx(home / 'Desktop' / FILE, sheets) + assert _run_verifier(env, home, work)['score'] == score, label + + def test_a_date_only_matches_a_cell_shown_as_that_date(tmp_path: Path, work: Path) -> None: """pandas reads a date-formatted cell as a datetime, so the right serial typed as a plain number does not match the reference's date.""" From 0a5ca24b8ead7cc7976fe4c1de5838862d3acf34 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 14:40:02 +0530 Subject: [PATCH 04/19] fix(osworld): run on a LibreOffice snapshot and fail setup when the document never opens --- deepgym/scripts/import_osworld.py | 64 +++++++++++++++++++++++----- deepgym/src/deepgym/models.py | 2 + deepgym/src/deepgym/registry.py | 24 ++++++----- deepgym/tests/test_import_osworld.py | 5 +++ 4 files changed, 74 insertions(+), 21 deletions(-) diff --git a/deepgym/scripts/import_osworld.py b/deepgym/scripts/import_osworld.py index f16cef1c..02a433cc 100644 --- a/deepgym/scripts/import_osworld.py +++ b/deepgym/scripts/import_osworld.py @@ -9,7 +9,7 @@ - setup.sh decodes the starting file and opens it in LibreOffice Calc/Writer - verifier.py scores the saved file with the task's own compare_table rules - variants.json the rules (progress and guards), the reference data, result path - - metadata.json type, domain, tags and step budget + - metadata.json type, domain, tags, step budget and the Daytona snapshot - assets/.b64 the starting file, staged into the session work directory The shared ``_checks.py`` (the readers and rule engine the verifier needs) is copied @@ -21,11 +21,13 @@ configs, instructions and files keep that license. Usage: - python scripts/import_osworld.py + python scripts/import_osworld.py # write the environments + python scripts/import_osworld.py --build-snapshot # once: the Daytona snapshot """ from __future__ import annotations +import argparse import base64 import importlib.resources import importlib.util @@ -45,6 +47,12 @@ _TASK_API_URL = ( f'https://api.github.com/repos/{_OSWORLD_REPO}/contents/evaluation_examples/examples' ) +SNAPSHOT = 'deepgym-osworld-libreoffice' +"""The Daytona snapshot every imported env boots; ``--build-snapshot`` creates it.""" +_SNAPSHOT_BASE = 'daytonaio/sandbox:0.9.0' +"""Daytona's desktop image (xfce, computer use) that the snapshot adds LibreOffice to.""" +_SNAPSHOT_PACKAGES = 'libreoffice-calc libreoffice-writer libreoffice-gtk3 fonts-dejavu-core' + # (task id, OSWorld domain, environment name): the LibreOffice tasks whose evaluator # scores the file the task saves, so a DeepGym verifier can check them with no live # desktop. Chrome tasks are excluded: the desktop is locked down and offline. @@ -271,8 +279,9 @@ def build_task_md(task: Task) -> str: def build_setup(task: Task) -> str: """The setup script: decode the starting file and open it in LibreOffice. - The document window is awaited like OSWorld's own open step does (by title), - with a plain wait as the fallback for images without wmctrl. + Setup waits for LibreOffice's lock file next to the document, which appears once + the document is open, and fails when it never does, so a desktop that can't open + the file gives no verdict rather than a 0. """ component = '--calc' if task.func == 'compare_table' else '--writer' return _SETUP.replace('', task.file_name).replace('', component) @@ -292,14 +301,16 @@ def build_setup(task: Task) -> str: mkdir -p "$(dirname "$file")" base64 -d "assets/.b64" > "$file" (setsid nohup "$soffice_app" --norestore "$file" >/tmp/deepgym_soffice.log 2>&1 &) -if command -v wmctrl >/dev/null 2>&1; then - for _ in $(seq 60); do - if wmctrl -l 2>/dev/null | grep -qF ''; then break; fi - sleep 1 - done -else - sleep 10 +lock="$HOME/Desktop/.~lock.#" +for _ in $(seq 90); do + if [ -e "$lock" ]; then break; fi + sleep 1 +done +if [ ! -e "$lock" ]; then + echo "LibreOffice did not open within 90 seconds." >&2 + exit 1 fi +sleep 3 """ @@ -443,6 +454,7 @@ def build_metadata(task: Task) -> str: 'domain': task.domain.replace('_', '-'), 'tags': _tags(task), 'max_steps': 30, + 'snapshot': SNAPSHOT, } return json.dumps(metadata, indent=2) + '\n' @@ -522,7 +534,36 @@ def install_shared(output_dir: Path) -> Path: return target +def build_snapshot() -> None: + """Create the Daytona snapshot the envs boot (:data:`SNAPSHOT`), once per account. + + Daytona builds it remotely from its desktop image plus LibreOffice; sessions run + offline, so the apps have to be in the image. + """ + from daytona import CreateSnapshotParams, Daytona, Image + + image = Image.base(_SNAPSHOT_BASE).run_commands( + 'if [ "$(id -u)" = 0 ]; then S=; else S="sudo -n"; fi; ' + '$S apt-get update && $S env DEBIAN_FRONTEND=noninteractive apt-get install -y ' + f'--no-install-recommends {_SNAPSHOT_PACKAGES} && $S rm -rf /var/lib/apt/lists/*' + ) + Daytona().snapshot.create( + CreateSnapshotParams(name=SNAPSHOT, image=image), on_logs=print, timeout=0 + ) + print(f'Created Daytona snapshot {SNAPSHOT}') + + def main() -> None: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + '--build-snapshot', + action='store_true', + help=f'create the Daytona snapshot {SNAPSHOT} the envs run on (needs DAYTONA_API_KEY)', + ) + if parser.parse_args().build_snapshot: + build_snapshot() + return + output_dir = Path.home() / '.deepgym' / 'environments' / 'osworld' output_dir.mkdir(parents=True, exist_ok=True) install_shared(output_dir) @@ -545,6 +586,7 @@ def main() -> None: json.dumps(entries, indent=2, ensure_ascii=False) + '\n', encoding='utf-8' ) print(f'Generated {len(entries)} OSWorld environments in {output_dir}') + print(f'They run on the Daytona snapshot {SNAPSHOT}; create it once with --build-snapshot.') if __name__ == '__main__': diff --git a/deepgym/src/deepgym/models.py b/deepgym/src/deepgym/models.py index cde43de1..62b4d213 100644 --- a/deepgym/src/deepgym/models.py +++ b/deepgym/src/deepgym/models.py @@ -210,6 +210,8 @@ class EnvMetadata(BaseModel): timeout: int = Field(default=30, ge=1) case_timeout: int = Field(default=10, ge=1) image: str | None = None + snapshot: str | None = None + """Multi-turn and computer-use envs: the Daytona snapshot their sessions boot.""" max_steps: int = Field(default=10, ge=1) timeout_per_step: int = Field(default=30, ge=1) repo_dir: str | None = None diff --git a/deepgym/src/deepgym/registry.py b/deepgym/src/deepgym/registry.py index 0bffb694..d39e9f00 100644 --- a/deepgym/src/deepgym/registry.py +++ b/deepgym/src/deepgym/registry.py @@ -324,6 +324,18 @@ def _read_reference_actions(env_dir: Path, variant: int = 0) -> list[Action] | N raise ValueError(f'{REFERENCE_ACTIONS}: {exc}') from exc +_SESSION_SETTINGS = ( + 'difficulty', + 'domain', + 'tags', + 'max_steps', + 'timeout_per_step', + 'image', + 'snapshot', +) +"""metadata.json keys a multi-turn or computer-use env takes as they are.""" + + def _load_computer_use_env(env_dir: Path, metadata: dict, variant: int) -> ComputerUseEnvironment: """Build a ComputerUseEnvironment from task.md, verifier.py, setup.sh, and app files. @@ -359,11 +371,7 @@ def _load_computer_use_env(env_dir: Path, metadata: dict, variant: int) -> Compu setup_script=setup_path.read_text(encoding='utf-8') if setup_path.exists() else '', files=files, reference_actions=_read_reference_actions(env_dir, variant), - **{ - key: metadata[key] - for key in ('difficulty', 'domain', 'tags', 'max_steps', 'timeout_per_step', 'image') - if key in metadata - }, + **{key: metadata[key] for key in _SESSION_SETTINGS if key in metadata}, ) @@ -405,11 +413,7 @@ def _load_multi_turn_env(env_dir: Path, settings: dict) -> MultiTurnEnvironment: final_verifier_code=''.join(future for future, _ in parts) + '\n\n'.join(rest for _, rest in parts), reference_actions=_read_reference_actions(env_dir), - **{ - key: settings[key] - for key in ('difficulty', 'domain', 'tags', 'max_steps', 'timeout_per_step', 'image') - if key in settings - }, + **{key: settings[key] for key in _SESSION_SETTINGS if key in settings}, ) diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 5edf26fc..0d208862 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -26,6 +26,7 @@ from import_osworld import ( # noqa: E402 _TASK_API_URL, _TASK_URL, + SNAPSHOT, TASKS, Task, build_environment, @@ -215,7 +216,9 @@ def test_metadata_and_registry_entry_describe_a_desktop_task(tmp_path: Path) -> 'domain': 'libreoffice-calc', 'tags': ['osworld', 'libreoffice', 'calc', 'desktop', 'gui'], 'max_steps': 30, + 'snapshot': SNAPSHOT, } + assert load_environment_dir(out / TASK_ID).snapshot == SNAPSHOT assert build_registry_entry(_parse(_task_config())) == { 'id': TASK_ID, 'name': TITLE, @@ -251,6 +254,8 @@ def test_setup_script_decodes_the_staged_file() -> None: assert f'"$HOME/Desktop/{FILE}"' in script assert '--calc' in script and '--writer' not in script assert 'sudo' not in script + # A document LibreOffice never opens fails setup (no verdict), not the episode. + assert f'.~lock.{FILE}#' in script and 'exit 1' in script writer = _parse(_task_config(DOC, func='compare_docx_files'), domain=WRITER) assert '--writer' in build_setup(writer) From 1c584fbb08b0a5f0cfff345aa1b1d8ed94f805f6 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 14:40:11 +0530 Subject: [PATCH 05/19] feat(osworld): ship a reference episode for every imported task --- deepgym/scripts/import_osworld.py | 9 + .../scripts/osworld_reference_actions.json | 243 ++++++++++++++++++ deepgym/tests/test_import_osworld.py | 12 +- 3 files changed, 263 insertions(+), 1 deletion(-) create mode 100644 deepgym/scripts/osworld_reference_actions.json diff --git a/deepgym/scripts/import_osworld.py b/deepgym/scripts/import_osworld.py index 02a433cc..539348fd 100644 --- a/deepgym/scripts/import_osworld.py +++ b/deepgym/scripts/import_osworld.py @@ -11,6 +11,7 @@ - variants.json the rules (progress and guards), the reference data, result path - metadata.json type, domain, tags, step budget and the Daytona snapshot - assets/.b64 the starting file, staged into the session work directory + - reference_actions.json a scripted episode that saves the reference end state The shared ``_checks.py`` (the readers and rule engine the verifier needs) is copied next to the environments, and ``registry.json`` indexes them all, so the imported @@ -53,6 +54,9 @@ """Daytona's desktop image (xfce, computer use) that the snapshot adds LibreOffice to.""" _SNAPSHOT_PACKAGES = 'libreoffice-calc libreoffice-writer libreoffice-gtk3 fonts-dejavu-core' +_REFERENCE_ACTIONS = Path(__file__).with_name('osworld_reference_actions.json') +"""One scripted episode per task that saves the task's reference end state.""" + # (task id, OSWorld domain, environment name): the LibreOffice tasks whose evaluator # scores the file the task saves, so a DeepGym verifier can check them with no live # desktop. Chrome tasks are excluded: the desktop is locked down and offline. @@ -568,6 +572,7 @@ def main() -> None: output_dir.mkdir(parents=True, exist_ok=True) install_shared(output_dir) checks = load_checks_module() + episodes = json.loads(_REFERENCE_ACTIONS.read_text(encoding='utf-8'))['episodes'] entries = [] for task_id, domain, title in TASKS: @@ -581,6 +586,10 @@ def main() -> None: gold = Path(tmp) / Path(task.gold_url).name fetch_file(task.gold_url, gold) entries.append(build_environment(task, source, gold, output_dir, checks)) + (output_dir / task_id / 'reference_actions.json').write_text( + json.dumps([episodes[task_id]], indent=1, ensure_ascii=False) + '\n', + encoding='utf-8', + ) (output_dir / 'registry.json').write_text( json.dumps(entries, indent=2, ensure_ascii=False) + '\n', encoding='utf-8' diff --git a/deepgym/scripts/osworld_reference_actions.json b/deepgym/scripts/osworld_reference_actions.json new file mode 100644 index 00000000..aaea0861 --- /dev/null +++ b/deepgym/scripts/osworld_reference_actions.json @@ -0,0 +1,243 @@ +{ + "source": "Episodes for the OSWorld tasks scripts/import_osworld.py imports (https://github.com/xlang-ai/OSWorld, Apache-2.0); the values they enter come from each task's reference file. Coordinates are for the 1024x768 Daytona desktop.", + "episodes": { + "42e0a640-4f19-4b28-973d-729602b5a4a7": [ + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 395, "y": 288}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "A1:B2\n"}, + {"action_type": "type", "content": "Total Revenue\tTotal Expenses\t867786\t411686\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "abed40dc-063f-4598-8ba5-9fe749c0615d": [ + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "D2:D7\n"}, + {"action_type": "type", "content": "Keira Daily\n=\"50 Dollar Dollar\"\nJonah Mountain\nJeff Tunnels\nEmma Stoned\nMorgan Prisoner\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "357ef137-7eeb-4c80-a3bb-0951f26a8aff": [ + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "E3\n"}, + {"action_type": "type", "content": "=D3*24*F3\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "1273e544-688f-496b-8d89-3e0f40aa0606": [ + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 395, "y": 288}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "A1:A20\n"}, + {"action_type": "type", "content": "Revenue\n39929\n55389\n52686\n49490\n38279\n30002\n56315\n54870\n39465\n26176\n32121\n59875\n42235\n55101\n35972\n51922\n42471\n46706\n58782\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "1e8df695-bd1b-45b3-b557-e7d599cf7597": [ + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "D1:D11\n"}, + {"action_type": "type", "content": "Profit\n11577.1772459512\n70773.46975547624\n82267.61299838945\n4763.329726733122\n50481.343477804636\n62432.32320034776\n40804.22167717412\n20648.43926580814\n97994.65439723036\n28668.942190516413\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "4de54231-e4b5-49e3-b2ba-61a0bec721c0": [ + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "B3:B30\n"}, + {"action_type": "type", "content": "=9.8*((A3-0.75*SIN(RADIANS(45))-0.25*0.75*COS(RADIANS(45)))/(A3+0.75))"}, + {"action_type": "key", "content": "alt+Return"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "D3:D30\n"}, + {"action_type": "type", "content": "=-9.8*((-C3+0.75*SIN(RADIANS(45))-0.25*0.75*COS(RADIANS(45)))/(C3+0.75))"}, + {"action_type": "key", "content": "alt+Return"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "E1\n"}, + {"action_type": "type", "content": "Combined Data\n"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "E2:E30\n"}, + {"action_type": "type", "content": "=$A$1&\": \"&FIXED(A2,2)&\", \"&$B$1&\": \"&FIXED(B2,2)&\", \"&$C$1&\": \"&FIXED(C2,2)&\", \"&$D$1&\": \"&FIXED(D2,2)"}, + {"action_type": "key", "content": "alt+Return"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "4172ea6e-6b77-4edb-a9cc-c0014bd1603b": [ + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "C1:C10\n"}, + {"action_type": "type", "content": "Maturity Date\n2011-01-14\n2011-03-09\n2011-04-05\n2010-12-19\n2011-03-22\n2011-02-14\n2011-05-12\n2011-03-17\n2010-11-24\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "37608790-6147-45d0-9f20-1137bb35703d": [ + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "B2:D22\n"}, + {"action_type": "type", "content": "Benedict\tCucumberbach\tManager\tBlake\tDreary\tCEO\tBey\tTwice\tDirector\tArnold\tShawarma\tCFO\tClint\tEastwood\tChairman\tOlivia\tCalme\tPresident\tParah\tSalin\tAccountant\tElijah\tPlastic\tSupervisor\tTom\tSofty\tTrainer\tJohn\tBadman\tAuditor\tKanye\t=\"East\"\tCTO\tReese\tWithoutaspoon\tAnalyst\tSwan\tRonson\tResearcher\tJonah\tMountain\tFounder\tJeff\tTunnels\tPromoter\tKeira\tDaily\tLegal\tMorgan\tPrisoner\tAssistant\tEmma\tStoned\tConsultant\t"}, + {"action_type": "type", "content": "Leonardo\tDihatric\tCopywriter\tTom\tDrive\tReceptionist\tSelena\tStopmez\tClerk\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "04d9aeaf-7bed-4024-bedb-e10e6f00eb7f": [ + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 395, "y": 288}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "A1:D6\n"}, + {"action_type": "type", "content": "Year\tCA changes\tFA changes\tOA changes\t2015\t0.101490720694521\t-0.0496043956043956\t-0.0167597765363128\t2016\t0.0721762896830247\t-0.0555696875794926\t0.0585227272727273\t2017\t0.134201897952018\t-0.0592801175318315\t0.0764895330112721\t2018\t0.0646274093551733\t-0.0667638408079336\t0.00473697332336076\t2019\t0.0655457868817789\t-0.0745523512020974\t0.0143920595533499\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "26a8440e-c166-4c50-aef4-bfb77314b46b": [ + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 395, "y": 288}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "A1:B7\n"}, + {"action_type": "type", "content": "Month\tTotal\tJan\t4852\tFeb\t4587\tMar\t3943\tApr\t3857\tMay\t4677\tJun\t4243\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "035f41ba-6653-43ab-aa63-c86d449d62e5": [ + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "J2:J10\n"}, + {"action_type": "type", "content": "55000\n47662\n53451\n60819\n62864\n57924\n59634\n50852\n43649\n"}, + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 395, "y": 288}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "A1:A10\n"}, + {"action_type": "type", "content": "=\"Year_Profit\"\n=\"2015_55000\"\n=\"2016_47662\"\n=\"2017_53451\"\n=\"2018_60819\"\n=\"2019_62864\"\n=\"2020_57924\"\n=\"2021_59634\"\n=\"2022_50852\"\n=\"2023_43649\"\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "1d17d234-e39d-4ed7-b46f-4417922a4e7c": [ + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 395, "y": 288}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "A1:C1\n"}, + {"action_type": "click", "x": 211, "y": 36}, + {"action_type": "click", "x": 294, "y": 263}, + {"action_type": "click", "x": 548, "y": 288}, + {"action_type": "type", "content": "Investment Summary\n"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "A2:B2\n"}, + {"action_type": "click", "x": 211, "y": 36}, + {"action_type": "click", "x": 294, "y": 263}, + {"action_type": "click", "x": 548, "y": 288}, + {"action_type": "type", "content": "High Interest Rate\n"}, + {"action_type": "click", "x": 70, "y": 146}, + {"action_type": "type", "content": "C2:D2\n"}, + {"action_type": "click", "x": 211, "y": 36}, + {"action_type": "click", "x": 294, "y": 263}, + {"action_type": "click", "x": 548, "y": 288}, + {"action_type": "type", "content": "Low Interest Rate\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "0cecd4f3-74de-457b-ba94-29ad6b5dafb6": [ + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 383, "y": 515}, + {"action_type": "wait", "amount": 2}, + {"action_type": "type", "content": "LARS Resources\n"}, + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 381, "y": 616}, + {"action_type": "wait", "amount": 2}, + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 383, "y": 515}, + {"action_type": "wait", "amount": 2}, + {"action_type": "type", "content": "LARS Resources (Backup)\n"}, + {"action_type": "key", "content": "ctrl+Page_Down"}, + {"action_type": "click", "x": 325, "y": 36}, + {"action_type": "click", "x": 383, "y": 515}, + {"action_type": "wait", "amount": 2}, + {"action_type": "type", "content": "LARS Resources (Offline)\n"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "adf5e2c3-64c7-4644-b7b6-d2f0167927e7": [ + {"action_type": "key", "content": "ctrl+h"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "type", "content": ""}, + {"action_type": "click", "x": 540, "y": 323}, + {"action_type": "type", "content": "[14]"}, + {"action_type": "click", "x": 782, "y": 378}, + {"action_type": "wait", "amount": 2}, + {"action_type": "click", "x": 810, "y": 641}, + {"action_type": "key", "content": "ctrl+End"}, + {"action_type": "key", "content": "Return"}, + {"action_type": "type", "content": "Steinberg, F. M., Bearden, M. M., & Keen, C. L. (2003). Cocoa and chocolate flavonoids: Implications for cardiovascular health. Journal of the American Dietetic Association, 103(2), 215-223. doi: 10.1053/jada.2003.50028"}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "d53ff5ee-3b1a-431e-b2be-30ed2673079b": [ + {"action_type": "key", "content": "ctrl+a"}, + {"action_type": "click", "x": 211, "y": 36}, + {"action_type": "click", "x": 224, "y": 61}, + {"action_type": "click", "x": 540, "y": 390}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ], + "e528b65e-1107-4b8c-8988-490e4fece599": [ + {"action_type": "key", "content": "ctrl+a"}, + {"action_type": "click", "x": 211, "y": 36}, + {"action_type": "click", "x": 224, "y": 61}, + {"action_type": "click", "x": 578, "y": 466}, + {"action_type": "key", "content": "ctrl+s"}, + {"action_type": "wait", "amount": 2}, + {"action_type": "key", "content": "Return"}, + {"action_type": "wait", "amount": 3}, + {"action_type": "done"} + ] + } +} diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 0d208862..4f64a0eb 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -15,7 +15,7 @@ import pytest -from deepgym import ComputerUseEnvironment +from deepgym import Action, ComputerUseEnvironment from deepgym.registry import load_environment_dir from tests.office_fixtures import Date, make_docx, make_xlsx @@ -24,6 +24,7 @@ sys.path.insert(0, str(scripts_dir)) from import_osworld import ( # noqa: E402 + _REFERENCE_ACTIONS, _TASK_API_URL, _TASK_URL, SNAPSHOT, @@ -610,6 +611,15 @@ def test_the_imported_tasks_are_unique_libreoffice_tasks() -> None: assert len(set(titles)) == len(titles) and all(titles) +def test_every_imported_task_ships_a_reference_episode_within_its_budget() -> None: + episodes = json.loads(_REFERENCE_ACTIONS.read_text(encoding='utf-8'))['episodes'] + assert set(episodes) == {task_id for task_id, _, _ in TASKS} + for task_id, actions in episodes.items(): + parsed = [Action.model_validate(action) for action in actions] + assert parsed[-1].action_type == 'done', task_id + assert len(parsed) <= 30, task_id # metadata.json's max_steps + + # --- Downloads: the fallbacks that make a blocked network work. --- From 9786977bcb65062c7374e8e99d8ac24c2bb906ac Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 14:40:20 +0530 Subject: [PATCH 06/19] fix(osworld): write OSWorld's license and a NOTICE next to the generated envs --- deepgym/scripts/import_osworld.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/deepgym/scripts/import_osworld.py b/deepgym/scripts/import_osworld.py index 539348fd..3380b273 100644 --- a/deepgym/scripts/import_osworld.py +++ b/deepgym/scripts/import_osworld.py @@ -19,7 +19,8 @@ scores the file the task saves, never the screen. OSWorld is Apache-2.0 (https://github.com/xlang-ai/OSWorld); the imported task -configs, instructions and files keep that license. +configs, instructions and files keep that license, and the output folder gets a copy +of it plus a NOTICE saying what was changed. Usage: python scripts/import_osworld.py # write the environments @@ -48,6 +49,11 @@ _TASK_API_URL = ( f'https://api.github.com/repos/{_OSWORLD_REPO}/contents/evaluation_examples/examples' ) +_LICENSE_URLS = [ + f'https://raw.githubusercontent.com/{_OSWORLD_REPO}/main/LICENSE', + f'https://api.github.com/repos/{_OSWORLD_REPO}/contents/LICENSE', +] + SNAPSHOT = 'deepgym-osworld-libreoffice' """The Daytona snapshot every imported env boots; ``--build-snapshot`` creates it.""" _SNAPSHOT_BASE = 'daytonaio/sandbox:0.9.0' @@ -57,6 +63,18 @@ _REFERENCE_ACTIONS = Path(__file__).with_name('osworld_reference_actions.json') """One scripted episode per task that saves the task's reference end state.""" +_NOTICE = f"""These environments were generated by DeepGym's scripts/import_osworld.py from +OSWorld (https://github.com/{_OSWORLD_REPO}), which is licensed under the Apache +License 2.0; a copy is in LICENSE next to this file. + +Each / folder is derived from the OSWorld task config +evaluation_examples/examples//.json and the files it names: +task.md adds where the file is and how to save it to the task's instruction, +verifier.py and variants.json port the task's evaluator, and assets/ holds its +starting file. reference_actions.json was written for DeepGym from the task's +reference file. +""" + # (task id, OSWorld domain, environment name): the LibreOffice tasks whose evaluator # scores the file the task saves, so a DeepGym verifier can check them with no live # desktop. Chrome tasks are excluded: the desktop is locked down and offline. @@ -594,6 +612,10 @@ def main() -> None: (output_dir / 'registry.json').write_text( json.dumps(entries, indent=2, ensure_ascii=False) + '\n', encoding='utf-8' ) + (output_dir / 'LICENSE').write_bytes( + download_first(_LICENSE_URLS, accept='application/vnd.github.raw') + ) + (output_dir / 'NOTICE').write_text(_NOTICE, encoding='utf-8') print(f'Generated {len(entries)} OSWorld environments in {output_dir}') print(f'They run on the Daytona snapshot {SNAPSHOT}; create it once with --build-snapshot.') From c385c1535fb322c0cf678f21632e5f695ad2107f Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 14:40:29 +0530 Subject: [PATCH 07/19] fix(check): computer-use folders need reference_actions.json, and no desktop is inconclusive --- deepgym/src/deepgym/factory/check.py | 32 ++++++++----------------- deepgym/tests/test_factory.py | 36 +++++++++++----------------- 2 files changed, 24 insertions(+), 44 deletions(-) diff --git a/deepgym/src/deepgym/factory/check.py b/deepgym/src/deepgym/factory/check.py index 2ba4a2bd..75f14700 100644 --- a/deepgym/src/deepgym/factory/check.py +++ b/deepgym/src/deepgym/factory/check.py @@ -59,9 +59,8 @@ def check_env(env_dir: Path, dg: DeepGym) -> list[QACheck]: Returns: files, metadata and verifier_source; then, if those pass, reference_passes, deterministic, empty_fails, stub_fails (single-turn) and protocol; then approval. - A desktop env whose reference can't run here (it ships no episode to replay, - or the backend gives no desktop) reports a single skipped reference_passes - instead of the run checks. + A desktop env off the Daytona backend reports a single inconclusive + reference_passes instead of the run checks: nothing here can replay it. """ if (env_dir / 'task.toml').is_file(): return [_harbor_check(env_dir), approval_check(env_dir)] @@ -120,12 +119,11 @@ def _files_check(env_dir: Path, metadata: EnvMetadata | None) -> QACheck: problems.append('task.md is missing or empty (it is the prompt the model sees)') if not (env_dir / 'verifier.py').is_file(): problems.append('verifier.py is missing (it scores every solution)') - if metadata is not None and metadata.type == 'multi-turn': + if metadata is not None and metadata.type in ('multi-turn', 'computer-use'): if not (env_dir / REFERENCE_ACTIONS).is_file(): problems.append(f'{REFERENCE_ACTIONS} is missing (the episode that must score 1.0)') - elif metadata is None or metadata.type != 'computer-use': - if not any((env_dir / name).is_file() for name in REFERENCE_FILES): - problems.append(f'{" or ".join(REFERENCE_FILES)} is missing (it must score 1.0)') + elif not any((env_dir / name).is_file() for name in REFERENCE_FILES): + problems.append(f'{" or ".join(REFERENCE_FILES)} is missing (it must score 1.0)') root = env_dir.resolve() for rel in metadata.support_files if metadata else []: path = (env_dir / rel).resolve() @@ -133,10 +131,7 @@ def _files_check(env_dir: Path, metadata: EnvMetadata | None) -> QACheck: problems.append(f'support file {rel!r} is not a file inside the folder') if problems: return QACheck(name='files', status='fail', details='; '.join(problems)) - details = 'task, verifier, reference and metadata' - if metadata is not None and metadata.type == 'computer-use': - details = 'task, verifier and metadata (a desktop env may ship a reference episode)' - return QACheck(name='files', status='pass', details=details) + return QACheck(name='files', status='pass', details='task, verifier, reference and metadata') def _verifier_check(env_dir: Path) -> QACheck: @@ -179,9 +174,8 @@ def lint_env(env_dir: Path) -> list[str]: def _run_checks(env_dir: Path, dg: DeepGym) -> list[QACheck]: """Run the reference, empty and stub solutions (or episodes) and judge them. - A desktop env whose reference can't run here — one with no episode to replay, or - one off the Daytona backend — reports a single skipped ``reference_passes``: no - local session can drive a desktop. + A desktop env off the Daytona backend reports a single inconclusive + ``reference_passes``: no local session can drive a desktop, so there is no evidence. """ try: env = load_environment_dir(env_dir) @@ -194,16 +188,10 @@ def _run_checks(env_dir: Path, dg: DeepGym) -> list[QACheck]: except SyntaxError as exc: details = f'verifier.py with its support files prepended: line {exc.lineno}: {exc.msg}' return [QACheck(name='load', status='fail', details=details)] - if env.reference_actions is None: - details = ( - f'this env ships no {REFERENCE_ACTIONS}: ' - 'an agent has to drive it, not a script to replay' - ) - return [QACheck(name='reference_passes', status='skip', details=details)] if env.desktop and dg.backend != 'daytona': details = 'a computer-use env needs a desktop: run with --mode daytona' - return [QACheck(name='reference_passes', status='skip', details=details)] - results = episode_results(env, env.reference_actions, dg, _RERUNS) + return [QACheck(name='reference_passes', status='inconclusive', details=details)] + results = episode_results(env, env.reference_actions or [], dg, _RERUNS) return [*run_verdicts(results, runs), protocol_check(results)] results = single_turn_results(env, env.reference_solution or '', dg, _RERUNS) return [*single_turn_verdicts(results, _RERUNS), protocol_check(results)] diff --git a/deepgym/tests/test_factory.py b/deepgym/tests/test_factory.py index ee8df478..9211038b 100644 --- a/deepgym/tests/test_factory.py +++ b/deepgym/tests/test_factory.py @@ -468,9 +468,11 @@ def test_check_rejects_invalid_reference_actions(tmp_path: Path, local_dg: DeepG assert check.details.startswith('reference_actions.json: ') -def test_check_skips_the_reference_a_desktop_env_cannot_run(local_dg: DeepGym) -> None: +def test_check_is_inconclusive_when_no_desktop_can_replay_the_reference( + local_dg: DeepGym, +) -> None: """A computer-use folder's reference episode drives a real desktop, so a local - session can't replay it: the run checks are a skip, not a failure.""" + session can't replay it: the run checks are inconclusive, never a pass.""" builtin = Path(__file__).resolve().parent.parent / 'src' / 'deepgym' / 'envs' folder = builtin / 'computer_use' / 'signup_form' @@ -479,35 +481,25 @@ def test_check_skips_the_reference_a_desktop_env_cannot_run(local_dg: DeepGym) - assert checks['files'].status == 'pass' assert checks['metadata'].status == 'pass' assert checks['verifier_source'].status == 'pass' - assert checks['reference_passes'].status == 'skip' + assert checks['reference_passes'].status == 'inconclusive' assert 'desktop' in checks['reference_passes'].details -def test_check_skips_a_desktop_env_with_no_reference_episode( +def test_check_requires_a_desktop_env_to_ship_its_reference_episode( tmp_path: Path, local_dg: DeepGym ) -> None: - """A desktop env an agent drives ships no episode to replay: the run checks are a - skip, not a failure over a missing reference_actions.json.""" - shared = Path(__file__).resolve().parent.parent / 'src' / 'deepgym' / 'envs' / 'computer_use' - root = tmp_path / 'computer_use' - folder = root / 'calc_task' - folder.mkdir(parents=True) - (root / '_checks.py').write_text( - (shared / '_checks.py').read_text(encoding='utf-8'), encoding='utf-8' - ) - shutil.copytree(shared / '_shared', root / '_shared') - (folder / 'task.md').write_text('Do the thing.', encoding='utf-8') - (folder / 'verifier.py').write_text('report(1.0, {})\n', encoding='utf-8') - (folder / 'variants.json').write_text('[{}]', encoding='utf-8') - (folder / 'metadata.json').write_text( + """A desktop env needs reference_actions.json, not a reference_solution.*.""" + (tmp_path / 'task.md').write_text('Do the thing.', encoding='utf-8') + (tmp_path / 'verifier.py').write_text('report(1.0, {})\n', encoding='utf-8') + (tmp_path / 'metadata.json').write_text( '{"name": "calc_task", "type": "computer-use"}', encoding='utf-8' ) - checks = {check.name: check for check in check_env(folder, local_dg)} + checks = {check.name: check for check in check_env(tmp_path, local_dg)} - assert checks['files'].status == 'pass' - assert checks['reference_passes'].status == 'skip' - assert 'an agent has to drive it' in checks['reference_passes'].details + assert checks['files'].status == 'fail' + assert 'reference_actions.json is missing' in checks['files'].details + assert 'reference_solution' not in checks['files'].details def test_check_skips_image_based_repo_patch_folders(tmp_path: Path, local_dg: DeepGym) -> None: From 9dc521e1c1163b137ae2fb0b54d45a62f8bbfdd1 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 14:40:39 +0530 Subject: [PATCH 08/19] docs: OSWorld importer section and changelog entry --- deepgym/CHANGELOG.md | 2 +- deepgym/docs/environments/importers.md | 34 ++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/deepgym/CHANGELOG.md b/deepgym/CHANGELOG.md index 5ed57bb7..e9de9876 100644 --- a/deepgym/CHANGELOG.md +++ b/deepgym/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- **OSWorld computer-use tasks:** `scripts/import_osworld.py` imports 16 of OSWorld's (Apache-2.0) LibreOffice Calc and Writer tasks as computer-use environments, one folder per task. The agent gets the task's starting file open in Calc or Writer and saves it; `verifier.py` scores the file it saved — with the task's own `compare_table` rules, ported into the shared `_checks.py` (workbook and document readers, the rule engine, pandas' rounding) and cross-checked rule-by-rule against OSWorld's own evaluator, or its reference text for the Writer tasks — never a screenshot. Each rule the starting file already satisfies is a guard rather than a progress check, so an untouched file or a vanished one scores 0 while a partial solution earns the fraction of its rules it has done (the evaluator was all-or-nothing). The importer downloads the task configs and files over the GitHub contents API and the hf-mirror host when the usual hosts are blocked. `deepgym check` now runs on a computer-use folder: it demanded a `reference_solution.*` every desktop env lacks and replayed a reference a local session can't run; a computer-use folder now passes the files check and reports a skipped `reference_passes` instead — no episode to replay, or no desktop (the skips `deepgym qa` already makes; `--mode daytona` replays the built-ins' episodes) — and `lint_env` reads a computer-use verifier the way the loader prepends it. +- **OSWorld computer-use tasks:** `scripts/import_osworld.py` imports 16 of OSWorld's (Apache-2.0) LibreOffice Calc and Writer tasks as computer-use environments, each with a reference episode, and writes OSWorld's license and a NOTICE next to them. They boot the Daytona snapshot `deepgym-osworld-libreoffice` (`--build-snapshot` creates it), since Daytona's default image has no LibreOffice; `metadata.json` takes a `snapshot`. The verifier scores the saved file with the task's own `compare_table` rules (ported into the shared `_checks.py`, standard library only) or its reference text, never a screenshot. Rules the starting file already holds, or holds once an empty sheet is added, are guards, so an untouched file or a blank sheet scores 0; a file too large or malformed to read scores 0. `deepgym check` asks a computer-use folder for `reference_actions.json` instead of `reference_solution.*`, and is inconclusive on one off Daytona. ## 0.5.0 (2026-10-14) diff --git a/deepgym/docs/environments/importers.md b/deepgym/docs/environments/importers.md index a5056867..645d6b65 100644 --- a/deepgym/docs/environments/importers.md +++ b/deepgym/docs/environments/importers.md @@ -2,8 +2,9 @@ This page covers the public benchmarks DeepGym can load or import: the code benchmarks (HumanEval, MBPP, EvalPlus, BigCodeBench), repo-level patch tasks (SWE-bench Pro, MiMo), -shell tasks (Terminal-Bench 2.0 and any Harbor dataset), CyberGym, mixing benchmarks -behind one reward function, and sharing environments on the Hugging Face Hub. +shell tasks (Terminal-Bench 2.0 and any Harbor dataset), CyberGym, OSWorld desktop +tasks, mixing benchmarks behind one reward function, and sharing environments on the +Hugging Face Hub. ## Code benchmarks @@ -283,6 +284,35 @@ TRL, verl or OpenRLHF. Keep training and evaluation splits separate, use a sandb untrusted completions, and don't train on metadata-only scores. See [CyberBench](../cyberbench.md) for the full workflow and diagnostic numbers. +## OSWorld desktop tasks + +`scripts/import_osworld.py` imports 16 LibreOffice Calc and Writer tasks from +[OSWorld](https://github.com/xlang-ai/OSWorld) (Apache-2.0) as +[computer-use](computer-use.md) environments. The agent gets the task's file open in +Calc or Writer and saves it with Ctrl+S. The verifier scores the saved file with the +task's own `compare_table` rules or reference text, never the screen. + +The desktop needs LibreOffice, which Daytona's default image doesn't have, so the +environments boot the Daytona snapshot `deepgym-osworld-libreoffice`. Create it once +per Daytona account, then import: + +```bash +python scripts/import_osworld.py --build-snapshot # needs DAYTONA_API_KEY +python scripts/import_osworld.py # writes ~/.deepgym/environments/osworld/ +deepgym qa --mode daytona ~/.deepgym/environments/osworld/ +``` + +- A rule the starting file already holds, or holds once an empty sheet is added, is a + guard rather than progress, so saving the untouched file or adding a blank sheet + earns nothing. OSWorld's evaluator is all-or-nothing; here a task with several rules + pays the share it got right. +- A saved file the readers can't read (missing, not a workbook, or past the size + limits) scores 0. +- Each task ships a reference episode, `reference_actions.json`, that `deepgym qa` + replays. +- The output folder keeps OSWorld's `LICENSE` and a `NOTICE` that lists what was + changed. + ## Mix benchmarks behind one reward function ```python From 925df49e3080bf3935cbbece0fc4c0487b44db79 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 15:34:05 +0530 Subject: [PATCH 09/19] fix(osworld): a saved file the readers can't read is a judged 0, never a crash or a timeout --- deepgym/CHANGELOG.md | 2 +- deepgym/docs/environments/importers.md | 5 +- .../src/deepgym/envs/computer_use/_checks.py | 231 +++++++++++---- deepgym/tests/test_import_osworld.py | 276 +++++++++++++++++- 4 files changed, 454 insertions(+), 60 deletions(-) diff --git a/deepgym/CHANGELOG.md b/deepgym/CHANGELOG.md index e9de9876..892f77df 100644 --- a/deepgym/CHANGELOG.md +++ b/deepgym/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- **OSWorld computer-use tasks:** `scripts/import_osworld.py` imports 16 of OSWorld's (Apache-2.0) LibreOffice Calc and Writer tasks as computer-use environments, each with a reference episode, and writes OSWorld's license and a NOTICE next to them. They boot the Daytona snapshot `deepgym-osworld-libreoffice` (`--build-snapshot` creates it), since Daytona's default image has no LibreOffice; `metadata.json` takes a `snapshot`. The verifier scores the saved file with the task's own `compare_table` rules (ported into the shared `_checks.py`, standard library only) or its reference text, never a screenshot. Rules the starting file already holds, or holds once an empty sheet is added, are guards, so an untouched file or a blank sheet scores 0; a file too large or malformed to read scores 0. `deepgym check` asks a computer-use folder for `reference_actions.json` instead of `reference_solution.*`, and is inconclusive on one off Daytona. +- **OSWorld computer-use tasks:** `scripts/import_osworld.py` imports 16 of OSWorld's (Apache-2.0) LibreOffice Calc and Writer tasks as computer-use environments, each with a reference episode, and writes OSWorld's license and a NOTICE next to them. They boot the Daytona snapshot `deepgym-osworld-libreoffice` (`--build-snapshot` creates it), since Daytona's default image has no LibreOffice; `metadata.json` takes a `snapshot`. The verifier scores the saved file with the task's own `compare_table` rules (ported into the shared `_checks.py`, standard library only) or its reference text, never a screenshot. Rules the starting file already holds, or holds once an empty sheet is added, are guards, so an untouched file or a blank sheet scores 0; a saved file the readers can't read (missing, corrupt, not a regular file, too large) scores 0 rather than crashing or stalling the verifier. `deepgym check` asks a computer-use folder for `reference_actions.json` instead of `reference_solution.*`, and is inconclusive on one off Daytona. ## 0.5.0 (2026-10-14) diff --git a/deepgym/docs/environments/importers.md b/deepgym/docs/environments/importers.md index 645d6b65..353e93ca 100644 --- a/deepgym/docs/environments/importers.md +++ b/deepgym/docs/environments/importers.md @@ -306,8 +306,9 @@ deepgym qa --mode daytona ~/.deepgym/environments/osworld/ guard rather than progress, so saving the untouched file or adding a blank sheet earns nothing. OSWorld's evaluator is all-or-nothing; here a task with several rules pays the share it got right. -- A saved file the readers can't read (missing, not a workbook, or past the size - limits) scores 0. +- Whatever is at the result path, the verifier gives a verdict: a file it can't read + (missing, corrupt, not a regular file, or past the size limits) scores 0, never a + crash or a timeout, and a rule it can't evaluate fails. - Each task ships a reference episode, `reference_actions.json`, that `deepgym qa` replays. - The output folder keeps OSWorld's `LICENSE` and a `NOTICE` that lists what was diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index 88b790d4..22b8704f 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -9,10 +9,18 @@ from __future__ import annotations import json +import math +import os import re +import signal +import stat +import threading import zipfile +from collections.abc import Iterator +from contextlib import contextmanager from pathlib import Path from xml.etree import ElementTree as ET +from xml.parsers import expat def load_state(path: str = 'state.json') -> dict: @@ -66,30 +74,112 @@ def report(progress: dict[str, bool], guards: dict[str, bool]) -> None: _PKG_REL_NS = '{http://schemas.openxmlformats.org/package/2006/relationships}' _W_NS = '{http://schemas.openxmlformats.org/wordprocessingml/2006/main}' -_COORD_RE = re.compile(r'([A-Z]+)(\d+)') +_COORD_RE = re.compile(r'([A-Z]{1,3})([1-9][0-9]{0,6})') +_MAX_ROW, _MAX_COL = 1_048_576, 16_384 +"""The largest row and column a spreadsheet cell can have.""" -_MAX_PART_BYTES = 32 << 20 -"""Largest unpacked XML part read from a saved file; a bigger one is unreadable.""" +_MAX_FILE_BYTES = 16 << 20 +"""Largest saved file read, packed or unpacked; a bigger one is unreadable.""" _MAX_CELLS = 1_000_000 -"""Largest used range (rows x columns) read from a sheet; a bigger one is unreadable.""" +"""Most cells (used rows x columns, all sheets together) read from one workbook.""" +_READ_SECONDS = 15 +"""Longest a saved file may take to read; the verifier has 30 s in all.""" +_RULES_SECONDS = 10 +"""Longest the rules may take to evaluate.""" -def _read_part(archive: zipfile.ZipFile, name: str) -> ET.Element: - """Parse one XML part, refusing one that unpacks past :data:`_MAX_PART_BYTES`.""" - if archive.getinfo(name).file_size > _MAX_PART_BYTES: - raise ValueError(f'{name} is larger than {_MAX_PART_BYTES} bytes') - return ET.fromstring(archive.read(name)) +@contextmanager +def _deadline(seconds: float) -> Iterator[None]: + """Raise TimeoutError in the block once it has run *seconds*, as a backstop. + + The bounds on the file keep every step short; this catches whatever they miss, + so a slow read scores 0 instead of the verifier running out of time. It needs + the main thread (where a verifier runs) and does nothing elsewhere. + """ + if threading.current_thread() is not threading.main_thread(): + yield + return + + def expire(*_: object) -> None: + raise TimeoutError(f'took longer than {seconds} s') + + previous = signal.signal(signal.SIGALRM, expire) + signal.setitimer(signal.ITIMER_REAL, seconds) + try: + yield + finally: + signal.setitimer(signal.ITIMER_REAL, 0) + signal.signal(signal.SIGALRM, previous) + + +@contextmanager +def _open_package(path: str) -> Iterator[zipfile.ZipFile]: + """Open the saved file at *path* as a zip package. + + The agent controls this file, so it is opened without blocking (a FIFO planted at + the path can't stall the verifier) and must be a regular file of at most + :data:`_MAX_FILE_BYTES`, packed and unpacked, whose parts are stored or deflated + (the methods :func:`_read_xml` reads with bounded output). + + Raises: + ValueError: If the file is not a regular file, is too large or uses another + compression method. + OSError, zipfile.BadZipFile: If it can't be opened as a zip package. + """ + with os.fdopen(os.open(path, os.O_RDONLY | os.O_NONBLOCK), 'rb') as handle: + info = os.fstat(handle.fileno()) + if not stat.S_ISREG(info.st_mode): + raise ValueError('not a regular file') + if info.st_size > _MAX_FILE_BYTES: + raise ValueError(f'larger than {_MAX_FILE_BYTES} bytes') + with zipfile.ZipFile(handle) as archive: + members = archive.infolist() + if sum(member.file_size for member in members) > _MAX_FILE_BYTES: + raise ValueError(f'unpacks to more than {_MAX_FILE_BYTES} bytes') + if any( + m.compress_type not in (zipfile.ZIP_STORED, zipfile.ZIP_DEFLATED) for m in members + ): + raise ValueError('a part uses a compression method other than deflate') + yield archive + + +def _read_xml(archive: zipfile.ZipFile, name: str) -> ET.Element: + """Parse one part, reading at most :data:`_MAX_FILE_BYTES` of it. + + A bounded read() caps what is decompressed at the part's declared size, so a + part that unpacks to more than it declares fails its CRC check instead. + """ + with archive.open(name) as member: + data = member.read(_MAX_FILE_BYTES + 1) + if len(data) > _MAX_FILE_BYTES: + raise ValueError(f'{name} unpacks to more than {_MAX_FILE_BYTES} bytes') + # Office parts never declare a DTD; refusing one rules out entity expansion. + scan = expat.ParserCreate() + + def refuse(*_: object) -> None: + raise ValueError(f'{name} declares a DTD') + + scan.StartDoctypeDeclHandler = refuse + scan.Parse(data, True) + return ET.fromstring(data) def _coord_to_cell(coord: str) -> tuple[int, int]: - """'C7' -> (row 7, column 3), both 1-based.""" + """'C7' -> (row 7, column 3), both 1-based. + + Raises: + ValueError: If *coord* is not a cell reference within a sheet's bounds. + """ match = _COORD_RE.fullmatch(coord) if not match: - raise ValueError(f'not a cell coordinate: {coord!r}') + raise ValueError(f'not a cell coordinate: {coord[:20]!r}') col = 0 for ch in match.group(1): col = col * 26 + (ord(ch) - ord('A') + 1) - return int(match.group(2)), col + row = int(match.group(2)) + if row > _MAX_ROW or col > _MAX_COL: + raise ValueError(f'{coord} is outside a sheet') + return row, col def _number(text: str) -> int | float | None: @@ -98,6 +188,8 @@ def _number(text: str) -> int | float | None: value = float(text) except ValueError: return None + if not math.isfinite(value): + raise ValueError(f'{text!r} is not a finite number') if value.is_integer() and abs(value) < 1e15: return int(value) return value @@ -142,29 +234,42 @@ def _date_styles(archive: zipfile.ZipFile) -> set[int]: """ if 'xl/styles.xml' not in archive.namelist(): return set() - root = _read_part(archive, 'xl/styles.xml') + root = _read_xml(archive, 'xl/styles.xml') custom = { int(fmt.get('numFmtId', '-1')): fmt.get('formatCode', '') for fmt in root.iter(f'{_XLSX_NS}numFmt') } + if any(len(code) > 255 for code in custom.values()): # longer than any real one + raise ValueError('a number format code is too long') + dated_ids = { + fmt_id + for fmt_id, code in custom.items() + if re.search(r'[dmyhs]', _FORMAT_NOISE.sub('', code.split(';')[0]), re.IGNORECASE) + } | (_DATE_FORMAT_IDS - set(custom)) xfs = root.find(f'{_XLSX_NS}cellXfs') - dated = set() - for index, xf in enumerate([] if xfs is None else xfs.findall(f'{_XLSX_NS}xf')): - fmt_id = int(xf.get('numFmtId', '0')) - code = custom.get(fmt_id) - if code is None: - if fmt_id in _DATE_FORMAT_IDS: - dated.add(index) - elif re.search(r'[dmyhs]', _FORMAT_NOISE.sub('', code.split(';')[0]), re.IGNORECASE): - dated.add(index) - return dated + return { + index + for index, xf in enumerate([] if xfs is None else xfs.findall(f'{_XLSX_NS}xf')) + if int(xf.get('numFmtId', '0')) in dated_ids + } def _merge_ranges(root: ET.Element) -> list[str]: + """The sheet's merged ranges ('A1:C1'). + + Raises: + ValueError: If a range is not two cell references, so a malformed merge makes + the file unreadable (a 0) instead of failing one rule while others pass. + """ merges = root.find(f'{_XLSX_NS}mergeCells') if merges is None: return [] - return [cell.get('ref', '') for cell in merges.findall(f'{_XLSX_NS}mergeCell')] + refs = [cell.get('ref', '') for cell in merges.findall(f'{_XLSX_NS}mergeCell')] + for ref in refs: + start, _, end = ref.partition(':') + _coord_to_cell(start) + _coord_to_cell(end) + return refs def _trim(grid: list[list[object]]) -> list[list[object]]: @@ -177,7 +282,9 @@ def _trim(grid: list[list[object]]) -> list[list[object]]: return grid -def _sheet_grid(root: ET.Element, shared: list[str], dated: set[int]) -> list[list[object]]: +def _sheet_grid( + root: ET.Element, shared: list[str], dated: set[int], budget: int +) -> list[list[object]]: """Rows of cell values over the sheet's used range; missing cells are None. The used range ends at the last non-empty row and column, as after :func:`_trim`. @@ -185,7 +292,7 @@ def _sheet_grid(root: ET.Element, shared: list[str], dated: set[int]) -> list[li ``{'date': serial}``, so it only equals the same serial shown as a date. Raises: - ValueError: If the used range holds more than :data:`_MAX_CELLS` cells. + ValueError: If the used range holds more than *budget* cells. """ cells: dict[tuple[int, int], object] = {} max_row = 0 @@ -207,7 +314,7 @@ def _sheet_grid(root: ET.Element, shared: list[str], dated: set[int]) -> list[li cells[(r, c)] = value max_row = max(max_row, r) max_col = max(max_col, c) - if max_row * max_col > _MAX_CELLS: + if max_row * max_col > budget: raise ValueError(f'used range of {max_row} x {max_col} cells is too large') grid: list[list[object]] = [] for r in range(1, max_row + 1): @@ -217,8 +324,8 @@ def _sheet_grid(root: ET.Element, shared: list[str], dated: set[int]) -> list[li def _sheet_parts(archive: zipfile.ZipFile) -> list[tuple[str, str]]: """(sheet name, part name) for every sheet, in workbook order.""" - workbook = _read_part(archive, 'xl/workbook.xml') - rels = _read_part(archive, 'xl/_rels/workbook.xml.rels') + workbook = _read_xml(archive, 'xl/workbook.xml') + rels = _read_xml(archive, 'xl/_rels/workbook.xml.rels') targets = { rel.get('Id'): rel.get('Target') for rel in rels.findall(f'{_PKG_REL_NS}Relationship') } @@ -246,24 +353,33 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: booleans, error text, or None for an empty cell. Raises: - ValueError: If the file is missing or not a readable workbook. + ValueError: If the file is missing or can't be read for any reason: the + verifier scores that a 0, since the agent controls the file. """ try: - with zipfile.ZipFile(path) as archive: + with _deadline(_READ_SECONDS), _open_package(path) as archive: shared_node = None if 'xl/sharedStrings.xml' in archive.namelist(): - shared_node = _read_part(archive, 'xl/sharedStrings.xml') + shared_node = _read_xml(archive, 'xl/sharedStrings.xml') shared = [ ''.join(t.text or '' for t in si.iter(f'{_XLSX_NS}t')) for si in (shared_node if shared_node is not None else []) ] dated = _date_styles(archive) sheets = [] + budget = _MAX_CELLS + parsed: dict[str, tuple[list[list[object]], list[str]]] = {} for name, part in _sheet_parts(archive): - root = _read_part(archive, part) - sheets.append((name, _sheet_grid(root, shared, dated), _merge_ranges(root))) + if part not in parsed: # sheets naming one part parse it once + root = _read_xml(archive, part) + parsed[part] = (_sheet_grid(root, shared, dated, budget), _merge_ranges(root)) + grid, merges = parsed[part] + budget -= len(grid) * max((len(row) for row in grid), default=0) + if budget < 0: + raise ValueError(f'more than {_MAX_CELLS} cells') + sheets.append((name, grid, merges)) return sheets - except (OSError, zipfile.BadZipFile, KeyError, ET.ParseError) as exc: + except Exception as exc: # any way the agent's file breaks reading is a scored 0 raise ValueError(f'{path} is not a readable workbook: {exc}') from exc @@ -299,13 +415,14 @@ def read_docx_text(path: str) -> str: The normalized text. Raises: - ValueError: If the file is missing or not a readable document. + ValueError: If the file is missing or can't be read for any reason: the + verifier scores that a 0, since the agent controls the file. """ try: - with zipfile.ZipFile(path) as archive: + with _deadline(_READ_SECONDS), _open_package(path) as archive: # The main part is the one the package's officeDocument relationship names, # as python-docx resolves it, not whatever sits at word/document.xml. - rels = _read_part(archive, '_rels/.rels') + rels = _read_xml(archive, '_rels/.rels') main = [ rel.get('Target', '').lstrip('/') for rel in rels.findall(f'{_PKG_REL_NS}Relationship') @@ -313,13 +430,10 @@ def read_docx_text(path: str) -> str: ] if len(main) != 1: raise ValueError(f'{len(main)} main document parts') - root = _read_part(archive, main[0]) - except (OSError, zipfile.BadZipFile, KeyError, ET.ParseError) as exc: - raise ValueError(f'{path} is not a readable document: {exc}') from exc - body = root.find(f'{_W_NS}body') - paragraphs = [] - if body is not None: - for para in body: + root = _read_xml(archive, main[0]) + body = root.find(f'{_W_NS}body') + paragraphs = [] + for para in [] if body is None else body: if para.tag != f'{_W_NS}p': continue text_parts = [] @@ -331,7 +445,9 @@ def read_docx_text(path: str) -> str: if run.tag == f'{_W_NS}r': text_parts.append(_run_text(run)) paragraphs.append(''.join(text_parts)) - return re.sub(r'\s+', ' ', '\n'.join(paragraphs)).strip() + return re.sub(r'\s+', ' ', '\n'.join(paragraphs)).strip() + except Exception as exc: # any way the agent's file breaks reading is a scored 0 + raise ValueError(f'{path} is not a readable document: {exc}') from exc def _np_round(value: float, precision: int) -> float: @@ -342,7 +458,10 @@ def _np_round(value: float, precision: int) -> float: round(-0.00005, 4) == -0.0001 while numpy rounds it to -0.0. """ scale = 10**precision - return round(value * scale) / scale + scaled = value * scale + if not math.isfinite(scaled): # too large to round; pandas leaves it as it is + return value + return round(scaled) / scale def _is_number(value: object) -> bool: @@ -568,17 +687,25 @@ def office_rule_checks( Returns: {rule name, e.g. 'rule0_sheet_data': passed}; a rule whose sheets, indices or cells cannot be resolved fails. A rule's 'name', when it has one, is - used as its key. + used as its key. If any rule can't be evaluated at all, every rule fails, so + an evaluation error can never leave partial credit standing. """ names_r = [sheet[0] for sheet in result or []] names_e = [sheet[0] for sheet in expected or []] - checks = {} + names: list[str] = [] for index, rule in enumerate(rules): name = str(rule.get('name') or f'rule{index}_{rule.get("type", "?")}') - while name in checks: # two rules must never share one verdict slot + while name in names: # two rules must never share one verdict slot name += '_' - checks[name] = _rule_verdict(rule, result, expected, names_r, names_e) - return checks + names.append(name) + try: + with _deadline(_RULES_SECONDS): + return { + name: _rule_verdict(rule, result, expected, names_r, names_e) + for name, rule in zip(names, rules) + } + except Exception: # a rule that can't decide fails closed, and so does every other + return dict.fromkeys(names, False) # }}} Office files # diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 4f64a0eb..8d54f185 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -6,8 +6,13 @@ import base64 import copy +import io import json +import math +import os +import re import shutil +import struct import subprocess import sys import zipfile @@ -159,7 +164,7 @@ def _run_verifier(env: ComputerUseEnvironment, home: Path, work: Path) -> dict: text=True, cwd=work, env={'HOME': str(work), 'DEEPGYM_AGENT_HOME': str(home)}, - timeout=60, + timeout=25, # under the 30 s a session gives the verifier ) assert proc.returncode == 0, proc.stderr return json.loads(proc.stdout.strip().splitlines()[-1]) @@ -357,12 +362,179 @@ def test_a_date_only_matches_a_cell_shown_as_that_date(tmp_path: Path, work: Pat assert _run_verifier(env, home, work)['score'] == score, label -def test_a_saved_file_too_large_to_read_scores_zero(tmp_path: Path, work: Path) -> None: - """One cell at the far corner of a sheet would make a dense grid of a billion - cells: the workbook counts as unreadable (a scored 0), not a verifier crash.""" +def _patch_zip(data: bytes, member: str, edit) -> bytes: + """*data* with *edit(header, payload)* applied to *member*'s local header and data.""" + with zipfile.ZipFile(io.BytesIO(data)) as archive: + info = archive.getinfo(member) + start = info.header_offset + name_len, extra_len = struct.unpack(' bytes: + """*data* with the 2-byte field at *local* / *central* of every member's local and + central directory header replaced by *update(old value)*.""" + out = bytearray(data) + for signature, offset in ((b'PK\x03\x04', local), (b'PK\x01\x02', central)): + at = out.find(signature) + while at != -1: + (value,) = struct.unpack(' None: + for at in range(8, 40): + payload[at] ^= 0xFF + + +_X = 'http://schemas.openxmlformats.org/spreadsheetml/2006/main' +_STYLE_BOMB = ( + f'' + '' +) +"""A 1 MB format code that a backtracking regex would take minutes over.""" +_REF_BOMB = ( + f'' + f'1' +) +"""A cell reference two million letters long.""" +_DTD = ( + '' + ']>' + f'&b;' + '' +) +"""A sheet that declares entities: office parts never carry a DTD.""" + + +def _repacked(path: Path, good: bytes, parts: dict[str, str], method: int = zipfile.ZIP_DEFLATED): + """Write *good*'s parts to *path*, replaced or added from *parts*.""" + with zipfile.ZipFile(io.BytesIO(good)) as source, zipfile.ZipFile(path, 'w', method) as out: + for name in source.namelist(): + out.writestr(name, parts.pop(name, None) or source.read(name)) + for name, text in parts.items(): + out.writestr(name, text) + + +def _understated(path: Path, good: bytes) -> None: + """The sheet part padded to 20 MB unpacked while it declares only its real size.""" + sheet = 'xl/worksheets/sheet1.xml' + with zipfile.ZipFile(io.BytesIO(good)) as source: + real = source.read(sheet) + _repacked(path, good, {sheet: real.decode() + ' ' * (20 << 20)}) + data = bytearray(path.read_bytes()) + for signature, size_at, name_at in ((b'PK\x03\x04', 22, 30), (b'PK\x01\x02', 24, 46)): + at = data.find(signature) + while at != -1: + if data[at + name_at : at + name_at + len(sheet)] == sheet.encode(): + data[at + size_at : at + size_at + 4] = struct.pack(' None: + """1,001 sheets that all name one 1,000-cell part: past the cell budget together.""" + column = {f'A{row}': row for row in range(1, 1001)} + base = make_xlsx(tmp_path / 'one.xlsx', [('Sheet1', column, [])]) + sheets = ''.join(f'' for i in range(1001)) + with zipfile.ZipFile(base) as source, zipfile.ZipFile(path, 'w') as out: + for name in source.namelist(): + data = source.read(name).decode() + if name == 'xl/workbook.xml': + data = re.sub('.*', f'{sheets}', data) + out.writestr(name, data) + + +def _plant(label: str, path: Path, tmp_path: Path) -> None: + """Leave *path* the way an agent could, by the GUI or a macro, instead of saving.""" + good = make_xlsx(tmp_path / 'good.xlsx', [('Sheet1', _GOLD, _MERGED)]).read_bytes() + deflated = tmp_path / 'deflated.xlsx' + with zipfile.ZipFile(io.BytesIO(good)) as source: + with zipfile.ZipFile(deflated, 'w', zipfile.ZIP_DEFLATED) as out: + for name in source.namelist(): + out.writestr(name, source.read(name)) + sheet = 'xl/worksheets/sheet1.xml' + pipe = tmp_path / 'pipe' + writers = { + 'empty file': lambda: path.write_bytes(b''), + 'not a zip': lambda: path.write_bytes(b'not a workbook'), + 'truncated': lambda: path.write_bytes(good[: len(good) // 2]), + 'corrupt deflate stream': lambda: path.write_bytes( + _patch_zip(deflated.read_bytes(), sheet, _flip) + ), + 'encrypted parts': lambda: path.write_bytes(_headers(good, 6, 8, lambda v: v | 1)), + 'unsupported compression': lambda: path.write_bytes(_headers(good, 8, 10, lambda v: 99)), + 'unpacks past the limit': lambda: _repacked( + path, good, {'xl/padding.xml': ' ' * (17 << 20)} + ), + 'unpacks past what it declares': lambda: _understated(path, good), + 'a bzip2 part': lambda: _repacked(path, good, {}, zipfile.ZIP_BZIP2), + 'a format code past any real one': lambda: _repacked( + path, good, {'xl/styles.xml': _STYLE_BOMB} + ), + 'a cell reference past any real one': lambda: _repacked( + path, good, {'xl/worksheets/sheet1.xml': _REF_BOMB} + ), + 'a malformed merge': lambda: make_xlsx(path, [('Sheet1', _GOLD, ['A1:BAD'])]), + 'a DTD': lambda: _repacked(path, good, {'xl/worksheets/sheet1.xml': _DTD}), + 'infinite number': lambda: make_xlsx(path, [('Sheet1', {**_GOLD, 'C1': math.inf}, [])]), + 'one part named by many sheets': lambda: _one_part_many_sheets(path, tmp_path), + 'used range too large': lambda: make_xlsx( + path, [('Sheet1', {**_GOLD, 'AMJ1048576': 'x'}, _MERGED)] + ), + 'a directory': path.mkdir, + 'a FIFO': lambda: os.mkfifo(path), + 'a link to a FIFO': lambda: (os.mkfifo(pipe), path.symlink_to(pipe)), + 'a dangling link': lambda: path.symlink_to(tmp_path / 'gone.xlsx'), + 'unreadable': lambda: (path.write_bytes(good), path.chmod(0)), + } + if label != 'missing': + writers[label]() + + +@pytest.mark.parametrize( + 'label', + [ + 'missing', + 'empty file', + 'not a zip', + 'truncated', + 'corrupt deflate stream', + 'encrypted parts', + 'unsupported compression', + 'unpacks past the limit', + 'unpacks past what it declares', + 'a bzip2 part', + 'a format code past any real one', + 'a cell reference past any real one', + 'a malformed merge', + 'a DTD', + 'infinite number', + 'used range too large', + 'one part named by many sheets', + 'a directory', + 'a FIFO', + 'a link to a FIFO', + 'a dangling link', + 'unreadable', + ], +) +def test_a_saved_workbook_the_agent_broke_is_a_judged_zero( + label: str, tmp_path: Path, work: Path +) -> None: + """Whatever the agent leaves at the result path, the verifier gives a verdict (it + exits 0 with protocol JSON) and scores 0, never a crash or a hang (no verdict).""" + if label == 'unreadable' and os.geteuid() == 0: + pytest.skip('root reads a file whatever its mode') env, _ = _calc_env(tmp_path) home = tmp_path / 'home' - _save(tmp_path, home, {**_GOLD, 'AMJ1048576': 'x'}, _MERGED) + (home / 'Desktop').mkdir(parents=True) + _plant(label, home / 'Desktop' / FILE, tmp_path) result = _run_verifier(env, home, work) @@ -370,6 +542,100 @@ def test_a_saved_file_too_large_to_read_scores_zero(tmp_path: Path, work: Path) assert 'workbook_saved' in result['details'] +def test_a_valid_file_with_700k_styles_is_read_in_time(tmp_path: Path, work: Path) -> None: + """700,000 cell styles sharing one long format code: each code is classified + once, so the verifier still finishes within its budget (and the file is correct).""" + env, _ = _calc_env(tmp_path) + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + good = make_xlsx(tmp_path / 'good.xlsx', [('Sheet1', _GOLD, _MERGED)]).read_bytes() + code = '"' + 'x' * 240 + '" yyyy' + xfs = '' * 700_000 + styles = ( + f'' + f'{xfs}' + ) + _repacked(home / 'Desktop' / FILE, good, {'xl/styles.xml': styles}) + + assert _run_verifier(env, home, work)['score'] == 1.0 + + +def test_the_read_deadline_raises_inside_a_slow_block() -> None: + checks = load_checks_module() + with pytest.raises(TimeoutError), checks._deadline(0.05): + while True: + pass + + +def test_unsaved_changes_and_a_wrong_sheet_name_are_judged_zeros( + tmp_path: Path, work: Path +) -> None: + """Changes left unsaved leave the starting file, and the right data on a sheet + with the wrong name fails the rule that names the sheet: both are a scored 0.""" + source = make_xlsx(tmp_path / 'source.xlsx', [('Sheet1', _START, [])]) + summary = {'A1': 'Total', 'A2': 1500} + gold = make_xlsx(tmp_path / 'gold.xlsx', [('Sheet1', _START, []), ('Sheet2', summary, [])]) + rules = [{'type': 'sheet_data', 'sheet_idx0': 'RNSheet2', 'sheet_idx1': 'ENSheet2'}] + env, _ = _import(tmp_path, _task_config(rules=rules), source, gold) + + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + saved = home / 'Desktop' / FILE + for label, sheets, score in ( + ('changes left unsaved', [('Sheet1', _START, [])], 0.0), + ('wrong sheet name', [('Sheet1', _START, []), ('Sheet 2', summary, [])], 0.0), + ('finished', [('Sheet1', _START, []), ('Sheet2', summary, [])], 1.0), + ): + make_xlsx(saved, sheets) + assert _run_verifier(env, home, work)['score'] == score, label + + +@pytest.mark.parametrize('label', ['missing', 'not a zip', 'a FIFO', 'no package', 'two mains']) +def test_a_saved_document_the_agent_broke_is_a_judged_zero( + label: str, tmp_path: Path, work: Path +) -> None: + env, _ = _doc_env(tmp_path) + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + saved = home / 'Desktop' / DOC + with zipfile.ZipFile(make_docx(tmp_path / 'gold-copy.docx', _DOC_GOLD)) as gold: + document = gold.read('word/document.xml') + rels = gold.read('_rels/.rels').decode() + if label == 'not a zip': + saved.write_bytes(b'not a document') + elif label == 'a FIFO': + os.mkfifo(saved) + elif label in ('no package', 'two mains'): + with zipfile.ZipFile(saved, 'w') as archive: + archive.writestr('word/document.xml', document) + if label == 'two mains': + twice = rels.replace('', rels[rels.index(' None: + checks = load_checks_module() + sheets = [('Sheet1', [['Item']], [])] + holds = { + 'type': 'check_cell', + 'sheet_idx': 0, + 'coordinate': 'A1', + 'props': {'value': {'method': 'eq', 'ref': 'Item'}}, + } + broken = {'type': 'check_cell', 'sheet_idx': 0, 'coordinate': 'A1', 'props': ['value']} + assert checks.office_rule_checks([holds], sheets, None) == {'rule0_check_cell': True} + # One rule that can't be evaluated fails every rule: no partial credit survives it. + assert checks.office_rule_checks([holds, broken], sheets, None) == { + 'rule0_check_cell': False, + 'rule1_check_cell': False, + } + + def test_the_main_document_part_is_the_one_the_package_names(tmp_path: Path) -> None: """python-docx reads the part the officeDocument relationship names, so a stray word/document.xml holding the expected text is not what gets scored.""" From 1d019a3a23ea7f7e14a45ba23331eb02126c63d1 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 15:47:07 +0530 Subject: [PATCH 10/19] fix(osworld): judge the saved file in a time- and memory-bounded child, so an overrun scores 0 --- deepgym/scripts/import_osworld.py | 62 +++++++----- .../src/deepgym/envs/computer_use/_checks.py | 94 ++++++++++++------- deepgym/tests/test_import_osworld.py | 37 +++++++- 3 files changed, 134 insertions(+), 59 deletions(-) diff --git a/deepgym/scripts/import_osworld.py b/deepgym/scripts/import_osworld.py index 3380b273..b929166e 100644 --- a/deepgym/scripts/import_osworld.py +++ b/deepgym/scripts/import_osworld.py @@ -369,18 +369,28 @@ def build_verifier(task: Task) -> str: from pathlib import Path workbook = Path(os.environ['DEEPGYM_AGENT_HOME']) / PARAMS['result'] -try: - sheets = read_sheets(str(workbook)) -except ValueError: - sheets = None - -report( - progress=office_rule_checks(PARAMS['progress'], sheets, PARAMS['expected']), - guards={ - 'workbook_saved': sheets is not None, - **office_rule_checks(PARAMS['guards'], sheets, PARAMS['expected']), - }, -) + + +def judge(): + try: + sheets = read_sheets(str(workbook)) + except ValueError: + sheets = None + return { + 'progress': office_rule_checks(PARAMS['progress'], sheets, PARAMS['expected']), + 'guards': { + 'workbook_saved': sheets is not None, + **office_rule_checks(PARAMS['guards'], sheets, PARAMS['expected']), + }, + } + + +# Judged in a bounded child: a file too slow or too large to judge scores 0. +verdict = bounded_call(judge) or { + 'progress': office_rule_checks(PARAMS['progress'], None, None), + 'guards': {'workbook_saved': False}, +} +report(**verdict) """ _TEXT_VERIFIER_BODY = """ @@ -388,15 +398,25 @@ def build_verifier(task: Task) -> str: from pathlib import Path document = Path(os.environ['DEEPGYM_AGENT_HOME']) / PARAMS['result'] -try: - text = read_docx_text(str(document)) -except ValueError: - text = None - -report( - progress={'matches_reference': text == PARAMS['expected_text']}, - guards={'document_saved': text is not None}, -) + + +def judge(): + try: + text = read_docx_text(str(document)) + except ValueError: + text = None + return { + 'progress': {'matches_reference': text == PARAMS['expected_text']}, + 'guards': {'document_saved': text is not None}, + } + + +# Judged in a bounded child: a file too slow or too large to judge scores 0. +verdict = bounded_call(judge) or { + 'progress': {'matches_reference': False}, + 'guards': {'document_saved': False}, +} +report(**verdict) """ diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index 22b8704f..15ceb29e 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -12,11 +12,13 @@ import math import os import re +import resource +import select import signal import stat -import threading +import time import zipfile -from collections.abc import Iterator +from collections.abc import Callable, Iterator from contextlib import contextmanager from pathlib import Path from xml.etree import ElementTree as ET @@ -82,34 +84,53 @@ def report(progress: dict[str, bool], guards: dict[str, bool]) -> None: """Largest saved file read, packed or unpacked; a bigger one is unreadable.""" _MAX_CELLS = 1_000_000 """Most cells (used rows x columns, all sheets together) read from one workbook.""" -_READ_SECONDS = 15 -"""Longest a saved file may take to read; the verifier has 30 s in all.""" -_RULES_SECONDS = 10 -"""Longest the rules may take to evaluate.""" +JUDGE_SECONDS = 20 +"""How long judging a saved file may take; a session gives the whole verifier 30 s.""" +_JUDGE_MEMORY = 1 << 30 +"""Address space the judging process may use, where the platform enforces it.""" -@contextmanager -def _deadline(seconds: float) -> Iterator[None]: - """Raise TimeoutError in the block once it has run *seconds*, as a backstop. - - The bounds on the file keep every step short; this catches whatever they miss, - so a slow read scores 0 instead of the verifier running out of time. It needs - the main thread (where a verifier runs) and does nothing elsewhere. - """ - if threading.current_thread() is not threading.main_thread(): - yield - return +def bounded_call(judge: Callable[[], dict], seconds: float = JUDGE_SECONDS) -> dict | None: + """Return ``judge()``, computed in a forked child under a time and memory limit. - def expire(*_: object) -> None: - raise TimeoutError(f'took longer than {seconds} s') + Reading a file the agent controls can't be made cheap in every case, so the work + runs where running out of memory or time kills only the child. - previous = signal.signal(signal.SIGALRM, expire) - signal.setitimer(signal.ITIMER_REAL, seconds) - try: - yield - finally: - signal.setitimer(signal.ITIMER_REAL, 0) - signal.signal(signal.SIGALRM, previous) + Returns: + The child's JSON-serializable result, or None if it raised, was killed, or + ran past *seconds*: the caller scores that a 0. + """ + read_fd, write_fd = os.pipe() + pid = os.fork() + if pid == 0: + status = 1 + try: + os.close(read_fd) + try: + resource.setrlimit(resource.RLIMIT_AS, (_JUDGE_MEMORY, _JUDGE_MEMORY)) + except (ValueError, OSError): # not enforced on macOS + pass + with os.fdopen(write_fd, 'wb') as out: + out.write(json.dumps(judge()).encode()) + status = 0 + finally: + os._exit(status) + os.close(write_fd) + deadline = time.monotonic() + seconds + chunks = [] + with os.fdopen(read_fd, 'rb') as pipe: + while True: + left = deadline - time.monotonic() + if left <= 0 or not select.select([pipe], [], [], left)[0]: + os.kill(pid, signal.SIGKILL) + os.waitpid(pid, 0) + return None + chunk = os.read(pipe.fileno(), 1 << 16) + if not chunk: + break + chunks.append(chunk) + _, status = os.waitpid(pid, 0) + return json.loads(b''.join(chunks)) if status == 0 else None @contextmanager @@ -154,12 +175,18 @@ def _read_xml(archive: zipfile.ZipFile, name: str) -> ET.Element: if len(data) > _MAX_FILE_BYTES: raise ValueError(f'{name} unpacks to more than {_MAX_FILE_BYTES} bytes') # Office parts never declare a DTD; refusing one rules out entity expansion. - scan = expat.ParserCreate() + # Nor a namespace URI longer than any real one, which every name would repeat. + scan = expat.ParserCreate(namespace_separator=' ') def refuse(*_: object) -> None: raise ValueError(f'{name} declares a DTD') + def check_namespace(_: str | None, uri: str) -> None: + if len(uri) > 200: + raise ValueError(f'{name} declares a namespace URI of {len(uri)} characters') + scan.StartDoctypeDeclHandler = refuse + scan.StartNamespaceDeclHandler = check_namespace scan.Parse(data, True) return ET.fromstring(data) @@ -357,7 +384,7 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: verifier scores that a 0, since the agent controls the file. """ try: - with _deadline(_READ_SECONDS), _open_package(path) as archive: + with _open_package(path) as archive: shared_node = None if 'xl/sharedStrings.xml' in archive.namelist(): shared_node = _read_xml(archive, 'xl/sharedStrings.xml') @@ -419,7 +446,7 @@ def read_docx_text(path: str) -> str: verifier scores that a 0, since the agent controls the file. """ try: - with _deadline(_READ_SECONDS), _open_package(path) as archive: + with _open_package(path) as archive: # The main part is the one the package's officeDocument relationship names, # as python-docx resolves it, not whatever sits at word/document.xml. rels = _read_xml(archive, '_rels/.rels') @@ -699,11 +726,10 @@ def office_rule_checks( name += '_' names.append(name) try: - with _deadline(_RULES_SECONDS): - return { - name: _rule_verdict(rule, result, expected, names_r, names_e) - for name, rule in zip(names, rules) - } + return { + name: _rule_verdict(rule, result, expected, names_r, names_e) + for name, rule in zip(names, rules) + } except Exception: # a rule that can't decide fails closed, and so does every other return dict.fromkeys(names, False) diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 8d54f185..21fbb88e 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -15,6 +15,7 @@ import struct import subprocess import sys +import time import zipfile from pathlib import Path @@ -560,11 +561,39 @@ def test_a_valid_file_with_700k_styles_is_read_in_time(tmp_path: Path, work: Pat assert _run_verifier(env, home, work)['score'] == 1.0 -def test_the_read_deadline_raises_inside_a_slow_block() -> None: +def _slow() -> dict: + time.sleep(60) + return {} + + +def _broken() -> dict: + raise RuntimeError('judge failed') + + +@pytest.mark.filterwarnings('ignore:This process .* is multi-threaded') +def test_the_judging_child_returns_its_verdict_or_none() -> None: checks = load_checks_module() - with pytest.raises(TimeoutError), checks._deadline(0.05): - while True: - pass + assert checks.bounded_call(lambda: {'ok': True}) == {'ok': True} + assert checks.bounded_call(_broken) is None + started = time.monotonic() + assert checks.bounded_call(_slow, 0.5) is None + assert time.monotonic() - started < 5 + + +def test_a_judge_that_runs_out_of_time_is_a_judged_zero(tmp_path: Path, work: Path) -> None: + """When judging the file overruns (or dies), the verifier still reports a 0.""" + env, _ = _calc_env(tmp_path) + home = tmp_path / 'home' + _save(tmp_path, home, _GOLD, _MERGED) + assert 'bounded_call(judge)' in env.final_verifier_code + stalled = env.final_verifier_code.replace( + 'bounded_call(judge)', "bounded_call(lambda: __import__('time').sleep(60), 0.5)" + ) + + result = _run_verifier(env.model_copy(update={'final_verifier_code': stalled}), home, work) + + assert result['score'] == 0.0 + assert 'workbook_saved' in result['details'] def test_unsaved_changes_and_a_wrong_sheet_name_are_judged_zeros( From 15ee318b71451999fd275bfe93e5468dda1f6392 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 15:51:34 +0530 Subject: [PATCH 11/19] fix(osworld): kill and reap the judging child whenever the parent stops waiting for it --- .../src/deepgym/envs/computer_use/_checks.py | 36 ++++++++++++------- deepgym/tests/test_import_osworld.py | 24 +++++++++++++ 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index 15ceb29e..49021356 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -101,7 +101,12 @@ def bounded_call(judge: Callable[[], dict], seconds: float = JUDGE_SECONDS) -> d ran past *seconds*: the caller scores that a 0. """ read_fd, write_fd = os.pipe() - pid = os.fork() + try: + pid = os.fork() + except OSError: # our sandbox couldn't start the child: no verdict, nothing left open + os.close(read_fd) + os.close(write_fd) + raise if pid == 0: status = 1 try: @@ -118,18 +123,23 @@ def bounded_call(judge: Callable[[], dict], seconds: float = JUDGE_SECONDS) -> d os.close(write_fd) deadline = time.monotonic() + seconds chunks = [] - with os.fdopen(read_fd, 'rb') as pipe: - while True: - left = deadline - time.monotonic() - if left <= 0 or not select.select([pipe], [], [], left)[0]: - os.kill(pid, signal.SIGKILL) - os.waitpid(pid, 0) - return None - chunk = os.read(pipe.fileno(), 1 << 16) - if not chunk: - break - chunks.append(chunk) - _, status = os.waitpid(pid, 0) + reaped = False + try: + with os.fdopen(read_fd, 'rb') as pipe: + while True: + left = deadline - time.monotonic() + if left <= 0 or not select.select([pipe], [], [], left)[0]: + return None + chunk = os.read(pipe.fileno(), 1 << 16) + if not chunk: + break + chunks.append(chunk) + _, status = os.waitpid(pid, 0) + reaped = True + finally: + if not reaped: # past the deadline, or the parent failed: never leave it running + os.kill(pid, signal.SIGKILL) + os.waitpid(pid, 0) return json.loads(b''.join(chunks)) if status == 0 else None diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 21fbb88e..d61e9999 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -580,6 +580,30 @@ def test_the_judging_child_returns_its_verdict_or_none() -> None: assert time.monotonic() - started < 5 +@pytest.mark.filterwarnings('ignore:This process .* is multi-threaded') +def test_a_failing_parent_still_kills_the_judging_child(monkeypatch: pytest.MonkeyPatch) -> None: + checks = load_checks_module() + children: list[int] = [] + fork = os.fork + + def recording_fork() -> int: + pid = fork() + children.append(pid) + return pid + + def broken_select(*_: object) -> None: + raise RuntimeError('select failed') + + monkeypatch.setattr(checks.os, 'fork', recording_fork) + monkeypatch.setattr(checks.select, 'select', broken_select) + + with pytest.raises(RuntimeError): + checks.bounded_call(_slow, 30) + + with pytest.raises(ProcessLookupError): # killed and reaped, not left running + os.kill(children[0], 0) + + def test_a_judge_that_runs_out_of_time_is_a_judged_zero(tmp_path: Path, work: Path) -> None: """When judging the file overruns (or dies), the verifier still reports a 0.""" env, _ = _calc_env(tmp_path) From 6608a080ee3343a48fa32746d02c9abf8bf16c4b Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 15:55:28 +0530 Subject: [PATCH 12/19] docs: describe the bounded OSWorld verifier --- deepgym/CHANGELOG.md | 2 +- deepgym/docs/environments/importers.md | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/deepgym/CHANGELOG.md b/deepgym/CHANGELOG.md index 892f77df..f089a90c 100644 --- a/deepgym/CHANGELOG.md +++ b/deepgym/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- **OSWorld computer-use tasks:** `scripts/import_osworld.py` imports 16 of OSWorld's (Apache-2.0) LibreOffice Calc and Writer tasks as computer-use environments, each with a reference episode, and writes OSWorld's license and a NOTICE next to them. They boot the Daytona snapshot `deepgym-osworld-libreoffice` (`--build-snapshot` creates it), since Daytona's default image has no LibreOffice; `metadata.json` takes a `snapshot`. The verifier scores the saved file with the task's own `compare_table` rules (ported into the shared `_checks.py`, standard library only) or its reference text, never a screenshot. Rules the starting file already holds, or holds once an empty sheet is added, are guards, so an untouched file or a blank sheet scores 0; a saved file the readers can't read (missing, corrupt, not a regular file, too large) scores 0 rather than crashing or stalling the verifier. `deepgym check` asks a computer-use folder for `reference_actions.json` instead of `reference_solution.*`, and is inconclusive on one off Daytona. +- **OSWorld computer-use tasks:** `scripts/import_osworld.py` imports 16 of OSWorld's (Apache-2.0) LibreOffice Calc and Writer tasks as computer-use environments, each with a reference episode, and writes OSWorld's license and a NOTICE next to them. They boot the Daytona snapshot `deepgym-osworld-libreoffice` (`--build-snapshot` creates it), since Daytona's default image has no LibreOffice; `metadata.json` takes a `snapshot`. The verifier scores the saved file with the task's own `compare_table` rules (ported into the shared `_checks.py`, standard library only) or its reference text, never a screenshot. Rules the starting file already holds, or holds once an empty sheet is added, are guards, so an untouched file or a blank sheet scores 0; a saved file the readers can't read (missing, corrupt, not a regular file, too large) scores 0, and reading and scoring run in a time- and memory-limited child whose failure also scores 0, so nothing in the file can crash or stall the verifier. `deepgym check` asks a computer-use folder for `reference_actions.json` instead of `reference_solution.*`, and is inconclusive on one off Daytona. ## 0.5.0 (2026-10-14) diff --git a/deepgym/docs/environments/importers.md b/deepgym/docs/environments/importers.md index 353e93ca..3f4ffff3 100644 --- a/deepgym/docs/environments/importers.md +++ b/deepgym/docs/environments/importers.md @@ -307,8 +307,9 @@ deepgym qa --mode daytona ~/.deepgym/environments/osworld/ earns nothing. OSWorld's evaluator is all-or-nothing; here a task with several rules pays the share it got right. - Whatever is at the result path, the verifier gives a verdict: a file it can't read - (missing, corrupt, not a regular file, or past the size limits) scores 0, never a - crash or a timeout, and a rule it can't evaluate fails. + (missing, corrupt, not a regular file, or past the size limits) scores 0, and a rule + it can't evaluate fails every rule. Reading and scoring run in a child process with + a 20 s and 1 GiB limit; a child that fails or overruns scores 0, never a timeout. - Each task ships a reference episode, `reference_actions.json`, that `deepgym qa` replays. - The output folder keeps OSWorld's `LICENSE` and a `NOTICE` that lists what was From 499252444a974656a9ff9f2a7e3c431a0cbb4e34 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 18:12:53 +0530 Subject: [PATCH 13/19] fix(osworld): read the workbook the package names, apply the 1904 epoch, and never equate a boolean with a number --- .../src/deepgym/envs/computer_use/_checks.py | 119 ++++++++++-------- deepgym/tests/office_fixtures.py | 24 ++-- deepgym/tests/test_import_osworld.py | 65 ++++++++++ 3 files changed, 152 insertions(+), 56 deletions(-) diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index 49021356..3964ccbb 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -11,6 +11,7 @@ import json import math import os +import posixpath import re import resource import select @@ -201,6 +202,41 @@ def check_namespace(_: str | None, uri: str) -> None: return ET.fromstring(data) +def _relationships(archive: zipfile.ZipFile, part: str) -> list[tuple[str, str, str]]: + """(id, type, target part) of each internal relationship of *part* ('' for the package). + + Targets resolve against the part's folder, as office readers resolve them. + """ + folder, _, name = part.rpartition('/') + rels = f'{folder}/_rels/{name}.rels' if folder else f'_rels/{name}.rels' + if rels not in archive.namelist(): + return [] + found = [] + for rel in _read_xml(archive, rels).findall(f'{_PKG_REL_NS}Relationship'): + if rel.get('TargetMode') == 'External': + continue + target = rel.get('Target', '') + if target.startswith('/'): + target = target[1:] + else: + target = posixpath.normpath(posixpath.join(folder, target)) + found.append((rel.get('Id', ''), rel.get('Type', ''), target)) + return found + + +def _main_part(archive: zipfile.ZipFile) -> str: + """The part the package's officeDocument relationship names, as python-docx and + openpyxl resolve it, not whatever sits at the usual path. + + Raises: + ValueError: If the package names no main part or more than one. + """ + main = [t for _, kind, t in _relationships(archive, '') if kind.endswith('/officeDocument')] + if len(main) != 1: + raise ValueError(f'{len(main)} main parts') + return main[0] + + def _coord_to_cell(coord: str) -> tuple[int, int]: """'C7' -> (row 7, column 3), both 1-based. @@ -263,15 +299,15 @@ def _cell_value(cell: ET.Element, shared: list[str]) -> object: """Quoted text, escapes, padding and [colour/locale/elapsed] sections of a format code.""" -def _date_styles(archive: zipfile.ZipFile) -> set[int]: - """Indexes of the cell styles whose number format shows a date or time. +def _date_styles(archive: zipfile.ZipFile, part: str | None) -> set[int]: + """Indexes of the cell styles in styles *part* whose number format shows a date. openpyxl reads a number in such a cell as a datetime, so pandas never equates it with a plain number; :func:`_sheet_grid` marks those cells the same way. """ - if 'xl/styles.xml' not in archive.namelist(): + if part is None: return set() - root = _read_xml(archive, 'xl/styles.xml') + root = _read_xml(archive, part) custom = { int(fmt.get('numFmtId', '-1')): fmt.get('formatCode', '') for fmt in root.iter(f'{_XLSX_NS}numFmt') @@ -320,13 +356,14 @@ def _trim(grid: list[list[object]]) -> list[list[object]]: def _sheet_grid( - root: ET.Element, shared: list[str], dated: set[int], budget: int + root: ET.Element, shared: list[str], dated: set[int], shift: int, budget: int ) -> list[list[object]]: """Rows of cell values over the sheet's used range; missing cells are None. The used range ends at the last non-empty row and column, as after :func:`_trim`. A number in a date-formatted cell (style index in *dated*) becomes - ``{'date': serial}``, so it only equals the same serial shown as a date. + ``{'date': serial + shift, 'serial': serial}``: *shift* puts a 1904-based serial on + the 1900 calendar, so a date only equals the same calendar date shown as a date. Raises: ValueError: If the used range holds more than *budget* cells. @@ -346,7 +383,7 @@ def _sheet_grid( if value is None: continue if _is_number(value) and int(cell.get('s', '0')) in dated: - value = {'date': value} + value = {'date': value + shift, 'serial': value} r, c = _coord_to_cell(ref) cells[(r, c)] = value max_row = max(max_row, r) @@ -359,25 +396,6 @@ def _sheet_grid( return _trim(grid) -def _sheet_parts(archive: zipfile.ZipFile) -> list[tuple[str, str]]: - """(sheet name, part name) for every sheet, in workbook order.""" - workbook = _read_xml(archive, 'xl/workbook.xml') - rels = _read_xml(archive, 'xl/_rels/workbook.xml.rels') - targets = { - rel.get('Id'): rel.get('Target') for rel in rels.findall(f'{_PKG_REL_NS}Relationship') - } - sheets = [] - node = workbook.find(f'{_XLSX_NS}sheets') - for sheet in [] if node is None else node: - target = targets.get(sheet.get(f'{_REL_NS}id'), '') - if target.startswith('/'): - target = target[1:] - elif not target.startswith('xl/'): - target = f'xl/{target}' - sheets.append((sheet.get('name', ''), target)) - return sheets - - def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: """Every sheet of *path*: (name, values grid, merged ranges in A1 notation). @@ -386,8 +404,8 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: Returns: Sheets in workbook order. A grid's cells are strings, numbers (int when - integral), ``{'date': serial}`` for a number shown as a date or time, - booleans, error text, or None for an empty cell. + integral), ``{'date': ..., 'serial': ...}`` for a number shown as a date or + time (see :func:`_sheet_grid`), booleans, error text, or None for an empty cell. Raises: ValueError: If the file is missing or can't be read for any reason: the @@ -395,21 +413,32 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: """ try: with _open_package(path) as archive: - shared_node = None - if 'xl/sharedStrings.xml' in archive.namelist(): - shared_node = _read_xml(archive, 'xl/sharedStrings.xml') + book_part = _main_part(archive) + book = _read_xml(archive, book_part) + related = _relationships(archive, book_part) + targets = {rel_id: target for rel_id, _, target in related} + parts = {kind.rsplit('/', 1)[-1]: target for _, kind, target in related} + shared_node = ( + _read_xml(archive, parts['sharedStrings']) if 'sharedStrings' in parts else None + ) shared = [ ''.join(t.text or '' for t in si.iter(f'{_XLSX_NS}t')) for si in (shared_node if shared_node is not None else []) ] - dated = _date_styles(archive) + dated = _date_styles(archive, parts.get('styles')) + properties = book.find(f'{_XLSX_NS}workbookPr') + epoch = '' if properties is None else properties.get('date1904', '') + shift = 1462 if epoch.lower() in ('1', 'true') else 0 + node = book.find(f'{_XLSX_NS}sheets') sheets = [] budget = _MAX_CELLS parsed: dict[str, tuple[list[list[object]], list[str]]] = {} - for name, part in _sheet_parts(archive): + for sheet in [] if node is None else node: + name, part = sheet.get('name', ''), targets[sheet.get(f'{_REL_NS}id', '')] if part not in parsed: # sheets naming one part parse it once root = _read_xml(archive, part) - parsed[part] = (_sheet_grid(root, shared, dated, budget), _merge_ranges(root)) + grid = _sheet_grid(root, shared, dated, shift, budget) + parsed[part] = (grid, _merge_ranges(root)) grid, merges = parsed[part] budget -= len(grid) * max((len(row) for row in grid), default=0) if budget < 0: @@ -457,17 +486,7 @@ def read_docx_text(path: str) -> str: """ try: with _open_package(path) as archive: - # The main part is the one the package's officeDocument relationship names, - # as python-docx resolves it, not whatever sits at word/document.xml. - rels = _read_xml(archive, '_rels/.rels') - main = [ - rel.get('Target', '').lstrip('/') - for rel in rels.findall(f'{_PKG_REL_NS}Relationship') - if rel.get('Type', '').endswith('/relationships/officeDocument') - ] - if len(main) != 1: - raise ValueError(f'{len(main)} main document parts') - root = _read_xml(archive, main[0]) + root = _read_xml(archive, _main_part(archive)) body = root.find(f'{_W_NS}body') paragraphs = [] for para in [] if body is None else body: @@ -507,12 +526,14 @@ def _is_number(value: object) -> bool: def _same_value(a: object, b: object) -> bool: """Elementwise equality as pandas uses it: None only equals None; numbers compare - across int/float (and bool, as Python's `==` does); other types must match, and - a date only equals the same date.""" + across int/float; a boolean only equals a boolean and a date only the same + calendar date; other types must match.""" if a is None or b is None: return a is None and b is None if isinstance(a, dict) or isinstance(b, dict): - return a == b + return isinstance(a, dict) and isinstance(b, dict) and a['date'] == b['date'] + if isinstance(a, bool) or isinstance(b, bool): + return isinstance(a, bool) and isinstance(b, bool) and a == b if isinstance(a, (int, float)) and isinstance(b, (int, float)): return float(a) == float(b) if isinstance(a, str) and isinstance(b, str): @@ -694,7 +715,7 @@ def grid_of(idx: object) -> list[list[object]] | None: if prop == 'value': value = cell_at(rows, rule['coordinate']) if isinstance(value, dict): # check_cell reads a date's raw serial - value = value['date'] + value = value['serial'] elif prop == 'merge': value = is_merged(merges, rule['coordinate']) else: diff --git a/deepgym/tests/office_fixtures.py b/deepgym/tests/office_fixtures.py index 4ba34d43..49f4d4ab 100644 --- a/deepgym/tests/office_fixtures.py +++ b/deepgym/tests/office_fixtures.py @@ -35,6 +35,7 @@ def make_xlsx(path: Path, sheets: list[tuple[str, dict[str, object], list[str]]] for index, (name, cells, merges) in enumerate(sheets, start=1): parts.append((name, _sheet_xml(cells, merges, shared), index)) members = { + '_rels/.rels': _package_rels('xl/workbook.xml'), 'xl/workbook.xml': _workbook_xml([name for name, _, _ in parts]), 'xl/_rels/workbook.xml.rels': _rels_xml(len(parts)), 'xl/styles.xml': _STYLES_XML, @@ -103,11 +104,25 @@ def _workbook_xml(names: list[str]) -> str: return f'{sheets}' +def _package_rels(main: str) -> str: + """The package's relationships: its main part, at *main*.""" + return ( + f'' + ) + + def _rels_xml(count: int) -> str: + """The workbook's relationships: its sheets, shared strings and styles.""" rels = ''.join( - f'' + f'' for index in range(1, count + 1) ) + rels += ( + f'' + f'' + ) return f'{rels}' @@ -118,12 +133,7 @@ def make_docx(path: Path, paragraphs: list[str]) -> Path: for para in paragraphs ) document = f'{body}' - main = f'{_REL_NS}/officeDocument' - rels = ( - f'' - f'' - ) with zipfile.ZipFile(path, 'w') as archive: - archive.writestr('_rels/.rels', rels) + archive.writestr('_rels/.rels', _package_rels('word/document.xml')) archive.writestr('word/document.xml', document) return path diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index d61e9999..ccfb7cf4 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -363,6 +363,71 @@ def test_a_date_only_matches_a_cell_shown_as_that_date(tmp_path: Path, work: Pat assert _run_verifier(env, home, work)['score'] == score, label +def _sheet_data_env(tmp_path: Path, start: dict, done: dict): + source = make_xlsx(tmp_path / 'source.xlsx', [('Sheet1', start, [])]) + gold = make_xlsx(tmp_path / 'gold.xlsx', [('Sheet1', done, [])]) + rules = [{'type': 'sheet_data', 'sheet_idx0': 0, 'sheet_idx1': 'EI0'}] + return _import(tmp_path, _task_config(rules=rules), source, gold)[0] + + +def test_the_workbook_scored_is_the_one_the_package_names(tmp_path: Path, work: Path) -> None: + """openpyxl reads the workbook the package's relationships name, so a stray + xl/workbook.xml holding the right answer is not what gets scored.""" + start, done = {'A1': 'Total'}, {'A1': 'Total', 'A2': 123} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + right = make_xlsx(tmp_path / 'right.xlsx', [('Sheet1', done, [])]).read_bytes() + with zipfile.ZipFile(io.BytesIO(right)) as archive: + book = archive.read('xl/workbook.xml').decode() + book_rels = archive.read('xl/_rels/workbook.xml.rels').decode() + wrong = make_xlsx(tmp_path / 'wrong.xlsx', [('Sheet1', {'A1': 'Total', 'A2': 999}, [])]) + with zipfile.ZipFile(wrong) as archive: + wrong_sheet = archive.read('xl/worksheets/sheet1.xml').decode() + rels = ( + '' + '' + ) + _repacked( + home / 'Desktop' / FILE, + right, + { + '_rels/.rels': rels, + 'xl/declared.xml': book, + 'xl/_rels/declared.xml.rels': book_rels.replace('sheet1.xml', 'wrong.xml'), + 'xl/worksheets/wrong.xml': wrong_sheet, + }, + ) + + assert _run_verifier(env, home, work)['score'] == 0.0 + + +def test_a_1904_workbook_compares_calendar_dates(tmp_path: Path, work: Path) -> None: + """The same serial is a different date in a 1904-based workbook: it fails.""" + start = {'A1': 'Start', 'B1': 'Due', 'A2': Date(40437)} + done = {**start, 'B2': Date(40557)} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + good = make_xlsx(tmp_path / 'good.xlsx', [('Sheet1', done, [])]).read_bytes() + with zipfile.ZipFile(io.BytesIO(good)) as archive: + book = archive.read('xl/workbook.xml').decode() + shifted = book.replace('', '', 1) + _repacked(home / 'Desktop' / FILE, good, {'xl/workbook.xml': shifted}) + + assert _run_verifier(env, home, work)['score'] == 0.0 + + +def test_a_boolean_does_not_match_a_number(tmp_path: Path, work: Path) -> None: + start, done = {'A1': 'Flag'}, {'A1': 'Flag', 'A2': 1} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + for label, cells, score in (('TRUE for 1', {**start, 'A2': True}, 0.0), ('1', done, 1.0)): + _save(tmp_path, home, cells, []) + assert _run_verifier(env, home, work)['score'] == score, label + + def _patch_zip(data: bytes, member: str, edit) -> bytes: """*data* with *edit(header, payload)* applied to *member*'s local header and data.""" with zipfile.ZipFile(io.BytesIO(data)) as archive: From a85f10372044daee8d3ea77d068bcfcf79fb13e8 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 18:21:22 +0530 Subject: [PATCH 14/19] fix(osworld): tell dates, times and durations apart, and refuse workbooks whose declarations disagree --- .../src/deepgym/envs/computer_use/_checks.py | 100 +++++++++++++----- deepgym/tests/office_fixtures.py | 42 +++++++- deepgym/tests/test_import_osworld.py | 48 ++++++++- 3 files changed, 157 insertions(+), 33 deletions(-) diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index 3964ccbb..e0925ab3 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -76,6 +76,7 @@ def report(progress: dict[str, bool], guards: dict[str, bool]) -> None: _REL_NS = '{http://schemas.openxmlformats.org/officeDocument/2006/relationships}' _PKG_REL_NS = '{http://schemas.openxmlformats.org/package/2006/relationships}' _W_NS = '{http://schemas.openxmlformats.org/wordprocessingml/2006/main}' +_CT_NS = '{http://schemas.openxmlformats.org/package/2006/content-types}' _COORD_RE = re.compile(r'([A-Z]{1,3})([1-9][0-9]{0,6})') _MAX_ROW, _MAX_COL = 1_048_576, 16_384 @@ -294,19 +295,25 @@ def _cell_value(cell: ET.Element, shared: list[str]) -> object: _DATE_FORMAT_IDS = frozenset({14, 15, 16, 17, 18, 19, 20, 21, 22, 45, 46, 47}) +_DURATION_FORMAT_IDS = frozenset({46}) +"""Built-in formats openpyxl reads as a duration ([h]:mm:ss) rather than a date.""" +_ELAPSED = re.compile(r'\[(h+|m+|s+)\]', re.IGNORECASE) +"""An elapsed-time section, which makes a format a duration.""" """Built-in number formats that show a date or time.""" _FORMAT_NOISE = re.compile(r'"[^"]*"|\\.|_.|\*.|\[[^\]]*\]') """Quoted text, escapes, padding and [colour/locale/elapsed] sections of a format code.""" -def _date_styles(archive: zipfile.ZipFile, part: str | None) -> set[int]: - """Indexes of the cell styles in styles *part* whose number format shows a date. +def _date_styles(archive: zipfile.ZipFile, part: str | None) -> dict[int, str]: + """{style index: 'date' or 'duration'} for the cell styles in styles *part* whose + number format shows a date, time or elapsed time. - openpyxl reads a number in such a cell as a datetime, so pandas never equates it - with a plain number; :func:`_sheet_grid` marks those cells the same way. + openpyxl reads a number in such a cell as a datetime, a time or a timedelta, so + pandas never equates it with a plain number or with another kind; + :func:`_sheet_grid` marks those cells the same way. """ if part is None: - return set() + return {} root = _read_xml(archive, part) custom = { int(fmt.get('numFmtId', '-1')): fmt.get('formatCode', '') @@ -314,17 +321,21 @@ def _date_styles(archive: zipfile.ZipFile, part: str | None) -> set[int]: } if any(len(code) > 255 for code in custom.values()): # longer than any real one raise ValueError('a number format code is too long') - dated_ids = { - fmt_id - for fmt_id, code in custom.items() - if re.search(r'[dmyhs]', _FORMAT_NOISE.sub('', code.split(';')[0]), re.IGNORECASE) - } | (_DATE_FORMAT_IDS - set(custom)) + kinds = {fmt_id: 'date' for fmt_id in _DATE_FORMAT_IDS - set(custom)} + kinds |= dict.fromkeys(_DURATION_FORMAT_IDS - set(custom), 'duration') + for fmt_id, code in custom.items(): + first = code.split(';')[0] + if _ELAPSED.search(first): + kinds[fmt_id] = 'duration' + elif re.search(r'[dmyhs]', _FORMAT_NOISE.sub('', first), re.IGNORECASE): + kinds[fmt_id] = 'date' xfs = root.find(f'{_XLSX_NS}cellXfs') - return { - index - for index, xf in enumerate([] if xfs is None else xfs.findall(f'{_XLSX_NS}xf')) - if int(xf.get('numFmtId', '0')) in dated_ids - } + styles = {} + for index, xf in enumerate([] if xfs is None else xfs.findall(f'{_XLSX_NS}xf')): + kind = kinds.get(int(xf.get('numFmtId', '0'))) + if kind: + styles[index] = kind + return styles def _merge_ranges(root: ET.Element) -> list[str]: @@ -356,14 +367,16 @@ def _trim(grid: list[list[object]]) -> list[list[object]]: def _sheet_grid( - root: ET.Element, shared: list[str], dated: set[int], shift: int, budget: int + root: ET.Element, shared: list[str], dated: dict[int, str], shift: int, budget: int ) -> list[list[object]]: """Rows of cell values over the sheet's used range; missing cells are None. The used range ends at the last non-empty row and column, as after :func:`_trim`. - A number in a date-formatted cell (style index in *dated*) becomes - ``{'date': serial + shift, 'serial': serial}``: *shift* puts a 1904-based serial on - the 1900 calendar, so a date only equals the same calendar date shown as a date. + A number in a date- or duration-formatted cell (style index in *dated*) is marked + the way openpyxl decodes it: ``{'duration': serial}``, ``{'time': serial}`` (a date + format below 1 day) or ``{'date': serial + shift}`` (*shift* puts a 1904-based serial + on the 1900 calendar), each with the raw ``'serial'`` that check_cell reads. A + marked cell only equals one of the same kind and value. Raises: ValueError: If the used range holds more than *budget* cells. @@ -382,7 +395,12 @@ def _sheet_grid( value = _cell_value(cell, shared) if value is None: continue - if _is_number(value) and int(cell.get('s', '0')) in dated: + kind = dated.get(int(cell.get('s', '0'))) if _is_number(value) else None + if kind == 'duration': + value = {'duration': value, 'serial': value} + elif kind and 0 <= value < 1: + value = {'time': value, 'serial': value} + elif kind: value = {'date': value + shift, 'serial': value} r, c = _coord_to_cell(ref) cells[(r, c)] = value @@ -396,6 +414,31 @@ def _sheet_grid( return _trim(grid) +def _check_declarations(archive: zipfile.ZipFile, book: str, parts: dict[str, str]) -> None: + """Refuse a workbook whose content types and relationships name different parts. + + openpyxl finds the workbook and shared strings through [Content_Types].xml and + reads styles from xl/styles.xml; this reader follows relationships. Where the two + could disagree, the file is unreadable, so both always read the same parts. + + Raises: + ValueError: If the declarations disagree. + """ + types = _read_xml(archive, '[Content_Types].xml') + declared = { + node.get('PartName', '').lstrip('/'): node.get('ContentType', '') + for node in types.findall(f'{_CT_NS}Override') + } + books = {name for name, kind in declared.items() if kind.endswith('.main+xml')} + strings = {name for name, kind in declared.items() if kind.endswith('.sharedStrings+xml')} + if books != {book}: + raise ValueError(f'content types declare {sorted(books)}, relationships {book}') + if strings != ({parts['sharedStrings']} if 'sharedStrings' in parts else set()): + raise ValueError('content types and relationships name different shared strings') + if parts.get('styles', 'xl/styles.xml') != 'xl/styles.xml': + raise ValueError(f'styles at {parts["styles"]}, not xl/styles.xml') + + def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: """Every sheet of *path*: (name, values grid, merged ranges in A1 notation). @@ -418,6 +461,7 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: related = _relationships(archive, book_part) targets = {rel_id: target for rel_id, _, target in related} parts = {kind.rsplit('/', 1)[-1]: target for _, kind, target in related} + _check_declarations(archive, book_part, parts) shared_node = ( _read_xml(archive, parts['sharedStrings']) if 'sharedStrings' in parts else None ) @@ -425,7 +469,9 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: ''.join(t.text or '' for t in si.iter(f'{_XLSX_NS}t')) for si in (shared_node if shared_node is not None else []) ] - dated = _date_styles(archive, parts.get('styles')) + # openpyxl reads styles from xl/styles.xml whatever the relationships say. + styles = 'xl/styles.xml' if 'xl/styles.xml' in archive.namelist() else None + dated = _date_styles(archive, styles) properties = book.find(f'{_XLSX_NS}workbookPr') epoch = '' if properties is None else properties.get('date1904', '') shift = 1462 if epoch.lower() in ('1', 'true') else 0 @@ -526,12 +572,16 @@ def _is_number(value: object) -> bool: def _same_value(a: object, b: object) -> bool: """Elementwise equality as pandas uses it: None only equals None; numbers compare - across int/float; a boolean only equals a boolean and a date only the same - calendar date; other types must match.""" + across int/float; a boolean only equals a boolean, and a date, time or duration + only one of the same kind and value; other types must match.""" if a is None or b is None: return a is None and b is None - if isinstance(a, dict) or isinstance(b, dict): - return isinstance(a, dict) and isinstance(b, dict) and a['date'] == b['date'] + if isinstance(a, dict) or isinstance(b, dict): # dates, times, durations + if not (isinstance(a, dict) and isinstance(b, dict)): + return False + return {k: v for k, v in a.items() if k != 'serial'} == { + k: v for k, v in b.items() if k != 'serial' + } if isinstance(a, bool) or isinstance(b, bool): return isinstance(a, bool) and isinstance(b, bool) and a == b if isinstance(a, (int, float)) and isinstance(b, (int, float)): diff --git a/deepgym/tests/office_fixtures.py b/deepgym/tests/office_fixtures.py index 49f4d4ab..e2286982 100644 --- a/deepgym/tests/office_fixtures.py +++ b/deepgym/tests/office_fixtures.py @@ -24,11 +24,15 @@ class Date(int): """A serial number in a cell formatted as a date (built-in number format 14).""" +class Duration(int): + """A serial number in a cell formatted as elapsed time (built-in format 46, [h]:mm:ss).""" + + def make_xlsx(path: Path, sheets: list[tuple[str, dict[str, object], list[str]]]) -> Path: """Write a minimal workbook: one entry per sheet, (name, cells, merged ranges). - A cell value may be a number, a bool, a Date, a str (shared string) or an Inline - (an inline string); a cell left out is empty. Cells are 1-based references like 'C7'. + A cell value may be a number, a bool, a Date, a Duration, a str (shared string) or + an Inline (an inline string); a cell left out is empty. Cells are 1-based references like 'C7'. """ shared: dict[str, int] = {} parts = [] @@ -43,6 +47,7 @@ def make_xlsx(path: Path, sheets: list[tuple[str, dict[str, object], list[str]]] members.update({f'xl/worksheets/sheet{index}.xml': xml for _, xml, index in parts}) if shared: members['xl/sharedStrings.xml'] = _shared_xml(shared) + members['[Content_Types].xml'] = _content_types(members) with zipfile.ZipFile(path, 'w') as archive: for member, xml in members.items(): archive.writestr(member, xml) @@ -54,6 +59,8 @@ def _cell_xml(ref: str, value: object, shared: dict[str, int]) -> str: return f'{int(value)}' if isinstance(value, Date): return f'{int(value)}' + if isinstance(value, Duration): + return f'{int(value)}' if isinstance(value, (int, float)): return f'{value!r}' if isinstance(value, Inline): @@ -83,10 +90,11 @@ def _sheet_xml(cells: dict[str, object], merges: list[str], shared: dict[str, in _STYLES_XML = ( - f'' - '' + f'' + '' + '' ) -"""Style 0 is General, style 1 a date.""" +"""Style 0 is General, style 1 a date, style 2 elapsed time.""" def _shared_xml(shared: dict[str, int]) -> str: @@ -104,6 +112,30 @@ def _workbook_xml(names: list[str]) -> str: return f'{sheets}' +_SHEET_TYPES = 'application/vnd.openxmlformats-officedocument.spreadsheetml' +_CT_NS = 'http://schemas.openxmlformats.org/package/2006/content-types' + + +def _content_types(members: dict[str, str]) -> str: + """[Content_Types].xml declaring the workbook, sheets, shared strings and styles.""" + kinds = { + 'xl/workbook.xml': 'sheet.main+xml', + 'xl/sharedStrings.xml': 'sharedStrings+xml', + 'xl/styles.xml': 'styles+xml', + } + overrides = ''.join( + f'' + for name in members + if name.startswith('xl/') and name.endswith('.xml') and '_rels' not in name + ) + return ( + f'' + f'{overrides}' + ) + + def _package_rels(main: str) -> str: """The package's relationships: its main part, at *main*.""" return ( diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index ccfb7cf4..03eb9687 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -23,7 +23,7 @@ from deepgym import Action, ComputerUseEnvironment from deepgym.registry import load_environment_dir -from tests.office_fixtures import Date, make_docx, make_xlsx +from tests.office_fixtures import Date, Duration, make_docx, make_xlsx scripts_dir = Path(__file__).resolve().parent.parent / 'scripts' if str(scripts_dir) not in sys.path: @@ -371,8 +371,9 @@ def _sheet_data_env(tmp_path: Path, start: dict, done: dict): def test_the_workbook_scored_is_the_one_the_package_names(tmp_path: Path, work: Path) -> None: - """openpyxl reads the workbook the package's relationships name, so a stray - xl/workbook.xml holding the right answer is not what gets scored.""" + """A stray workbook holding the right answer is never what gets scored: here the + relationships name another workbook, the content types this one, and a package + whose declarations disagree is unreadable.""" start, done = {'A1': 'Total'}, {'A1': 'Total', 'A2': 123} env = _sheet_data_env(tmp_path, start, done) home = tmp_path / 'home' @@ -419,6 +420,47 @@ def test_a_1904_workbook_compares_calendar_dates(tmp_path: Path, work: Path) -> assert _run_verifier(env, home, work)['score'] == 0.0 +def test_shared_strings_the_declarations_disagree_on_are_unreadable( + tmp_path: Path, work: Path +) -> None: + start, done = {'A1': 'Total'}, {'A1': 'Total', 'A2': 'Correct'} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + good = make_xlsx(tmp_path / 'good.xlsx', [('Sheet1', done, [])]).read_bytes() + with zipfile.ZipFile(io.BytesIO(good)) as archive: + types = archive.read('[Content_Types].xml').decode() + strings = archive.read('xl/sharedStrings.xml').decode() + _repacked( + home / 'Desktop' / FILE, + good, + { + '[Content_Types].xml': types.replace('/xl/sharedStrings.xml', '/xl/other.xml'), + 'xl/other.xml': strings.replace('Correct', 'Wrong'), + }, + ) + + result = _run_verifier(env, home, work) + + assert result['score'] == 0.0 + assert 'workbook_saved' in result['details'] + + +def test_an_elapsed_time_does_not_match_a_date(tmp_path: Path, work: Path) -> None: + """openpyxl reads [h]:mm:ss as a timedelta, so the date's serial shown as elapsed + time is not the date.""" + start = {'A1': 'Start', 'B1': 'Due', 'A2': Date(40437)} + done = {**start, 'B2': Date(40557)} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + for label, cells, score in ( + ('elapsed time', {**start, 'B2': Duration(40557)}, 0.0), + ('the date', done, 1.0), + ): + _save(tmp_path, home, cells, []) + assert _run_verifier(env, home, work)['score'] == score, label + + def test_a_boolean_does_not_match_a_number(tmp_path: Path, work: Path) -> None: start, done = {'A1': 'Flag'}, {'A1': 'Flag', 'A2': 1} env = _sheet_data_env(tmp_path, start, done) From 3a8742157298beb74ecf5ab9814eaace2e1ce8da Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 18:33:56 +0530 Subject: [PATCH 15/19] fix(osworld): classify formats as openpyxl does, refuse duplicate declarations, and require a Word manifest --- .../src/deepgym/envs/computer_use/_checks.py | 55 ++++++++++++------ deepgym/tests/office_fixtures.py | 11 ++++ deepgym/tests/test_import_osworld.py | 58 ++++++++++++++++++- 3 files changed, 105 insertions(+), 19 deletions(-) diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index e0925ab3..ea34dd9b 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -77,6 +77,7 @@ def report(progress: dict[str, bool], guards: dict[str, bool]) -> None: _PKG_REL_NS = '{http://schemas.openxmlformats.org/package/2006/relationships}' _W_NS = '{http://schemas.openxmlformats.org/wordprocessingml/2006/main}' _CT_NS = '{http://schemas.openxmlformats.org/package/2006/content-types}' +_WML_MAIN = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document.main+xml' _COORD_RE = re.compile(r'([A-Z]{1,3})([1-9][0-9]{0,6})') _MAX_ROW, _MAX_COL = 1_048_576, 16_384 @@ -297,11 +298,13 @@ def _cell_value(cell: ET.Element, shared: list[str]) -> object: _DATE_FORMAT_IDS = frozenset({14, 15, 16, 17, 18, 19, 20, 21, 22, 45, 46, 47}) _DURATION_FORMAT_IDS = frozenset({46}) """Built-in formats openpyxl reads as a duration ([h]:mm:ss) rather than a date.""" -_ELAPSED = re.compile(r'\[(h+|m+|s+)\]', re.IGNORECASE) -"""An elapsed-time section, which makes a format a duration.""" +_TIMEDELTA = re.compile( + r'\[hh?\](:mm(:ss(\.0*)?)?)?|\[mm?\](:ss(\.0*)?)?|\[ss?\](\.0*)?', re.IGNORECASE +) +"""openpyxl's TIMEDELTA_RE: an elapsed-time section makes a date format a duration.""" """Built-in number formats that show a date or time.""" -_FORMAT_NOISE = re.compile(r'"[^"]*"|\\.|_.|\*.|\[[^\]]*\]') -"""Quoted text, escapes, padding and [colour/locale/elapsed] sections of a format code.""" +_FORMAT_NOISE = re.compile(r'".*?"|\[(?!hh?\]|mm?\]|ss?\])[^\]]*\]') +"""openpyxl's STRIP_RE: quoted text and bracketed sections other than [h], [m], [s].""" def _date_styles(archive: zipfile.ZipFile, part: str | None) -> dict[int, str]: @@ -323,12 +326,10 @@ def _date_styles(archive: zipfile.ZipFile, part: str | None) -> dict[int, str]: raise ValueError('a number format code is too long') kinds = {fmt_id: 'date' for fmt_id in _DATE_FORMAT_IDS - set(custom)} kinds |= dict.fromkeys(_DURATION_FORMAT_IDS - set(custom), 'duration') - for fmt_id, code in custom.items(): + for fmt_id, code in custom.items(): # openpyxl's is_date_format, then is_timedelta_format first = code.split(';')[0] - if _ELAPSED.search(first): - kinds[fmt_id] = 'duration' - elif re.search(r'[dmyhs]', _FORMAT_NOISE.sub('', first), re.IGNORECASE): - kinds[fmt_id] = 'date' + if re.search(r'(? tuple[dict[str, str], dict[str, str]]: + """[Content_Types].xml as ({part name: type}, {extension: type}). + + Raises: + ValueError: If a part or extension is declared twice: readers would disagree + on which declaration counts. + """ + types = _read_xml(archive, '[Content_Types].xml') + found: tuple[dict[str, str], dict[str, str]] = ({}, {}) + for table, tag, key in ((found[0], 'Override', 'PartName'), (found[1], 'Default', 'Extension')): + for node in types.findall(f'{_CT_NS}{tag}'): + name = node.get(key, '').lstrip('/').lower() + if name in table: + raise ValueError(f'[Content_Types].xml declares {name} twice') + table[name] = node.get('ContentType', '') + return found + + def _check_declarations(archive: zipfile.ZipFile, book: str, parts: dict[str, str]) -> None: """Refuse a workbook whose content types and relationships name different parts. @@ -424,16 +443,12 @@ def _check_declarations(archive: zipfile.ZipFile, book: str, parts: dict[str, st Raises: ValueError: If the declarations disagree. """ - types = _read_xml(archive, '[Content_Types].xml') - declared = { - node.get('PartName', '').lstrip('/'): node.get('ContentType', '') - for node in types.findall(f'{_CT_NS}Override') - } + declared, _ = _content_types(archive) books = {name for name, kind in declared.items() if kind.endswith('.main+xml')} strings = {name for name, kind in declared.items() if kind.endswith('.sharedStrings+xml')} - if books != {book}: + if books != {book.lower()}: raise ValueError(f'content types declare {sorted(books)}, relationships {book}') - if strings != ({parts['sharedStrings']} if 'sharedStrings' in parts else set()): + if strings != ({parts['sharedStrings'].lower()} if 'sharedStrings' in parts else set()): raise ValueError('content types and relationships name different shared strings') if parts.get('styles', 'xl/styles.xml') != 'xl/styles.xml': raise ValueError(f'styles at {parts["styles"]}, not xl/styles.xml') @@ -532,7 +547,13 @@ def read_docx_text(path: str) -> str: """ try: with _open_package(path) as archive: - root = _read_xml(archive, _main_part(archive)) + main = _main_part(archive) + # python-docx opens only a main part whose content type is a Word document. + declared, defaults = _content_types(archive) + kind = declared.get(main.lower()) or defaults.get(main.rpartition('.')[2].lower()) + if kind != _WML_MAIN: + raise ValueError(f'{main} has content type {kind!r}, not a Word document') + root = _read_xml(archive, main) body = root.find(f'{_W_NS}body') paragraphs = [] for para in [] if body is None else body: diff --git a/deepgym/tests/office_fixtures.py b/deepgym/tests/office_fixtures.py index e2286982..8b6e696d 100644 --- a/deepgym/tests/office_fixtures.py +++ b/deepgym/tests/office_fixtures.py @@ -166,6 +166,17 @@ def make_docx(path: Path, paragraphs: list[str]) -> Path: ) document = f'{body}' with zipfile.ZipFile(path, 'w') as archive: + archive.writestr('[Content_Types].xml', docx_content_types('word/document.xml')) archive.writestr('_rels/.rels', _package_rels('word/document.xml')) archive.writestr('word/document.xml', document) return path + + +def docx_content_types(main: str) -> str: + """[Content_Types].xml declaring *main* as a Word document.""" + wml = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document.main+xml' + return ( + f'' + f'' + ) diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 03eb9687..344f26dd 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -23,7 +23,7 @@ from deepgym import Action, ComputerUseEnvironment from deepgym.registry import load_environment_dir -from tests.office_fixtures import Date, Duration, make_docx, make_xlsx +from tests.office_fixtures import Date, Duration, docx_content_types, make_docx, make_xlsx scripts_dir = Path(__file__).resolve().parent.parent / 'scripts' if str(scripts_dir) not in sys.path: @@ -461,6 +461,51 @@ def test_an_elapsed_time_does_not_match_a_date(tmp_path: Path, work: Path) -> No assert _run_verifier(env, home, work)['score'] == score, label +def test_a_part_declared_twice_is_unreadable(tmp_path: Path, work: Path) -> None: + """Readers disagree on which of two declarations counts, so the file is a 0.""" + start, done = {'A1': 'Total'}, {'A1': 'Total', 'A2': 123} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + good = make_xlsx(tmp_path / 'good.xlsx', [('Sheet1', done, [])]).read_bytes() + with zipfile.ZipFile(io.BytesIO(good)) as archive: + types = archive.read('[Content_Types].xml').decode() + twice = types.replace( + '', + '', + ) + _repacked(home / 'Desktop' / FILE, good, {'[Content_Types].xml': twice}) + + assert _run_verifier(env, home, work)['score'] == 0.0 + + +def test_a_number_in_a_format_openpyxl_reads_as_a_date_is_not_that_number( + tmp_path: Path, work: Path +) -> None: + """'*d0' pads with 'd', and openpyxl's classifier counts it as a date format.""" + start, done = {'A1': 'Total'}, {'A1': 'Total', 'A2': 123} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + good = make_xlsx(tmp_path / 'good.xlsx', [('Sheet1', done, [])]).read_bytes() + with zipfile.ZipFile(io.BytesIO(good)) as archive: + sheet = archive.read('xl/worksheets/sheet1.xml').decode() + styles = ( + f'' + '' + ) + padded = sheet.replace('', '') + assert padded != sheet + _repacked( + home / 'Desktop' / FILE, + good, + {'xl/styles.xml': styles, 'xl/worksheets/sheet1.xml': padded}, + ) + + assert _run_verifier(env, home, work)['score'] == 0.0 + + def test_a_boolean_does_not_match_a_number(tmp_path: Path, work: Path) -> None: start, done = {'A1': 'Flag'}, {'A1': 'Flag', 'A2': 1} env = _sheet_data_env(tmp_path, start, done) @@ -750,7 +795,9 @@ def test_unsaved_changes_and_a_wrong_sheet_name_are_judged_zeros( assert _run_verifier(env, home, work)['score'] == score, label -@pytest.mark.parametrize('label', ['missing', 'not a zip', 'a FIFO', 'no package', 'two mains']) +@pytest.mark.parametrize( + 'label', ['missing', 'not a zip', 'a FIFO', 'no package', 'two mains', 'no manifest'] +) def test_a_saved_document_the_agent_broke_is_a_judged_zero( label: str, tmp_path: Path, work: Path ) -> None: @@ -763,6 +810,12 @@ def test_a_saved_document_the_agent_broke_is_a_judged_zero( rels = gold.read('_rels/.rels').decode() if label == 'not a zip': saved.write_bytes(b'not a document') + elif label == 'no manifest': # python-docx can't open a package without one + with zipfile.ZipFile(make_docx(tmp_path / 'right.docx', _DOC_GOLD)) as right: + with zipfile.ZipFile(saved, 'w') as archive: + for name in right.namelist(): + if name != '[Content_Types].xml': + archive.writestr(name, right.read(name)) elif label == 'a FIFO': os.mkfifo(saved) elif label in ('no package', 'two mains'): @@ -811,6 +864,7 @@ def test_the_main_document_part_is_the_one_the_package_names(tmp_path: Path) -> '' f'', ) + archive.writestr('[Content_Types].xml', docx_content_types('word/real.xml')) archive.writestr('word/real.xml', real) with zipfile.ZipFile(make_docx(tmp_path / 'gold.docx', _DOC_GOLD)) as gold: archive.writestr('word/document.xml', gold.read('word/document.xml')) From 14e235c84c614116cab8c0f2e4039ee4209c16da Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 18:41:04 +0530 Subject: [PATCH 16/19] fix(api): refuse a caller-chosen snapshot in session requests, as an image is --- deepgym/docs/api-server.md | 4 ++-- deepgym/docs/environments/sessions.md | 2 +- deepgym/src/deepgym/api/schemas.py | 8 ++++---- deepgym/tests/test_api.py | 18 +++++++++++++++--- deepgym/tests/test_harbor.py | 14 ++++++++++++++ 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/deepgym/docs/api-server.md b/deepgym/docs/api-server.md index 0606f89b..cecfaaeb 100644 --- a/deepgym/docs/api-server.md +++ b/deepgym/docs/api-server.md @@ -49,8 +49,8 @@ request per output; they don't call `/v1/run/batch` or `/v1/eval`. Registering an environment takes the same fields as an inline run. An invalid environment is a 422 before anything is queued. Environments sent to the run, batch, job and environment endpoints have no `image` or `snapshot` field, and one sent anyway is -dropped. A session request whose inline environment sets an `image` gets 422; a -`snapshot` is accepted. +dropped. A session request whose inline environment sets an `image` or a `snapshot` +gets 422. ## Limits diff --git a/deepgym/docs/environments/sessions.md b/deepgym/docs/environments/sessions.md index 9cedaf5e..7a44383b 100644 --- a/deepgym/docs/environments/sessions.md +++ b/deepgym/docs/environments/sessions.md @@ -136,7 +136,7 @@ DELETE /v1/sessions/{id} ``` - `environment` can also be an inline `MultiTurnEnvironment` spec; the API refuses one - that sets an `image` (a `snapshot` is accepted). + that sets an `image` or a `snapshot`. - At most 100 sessions run at once (429 above). Sessions idle for 30 minutes are closed by a sweep that runs every 60 seconds, and the server tries to close every session on shutdown. diff --git a/deepgym/src/deepgym/api/schemas.py b/deepgym/src/deepgym/api/schemas.py index a5f55548..e49f9f87 100644 --- a/deepgym/src/deepgym/api/schemas.py +++ b/deepgym/src/deepgym/api/schemas.py @@ -209,12 +209,12 @@ class CreateSessionRequest(BaseModel): @field_validator('environment') @classmethod - def reject_image( + def reject_runtime( cls, v: str | MultiTurnEnvironment | ComputerUseEnvironment ) -> str | MultiTurnEnvironment | ComputerUseEnvironment: - """Refuse a caller-chosen image: root in the sandbox runs the image's binaries.""" - if not isinstance(v, str) and v.image is not None: - raise ValueError('image cannot be set over the API') + """Refuse a caller-chosen image or snapshot: root in the sandbox runs its binaries.""" + if not isinstance(v, str) and (v.image is not None or v.snapshot is not None): + raise ValueError('image and snapshot cannot be set over the API') return v diff --git a/deepgym/tests/test_api.py b/deepgym/tests/test_api.py index fdb35c61..149aaae3 100644 --- a/deepgym/tests/test_api.py +++ b/deepgym/tests/test_api.py @@ -20,7 +20,7 @@ from deepgym.api.app import app from deepgym.api.deps import ServerDeepGym, get_deepgym from deepgym.api.routes import _environments, _sessions -from deepgym.api.schemas import RunRequest +from deepgym.api.schemas import APIEnvironment, RunRequest from deepgym.async_core import AsyncDeepGym from deepgym.core import DeepGym from deepgym.exceptions import SandboxError @@ -519,11 +519,23 @@ def test_unknown_env_is_404(self, client: TestClient) -> None: resp = client.post('/v1/sessions', json={'environment': 'no_such_env_xyz'}) assert resp.status_code == 404 - def test_caller_chosen_image_is_refused(self, client: TestClient) -> None: - env = {**FILE_CHECK_ENV, 'image': 'attacker/image:latest'} + @pytest.mark.parametrize('field', ['image', 'snapshot']) + def test_caller_chosen_runtime_is_refused(self, client: TestClient, field: str) -> None: + env = {**FILE_CHECK_ENV, field: 'attacker-runtime'} resp = client.post('/v1/sessions', json={'environment': env}) assert resp.status_code == 422 + def test_run_requests_never_carry_an_image_or_snapshot(self) -> None: + """The run, batch, job and environment endpoints drop both fields.""" + spec = { + 'task': 't', + 'verifier_code': 'return 1.0', + 'image': 'attacker/image:latest', + 'snapshot': 'attacker-snapshot', + } + env = APIEnvironment.model_validate(spec).to_environment() + assert (env.image, env.snapshot) == (None, None) + def test_session_cap_counts_booting_and_live_sessions( self, client: TestClient, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/deepgym/tests/test_harbor.py b/deepgym/tests/test_harbor.py index 49453649..03f3fed4 100644 --- a/deepgym/tests/test_harbor.py +++ b/deepgym/tests/test_harbor.py @@ -65,6 +65,20 @@ def test_parses_a_prebuilt_image_task() -> None: assert task.solution['answer.txt'] == b'hello\n' +def test_a_task_cannot_choose_a_snapshot(tmp_path: Path) -> None: + """task.toml has no snapshot field: one written there never reaches the sandbox.""" + folder = _task_copy(tmp_path) + toml = folder / 'task.toml' + toml.write_text( + toml.read_text().replace('[environment]', '[environment]\nsnapshot = "attacker"', 1) + ) + + env = HarborEnvironment.from_dir(folder) + + assert env.snapshot is None + assert env.image == 'python:3.12-slim' + + def test_parses_a_dockerfile_task_with_harbor_defaults() -> None: task = HarborTask.from_dir(BUILT) From bd0d6b0909b6f295f2da60fb3beabd75b14db9c6 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 18:42:55 +0530 Subject: [PATCH 17/19] fix(osworld): treat packages openpyxl or python-docx can't open as unreadable, and read booleans as openpyxl does --- .../src/deepgym/envs/computer_use/_checks.py | 78 ++++++++++++++---- deepgym/tests/test_import_osworld.py | 79 ++++++++++++++++++- 2 files changed, 140 insertions(+), 17 deletions(-) diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index ea34dd9b..0e370672 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -78,6 +78,21 @@ def report(progress: dict[str, bool], guards: dict[str, bool]) -> None: _W_NS = '{http://schemas.openxmlformats.org/wordprocessingml/2006/main}' _CT_NS = '{http://schemas.openxmlformats.org/package/2006/content-types}' _WML_MAIN = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document.main+xml' +_WORKBOOK_TYPES = frozenset( + { + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet.main+xml', + 'application/vnd.ms-excel.sheet.macroEnabled.main+xml', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.template.main+xml', + 'application/vnd.ms-excel.template.macroEnabled.main+xml', + } +) +"""The workbook content types openpyxl opens.""" +_SHARED_STRINGS_TYPE = ( + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml' +) +_OFFICE_DOCUMENT = ( + 'http://schemas.openxmlformats.org/officeDocument/2006/relationships/officeDocument' +) _COORD_RE = re.compile(r'([A-Z]{1,3})([1-9][0-9]{0,6})') _MAX_ROW, _MAX_COL = 1_048_576, 16_384 @@ -226,6 +241,26 @@ def _relationships(archive: zipfile.ZipFile, part: str) -> list[tuple[str, str, return found +def _check_related_parts(archive: zipfile.ZipFile) -> None: + """Refuse a package whose relationships name a part it doesn't hold. + + python-docx loads every part reachable from the package's relationships and fails + on a missing one, so such a document is unreadable here too. + + Raises: + ValueError: If a relationship's target is missing. + """ + names = set(archive.namelist()) + seen, queue = {''}, [''] + while queue: + for _, _, target in _relationships(archive, queue.pop()): + if target not in names: + raise ValueError(f'a relationship names {target}, which the package lacks') + if target not in seen: + seen.add(target) + queue.append(target) + + def _main_part(archive: zipfile.ZipFile) -> str: """The part the package's officeDocument relationship names, as python-docx and openpyxl resolve it, not whatever sits at the usual path. @@ -233,7 +268,7 @@ def _main_part(archive: zipfile.ZipFile) -> str: Raises: ValueError: If the package names no main part or more than one. """ - main = [t for _, kind, t in _relationships(archive, '') if kind.endswith('/officeDocument')] + main = [t for _, kind, t in _relationships(archive, '') if kind == _OFFICE_DOCUMENT] if len(main) != 1: raise ValueError(f'{len(main)} main parts') return main[0] @@ -289,7 +324,7 @@ def _cell_value(cell: ET.Element, shared: list[str]) -> object: if kind == 'str': return value.text if kind == 'b': - return value.text == '1' + return bool(int(value.text)) # as openpyxl reads it; not an integer is unreadable if kind == 'e': return value.text return _number(value.text) @@ -307,16 +342,16 @@ def _cell_value(cell: ET.Element, shared: list[str]) -> object: """openpyxl's STRIP_RE: quoted text and bracketed sections other than [h], [m], [s].""" -def _date_styles(archive: zipfile.ZipFile, part: str | None) -> dict[int, str]: - """{style index: 'date' or 'duration'} for the cell styles in styles *part* whose - number format shows a date, time or elapsed time. +def _date_styles(archive: zipfile.ZipFile, part: str | None) -> tuple[dict[int, str], int]: + """({style index: 'date' or 'duration'}, how many cell styles there are) for styles + *part*: the styles whose number format shows a date, time or elapsed time. openpyxl reads a number in such a cell as a datetime, a time or a timedelta, so pandas never equates it with a plain number or with another kind; :func:`_sheet_grid` marks those cells the same way. """ if part is None: - return {} + return {}, 1 # openpyxl's default stylesheet has one cell style root = _read_xml(archive, part) custom = { int(fmt.get('numFmtId', '-1')): fmt.get('formatCode', '') @@ -331,12 +366,13 @@ def _date_styles(archive: zipfile.ZipFile, part: str | None) -> dict[int, str]: if re.search(r'(? list[str]: @@ -368,20 +404,24 @@ def _trim(grid: list[list[object]]) -> list[list[object]]: def _sheet_grid( - root: ET.Element, shared: list[str], dated: dict[int, str], shift: int, budget: int + root: ET.Element, shared: list[str], styles: tuple[dict[int, str], int], shift: int, budget: int ) -> list[list[object]]: """Rows of cell values over the sheet's used range; missing cells are None. The used range ends at the last non-empty row and column, as after :func:`_trim`. - A number in a date- or duration-formatted cell (style index in *dated*) is marked + *styles* is :func:`_date_styles`' result; a cell naming a style that doesn't exist + makes the sheet unreadable, as it does for openpyxl. A number in a date- or + duration-formatted cell is marked the way openpyxl decodes it: ``{'duration': serial}``, ``{'time': serial}`` (a date format below 1 day) or ``{'date': serial + shift}`` (*shift* puts a 1904-based serial on the 1900 calendar), each with the raw ``'serial'`` that check_cell reads. A marked cell only equals one of the same kind and value. Raises: - ValueError: If the used range holds more than *budget* cells. + ValueError: If the used range holds more than *budget* cells, or a cell names a + style that doesn't exist. """ + dated, style_count = styles cells: dict[tuple[int, int], object] = {} max_row = 0 max_col = 0 @@ -390,13 +430,16 @@ def _sheet_grid( return [] for row in data.findall(f'{_XLSX_NS}row'): for cell in row.findall(f'{_XLSX_NS}c'): + style = int(cell.get('s', '0')) + if not 0 <= style < style_count: + raise ValueError(f'a cell names style {style}; there are {style_count}') ref = cell.get('r') if not ref: continue value = _cell_value(cell, shared) if value is None: continue - kind = dated.get(int(cell.get('s', '0'))) if _is_number(value) else None + kind = dated.get(style) if _is_number(value) else None if kind == 'duration': value = {'duration': value, 'serial': value} elif kind and 0 <= value < 1: @@ -444,8 +487,8 @@ def _check_declarations(archive: zipfile.ZipFile, book: str, parts: dict[str, st ValueError: If the declarations disagree. """ declared, _ = _content_types(archive) - books = {name for name, kind in declared.items() if kind.endswith('.main+xml')} - strings = {name for name, kind in declared.items() if kind.endswith('.sharedStrings+xml')} + books = {name for name, kind in declared.items() if kind in _WORKBOOK_TYPES} + strings = {name for name, kind in declared.items() if kind == _SHARED_STRINGS_TYPE} if books != {book.lower()}: raise ValueError(f'content types declare {sorted(books)}, relationships {book}') if strings != ({parts['sharedStrings'].lower()} if 'sharedStrings' in parts else set()): @@ -486,7 +529,7 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: ] # openpyxl reads styles from xl/styles.xml whatever the relationships say. styles = 'xl/styles.xml' if 'xl/styles.xml' in archive.namelist() else None - dated = _date_styles(archive, styles) + cell_styles = _date_styles(archive, styles) properties = book.find(f'{_XLSX_NS}workbookPr') epoch = '' if properties is None else properties.get('date1904', '') shift = 1462 if epoch.lower() in ('1', 'true') else 0 @@ -498,7 +541,7 @@ def read_sheets(path: str) -> list[tuple[str, list[list[object]], list[str]]]: name, part = sheet.get('name', ''), targets[sheet.get(f'{_REL_NS}id', '')] if part not in parsed: # sheets naming one part parse it once root = _read_xml(archive, part) - grid = _sheet_grid(root, shared, dated, shift, budget) + grid = _sheet_grid(root, shared, cell_styles, shift, budget) parsed[part] = (grid, _merge_ranges(root)) grid, merges = parsed[part] budget -= len(grid) * max((len(row) for row in grid), default=0) @@ -553,7 +596,10 @@ def read_docx_text(path: str) -> str: kind = declared.get(main.lower()) or defaults.get(main.rpartition('.')[2].lower()) if kind != _WML_MAIN: raise ValueError(f'{main} has content type {kind!r}, not a Word document') + _check_related_parts(archive) root = _read_xml(archive, main) + if root.tag != f'{_W_NS}document': + raise ValueError(f'{main} is not a w:document') body = root.find(f'{_W_NS}body') paragraphs = [] for para in [] if body is None else body: diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 344f26dd..90748c5d 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -506,6 +506,47 @@ def test_a_number_in_a_format_openpyxl_reads_as_a_date_is_not_that_number( assert _run_verifier(env, home, work)['score'] == 0.0 +def test_workbooks_openpyxl_cannot_open_are_unreadable(tmp_path: Path, work: Path) -> None: + """A style index past the stylesheet, or a workbook content type openpyxl doesn't + know, makes openpyxl fail; here it makes the file a 0, not a pass.""" + start, done = {'A1': 'Total'}, {'A1': 'Total', 'A2': 123} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + good = make_xlsx(tmp_path / 'good.xlsx', [('Sheet1', done, [])]).read_bytes() + with zipfile.ZipFile(io.BytesIO(good)) as archive: + sheet = archive.read('xl/worksheets/sheet1.xml').decode() + types = archive.read('[Content_Types].xml').decode() + for label, parts in ( + ( + 'a missing style', + {'xl/worksheets/sheet1.xml': sheet.replace('', '')}, + ), + ( + 'an unknown workbook type', + {'[Content_Types].xml': types.replace('spreadsheetml.sheet.main', 'nonsense.main')}, + ), + ): + _repacked(home / 'Desktop' / FILE, good, parts) + assert _run_verifier(env, home, work)['score'] == 0.0, label + + +def test_a_boolean_is_read_as_openpyxl_reads_it(tmp_path: Path, work: Path) -> None: + """openpyxl reads 2 in a boolean cell as True, so it doesn't match False.""" + start, done = {'A1': 'Flag'}, {'A1': 'Flag', 'A2': False} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + (home / 'Desktop').mkdir(parents=True) + good = make_xlsx(tmp_path / 'good.xlsx', [('Sheet1', done, [])]).read_bytes() + with zipfile.ZipFile(io.BytesIO(good)) as archive: + sheet = archive.read('xl/worksheets/sheet1.xml').decode() + two = sheet.replace('0', '2') + assert two != sheet + _repacked(home / 'Desktop' / FILE, good, {'xl/worksheets/sheet1.xml': two}) + + assert _run_verifier(env, home, work)['score'] == 0.0 + + def test_a_boolean_does_not_match_a_number(tmp_path: Path, work: Path) -> None: start, done = {'A1': 'Flag'}, {'A1': 'Flag', 'A2': 1} env = _sheet_data_env(tmp_path, start, done) @@ -796,7 +837,18 @@ def test_unsaved_changes_and_a_wrong_sheet_name_are_judged_zeros( @pytest.mark.parametrize( - 'label', ['missing', 'not a zip', 'a FIFO', 'no package', 'two mains', 'no manifest'] + 'label', + [ + 'missing', + 'not a zip', + 'a FIFO', + 'no package', + 'two mains', + 'no manifest', + 'a missing related part', + 'an inexact main relationship', + 'not a w:document', + ], ) def test_a_saved_document_the_agent_broke_is_a_judged_zero( label: str, tmp_path: Path, work: Path @@ -810,6 +862,31 @@ def test_a_saved_document_the_agent_broke_is_a_judged_zero( rels = gold.read('_rels/.rels').decode() if label == 'not a zip': saved.write_bytes(b'not a document') + elif label in ('a missing related part', 'an inexact main relationship', 'not a w:document'): + with zipfile.ZipFile(make_docx(tmp_path / 'right.docx', _DOC_GOLD)) as right: + parts = {name: right.read(name) for name in right.namelist()} + if label == 'a missing related part': # python-docx loads every related part + parts['word/_rels/document.xml.rels'] = ( + b'' + b'' + ) + elif label == 'an inexact main relationship': + parts['_rels/.rels'] = parts['_rels/.rels'].replace( + b'http://schemas.openxmlformats.org/officeDocument/2006/relationships/' + b'officeDocument', + b'https://invalid/officeDocument', + ) + else: + parts['word/document.xml'] = ( + parts['word/document.xml'] + .replace(b'', b'') + ) + with zipfile.ZipFile(saved, 'w') as archive: + for name, data in parts.items(): + archive.writestr(name, data) elif label == 'no manifest': # python-docx can't open a package without one with zipfile.ZipFile(make_docx(tmp_path / 'right.docx', _DOC_GOLD)) as right: with zipfile.ZipFile(saved, 'w') as archive: From c7953abcfdbfe69773634dcaa7c162432e04f770 Mon Sep 17 00:00:00 2001 From: Abhishek Gahlot Date: Wed, 14 Oct 2026 18:55:19 +0530 Subject: [PATCH 18/19] fix(osworld): type numbers and columns as openpyxl and pandas do, and parse a document's related parts --- .../src/deepgym/envs/computer_use/_checks.py | 57 ++++++++++++++----- deepgym/tests/test_import_osworld.py | 24 +++++++- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/deepgym/src/deepgym/envs/computer_use/_checks.py b/deepgym/src/deepgym/envs/computer_use/_checks.py index 0e370672..55831d72 100644 --- a/deepgym/src/deepgym/envs/computer_use/_checks.py +++ b/deepgym/src/deepgym/envs/computer_use/_checks.py @@ -244,11 +244,13 @@ def _relationships(archive: zipfile.ZipFile, part: str) -> list[tuple[str, str, def _check_related_parts(archive: zipfile.ZipFile) -> None: """Refuse a package whose relationships name a part it doesn't hold. - python-docx loads every part reachable from the package's relationships and fails - on a missing one, so such a document is unreadable here too. + python-docx loads every part reachable from the package's relationships and parses + its XML parts, failing on a missing or malformed one, so such a document is + unreadable here too. Raises: ValueError: If a relationship's target is missing. + ET.ParseError: If a related XML part doesn't parse. """ names = set(archive.namelist()) seen, queue = {''}, [''] @@ -257,6 +259,8 @@ def _check_related_parts(archive: zipfile.ZipFile) -> None: if target not in names: raise ValueError(f'a relationship names {target}, which the package lacks') if target not in seen: + if target.lower().endswith('.xml'): # python-docx parses its XML parts + _read_xml(archive, target) seen.add(target) queue.append(target) @@ -292,17 +296,20 @@ def _coord_to_cell(coord: str) -> tuple[int, int]: return row, col -def _number(text: str) -> int | float | None: - """A numeric cell's value: float when fractional, int when integral (pandas-like).""" - try: +def _number(text: str) -> int | float: + """A numeric cell's value as openpyxl casts it: a float if the text has a '.' or an + exponent, else an int. pandas types the column from these, so the type matters. + + Raises: + ValueError: If the text isn't such a number (openpyxl fails on it too) or the + float isn't finite. + """ + if '.' in text or 'e' in text.lower(): value = float(text) - except ValueError: - return None - if not math.isfinite(value): - raise ValueError(f'{text!r} is not a finite number') - if value.is_integer() and abs(value) < 1e15: - return int(value) - return value + if not math.isfinite(value): + raise ValueError(f'{text!r} is not a finite number') + return value + return int(text) def _cell_value(cell: ET.Element, shared: list[str]) -> object: @@ -687,20 +694,42 @@ def _round_grid(grid: list[list[object]], precision: int) -> list[list[object]]: return rounded +def _numeric_dtypes(grid: list[list[object]]) -> list[str | None]: + """The dtype pandas gives each all-number data column: int64 when every cell holds an + int, float64 once one holds a float or is empty; None for any other column. + + Other dtype differences need values _same_value already tells apart. + """ + width = max((len(row) for row in grid), default=0) + dtypes: list[str | None] = [] + for column in range(width): + values = [row[column] if column < len(row) else None for row in grid[1:]] + present = [value for value in values if value is not None] + if not present or not all(_is_number(value) for value in present): + dtypes.append(None) + elif len(present) == len(values) and all(isinstance(v, int) for v in present): + dtypes.append('int64') + else: + dtypes.append('float64') + return dtypes + + def sheets_match( agent: list[list[object]], expected: list[list[object]], precision: int = 4 ) -> bool: """Whether two grids are the same sheet, as compare_table's sheet_data sees it. Mirrors `pd.read_excel(a).round(precision).equals(pd.read_excel(b).round(precision))`: - each side is rounded by its own column dtypes first, then shapes and every value - must match. + each side is rounded by its own column dtypes first, then shapes, numeric column + dtypes (int64 against float64 is unequal, as in pandas) and every value must match. """ if len(agent) != len(expected): return False for row_a, row_e in zip(agent, expected): if len(row_a) != len(row_e): return False + if _numeric_dtypes(agent) != _numeric_dtypes(expected): + return False agent = _round_grid(agent, precision) expected = _round_grid(expected, precision) for row_a, row_e in zip(agent, expected): diff --git a/deepgym/tests/test_import_osworld.py b/deepgym/tests/test_import_osworld.py index 90748c5d..1e333b1a 100644 --- a/deepgym/tests/test_import_osworld.py +++ b/deepgym/tests/test_import_osworld.py @@ -547,6 +547,17 @@ def test_a_boolean_is_read_as_openpyxl_reads_it(tmp_path: Path, work: Path) -> N assert _run_verifier(env, home, work)['score'] == 0.0 +def test_an_int_column_does_not_match_a_float_column(tmp_path: Path, work: Path) -> None: + """pandas' equals compares dtypes: int64 1 against float64 1.00001 is unequal, even + though both round to 1 at the rule's precision.""" + start, done = {'A1': 'Amount'}, {'A1': 'Amount', 'A2': 1} + env = _sheet_data_env(tmp_path, start, done) + home = tmp_path / 'home' + for label, cells, score in (('a float', {**start, 'A2': 1.00001}, 0.0), ('the int', done, 1.0)): + _save(tmp_path, home, cells, []) + assert _run_verifier(env, home, work)['score'] == score, label + + def test_a_boolean_does_not_match_a_number(tmp_path: Path, work: Path) -> None: start, done = {'A1': 'Flag'}, {'A1': 'Flag', 'A2': 1} env = _sheet_data_env(tmp_path, start, done) @@ -846,6 +857,7 @@ def test_unsaved_changes_and_a_wrong_sheet_name_are_judged_zeros( 'two mains', 'no manifest', 'a missing related part', + 'a malformed related part', 'an inexact main relationship', 'not a w:document', ], @@ -862,10 +874,18 @@ def test_a_saved_document_the_agent_broke_is_a_judged_zero( rels = gold.read('_rels/.rels').decode() if label == 'not a zip': saved.write_bytes(b'not a document') - elif label in ('a missing related part', 'an inexact main relationship', 'not a w:document'): + elif label in ( + 'a missing related part', + 'a malformed related part', + 'an inexact main relationship', + 'not a w:document', + ): with zipfile.ZipFile(make_docx(tmp_path / 'right.docx', _DOC_GOLD)) as right: parts = {name: right.read(name) for name in right.namelist()} - if label == 'a missing related part': # python-docx loads every related part + if label in ('a missing related part', 'a malformed related part'): + # python-docx loads and parses every related part + if label == 'a malformed related part': + parts['word/styles.xml'] = b'', '999' + ) + _repacked(home / 'Desktop' / FILE, good, {'xl/worksheets/sheet1.xml': extra}) + + result = _run_verifier(env, home, work) + + assert result['score'] == 0.0 + assert 'workbook_saved' in result['details'] + + def test_a_boolean_does_not_match_a_number(tmp_path: Path, work: Path) -> None: start, done = {'A1': 'Flag'}, {'A1': 'Flag', 'A2': 1} env = _sheet_data_env(tmp_path, start, done)