From d4d6297e0a9dcd58611272e454bb543b0953673f Mon Sep 17 00:00:00 2001 From: labadmin Date: Mon, 2 Nov 2026 22:37:32 +0000 Subject: [PATCH 01/10] Initialize agent project facade --- .agent/project.yaml | 166 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 .agent/project.yaml diff --git a/.agent/project.yaml b/.agent/project.yaml new file mode 100644 index 0000000..851eeb7 --- /dev/null +++ b/.agent/project.yaml @@ -0,0 +1,166 @@ +schema_version: 2 +registry: + required_profiles: + - project/script_toolkit@1 + - language/python@1 + - runtime/command_line@1 + - framework/python_scripts@1 + - interface/cli@1 + - environment/python_environment@1 + - tool/python_pytest@1 + - acceptance/cli_command@1 + - acceptance/package_import@1 +project: + kind: script_toolkit + name: product-54354138-1 +contract: + required_paths: + - tools + - tests + - tests/test_example.py + - tools/example.py + - examples/input.json + support_paths: + - .env.example + - pyproject.toml + - uv.lock + command_paths: + - tools + - tests + - tests/test_example.py + - tools/example.py + - examples/input.json + protected_paths: + exact: + - .agent/project.yaml + - .agent/acceptance.yaml + prefixes: + - .gitea/workflows/ +configuration: + files: + - id: sample-input + path: examples/input.json + kind: input_example + required: true + - id: example-env + path: .env.example + kind: env_example + required: true + environment: + required: [] + optional: + - LOG_LEVEL +components: +- id: tools + kind: toolkit + languages: + - python + runtime: command_line + interfaces: + - cli + framework_profiles: + - python_scripts + environment: + profile: python_environment + dependency_files: + - pyproject.toml + required_commands: + - python + path_prepend: + - .venv/bin + bootstrap: + command: python -m venv .venv && .venv/bin/python -m pip install -e '.[dev]' + owner: facade + repair_role: provisioner + required_paths: + - pyproject.toml + ephemeral_paths: + - .venv + dependencies: + model: language/python@1:uv_project + working_directory: . + manager: uv + manifests: + - pyproject.toml + resolution_files: + - uv.lock + reproducibility: lockfile + operations: + update: + status: supported + phases: + - id: resolve + command: uv lock + failure_disposition: continue_session + - id: materialize + command: uv sync --locked --no-default-groups --extra dev + failure_disposition: recreate_session + restore: + status: supported + phases: + - id: materialize + command: uv sync --locked --no-default-groups --extra dev + failure_disposition: recreate_session + requirements: + - id: python_pytest + profile: tool/python_pytest@1 + provision_source: project_dependency + dependency_role: test + package: + ecosystem: python + name: pytest + commands: + validate: + verification_roles: + - artifact_validation + - quality_gate + command: python -m compileall tools tests + required_paths: + - pyproject.toml + - tools + - tests + test: + verification_roles: + - automated_test + - quality_gate + execution_profile: tool/python_pytest@1 + command: pytest + required_paths: + - pyproject.toml + - tests/test_example.py + smoke: + verification_roles: + - behavior_smoke + - integration_validation + command: python -m tools.example examples/input.json + config: sample-input + required_paths: + - pyproject.toml + - tools/example.py + - examples/input.json +acceptance: +- id: import-package + verification_roles: + - artifact_validation + kind: package_import + component: tools + package: tools +- id: help-command + verification_roles: + - interface_acceptance + kind: cli_command + component: tools + config: sample-input + command: python -m tools.example --help + expect_exit_code: 0 + required_paths: + - tools/example.py +ci: + required: true + required_checks: + - repository_validation + required_commands: + - validate + - test +facade_placement: + kind: repository_root -- 2.54.0 From 6d79fef8c063fc5cac7b5f067580fee5b01da989 Mon Sep 17 00:00:00 2001 From: labadmin Date: Mon, 2 Nov 2026 22:37:32 +0000 Subject: [PATCH 02/10] Initialize dependency declaration --- pyproject.toml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 pyproject.toml diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..13a7c65 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,8 @@ +# Initial declaration; add project dependencies and development tools here. +[project] +name = "product-54354138-1-tools" +version = "0.1.0" +dependencies = [] + +[project.optional-dependencies] +dev = ["pytest"] -- 2.54.0 From ab7ab98cbfc259bb24ff7b9accccea9696e5c3fc Mon Sep 17 00:00:00 2001 From: labadmin Date: Mon, 2 Nov 2026 22:37:33 +0000 Subject: [PATCH 03/10] Materialize agent project CI --- .gitea/workflows/ci.yaml | 329 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 329 insertions(+) create mode 100644 .gitea/workflows/ci.yaml diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml new file mode 100644 index 0000000..97b8bbb --- /dev/null +++ b/.gitea/workflows/ci.yaml @@ -0,0 +1,329 @@ +name: Repository Validate + +on: + push: + branches: + - master + - main + pull_request: + branches: + - master + - main + - task/** + +jobs: + repository-checks: + runs-on: agent-environment + steps: + - name: Checkout + shell: bash + env: + CHECKOUT_SERVER_URL: ${{ github.server_url }} + CHECKOUT_REPOSITORY: ${{ github.repository }} + CHECKOUT_SHA: ${{ github.sha }} + CHECKOUT_TOKEN: ${{ github.token }} + run: | + python3 - <<'CHECKOUT_PY' + """Self-contained checkout embedded in generated CI; no downloaded actions.""" + from __future__ import annotations + + import base64 + import os + from pathlib import Path + import re + import subprocess + from urllib.parse import urlsplit + + + def checkout(environment=None): + env = dict(os.environ if environment is None else environment) + server = env.pop('CHECKOUT_SERVER_URL', '').rstrip('/') + repo = env.pop('CHECKOUT_REPOSITORY', '') + sha = env.pop('CHECKOUT_SHA', '') + token = env.pop('CHECKOUT_TOKEN', '') + parsed = urlsplit(server) + if (parsed.scheme not in ('https', 'http') or not parsed.hostname or parsed.username + or parsed.password or parsed.query or parsed.fragment): + raise ValueError('invalid checkout server URL') + if not re.fullmatch(r'[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+', repo) or any(p in ('.', '..') for p in repo.split('/')): + raise ValueError('invalid checkout repository') + if not re.fullmatch(r'[0-9a-fA-F]{40}|[0-9a-fA-F]{64}', sha): + raise ValueError('checkout requires an exact commit SHA') + workspace = Path(env.get('GITHUB_WORKSPACE') or env.get('GITEA_WORKSPACE') or os.getcwd()).resolve() + workspace.mkdir(parents=True, exist_ok=True) + # Never inherit authentication overrides, tracing, hooks or checkout filters. + env = {k: v for k, v in env.items() if not k.startswith('GIT_')} + env.update(GIT_CONFIG_NOSYSTEM='1', GIT_CONFIG_GLOBAL='/dev/null', + GIT_TERMINAL_PROMPT='0', GIT_LFS_SKIP_SMUDGE='1') + if (workspace / '.git').exists(): + raise ValueError('checkout requires a fresh workspace') + + def git(*args, auth=False): + child = dict(env) + if auth and token: + credential = base64.b64encode(('x-access-token:' + token).encode()).decode() + child.update(GIT_CONFIG_COUNT='1', + GIT_CONFIG_KEY_0='http.extraHeader', + GIT_CONFIG_VALUE_0='Authorization: Basic ' + credential) + result = subprocess.run(['git', '-c', 'core.hooksPath=/dev/null', '-C', str(workspace), *args], + env=child, capture_output=True, text=True, timeout=180) + if result.returncode: + # Transport output may contain credentials or upstream-controlled data. + raise RuntimeError('checkout git operation failed: ' + args[0]) + return result.stdout.strip() + + git('init') + git('remote', 'add', 'origin', server + '/' + repo + '.git') + git('-c', 'http.followRedirects=false', 'fetch', '--no-tags', '--depth=1', 'origin', sha, auth=True) + actual = git('rev-parse', 'FETCH_HEAD^{commit}') + if actual.lower() != sha.lower(): + raise ValueError('fetched commit does not match requested SHA') + git('checkout', '--detach', '--force', actual) + if git('rev-parse', 'HEAD').lower() != sha.lower(): + raise ValueError('checked out commit does not match requested SHA') + print('Checked out requested commit ' + actual) + + + if __name__ == '__main__': + try: + checkout() + except (ValueError, RuntimeError, subprocess.TimeoutExpired) as exc: + print('AGENT_CI_RESULT command_id=checkout stage=checkout owner=infrastructure ' + 'repair_role=operator exit_code=1 expected_exit_code=0 status=failed') + # Do not stringify subprocess exceptions: they may include command output. + raise SystemExit('Checkout failed (' + type(exc).__name__ + ')') from None + CHECKOUT_PY + + - name: Validate facade structure + shell: bash + run: | + python3 - <<'PY' + from pathlib import Path + + path = Path(".agent/project.yaml") + if not path.is_file(): + raise SystemExit(".agent/project.yaml is missing") + text = path.read_text(encoding="utf-8") + for key in ["schema_version", "registry", "project", "components"]: + if not any(line.startswith(key + ":") for line in text.splitlines()): + raise SystemExit(f".agent/project.yaml missing required key: {key}") + if not any(line.lstrip().startswith("- ") for line in text.splitlines()): + raise SystemExit(".agent/project.yaml must declare at least one component") + print("facade structure ok") + PY + + - name: Run facade commands + shell: bash + run: | + set -euo pipefail + full_run=true + if [ "${GITHUB_EVENT_NAME:-}" = "pull_request" ] && [[ "${GITHUB_BASE_REF:-}" == task/* ]]; then + full_run=false + fi + export PATH="$PWD/.venv/bin:$PATH" + printf 'AGENT_CI_MODE full_run=%s event=%s base_ref=%s\n' "$full_run" "${GITHUB_EVENT_NAME:-unknown}" "${GITHUB_BASE_REF:-}" + run_ci_stage() { + command_id="$1" + stage="$2" + owner="$3" + repair_role="$4" + command="$5" + expected_exit_code="$6" + missing_policy="$7" + shift 7 + missing_paths=() + for required_path in "$@"; do + if [ ! -e "$required_path" ]; then + missing_paths+=("$required_path") + fi + done + if [ "${#missing_paths[@]}" -gt 0 ]; then + missing_csv="$(IFS=,; echo "${missing_paths[*]}")" + if [ "$full_run" != true ] && [ "$missing_policy" = skip_partial ]; then + printf 'AGENT_CI_RESULT command_id=%s stage=%s owner=%s repair_role=%s exit_code=0 expected_exit_code=%s status=skipped_missing_prerequisites missing_paths=%s\n' "$command_id" "$stage" "$owner" "$repair_role" "$expected_exit_code" "$missing_csv" + return 0 + fi + printf 'AGENT_CI_RESULT command_id=%s stage=%s owner=%s repair_role=%s exit_code=97 expected_exit_code=%s status=missing_prerequisites missing_paths=%s\n' "$command_id" "$stage" "$owner" "$repair_role" "$expected_exit_code" "$missing_csv" + return 97 + fi + if [ -z "$command" ]; then + echo "skip $command_id (not declared)" + return 0 + fi + echo "::group::$command_id" + set +e + bash -o pipefail -eu -c "$command" + exit_code="$?" + set -e + # Executor reserves 78 for tracked preparation-output conflicts. + # Other dependency failures retain infrastructure ownership. + if [ "$stage" = dependency_preparation ] && [ "$exit_code" -eq 78 ]; then + owner=project + repair_role=developer + fi + status=failed + if [ "$exit_code" -eq "$expected_exit_code" ]; then + status=passed + fi + if [ "$status" = failed ]; then + python3 - "$command_id" "$exit_code" <<'CI_DIAGNOSTICS_PY' || echo 'AGENT_CI_DIAGNOSTICS_ARTIFACT collector_failed' + """Bounded, opt-in CI failure reports; embedded in the canonical workflow. + + Gitea Actions artifact v3 protocol (no external Action or package dependency). + """ + import base64 + import hashlib + import json + import os + from pathlib import PurePosixPath + import re + import stat + import sys + import urllib.error + import urllib.parse + import urllib.request + + FILE_LIMIT = 65536 + FILE_COUNT = 8 + MANIFEST = '.ci/diagnostics.json' + + + def read_relative(root, name, limit=FILE_LIMIT): + path = PurePosixPath(name) + if not name or path.is_absolute() or any(p in ('', '.', '..') for p in name.split('/')): + raise ValueError('invalid path') + fd = os.open(root, os.O_RDONLY | os.O_DIRECTORY) + try: + for part in path.parts[:-1]: + child = os.open(part, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW, dir_fd=fd) + os.close(fd) + fd = child + child = os.open(path.name, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK, dir_fd=fd) + with os.fdopen(child, 'rb') as stream: + if not stat.S_ISREG(os.fstat(stream.fileno()).st_mode): + raise ValueError('not a regular file') + return stream.read(limit + 1) + finally: + os.close(fd) + + + def scrub(text, env): + for key, value in env.items(): + if len(value) >= 8 and any(word in key.upper() for word in ('TOKEN', 'SECRET', 'PASSWORD', 'API_KEY')): + text = text.replace(value, '') + return text + + + def collect(root, command, exit_code, env): + report = {'version': 1, 'command_id': command, 'exit_code': exit_code, + 'sha': env.get('GITHUB_SHA', ''), 'files': []} + try: + raw = read_relative(root, MANIFEST) + if len(raw) > FILE_LIMIT: + raise ValueError('manifest too large') + spec = json.loads(raw) + if spec.get('version') != 1 or not isinstance(spec.get('commands'), dict): + raise ValueError('invalid manifest') + names = spec['commands'].get(command, []) + if not isinstance(names, list) or len(names) > FILE_COUNT or any(not isinstance(n, str) for n in names): + raise ValueError('invalid file list') + report['manifest_status'] = 'loaded' + except FileNotFoundError: + report['manifest_status'] = 'missing' + names = [] + except (OSError, ValueError, TypeError, AttributeError): + report['manifest_status'] = 'invalid' + names = [] + for name in names: + item = {'path': name} + try: + # No directory traversal, symlinks, devices, hidden credential files, + # wildcards or recursive workspace scans. + if any(p.startswith('.') for p in PurePosixPath(name).parts) or len(name) > 512: + raise ValueError('hidden or invalid path') + raw = read_relative(root, name) + item.update(status='captured', truncated=len(raw) > FILE_LIMIT, + content=scrub(raw[:FILE_LIMIT].decode('utf-8', errors='replace'), env)) + except (OSError, ValueError): + item['status'] = 'unavailable' + report['files'].append(item) + return report + + + class NoRedirect(urllib.request.HTTPRedirectHandler): + def redirect_request(self, *args, **kwargs): + return None + + + def upload(report, env, request=None): + base = env.get('ACTIONS_RUNTIME_URL', '') + token = env.get('ACTIONS_RUNTIME_TOKEN', '') + run = env.get('GITHUB_RUN_ID', '') + parsed = urllib.parse.urlsplit(base) + if not base or not token or not run: + return 'runtime_unavailable' + if parsed.scheme not in ('http', 'https') or not parsed.netloc or parsed.username or parsed.password or parsed.query or parsed.fragment: + raise ValueError('invalid runtime URL') + if request is None: + opener = urllib.request.build_opener(NoRedirect) + def request(method, url, body, headers): + req = urllib.request.Request(url, data=body, method=method, headers=headers) + try: + with opener.open(req, timeout=10) as response: + return response.read(65536) + except urllib.error.HTTPError as exc: + raise RuntimeError(f'artifact {method} HTTP {exc.code}') from None + endpoint = base.rstrip('/')+'/_apis/pipelines/workflows/'+urllib.parse.quote(run, safe='')+'/artifacts' + name = 'ci-failure-'+re.sub('[^A-Za-z0-9_-]', '-', report['command_id'])[:60]+'-'+re.sub('[^0-9]', '', env.get('GITHUB_RUN_ATTEMPT', '1'))[:8] + headers = {'Authorization': 'Bearer '+token, 'Content-Type': 'application/json'} + created = request('POST', endpoint+'?api-version=6.0-preview', json.dumps({'Type': 'actions_storage', 'Name': name}).encode(), headers) + location = json.loads(created)['fileContainerResourceUrl'] + location = urllib.parse.urljoin(endpoint, location) + target = urllib.parse.urlsplit(location) + if (target.scheme, target.netloc) != (parsed.scheme, parsed.netloc) or target.username or target.password: + raise ValueError('cross-origin artifact URL') + data = json.dumps(report, ensure_ascii=True).encode() + url = location+('&' if target.query else '?')+urllib.parse.urlencode({'itemPath': name+'/diagnostics.json'}) + request('PUT', url, data, {**headers, 'Content-Type': 'application/octet-stream', + 'Content-Range': f'bytes 0-{len(data)-1}/{len(data)}', 'x-tfs-filelength': str(len(data)), + 'x-actions-results-md5': base64.b64encode(hashlib.md5(data).digest()).decode('ascii')}) + request('PATCH', endpoint+'?'+urllib.parse.urlencode({'artifactName': name, 'api-version': '6.0-preview'}), + json.dumps({'Size': len(data)}).encode(), headers) + return 'uploaded:'+name + + + def main(): + report = collect(os.getcwd(), sys.argv[1], int(sys.argv[2]), os.environ) + # A bounded preview remains in the job log even if artifact upload fails. + preview = json.loads(json.dumps(report)) + for item in preview['files']: + if 'content' in item: + item['content'] = item['content'][:1000] + print('AGENT_CI_DIAGNOSTICS '+json.dumps(preview, ensure_ascii=True)) + try: + result = upload(report, os.environ) + except Exception as exc: + result = 'upload_failed:'+str(exc) if isinstance(exc, RuntimeError) else 'upload_failed:'+type(exc).__name__ + print('AGENT_CI_DIAGNOSTICS_ARTIFACT '+result) + + + if __name__ == '__main__': + main() + CI_DIAGNOSTICS_PY + fi + printf 'AGENT_CI_RESULT command_id=%s stage=%s owner=%s repair_role=%s exit_code=%s expected_exit_code=%s status=%s\n' "$command_id" "$stage" "$owner" "$repair_role" "$exit_code" "$expected_exit_code" "$status" + echo "::endgroup::" + if [ "$status" = passed ]; then + return 0 + fi + if [ "$exit_code" -eq 0 ]; then + return 1 + fi + return "$exit_code" + } + run_ci_stage 'tools.dependencies.resolution' 'dependency_prerequisite' 'project' 'developer' '' '0' 'fail' 'uv.lock' + run_ci_stage 'tools.dependencies.restore' 'dependency_preparation' 'infrastructure' 'infrastructure' '/opt/agent-environment/bin/python -I -m agent_environment_executor.cli --workspace "$PWD" --component '"'"'tools'"'"' --mode restore' '0' 'fail' + run_ci_stage 'tools.validate' 'command' 'project' 'developer' 'python -m compileall tools tests' '0' 'skip_partial' 'pyproject.toml' 'tools' 'tests' + run_ci_stage 'tools.test' 'command' 'project' 'developer' 'pytest' '0' 'skip_partial' 'pyproject.toml' 'tests/test_example.py' + run_ci_stage 'tools.smoke' 'command' 'project' 'developer' 'python -m tools.example examples/input.json' '0' 'skip_partial' 'pyproject.toml' 'tools/example.py' 'examples/input.json' + run_ci_stage 'acceptance.help-command' 'acceptance' 'project' 'developer' 'python -m tools.example --help' '0' 'skip_partial' 'examples/input.json' 'tools/example.py' -- 2.54.0 From d0b7f8f5aa2df831b5bfce585314f0edb3b5608d Mon Sep 17 00:00:00 2001 From: Tool Backend Date: Mon, 2 Nov 2026 23:23:26 +0000 Subject: [PATCH 04/10] change: Implement 1: Bootstrap the toolkit tree, project config and initial dependency lock Bootstrapped the repository-root toolkit tree required by the facade. - `tools/__init__.py`: minimal importable package exposing `__version__` only. - `tools/example.py`: offline CLI (`python -m tools.example REQUEST`) that loads and validates a JSON request document with a validate-only `dry_run` semantics, prints exactly one structured JSON envelope (sorted keys) and fixes the status/exit vocabulary now (0 ok, 1 invalid request document, 2 usage error). `--help` supported. - `examples/input.json`: canonical offline validate-only request document. - `tests/test_example.py`: focused tests for the smoke command, envelope shape, request-document validation, `--help`, and the module entrypoint (13 tests). - `pyproject.toml`: project metadata, setuptools build backend with `packages = ["tools"]`, `dev` extra with `pytest>=8.0`, pytest options `pythonpath=["."]` / `testpaths=["tests"]` so the repository root is importable. - `uv.lock`: created through the supported dependency update operation for the `tools` component; pinned only from the approved dependency mirror (no public-index fallbacks, no credentials). - `.gitignore` / `.env.example`: keep build artifacts out of the tree and document that no environment variables are required. Note: `requires-python` was raised to `>=3.11` because the approved mirror only offers `exceptiongroup<1.0.0rc8`, which cannot satisfy pytest on Python <3.11; the prepared environment (CPython 3.12.3) and CI bootstrap interpreter satisfy this. No protected facade paths were modified. Tests: Ran the facade commands with the prepared environment (`.venv/bin/python`): - `python -m compileall -q tools tests` -> OK (tools.validate) - `python -m pytest -q` -> 13 passed (tools.test) - `python -m tools.example examples/input.json` -> single JSON envelope, exit 0 (tools.smoke) - `python -m tools.example --help` -> usage text, exit 0 (help-command) Dependency lock/materialization performed via prepare_environment(component='tools', mode='update'): resolved + materialized, `uv.lock` written. Target branch `origin/task/1-task` is already an ancestor of HEAD, so the branch contains the current target. Progress: Created tools package (tools/__init__.py, tools/example.py), examples/input.json, tests/test_example.py, .gitignore, .env.example; updated pyproject.toml (metadata, setuptools packages, dev pytest extra, pytest pythonpath/testpaths, requires-python >=3.11 due to approved-mirror exceptiongroup constraint) and generated uv.lock via prepare_environment update. Facade validate/test/smoke and --help all pass locally. Published by developer-agent through Tool Backend. Closes #3 --- .agent-work/ledger.json | 9 ++ .env.example | 3 + .gitignore | 7 ++ examples/input.json | 9 ++ pyproject.toml | 19 +++- tests/test_example.py | 118 ++++++++++++++++++++++++ tools/__init__.py | 12 +++ tools/example.py | 200 ++++++++++++++++++++++++++++++++++++++++ uv.lock | 63 +++++++++++++ 9 files changed, 438 insertions(+), 2 deletions(-) create mode 100644 .agent-work/ledger.json create mode 100644 .env.example create mode 100644 .gitignore create mode 100644 examples/input.json create mode 100644 tests/test_example.py create mode 100644 tools/__init__.py create mode 100644 tools/example.py create mode 100644 uv.lock diff --git a/.agent-work/ledger.json b/.agent-work/ledger.json new file mode 100644 index 0000000..84d6e0c --- /dev/null +++ b/.agent-work/ledger.json @@ -0,0 +1,9 @@ +{ + "tasks": { + "projects/product-54354138-1#3": { + "events": [ + "Created tools package (tools/__init__.py, tools/example.py), examples/input.json, tests/test_example.py, .gitignore, .env.example; updated pyproject.toml (metadata, setuptools packages, dev pytest extra, pytest pythonpath/testpaths, requires-python >=3.11 due to approved-mirror exceptiongroup constraint) and generated uv.lock via prepare_environment update. Facade validate/test/smoke and --help all pass locally." + ] + } + } +} diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..a827b65 --- /dev/null +++ b/.env.example @@ -0,0 +1,3 @@ +# Optional environment overrides for the toolkit CLI. +# The CLI is offline and does not require any variable to be set. +LOG_LEVEL=info diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..f01c06e --- /dev/null +++ b/.gitignore @@ -0,0 +1,7 @@ +__pycache__/ +*.py[cod] +.venv/ +.pytest_cache/ +build/ +dist/ +*.egg-info/ diff --git a/examples/input.json b/examples/input.json new file mode 100644 index 0000000..6ed9590 --- /dev/null +++ b/examples/input.json @@ -0,0 +1,9 @@ +{ + "version": 1, + "mode": "validate", + "dry_run": true, + "command": "example", + "arguments": { + "request": "examples/input.json" + } +} diff --git a/pyproject.toml b/pyproject.toml index 13a7c65..af98515 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,8 +1,23 @@ -# Initial declaration; add project dependencies and development tools here. +[build-system] +requires = ["setuptools>=68"] +build-backend = "setuptools.build_meta" + [project] name = "product-54354138-1-tools" version = "0.1.0" +description = "Offline toolkit for validating and running structured JSON request documents." +readme = "README.md" +requires-python = ">=3.11" dependencies = [] [project.optional-dependencies] -dev = ["pytest"] +dev = ["pytest>=8.0"] + +[tool.setuptools] +packages = ["tools"] + +[tool.pytest.ini_options] +minversion = "8.0" +pythonpath = ["."] +testpaths = ["tests"] +addopts = "-ra" diff --git a/tests/test_example.py b/tests/test_example.py new file mode 100644 index 0000000..1574b48 --- /dev/null +++ b/tests/test_example.py @@ -0,0 +1,118 @@ +"""Tests for the placeholder toolkit CLI and its request document.""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +from tools import example + +REPO_ROOT = Path(__file__).resolve().parents[1] +EXAMPLE_REQUEST = REPO_ROOT / "examples" / "input.json" + + +def run_cli(*args: str) -> subprocess.CompletedProcess[str]: + """Run the CLI in a subprocess so stdout/exit codes are asserted end to end.""" + return subprocess.run( + [sys.executable, "-m", "tools.example", *args], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + + +def test_example_request_document_is_valid_json_object() -> None: + document = json.loads(EXAMPLE_REQUEST.read_text(encoding="utf-8")) + + assert isinstance(document, dict) + assert set(example.REQUIRED_FIELDS) <= set(document) + assert document["dry_run"] is True + assert document["mode"] == "validate" + + +def test_example_request_document_passes_validation() -> None: + request = example.validate_request(example.load_request(EXAMPLE_REQUEST)) + + assert request["mode"] == "validate" + assert request["dry_run"] is True + assert request["arguments"]["request"] == "examples/input.json" + + +def test_cli_prints_single_json_envelope_and_exits_zero() -> None: + result = run_cli("examples/input.json") + + assert result.returncode == 0, result.stderr + assert result.stderr == "" + lines = [line for line in result.stdout.splitlines() if line.strip()] + assert len(lines) == 1 + + envelope = json.loads(lines[0]) + assert envelope == { + "status": "ok", + "command": "example", + "mode": "validate", + "dry_run": True, + "version": 1, + "request": "examples/input.json", + } + + +def test_cli_help_exits_zero() -> None: + result = run_cli("--help") + + assert result.returncode == 0 + assert "REQUEST" in result.stdout + + +def test_cli_reports_missing_request_document(tmp_path: Path) -> None: + missing = tmp_path / "missing.json" + + result = run_cli(str(missing)) + + assert result.returncode == example.EXIT_INVALID_REQUEST + envelope = json.loads(result.stdout) + assert envelope["status"] == "error" + assert envelope["error"]["code"] == "request_not_found" + + +def test_cli_reports_malformed_json(tmp_path: Path) -> None: + malformed = tmp_path / "malformed.json" + malformed.write_text("{not json", encoding="utf-8") + + result = run_cli(str(malformed)) + + assert result.returncode == example.EXIT_INVALID_REQUEST + assert json.loads(result.stdout)["error"]["code"] == "request_not_json" + + +@pytest.mark.parametrize( + ("document", "code"), + [ + ([], "request_not_object"), + ({"mode": "validate", "dry_run": True, "command": "example", "arguments": {}}, "request_field_missing"), + ({"version": 1, "mode": "validate", "dry_run": False, "command": "example", "arguments": {"request": "x"}}, "request_not_dry_run"), + ({"version": 1, "mode": "execute", "dry_run": True, "command": "example", "arguments": {"request": "x"}}, "request_mode_unsupported"), + ({"version": 1, "mode": "validate", "dry_run": True, "command": "example", "arguments": {}}, "request_field_missing"), + ({"version": 1, "mode": "validate", "dry_run": True, "command": "", "arguments": {"request": "x"}}, "request_field_invalid"), + ], +) +def test_validate_request_rejects_invalid_documents(document: object, code: str) -> None: + with pytest.raises(example.RequestError) as excinfo: + example.validate_request(document) + + assert excinfo.value.code == code + + +def test_envelope_is_deterministic() -> None: + request = example.validate_request(example.load_request(EXAMPLE_REQUEST)) + + first = example.build_envelope(request, "examples/input.json") + second = example.build_envelope(request, "examples/input.json") + + assert first == second + assert json.dumps(first, sort_keys=True) == json.dumps(second, sort_keys=True) diff --git a/tools/__init__.py b/tools/__init__.py new file mode 100644 index 0000000..a1c96b3 --- /dev/null +++ b/tools/__init__.py @@ -0,0 +1,12 @@ +"""Importable toolkit package for the request-document CLI. + +The package intentionally exposes only its version metadata here; the +executable behaviour lives in :mod:`tools.example` (and in the modules that +later increments add). +""" + +from __future__ import annotations + +__all__ = ["__version__"] + +__version__ = "0.1.0" diff --git a/tools/example.py b/tools/example.py new file mode 100644 index 0000000..32d2b87 --- /dev/null +++ b/tools/example.py @@ -0,0 +1,200 @@ +"""Placeholder toolkit entry point that loads and validates a JSON request. + +This increment only establishes the stable command-line surface: read a single +JSON request document, validate its shape, and print one structured JSON +envelope. Later modules extend the same document format and envelope without +changing the invocation contract, so the module stays deterministic and works +fully offline. + +Usage:: + + python -m tools.example examples/input.json +""" + +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path +from typing import Any, Mapping, Sequence + +__all__ = [ + "EXIT_INVALID_REQUEST", + "EXIT_OK", + "EXIT_USAGE", + "RequestError", + "build_envelope", + "load_request", + "main", + "validate_request", +] + +EXIT_OK = 0 +EXIT_INVALID_REQUEST = 1 +EXIT_USAGE = 2 + +#: Fields every request document must provide. +REQUIRED_FIELDS = ("version", "mode", "dry_run", "command", "arguments") + +#: Modes understood by this increment of the toolkit. +SUPPORTED_MODES = frozenset({"validate"}) + +PROG = "tools.example" + + +class RequestError(ValueError): + """Raised when a request document is missing or malformed.""" + + def __init__(self, code: str, message: str) -> None: + super().__init__(message) + self.code = code + self.message = message + + +def load_request(path: str | Path) -> Any: + """Read and decode the JSON document stored at ``path``.""" + try: + raw = Path(path).read_text(encoding="utf-8") + except FileNotFoundError as exc: + raise RequestError("request_not_found", f"request document not found: {path}") from exc + except IsADirectoryError as exc: + raise RequestError("request_not_a_file", f"request document is not a file: {path}") from exc + except OSError as exc: + raise RequestError("request_unreadable", f"request document could not be read: {path}") from exc + except UnicodeDecodeError as exc: + raise RequestError("request_not_utf8", f"request document is not valid UTF-8: {path}") from exc + + try: + return json.loads(raw) + except json.JSONDecodeError as exc: + raise RequestError( + "request_not_json", + f"request document is not valid JSON: {path} (line {exc.lineno}, column {exc.colno})", + ) from exc + + +def _require(mapping: Mapping[str, Any], key: str, expected: type | tuple[type, ...], label: str) -> Any: + if key not in mapping: + raise RequestError("request_field_missing", f"request field {label} is required") + value = mapping[key] + if isinstance(value, bool) and bool not in ( + expected if isinstance(expected, tuple) else (expected,) + ): + raise RequestError("request_field_invalid", f"request field {label} must be {_describe(expected)}") + if not isinstance(value, expected): + raise RequestError("request_field_invalid", f"request field {label} must be {_describe(expected)}") + return value + + +def _describe(expected: type | tuple[type, ...]) -> str: + names = expected if isinstance(expected, tuple) else (expected,) + return " or ".join(sorted({name.__name__ for name in names})) + + +def validate_request(document: Any) -> dict[str, Any]: + """Validate ``document`` and return the normalised request mapping.""" + if not isinstance(document, dict): + raise RequestError("request_not_object", "request document must be a JSON object") + + for key in REQUIRED_FIELDS: + if key not in document: + raise RequestError("request_field_missing", f"request field '{key}' is required") + + version = _require(document, "version", int, "version") + if version < 1: + raise RequestError("request_field_invalid", "request field 'version' must be a positive integer") + + mode = _require(document, "mode", str, "mode") + if mode not in SUPPORTED_MODES: + supported = ", ".join(sorted(SUPPORTED_MODES)) + raise RequestError( + "request_mode_unsupported", + f"request field 'mode' must be one of: {supported}", + ) + + dry_run = _require(document, "dry_run", bool, "dry_run") + if not dry_run: + raise RequestError( + "request_not_dry_run", + "this increment only supports validate-only dry_run requests", + ) + + command = _require(document, "command", str, "command") + if not command.strip(): + raise RequestError("request_field_invalid", "request field 'command' must not be empty") + + arguments = _require(document, "arguments", dict, "arguments") + request_path = _require(arguments, "request", str, "arguments.request") + if not request_path.strip(): + raise RequestError("request_field_invalid", "request field 'arguments.request' must not be empty") + + return { + "version": version, + "mode": mode, + "dry_run": dry_run, + "command": command, + "arguments": dict(arguments), + } + + +def build_envelope(request: Mapping[str, Any], source: str) -> dict[str, Any]: + """Build the deterministic success envelope for a validated request.""" + return { + "status": "ok", + "command": request["command"], + "mode": request["mode"], + "dry_run": request["dry_run"], + "version": request["version"], + "request": source, + } + + +def build_error_envelope(error: RequestError, source: str) -> dict[str, Any]: + """Build the deterministic failure envelope for an invalid request.""" + return { + "status": "error", + "error": {"code": error.code, "message": error.message}, + "request": source, + } + + +def emit(envelope: Mapping[str, Any]) -> None: + """Print exactly one JSON document to standard output.""" + print(json.dumps(envelope, sort_keys=True, ensure_ascii=True)) + + +def _build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + prog=PROG, + description=( + "Load and validate a JSON request document. The command is offline " + "and prints one structured JSON envelope." + ), + ) + parser.add_argument( + "request", + metavar="REQUEST", + help="path to the JSON request document (for example examples/input.json)", + ) + return parser + + +def main(argv: Sequence[str] | None = None) -> int: + """Run the placeholder CLI and return the process exit code.""" + parser = _build_parser() + args = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) + + try: + document = load_request(args.request) + request = validate_request(document) + except RequestError as error: + emit(build_error_envelope(error, args.request)) + return EXIT_INVALID_REQUEST + + emit(build_envelope(request, args.request)) + return EXIT_OK + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/uv.lock b/uv.lock new file mode 100644 index 0000000..587cd93 --- /dev/null +++ b/uv.lock @@ -0,0 +1,63 @@ +version = 1 +revision = 5 +requires-python = ">=3.11" + +[[package]] +name = "colorama" +version = "0.4.6" +source = { registry = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/simple" } +wheels = [ + { url = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/files/colorama/0.4.6/colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6" }, +] + +[[package]] +name = "iniconfig" +version = "2.3.1" +source = { registry = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/simple" } +wheels = [ + { url = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/files/iniconfig/2.3.1/iniconfig-2.3.1-py3-none-any.whl", hash = "sha256:9121e2c1fdb355232495be3194c8dfe87ccc2d5dee45947b78e68f499790d7a7" }, +] + +[[package]] +name = "packaging" +version = "26.3" +source = { registry = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/simple" } +wheels = [ + { url = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/files/packaging/26.3/packaging-26.3-py3-none-any.whl", hash = "sha256:d7193f7c8e4e93f444fde0262bf90af30e16fa0ad0ad44cb553c87339b23cd1c" }, +] + +[[package]] +name = "pluggy" +version = "1.7.0" +source = { registry = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/simple" } +wheels = [ + { url = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/files/pluggy/1.7.0/pluggy-1.7.0-py3-none-any.whl", hash = "sha256:7dd7b0d8832ba3cb632c306926ded123429211b83641b35dc5c41ad2d34f9bec" }, +] + +[[package]] +name = "product-54354138-1-tools" +version = "0.1.0" +source = { editable = "." } + +[package.optional-dependencies] +dev = [ + { name = "pytest" }, +] + +[package.metadata] +requires-dist = [{ name = "pytest", marker = "extra == 'dev'", specifier = ">=8.0" }] +provides-extras = ["dev"] + +[[package]] +name = "pytest" +version = "8.3.3" +source = { registry = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/simple" } +dependencies = [ + { name = "colorama", marker = "sys_platform == 'win32'" }, + { name = "iniconfig" }, + { name = "packaging" }, + { name = "pluggy" }, +] +wheels = [ + { url = "https://git.p2prod.ru/api/packages/dependency-mirror/pypi/files/pytest/8.3.3/pytest-8.3.3-py3-none-any.whl", hash = "sha256:a6853c7375b2663155079443d2e45de913a911a11d669df02a50814944db57b2" }, +] -- 2.54.0 From 1f929c5d38f2ba30057b8e9bbfa4245453707d30 Mon Sep 17 00:00:00 2001 From: Tool Backend Date: Tue, 3 Nov 2026 00:08:32 +0000 Subject: [PATCH 05/10] change: Implement 2: Add the pluggable credential-free image source registry and HTTPS fetch layer Added tools/errors.py (typed error hierarchy: ToolkitError -> SourceError -> UnknownSourceError/DuplicateSourceError/SearchRequestError/SourceAccessError with SourceUnreachableError/SourceHTTPError/SourcePayloadError/HostNotAllowedError, each with a stable code, details payload and to_dict()) and tools/sources.py (credential-free image source layer). tools/sources.py provides: an HTTPSClient restricted to the approved hosts commons.wikimedia.org, upload.wikimedia.org and api.openverse.org over HTTPS/443, sending an identifying User-Agent, with bounded exponential retry/backoff (capped) on 429/5xx and typed SourceUnreachableError/SourceHTTPError/SourcePayloadError instead of empty results; a SourceAdapter protocol; WikimediaCommonsAdapter (MediaWiki API generator=search + imageinfo) and OpenverseAdapter (/v1/images/) that pass the caller topic and result limit and normalize results into SourceCandidate records carrying source_id, file_url, reported mime type, dimensions, page URL and a byte-fetch callable; and a SourceRegistry that reports available source ids, rejects duplicate registration and raises UnknownSourceError listing the available ids. No adapter or registry path reads API keys, tokens, registrations or secrets from arguments, environment or config. Added tests/test_sources.py covering registry lookup, duplicate registration, unknown-source errors listing ids, request-URL construction with topic/limit, 429/5xx retry and exhaustion, non-retryable statuses, unreachable hosts, malformed payloads, host allowlist rejection without network contact, candidate normalization and the credential-free guarantee. Tests: prepare_environment restore of the tools component (locked env); tools.validate `python -m compileall tools tests` succeeded; tools.test `pytest` succeeded (54 passed, incl. 41 in tests/test_sources.py); tools.smoke `python -m tools.example examples/input.json` returned status ok. Progress: Implemented tools/errors.py, tools/sources.py and tests/test_sources.py for the credential-free source registry and HTTPS fetch layer. Verified with the facade commands tools.validate, tools.test (54 passed) and tools.smoke, all green on the integrated branch (target task/1-task was already at HEAD). No protected facade paths touched. Published by developer-agent through Tool Backend. Closes #4 --- .agent-work/ledger.json | 5 + tests/test_sources.py | 452 +++++++++++++++++++++++++++++++ tools/errors.py | 187 +++++++++++++ tools/sources.py | 578 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1222 insertions(+) create mode 100644 tests/test_sources.py create mode 100644 tools/errors.py create mode 100644 tools/sources.py diff --git a/.agent-work/ledger.json b/.agent-work/ledger.json index 84d6e0c..8240ce2 100644 --- a/.agent-work/ledger.json +++ b/.agent-work/ledger.json @@ -4,6 +4,11 @@ "events": [ "Created tools package (tools/__init__.py, tools/example.py), examples/input.json, tests/test_example.py, .gitignore, .env.example; updated pyproject.toml (metadata, setuptools packages, dev pytest extra, pytest pythonpath/testpaths, requires-python >=3.11 due to approved-mirror exceptiongroup constraint) and generated uv.lock via prepare_environment update. Facade validate/test/smoke and --help all pass locally." ] + }, + "projects/product-54354138-1#4": { + "events": [ + "Implemented tools/errors.py, tools/sources.py and tests/test_sources.py for the credential-free source registry and HTTPS fetch layer. Verified with the facade commands tools.validate, tools.test (54 passed) and tools.smoke, all green on the integrated branch (target task/1-task was already at HEAD). No protected facade paths touched." + ] } } } diff --git a/tests/test_sources.py b/tests/test_sources.py new file mode 100644 index 0000000..cf02324 --- /dev/null +++ b/tests/test_sources.py @@ -0,0 +1,452 @@ +"""Tests for the credential-free image source registry and HTTPS layer.""" + +from __future__ import annotations + +import urllib.parse + +import pytest + +from tools import sources +from tools.errors import ( + DuplicateSourceError, + HostNotAllowedError, + SearchRequestError, + SourceAccessError, + SourceHTTPError, + SourceUnreachableError, + ToolkitError, + UnknownSourceError, +) +from tools.sources import ( + APPROVED_HOSTS, + DEFAULT_LIMIT, + HTTPSClient, + HTTPResponse, + OpenverseAdapter, + SourceRegistry, + WikimediaCommonsAdapter, + build_registry, + is_approved_url, + validate_source_url, +) + + +urllib_parse = urllib.parse.urlsplit +urllib_parse_qsl = urllib.parse.parse_qsl + + +class FakeTransport: + """Scripted transport: records requests and replays canned responses.""" + + def __init__(self, responses): + self._responses = list(responses) + self.calls = [] + + def __call__(self, url, headers, timeout): + self.calls.append({"url": url, "headers": dict(headers), "timeout": timeout}) + item = self._responses.pop(0) if len(self._responses) > 1 else self._responses[0] + if isinstance(item, Exception): + raise item + return item + + +def json_response(payload, status=200): + import json + + return HTTPResponse(status=status, body=json.dumps(payload).encode("utf-8")) + + +def make_client(transport, **kwargs): + kwargs.setdefault("sleep", lambda _seconds: None) + kwargs.setdefault("max_attempts", 3) + return HTTPSClient(transport=transport, **kwargs) + + +# -- registry --------------------------------------------------------------- + + +def test_default_registry_reports_available_source_ids(): + registry = build_registry() + assert registry.available_sources() == ["openverse", "wikimedia_commons"] + assert "wikimedia_commons" in registry + assert "openverse" in registry + assert len(registry) == 2 + assert sorted(iter(registry)) == ["openverse", "wikimedia_commons"] + + +def test_registry_lookup_returns_registered_adapter(): + registry = build_registry() + assert isinstance(registry.get("wikimedia_commons"), WikimediaCommonsAdapter) + assert isinstance(registry.get("openverse"), OpenverseAdapter) + assert registry.get("openverse").source_id == "openverse" + + +def test_registry_rejects_duplicate_registration(): + registry = build_registry() + with pytest.raises(DuplicateSourceError) as excinfo: + registry.register(OpenverseAdapter()) + assert excinfo.value.source_id == "openverse" + assert isinstance(excinfo.value, ToolkitError) + # replacing explicitly is still possible + registry.register(OpenverseAdapter(), replace=True) + assert registry.available_sources() == ["openverse", "wikimedia_commons"] + + +def test_unknown_source_raises_typed_error_listing_available_ids(): + registry = build_registry() + with pytest.raises(UnknownSourceError) as excinfo: + registry.get("nope") + error = excinfo.value + assert error.available == ("openverse", "wikimedia_commons") + assert "openverse" in str(error) + assert "wikimedia_commons" in str(error) + assert error.details["available_sources"] == ["openverse", "wikimedia_commons"] + assert error.code == "source_unknown" + + +def test_registry_search_with_unknown_source_does_not_contact_network(): + transport = FakeTransport([RuntimeError("must not be called")]) + registry = build_registry(client=make_client(transport)) + with pytest.raises(UnknownSourceError): + registry.search("missing", "cats") + assert transport.calls == [] + + +# -- request construction --------------------------------------------------- + +COMMONS_PAYLOAD = { + "query": { + "pages": [ + { + "title": "File:Cat one.svg", + "imageinfo": [ + { + "url": "https://upload.wikimedia.org/wikipedia/commons/1/1a/Cat_one.svg", + "mime": "image/svg+xml", + "width": 48, + "height": 48, + "descriptionurl": "https://commons.wikimedia.org/wiki/File:Cat_one.svg", + } + ], + }, + { + "title": "File:Cat two.png", + "imageinfo": [ + { + "url": "https://upload.wikimedia.org/wikipedia/commons/2/2b/Cat_two.png", + "mime": "image/png", + "width": 64, + "height": 64, + "descriptionurl": "https://commons.wikimedia.org/wiki/File:Cat_two.png", + } + ], + }, + { + "title": "File:Cat three.ico", + "imageinfo": [ + { + "url": "https://upload.wikimedia.org/wikipedia/commons/3/3c/Cat_three.ico", + "mime": "image/x-icon", + "width": 32, + "height": 32, + "descriptionurl": "https://commons.wikimedia.org/wiki/File:Cat_three.ico", + } + ], + }, + ] + } +} + +OPENVERSE_PAYLOAD = { + "result_count": 2, + "results": [ + { + "title": "Sleeping cat", + "url": "https://live.staticflickr.com/1/cat.jpg", + "thumbnail": "https://api.openverse.org/v1/images/aaa/thumb/", + "filetype": "jpg", + "width": 1024, + "height": 768, + "license": "by", + "creator": "someone", + "foreign_landing_url": "https://www.flickr.com/photos/someone/1", + }, + { + "title": "Second cat", + "url": "https://live.staticflickr.com/2/cat.jpg", + "thumbnail": "https://api.openverse.org/v1/images/bbb/thumb/", + "filetype": "jpg", + "width": 800, + "height": 600, + "license": "cc0", + "creator": "other", + "foreign_landing_url": "https://www.flickr.com/photos/other/2", + }, + ], +} + + +def test_commons_search_url_carries_topic_and_limit(): + adapter = WikimediaCommonsAdapter(client=make_client(FakeTransport([HTTPResponse(500, b"")]))) + url = adapter.build_search_url("sleeping cat", 3) + parsed = urllib_parse(url) + assert parsed.scheme == "https" + assert parsed.hostname == "commons.wikimedia.org" + assert parsed.port is None + query = dict(urllib_parse_qsl(parsed.query)) + assert query["gsrsearch"] == "sleeping cat" + assert query["gsrlimit"] == "3" + assert query["action"] == "query" + assert query["format"] == "json" + assert query["generator"] == "search" + assert query["gsrnamespace"] == "6" + assert "imageinfo" in query["prop"] + assert is_approved_url(url) + + +def test_openverse_search_url_carries_topic_and_limit(): + adapter = OpenverseAdapter(client=make_client(FakeTransport([HTTPResponse(500, b"")]))) + url = adapter.build_search_url("sleeping cat", 4) + parsed = urllib_parse(url) + assert parsed.scheme == "https" + assert parsed.hostname == "api.openverse.org" + assert parsed.path == "/v1/images/" + query = dict(urllib_parse_qsl(parsed.query)) + assert query["q"] == "sleeping cat" + assert query["page_size"] == "4" + assert is_approved_url(url) + + +def test_topic_and_limit_reach_the_request_through_the_registry(): + transport = FakeTransport([json_response(COMMONS_PAYLOAD)]) + registry = build_registry(client=make_client(transport)) + registry.search("wikimedia_commons", " red panda ", limit=2) + assert len(transport.calls) == 1 + query = dict(urllib_parse_qsl(urllib_parse(transport.calls[0]["url"]).query)) + assert query["gsrsearch"] == "red panda" + assert query["gsrlimit"] == "2" + + +@pytest.mark.parametrize("topic,limit", [("", 3), (" ", 3), ("cats", 0), ("cats", -1), ("cats", 10_000)]) +def test_invalid_topic_or_limit_is_rejected_before_any_request(topic, limit): + transport = FakeTransport([json_response(COMMONS_PAYLOAD)]) + registry = build_registry(client=make_client(transport)) + with pytest.raises(SearchRequestError): + registry.search("wikimedia_commons", topic, limit=limit) + assert transport.calls == [] + + +def test_default_limit_is_used_when_caller_omits_it(): + transport = FakeTransport([json_response(COMMONS_PAYLOAD)]) + registry = build_registry(client=make_client(transport)) + registry.search("wikimedia_commons", "cats") + query = dict(urllib_parse_qsl(urllib_parse(transport.calls[0]["url"]).query)) + assert query["gsrlimit"] == str(DEFAULT_LIMIT) + + +# -- normalization ---------------------------------------------------------- + + +def test_commons_results_are_normalized_into_candidates(): + transport = FakeTransport([json_response(COMMONS_PAYLOAD)]) + adapter = WikimediaCommonsAdapter(client=make_client(transport)) + candidates = adapter.search("cats", 3) + assert len(candidates) == 3 + first = candidates[0] + assert first.source_id == "wikimedia_commons" + assert first.file_url.endswith("Cat_one.svg") + assert first.mime_type == "image/svg+xml" + assert first.width == 48 and first.height == 48 + assert first.page_url.startswith("https://commons.wikimedia.org/") + assert callable(first.fetch_bytes) + assert first.to_dict()["mime_type"] == "image/svg+xml" + assert "fetch_bytes" not in first.to_dict() + + +def test_commons_respects_the_requested_limit(): + transport = FakeTransport([json_response(COMMONS_PAYLOAD)]) + adapter = WikimediaCommonsAdapter(client=make_client(transport)) + assert len(adapter.search("cats", 1)) == 1 + assert len(adapter.search("cats", 2)) == 2 + + +def test_openverse_results_are_normalized_into_candidates(): + transport = FakeTransport([json_response(OPENVERSE_PAYLOAD)]) + adapter = OpenverseAdapter(client=make_client(transport)) + candidates = adapter.search("cats", 5) + assert len(candidates) == 2 + first = candidates[0] + assert first.source_id == "openverse" + assert first.file_url == "https://api.openverse.org/v1/images/aaa/thumb/" + assert first.mime_type == "jpg" + assert first.extra["original_url"] == "https://live.staticflickr.com/1/cat.jpg" + assert callable(first.fetch_bytes) + + +def test_candidate_fetch_bytes_downloads_through_the_client(): + transport = FakeTransport([json_response(COMMONS_PAYLOAD), HTTPResponse(200, b"IMG-bytes")]) + adapter = WikimediaCommonsAdapter(client=make_client(transport)) + candidate = adapter.search("cats", 1)[0] + assert candidate.fetch_bytes() == b"IMG-bytes" + assert transport.calls[-1]["url"] == candidate.file_url + assert transport.calls[-1]["headers"]["User-Agent"] == sources.USER_AGENT + + +def test_adapters_send_an_identifying_user_agent(): + transport = FakeTransport([json_response(OPENVERSE_PAYLOAD)]) + OpenverseAdapter(client=make_client(transport)).search("cats", 1) + agent = transport.calls[0]["headers"]["User-Agent"] + assert sources.PRODUCT_NAME in agent + assert "toolkit" in agent.lower() + + +# -- retry / backoff -------------------------------------------------------- + + +def test_retry_statuses_are_retried_then_succeed(): + sleeps = [] + transport = FakeTransport( + [HTTPResponse(429, b""), HTTPResponse(503, b""), json_response(OPENVERSE_PAYLOAD)] + ) + client = HTTPSClient(transport=transport, sleep=sleeps.append, max_attempts=3) + response = client.get("https://api.openverse.org/v1/images/?q=cats", source_id="openverse") + assert response.status == 200 + assert len(transport.calls) == 3 + assert sleeps == [client.backoff_delay(1), client.backoff_delay(2)] + assert sleeps[0] < sleeps[1] + + +def test_exhausted_retries_raise_source_http_error(): + transport = FakeTransport([HTTPResponse(500, b"boom")]) + sleeps = [] + client = HTTPSClient(transport=transport, sleep=sleeps.append, max_attempts=3) + with pytest.raises(SourceHTTPError) as excinfo: + client.get("https://api.openverse.org/v1/images/?q=cats", source_id="openverse") + assert excinfo.value.status == 500 + assert len(transport.calls) == 3 + assert len(sleeps) == 2 + assert isinstance(excinfo.value, SourceAccessError) + + +def test_backoff_is_capped(): + client = HTTPSClient(transport=FakeTransport([HTTPResponse(500, b"")]), sleep=lambda _s: None) + assert client.backoff_delay(50) <= sources.MAX_BACKOFF_SECONDS + + +def test_non_retryable_status_is_raised_immediately(): + transport = FakeTransport([HTTPResponse(404, b"nope")]) + client = HTTPSClient(transport=transport, sleep=lambda _s: None, max_attempts=3) + with pytest.raises(SourceHTTPError): + client.get("https://commons.wikimedia.org/w/api.php?action=query") + assert len(transport.calls) == 1 + + +def test_unreachable_host_raises_access_error_not_empty_result(): + transport = FakeTransport([OSError("name resolution failed")]) + client = HTTPSClient(transport=transport, sleep=lambda _s: None, max_attempts=2) + with pytest.raises(SourceUnreachableError) as excinfo: + client.get("https://commons.wikimedia.org/w/api.php?action=query") + assert len(transport.calls) == 2 + assert isinstance(excinfo.value, SourceAccessError) + + +def test_search_propagates_access_errors_instead_of_returning_empty_list(): + transport = FakeTransport([OSError("network down")]) + registry = build_registry(client=make_client(transport, max_attempts=1)) + with pytest.raises(SourceAccessError): + registry.search("wikimedia_commons", "cats") + + +def test_malformed_payload_raises_access_error(): + transport = FakeTransport([HTTPResponse(200, b"not json")]) + adapter = OpenverseAdapter(client=make_client(transport)) + with pytest.raises(SourceAccessError): + adapter.search("cats", 1) + + +# -- host allowlist --------------------------------------------------------- + +REJECTED_URLS = [ + "http://commons.wikimedia.org/w/api.php?action=query", + "https://evil.example.com/cat.png", + "https://commons.wikimedia.org.evil.example/w/api.php", + "file:///etc/passwd", + "ftp://upload.wikimedia.org/cat.png", + "https://commons.wikimedia.org:8443/w/api.php", + "", +] + + +@pytest.mark.parametrize("url", REJECTED_URLS) +def test_host_allowlist_rejects_unapproved_urls_without_network(url): + transport = FakeTransport([RuntimeError("network must not be contacted")]) + client = HTTPSClient(transport=transport, sleep=lambda _s: None) + with pytest.raises(HostNotAllowedError): + client.get(url) + assert transport.calls == [] + assert is_approved_url(url) is False + + +@pytest.mark.parametrize( + "url", + [ + "https://commons.wikimedia.org/w/api.php?action=query", + "https://upload.wikimedia.org/wikipedia/commons/1/1a/Cat.svg", + "https://api.openverse.org/v1/images/?q=cats", + "https://commons.wikimedia.org:443/w/api.php", + ], +) +def test_host_allowlist_accepts_approved_https_hosts(url): + assert validate_source_url(url) == url + assert is_approved_url(url) is True + + +def test_approved_hosts_are_the_two_credential_free_sources(): + assert "commons.wikimedia.org" in APPROVED_HOSTS + assert "upload.wikimedia.org" in APPROVED_HOSTS + assert "api.openverse.org" in APPROVED_HOSTS + + +# -- credential-free guarantee --------------------------------------------- + + +def test_search_takes_no_credential_arguments(): + import inspect + + for adapter in (WikimediaCommonsAdapter, OpenverseAdapter): + params = set(inspect.signature(adapter.search).parameters) - {"self"} + assert params == {"topic", "limit"}, params + + registry = build_registry() + params = set(inspect.signature(registry.search).parameters) - {"self"} + assert params == {"source_id", "topic", "limit"}, params + + +def test_requests_carry_no_api_keys_tokens_or_credentials(monkeypatch): + for name in ("OPENVERSE_API_KEY", "WIKIMEDIA_TOKEN", "API_KEY", "CLIENT_SECRET"): + monkeypatch.setenv(name, "should-never-be-sent") + + transport = FakeTransport([json_response(COMMONS_PAYLOAD)]) + registry = build_registry(client=make_client(transport)) + for source_id in registry.available_sources(): + transport_calls = len(transport.calls) + try: + registry.search(source_id, "cats", limit=1) + except SourceAccessError: # payload shapes differ per source; URL is what matters + pass + new_calls = transport.calls[transport_calls:] + assert new_calls, source_id + for call in new_calls: + blob = call["url"] + " " + repr(call["headers"]) + for needle in ("should-never-be-sent", "api_key", "apikey", "token", "Authorization"): + assert needle.lower() not in blob.lower(), (source_id, needle, blob) + + +def test_module_reads_no_credentials_from_environment(monkeypatch): + for name in ("OPENVERSE_API_KEY", "WIKIMEDIA_TOKEN", "API_KEY", "CLIENT_SECRET"): + monkeypatch.delattr("os.environ", name, raising=False) + first = build_registry() + assert first.available_sources() == ["openverse", "wikimedia_commons"] + monkeypatch.setenv("OPENVERSE_API_KEY", "secret") + second = build_registry() + assert second.available_sources() == first.available_sources() diff --git a/tools/errors.py b/tools/errors.py new file mode 100644 index 0000000..013ae8b --- /dev/null +++ b/tools/errors.py @@ -0,0 +1,187 @@ +"""Typed errors raised deliberately by the toolkit. + +The image-source layer never degrades a failed or unknown source into a silent, +empty result. Every such situation raises a :class:`ToolkitError` subclass that +carries a stable ``code`` string plus machine-readable ``details`` so the CLI can +map it onto an exit status and a structured JSON envelope. + +Hierarchy:: + + ToolkitError + +-- SourceError + +-- UnknownSourceError (unknown source id, lists available ids) + +-- DuplicateSourceError (registry already holds that id) + +-- SearchRequestError (topic/limit rejected before any request) + +-- SourceAccessError (the source could not be used) + +-- SourceUnreachableError (transport/DNS/timeout failure) + +-- SourceHTTPError (non-retryable or exhausted HTTP status) + +-- SourcePayloadError (body is not the expected JSON shape) + +-- HostNotAllowedError (URL outside the approved hosts) +""" + +from __future__ import annotations + +from typing import Any, Iterable, Mapping + +__all__ = [ + "DuplicateSourceError", + "HostNotAllowedError", + "SearchRequestError", + "SourceAccessError", + "SourceError", + "SourceHTTPError", + "SourcePayloadError", + "SourceUnreachableError", + "ToolkitError", + "UnknownSourceError", +] + + +class ToolkitError(Exception): + """Base class for errors the toolkit raises on purpose.""" + + code = "toolkit_error" + + def __init__( + self, + message: str, + *, + code: str | None = None, + details: Mapping[str, Any] | None = None, + ) -> None: + super().__init__(message) + self.message = message + if code is not None: + self.code = code + self.details: dict[str, Any] = dict(details or {}) + + def to_dict(self) -> dict[str, Any]: + """Return the JSON-safe envelope fragment describing this error.""" + + payload: dict[str, Any] = {"code": self.code, "message": self.message} + if self.details: + payload["details"] = dict(self.details) + return payload + + def __str__(self) -> str: # pragma: no cover - trivial + return self.message + + +class SourceError(ToolkitError): + """Base class for every image-source problem.""" + + code = "source_error" + + +class UnknownSourceError(SourceError): + """Raised when the caller asks for a source id the registry does not know.""" + + code = "source_unknown" + + def __init__(self, source_id: str, available: Iterable[str]) -> None: + self.source_id = str(source_id) + self.available: tuple[str, ...] = tuple(sorted(str(item) for item in available)) + listing = ", ".join(self.available) if self.available else "" + super().__init__( + f"unknown image source {self.source_id!r}; available sources: {listing}", + details={"source_id": self.source_id, "available_sources": list(self.available)}, + ) + + +class DuplicateSourceError(SourceError): + """Raised when an adapter id is registered twice.""" + + code = "source_duplicate" + + def __init__(self, source_id: str) -> None: + self.source_id = str(source_id) + super().__init__( + f"image source {self.source_id!r} is already registered", + details={"source_id": self.source_id}, + ) + + +class SearchRequestError(SourceError): + """Raised when the caller supplies a topic/limit a source cannot honour.""" + + code = "search_invalid_request" + + def __init__(self, message: str, *, details: Mapping[str, Any] | None = None) -> None: + super().__init__(message, details=details) + + +class SourceAccessError(SourceError): + """Raised when a known source cannot be reached or refuses to answer.""" + + code = "source_access" + + def __init__( + self, + message: str, + *, + url: str | None = None, + source_id: str | None = None, + details: Mapping[str, Any] | None = None, + ) -> None: + merged: dict[str, Any] = dict(details or {}) + if url is not None: + merged["url"] = url + if source_id is not None: + merged["source_id"] = source_id + super().__init__(message, details=merged) + self.url = url + self.source_id = source_id + + +class SourceUnreachableError(SourceAccessError): + """Raised when the transport cannot deliver a request to the host at all.""" + + code = "source_unreachable" + + def __init__(self, url: str, *, source_id: str | None = None, reason: str | None = None) -> None: + message = f"image source host is unreachable: {url}" + if reason: + message = f"{message} ({reason})" + super().__init__(message, url=url, source_id=source_id, details={"reason": reason} if reason else None) + + +class SourceHTTPError(SourceAccessError): + """Raised for an HTTP status the client cannot recover from.""" + + code = "source_http_error" + + def __init__(self, url: str, status: int, *, source_id: str | None = None) -> None: + super().__init__( + f"image source returned HTTP status {status}: {url}", + url=url, + source_id=source_id, + details={"status": status}, + ) + self.status = int(status) + + +class SourcePayloadError(SourceAccessError): + """Raised when a source answers with something the adapter cannot read.""" + + code = "source_invalid_response" + + def __init__(self, url: str, *, source_id: str | None = None, reason: str | None = None) -> None: + message = f"image source returned an unusable response: {url}" + if reason: + message = f"{message} ({reason})" + super().__init__(message, url=url, source_id=source_id, details={"reason": reason} if reason else None) + + +class HostNotAllowedError(SourceAccessError): + """Raised when a URL leaves the approved HTTPS host allowlist.""" + + code = "source_host_not_allowed" + + def __init__(self, url: str, *, allowed: Iterable[str] = ()) -> None: + self.allowed: tuple[str, ...] = tuple(sorted(str(item) for item in allowed)) + super().__init__( + f"refusing to contact {url!r}: only approved HTTPS hosts are allowed " + f"({', '.join(self.allowed) if self.allowed else 'none configured'})", + url=str(url), + details={"allowed_hosts": list(self.allowed)}, + ) diff --git a/tools/sources.py b/tools/sources.py new file mode 100644 index 0000000..5aa469a --- /dev/null +++ b/tools/sources.py @@ -0,0 +1,578 @@ +"""Pluggable, credential-free image source registry and its HTTPS layer. + +Nothing in this module reads an API key, token, registration or secret from +arguments, environment variables or configuration files: Wikimedia Commons and +Openverse are queried anonymously. The module provides + +* :class:`HTTPSClient` -- a small HTTPS client that identifies itself with a + stable ``User-Agent``, refuses every URL outside the approved hosts + (Wikimedia Commons and Openverse) on port 443, retries HTTP 429/5xx a bounded + number of times with capped exponential backoff, and raises + :class:`~tools.errors.SourceAccessError` instead of returning an empty result + when the host is unreachable or keeps failing; +* :class:`SourceAdapter` -- the interface every image source implements; +* :class:`WikimediaCommonsAdapter` and :class:`OpenverseAdapter` -- two working + credential-free sources behind that interface; +* :class:`SourceRegistry` -- the plugin list that reports available source ids + and turns an unknown id into :class:`~tools.errors.UnknownSourceError` listing + them. +""" + +from __future__ import annotations + +import json +import time +import urllib.error +import urllib.parse +import urllib.request +from dataclasses import dataclass, field +from typing import Any, Callable, Iterable, Mapping, Protocol, runtime_checkable + +from .errors import ( + DuplicateSourceError, + HostNotAllowedError, + SearchRequestError, + SourceAccessError, + SourceHTTPError, + SourceError, + SourcePayloadError, + SourceUnreachableError, + UnknownSourceError, +) + +__all__ = [ + "APPROVED_HOSTS", + "DEFAULT_LIMIT", + "DEFAULT_MAX_ATTEMPTS", + "HTTPSClient", + "HTTPResponse", + "MAX_LIMIT", + "OPENVERSE_API_HOST", + "OpenverseAdapter", + "SourceAdapter", + "SourceCandidate", + "SourceRegistry", + "USER_AGENT", + "WIKIMEDIA_COMMONS_API_HOST", + "WIKIMEDIA_COMMONS_MEDIA_HOST", + "build_registry", + "default_registry", + "is_approved_url", + "search", + "validate_source_url", +] + +PRODUCT_NAME = "product-54354138-1" +USER_AGENT = f"{PRODUCT_NAME}-image-toolkit/0.1.0 (credential-free image source client)" + +WIKIMEDIA_COMMONS_API_HOST = "commons.wikimedia.org" +WIKIMEDIA_COMMONS_MEDIA_HOST = "upload.wikimedia.org" +OPENVERSE_API_HOST = "api.openverse.org" + +# Wikimedia Commons is reachable through its API host plus the media host that +# serves the original files, Openverse through its API host (and the thumbnail +# endpoint on the same API host). +APPROVED_HOSTS = frozenset( + {WIKIMEDIA_COMMONS_API_HOST, WIKIMEDIA_COMMONS_MEDIA_HOST, OPENVERSE_API_HOST} +) +APPROVED_SCHEME = "https" +APPROVED_PORT = 443 + +DEFAULT_TIMEOUT = 10.0 +DEFAULT_MAX_ATTEMPTS = 3 +DEFAULT_BACKOFF_SECONDS = 0.5 +MAX_BACKOFF_SECONDS = 8.0 +RETRY_STATUS_CODES = frozenset({429}) | frozenset(range(500, 600)) + + +@dataclass(frozen=True) +class HTTPResponse: + """A transport-independent HTTP response.""" + + status: int + body: bytes + headers: Mapping[str, str] = field(default_factory=dict) + + def json(self, *, url: str = "", source_id: str | None = None) -> Any: + try: + return json.loads(self.body) + except (ValueError, UnicodeDecodeError) as exc: # pragma: no cover - defensive + raise SourcePayloadError(url or "", source_id=source_id, reason=str(exc)) from exc + + +class Transport(Protocol): + """Callable performing one HTTP GET; injectable so tests never hit the net.""" + + def __call__(self, url: str, headers: Mapping[str, str], timeout: float) -> HTTPResponse: ... + + +def validate_source_url(url: str, *, allowed: Iterable[str] = APPROVED_HOSTS) -> str: + """Return *url* when it is an approved HTTPS URL, else raise a typed error.""" + + allowed_set = frozenset(allowed) + if not isinstance(url, str) or not url: + raise HostNotAllowedError(str(url), allowed=allowed_set) + parts = urllib.parse.urlsplit(url) + if parts.scheme != APPROVED_SCHEME: + raise HostNotAllowedError(url, allowed=allowed_set) + host = (parts.hostname or "").lower() + if host not in allowed_set: + raise HostNotAllowedError(url, allowed=allowed_set) + try: + port = parts.port + except ValueError as exc: + raise HostNotAllowedError(url, allowed=allowed_set) from exc + if port not in (None, APPROVED_PORT): + raise HostNotAllowedError(url, allowed=allowed_set) + return url + + +def is_approved_url(url: str, *, allowed: Iterable[str] = APPROVED_HOSTS) -> bool: + """Non-raising variant of :func:`validate_source_url` used by tests/callers.""" + + try: + validate_source_url(url, allowed=allowed) + except HostNotAllowedError: + return False + return True + + +def urllib_transport(url: str, headers: Mapping[str, str], timeout: float) -> HTTPResponse: + """Default transport: a plain stdlib GET returning status, body and headers.""" + + request = urllib.request.Request(url, headers=dict(headers), method="GET") + try: + with urllib.request.urlopen(request, timeout=timeout) as response: + return HTTPResponse( + status=int(getattr(response, "status", 200)), + body=response.read(), + headers=dict(response.headers.items()), + ) + except urllib.error.HTTPError as exc: + body = b"" + try: + body = exc.read() + except Exception: # pragma: no cover - body optional + body = b"" + return HTTPResponse( + status=int(exc.code), + body=body, + headers=dict(getattr(exc, "headers", {}) or {}), + ) + + +class HTTPSClient: + """Fetch a URL only when it is approved, with bounded retry/backoff. + + The client never returns an empty result for a broken source: unreachable + hosts raise :class:`SourceUnreachableError` and exhausted/non-retryable + statuses raise :class:`SourceHTTPError`. Both derive from + :class:`SourceAccessError`. + """ + + def __init__( + self, + *, + user_agent: str = USER_AGENT, + allowed_hosts: Iterable[str] = APPROVED_HOSTS, + timeout: float = DEFAULT_TIMEOUT, + max_attempts: int = DEFAULT_MAX_ATTEMPTS, + backoff_seconds: float = DEFAULT_BACKOFF_SECONDS, + max_backoff_seconds: float = MAX_BACKOFF_SECONDS, + transport: Transport | None = None, + sleep: Callable[[float], None] = time.sleep, + ) -> None: + if max_attempts < 1: + raise ValueError("max_attempts must be >= 1") + self.user_agent = user_agent + self.allowed_hosts = frozenset(allowed_hosts) + self.timeout = float(timeout) + self.max_attempts = int(max_attempts) + self.backoff_seconds = float(backoff_seconds) + self.max_backoff_seconds = float(max_backoff_seconds) + self._transport: Transport = transport or urllib_transport + self._sleep = sleep + + # -- public API ----------------------------------------------------- + def headers(self) -> dict[str, str]: + """Request headers, always including the identifying User-Agent.""" + + return { + "User-Agent": self.user_agent, + "Accept": "application/json, image/*;q=0.8, */*;q=0.5", + } + + def backoff_delay(self, attempt: int) -> float: + """Capped exponential backoff delay for 1-based *attempt*.""" + + delay = self.backoff_seconds * (2 ** max(0, attempt - 1)) + return min(delay, self.max_backoff_seconds) + + def get(self, url: str, *, source_id: str | None = None) -> HTTPResponse: + """GET *url*, retrying transient statuses, then raise typed errors.""" + + validate_source_url(url, allowed=self.allowed_hosts) + response: HTTPResponse | None = None + for attempt in range(1, self.max_attempts + 1): + try: + response = self._transport(url, self.headers(), self.timeout) + except SourceAccessError: + raise + except Exception as exc: # transport-level failure + if attempt >= self.max_attempts: + raise SourceUnreachableError( + url, source_id=source_id, reason=f"{type(exc).__name__}: {exc}" + ) from exc + self._sleep(self.backoff_delay(attempt)) + continue + if response.status in RETRY_STATUS_CODES and attempt < self.max_attempts: + self._sleep(self.backoff_delay(attempt)) + continue + if 200 <= response.status < 300: + return response + raise SourceHTTPError(url, response.status, source_id=source_id) + # Defensive: the loop either returned or raised above. + status = response.status if response is not None else 0 + raise SourceHTTPError(url, status, source_id=source_id) + + def get_json(self, url: str, *, source_id: str | None = None) -> Any: + response = self.get(url, source_id=source_id) + try: + return json.loads(response.body) + except (ValueError, UnicodeDecodeError) as exc: + raise SourcePayloadError(url, source_id=source_id, reason=str(exc)) from exc + + +DEFAULT_LIMIT = 10 +MAX_LIMIT = 50 + + +@dataclass(frozen=True) +class SourceCandidate: + """One normalized search hit. + + ``source_id`` identifies the adapter, ``file_url`` is the direct HTTPS + location of the original image, ``mime_type`` is the type the source + reports (may be ``None`` when unstated) and ``fetch_bytes`` is a zero-argument + callable returning the image bytes through the approved HTTPS client. + """ + + source_id: str + file_url: str + mime_type: str | None + fetch_bytes: Callable[[], bytes] + title: str | None = None + page_url: str | None = None + width: int | None = None + height: int | None = None + extra: Mapping[str, Any] = field(default_factory=dict) + + def fetch(self) -> bytes: + """Convenience passthrough to :attr:`fetch_bytes`.""" + + return self.fetch_bytes() + + def to_dict(self) -> dict[str, Any]: + """Metadata-only projection (the callable is intentionally omitted).""" + + payload: dict[str, Any] = { + "source_id": self.source_id, + "file_url": self.file_url, + "mime_type": self.mime_type, + } + for key in ("title", "page_url", "width", "height"): + value = getattr(self, key) + if value is not None: + payload[key] = value + return payload + + +@runtime_checkable +class SourceAdapter(Protocol): + """Interface implemented by every image source.""" + + source_id: str + + def search(self, topic: str, limit: int) -> list[SourceCandidate]: ... + + +def normalize_search_args(topic: str, limit: int) -> tuple[str, int]: + """Validate the caller-supplied topic and result limit.""" + + if not isinstance(topic, str): + raise SearchRequestError( + f"topic must be a string, got {type(topic).__name__}", + details={"topic": repr(topic)}, + ) + topic = topic.strip() + if not topic: + raise SearchRequestError("topic must not be empty", details={"topic": topic}) + if isinstance(limit, bool) or not isinstance(limit, int): + raise SearchRequestError( + f"limit must be an integer, got {type(limit).__name__}", + details={"limit": repr(limit)}, + ) + if limit < 1: + raise SearchRequestError(f"limit must be >= 1, got {limit}", details={"limit": limit}) + if limit > MAX_LIMIT: + raise SearchRequestError( + f"limit must be <= {MAX_LIMIT}, got {limit}", details={"limit": limit} + ) + return topic, limit + + +class _BaseAdapter: + """Shared normalization helpers for concrete adapters.""" + + source_id = "" + + def __init__(self, client: HTTPSClient | None = None) -> None: + self.client = client or HTTPSClient() + + def _candidate( + self, + *, + file_url: str, + mime_type: str | None, + title: str | None = None, + page_url: str | None = None, + width: int | None = None, + height: int | None = None, + extra: Mapping[str, Any] | None = None, + validate: bool = True, + ) -> SourceCandidate: + if validate: + validate_source_url(file_url, allowed=self.client.allowed_hosts) + source_id = self.source_id + client = self.client + return SourceCandidate( + source_id=source_id, + file_url=file_url, + mime_type=mime_type, + fetch_bytes=lambda url=file_url: client.get(url, source_id=source_id).body, + title=title, + page_url=page_url, + width=width, + height=height, + extra=dict(extra or {}), + ) + + def __repr__(self) -> str: # pragma: no cover - trivial + return f"<{type(self).__name__} source_id={self.source_id!r}>" + + +class WikimediaCommonsAdapter(_BaseAdapter): + """Wikimedia Commons search through the anonymous MediaWiki action API.""" + + source_id = "wikimedia_commons" + api_host = WIKIMEDIA_COMMONS_API_HOST + + def build_search_url(self, topic: str, limit: int) -> str: + """Construct the generator search URL for *topic* and *limit*.""" + + topic, limit = normalize_search_args(topic, limit) + params = { + "action": "query", + "format": "json", + "formatversion": "2", + "generator": "search", + "gsrsearch": topic, + "gsrnamespace": "6", + "gsrlimit": str(limit), + "prop": "imageinfo", + "iiprop": "url|mime|size", + "iiurlwidth": "1024", + } + query = urllib.parse.urlencode(params) + return f"https://{self.api_host}/w/api.php?{query}" + + def search(self, topic: str, limit: int) -> list[SourceCandidate]: + url = self.build_search_url(topic, limit) + payload = self.client.get_json(url, source_id=self.source_id) + if not isinstance(payload, Mapping): + raise SourcePayloadError(url, source_id=self.source_id, reason="expected a JSON object") + if "error" in payload: + raise SourceAccessError( + f"Wikimedia Commons API error: {payload['error']}", + url=url, + source_id=self.source_id, + ) + pages = (payload.get("query") or {}).get("pages") or [] + candidates: list[SourceCandidate] = [] + for page in pages: + infos = page.get("imageinfo") or [] + if not infos: + continue + info = infos[0] + file_url = info.get("url") + if not file_url: + continue + candidates.append( + self._candidate( + file_url=file_url, + mime_type=info.get("mime"), + title=page.get("title"), + page_url=info.get("descriptionurl"), + width=info.get("width"), + height=info.get("height"), + extra={"thumb_url": info.get("thumburl")} if info.get("thumburl") else None, + ) + ) + if len(candidates) >= limit: + break + return candidates[:limit] + + +class OpenverseAdapter(_BaseAdapter): + """Openverse search through its anonymous public images endpoint.""" + + source_id = "openverse" + api_host = OPENVERSE_API_HOST + + def build_search_url(self, topic: str, limit: int) -> str: + """Construct the Openverse image search URL for *topic* and *limit*.""" + + topic, limit = normalize_search_args(topic, limit) + query = urllib.parse.urlencode({"q": topic, "page_size": str(limit)}) + return f"https://{self.api_host}/v1/images/?{query}" + + def search(self, topic: str, limit: int) -> list[SourceCandidate]: + url = self.build_search_url(topic, limit) + payload = self.client.get_json(url, source_id=self.source_id) + if not isinstance(payload, Mapping): + raise SourcePayloadError(url, source_id=self.source_id, reason="expected a JSON object") + results = payload.get("results") + if results is None: + raise SourcePayloadError(url, source_id=self.source_id, reason="missing 'results'") + candidates: list[SourceCandidate] = [] + for item in results: + if not isinstance(item, Mapping): + continue + original_url = item.get("url") + file_url = item.get("thumbnail") or original_url + if not file_url: + continue + candidates.append( + self._candidate( + file_url=file_url, + # Origin files may live on third-party hosts; the client + # still refuses them at fetch time with a typed error. + validate=is_approved_url(file_url, allowed=self.client.allowed_hosts), + mime_type=item.get("filetype"), + title=item.get("title"), + page_url=item.get("foreign_landing_url") or item.get("detail_url"), + width=item.get("width"), + height=item.get("height"), + extra={ + "license": item.get("license"), + "creator": item.get("creator"), + "original_url": original_url, + }, + ) + ) + if len(candidates) >= limit: + break + return candidates[:limit] + + +class SourceRegistry: + """Ordered plugin registry of credential-free image sources.""" + + def __init__(self, adapters: Iterable[SourceAdapter] | None = None) -> None: + self._adapters: dict[str, SourceAdapter] = {} + for adapter in adapters or (): + self.register(adapter) + + # -- registration --------------------------------------------------- + def register(self, adapter: SourceAdapter, *, replace: bool = False) -> SourceAdapter: + """Add *adapter*; a duplicate id raises :class:`DuplicateSourceError`.""" + + source_id = getattr(adapter, "source_id", None) + if not isinstance(source_id, str) or not source_id: + raise SourceError("adapter must expose a non-empty string 'source_id'") + if source_id in self._adapters and not replace: + raise DuplicateSourceError(source_id) + self._adapters[source_id] = adapter + return adapter + + def unregister(self, source_id: str) -> SourceAdapter: + if source_id not in self._adapters: + raise UnknownSourceError(source_id, self.available_sources()) + return self._adapters.pop(source_id) + + # -- lookup --------------------------------------------------------- + def available_sources(self) -> list[str]: + """Sorted ids of every registered source.""" + + return sorted(self._adapters) + + def __contains__(self, source_id: object) -> bool: + return isinstance(source_id, str) and source_id in self._adapters + + def __len__(self) -> int: + return len(self._adapters) + + def __iter__(self): + return iter(self.available_sources()) + + def get(self, source_id: str) -> SourceAdapter: + """Return the adapter for *source_id* or raise a listing error.""" + + try: + return self._adapters[source_id] + except (KeyError, TypeError): + raise UnknownSourceError(source_id, self.available_sources()) from None + + def create(self, source_id: str) -> SourceAdapter: + """Explicit alias of :meth:`get` used by the CLI layer.""" + + return self.get(source_id) + + # -- search --------------------------------------------------------- + def search(self, source_id: str, topic: str, limit: int = DEFAULT_LIMIT) -> list[SourceCandidate]: + """Search *source_id* for *topic*, returning at most *limit* candidates.""" + + adapter = self.get(source_id) + topic, limit = normalize_search_args(topic, limit) + return list(adapter.search(topic, limit)) + + def __repr__(self) -> str: # pragma: no cover - trivial + return f"SourceRegistry({self.available_sources()!r})" + + + + +def build_registry( + *, + client: HTTPSClient | None = None, + adapters: Iterable[SourceAdapter] | None = None, +) -> SourceRegistry: + """Build the default registry (Wikimedia Commons + Openverse).""" + + shared_client = client or HTTPSClient() + if adapters is None: + adapters = ( + WikimediaCommonsAdapter(shared_client), + OpenverseAdapter(shared_client), + ) + return SourceRegistry(adapters) + + +def default_registry(client: HTTPSClient | None = None) -> SourceRegistry: + """Convenience wrapper returning a fresh default registry.""" + + return build_registry(client=client) + + +def search( + topic: str, + *, + source: str = "wikimedia_commons", + limit: int = DEFAULT_LIMIT, + registry: SourceRegistry | None = None, + client: HTTPSClient | None = None, +) -> list[SourceCandidate]: + """Search *source* for *topic* using *registry* (default registry by default).""" + + active = registry if registry is not None else build_registry(client=client) + return active.search(source, topic, limit) -- 2.54.0 From f20e58e85cec31cdfba1c69d5a7edf89e19cf384 Mon Sep 17 00:00:00 2001 From: Tool Backend Date: Tue, 3 Nov 2026 01:48:00 +0000 Subject: [PATCH 06/10] change: Implement 3: Complete the native-process CLI, JSON request handling, status vocabulary and exit codes Completed the native-process CLI for `python -m tools.example`. Added `tools/envelope.py` with the fixed status vocabulary (`ok`, `dry_run`, `invalid_request`, `no_format`, `no_size`, `access_error`, `partial_download`) mapped to exit codes 0/0/2/3/4/5/6 and the `Response`/`ResponseBuilder` JSON envelope (keys: status, source, request, items, error). Extended `tools/errors.py` with request errors (`RequestError`, `RequestDocumentError`, `RequestFieldError`, `UnsupportedFormatError`) and `ACCEPTED_FORMATS`. Rewrote `tools/example.py` as a plain executable CLI accepting either a JSON request document or flags (--topic, --width, --height, --format png|ico, --source, --count, --download, --output-dir, --resize, --dry-run), with a custom parser that turns usage errors into `invalid_request` envelopes, unknown sources reported together with the available source ids, unsupported formats rejected with the accepted values, and format/size filtering, download and partial-download handling mapped onto the shared status table. `examples/input.json` is now the canonical offline dry_run request, and `tests/test_cli.py` (new) plus a rewritten `tests/test_example.py` cover the status/exit-code table, single-JSON-document output, help text, unknown source/unsupported format rejection and the download branches. Tests: Facade checks run from /workspace with the prepared .venv: `python -m compileall tools tests` (tools.validate) OK; `pytest` (tools.test) 76 passed; `python -m tools.example examples/input.json` (tools.smoke) exit 0 with a single `dry_run` JSON document; `python -m tools.example --help` exits 0 and documents every parameter plus the JSON document contract. Origin target branch `origin/task/1-task` (1f929c5) is an ancestor of the work branch, so the target is integrated and the checks above ran on the integrated revision. No dependency changes were needed, so no prepare_environment update was required. Progress: Implemented envelope status/exit-code table and response builder, request errors, the full flag/JSON-document CLI in tools/example.py, canonical offline examples/input.json, and CLI tests; facade validate/test/smoke and --help all pass (76 tests). Published by developer-agent through Tool Backend. Closes #5 --- .agent-work/ledger.json | 5 + examples/input.json | 16 +- tests/test_cli.py | 342 +++++++++++++++++++++ tests/test_example.py | 116 +++---- tools/envelope.py | 263 ++++++++++++++++ tools/errors.py | 74 +++++ tools/example.py | 647 ++++++++++++++++++++++++++++++---------- 7 files changed, 1244 insertions(+), 219 deletions(-) create mode 100644 tests/test_cli.py create mode 100644 tools/envelope.py diff --git a/.agent-work/ledger.json b/.agent-work/ledger.json index 8240ce2..bfda518 100644 --- a/.agent-work/ledger.json +++ b/.agent-work/ledger.json @@ -9,6 +9,11 @@ "events": [ "Implemented tools/errors.py, tools/sources.py and tests/test_sources.py for the credential-free source registry and HTTPS fetch layer. Verified with the facade commands tools.validate, tools.test (54 passed) and tools.smoke, all green on the integrated branch (target task/1-task was already at HEAD). No protected facade paths touched." ] + }, + "projects/product-54354138-1#5": { + "events": [ + "Implemented envelope status/exit-code table and response builder, request errors, the full flag/JSON-document CLI in tools/example.py, canonical offline examples/input.json, and CLI tests; facade validate/test/smoke and --help all pass (76 tests)." + ] } } } diff --git a/examples/input.json b/examples/input.json index 6ed9590..b797f90 100644 --- a/examples/input.json +++ b/examples/input.json @@ -1,9 +1,13 @@ { "version": 1, - "mode": "validate", - "dry_run": true, - "command": "example", - "arguments": { - "request": "examples/input.json" - } + "topic": "red fox in the snow", + "width": 640, + "height": 480, + "format": "png", + "source": "wikimedia_commons", + "count": 5, + "download": false, + "output_dir": "downloads", + "resize": false, + "dry_run": true } diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..b86138d --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,342 @@ +"""Contract tests for the native-process CLI in :mod:`tools.example`.""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path +from typing import Any, Sequence + +import pytest + +from tools import envelope, example +from tools.errors import SourceAccessError + +REPO_ROOT = Path(__file__).resolve().parents[1] +EXAMPLE_REQUEST = REPO_ROOT / "examples" / "input.json" + + +def run_cli(*args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, "-m", "tools.example", *args], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + + +def single_document(result: subprocess.CompletedProcess[str]) -> dict[str, Any]: + """Assert stdout holds exactly one machine-readable JSON document.""" + + lines = [line for line in result.stdout.splitlines() if line.strip()] + assert len(lines) == 1, result.stdout + assert result.stderr == "" + document = json.loads(lines[0]) + assert isinstance(document, dict) + assert set(document) == {"status", "source", "request", "items", "error"} + return document + + +def test_status_vocabulary_and_exit_codes_are_fixed() -> None: + assert envelope.STATUS_EXIT_CODES == { + "ok": 0, + "dry_run": 0, + "invalid_request": 2, + "no_format": 3, + "no_size": 4, + "access_error": 5, + "partial_download": 6, + } + assert envelope.STATUS_VOCABULARY == ( + "ok", + "dry_run", + "invalid_request", + "no_format", + "no_size", + "access_error", + "partial_download", + ) + + +def test_help_documents_every_parameter() -> None: + result = run_cli("--help") + + assert result.returncode == 0 + for fragment in ( + "REQUEST.json", + "--topic", + "--width", + "--height", + "--format", + "--source", + "--count", + "--download", + "--output-dir", + "--resize", + "--dry-run", + "png|ico", + "JSON", + ): + assert fragment in result.stdout, fragment + + +def test_example_request_document_is_canonical_dry_run() -> None: + document = json.loads(EXAMPLE_REQUEST.read_text(encoding="utf-8")) + assert document["dry_run"] is True + assert document["format"] in ("png", "ico") + + result = run_cli("examples/input.json") + + assert result.returncode == envelope.EXIT_OK + payload = single_document(result) + assert payload["status"] == envelope.STATUS_DRY_RUN + assert payload["source"] == document["source"] + assert payload["items"] == [] + assert payload["error"] is None + assert payload["request"]["topic"] == document["topic"] + assert payload["request"]["format"] == document["format"] + + +def test_flags_dry_run_emits_one_document() -> None: + result = run_cli("--topic", "fox", "--format", "png", "--width", "640", "--dry-run") + + assert result.returncode == envelope.EXIT_OK + payload = single_document(result) + assert payload["status"] == envelope.STATUS_DRY_RUN + assert payload["request"]["width"] == 640 + assert payload["request"]["dry_run"] is True + + +def test_unsupported_format_is_invalid_request() -> None: + result = run_cli("--topic", "fox", "--format", "gif") + + assert result.returncode == envelope.EXIT_INVALID_REQUEST + payload = single_document(result) + assert payload["status"] == envelope.STATUS_INVALID_REQUEST + assert payload["error"]["code"] == "request_format_unsupported" + assert payload["error"]["details"]["accepted_formats"] == ["png", "ico"] + + +def test_unknown_source_lists_available_ids() -> None: + result = run_cli("--topic", "fox", "--format", "png", "--source", "nope") + + assert result.returncode == envelope.EXIT_INVALID_REQUEST + payload = single_document(result) + assert payload["status"] == envelope.STATUS_INVALID_REQUEST + assert payload["error"]["code"] == "source_unknown" + available = payload["error"]["details"]["available_sources"] + assert "nope" not in available + assert "wikimedia_commons" in available + assert len(available) >= 2 + + +def test_no_request_is_invalid_request() -> None: + result = run_cli() + + assert result.returncode == envelope.EXIT_INVALID_REQUEST + payload = single_document(result) + assert payload["status"] == envelope.STATUS_INVALID_REQUEST + + +def test_document_combined_with_flags_is_invalid_request() -> None: + result = run_cli("examples/input.json", "--topic", "fox") + + assert result.returncode == envelope.EXIT_INVALID_REQUEST + payload = single_document(result) + assert payload["error"]["details"]["conflicting_flags"] == ["topic"] + + +@pytest.mark.parametrize( + "arguments", + [ + ("--format", "png"), + ("--topic", "fox"), + ("--topic", "fox", "--format", "png", "--count", "0"), + ("--topic", " ", "--format", "png"), + ], +) +def test_incomplete_requests_are_invalid(arguments: Sequence[str]) -> None: + result = run_cli(*arguments) + + assert result.returncode == envelope.EXIT_INVALID_REQUEST + assert single_document(result)["status"] == envelope.STATUS_INVALID_REQUEST + + +def test_missing_document_file_is_invalid_request(tmp_path: Path) -> None: + result = run_cli(str(tmp_path / "absent.json")) + + assert result.returncode == envelope.EXIT_INVALID_REQUEST + assert single_document(result)["status"] == envelope.STATUS_INVALID_REQUEST + + +def test_malformed_document_is_invalid_request(tmp_path: Path) -> None: + broken = tmp_path / "broken.json" + broken.write_text("{not json", encoding="utf-8") + + result = run_cli(str(broken)) + + assert result.returncode == envelope.EXIT_INVALID_REQUEST + assert single_document(result)["status"] == envelope.STATUS_INVALID_REQUEST + + +class FakeCandidate: + """Minimal stand-in for :class:`tools.sources.SourceCandidate`.""" + + def __init__( + self, + title: str, + file_url: str, + mime_type: str, + width: int | None, + height: int | None, + *, + payload: bytes = b"payload", + fail: bool = False, + ) -> None: + self.source_id = "fake" + self.title = title + self.file_url = file_url + self.page_url = file_url + ".html" + self.mime_type = mime_type + self.width = width + self.height = height + self.license = "CC0" + self.creator = "tester" + self._payload = payload + self._fail = fail + + def to_dict(self) -> dict[str, Any]: + return { + "source_id": self.source_id, + "title": self.title, + "file_url": self.file_url, + "page_url": self.page_url, + "mime_type": self.mime_type, + "width": self.width, + "height": self.height, + } + + def fetch_bytes(self) -> bytes: + if self._fail: + raise SourceAccessError("download refused", url=self.file_url) + return self._payload + + +class FakeRegistry: + def __init__(self, candidates: Sequence[FakeCandidate] = (), error: Exception | None = None) -> None: + self._candidates = list(candidates) + self._error = error + self.calls: list[tuple[str, str, int]] = [] + + def available_sources(self) -> Sequence[str]: + return ["fake"] + + def search(self, source_id: str, topic: str, limit: int = 10) -> list[FakeCandidate]: + self.calls.append((source_id, topic, limit)) + if self._error is not None: + raise self._error + return self._candidates[:limit] + + + + +def run_main(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], registry: FakeRegistry, *args: str) -> tuple[int, dict[str, Any]]: + monkeypatch.setattr(example, "default_registry", lambda: registry) + code = example.main(list(args)) + captured = capsys.readouterr() + assert captured.err == "" + lines = [line for line in captured.out.splitlines() if line.strip()] + assert len(lines) == 1, captured.out + return code, json.loads(lines[0]) + + +BASE_ARGS = ("--topic", "fox", "--format", "png", "--width", "100", "--height", "100", "--source", "fake") + + +def test_ok_lists_items_without_download(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: + registry = FakeRegistry([FakeCandidate("fox one", "https://example.test/a.png", "image/png", 100, 100)]) + + code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS) + + assert code == envelope.EXIT_OK + assert payload["status"] == envelope.STATUS_OK + assert payload["source"] == "fake" + assert [item["file_url"] for item in payload["items"]] == ["https://example.test/a.png"] + assert payload["items"][0]["width"] == 100 + assert registry.calls == [("fake", "fox", 10)] + + +def test_no_format_when_source_has_no_matching_mime(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: + registry = FakeRegistry([FakeCandidate("fox", "https://example.test/a.jpg", "image/jpeg", 100, 100)]) + + code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS) + + assert code == envelope.EXIT_NO_FORMAT + assert payload["status"] == envelope.STATUS_NO_FORMAT + + +def test_no_size_when_only_smaller_images_exist(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: + registry = FakeRegistry([FakeCandidate("fox", "https://example.test/a.png", "image/png", 50, 50)]) + + code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS) + + assert code == envelope.EXIT_NO_SIZE + assert payload["status"] == envelope.STATUS_NO_SIZE + + +def test_resize_accepts_larger_images(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: + registry = FakeRegistry([FakeCandidate("fox", "https://example.test/a.png", "image/png", 400, 300)]) + + code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS, "--resize") + + assert code == envelope.EXIT_OK + assert payload["request"]["resize"] is True + assert len(payload["items"]) == 1 + + +def test_download_success_writes_files(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path) -> None: + registry = FakeRegistry([FakeCandidate("fox", "https://example.test/a.png", "image/png", 100, 100)]) + + code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS, "--download", "--output-dir", str(tmp_path)) + + assert code == envelope.EXIT_OK + download = payload["items"][0]["download"] + assert download["ok"] is True + target = Path(download["path"]) + assert target.parent == tmp_path + assert target.read_bytes() == b"payload" + + +def test_partial_download_reports_status(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path) -> None: + registry = FakeRegistry( + [ + FakeCandidate("fox", "https://example.test/a.png", "image/png", 100, 100), + FakeCandidate("fox2", "https://example.test/b.png", "image/png", 100, 100, fail=True), + ] + ) + + code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS, "--count", "2", "--download", "--output-dir", str(tmp_path)) + + assert code == envelope.EXIT_PARTIAL_DOWNLOAD + assert payload["status"] == envelope.STATUS_PARTIAL_DOWNLOAD + + +def test_all_downloads_failing_is_access_error(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path) -> None: + registry = FakeRegistry([FakeCandidate("fox", "https://example.test/a.png", "image/png", 100, 100, fail=True)]) + + code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS, "--download", "--output-dir", str(tmp_path)) + + assert code == envelope.EXIT_ACCESS_ERROR + assert payload["status"] == envelope.STATUS_ACCESS_ERROR + + +def test_search_access_error_maps_to_access_error(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: + registry = FakeRegistry(error=SourceAccessError("host unreachable", url="https://example.test")) + + code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS) + + assert code == envelope.EXIT_ACCESS_ERROR + assert payload["status"] == envelope.STATUS_ACCESS_ERROR + assert payload["error"]["code"] == "source_access" diff --git a/tests/test_example.py b/tests/test_example.py index 1574b48..46cd291 100644 --- a/tests/test_example.py +++ b/tests/test_example.py @@ -1,4 +1,4 @@ -"""Tests for the placeholder toolkit CLI and its request document.""" +"""End-to-end tests for the example image-search CLI and its request document.""" from __future__ import annotations @@ -9,7 +9,7 @@ from pathlib import Path import pytest -from tools import example +from tools import envelope, example REPO_ROOT = Path(__file__).resolve().parents[1] EXAMPLE_REQUEST = REPO_ROOT / "examples" / "input.json" @@ -26,43 +26,58 @@ def run_cli(*args: str) -> subprocess.CompletedProcess[str]: ) -def test_example_request_document_is_valid_json_object() -> None: +def single_document(stdout: str) -> dict: + lines = [line for line in stdout.splitlines() if line.strip()] + assert len(lines) == 1, stdout + document = json.loads(lines[0]) + assert isinstance(document, dict) + return document + + +def test_example_request_document_is_canonical_dry_run_request() -> None: document = json.loads(EXAMPLE_REQUEST.read_text(encoding="utf-8")) assert isinstance(document, dict) - assert set(example.REQUIRED_FIELDS) <= set(document) + assert set(example.REQUEST_FIELDS) <= set(document) assert document["dry_run"] is True - assert document["mode"] == "validate" + assert document["format"] in example.ACCEPTED_FORMATS + assert isinstance(document["topic"], str) and document["topic"].strip() + assert document["source"] in example.default_registry().available_sources() -def test_example_request_document_passes_validation() -> None: - request = example.validate_request(example.load_request(EXAMPLE_REQUEST)) +def test_example_request_document_passes_normalization() -> None: + document = json.loads(EXAMPLE_REQUEST.read_text(encoding="utf-8")) + + request = example.normalize_request(document) - assert request["mode"] == "validate" assert request["dry_run"] is True - assert request["arguments"]["request"] == "examples/input.json" + assert request["format"] == "png" + assert request["source"] == "wikimedia_commons" + assert request["count"] == document["count"] def test_cli_prints_single_json_envelope_and_exits_zero() -> None: result = run_cli("examples/input.json") - assert result.returncode == 0, result.stderr + assert result.returncode == envelope.EXIT_OK, result.stderr assert result.stderr == "" - lines = [line for line in result.stdout.splitlines() if line.strip()] - assert len(lines) == 1 - - envelope = json.loads(lines[0]) - assert envelope == { - "status": "ok", - "command": "example", - "mode": "validate", - "dry_run": True, - "version": 1, - "request": "examples/input.json", - } + response = single_document(result.stdout) + assert response["status"] == envelope.STATUS_DRY_RUN + assert response["source"] == "wikimedia_commons" + assert response["items"] == [] + assert response["request"]["dry_run"] is True + assert response["error"] is None -def test_cli_help_exits_zero() -> None: +def test_cli_envelope_is_deterministic() -> None: + first = run_cli("examples/input.json") + second = run_cli("examples/input.json") + + assert first.returncode == second.returncode == envelope.EXIT_OK + assert first.stdout == second.stdout + + +def test_cli_help_exits_zero_and_documents_parameters() -> None: result = run_cli("--help") assert result.returncode == 0 @@ -74,10 +89,10 @@ def test_cli_reports_missing_request_document(tmp_path: Path) -> None: result = run_cli(str(missing)) - assert result.returncode == example.EXIT_INVALID_REQUEST - envelope = json.loads(result.stdout) - assert envelope["status"] == "error" - assert envelope["error"]["code"] == "request_not_found" + assert result.returncode == envelope.EXIT_INVALID_REQUEST + response = single_document(result.stdout) + assert response["status"] == envelope.STATUS_INVALID_REQUEST + assert response["error"]["code"] == "request_document" def test_cli_reports_malformed_json(tmp_path: Path) -> None: @@ -86,33 +101,32 @@ def test_cli_reports_malformed_json(tmp_path: Path) -> None: result = run_cli(str(malformed)) - assert result.returncode == example.EXIT_INVALID_REQUEST - assert json.loads(result.stdout)["error"]["code"] == "request_not_json" + assert result.returncode == envelope.EXIT_INVALID_REQUEST + response = single_document(result.stdout) + assert response["status"] == envelope.STATUS_INVALID_REQUEST + assert response["error"]["code"] == "request_document" + + +def test_cli_reports_non_object_document(tmp_path: Path) -> None: + payload = tmp_path / "list.json" + payload.write_text("[]", encoding="utf-8") + + result = run_cli(str(payload)) + + assert result.returncode == envelope.EXIT_INVALID_REQUEST + assert single_document(result.stdout)["status"] == envelope.STATUS_INVALID_REQUEST @pytest.mark.parametrize( - ("document", "code"), + "document", [ - ([], "request_not_object"), - ({"mode": "validate", "dry_run": True, "command": "example", "arguments": {}}, "request_field_missing"), - ({"version": 1, "mode": "validate", "dry_run": False, "command": "example", "arguments": {"request": "x"}}, "request_not_dry_run"), - ({"version": 1, "mode": "execute", "dry_run": True, "command": "example", "arguments": {"request": "x"}}, "request_mode_unsupported"), - ({"version": 1, "mode": "validate", "dry_run": True, "command": "example", "arguments": {}}, "request_field_missing"), - ({"version": 1, "mode": "validate", "dry_run": True, "command": "", "arguments": {"request": "x"}}, "request_field_invalid"), + {}, + {"topic": "red fox"}, + {"topic": "", "format": "png"}, + {"topic": "red fox", "format": "gif"}, + {"topic": "red fox", "format": "png", "count": 0}, ], ) -def test_validate_request_rejects_invalid_documents(document: object, code: str) -> None: - with pytest.raises(example.RequestError) as excinfo: - example.validate_request(document) - - assert excinfo.value.code == code - - -def test_envelope_is_deterministic() -> None: - request = example.validate_request(example.load_request(EXAMPLE_REQUEST)) - - first = example.build_envelope(request, "examples/input.json") - second = example.build_envelope(request, "examples/input.json") - - assert first == second - assert json.dumps(first, sort_keys=True) == json.dumps(second, sort_keys=True) +def test_normalize_request_rejects_invalid_documents(document: object) -> None: + with pytest.raises(example.RequestError): + example.normalize_request(document) # type: ignore[arg-type] diff --git a/tools/envelope.py b/tools/envelope.py new file mode 100644 index 0000000..8c351f8 --- /dev/null +++ b/tools/envelope.py @@ -0,0 +1,263 @@ +"""Status vocabulary, exit codes and the JSON response envelope. + +Every invocation of ``python -m tools.example`` writes exactly one JSON +document to stdout. The document always carries a ``status`` value from the +fixed vocabulary below and the process exit code is derived from that value, so +a calling AI agent can tell a successful search apart from "nothing matched", +a broken request and an unusable source without parsing prose. +""" + +from __future__ import annotations + +import json +import sys +from dataclasses import dataclass +from typing import Any, Iterable, Mapping, TextIO + +from .errors import ( + RequestError, + SearchRequestError, + SourceAccessError, + ToolkitError, + UnknownSourceError, +) + +__all__ = [ + "EXIT_ACCESS_ERROR", + "EXIT_INVALID_REQUEST", + "EXIT_NO_FORMAT", + "EXIT_NO_SIZE", + "EXIT_OK", + "EXIT_PARTIAL_DOWNLOAD", + "Response", + "ResponseBuilder", + "STATUS_ACCESS_ERROR", + "STATUS_DRY_RUN", + "STATUS_EXIT_CODES", + "STATUS_INVALID_REQUEST", + "STATUS_NO_FORMAT", + "STATUS_NO_SIZE", + "STATUS_OK", + "STATUS_PARTIAL_DOWNLOAD", + "STATUS_VOCABULARY", + "error_fragment", + "exit_code_for", + "is_success", + "status_for_error", +] + +#: Successful search/download. +STATUS_OK = "ok" +#: The request was validated and echoed without contacting a source. +STATUS_DRY_RUN = "dry_run" +#: The request itself is unusable (bad flags, bad document, unknown source or format). +STATUS_INVALID_REQUEST = "invalid_request" +#: The source answered but offers nothing in the requested format. +STATUS_NO_FORMAT = "no_format" +#: The source answered but offers nothing in the requested size. +STATUS_NO_SIZE = "no_size" +#: The source could not be reached, refused to answer or returned unusable data. +STATUS_ACCESS_ERROR = "access_error" +#: Some downloads succeeded and some failed. +STATUS_PARTIAL_DOWNLOAD = "partial_download" + +#: The single source of truth for the status -> exit code mapping. +STATUS_EXIT_CODES: dict[str, int] = { + STATUS_OK: 0, + STATUS_DRY_RUN: 0, + STATUS_INVALID_REQUEST: 2, + STATUS_NO_FORMAT: 3, + STATUS_NO_SIZE: 4, + STATUS_ACCESS_ERROR: 5, + STATUS_PARTIAL_DOWNLOAD: 6, +} + +#: Stable ordering used in help text and documentation. +STATUS_VOCABULARY: tuple[str, ...] = tuple(STATUS_EXIT_CODES) + +EXIT_OK = STATUS_EXIT_CODES[STATUS_OK] +EXIT_INVALID_REQUEST = STATUS_EXIT_CODES[STATUS_INVALID_REQUEST] +EXIT_NO_FORMAT = STATUS_EXIT_CODES[STATUS_NO_FORMAT] +EXIT_NO_SIZE = STATUS_EXIT_CODES[STATUS_NO_SIZE] +EXIT_ACCESS_ERROR = STATUS_EXIT_CODES[STATUS_ACCESS_ERROR] +EXIT_PARTIAL_DOWNLOAD = STATUS_EXIT_CODES[STATUS_PARTIAL_DOWNLOAD] + + +def exit_code_for(status: str) -> int: + """Return the process exit code for ``status``. + + Raises :class:`ValueError` for anything outside the fixed vocabulary, so a + typo can never silently produce an unknown exit code. + """ + + try: + return STATUS_EXIT_CODES[status] + except KeyError: + raise ValueError( + f"unknown status {status!r}; expected one of: {', '.join(STATUS_VOCABULARY)}" + ) from None + + +def is_success(status: str) -> bool: + """True when ``status`` maps onto a zero exit code.""" + + return exit_code_for(status) == 0 + + +def error_fragment( + code: str, + message: str, + *, + details: Mapping[str, Any] | None = None, +) -> dict[str, Any]: + """Build the machine-readable ``error`` fragment of a response.""" + + payload: dict[str, Any] = {"code": str(code), "message": str(message)} + if details: + payload["details"] = dict(details) + return payload + + +def status_for_error(error: BaseException) -> str: + """Map a toolkit error onto the fixed status vocabulary.""" + + if isinstance(error, (RequestError, UnknownSourceError, SearchRequestError)): + return STATUS_INVALID_REQUEST + if isinstance(error, SourceAccessError): + return STATUS_ACCESS_ERROR + if isinstance(error, ToolkitError): + return STATUS_ACCESS_ERROR + return STATUS_ACCESS_ERROR + + +@dataclass(frozen=True) +class Response: + """One complete machine-readable answer of the toolkit.""" + + status: str + source: str | None = None + request: Mapping[str, Any] | None = None + items: tuple[Mapping[str, Any], ...] = () + error: Mapping[str, Any] | None = None + + def __post_init__(self) -> None: + if self.status not in STATUS_EXIT_CODES: + raise ValueError( + f"unknown status {self.status!r}; expected one of: " + f"{', '.join(STATUS_VOCABULARY)}" + ) + if self.error is not None and "code" not in self.error: + raise ValueError("error fragment must carry a 'code'") + + @property + def exit_code(self) -> int: + """Process exit code implied by :attr:`status`.""" + + return exit_code_for(self.status) + + @property + def succeeded(self) -> bool: + """True when the status is one of the zero-exit statuses.""" + + return self.exit_code == 0 + + def to_dict(self) -> dict[str, Any]: + """Return the JSON-ready document with a fixed set of top-level keys.""" + + return { + "status": self.status, + "source": self.source, + "request": dict(self.request) if self.request is not None else None, + "items": [dict(item) for item in self.items], + "error": dict(self.error) if self.error is not None else None, + } + + def to_json(self) -> str: + """Serialise the response as one deterministic JSON line.""" + + return json.dumps(self.to_dict(), sort_keys=True, ensure_ascii=True) + + def emit(self, stream: TextIO | None = None) -> None: + """Write exactly one JSON document to ``stream`` (stdout by default).""" + + print(self.to_json(), file=stream if stream is not None else sys.stdout) + + +class ResponseBuilder: + """Build :class:`Response` objects sharing one source/request context.""" + + def __init__( + self, + *, + source: str | None = None, + request: Mapping[str, Any] | None = None, + ) -> None: + self.source = source + self.request = request + + def with_context( + self, + *, + source: str | None = None, + request: Mapping[str, Any] | None = None, + ) -> "ResponseBuilder": + """Return a copy carrying the supplied context.""" + + return ResponseBuilder( + source=self.source if source is None else source, + request=self.request if request is None else request, + ) + + def build( + self, + status: str, + *, + items: Iterable[Mapping[str, Any]] = (), + error: Mapping[str, Any] | None = None, + ) -> Response: + """Assemble a response for an explicit ``status``.""" + + return Response( + status=status, + source=self.source, + request=self.request, + items=tuple(items), + error=error, + ) + + # -- convenience constructors -------------------------------------- + def ok(self, *, items: Iterable[Mapping[str, Any]] = ()) -> Response: + return self.build(STATUS_OK, items=items) + + def dry_run(self, *, items: Iterable[Mapping[str, Any]] = ()) -> Response: + return self.build(STATUS_DRY_RUN, items=items) + + def invalid_request(self, *, error: Mapping[str, Any] | None = None) -> Response: + return self.build(STATUS_INVALID_REQUEST, error=error) + + def no_format(self, *, error: Mapping[str, Any] | None = None) -> Response: + return self.build(STATUS_NO_FORMAT, error=error) + + def no_size(self, *, error: Mapping[str, Any] | None = None) -> Response: + return self.build(STATUS_NO_SIZE, error=error) + + def access_error( + self, + *, + items: Iterable[Mapping[str, Any]] = (), + error: Mapping[str, Any] | None = None, + ) -> Response: + return self.build(STATUS_ACCESS_ERROR, items=items, error=error) + + def partial_download( + self, + *, + items: Iterable[Mapping[str, Any]] = (), + error: Mapping[str, Any] | None = None, + ) -> Response: + return self.build(STATUS_PARTIAL_DOWNLOAD, items=items, error=error) + + def from_error(self, error: ToolkitError) -> Response: + """Build a response for ``error`` using :func:`status_for_error`.""" + + return self.build(status_for_error(error), error=error.to_dict()) diff --git a/tools/errors.py b/tools/errors.py index 013ae8b..82925e6 100644 --- a/tools/errors.py +++ b/tools/errors.py @@ -185,3 +185,77 @@ class HostNotAllowedError(SourceAccessError): url=str(url), details={"allowed_hosts": list(self.allowed)}, ) + + +# --------------------------------------------------------------------------- +# Request errors: invalid command lines and JSON request documents +# --------------------------------------------------------------------------- + +#: Image formats the CLI accepts. +ACCEPTED_FORMATS: tuple[str, ...] = ("png", "ico") + + +class RequestError(ToolkitError): + """Base class for every unusable request (CLI flags or request document).""" + + code = "request_invalid" + + def __init__( + self, + message: str, + *, + code: str | None = None, + details: Mapping[str, Any] | None = None, + field: str | None = None, + accepted_fields: Iterable[str] | None = None, + accepted_formats: Iterable[str] | None = None, + ) -> None: + merged: dict[str, Any] = dict(details or {}) + if field is not None: + merged["field"] = field + if accepted_fields is not None: + merged["accepted_fields"] = list(accepted_fields) + if accepted_formats is not None: + merged["accepted_formats"] = list(accepted_formats) + super().__init__(message, code=code, details=merged) + self.field = field + self.accepted_fields: tuple[str, ...] = tuple(accepted_fields or ()) + self.accepted_formats: tuple[str, ...] = tuple(accepted_formats or ()) + + +class RequestDocumentError(RequestError): + """Raised when the JSON request document cannot be read or decoded.""" + + code = "request_document" + + +class RequestFieldError(RequestError): + """Raised when a single request field is missing, unknown or malformed.""" + + code = "request_field" + + +class UnsupportedFormatError(RequestError): + """Raised for a format outside :data:`ACCEPTED_FORMATS`.""" + + code = "request_format_unsupported" + + def __init__(self, value: Any, accepted: Iterable[str] = ACCEPTED_FORMATS) -> None: + accepted = tuple(accepted) + self.value = str(value) + super().__init__( + f"unsupported image format {self.value!r}; " + f"accepted formats: {', '.join(accepted)}", + field="format", + accepted_formats=accepted, + details={"requested_format": self.value}, + ) + + +__all__ += [ + "ACCEPTED_FORMATS", + "RequestDocumentError", + "RequestError", + "RequestFieldError", + "UnsupportedFormatError", +] diff --git a/tools/example.py b/tools/example.py index 32d2b87..2f266e3 100644 --- a/tools/example.py +++ b/tools/example.py @@ -1,200 +1,523 @@ -"""Placeholder toolkit entry point that loads and validates a JSON request. - -This increment only establishes the stable command-line surface: read a single -JSON request document, validate its shape, and print one structured JSON -envelope. Later modules extend the same document format and envelope without -changing the invocation contract, so the module stays deterministic and works -fully offline. +"""Plain native-process CLI for the image toolkit. Usage:: python -m tools.example examples/input.json + python -m tools.example --topic "red fox" --width 640 --height 480 --format png + +Exactly one machine-readable JSON document is written to stdout. The process +exit code is derived from the fixed status vocabulary in :mod:`tools.envelope`. """ from __future__ import annotations import argparse import json +import re import sys from pathlib import Path from typing import Any, Mapping, Sequence -__all__ = [ - "EXIT_INVALID_REQUEST", - "EXIT_OK", - "EXIT_USAGE", - "RequestError", - "build_envelope", - "load_request", - "main", - "validate_request", -] +from .envelope import ( + STATUS_ACCESS_ERROR, + STATUS_NO_FORMAT, + STATUS_NO_SIZE, + STATUS_OK, + STATUS_PARTIAL_DOWNLOAD, + Response, + ResponseBuilder, + error_fragment, +) +from .errors import ( + ACCEPTED_FORMATS, + RequestDocumentError, + RequestError, + RequestFieldError, + SourceAccessError, + ToolkitError, + UnknownSourceError, + UnsupportedFormatError, +) +from .sources import DEFAULT_LIMIT, SourceCandidate, default_registry -EXIT_OK = 0 -EXIT_INVALID_REQUEST = 1 -EXIT_USAGE = 2 +PROG = "python -m tools.example" -#: Fields every request document must provide. -REQUIRED_FIELDS = ("version", "mode", "dry_run", "command", "arguments") +#: Keys accepted in a JSON request document (``version`` is optional metadata). +REQUEST_FIELDS: tuple[str, ...] = ( + "topic", + "width", + "height", + "format", + "source", + "count", + "download", + "output_dir", + "resize", + "dry_run", +) -#: Modes understood by this increment of the toolkit. -SUPPORTED_MODES = frozenset({"validate"}) +OPTIONAL_DOCUMENT_FIELDS: tuple[str, ...] = ("version",) -PROG = "tools.example" +DEFAULT_SOURCE = "wikimedia_commons" +DEFAULT_OUTPUT_DIR = "downloads" +MIN_COUNT = 1 +MAX_COUNT = 50 + +#: MIME types (and filename suffixes) that count as a hit for each format. +FORMAT_MATCHERS: dict[str, tuple[str, ...]] = { + "png": ("image/png", ".png"), + "ico": ("image/x-icon", "image/vnd.microsoft.icon", "image/ico", ".ico"), +} -class RequestError(ValueError): - """Raised when a request document is missing or malformed.""" +class _Parser(argparse.ArgumentParser): + """Argument parser that reports bad flags as request errors.""" - def __init__(self, code: str, message: str) -> None: - super().__init__(message) - self.code = code - self.message = message + def error(self, message: str) -> Any: # pragma: no cover - argparse hook + raise RequestError(message, accepted_fields=REQUEST_FIELDS) -def load_request(path: str | Path) -> Any: - """Read and decode the JSON document stored at ``path``.""" - try: - raw = Path(path).read_text(encoding="utf-8") - except FileNotFoundError as exc: - raise RequestError("request_not_found", f"request document not found: {path}") from exc - except IsADirectoryError as exc: - raise RequestError("request_not_a_file", f"request document is not a file: {path}") from exc - except OSError as exc: - raise RequestError("request_unreadable", f"request document could not be read: {path}") from exc - except UnicodeDecodeError as exc: - raise RequestError("request_not_utf8", f"request document is not valid UTF-8: {path}") from exc +def build_parser() -> argparse.ArgumentParser: + """Return the CLI parser; ``--help`` documents every supported parameter.""" - try: - return json.loads(raw) - except json.JSONDecodeError as exc: - raise RequestError( - "request_not_json", - f"request document is not valid JSON: {path} (line {exc.lineno}, column {exc.colno})", - ) from exc - - -def _require(mapping: Mapping[str, Any], key: str, expected: type | tuple[type, ...], label: str) -> Any: - if key not in mapping: - raise RequestError("request_field_missing", f"request field {label} is required") - value = mapping[key] - if isinstance(value, bool) and bool not in ( - expected if isinstance(expected, tuple) else (expected,) - ): - raise RequestError("request_field_invalid", f"request field {label} must be {_describe(expected)}") - if not isinstance(value, expected): - raise RequestError("request_field_invalid", f"request field {label} must be {_describe(expected)}") - return value - - -def _describe(expected: type | tuple[type, ...]) -> str: - names = expected if isinstance(expected, tuple) else (expected,) - return " or ".join(sorted({name.__name__ for name in names})) - - -def validate_request(document: Any) -> dict[str, Any]: - """Validate ``document`` and return the normalised request mapping.""" - if not isinstance(document, dict): - raise RequestError("request_not_object", "request document must be a JSON object") - - for key in REQUIRED_FIELDS: - if key not in document: - raise RequestError("request_field_missing", f"request field '{key}' is required") - - version = _require(document, "version", int, "version") - if version < 1: - raise RequestError("request_field_invalid", "request field 'version' must be a positive integer") - - mode = _require(document, "mode", str, "mode") - if mode not in SUPPORTED_MODES: - supported = ", ".join(sorted(SUPPORTED_MODES)) - raise RequestError( - "request_mode_unsupported", - f"request field 'mode' must be one of: {supported}", - ) - - dry_run = _require(document, "dry_run", bool, "dry_run") - if not dry_run: - raise RequestError( - "request_not_dry_run", - "this increment only supports validate-only dry_run requests", - ) - - command = _require(document, "command", str, "command") - if not command.strip(): - raise RequestError("request_field_invalid", "request field 'command' must not be empty") - - arguments = _require(document, "arguments", dict, "arguments") - request_path = _require(arguments, "request", str, "arguments.request") - if not request_path.strip(): - raise RequestError("request_field_invalid", "request field 'arguments.request' must not be empty") - - return { - "version": version, - "mode": mode, - "dry_run": dry_run, - "command": command, - "arguments": dict(arguments), - } - - -def build_envelope(request: Mapping[str, Any], source: str) -> dict[str, Any]: - """Build the deterministic success envelope for a validated request.""" - return { - "status": "ok", - "command": request["command"], - "mode": request["mode"], - "dry_run": request["dry_run"], - "version": request["version"], - "request": source, - } - - -def build_error_envelope(error: RequestError, source: str) -> dict[str, Any]: - """Build the deterministic failure envelope for an invalid request.""" - return { - "status": "error", - "error": {"code": error.code, "message": error.message}, - "request": source, - } - - -def emit(envelope: Mapping[str, Any]) -> None: - """Print exactly one JSON document to standard output.""" - print(json.dumps(envelope, sort_keys=True, ensure_ascii=True)) - - -def _build_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser( + parser = _Parser( prog=PROG, description=( - "Load and validate a JSON request document. The command is offline " - "and prints one structured JSON envelope." + "Search an image source and report the matching items as one JSON " + "document. Either pass a JSON request document or use the flags." + ), + epilog=( + "Every run writes exactly one JSON document to stdout with the keys " + "status, source, request, items and error." ), ) parser.add_argument( "request", - metavar="REQUEST", - help="path to the JSON request document (for example examples/input.json)", + nargs="?", + metavar="REQUEST.json", + help=( + "path to a JSON request document holding the same fields as the " + "flags; it cannot be combined with any of the flags" + ), + ) + parser.add_argument( + "-t", + "--topic", + metavar="TEXT", + help="search topic (required unless a request document is given)", + ) + parser.add_argument( + "--width", + type=int, + metavar="PX", + help="desired image width in pixels", + ) + parser.add_argument( + "--height", + type=int, + metavar="PX", + help="desired image height in pixels", + ) + parser.add_argument( + "-f", + "--format", + metavar="FMT", + help="image format: " + "|".join(ACCEPTED_FORMATS), + ) + parser.add_argument( + "-s", + "--source", + metavar="ID", + help=f"image source id (default: {DEFAULT_SOURCE}); unknown ids are rejected", + ) + parser.add_argument( + "-c", + "--count", + type=int, + metavar="N", + help=f"how many candidates to request, {MIN_COUNT}-{MAX_COUNT} (default: {DEFAULT_LIMIT})", + ) + parser.add_argument( + "--download", + action="store_true", + help="download every matching image into the output directory", + ) + parser.add_argument( + "-o", + "--output-dir", + metavar="DIR", + help=f"download directory (default: ./{DEFAULT_OUTPUT_DIR})", + ) + parser.add_argument( + "--resize", + action="store_true", + help=( + "accept images larger than the requested size and downscale them; " + "without this flag the size must match exactly" + ), + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="validate the request and emit the envelope without any network access", ) return parser -def main(argv: Sequence[str] | None = None) -> int: - """Run the placeholder CLI and return the process exit code.""" - parser = _build_parser() - args = parser.parse_args(list(sys.argv[1:] if argv is None else argv)) +def _load_document(path: str) -> Mapping[str, Any]: + """Read a JSON request document from ``path``.""" + target = Path(path) try: - document = load_request(args.request) - request = validate_request(document) - except RequestError as error: - emit(build_error_envelope(error, args.request)) - return EXIT_INVALID_REQUEST - - emit(build_envelope(request, args.request)) - return EXIT_OK + text = target.read_text(encoding="utf-8") + except OSError as exc: + raise RequestDocumentError( + f"cannot read request document {path!r}: {exc.strerror or exc}", + details={"path": str(path)}, + ) from exc + try: + payload = json.loads(text) + except json.JSONDecodeError as exc: + raise RequestDocumentError( + f"request document {path!r} is not valid JSON: {exc.msg}", + details={"path": str(path), "line": exc.lineno, "column": exc.colno}, + ) from exc + if not isinstance(payload, Mapping): + raise RequestDocumentError( + f"request document {path!r} must contain a JSON object", + details={"path": str(path), "type": type(payload).__name__}, + ) + return payload -if __name__ == "__main__": - raise SystemExit(main()) +def _document_to_mapping(payload: Mapping[str, Any]) -> dict[str, Any]: + """Reject unknown keys and return the accepted request fields.""" + + accepted = REQUEST_FIELDS + OPTIONAL_DOCUMENT_FIELDS + unknown = sorted(str(key) for key in payload if key not in accepted) + if unknown: + raise RequestFieldError( + "request document contains unsupported fields: " + ", ".join(unknown), + field=unknown[0], + details={"unknown_fields": unknown, "accepted_fields": list(REQUEST_FIELDS)}, + ) + return {key: payload[key] for key in REQUEST_FIELDS if key in payload} + + +def _flags_to_mapping(namespace: argparse.Namespace) -> dict[str, Any]: + """Collect the explicitly supplied flags into a request mapping.""" + + mapping: dict[str, Any] = {} + for field in ("topic", "width", "height", "format", "source", "count", "output_dir"): + value = getattr(namespace, field, None) + if value is not None: + mapping[field] = value + for field in ("download", "resize", "dry_run"): + if getattr(namespace, field, False): + mapping[field] = True + return mapping + + +def _as_text(value: Any, field: str) -> str: + if not isinstance(value, str): + raise RequestFieldError( + f"{field} must be a string, got {type(value).__name__}", field=field + ) + text = value.strip() + if not text: + raise RequestFieldError(f"{field} must not be empty", field=field) + return text + + +def _as_int(value: Any, field: str) -> int: + if isinstance(value, bool) or not isinstance(value, int): + raise RequestFieldError( + f"{field} must be an integer, got {type(value).__name__}", field=field + ) + return int(value) + + +def _as_bool(value: Any, field: str) -> bool: + if not isinstance(value, bool): + raise RequestFieldError( + f"{field} must be a boolean, got {type(value).__name__}", field=field + ) + return bool(value) + + +def normalize_request(raw: Mapping[str, Any]) -> dict[str, Any]: + """Validate ``raw`` and return the complete, typed request mapping.""" + + if "topic" not in raw: + raise RequestFieldError( + "topic is required", + field="topic", + details={"required_fields": ["topic"], "accepted_fields": list(REQUEST_FIELDS)}, + ) + topic = _as_text(raw["topic"], "topic") + + size: dict[str, int | None] = {"width": None, "height": None} + for field in ("width", "height"): + if raw.get(field) is None: + continue + number = _as_int(raw[field], field) + if number <= 0: + raise RequestFieldError(f"{field} must be positive", field=field) + size[field] = number + + if "format" not in raw or raw["format"] is None: + raise RequestFieldError( + "format is required", + field="format", + details={"accepted_formats": list(ACCEPTED_FORMATS)}, + ) + image_format = _as_text(raw["format"], "format").lower() + if image_format not in ACCEPTED_FORMATS: + raise UnsupportedFormatError(image_format) + + source = DEFAULT_SOURCE + if raw.get("source") is not None: + source = _as_text(raw["source"], "source") + + count = DEFAULT_LIMIT + if raw.get("count") is not None: + count = _as_int(raw["count"], "count") + if not MIN_COUNT <= count <= MAX_COUNT: + raise RequestFieldError( + f"count must be between {MIN_COUNT} and {MAX_COUNT}", + field="count", + details={"min": MIN_COUNT, "max": MAX_COUNT}, + ) + + output_dir = DEFAULT_OUTPUT_DIR + if raw.get("output_dir") is not None: + output_dir = _as_text(raw["output_dir"], "output_dir") + + return { + "topic": topic, + "width": size["width"], + "height": size["height"], + "format": image_format, + "source": source, + "count": count, + "download": _as_bool(raw.get("download", False), "download"), + "output_dir": output_dir, + "resize": _as_bool(raw.get("resize", False), "resize"), + "dry_run": _as_bool(raw.get("dry_run", False), "dry_run"), + } + + +def _matches_format(candidate: SourceCandidate, image_format: str) -> bool: + matchers = FORMAT_MATCHERS[image_format] + mime = str(getattr(candidate, "mime_type", None) or "").split(";")[0].strip().lower() + if mime and mime in matchers: + return True + url = str(getattr(candidate, "file_url", "") or "").lower() + return any(url.endswith(suffix) for suffix in matchers if suffix.startswith(".")) + + +def _matches_size(candidate: SourceCandidate, request: Mapping[str, Any]) -> bool: + width = request.get("width") + height = request.get("height") + if width is None and height is None: + return True + actual_width = getattr(candidate, "width", None) + actual_height = getattr(candidate, "height", None) + if isinstance(actual_width, bool) or isinstance(actual_height, bool): + return False + if not isinstance(actual_width, int) or not isinstance(actual_height, int): + return False + if request.get("resize"): + fits_width = width is None or actual_width >= width + fits_height = height is None or actual_height >= height + return fits_width and fits_height + matches_width = width is None or actual_width == width + matches_height = height is None or actual_height == height + return matches_width and matches_height + + +def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -> tuple[list[SourceCandidate], list[SourceCandidate]]: + """Return ``(matching, rejected_by_size)`` split for the request.""" + + by_format = [c for c in candidates if _matches_format(c, request["format"])] + matching = [c for c in by_format if _matches_size(c, request)] + rejected = [c for c in by_format if not _matches_size(c, request)] + return matching, rejected + + +def _slug(candidate: SourceCandidate) -> str: + raw = str(getattr(candidate, "title", None) or getattr(candidate, "source_id", "") or "image").lower() + slug = re.sub(r"[^a-z0-9]+", "-", raw).strip("-") + return (slug or "image")[:60] + + +def _download(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -> tuple[list[dict[str, Any]], int]: + """Download ``candidates``; return ``(items, failure_count)``.""" + + directory = Path(request["output_dir"]).expanduser() + try: + directory.mkdir(parents=True, exist_ok=True) + except OSError as exc: + raise SourceAccessError( + f"cannot create output directory {str(directory)!r}: {exc.strerror or exc}", + details={"output_dir": str(directory)}, + ) from exc + + suffix = "." + str(request["format"]) + items: list[dict[str, Any]] = [] + failures = 0 + for index, candidate in enumerate(candidates, start=1): + record = dict(candidate.to_dict()) + destination = directory / f"{index:03d}-{_slug(candidate)}{suffix}" + try: + payload = candidate.fetch_bytes() + destination.write_bytes(payload) + except (ToolkitError, OSError) as exc: + failures += 1 + record["download"] = { + "ok": False, + "error": error_fragment(getattr(exc, "code", "download_failed"), str(exc)), + } + else: + record["download"] = { + "ok": True, + "path": str(destination), + "bytes": len(payload), + } + items.append(record) + return items, failures + + +def _guess_source(argv: Sequence[str]) -> str | None: + """Best-effort source id extraction for error envelopes.""" + + for index, token in enumerate(argv): + if token == "--source" or token == "-s": + if index + 1 < len(argv): + return argv[index + 1] + elif token.startswith("--source="): + return token.split("=", 1)[1] + return None + + +def _request_from(argv: Sequence[str]) -> tuple[dict[str, Any], str, Any]: + """Turn CLI arguments into ``(normalized_request, source_id, registry)``.""" + + parser = build_parser() + namespace = parser.parse_args(list(argv)) + flags = _flags_to_mapping(namespace) + document = namespace.request + + if document is not None: + if flags: + raise RequestError( + "a request document cannot be combined with flags: " + + ", ".join(sorted(flags)), + details={"conflicting_flags": sorted(flags)}, + accepted_fields=REQUEST_FIELDS, + ) + raw = _document_to_mapping(_load_document(document)) + elif flags: + raw = flags + else: + raise RequestError( + "no request given; pass a JSON request document or at least one flag", + accepted_fields=REQUEST_FIELDS, + ) + + request = normalize_request(raw) + registry = default_registry() + available = list(registry.available_sources()) + if request["source"] not in available: + raise UnknownSourceError(request["source"], available) + return request, request["source"], registry + + +def _execute(argv: Sequence[str]) -> Response: + """Run one request and return the response that must be printed.""" + + request, source, registry = _request_from(argv) + builder = ResponseBuilder(source=source, request=request) + + if request["dry_run"]: + return builder.dry_run() + + candidates = registry.search(source, request["topic"], request["count"]) + matching, rejected = _select(candidates, request) + + if not matching: + details = { + "requested_format": request["format"], + "width": request["width"], + "height": request["height"], + "resize": request["resize"], + "candidate_count": len(candidates), + "format_matches": len(matching) + len(rejected), + "size_matches": 0, + } + if rejected: + return builder.build( + STATUS_NO_SIZE, + error=error_fragment( + "no_size_match", + "no image matched the requested width and height", + details=details, + ), + ) + return builder.build( + STATUS_NO_FORMAT, + error=error_fragment( + "no_format_match", + f"the source returned no {request['format']} images for {request['topic']!r}", + details=details, + ), + ) + + if not request["download"]: + return builder.build(STATUS_OK, items=[c.to_dict() for c in matching]) + + items, failures = _download(matching, request) + if failures and failures == len(items): + return builder.build( + STATUS_ACCESS_ERROR, + items=items, + error=error_fragment( + "download_failed", + "every download failed", + details={"failed": failures, "total": len(items)}, + ), + ) + if failures: + return builder.build( + STATUS_PARTIAL_DOWNLOAD, + items=items, + error=error_fragment( + "download_incomplete", + f"{failures} of {len(items)} downloads failed", + details={"failed": failures, "total": len(items)}, + ), + ) + return builder.build(STATUS_OK, items=items) + + +def main(argv: Sequence[str] | None = None) -> int: + """Entry point: emit exactly one JSON document and return the exit code.""" + + arguments = list(sys.argv[1:] if argv is None else argv) + try: + response = _execute(arguments) + except ToolkitError as error: + builder = ResponseBuilder(source=_guess_source(arguments)) + response = builder.from_error(error) + except OSError as error: + builder = ResponseBuilder(source=_guess_source(arguments)) + response = builder.build( + STATUS_ACCESS_ERROR, + error=error_fragment("access_error", str(error)), + ) + response.emit() + return response.exit_code + + +if __name__ == "__main__": # pragma: no cover - module entry point + sys.exit(main()) -- 2.54.0 From b544968cb8188439c85055ac750101f7ab5109e1 Mon Sep 17 00:00:00 2001 From: Tool Backend Date: Tue, 3 Nov 2026 02:11:36 +0000 Subject: [PATCH 07/10] change: Implement 4: Filter candidates by real format and minimum size from downloaded bytes Added tools/images.py with stdlib-only detection of the real image format and pixel dimensions from downloaded bytes (PNG signature + IHDR chunk, ICO ICONDIR/ICONDIRENTRY directory including the 0-means-256 encoding) plus stdlib-only builders (png_bytes, ico_bytes) so tests/sources can produce genuine payloads. Wired it into the CLI search path in tools/example.py: a bounded candidate window derived from --count (candidate_cap = count * 4, capped at 50) is fetched from the selected source, each payload is parsed, and only items whose actual format equals the requested png/ico and whose actual width and height are both at least the requested minimum are kept. Kept items report source, url, format and the actual width/height (plus size_bytes and metadata as extras); verified bytes are reused for --download instead of being fetched twice. Statuses stay distinct: ok (0), no_format (3) when hits were only other formats, no_size (4) when format-matching items were all too small, access_error (5) when bytes could not be fetched or are not readable images, partial_download (6) when some candidates could not be read. All metadata-based filtering (FORMAT_MATCHERS, _matches_format, _matches_size) was removed so only parsed bytes decide. Added tests/test_images.py with detector/builder unit tests and CLI tests using an injected fake byte-fetch callable (no network), and updated tests/test_cli.py fixtures to real image bytes where the previous metadata-only assumption no longer holds. Tests: Facade commands run with .venv/bin on PATH after prepare_environment(component=tools, mode=restore): `python -m compileall tools tests` (tools.validate) succeeded; `pytest` (tools.test) => 114 passed, including the new tests/test_images.py (31 tests); `python -m tools.example examples/input.json` (tools.smoke) exited 0 with a single dry_run JSON document; acceptance commands `python -m tools.example --help` exited 0 and `import tools` resolved. Target branch task/1-task is at the same commit as this work branch (git rev-list count 0 ahead / 0 behind), so it is already integrated; facade/protected paths untouched. Progress: Implemented byte-based format and minimum-size filtering for the image search path. New tools/images.py: detect_format (png/ico plus jpeg/gif/bmp/webp so "other format" is distinguishable from "unreadable"), read_png_dimensions (IHDR), read_ico_dimensions (ICONDIR entries, 0 => 256, largest entry), inspect_image/ImageInfo, ImageFormatError (code image_unreadable), and png_bytes/ico_bytes builders. tools/example.py: candidate_cap(count) bounded window (count*4, max 50) requested from the source, _select() fetches and parses every candidate, KeptImage.to_item() reports source/url/format/actual width+height/size_bytes, _download() writes the already verified bytes, and _execute() distinguishes ok/no_format/no_size/access_error/partial_download with the existing exit codes; metadata-based filtering removed. tests/test_images.py added (31 tests) and tests/test_cli.py fixtures aligned to real image bytes. All facade commands pass: compileall, pytest (114 passed), smoke, help, package import. Published by developer-agent through Tool Backend. Closes #6 --- .agent-work/ledger.json | 5 + tests/test_cli.py | 59 ++++-- tests/test_images.py | 457 ++++++++++++++++++++++++++++++++++++++++ tools/example.py | 279 ++++++++++++++++++------ tools/images.py | 367 ++++++++++++++++++++++++++++++++ 5 files changed, 1093 insertions(+), 74 deletions(-) create mode 100644 tests/test_images.py create mode 100644 tools/images.py diff --git a/.agent-work/ledger.json b/.agent-work/ledger.json index 774c236..4df4438 100644 --- a/.agent-work/ledger.json +++ b/.agent-work/ledger.json @@ -15,6 +15,11 @@ "Implemented envelope status/exit-code table and response builder, request errors, the full flag/JSON-document CLI in tools/example.py, canonical offline examples/input.json, and CLI tests; facade validate/test/smoke and --help all pass (76 tests).", "Addressed the changes-requested review on PR #13: requested width/height are now a minimum target size (selection no longer depends on --resize), and --resize now actually downscales downloaded images (stdlib PNG codec + ICO/PNG container support, shrink-to-fit, never upscaling) with per-item resize metadata and typed ImageResizeError for undecodable payloads. Updated tests/test_cli.py with minimum-size and offline resize/download contract tests (83 passed). Facade validate/test/smoke/help all green; examples/input.json stays the canonical dry_run request." ] + }, + "projects/product-54354138-1#6": { + "events": [ + "Implemented byte-based format and minimum-size filtering for the image search path. New tools/images.py: detect_format (png/ico plus jpeg/gif/bmp/webp so \"other format\" is distinguishable from \"unreadable\"), read_png_dimensions (IHDR), read_ico_dimensions (ICONDIR entries, 0 => 256, largest entry), inspect_image/ImageInfo, ImageFormatError (code image_unreadable), and png_bytes/ico_bytes builders. tools/example.py: candidate_cap(count) bounded window (count*4, max 50) requested from the source, _select() fetches and parses every candidate, KeptImage.to_item() reports source/url/format/actual width+height/size_bytes, _download() writes the already verified bytes, and _execute() distinguishes ok/no_format/no_size/access_error/partial_download with the existing exit codes; metadata-based filtering removed. tests/test_images.py added (31 tests) and tests/test_cli.py fixtures aligned to real image bytes. All facade commands pass: compileall, pytest (114 passed), smoke, help, package import." + ] } } } diff --git a/tests/test_cli.py b/tests/test_cli.py index bd7f362..5758d42 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -194,7 +194,7 @@ class FakeCandidate: width: int | None, height: int | None, *, - payload: bytes = b"payload", + payload: bytes | None = None, fail: bool = False, ) -> None: self.source_id = "fake" @@ -223,7 +223,11 @@ class FakeCandidate: def fetch_bytes(self) -> bytes: if self._fail: raise SourceAccessError("download refused", url=self.file_url) - return self._payload + if self._payload is not None: + return self._payload + # Default to bytes that really are a PNG of the advertised size, because + # the CLI filters on downloaded bytes and never on source metadata. + return png_bytes(self.width or 1, self.height or 1) class FakeRegistry: @@ -265,18 +269,29 @@ def test_ok_lists_items_without_download(monkeypatch: pytest.MonkeyPatch, capsys assert code == envelope.EXIT_OK assert payload["status"] == envelope.STATUS_OK assert payload["source"] == "fake" - assert [item["file_url"] for item in payload["items"]] == ["https://example.test/a.png"] - assert payload["items"][0]["width"] == 100 - assert registry.calls == [("fake", "fox", 10)] + item = payload["items"][0] + assert item["url"] == "https://example.test/a.png" + assert item["file_url"] == "https://example.test/a.png" + assert item["source"] == "fake" + assert item["format"] == "png" + assert item["width"] == 100 + assert item["height"] == 100 + assert registry.calls == [("fake", "fox", example.candidate_cap(10))] -def test_no_format_when_source_has_no_matching_mime(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: - registry = FakeRegistry([FakeCandidate("fox", "https://example.test/a.jpg", "image/jpeg", 100, 100)]) +def test_no_format_when_every_payload_really_is_another_format( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + registry = FakeRegistry( + [FakeCandidate("fox", "https://example.test/a.jpg", "image/jpeg", 100, 100, payload=gif_bytes())] + ) code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS) assert code == envelope.EXIT_NO_FORMAT assert payload["status"] == envelope.STATUS_NO_FORMAT + assert payload["error"]["details"]["unreadable_candidates"] == 0 + assert payload["error"]["details"]["requested_format"] == "png" def test_no_size_when_only_smaller_images_exist(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: @@ -311,7 +326,7 @@ def test_exact_size_is_accepted(monkeypatch: pytest.MonkeyPatch, capsys: pytest. assert len(payload["items"]) == 1 -def test_resize_without_download_keeps_reported_source_size( +def test_resize_without_download_keeps_actual_size( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: registry = FakeRegistry([FakeCandidate("fox", "https://example.test/a.png", "image/png", 400, 300)]) @@ -325,7 +340,10 @@ def test_resize_without_download_keeps_reported_source_size( def test_download_success_writes_files(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path) -> None: - registry = FakeRegistry([FakeCandidate("fox", "https://example.test/a.png", "image/png", 100, 100)]) + expected = png_bytes(100, 100) + registry = FakeRegistry( + [FakeCandidate("fox", "https://example.test/a.png", "image/png", 100, 100, payload=expected)] + ) code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS, "--download", "--output-dir", str(tmp_path)) @@ -334,7 +352,8 @@ def test_download_success_writes_files(monkeypatch: pytest.MonkeyPatch, capsys: assert download["ok"] is True target = Path(download["path"]) assert target.parent == tmp_path - assert target.read_bytes() == b"payload" + # The verified bytes are stored verbatim, without a second download. + assert target.read_bytes() == expected def test_partial_download_reports_status(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path) -> None: @@ -349,6 +368,9 @@ def test_partial_download_reports_status(monkeypatch: pytest.MonkeyPatch, capsys assert code == envelope.EXIT_PARTIAL_DOWNLOAD assert payload["status"] == envelope.STATUS_PARTIAL_DOWNLOAD + # The readable candidate is still reported and downloaded. + assert payload["items"][0]["download"]["ok"] is True + assert payload["error"]["details"]["failed"] == 1 def test_all_downloads_failing_is_access_error(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path) -> None: @@ -358,6 +380,7 @@ def test_all_downloads_failing_is_access_error(monkeypatch: pytest.MonkeyPatch, assert code == envelope.EXIT_ACCESS_ERROR assert payload["status"] == envelope.STATUS_ACCESS_ERROR + assert payload["error"]["code"] == "candidate_unreadable" def test_search_access_error_maps_to_access_error(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: @@ -398,6 +421,12 @@ def png_bytes(width: int, height: int, rgba: tuple[int, int, int, int] = (255, 0 ) +def gif_bytes(width: int = 100, height: int = 100) -> bytes: + """Build a payload that really is a GIF, i.e. a format we must never accept.""" + + return b"GIF89a" + struct.pack(" tuple[int, int]: assert payload.startswith(PNG_SIGNATURE) return struct.unpack(">II", payload[16:24]) @@ -528,9 +557,11 @@ def test_resize_downscales_png_stored_inside_ico( assert png_size(inner) == (50, 50) -def test_undecodable_payload_with_resize_fails_the_item( +def test_unreadable_payload_is_an_access_error( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path ) -> None: + """Bytes that are not an image must never become a "no match" verdict.""" + registry = FakeRegistry( [ FakeCandidate( @@ -545,6 +576,6 @@ def test_undecodable_payload_with_resize_fails_the_item( assert code == envelope.EXIT_ACCESS_ERROR assert document["status"] == envelope.STATUS_ACCESS_ERROR - download = document["items"][0]["download"] - assert download["ok"] is False - assert download["error"]["code"] == "resize_failed" + assert document["error"]["code"] == "candidate_unreadable" + assert document["items"] == [] + assert list(tmp_path.iterdir()) == [] diff --git a/tests/test_images.py b/tests/test_images.py new file mode 100644 index 0000000..a367885 --- /dev/null +++ b/tests/test_images.py @@ -0,0 +1,457 @@ +"""Tests for byte-based format and size filtering. + +Two layers are covered: + +* :mod:`tools.images` -- the stdlib-only detector and the stdlib-only builders + for the two accepted formats, so fixtures are real images rather than magic + strings; +* the CLI search path -- candidates are filtered on the *downloaded bytes*, so + a source that mislabels a file, claims the wrong size or serves an unreadable + body produces the correct ``ok`` / ``no_format`` / ``no_size`` / + ``access_error`` answer. Every CLI test injects a fake byte-fetch callable, + so nothing here touches the network. +""" + +from __future__ import annotations + +import json +import struct +from typing import Any, Mapping, Sequence + +import pytest + +from tools import envelope, example, images +from tools.errors import SourceAccessError +from tools.images import ImageFormatError, ImageInfo + +# --------------------------------------------------------------------------- +# Real-format builders used as fixtures +# --------------------------------------------------------------------------- + + +def png_payload(width: int, height: int) -> bytes: + """A real PNG built with :func:`tools.images.png_bytes`.""" + + return images.png_bytes(width, height) + + +def ico_payload(width: int, height: int) -> bytes: + """A real ICO (PNG-compressed entry) built with :func:`tools.images.ico_bytes`.""" + + return images.ico_bytes(width, height) + + +def gif_payload(width: int = 100, height: int = 100) -> bytes: + """A payload whose real format is GIF, i.e. one we must never accept.""" + + return b"GIF89a" + struct.pack(" None: + payload = png_payload(13, 7) + + assert payload.startswith(images.PNG_SIGNATURE) + assert images.detect_format(payload) == images.PNG + assert images.inspect_image(payload) == ImageInfo(format="png", width=13, height=7) + # The IHDR chunk really carries the dimensions, so the bytes are the source + # of truth rather than a helper function agreeing with itself. + assert payload[12:16] == b"IHDR" + assert struct.unpack(">II", payload[16:24]) == (13, 7) + assert images.read_png_dimensions(payload) == (13, 7) + + +def test_ico_bytes_builds_a_directory_that_reports_the_requested_sizes() -> None: + payload = ico_payload(48, 37) + + assert images.detect_format(payload) == images.ICO + assert images.inspect_image(payload) == ImageInfo(format="ico", width=48, height=37) + # One directory entry, and its data range lies inside the payload. + reserved, kind, count = struct.unpack(" None: + payload = ico_payload(256, 256) + + assert payload[6] == 0 and payload[7] == 0 + assert images.read_ico_dimensions(payload) == (256, 256) + + +def test_ico_dimensions_use_the_largest_directory_entry() -> None: + payload = images.ico_bytes(16, 16, frames=[(16, 16), (32, 32), (256, 256)]) + + assert len(images.ico_entries(payload)) == 3 + assert images.read_ico_dimensions(payload) == (256, 256) + assert images.inspect_image(payload).format == "ico" + + +@pytest.mark.parametrize( + ("payload", "expected"), + [ + (png_payload(2, 2), images.PNG), + (ico_payload(2, 2), images.ICO), + (gif_payload(), images.GIF), + (b"\xff\xd8\xff\xe0" + b"\x00" * 12, images.JPEG), + (b"BM" + b"\x00" * 20, images.BMP), + (b"RIFF" + b"\x00\x00\x00\x00" + b"WEBP" + b"\x00" * 8, images.WEBP), + ], +) +def test_detect_format_names_the_real_format(payload: bytes, expected: str) -> None: + assert images.detect_format(payload) == expected + + +@pytest.mark.parametrize("payload", [b"", b"payload", b"not-an-image", b""]) +def test_detect_format_returns_none_for_non_images(payload: bytes) -> None: + assert images.detect_format(payload) is None + + +def test_inspect_image_rejects_formats_whose_size_is_not_readable() -> None: + with pytest.raises(ImageFormatError) as excinfo: + images.inspect_image(gif_payload()) + + assert excinfo.value.code == "image_unreadable" + assert excinfo.value.details["format"] == images.GIF + + +@pytest.mark.parametrize( + "payload", + [ + b"not-an-image", + images.PNG_SIGNATURE, + images.PNG_SIGNATURE + struct.pack(">I", 13) + b"IDAT" + b"\x00" * 13, + b"\x00\x00\x01\x00" + struct.pack(" None: + with pytest.raises(ImageFormatError): + images.inspect_image(payload) + + +def test_png_dimensions_reject_a_zero_sized_image() -> None: + payload = images.PNG_SIGNATURE + struct.pack(">I", 13) + b"IHDR" + struct.pack(">IIBBBBB", 0, 0, 8, 6, 0, 0, 0) + + with pytest.raises(ImageFormatError): + images.read_png_dimensions(payload) + + +# --------------------------------------------------------------------------- +# CLI search path: filtering on downloaded bytes +# --------------------------------------------------------------------------- + + +class StubCandidate: + """Candidate whose bytes are supplied by a fake fetch callable.""" + + def __init__( + self, + title: str, + url: str, + *, + payload: bytes | None = None, + error: Exception | None = None, + mime_type: str | None = None, + width: int | None = None, + height: int | None = None, + ) -> None: + self.source_id = "fake" + self.title = title + self.file_url = url + self.page_url = url + ".html" + # Deliberately misleading metadata: filtering must ignore all of this. + self.mime_type = mime_type + self.width = width + self.height = height + self._payload = payload + self._error = error + self.fetch_calls = 0 + + def fetch_bytes(self) -> bytes: + self.fetch_calls += 1 + if self._error is not None: + raise self._error + assert self._payload is not None + return self._payload + + +class StubRegistry: + """Registry stand-in that returns a fixed candidate list.""" + + def __init__(self, candidates: Sequence[StubCandidate]) -> None: + self.candidates = list(candidates) + self.calls: list[tuple[str, str, int]] = [] + + def available_sources(self) -> Sequence[str]: + return ["fake"] + + def search(self, source_id: str, topic: str, limit: int = 10) -> list[StubCandidate]: + self.calls.append((source_id, topic, limit)) + return self.candidates[:limit] + + +def run_search( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + candidates: Sequence[StubCandidate], + *args: str, +) -> tuple[int, dict[str, Any], StubRegistry]: + registry = StubRegistry(candidates) + monkeypatch.setattr(example, "default_registry", lambda: registry) + code = example.main(list(args)) + captured = capsys.readouterr() + assert captured.err == "" + lines = [line for line in captured.out.splitlines() if line.strip()] + assert len(lines) == 1, captured.out + document = json.loads(lines[0]) + assert isinstance(document, Mapping) + return code, document, registry + + +BASE_ARGS = ("--topic", "red fox", "--format", "png", "--source", "fake") + + +def test_png_candidate_is_kept_with_its_actual_dimensions( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + payload = png_payload(800, 600) + candidate = StubCandidate( + "fox", + "https://example.test/fox.png", + payload=payload, + # The source claims something else entirely; the bytes win. + mime_type="image/jpeg", + width=12, + height=12, + ) + + code, document, registry = run_search( + monkeypatch, capsys, [candidate], *BASE_ARGS, "--width", "640", "--height", "480" + ) + + assert code == envelope.EXIT_OK + assert document["status"] == envelope.STATUS_OK + assert document["error"] is None + item = document["items"][0] + assert item["source"] == "fake" + assert item["url"] == "https://example.test/fox.png" + assert item["format"] == "png" + assert (item["width"], item["height"]) == (800, 600) + assert item["size_bytes"] == len(payload) + assert candidate.fetch_calls == 1 + # The bounded candidate window is requested from the source, not --count. + assert registry.calls == [("fake", "red fox", example.candidate_cap(10))] + + +def test_ico_request_with_only_png_payloads_yields_no_format( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + candidates = [ + StubCandidate("fox", "https://example.test/a.png", payload=png_payload(400, 400), mime_type="image/x-icon"), + StubCandidate("fox2", "https://example.test/b.png", payload=png_payload(500, 500), mime_type="image/ico"), + ] + + code, document, _ = run_search( + monkeypatch, + capsys, + candidates, + "--topic", + "red fox", + "--format", + "ico", + "--source", + "fake", + "--width", + "64", + "--height", + "64", + ) + + assert code == envelope.EXIT_NO_FORMAT + assert document["status"] == envelope.STATUS_NO_FORMAT + assert document["items"] == [] + assert document["error"]["code"] == "no_format_match" + assert document["error"]["details"]["requested_format"] == "ico" + assert document["error"]["details"]["unreadable_candidates"] == 0 + assert document["error"]["details"]["format_matches"] == 0 + + +def test_ico_payload_is_kept_for_an_ico_request( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + candidate = StubCandidate("fox", "https://example.test/fox.ico", payload=ico_payload(64, 48)) + + code, document, _ = run_search( + monkeypatch, + capsys, + [candidate], + "--topic", + "red fox", + "--format", + "ico", + "--source", + "fake", + "--width", + "32", + "--height", + "32", + ) + + assert code == envelope.EXIT_OK + item = document["items"][0] + assert item["format"] == "ico" + assert (item["width"], item["height"]) == (64, 48) + + +def test_all_too_small_payloads_yield_no_size( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + candidates = [ + StubCandidate("small", "https://example.test/a.png", payload=png_payload(100, 100), width=4000, height=4000), + StubCandidate("tall", "https://example.test/b.png", payload=png_payload(900, 100)), + ] + + code, document, _ = run_search( + monkeypatch, capsys, candidates, *BASE_ARGS, "--width", "200", "--height", "200" + ) + + assert code == envelope.EXIT_NO_SIZE + assert document["status"] == envelope.STATUS_NO_SIZE + assert document["items"] == [] + assert document["error"]["code"] == "no_size_match" + assert document["error"]["details"]["format_matches"] == 2 + assert document["error"]["details"]["unreadable_candidates"] == 0 + + +def test_exactly_minimum_size_is_accepted( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + candidate = StubCandidate("edge", "https://example.test/edge.png", payload=png_payload(200, 200)) + + code, document, _ = run_search( + monkeypatch, capsys, [candidate], *BASE_ARGS, "--width", "200", "--height", "200" + ) + + assert code == envelope.EXIT_OK + assert (document["items"][0]["width"], document["items"][0]["height"]) == (200, 200) + + +def test_no_format_and_no_size_and_access_error_stay_distinct( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + format_code, format_document, _ = run_search( + monkeypatch, + capsys, + [StubCandidate("gif", "https://example.test/a.png", payload=gif_payload())], + *BASE_ARGS, + ) + size_code, size_document, _ = run_search( + monkeypatch, + capsys, + [StubCandidate("small", "https://example.test/a.png", payload=png_payload(10, 10))], + *BASE_ARGS, + "--width", + "100", + "--height", + "100", + ) + access_code, access_document, _ = run_search( + monkeypatch, + capsys, + [StubCandidate("broken", "https://example.test/a.png", error=SourceAccessError("refused"))], + *BASE_ARGS, + ) + + assert format_document["status"] == envelope.STATUS_NO_FORMAT + assert size_document["status"] == envelope.STATUS_NO_SIZE + assert access_document["status"] == envelope.STATUS_ACCESS_ERROR + assert len({format_code, size_code, access_code}) == 3 + assert (format_code, size_code, access_code) == ( + envelope.EXIT_NO_FORMAT, + envelope.EXIT_NO_SIZE, + envelope.EXIT_ACCESS_ERROR, + ) + + +def test_fetch_failure_yields_access_error( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + candidates = [ + StubCandidate( + "broken", + "https://example.test/a.png", + error=SourceAccessError("image source returned HTTP status 503", url="https://example.test/a.png"), + ) + ] + + code, document, _ = run_search(monkeypatch, capsys, candidates, *BASE_ARGS) + + assert code == envelope.EXIT_ACCESS_ERROR + assert document["status"] == envelope.STATUS_ACCESS_ERROR + assert document["items"] == [] + assert document["error"]["code"] == "candidate_unreadable" + assert document["error"]["details"]["failures"][0]["url"] == "https://example.test/a.png" + + +def test_unreadable_payload_yields_access_error_not_no_format( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + candidate = StubCandidate( + "junk", "https://example.test/a.png", payload=b"not-an-image", mime_type="image/png" + ) + + code, document, _ = run_search(monkeypatch, capsys, [candidate], *BASE_ARGS) + + assert code == envelope.EXIT_ACCESS_ERROR + assert document["status"] == envelope.STATUS_ACCESS_ERROR + assert document["error"]["details"]["failures"][0]["error"]["code"] == "image_unreadable" + + +def test_candidate_window_is_bounded_by_the_requested_count( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + candidates = [ + StubCandidate(f"fox{n}", f"https://example.test/{n}.png", payload=png_payload(100, 100)) + for n in range(40) + ] + + code, document, registry = run_search( + monkeypatch, capsys, candidates, *BASE_ARGS, "--count", "3" + ) + + assert code == envelope.EXIT_OK + assert len(document["items"]) == 3 + assert registry.calls == [("fake", "red fox", example.candidate_cap(3))] + assert example.candidate_cap(3) == 12 + # Fetching stops as soon as enough usable images were found. + assert sum(candidate.fetch_calls for candidate in candidates) == 3 + + +def test_filtering_walks_past_rejected_candidates_until_the_count_is_met( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + candidates = [ + StubCandidate("gif", "https://example.test/0.png", payload=gif_payload()), + StubCandidate("tiny", "https://example.test/1.png", payload=png_payload(10, 10)), + StubCandidate("ok1", "https://example.test/2.png", payload=png_payload(120, 120)), + StubCandidate("ok2", "https://example.test/3.png", payload=png_payload(130, 130)), + StubCandidate("never", "https://example.test/4.png", payload=png_payload(140, 140)), + ] + + code, document, _ = run_search( + monkeypatch, capsys, candidates, *BASE_ARGS, "--count", "2", "--width", "100", "--height", "100" + ) + + assert code == envelope.EXIT_OK + assert [item["url"] for item in document["items"]] == [ + "https://example.test/2.png", + "https://example.test/3.png", + ] + assert [(item["width"], item["height"]) for item in document["items"]] == [(120, 120), (130, 130)] + assert candidates[-1].fetch_calls == 0 diff --git a/tools/example.py b/tools/example.py index ef733af..272ff53 100644 --- a/tools/example.py +++ b/tools/example.py @@ -17,9 +17,11 @@ import re import struct import sys import zlib +from dataclasses import dataclass from pathlib import Path from typing import Any, Mapping, Sequence +from . import images from .envelope import ( STATUS_ACCESS_ERROR, STATUS_NO_FORMAT, @@ -41,6 +43,7 @@ from .errors import ( UnknownSourceError, UnsupportedFormatError, ) +from .images import ImageFormatError, ImageInfo from .sources import DEFAULT_LIMIT, SourceCandidate, default_registry PROG = "python -m tools.example" @@ -66,11 +69,21 @@ DEFAULT_OUTPUT_DIR = "downloads" MIN_COUNT = 1 MAX_COUNT = 50 -#: MIME types (and filename suffixes) that count as a hit for each format. -FORMAT_MATCHERS: dict[str, tuple[str, ...]] = { - "png": ("image/png", ".png"), - "ico": ("image/x-icon", "image/vnd.microsoft.icon", "image/ico", ".ico"), -} +#: How many candidates are fetched per wanted image. Nothing a source reports +#: about a hit is trusted, so a candidate is only known to be usable once its +#: bytes have been downloaded and parsed; the source is therefore asked for a +#: window of ``count * SAMPLE_FACTOR`` hits and the window is cut short as soon +#: as ``count`` usable images have been found. +SAMPLE_FACTOR = 4 +#: Hard upper bound on the number of candidates inspected in one invocation. +MAX_CANDIDATE_CAP = MAX_COUNT + + +def candidate_cap(count: int) -> int: + """Return the bounded number of candidates to fetch for ``count`` images.""" + + wanted = max(MIN_COUNT, int(count)) + return min(MAX_CANDIDATE_CAP, wanted * SAMPLE_FACTOR) class _Parser(argparse.ArgumentParser): @@ -312,49 +325,145 @@ def normalize_request(raw: Mapping[str, Any]) -> dict[str, Any]: } -def _matches_format(candidate: SourceCandidate, image_format: str) -> bool: - matchers = FORMAT_MATCHERS[image_format] - mime = str(getattr(candidate, "mime_type", None) or "").split(";")[0].strip().lower() - if mime and mime in matchers: - return True - url = str(getattr(candidate, "file_url", "") or "").lower() - return any(url.endswith(suffix) for suffix in matchers if suffix.startswith(".")) +def _meets_minimum_size(info: ImageInfo, request: Mapping[str, Any]) -> bool: + """True when the *real* pixel size of an image is usable for the request. - -def _matches_size(candidate: SourceCandidate, request: Mapping[str, Any]) -> bool: - """True when ``candidate`` is usable for the requested target size. - - The requested width and height are a target size, not an exact match: any - image that is at least that large is usable because it can always be - downscaled, while a smaller image would have to be scaled up and is - therefore rejected. ``--resize`` only decides whether the downloaded bytes - are downscaled; it never widens or narrows this filter. + The requested width and height are a minimum, not an exact match: any image + that is at least that large is usable because it can always be downscaled, + while a smaller image would have to be scaled up and is therefore rejected. + ``--resize`` only decides whether the downloaded bytes are downscaled; it + never widens or narrows this filter. """ wanted_width = request.get("width") wanted_height = request.get("height") - if wanted_width is None and wanted_height is None: - return True - actual_width = getattr(candidate, "width", None) - actual_height = getattr(candidate, "height", None) - if isinstance(actual_width, bool) or isinstance(actual_height, bool): + if wanted_width is not None and info.width < wanted_width: return False - if not isinstance(actual_width, int) or not isinstance(actual_height, int): - return False - if wanted_width is not None and actual_width < wanted_width: - return False - if wanted_height is not None and actual_height < wanted_height: + if wanted_height is not None and info.height < wanted_height: return False return True -def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -> tuple[list[SourceCandidate], list[SourceCandidate]]: - """Return ``(matching, rejected_by_size)`` split for the request.""" +def _failure(candidate: SourceCandidate, fragment: Mapping[str, Any]) -> dict[str, Any]: + """Describe one candidate whose bytes could not be turned into an image.""" - by_format = [c for c in candidates if _matches_format(c, request["format"])] - matching = [c for c in by_format if _matches_size(c, request)] - rejected = [c for c in by_format if not _matches_size(c, request)] - return matching, rejected + return { + "source": str(getattr(candidate, "source_id", "") or ""), + "url": str(getattr(candidate, "file_url", "") or ""), + "error": dict(fragment), + } + + +@dataclass(frozen=True) +class KeptImage: + """One candidate whose downloaded bytes really match the request.""" + + candidate: SourceCandidate + info: ImageInfo + payload: bytes + + def to_item(self) -> dict[str, Any]: + """Report ``source``, ``url``, ``format`` and the *actual* dimensions.""" + + source = str(getattr(self.candidate, "source_id", "") or "") + url = str(getattr(self.candidate, "file_url", "") or "") + item: dict[str, Any] = { + "source": source, + "url": url, + # Kept as an alias so existing consumers keep working unchanged. + "file_url": url, + "format": self.info.format, + "width": self.info.width, + "height": self.info.height, + "size_bytes": len(self.payload), + } + for key in ("title", "page_url", "mime_type"): + value = getattr(self.candidate, key, None) + if value is not None: + item[key] = value + return item + + +@dataclass(frozen=True) +class Selection: + """Outcome of inspecting a bounded window of downloaded candidates.""" + + kept: tuple[KeptImage, ...] = () + format_rejects: int = 0 + size_rejects: int = 0 + failures: tuple[dict[str, Any], ...] = () + + @property + def inspected(self) -> int: + """Number of candidates whose bytes were read successfully.""" + + return len(self.kept) + self.format_rejects + self.size_rejects + + +def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -> Selection: + """Fetch and inspect candidates, keeping only what the real bytes justify. + + Every kept item has been downloaded and parsed: its format comes from the + container signature and its width/height come from the PNG ``IHDR`` or the + ICO directory, never from what the source claimed. A candidate that serves + other-format bytes counts as a format rejection, a candidate of the wanted + format that is too small counts as a size rejection, and a candidate whose + bytes cannot be fetched or parsed is recorded as a failure instead of being + silently dropped. + """ + + wanted_format = str(request["format"]) + limit = int(request["count"]) + kept: list[KeptImage] = [] + format_rejects = 0 + size_rejects = 0 + failures: list[dict[str, Any]] = [] + + for candidate in candidates: + if len(kept) >= limit: + break + try: + payload = candidate.fetch_bytes() + except ToolkitError as error: + failures.append(_failure(candidate, error.to_dict())) + continue + except OSError as error: + failures.append( + _failure(candidate, error_fragment("fetch_failed", str(error))) + ) + continue + detected = images.detect_format(payload) + if detected is None: + failures.append( + _failure( + candidate, + error_fragment( + "image_unreadable", + "downloaded payload is not a recognised image format", + details={"bytes": len(payload)}, + ), + ) + ) + continue + if detected != wanted_format: + format_rejects += 1 + continue + try: + info = images.inspect_image(payload) + except ImageFormatError as error: + failures.append(_failure(candidate, error.to_dict())) + continue + if not _meets_minimum_size(info, request): + size_rejects += 1 + continue + kept.append(KeptImage(candidate=candidate, info=info, payload=payload)) + + return Selection( + kept=tuple(kept), + format_rejects=format_rejects, + size_rejects=size_rejects, + failures=tuple(failures), + ) def _slug(candidate: SourceCandidate) -> str: @@ -665,12 +774,16 @@ def resize_image(payload: bytes, request: Mapping[str, Any]) -> tuple[bytes, dic return output, fragment -def _download(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -> tuple[list[dict[str, Any]], int]: - """Download ``candidates``; return ``(items, failure_count)``. +def _download( + selected: Sequence[KeptImage], request: Mapping[str, Any] +) -> tuple[list[dict[str, Any]], int]: + """Write the already-fetched payloads; return ``(items, failure_count)``. - With ``resize`` enabled every stored file is downscaled to the requested - target size first. A payload that cannot be decoded or downscaled is - reported as a failed item rather than silently stored at the wrong size. + The bytes verified by :func:`_select` are stored as they are (or downscaled + first when ``resize`` is enabled), so an item can never be downloaded twice + or stored in a format that was not verified. A payload that cannot be + downscaled to the requested size is reported as a failed item instead of + being silently stored at the wrong size. """ directory = Path(request["output_dir"]).expanduser() @@ -685,11 +798,11 @@ def _download(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) suffix = "." + str(request["format"]) items: list[dict[str, Any]] = [] failures = 0 - for index, candidate in enumerate(candidates, start=1): - record = dict(candidate.to_dict()) - destination = directory / f"{index:03d}-{_slug(candidate)}{suffix}" + for index, entry in enumerate(selected, start=1): + record = entry.to_item() + destination = directory / f"{index:03d}-{_slug(entry.candidate)}{suffix}" + payload = entry.payload try: - payload = candidate.fetch_bytes() resize_fragment = resize_image(payload, request) if request["resize"] else None if resize_fragment is not None: payload, resize_fragment = resize_fragment @@ -756,7 +869,21 @@ def _request_from(argv: Sequence[str]) -> tuple[dict[str, Any], str, Any]: def _execute(argv: Sequence[str]) -> Response: - """Run one request and return the response that must be printed.""" + """Run one request and return the response that must be printed. + + The search path downloads a bounded window of candidates and filters that + window on the parsed bytes, so the answer distinguishes three different + situations through both the status and the exit code: + + * ``ok`` -- at least one image really is the requested format and at least + the requested width and height; + * ``no_format`` -- the source had hits, but none of them really are the + requested format; + * ``no_size`` -- images of the requested format existed, but all of them + are smaller than requested; + * ``access_error`` -- the source could not be searched or served bytes that + are not readable images, which is never reported as a "no match". + """ request, source, registry = _request_from(argv) builder = ResponseBuilder(source=source, request=request) @@ -764,24 +891,36 @@ def _execute(argv: Sequence[str]) -> Response: if request["dry_run"]: return builder.dry_run() - candidates = registry.search(source, request["topic"], request["count"]) - matching, rejected = _select(candidates, request) + cap = candidate_cap(request["count"]) + candidates = registry.search(source, request["topic"], cap) + selection = _select(candidates, request) + unreadable = list(selection.failures) - if not matching: + if not selection.kept: details = { "requested_format": request["format"], "requested_size": {"width": request["width"], "height": request["height"]}, "minimum_size": {"width": request["width"], "height": request["height"]}, "candidate_count": len(candidates), - "format_matches": len(matching) + len(rejected), + "candidates_inspected": selection.inspected, + "format_matches": selection.size_rejects, "size_matches": 0, + "unreadable_candidates": len(unreadable), } - if rejected: + if unreadable: + return builder.access_error( + error=error_fragment( + "candidate_unreadable", + "the source served images that could not be read", + details={**details, "failures": unreadable}, + ) + ) + if selection.size_rejects: return builder.build( STATUS_NO_SIZE, error=error_fragment( "no_size_match", - "no image is at least the requested width and height", + "the source returned no images at least the requested width and height", details=details, ), ) @@ -795,27 +934,47 @@ def _execute(argv: Sequence[str]) -> Response: ) if not request["download"]: - return builder.build(STATUS_OK, items=[c.to_dict() for c in matching]) + if unreadable: + return builder.build( + STATUS_PARTIAL_DOWNLOAD, + items=[entry.to_item() for entry in selection.kept], + error=error_fragment( + "candidate_incomplete", + f"{len(unreadable)} of {selection.inspected + len(unreadable)} " + "candidates could not be read", + details={ + "failed": len(unreadable), + "total": selection.inspected + len(unreadable), + "failures": unreadable, + }, + ), + ) + return builder.build(STATUS_OK, items=[entry.to_item() for entry in selection.kept]) - items, failures = _download(matching, request) - if failures and failures == len(items): + items, download_failures = _download(selection.kept, request) + failed = download_failures + len(unreadable) + if download_failures == len(items): return builder.build( STATUS_ACCESS_ERROR, items=items, error=error_fragment( "download_failed", "every download failed", - details={"failed": failures, "total": len(items)}, + details={"failed": download_failures, "total": len(items)}, ), ) - if failures: + if failed: return builder.build( STATUS_PARTIAL_DOWNLOAD, items=items, error=error_fragment( "download_incomplete", - f"{failures} of {len(items)} downloads failed", - details={"failed": failures, "total": len(items)}, + f"{failed} of {len(items) + len(unreadable)} images could not be handled", + details={ + "failed": failed, + "total": len(items) + len(unreadable), + "failures": unreadable, + }, ), ) return builder.build(STATUS_OK, items=items) diff --git a/tools/images.py b/tools/images.py new file mode 100644 index 0000000..3044a80 --- /dev/null +++ b/tools/images.py @@ -0,0 +1,367 @@ +"""Stdlib-only inspection and construction of the image formats we accept. + +The CLI decides what to keep from the **downloaded bytes** and never from the +metadata a source reports about a file: a search hit that claims to be a +``image/png`` of 800x600 pixels is kept only when the bytes really are a PNG of +at least the requested size. This module is the single place that knows how to +read the real format and the real pixel dimensions out of those bytes. + +It deliberately depends on nothing but the standard library: + +* :func:`detect_format` recognises the container signatures of the two accepted + formats (``png``, ``ico``) plus a handful of common formats (``jpeg``, + ``gif``, ``bmp``, ``webp``). Recognising the *other* formats is what makes + the ``no_format`` outcome possible: "the source has images, but none of them + are the format you asked for" is a different answer from "the source has no + usable images at all". +* :func:`read_png_dimensions` reads the ``IHDR`` header of a PNG. +* :func:`read_ico_dimensions` reads the ICONDIR/ICONDIRENTRY directory of an + ICO file (the directory stores the real sizes, including the ``0``-means-256 + encoding). +* :func:`inspect_image` combines both into an :class:`ImageInfo`. +* :func:`png_bytes` and :func:`ico_bytes` *build* the same formats so tests (and + callers) can produce real payloads without Pillow or any other dependency. + +Every failure raises :class:`ImageFormatError`, a :class:`~tools.errors.ToolkitError` +subclass whose ``code`` is stable so the CLI can report an unreadable payload as +an access problem instead of silently treating it as "no match". +""" + +from __future__ import annotations + +import struct +import zlib +from dataclasses import dataclass +from typing import Any, Iterable, Sequence + +from .errors import ToolkitError + +__all__ = [ + "BMP", + "GIF", + "ICO", + "ICO_ENTRY_SIZE", + "ICO_HEADER_SIZE", + "ICO_MAX_DIMENSION", + "ImageFormatError", + "ImageInfo", + "JPEG", + "KNOWN_FORMATS", + "PNG", + "PNG_SIGNATURE", + "WEBP", + "detect_format", + "ico_bytes", + "ico_entries", + "inspect_image", + "png_bytes", + "read_ico_dimensions", + "read_png_dimensions", +] + +#: The two formats the CLI accepts as search requests. +PNG = "png" +ICO = "ico" +#: Formats that are recognised (so they can be reported as "another format") +#: but never accepted by the CLI. +JPEG = "jpeg" +GIF = "gif" +BMP = "bmp" +WEBP = "webp" + +#: Every format :func:`detect_format` can name. +KNOWN_FORMATS: tuple[str, ...] = (PNG, ICO, JPEG, GIF, BMP, WEBP) + +#: The 8-byte signature every PNG file starts with. +PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n" + +#: ICONDIR: reserved (0), type (1 = icon) and the entry count. +ICO_HEADER_SIZE = 6 +#: One ICONDIRENTRY: 16 bytes of width/height/colours/planes/bpp/size/offset. +ICO_ENTRY_SIZE = 16 +#: A single ICO dimension byte of 0 encodes 256 pixels. +ICO_MAX_DIMENSION = 256 + +#: Magic prefixes of the recognised but unsupported formats. +_JPEG_SIGNATURE = b"\xff\xd8\xff" +_GIF_SIGNATURES = (b"GIF87a", b"GIF89a") +_BMP_SIGNATURE = b"BM" +_RIFF_SIGNATURE = b"RIFF" +_WEBP_SIGNATURE = b"WEBP" + +#: Number of payload bytes worth quoting in an error detail. +_SIGNATURE_PROBE = 16 + + +class ImageFormatError(ToolkitError): + """Raised when downloaded bytes are not a readable image of a known format. + + The bytes could not be attributed to any format the toolkit understands, or + they claim to be an accepted format but their header is truncated/corrupt. + It derives from :class:`~tools.errors.ToolkitError`, so the CLI reports it + as an access problem (the source handed us something unusable) rather than + as a "no such format" / "no such size" verdict about the topic. + """ + + code = "image_unreadable" + + def __init__(self, message: str, *, details: dict[str, Any] | None = None) -> None: + super().__init__(message, details=details) + + +@dataclass(frozen=True) +class ImageInfo: + """The real format and real pixel size of one payload.""" + + format: str + width: int + height: int + + def to_dict(self) -> dict[str, Any]: + """JSON-ready projection used in CLI items.""" + + return {"format": self.format, "width": self.width, "height": self.height} + + @property + def area(self) -> int: + return self.width * self.height + + +def _probe(payload: bytes) -> bytes: + return bytes(payload[:_SIGNATURE_PROBE]) + + +def detect_format(payload: bytes) -> str | None: + """Return the real container format of *payload*, or ``None``. + + ``None`` means "these bytes are not an image format the toolkit knows", which + is an access/payload problem. A recognised format other than ``png``/``ico`` + is a perfectly good answer that the CLI turns into ``no_format``. + """ + + if not isinstance(payload, (bytes, bytearray, memoryview)): + return None + data = bytes(payload) + if data.startswith(PNG_SIGNATURE): + return PNG + if data[:4] == b"\x00\x00\x01\x00": + return ICO + if data.startswith(_JPEG_SIGNATURE): + return JPEG + if data.startswith(_GIF_SIGNATURES): + return GIF + if data.startswith(_BMP_SIGNATURE) and len(data) >= 14: + return BMP + if data.startswith(_RIFF_SIGNATURE) and data[8:12] == _WEBP_SIGNATURE: + return WEBP + return None + + +def read_png_dimensions(payload: bytes) -> tuple[int, int]: + """Return ``(width, height)`` from the ``IHDR`` chunk of a PNG payload.""" + + data = bytes(payload) + if not data.startswith(PNG_SIGNATURE): + raise ImageFormatError( + "payload does not start with the PNG signature", + details={"signature": _probe(data).hex()}, + ) + offset = len(PNG_SIGNATURE) + if len(data) < offset + 8: + raise ImageFormatError("PNG header is truncated") + (length,) = struct.unpack(">I", data[offset : offset + 4]) + chunk_type = data[offset + 4 : offset + 8] + if chunk_type != b"IHDR": + raise ImageFormatError( + "PNG payload does not start with an IHDR chunk", + details={"first_chunk": chunk_type.decode("latin-1")}, + ) + if length != 13: + raise ImageFormatError( + "PNG IHDR chunk has an unexpected length", + details={"length": length}, + ) + header = data[offset + 8 : offset + 8 + length] + if len(header) != length: + raise ImageFormatError("PNG IHDR chunk is truncated") + width, height = struct.unpack(">II", header[:8]) + if width < 1 or height < 1: + raise ImageFormatError( + "PNG image declares no pixels", + details={"width": width, "height": height}, + ) + return int(width), int(height) + + +def ico_entries(payload: bytes) -> tuple[tuple[int, int, int, int], ...]: + """Return ``(width, height, size, offset)`` for every ICO directory entry.""" + + data = bytes(payload) + if len(data) < ICO_HEADER_SIZE: + raise ImageFormatError("ICO header is truncated") + reserved, kind, count = struct.unpack(" len(data): + raise ImageFormatError( + "ICO directory entry points outside the payload", + details={"index": index, "size": size, "offset": offset, "bytes": len(data)}, + ) + entries.append((width, height, size, offset)) + return tuple(entries) + + +def read_ico_dimensions(payload: bytes) -> tuple[int, int]: + """Return the dimensions of the largest image stored in an ICO payload. + + The ICONDIRENTRY directory is authoritative: it carries one width/height pair + per stored image, so no decoding of the embedded bitmap or PNG is needed. + """ + + entries = ico_entries(payload) + width, height, _size, _offset = max(entries, key=lambda entry: entry[0] * entry[1]) + return int(width), int(height) + + +def inspect_image(payload: bytes) -> ImageInfo: + """Return the real format and pixel size of *payload*. + + Raises :class:`ImageFormatError` when the bytes are not an image at all, or + when they are not one of the two formats whose dimensions can be read. + """ + + image_format = detect_format(payload) + if image_format is None: + raise ImageFormatError( + "downloaded payload is not a recognised image format", + details={"signature": _probe(bytes(payload)).hex()}, + ) + if image_format == PNG: + width, height = read_png_dimensions(payload) + return ImageInfo(format=PNG, width=width, height=height) + if image_format == ICO: + width, height = read_ico_dimensions(payload) + return ImageInfo(format=ICO, width=width, height=height) + raise ImageFormatError( + f"pixel dimensions are only readable for png and ico, not {image_format}", + details={"format": image_format}, + ) + + +# --------------------------------------------------------------------------- +# Builders: the same formats, produced with the standard library only +# --------------------------------------------------------------------------- + + +def _png_chunk(chunk_type: bytes, chunk_data: bytes) -> bytes: + crc = zlib.crc32(chunk_type + chunk_data) & 0xFFFFFFFF + return struct.pack(">I", len(chunk_data)) + chunk_type + chunk_data + struct.pack(">I", crc) + + +def png_bytes( + width: int, + height: int, + *, + rgba: tuple[int, int, int, int] = (255, 255, 255, 255), + rows: Sequence[bytes] | None = None, + level: int = 6, +) -> bytes: + """Build a valid 8-bit RGBA PNG of exactly ``width`` x ``height`` pixels. + + ``rows`` may supply raw (filter-0) RGBA scanlines; otherwise every pixel is + filled with ``rgba``. The result is a real PNG file: a correct signature, + a 13-byte ``IHDR``, a zlib-compressed ``IDAT`` and an empty ``IEND``. + """ + + if isinstance(width, bool) or isinstance(height, bool): + raise ValueError("width and height must be integers") + if width < 1 or height < 1: + raise ValueError("width and height must be >= 1") + if rows is None: + pixel = bytes(rgba) + line = pixel * width + raw = bytearray() + for _ in range(height): + raw.append(0) + raw += line + raw_bytes = bytes(raw) + else: + if len(rows) != height: + raise ValueError("rows must contain exactly height scanlines") + raw = bytearray() + for row in rows: + if len(row) != width * 4: + raise ValueError("each scanline must hold width * 4 bytes") + raw.append(0) + raw += row + raw_bytes = bytes(raw) + header = struct.pack(">IIBBBBB", width, height, 8, 6, 0, 0, 0) + return ( + PNG_SIGNATURE + + _png_chunk(b"IHDR", header) + + _png_chunk(b"IDAT", zlib.compress(raw_bytes, level)) + + _png_chunk(b"IEND", b"") + ) + + +def ico_bytes( + width: int, + height: int, + *, + rgba: tuple[int, int, int, int] = (255, 255, 255, 255), + frames: Iterable[tuple[int, int]] | None = None, +) -> bytes: + """Build an ICO payload whose directory lists the requested sizes. + + By default the ICO stores one embedded PNG of ``width`` x ``height`` pixels + and the directory entry repeats those dimensions (``0`` encodes 256). Pass + ``frames`` to describe several directory entries instead, for example + ``ico_bytes(48, 48, frames=[(16, 16), (48, 48)])``: the dimensions of the + *largest* entry then become the reported size, which is exactly what + :func:`read_ico_dimensions` returns. + """ + + sizes = list(frames) if frames is not None else [(width, height)] + if not sizes: + raise ValueError("an ICO payload needs at least one frame") + payloads: list[bytes] = [] + for frame_width, frame_height in sizes: + if not 1 <= frame_width <= ICO_MAX_DIMENSION: + raise ValueError("ICO widths must be between 1 and 256 pixels") + if not 1 <= frame_height <= ICO_MAX_DIMENSION: + raise ValueError("ICO heights must be between 1 and 256 pixels") + payloads.append(png_bytes(frame_width, frame_height, rgba=rgba)) + header = struct.pack(" Date: Tue, 3 Nov 2026 02:25:41 +0000 Subject: [PATCH 08/10] change: Implement 5: Save selected images to the caller directory and downscale only behind the resize flag Implemented image saving into the caller directory and exact-size downscaling behind --resize. tools/images.py: added stdlib-only helpers `png_chunks`, `decode_png`, `encode_png`, `ico_wrap`, `ico_unwrap`, plus `decode_image`, `resample_rows`, `encode_image` and the `DecodedImage` dataclass; also fixed a real pixel-stride bug in RGBA upsampling (channel step was used as pixel stride, corrupting 8-bit pixels). tools/example.py: removed the inlined codec (now delegated to tools.images); `--download` writes each selected image into the caller-specified directory; each stored file is read back and decoded, so a reported path only names a file that exists and opens as the requested format (`format`/`width`/`height`/`bytes` reported per item, no path for a failed item); `--download` without `--output-dir` is now an `invalid_request` (`request_field`, field=output_dir) instead of a hidden default, and DEFAULT_OUTPUT_DIR is gone; `--resize` stores exactly the requested width/height (one open dimension keeps the source value), re-encodes in the requested format, never upscales and leaves a fitting image's bytes untouched; a per-item save failure is reported in that item and ends the run with `partial_download` (exit 6), including `saved_paths` in the error details. tests/test_download_resize.py: new focused suite over tmp_path dirs and locally built png/ico fixtures (save paths exist and decode, requested-format check, missing --output-dir invalid_request, failing save => partial_download, exact resize dims for png and ico, no upscaling, byte/dimension preservation without --resize, plus codec round-trip tests). tests/test_cli.py: updated the aspect-ratio resize expectation to the required exact 100x100 stored size. Target branch task/1-task is already the current head (d5bb24f), so the tested revision contains the target. Tests: Facade commands run from /workspace with .venv/bin on PATH (prepared via prepare_environment tools restore): `python -m compileall tools tests` (tools.validate) OK; `python -m tools.example examples/input.json` (tools.smoke) exit 0 with status dry_run; `python -m tools.example --help` (acceptance help-command) exit 0; `python -c "import tools, tools.example, tools.images"` (package_import) OK; `pytest` (tools.test, the exact facade entrypoint) => 142 passed. New tests: tests/test_download_resize.py (13 tests). No network-dependent live-source check was run (sandbox has no egress); all fixtures are locally built real png/ico payloads. Progress: Completed issue 7: stdlib PNG/ICO decode/encode/wrap/unwrap plus resample helpers live in tools/images.py; tools/example.py saves selected images into the caller directory, requires --output-dir with --download (invalid_request otherwise), verifies every stored file by decoding it before reporting its path, downscales to exactly the requested width/height in the requested format only when --resize is set (never upscaling, otherwise byte-identical), and reports a per-item save failure with the partial_download status/exit code. New tests in tests/test_download_resize.py cover all of that; tests/test_cli.py resize expectation aligned with the exact-size requirement. Facade validate/smoke/help/import all pass and `pytest` reports 142 passed on the revision that already contains target task/1-task. Published by developer-agent through Tool Backend. Closes #7 --- .agent-work/ledger.json | 5 + tests/test_cli.py | 8 +- tests/test_download_resize.py | 326 ++++++++++++++++++++++++ tools/example.py | 454 ++++++++++------------------------ tools/images.py | 368 +++++++++++++++++++++++++++ 5 files changed, 841 insertions(+), 320 deletions(-) create mode 100644 tests/test_download_resize.py diff --git a/.agent-work/ledger.json b/.agent-work/ledger.json index 152f680..4ee58a1 100644 --- a/.agent-work/ledger.json +++ b/.agent-work/ledger.json @@ -21,6 +21,11 @@ "Implemented byte-based format and minimum-size filtering for the image search path. New tools/images.py: detect_format (png/ico plus jpeg/gif/bmp/webp so \"other format\" is distinguishable from \"unreadable\"), read_png_dimensions (IHDR), read_ico_dimensions (ICONDIR entries, 0 => 256, largest entry), inspect_image/ImageInfo, ImageFormatError (code image_unreadable), and png_bytes/ico_bytes builders. tools/example.py: candidate_cap(count) bounded window (count*4, max 50) requested from the source, _select() fetches and parses every candidate, KeptImage.to_item() reports source/url/format/actual width+height/size_bytes, _download() writes the already verified bytes, and _execute() distinguishes ok/no_format/no_size/access_error/partial_download with the existing exit codes; metadata-based filtering removed. tests/test_images.py added (31 tests) and tests/test_cli.py fixtures aligned to real image bytes. All facade commands pass: compileall, pytest (114 passed), smoke, help, package import.", "Addressed changes-requested review on PR #14: the CLI no longer turns common live-source results into partial_download/access_error. Any payload whose real format is not the requested png/ico (jpeg, gif, bmp, webp, svg, tiff, pdf, or an unrecognised body such as an HTML page) is now a format mismatch feeding no_format, with observed_formats/other_format_candidates detail; only unfetchable candidates are failures, and access_error is emitted only when nothing at all could be fetched. images.detect_format gained svg/tiff/pdf recognition and ImageFormatError is scoped to genuinely unusable png/ico payloads. Statuses/exits unchanged (no_format 3, no_size 4, access_error 5). Tests extended in tests/test_images.py (svg/tiff/pdf detection, mixed-format window stays ok, ico-only-foreign yields no_format, no_size preserved alongside other formats, non-image body is no_format, unfetchable candidate does not demote a search, corrupt png does not hide a match, empty window) and tests/test_cli.py aligned. Facade validate/test/smoke/help all green (129 passed) on the integrated branch; no protected facade paths touched." ] + }, + "projects/product-54354138-1#7": { + "events": [ + "Completed issue 7: stdlib PNG/ICO decode/encode/wrap/unwrap plus resample helpers live in tools/images.py; tools/example.py saves selected images into the caller directory, requires --output-dir with --download (invalid_request otherwise), verifies every stored file by decoding it before reporting its path, downscales to exactly the requested width/height in the requested format only when --resize is set (never upscaling, otherwise byte-identical), and reports a per-item save failure with the partial_download status/exit code. New tests in tests/test_download_resize.py cover all of that; tests/test_cli.py resize expectation aligned with the exact-size requirement. Facade validate/smoke/help/import all pass and `pytest` reports 142 passed on the revision that already contains target task/1-task." + ] } } } diff --git a/tests/test_cli.py b/tests/test_cli.py index 9adbc3f..c01c902 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -477,11 +477,13 @@ def test_download_resize_downscales_stored_image( assert download["ok"] is True assert download["resize"]["applied"] is True assert download["resize"]["from"] == {"width": 200, "height": 100} - assert download["resize"]["to"] == {"width": 100, "height": 50} + # --resize stores exactly the requested width and height, whatever the + # source aspect ratio was. + assert download["resize"]["to"] == {"width": 100, "height": 100} stored = Path(download["path"]).read_bytes() - assert png_size(stored) == (100, 50) + assert png_size(stored) == (100, 100) assert download["width"] == 100 - assert download["height"] == 50 + assert download["height"] == 100 def test_download_without_resize_keeps_original_payload( diff --git a/tests/test_download_resize.py b/tests/test_download_resize.py new file mode 100644 index 0000000..9dcd614 --- /dev/null +++ b/tests/test_download_resize.py @@ -0,0 +1,326 @@ +"""Saving selected images into the caller directory and downscaling on demand. + +The contract under test: + +* ``--download`` writes every selected image into the directory named by the + caller and reports paths that really exist and really open as the requested + format; +* ``--download`` without ``--output-dir`` is an ``invalid_request`` -- there is + no hidden default directory; +* one item whose save fails is reported in that item while the run ends in the + distinct ``partial_download`` status (and exit code); +* ``--resize`` stores exactly the requested width and height, re-encoded in the + requested format, and never enlarges or otherwise rewrites an image that + already fits; without the flag the original bytes and dimensions are kept. + +Every fixture is a real payload built with :mod:`tools.images`, so the tests +assert on decoded files rather than on helper functions agreeing with +themselves. Nothing here touches the network. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any, Sequence + +import pytest + +from tools import envelope, example, images +from tools.errors import RequestFieldError + + +class FakeCandidate: + """Minimal stand-in for :class:`tools.sources.SourceCandidate`.""" + + def __init__(self, title: str, payload: bytes, *, fail: bool = False) -> None: + self.source_id = "fake" + self.title = title + self.file_url = f"https://example.test/{title.replace(' ', '-')}" + self.page_url = self.file_url + ".html" + self.mime_type = "image/png" + self.license = "CC0" + self.creator = "tester" + self.width: int | None = None + self.height: int | None = None + self._payload = payload + self._fail = fail + + def to_dict(self) -> dict[str, Any]: + return {"source_id": self.source_id, "title": self.title, "file_url": self.file_url} + + def fetch_bytes(self) -> bytes: + if self._fail: + raise OSError("download refused") + return self._payload + + +class FakeRegistry: + def __init__(self, candidates: Sequence[FakeCandidate] = ()) -> None: + self._candidates = list(candidates) + + def available_sources(self) -> Sequence[str]: + return ["fake"] + + def search(self, source_id: str, topic: str, limit: int = 10) -> list[FakeCandidate]: + return self._candidates[:limit] + + +def run_main( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + registry: FakeRegistry, + *args: str, +) -> tuple[int, dict[str, Any]]: + monkeypatch.setattr(example, "default_registry", lambda: registry) + code = example.main(list(args)) + captured = capsys.readouterr() + assert captured.err == "" + lines = [line for line in captured.out.splitlines() if line.strip()] + assert len(lines) == 1, captured.out + return code, json.loads(lines[0]) + + +def png(width: int, height: int) -> bytes: + return images.png_bytes(width, height) + + +def ico(width: int, height: int) -> bytes: + return images.ico_bytes(width, height) + + +def saved_image(path: Path) -> images.DecodedImage: + """Decode a reported path, failing the test if it cannot be opened.""" + + assert path.is_file(), f"reported path does not exist: {path}" + return images.decode_image(path.read_bytes()) + + +PNG_ARGS = ("--topic", "fox", "--format", "png", "--source", "fake", "--width", "100", "--height", "100") +ICO_ARGS = ("--topic", "fox", "--format", "ico", "--source", "fake", "--width", "50", "--height", "50") + + +# --------------------------------------------------------------------------- +# tools.images: the PNG/ICO helpers the saver is built on +# --------------------------------------------------------------------------- + + +def test_encode_and_decode_png_round_trip_the_requested_size() -> None: + rows = (bytes((10, 20, 30, 255)) * 4,) * 3 + + payload = images.encode_png(4, 3, rows) + + assert images.detect_format(payload) == images.PNG + decoded = images.decode_image(payload) + assert (decoded.format, decoded.width, decoded.height) == ("png", 4, 3) + assert decoded.rows == rows + + +def test_ico_wrap_and_unwrap_round_trip_the_embedded_png() -> None: + inner = png(6, 5) + + payload = images.ico_wrap(inner, 6, 5) + + assert images.detect_format(payload) == images.ICO + assert images.ico_unwrap(payload) == (6, 5, inner) + decoded = images.decode_image(payload) + assert (decoded.format, decoded.width, decoded.height) == ("ico", 6, 5) + + +def test_resample_rows_hits_the_target_size_exactly() -> None: + rows = tuple(bytes((x, x, x, 255)) * 4 for x in range(4)) + + resized = images.resample_rows(rows, 4, 4, 2, 3) + + assert len(resized) == 3 + assert {len(row) for row in resized} == {2 * 4} + + +def test_encode_image_re_encodes_both_accepted_formats() -> None: + rows = (bytes((1, 2, 3, 255)) * 2,) * 2 + + png_payload = images.encode_image("png", 2, 2, rows) + ico_payload = images.encode_image("ico", 2, 2, rows) + + assert images.decode_image(png_payload).format == images.PNG + assert images.decode_image(ico_payload).format == images.ICO + + +# --------------------------------------------------------------------------- +# --download: caller directory, reported paths, missing directory, save failure +# --------------------------------------------------------------------------- + + +def test_download_writes_into_the_caller_directory( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path +) -> None: + registry = FakeRegistry([FakeCandidate("fox one", png(100, 100))]) + + code, document = run_main(monkeypatch, capsys, registry, *PNG_ARGS, "--download", "--output-dir", str(tmp_path)) + + assert code == envelope.EXIT_OK + assert document["status"] == envelope.STATUS_OK + download = document["items"][0]["download"] + assert download["ok"] is True + target = Path(download["path"]) + assert target.parent == tmp_path + assert target.suffix == ".png" + stored = saved_image(target) + assert (stored.format, stored.width, stored.height) == ("png", 100, 100) + assert download["format"] == "png" + assert (download["width"], download["height"]) == (100, 100) + assert sorted(path.name for path in tmp_path.iterdir()) == [target.name] + + +def test_download_without_output_dir_is_invalid_request( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path +) -> None: + registry = FakeRegistry([FakeCandidate("fox one", png(100, 100))]) + + code, document = run_main(monkeypatch, capsys, registry, *PNG_ARGS, "--download") + + assert code == envelope.EXIT_INVALID_REQUEST + assert document["status"] == envelope.STATUS_INVALID_REQUEST + assert document["error"]["code"] == "request_field" + assert document["error"]["details"]["field"] == "output_dir" + # Nothing was written anywhere, least of all into a hidden default. + assert list(tmp_path.iterdir()) == [] + + +def test_request_document_without_output_dir_is_invalid_request() -> None: + with pytest.raises(RequestFieldError) as excinfo: + example.normalize_request({"topic": "fox", "format": "png", "download": True}) + + assert excinfo.value.field == "output_dir" + # Without --download the directory is not needed, so it stays optional. + request = example.normalize_request({"topic": "fox", "format": "png"}) + assert request["download"] is False + assert request["output_dir"] is None + + +def test_failing_save_yields_partial_download( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path +) -> None: + registry = FakeRegistry( + [FakeCandidate("fox one", png(100, 100)), FakeCandidate("fox two", png(100, 100))] + ) + # The second destination is a directory, so writing the file must fail while + # the first item still saves successfully. + blocked = tmp_path / "002-fox-two.png" + blocked.mkdir() + + code, document = run_main( + monkeypatch, capsys, registry, *PNG_ARGS, "--count", "2", "--download", "--output-dir", str(tmp_path) + ) + + assert code == envelope.EXIT_PARTIAL_DOWNLOAD == 6 + assert document["status"] == envelope.STATUS_PARTIAL_DOWNLOAD + first, second = document["items"] + assert first["download"]["ok"] is True + assert saved_image(Path(first["download"]["path"])).width == 100 + assert second["download"]["ok"] is False + assert second["download"]["error"]["code"] + assert "path" not in second["download"] + assert document["error"]["code"] == "download_incomplete" + assert document["error"]["details"]["failed"] == 1 + # The path list only names files that exist and open as the requested format. + saved_paths = document["error"]["details"]["saved_paths"] + assert saved_paths == [first["download"]["path"]] + for path in saved_paths: + assert saved_image(Path(path)).format == "png" + + +# --------------------------------------------------------------------------- +# --resize: exact requested size, requested format, never enlarge +# --------------------------------------------------------------------------- + + +def test_resize_stores_exactly_the_requested_size( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path +) -> None: + # 200x100 is larger than the 100x100 request in one dimension only: --resize + # still stores exactly 100x100 rather than preserving the aspect ratio. + registry = FakeRegistry([FakeCandidate("fox one", png(200, 100))]) + + code, document = run_main( + monkeypatch, capsys, registry, *PNG_ARGS, "--resize", "--download", "--output-dir", str(tmp_path) + ) + + assert code == envelope.EXIT_OK + download = document["items"][0]["download"] + assert download["resize"]["applied"] is True + assert download["resize"]["from"] == {"width": 200, "height": 100} + assert download["resize"]["to"] == {"width": 100, "height": 100} + stored = saved_image(Path(download["path"])) + assert (stored.format, stored.width, stored.height) == ("png", 100, 100) + + +def test_resize_re_encodes_ico_in_the_requested_format( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path +) -> None: + registry = FakeRegistry([FakeCandidate("fox one", ico(100, 100))]) + + code, document = run_main( + monkeypatch, capsys, registry, *ICO_ARGS, "--resize", "--download", "--output-dir", str(tmp_path) + ) + + assert code == envelope.EXIT_OK + download = document["items"][0]["download"] + target = Path(download["path"]) + assert target.suffix == ".ico" + stored = saved_image(target) + assert (stored.format, stored.width, stored.height) == ("ico", 50, 50) + assert images.read_ico_dimensions(target.read_bytes()) == (50, 50) + + +def test_resize_never_enlarges_or_rewrites_a_fitting_image( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path +) -> None: + payload = png(100, 100) + registry = FakeRegistry([FakeCandidate("fox one", payload)]) + + code, document = run_main( + monkeypatch, capsys, registry, *PNG_ARGS, "--resize", "--download", "--output-dir", str(tmp_path) + ) + + assert code == envelope.EXIT_OK + download = document["items"][0]["download"] + assert download["resize"]["applied"] is False + assert download["resize"]["reason"] == "already_fits" + assert Path(download["path"]).read_bytes() == payload + assert (download["width"], download["height"]) == (100, 100) + + +def test_without_resize_the_original_bytes_and_dimensions_are_kept( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path +) -> None: + payload = png(200, 100) + registry = FakeRegistry([FakeCandidate("fox one", payload)]) + + code, document = run_main( + monkeypatch, capsys, registry, *PNG_ARGS, "--download", "--output-dir", str(tmp_path) + ) + + assert code == envelope.EXIT_OK + download = document["items"][0]["download"] + assert "resize" not in download + assert Path(download["path"]).read_bytes() == payload + stored = saved_image(Path(download["path"])) + assert (stored.format, stored.width, stored.height) == ("png", 200, 100) + + +def test_without_resize_an_ico_keeps_its_container_and_size( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path +) -> None: + payload = ico(100, 100) + registry = FakeRegistry([FakeCandidate("fox one", payload)]) + + code, document = run_main( + monkeypatch, capsys, registry, *ICO_ARGS, "--download", "--output-dir", str(tmp_path) + ) + + assert code == envelope.EXIT_OK + download = document["items"][0]["download"] + assert Path(download["path"]).read_bytes() == payload + stored = saved_image(Path(download["path"])) + assert (stored.format, stored.width, stored.height) == ("ico", 100, 100) diff --git a/tools/example.py b/tools/example.py index ce667c4..36fdb03 100644 --- a/tools/example.py +++ b/tools/example.py @@ -14,9 +14,7 @@ from __future__ import annotations import argparse import json import re -import struct import sys -import zlib from dataclasses import dataclass, field from pathlib import Path from typing import Any, Mapping, Sequence @@ -65,7 +63,6 @@ REQUEST_FIELDS: tuple[str, ...] = ( OPTIONAL_DOCUMENT_FIELDS: tuple[str, ...] = ("version",) DEFAULT_SOURCE = "wikimedia_commons" -DEFAULT_OUTPUT_DIR = "downloads" MIN_COUNT = 1 MAX_COUNT = 50 @@ -162,16 +159,16 @@ def build_parser() -> argparse.ArgumentParser: "-o", "--output-dir", metavar="DIR", - help=f"download directory (default: ./{DEFAULT_OUTPUT_DIR})", + help="caller-specified download directory; required with --download", ) parser.add_argument( "--resize", action="store_true", help=( - "downscale every downloaded image so it fits the requested width " - "and height, keeping the aspect ratio and never scaling up; " - "without it the original bytes are written and the actual size is " - "reported" + "downscale every downloaded image that is larger than the request " + "in either dimension to exactly the requested width and height, " + "re-encoded in the requested format; images are never scaled up; " + "without it the original bytes and dimensions are written" ), ) parser.add_argument( @@ -307,10 +304,20 @@ def normalize_request(raw: Mapping[str, Any]) -> dict[str, Any]: details={"min": MIN_COUNT, "max": MAX_COUNT}, ) - output_dir = DEFAULT_OUTPUT_DIR + output_dir: str | None = None if raw.get("output_dir") is not None: output_dir = _as_text(raw["output_dir"], "output_dir") + download = _as_bool(raw.get("download", False), "download") + if download and output_dir is None: + # There is no hidden default: a download run without a caller-supplied + # directory could only guess at where the files should go. + raise RequestFieldError( + "output_dir is required when download is enabled; pass --output-dir ", + field="output_dir", + details={"required_with": "download", "accepted_fields": list(REQUEST_FIELDS)}, + ) + return { "topic": topic, "width": size["width"], @@ -318,7 +325,7 @@ def normalize_request(raw: Mapping[str, Any]) -> dict[str, Any]: "format": image_format, "source": source, "count": count, - "download": _as_bool(raw.get("download", False), "download"), + "download": download, "output_dir": output_dir, "resize": _as_bool(raw.get("resize", False), "resize"), "dry_run": _as_bool(raw.get("dry_run", False), "dry_run"), @@ -489,318 +496,118 @@ def _slug(candidate: SourceCandidate) -> str: return (slug or "image")[:60] -PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n" -ICO_HEADER_SIZE = 6 -ICO_ENTRY_SIZE = 16 -ICO_MAX_DIMENSION = 256 -_PNG_CHANNELS = {0: 1, 2: 3, 3: 1, 4: 2, 6: 4} - - -def _png_chunks(payload: bytes) -> list[tuple[bytes, bytes]]: - """Split a PNG payload into ``(type, data)`` chunks.""" - - if not payload.startswith(PNG_SIGNATURE): - raise ImageResizeError("payload is not a PNG image") - chunks: list[tuple[bytes, bytes]] = [] - offset = len(PNG_SIGNATURE) - while offset + 8 <= len(payload): - (length,) = struct.unpack(">I", payload[offset : offset + 4]) - chunk_type = payload[offset + 4 : offset + 8] - data = payload[offset + 8 : offset + 8 + length] - if len(data) != length: - raise ImageResizeError("truncated PNG chunk") - chunks.append((chunk_type, data)) - offset += 12 + length - if chunk_type == b"IEND": - return chunks - raise ImageResizeError("PNG image has no IEND chunk") - - -def _unfilter_rows(raw: bytes, width: int, height: int, bpp: int) -> list[bytearray]: - """Undo the per-scanline PNG filters, returning one row per pixel line.""" - - stride = width * bpp - if len(raw) < height * (stride + 1): - raise ImageResizeError("truncated PNG image data") - rows: list[bytearray] = [] - previous = bytearray(stride) - position = 0 - for _ in range(height): - filter_type = raw[position] - position += 1 - line = bytearray(raw[position : position + stride]) - position += stride - if filter_type == 1: - for index in range(bpp, stride): - line[index] = (line[index] + line[index - bpp]) & 0xFF - elif filter_type == 2: - for index in range(stride): - line[index] = (line[index] + previous[index]) & 0xFF - elif filter_type == 3: - for index in range(stride): - left = line[index - bpp] if index >= bpp else 0 - line[index] = (line[index] + ((left + previous[index]) >> 1)) & 0xFF - elif filter_type == 4: - for index in range(stride): - left = line[index - bpp] if index >= bpp else 0 - up = previous[index] - up_left = previous[index - bpp] if index >= bpp else 0 - estimate = left + up - up_left - to_left = abs(estimate - left) - to_up = abs(estimate - up) - to_up_left = abs(estimate - up_left) - if to_left <= to_up and to_left <= to_up_left: - predictor = left - elif to_up <= to_up_left: - predictor = up - else: - predictor = up_left - line[index] = (line[index] + predictor) & 0xFF - elif filter_type != 0: - raise ImageResizeError(f"unsupported PNG filter type {filter_type}") - previous = line - rows.append(line) - return rows - - -def _row_to_rgba( - row: bytearray, - width: int, - color_type: int, - depth: int, - palette: bytes, - transparency: bytes | None, -) -> bytes: - """Convert one unfiltered scanline into 8-bit RGBA samples.""" - - step = depth // 8 - out = bytearray() - if color_type == 3: - for x in range(width): - index = row[x] - base = index * 3 - if base + 2 >= len(palette): - raise ImageResizeError("PNG palette index is out of range") - alpha = transparency[index] if transparency and index < len(transparency) else 255 - out += bytes((palette[base], palette[base + 1], palette[base + 2], alpha)) - return bytes(out) - for x in range(width): - offset = x * step - if color_type == 0: - value = row[offset] - out += bytes((value, value, value, 255)) - elif color_type == 4: - value = row[offset] - out += bytes((value, value, value, row[offset + step])) - elif color_type == 2: - out += bytes((row[offset], row[offset + step], row[offset + 2 * step], 255)) - else: - out += bytes( - ( - row[offset], - row[offset + step], - row[offset + 2 * step], - row[offset + 3 * step], - ) - ) - return bytes(out) - - -def _decode_png(payload: bytes) -> tuple[int, int, list[bytes]]: - """Decode a PNG payload into ``(width, height, rgba_rows)``.""" - - header: tuple[int, ...] | None = None - palette = b"" - transparency: bytes | None = None - data = bytearray() - for chunk_type, chunk_data in _png_chunks(payload): - if chunk_type == b"IHDR": - if len(chunk_data) < 13: - raise ImageResizeError("truncated PNG header") - header = struct.unpack(">IIBBBBB", chunk_data[:13]) - elif chunk_type == b"PLTE": - palette = chunk_data - elif chunk_type == b"tRNS": - transparency = chunk_data - elif chunk_type == b"IDAT": - data += chunk_data - if header is None: - raise ImageResizeError("PNG image has no IHDR chunk") - width, height, depth, color_type, compression, filter_method, interlace = header - if width < 1 or height < 1: - raise ImageResizeError("PNG image has no pixels") - if compression != 0 or filter_method != 0: - raise ImageResizeError("unsupported PNG compression or filter method") - if interlace != 0: - raise ImageResizeError("interlaced PNG images are not supported") - channels = _PNG_CHANNELS.get(color_type) - if channels is None: - raise ImageResizeError(f"unsupported PNG colour type {color_type}") - if depth not in (8, 16): - raise ImageResizeError(f"unsupported PNG bit depth {depth}") - if color_type == 3 and depth > 8: - raise ImageResizeError("unsupported PNG palette bit depth") - if color_type == 3 and not palette: - raise ImageResizeError("PNG palette image has no PLTE chunk") - try: - raw = zlib.decompress(bytes(data)) - except zlib.error as exc: - raise ImageResizeError(f"PNG image data is corrupt: {exc}") from exc - rows = _unfilter_rows(raw, width, height, channels * (depth // 8)) - return width, height, [ - _row_to_rgba(row, width, color_type, depth, palette, transparency) for row in rows - ] - - -def _encode_png(width: int, height: int, rows: Sequence[bytes]) -> bytes: - """Encode RGBA rows as an uncompressed-filter 8-bit PNG.""" - - raw = bytearray() - for row in rows: - raw.append(0) - raw += row - - def chunk(chunk_type: bytes, chunk_data: bytes) -> bytes: - crc = zlib.crc32(chunk_type + chunk_data) & 0xFFFFFFFF - return struct.pack(">I", len(chunk_data)) + chunk_type + chunk_data + struct.pack(">I", crc) - - header = struct.pack(">IIBBBBB", width, height, 8, 6, 0, 0, 0) - return ( - PNG_SIGNATURE - + chunk(b"IHDR", header) - + chunk(b"IDAT", zlib.compress(bytes(raw), 6)) - + chunk(b"IEND", b"") - ) - - -def _ico_entries(payload: bytes) -> list[tuple[int, int, int, int]]: - """Return ``(width, height, size, offset)`` for every ICO directory entry.""" - - if len(payload) < ICO_HEADER_SIZE: - raise ImageResizeError("payload is not an ICO image") - reserved, kind, count = struct.unpack(" len(payload): - raise ImageResizeError("truncated ICO directory") - width, height, _colours, _reserved, _planes, _bits, size, offset = struct.unpack( - " bytes: - """Wrap a PNG payload in a single-entry ICO container.""" - - if width > ICO_MAX_DIMENSION or height > ICO_MAX_DIMENSION: - raise ImageResizeError("ICO images cannot exceed 256x256 pixels") - header = struct.pack(" tuple[str, int, int, list[bytes]]: - """Decode a PNG or ICO payload into ``(container, width, height, rows)``.""" - - if payload.startswith(PNG_SIGNATURE): - width, height, rows = _decode_png(payload) - return "png", width, height, rows - if len(payload) >= ICO_HEADER_SIZE and payload[:4] == b"\x00\x00\x01\x00": - entries = _ico_entries(payload) - _width, _height, size, offset = max(entries, key=lambda item: item[0] * item[1]) - embedded = payload[offset : offset + size] - if not embedded.startswith(PNG_SIGNATURE): - raise ImageResizeError("ICO images stored as raw bitmaps are not supported") - width, height, rows = _decode_png(embedded) - return "ico", width, height, rows - raise ImageResizeError("payload is neither a PNG nor an ICO image") - - def _target_size(width: int, height: int, request: Mapping[str, Any]) -> tuple[int, int]: - """Shrink-to-fit target inside the requested box, never enlarging.""" + """Return the exact size to store: the requested size, never enlarged. + + ``--resize`` means "give me the requested size" rather than "give me + something that fits in it": a requested width **and** height are hit + exactly, and a dimension the caller left open keeps the source value. A + source dimension that is already at or below the requested one is never + scaled up, so ``--resize`` can only ever shrink an image. + """ wanted_width = request.get("width") wanted_height = request.get("height") - scale = 1.0 - if wanted_width is not None: - scale = min(scale, wanted_width / width) - if wanted_height is not None: - scale = min(scale, wanted_height / height) - if scale >= 1.0: - return width, height - return max(1, int(round(width * scale))), max(1, int(round(height * scale))) - - -def _resample(rows: Sequence[bytes], width: int, height: int, target_width: int, target_height: int) -> list[bytes]: - """Nearest-neighbour downscale of RGBA rows.""" - - columns = [min(width - 1, (x * width) // target_width) * 4 for x in range(target_width)] - resized: list[bytes] = [] - for y in range(target_height): - source = rows[min(height - 1, (y * height) // target_height)] - line = bytearray() - for offset in columns: - line += source[offset : offset + 4] - resized.append(bytes(line)) - return resized + target_width = width if wanted_width is None else min(width, int(wanted_width)) + target_height = height if wanted_height is None else min(height, int(wanted_height)) + return max(1, target_width), max(1, target_height) def resize_image(payload: bytes, request: Mapping[str, Any]) -> tuple[bytes, dict[str, Any]]: - """Downscale ``payload`` so it fits the requested size. + """Downscale ``payload`` to the requested size and re-encode it. - The requested width and height are a bounding box: the aspect ratio is - preserved, an image that already fits is left untouched and an image is - never scaled up. The returned fragment records what happened so the caller - can report the real dimensions of the stored file. + The stored image is re-encoded in the requested container format (``png`` + or ``ico``) so the saved bytes really open as that format. An image that + is not larger than the request in either dimension is returned untouched: + ``--resize`` never enlarges and never rewrites pixels it does not have to. """ - container, width, height, rows = _decode_image(payload) - target_width, target_height = _target_size(width, height, request) + decoded = images.decode_image(payload) + wanted_format = str(request.get("format") or decoded.format) + if decoded.format != wanted_format: + raise ImageResizeError( + f"cannot re-encode a {decoded.format} image as {wanted_format}", + details={"from_format": decoded.format, "to_format": wanted_format}, + ) + target_width, target_height = _target_size(decoded.width, decoded.height, request) fragment: dict[str, Any] = { "requested": {"width": request.get("width"), "height": request.get("height")}, - "from": {"width": width, "height": height}, + "from": {"width": decoded.width, "height": decoded.height}, + "format": wanted_format, } - if (target_width, target_height) == (width, height): + if (target_width, target_height) == (decoded.width, decoded.height): fragment.update( - {"applied": False, "reason": "already_fits", "to": {"width": width, "height": height}} + { + "applied": False, + "reason": "already_fits", + "to": {"width": decoded.width, "height": decoded.height}, + } ) return payload, fragment - png_payload = _encode_png( - target_width, target_height, _resample(rows, width, height, target_width, target_height) - ) - output = ( - _encode_ico(png_payload, target_width, target_height) - if container == "ico" - else png_payload + rows = images.resample_rows( + decoded.rows, decoded.width, decoded.height, target_width, target_height ) + output = images.encode_image(wanted_format, target_width, target_height, rows) fragment.update({"applied": True, "to": {"width": target_width, "height": target_height}}) return output, fragment +def _discard(path: Path) -> None: + """Remove a file whose stored bytes failed verification.""" + + try: + path.unlink() + except OSError: + # Never chase a path that is not a regular file we just wrote: the run + # must not delete anything else in the caller's directory. + pass + + +def _verify_saved( + path: Path, request: Mapping[str, Any], expected: tuple[int, int] +) -> ImageInfo: + """Read a stored file back and prove it opens as the requested format. + + A reported path is only trustworthy when the file really exists and really + holds bytes of the requested format at the expected size, so the stored + payload is decoded again instead of holding the writer to its word. + """ + + wanted_format = str(request["format"]) + decoded = images.decode_image(path.read_bytes()) + if decoded.format != wanted_format: + raise ImageFormatError( + "stored file does not open as the requested format", + details={ + "path": str(path), + "expected_format": wanted_format, + "actual_format": decoded.format, + }, + ) + if (decoded.width, decoded.height) != expected: + raise ImageFormatError( + "stored file does not have the expected pixel size", + details={ + "path": str(path), + "expected": {"width": expected[0], "height": expected[1]}, + "actual": {"width": decoded.width, "height": decoded.height}, + }, + ) + return ImageInfo(format=decoded.format, width=decoded.width, height=decoded.height) + + def _download( selected: Sequence[KeptImage], request: Mapping[str, Any] ) -> tuple[list[dict[str, Any]], int]: """Write the already-fetched payloads; return ``(items, failure_count)``. - The bytes verified by :func:`_select` are stored as they are (or downscaled - first when ``resize`` is enabled), so an item can never be downloaded twice - or stored in a format that was not verified. A payload that cannot be - downscaled to the requested size is reported as a failed item instead of - being silently stored at the wrong size. + Every selected image is stored inside the caller-specified directory. The + bytes verified by :func:`_select` are written as they are, or downscaled + first when ``resize`` is enabled, so an item can never be downloaded twice + or stored in a format that was not verified. Each stored file is read back + and decoded before its path is reported, and a file that does not open as + the requested format at the expected size is deleted and reported as a + per-item save failure instead of being left behind as a broken path. """ directory = Path(request["output_dir"]).expanduser() @@ -819,23 +626,38 @@ def _download( record = entry.to_item() destination = directory / f"{index:03d}-{_slug(entry.candidate)}{suffix}" payload = entry.payload + resize_fragment: dict[str, Any] | None = None + expected = (entry.info.width, entry.info.height) try: - resize_fragment = resize_image(payload, request) if request["resize"] else None - if resize_fragment is not None: - payload, resize_fragment = resize_fragment + if request["resize"]: + payload, resize_fragment = resize_image(payload, request) + expected = ( + int(resize_fragment["to"]["width"]), + int(resize_fragment["to"]["height"]), + ) destination.write_bytes(payload) + stored = _verify_saved(destination, request, expected) except (ToolkitError, OSError) as exc: + _discard(destination) failures += 1 error = error_fragment(getattr(exc, "code", "download_failed"), str(exc)) if isinstance(exc, ToolkitError) and exc.details: error["details"] = dict(exc.details) - record["download"] = {"ok": False, "error": error} + # A failed save reports no path: only files that really exist and + # really open are ever named as saved. + record["download"] = {"ok": False, "target": str(destination), "error": error} else: - record["download"] = {"ok": True, "path": str(destination), "bytes": len(payload)} + download: dict[str, Any] = { + "ok": True, + "path": str(destination), + "bytes": len(payload), + "format": stored.format, + "width": stored.width, + "height": stored.height, + } if resize_fragment is not None: - record["download"]["resize"] = resize_fragment - record["download"]["width"] = resize_fragment["to"]["width"] - record["download"]["height"] = resize_fragment["to"]["height"] + download["resize"] = resize_fragment + record["download"] = download items.append(record) return items, failures @@ -966,28 +788,26 @@ def _execute(argv: Sequence[str]) -> Response: # otherwise usable answer to a partial-download exit code. return builder.build(STATUS_OK, items=[entry.to_item() for entry in selection.kept]) - items, download_failures = _download(selection.kept, request) - failed = download_failures + len(unreadable) - if download_failures == len(items): - return builder.build( - STATUS_ACCESS_ERROR, - items=items, - error=error_fragment( - "download_failed", - "every download failed", - details={"failed": download_failures, "total": len(items)}, - ), - ) + items, save_failures = _download(selection.kept, request) + saved_paths = [ + item["download"]["path"] for item in items if item["download"].get("ok") is True + ] + failed = save_failures + len(unreadable) if failed: + # The run still reports every saved path, but a per-item save (or fetch) + # failure is never hidden behind ``ok``: it has its own status and exit + # code so the caller can tell a complete run from an incomplete one. return builder.build( STATUS_PARTIAL_DOWNLOAD, items=items, error=error_fragment( "download_incomplete", - f"{failed} of {len(items) + len(unreadable)} images could not be handled", + f"{failed} of {len(items) + len(unreadable)} images could not be saved", details={ "failed": failed, + "saved": len(saved_paths), "total": len(items) + len(unreadable), + "saved_paths": saved_paths, "failures": unreadable, }, ), diff --git a/tools/images.py b/tools/images.py index 5c1f31a..fd16617 100644 --- a/tools/images.py +++ b/tools/images.py @@ -22,6 +22,11 @@ It deliberately depends on nothing but the standard library: * :func:`inspect_image` combines both into an :class:`ImageInfo`. * :func:`png_bytes` and :func:`ico_bytes` *build* the same formats so tests (and callers) can produce real payloads without Pillow or any other dependency. +* :func:`decode_png`, :func:`encode_png`, :func:`ico_wrap` and + :func:`ico_unwrap` decode and re-encode those containers, and + :func:`decode_image`, :func:`resample_rows` and :func:`encode_image` are the + building blocks the CLI uses to downscale a downloaded image and store it in + the requested format -- still with nothing but the standard library. :class:`ImageFormatError` -- a :class:`~tools.errors.ToolkitError` subclass with a stable ``code`` -- is reserved for bytes that are genuinely unusable: an @@ -63,6 +68,15 @@ __all__ = [ "png_bytes", "read_ico_dimensions", "read_png_dimensions", + "DecodedImage", + "decode_image", + "decode_png", + "encode_image", + "encode_png", + "ico_unwrap", + "ico_wrap", + "png_chunks", + "resample_rows", ] #: The two formats the CLI accepts as search requests. @@ -401,3 +415,357 @@ def ico_bytes( ) offset += len(payload) return header + bytes(entries) + b"".join(payloads) + + +# --------------------------------------------------------------------------- +# Decoding and re-encoding: PNG pixels and ICO containers +# --------------------------------------------------------------------------- + +#: Channels per PNG colour type (0 grey, 2 rgb, 3 palette, 4 grey+alpha, 6 rgba). +_PNG_CHANNELS = {0: 1, 2: 3, 3: 1, 4: 2, 6: 4} + + +@dataclass(frozen=True) +class DecodedImage: + """The container format, pixel size and 8-bit RGBA scanlines of a payload.""" + + format: str + width: int + height: int + rows: tuple[bytes, ...] + + def to_dict(self) -> dict[str, Any]: + """JSON-ready projection: format plus the real pixel size.""" + + return {"format": self.format, "width": self.width, "height": self.height} + + +def png_chunks(payload: bytes) -> tuple[tuple[bytes, bytes], ...]: + """Split a PNG payload into ``(chunk_type, chunk_data)`` pairs. + + Raises :class:`ImageFormatError` when the signature is missing, a chunk is + truncated or the stream never reaches ``IEND``: those bytes cannot be + decoded, so stitching a half image would be worse than reporting it. + """ + + data = bytes(payload) + if not data.startswith(PNG_SIGNATURE): + raise ImageFormatError( + "payload does not start with the PNG signature", + details={"signature": _probe(data).hex()}, + ) + chunks: list[tuple[bytes, bytes]] = [] + offset = len(PNG_SIGNATURE) + while offset + 8 <= len(data): + (length,) = struct.unpack(">I", data[offset : offset + 4]) + chunk_type = data[offset + 4 : offset + 8] + chunk_data = data[offset + 8 : offset + 8 + length] + if len(chunk_data) != length: + raise ImageFormatError( + "PNG chunk is truncated", + details={"chunk": chunk_type.decode("latin-1"), "length": length}, + ) + chunks.append((chunk_type, chunk_data)) + offset += 12 + length + if chunk_type == b"IEND": + return tuple(chunks) + raise ImageFormatError("PNG image has no IEND chunk") + + +def _unfilter_rows(raw: bytes, width: int, height: int, bpp: int) -> list[bytearray]: + """Undo the per-scanline PNG filters, returning one row per pixel line.""" + + stride = width * bpp + if len(raw) < height * (stride + 1): + raise ImageFormatError( + "PNG image data is truncated", + details={"width": width, "height": height, "bytes": len(raw)}, + ) + rows: list[bytearray] = [] + previous = bytearray(stride) + position = 0 + for _ in range(height): + filter_type = raw[position] + position += 1 + line = bytearray(raw[position : position + stride]) + position += stride + if filter_type == 1: + for index in range(bpp, stride): + line[index] = (line[index] + line[index - bpp]) & 0xFF + elif filter_type == 2: + for index in range(stride): + line[index] = (line[index] + previous[index]) & 0xFF + elif filter_type == 3: + for index in range(stride): + left = line[index - bpp] if index >= bpp else 0 + line[index] = (line[index] + ((left + previous[index]) >> 1)) & 0xFF + elif filter_type == 4: + for index in range(stride): + left = line[index - bpp] if index >= bpp else 0 + up = previous[index] + up_left = previous[index - bpp] if index >= bpp else 0 + estimate = left + up - up_left + to_left = abs(estimate - left) + to_up = abs(estimate - up) + to_up_left = abs(estimate - up_left) + if to_left <= to_up and to_left <= to_up_left: + predictor = left + elif to_up <= to_up_left: + predictor = up + else: + predictor = up_left + line[index] = (line[index] + predictor) & 0xFF + elif filter_type != 0: + raise ImageFormatError( + "unsupported PNG filter type", + details={"filter_type": int(filter_type)}, + ) + previous = line + rows.append(line) + return rows + + +def _row_to_rgba( + row: bytearray, + width: int, + color_type: int, + depth: int, + palette: bytes, + transparency: bytes | None, +) -> bytes: + """Convert one unfiltered scanline into 8-bit RGBA samples.""" + + step = depth // 8 + # Bytes per pixel: one sample per channel, so the channel offsets inside a + # pixel are ``step`` apart while pixels are ``channels * step`` apart. + pixel_stride = _PNG_CHANNELS[color_type] * step + out = bytearray() + if color_type == 3: + for x in range(width): + index = row[x] + base = index * 3 + if base + 2 >= len(palette): + raise ImageFormatError( + "PNG palette index is out of range", + details={"index": int(index), "palette_bytes": len(palette)}, + ) + alpha = transparency[index] if transparency and index < len(transparency) else 255 + out += bytes((palette[base], palette[base + 1], palette[base + 2], alpha)) + return bytes(out) + for x in range(width): + offset = x * pixel_stride + if color_type == 0: + value = row[offset] + out += bytes((value, value, value, 255)) + elif color_type == 4: + value = row[offset] + out += bytes((value, value, value, row[offset + step])) + elif color_type == 2: + out += bytes((row[offset], row[offset + step], row[offset + 2 * step], 255)) + else: + out += bytes( + ( + row[offset], + row[offset + step], + row[offset + 2 * step], + row[offset + 3 * step], + ) + ) + return bytes(out) + + +def decode_png(payload: bytes) -> tuple[int, int, tuple[bytes, ...]]: + """Decode a PNG payload into ``(width, height, rgba_rows)``. + + Supports the non-interlaced 8-bit and 16-bit PNG colour types ImageMagick, + GIMP and the Wikimedia thumbnailers produce; anything else raises + :class:`ImageFormatError` instead of guessing at pixels. + """ + + header: tuple[int, ...] | None = None + palette = b"" + transparency: bytes | None = None + data = bytearray() + for chunk_type, chunk_data in png_chunks(payload): + if chunk_type == b"IHDR": + if len(chunk_data) < 13: + raise ImageFormatError("PNG header is truncated") + header = struct.unpack(">IIBBBBB", chunk_data[:13]) + elif chunk_type == b"PLTE": + palette = chunk_data + elif chunk_type == b"tRNS": + transparency = chunk_data + elif chunk_type == b"IDAT": + data += chunk_data + if header is None: + raise ImageFormatError("PNG image has no IHDR chunk") + width, height, depth, color_type, compression, filter_method, interlace = header + if width < 1 or height < 1: + raise ImageFormatError("PNG image declares no pixels") + if compression != 0 or filter_method != 0: + raise ImageFormatError( + "unsupported PNG compression or filter method", + details={"compression": compression, "filter_method": filter_method}, + ) + if interlace != 0: + raise ImageFormatError( + "interlaced PNG images are not supported", + details={"interlace": interlace}, + ) + channels = _PNG_CHANNELS.get(color_type) + if channels is None: + raise ImageFormatError( + "unsupported PNG colour type", + details={"color_type": color_type}, + ) + if depth not in (8, 16): + raise ImageFormatError( + "unsupported PNG bit depth", + details={"bit_depth": depth}, + ) + if color_type == 3 and depth > 8: + raise ImageFormatError( + "unsupported PNG palette bit depth", + details={"bit_depth": depth}, + ) + if color_type == 3 and not palette: + raise ImageFormatError("PNG palette image has no PLTE chunk") + try: + raw = zlib.decompress(bytes(data)) + except zlib.error as exc: + raise ImageFormatError( + "PNG image data is corrupt", + details={"reason": str(exc)}, + ) from exc + rows = _unfilter_rows(raw, width, height, channels * (depth // 8)) + rgba_rows = tuple( + _row_to_rgba(row, width, color_type, depth, palette, transparency) for row in rows + ) + return int(width), int(height), rgba_rows + + +def encode_png(width: int, height: int, rows: Sequence[bytes]) -> bytes: + """Encode 8-bit RGBA ``rows`` as a real PNG with correct chunk CRCs.""" + + return png_bytes(width, height, rows=list(rows)) + + +def ico_wrap(embedded_png: bytes, width: int, height: int) -> bytes: + """Wrap a PNG payload in a single-entry ICO container of that size.""" + + if not 1 <= width <= ICO_MAX_DIMENSION: + raise ImageFormatError( + "ICO widths must be between 1 and 256 pixels", + details={"width": width}, + ) + if not 1 <= height <= ICO_MAX_DIMENSION: + raise ImageFormatError( + "ICO heights must be between 1 and 256 pixels", + details={"height": height}, + ) + png_payload = bytes(embedded_png) + if not png_payload.startswith(PNG_SIGNATURE): + raise ImageFormatError("an ICO container can only wrap a PNG payload here") + header = struct.pack(" tuple[int, int, bytes]: + """Return the ``(width, height, embedded_png)`` of an ICO's largest frame.""" + + entries = ico_entries(payload) + width, height, size, offset = max(entries, key=lambda entry: entry[0] * entry[1]) + embedded = bytes(payload)[offset : offset + size] + if not embedded.startswith(PNG_SIGNATURE): + raise ImageFormatError( + "ICO frames stored as raw bitmaps are not supported", + details={"width": width, "height": height}, + ) + return int(width), int(height), embedded + + +def decode_image(payload: bytes) -> DecodedImage: + """Decode a PNG or ICO payload into :class:`DecodedImage` RGBA scanlines.""" + + data = bytes(payload) + image_format = detect_format(data) + if image_format == PNG: + width, height, rows = decode_png(data) + return DecodedImage(format=PNG, width=width, height=height, rows=rows) + if image_format == ICO: + width, height, embedded = ico_unwrap(data) + inner_width, inner_height, rows = decode_png(embedded) + if (inner_width, inner_height) != (width, height): + # The directory is authoritative for the reported size, but the + # pixels must agree or the container is inconsistent. + raise ImageFormatError( + "ICO directory and embedded PNG disagree about the size", + details={ + "directory": {"width": width, "height": height}, + "embedded": {"width": inner_width, "height": inner_height}, + }, + ) + return DecodedImage(format=ICO, width=width, height=height, rows=rows) + raise ImageFormatError( + "pixel data is only decodable for png and ico", + details={"format": image_format}, + ) + + +def resample_rows( + rows: Sequence[bytes], + width: int, + height: int, + target_width: int, + target_height: int, +) -> tuple[bytes, ...]: + """Nearest-neighbour resample of RGBA ``rows`` to exactly the target size.""" + + if target_width < 1 or target_height < 1: + raise ImageFormatError("resampled dimensions must be at least 1x1") + if len(rows) != height: + raise ImageFormatError( + "row count does not match the source height", + details={"rows": len(rows), "height": height}, + ) + if width < 1 or height < 1: + raise ImageFormatError("source image has no pixels") + columns = [min(width - 1, (x * width) // target_width) * 4 for x in range(target_width)] + resized: list[bytes] = [] + for y in range(target_height): + source = rows[min(height - 1, (y * height) // target_height)] + line = bytearray() + for offset in columns: + line += source[offset : offset + 4] + resized.append(bytes(line)) + return tuple(resized) + + +def encode_image( + image_format: str, + width: int, + height: int, + rows: Sequence[bytes], +) -> bytes: + """Encode RGBA ``rows`` in ``image_format`` (``png`` or ``ico``).""" + + png_payload = encode_png(width, height, rows) + if image_format == PNG: + return png_payload + if image_format == ICO: + return ico_wrap(png_payload, width, height) + raise ImageFormatError( + "only png and ico can be encoded", + details={"format": image_format}, + ) -- 2.54.0 From d506c7a65235a20033f7b4c54b5a183378209ccc Mon Sep 17 00:00:00 2001 From: Tool Backend Date: Tue, 3 Nov 2026 02:34:16 +0000 Subject: [PATCH 09/10] change: Implement 6: Verify every CLI status branch end to end and run the live credential-free source check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added executable, process-level verification for every CLI status branch plus a live credential-free source check. - `tests/test_status_branches.py` (new): each case starts a fresh interpreter that runs the real `tools.example.main` entry point as a subprocess with a scrubbed environment (no API/KEY/TOKEN/SECRET/PASSWORD/CREDENTIAL/AUTH variables) and an injected offline source registry. It asserts the whole contract — exactly one JSON document with the fixed five keys, the expected `status`, the matching exit code, and the echoed source/request — for `ok`, `no_format`, `no_size`, `access_error` (both candidate-fetch and search failures), `invalid_request`/unknown source and `partial_download`. It also proves the branches have mutually distinct exit codes, that `no_format`/`no_size` are distinct explanations and never access errors, that partial download names only files that really exist, and that the shipped `examples/input.json` request runs and parses. - `tests/test_live_source.py` (new): searches a known Wikimedia Commons topic for png at ≥64x64 with no credential variables present, asserting the process contract on every live answer; on success it records the observed JSON (record_property + artifact file) and independently re-fetches each reported URL to prove it serves real PNG bytes at the reported dimensions. When the approved hosts are unusable the case is reported as an explicit unresolved skip carrying the observed status/reason — synthetic results are never substituted. In this environment the run observed `access_error` because `upload.wikimedia.org` returned HTTP 429 for every candidate, and the test skipped with that exact reason; an always-passing offline credential-free dry-run case keeps the live file green. - No product code, facade paths or dependency declarations were changed; the target branch `task/1-task` is already contained in the work branch. Tests: Ran through the facade entrypoints with the prepared `.venv`: `tools.validate` (`python -m compileall tools tests`) → OK; `tools.test` (`pytest`) → 158 passed, 1 skipped (the live case skipped as an explicit unresolved egress failure: upload.wikimedia.org HTTP 429, reason recorded); `tools.smoke` (`python -m tools.example examples/input.json`) → exit 0 with a parsed dry_run envelope. Also ran the two new files directly: 16 passed, 1 skipped. Progress: Added tests/test_status_branches.py (subprocess-driven checks for ok, no_format, no_size, access_error x2, invalid_request/unknown-source, partial_download: status + exit code + source/request echo + single JSON document, distinctness, saved-path truthfulness, shipped example run) and tests/test_live_source.py (credential-free live Commons png search with recorded observation and real-dimension/URL re-verification, explicit unresolved skip with observed reason when egress is unusable). Facade validate/test/smoke all succeed; target branch already integrated; no product or protected facade files touched. Published by developer-agent through Tool Backend. Closes #8 --- .agent-work/ledger.json | 5 + tests/test_live_source.py | 251 +++++++++++++++++++ tests/test_status_branches.py | 450 ++++++++++++++++++++++++++++++++++ 3 files changed, 706 insertions(+) create mode 100644 tests/test_live_source.py create mode 100644 tests/test_status_branches.py diff --git a/.agent-work/ledger.json b/.agent-work/ledger.json index 4ee58a1..123c0f3 100644 --- a/.agent-work/ledger.json +++ b/.agent-work/ledger.json @@ -26,6 +26,11 @@ "events": [ "Completed issue 7: stdlib PNG/ICO decode/encode/wrap/unwrap plus resample helpers live in tools/images.py; tools/example.py saves selected images into the caller directory, requires --output-dir with --download (invalid_request otherwise), verifies every stored file by decoding it before reporting its path, downscales to exactly the requested width/height in the requested format only when --resize is set (never upscaling, otherwise byte-identical), and reports a per-item save failure with the partial_download status/exit code. New tests in tests/test_download_resize.py cover all of that; tests/test_cli.py resize expectation aligned with the exact-size requirement. Facade validate/smoke/help/import all pass and `pytest` reports 142 passed on the revision that already contains target task/1-task." ] + }, + "projects/product-54354138-1#8": { + "events": [ + "Added tests/test_status_branches.py (subprocess-driven checks for ok, no_format, no_size, access_error x2, invalid_request/unknown-source, partial_download: status + exit code + source/request echo + single JSON document, distinctness, saved-path truthfulness, shipped example run) and tests/test_live_source.py (credential-free live Commons png search with recorded observation and real-dimension/URL re-verification, explicit unresolved skip with observed reason when egress is unusable). Facade validate/test/smoke all succeed; target branch already integrated; no product or protected facade files touched." + ] } } } diff --git a/tests/test_live_source.py b/tests/test_live_source.py new file mode 100644 index 0000000..790a6f0 --- /dev/null +++ b/tests/test_live_source.py @@ -0,0 +1,251 @@ +"""Live, credential-free end-to-end check against the real Wikimedia Commons. + +The case drives the shipped CLI as a subprocess with a scrubbed environment: no +API key, token, password or other credential variable is passed in, because the +source is queried anonymously (REQ-003). + +When the approved hosts are usable the observed JSON is recorded and every +reported item is re-fetched independently to prove that its URL works and that +the reported dimensions are the real pixel size of the PNG bytes. When egress +to the approved hosts is unavailable or the host refuses to serve files, the +case is reported as an explicit *unresolved skip* carrying the observed status +and error reason. Synthetic or canned results are never substituted for a live +observation. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +import urllib.parse +from pathlib import Path +from typing import Any + +import pytest + +from tools import envelope, images, sources + +REPO_ROOT = Path(__file__).resolve().parents[1] + +SOURCE_ID = "wikimedia_commons" + +#: Topic with known PNG files on Wikimedia Commons (the Nuvola icon set). +#: Further known topics are tried when the source answers but has no PNG of the +#: requested size, so a reachable source is not reported as unreachable. +KNOWN_TOPICS = ( + "Nuvola icons", + "Wikimedia Commons logo", + "red fox in the snow", +) +MIN_WIDTH = 64 +MIN_HEIGHT = 64 +COUNT = 2 +LIVE_TIMEOUT = 180 + +#: Environment variable names that could carry a credential of some kind. +_CREDENTIAL_MARKERS = ("API", "AUTH", "CREDENTIAL", "KEY", "PASSWORD", "SECRET", "TOKEN") + + +def _is_credential_name(name: str) -> bool: + """True when *name* looks like it would carry a credential.""" + + upper = name.upper() + return any(marker in upper for marker in _CREDENTIAL_MARKERS) + + +def scrubbed_environment() -> dict[str, str]: + """Return an environment with every credential-looking variable removed.""" + + env = { + name: value + for name, value in os.environ.items() + if not _is_credential_name(name) + } + env["PYTHONPATH"] = str(REPO_ROOT) + return env + + +def run_cli(*args: str) -> subprocess.CompletedProcess[str]: + """Run ``python -m tools.example`` in a credential-free subprocess.""" + + return subprocess.run( + [sys.executable, "-m", "tools.example", *args], + cwd=REPO_ROOT, + env=scrubbed_environment(), + capture_output=True, + text=True, + check=False, + timeout=LIVE_TIMEOUT, + ) + + +def single_document(result: subprocess.CompletedProcess[str]) -> dict[str, Any]: + """Assert the process wrote exactly one complete JSON envelope.""" + + assert result.stderr == "", result.stderr + lines = [line for line in result.stdout.splitlines() if line.strip()] + assert len(lines) == 1, result.stdout + document = json.loads(lines[0]) + assert isinstance(document, dict) + assert set(document) == {"status", "source", "request", "items", "error"} + return document + + +def _access_reason(document: dict[str, Any]) -> str: + """Describe the observed access failure carried by an access-error answer.""" + + error = document.get("error") or {} + message = str(error.get("message") or "no error message reported") + details = error.get("details") or {} + reasons: list[str] = [] + for failure in details.get("failures") or (): + fragment = failure.get("error") or {} + text = str(fragment.get("message") or "") + if text: + reasons.append(text) + if reasons: + return f"{message}; first candidate failure: {reasons[0]}" + return message + + +def _record(record_property: Any, tmp_path: Path, observations: list[dict[str, Any]]) -> str: + """Persist the raw observation so it is available as evidence.""" + + encoded = json.dumps(observations, sort_keys=True, ensure_ascii=True) + record_property("live_source_observation", encoded) + artifact = tmp_path / "live-source-observation.json" + artifact.write_text(encoded, encoding="utf-8") + print("LIVE SOURCE OBSERVATION " + encoded, flush=True) + return encoded + + +def test_cli_needs_no_credentials_and_dry_run_answers() -> None: + env = scrubbed_environment() + assert sorted(name for name in env if _is_credential_name(name)) == [] + + result = run_cli( + "--topic", + KNOWN_TOPICS[0], + "--format", + "png", + "--width", + str(MIN_WIDTH), + "--height", + str(MIN_HEIGHT), + "--source", + SOURCE_ID, + "--dry-run", + ) + document = single_document(result) + + assert result.returncode == envelope.EXIT_OK + assert document["status"] == envelope.STATUS_DRY_RUN + assert document["source"] == SOURCE_ID + assert document["items"] == [] + assert document["error"] is None + assert document["request"]["format"] == "png" + + +def test_live_wikimedia_commons_png_search_without_credentials( + tmp_path: Path, record_property: Any +) -> None: + observations: list[dict[str, Any]] = [] + final: dict[str, Any] | None = None + + for topic in KNOWN_TOPICS: + result = run_cli( + "--topic", + topic, + "--format", + "png", + "--width", + str(MIN_WIDTH), + "--height", + str(MIN_HEIGHT), + "--count", + str(COUNT), + "--source", + SOURCE_ID, + ) + document = single_document(result) + observations.append( + {"topic": topic, "exit_code": result.returncode, "response": document} + ) + + # The process contract holds for every live answer, matched or not. + assert document["source"] == SOURCE_ID + request = document["request"] + if request is not None: + assert request["source"] == SOURCE_ID + assert request["format"] == "png" + assert result.returncode == envelope.STATUS_EXIT_CODES[document["status"]] + + status = document["status"] + final = document + if status == envelope.STATUS_ACCESS_ERROR: + # The approved hosts are unusable from here: no further topic can + # succeed, and the failure must not be disguised as a no-match. + break + if status == envelope.STATUS_OK: + break + if status in (envelope.STATUS_NO_FORMAT, envelope.STATUS_NO_SIZE): + continue + raise AssertionError(f"unexpected live status {status!r}: {json.dumps(document)}") + + assert final is not None + _record(record_property, tmp_path, observations) + status = final["status"] + + if status == envelope.STATUS_ACCESS_ERROR: + pytest.skip( + "unresolved live check: the approved Wikimedia host is not usable " + f"from this environment ({_access_reason(final)})" + ) + + if status in (envelope.STATUS_NO_FORMAT, envelope.STATUS_NO_SIZE): + pytest.skip( + "unresolved live check: the source answered but served no png image " + f"of at least {MIN_WIDTH}x{MIN_HEIGHT} px for the known topics " + f"({', '.join(KNOWN_TOPICS)}); observed status {status!r}" + ) + + assert status == envelope.STATUS_OK, json.dumps(final) + assert final["error"] is None + items = final["items"] + assert items, json.dumps(final) + assert len(items) <= COUNT + + for item in items: + assert item["source"] == SOURCE_ID + assert item["format"] == "png" + assert isinstance(item["width"], int) and item["width"] >= MIN_WIDTH + assert isinstance(item["height"], int) and item["height"] >= MIN_HEIGHT + parts = urllib.parse.urlsplit(item["url"]) + assert parts.scheme == "https" + assert parts.hostname in sources.APPROVED_HOSTS + assert sources.is_approved_url(item["url"]) + + # Prove the reported URLs really serve the reported PNG bytes and that the + # dimensions in the JSON are the real pixel size, not source metadata. + for item in items: + try: + response = sources.urllib_transport( + item["url"], {"User-Agent": sources.USER_AGENT}, 30.0 + ) + except OSError as error: + pytest.skip( + "unresolved live check: the reported URL became unreachable " + f"({item['url']}: {error})" + ) + if response.status != 200: + pytest.skip( + "unresolved live check: the reported URL stopped working " + f"({item['url']} returned HTTP {response.status})" + ) + assert images.detect_format(response.body) == "png", item["url"] + assert images.read_png_dimensions(response.body) == ( + item["width"], + item["height"], + ), item["url"] diff --git a/tests/test_status_branches.py b/tests/test_status_branches.py new file mode 100644 index 0000000..99b7970 --- /dev/null +++ b/tests/test_status_branches.py @@ -0,0 +1,450 @@ +"""End-to-end process-level checks for every CLI status branch. + +Each case launches a fresh interpreter that runs the real CLI entry point +(:func:`tools.example.main`) as a subprocess, with a scrubbed environment (no +API key, token or credential variable) and an injected offline source registry, +so the assertions cover the whole observable contract of the process: exactly +one JSON document on stdout, the expected ``status``, the matching exit code and +the echoed source and request. + +The branches exercised here are the ones a caller must be able to tell apart: + +* ``ok`` -- matching images really were found; +* ``no_format`` -- the source had hits but none in the requested format; +* ``no_size`` -- images of the requested format existed but were too small; +* ``access_error`` -- the source could not be searched or fetched; +* ``invalid_request`` -- the request named an unknown source; +* ``partial_download`` -- some images were saved and some could not be fetched. + +Only the *source* is replaced by the injected registry; argument parsing, +request validation, format/size selection, download handling and envelope +rendering all run unmodified inside the subprocess. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path +from typing import Any + +import pytest + +from tools import envelope + +REPO_ROOT = Path(__file__).resolve().parents[1] +EXAMPLE_REQUEST = REPO_ROOT / "examples" / "input.json" + +SOURCE_ID = "wikimedia_commons" +TOPIC = "red fox" + +#: Environment variable names that could carry a credential of some kind. +_CREDENTIAL_MARKERS = ("API", "AUTH", "CREDENTIAL", "KEY", "PASSWORD", "SECRET", "TOKEN") + +#: CLI arguments shared by every injected-registry scenario. +BASE_ARGS = ( + "--topic", + TOPIC, + "--format", + "png", + "--width", + "100", + "--height", + "100", + "--source", + SOURCE_ID, +) + +#: Process-level driver: replaces the source registry, then runs the real CLI. +#: +#: It never re-implements CLI logic. It only provides an offline stand-in for +#: the remote source so every status branch can be driven deterministically +#: through a real operating-system process and its real exit code. +_DRIVER_SOURCE = ''' +import sys + +from tools import example, images +from tools.errors import SourceUnreachableError + +PNG_MATCHING = images.png_bytes(160, 120) +PNG_TOO_SMALL = images.png_bytes(20, 20) +JPEG_PAYLOAD = bytes([0xFF, 0xD8, 0xFF, 0xE0]) + bytes(32) + + +class Candidate: + """Minimal stand-in for tools.sources.SourceCandidate.""" + + source_id = "wikimedia_commons" + mime_type = "image/png" + page_url = None + + def __init__(self, name, payload=None, fail=False, width=None, height=None): + self.title = name + self.file_url = "https://upload.wikimedia.org/practice/" + name + ".png" + self.width = width + self.height = height + self._payload = payload + self._fail = fail + + def to_dict(self): + return { + "source_id": self.source_id, + "title": self.title, + "file_url": self.file_url, + } + + def fetch_bytes(self): + if self._fail: + raise SourceUnreachableError( + self.file_url, + source_id=self.source_id, + reason="simulated transport failure", + ) + return bytes(self._payload) + + +class Registry: + """Offline registry exposing the same surface the CLI uses.""" + + def __init__(self, candidates=(), error=None): + self._candidates = list(candidates) + self._error = error + + def available_sources(self): + return ["wikimedia_commons"] + + def search(self, source_id, topic, limit=10): + if self._error is not None: + raise self._error + return self._candidates[:limit] + + +SCENARIOS = { + "ok": lambda: Registry( + [Candidate("good", PNG_MATCHING, width=160, height=120)] + ), + "no_format": lambda: Registry([Candidate("jpeg", JPEG_PAYLOAD)]), + "no_size": lambda: Registry( + [Candidate("small", PNG_TOO_SMALL, width=20, height=20)] + ), + "access_error": lambda: Registry([Candidate("broken", fail=True)]), + "search_access_error": lambda: Registry( + error=SourceUnreachableError( + "https://commons.wikimedia.org/w/api.php", + source_id="wikimedia_commons", + reason="simulated transport failure", + ) + ), + "partial_download": lambda: Registry( + [ + Candidate("good", PNG_MATCHING, width=160, height=120), + Candidate("broken", fail=True), + ] + ), +} + + +def main(): + scenario = sys.argv[1] + registry = SCENARIOS[scenario]() + example.default_registry = lambda: registry + raise SystemExit(example.main(sys.argv[2:])) + + +if __name__ == "__main__": + main() +''' + + +def _is_credential_name(name: str) -> bool: + """True when *name* looks like it would carry a credential.""" + + upper = name.upper() + return any(marker in upper for marker in _CREDENTIAL_MARKERS) + + +def scrubbed_environment() -> dict[str, str]: + """Return an environment with every credential-looking variable removed.""" + + env = { + name: value + for name, value in os.environ.items() + if not _is_credential_name(name) + } + env["PYTHONPATH"] = str(REPO_ROOT) + return env + + +def run_cli(*args: str) -> subprocess.CompletedProcess[str]: + """Run ``python -m tools.example`` in a scrubbed subprocess.""" + + return subprocess.run( + [sys.executable, "-m", "tools.example", *args], + cwd=REPO_ROOT, + env=scrubbed_environment(), + capture_output=True, + text=True, + check=False, + timeout=120, + ) + + +def run_scenario( + tmp_path: Path, scenario: str, *args: str +) -> subprocess.CompletedProcess[str]: + """Run the driver subprocess for *scenario* with the real CLI arguments.""" + + driver = tmp_path / "status_branch_driver.py" + if not driver.exists(): + driver.write_text(_DRIVER_SOURCE, encoding="utf-8") + return subprocess.run( + [sys.executable, str(driver), scenario, *args], + cwd=REPO_ROOT, + env=scrubbed_environment(), + capture_output=True, + text=True, + check=False, + timeout=120, + ) + + +def single_document(result: subprocess.CompletedProcess[str]) -> dict[str, Any]: + """Assert the process wrote exactly one complete JSON envelope.""" + + assert result.stderr == "", result.stderr + lines = [line for line in result.stdout.splitlines() if line.strip()] + assert len(lines) == 1, result.stdout + document = json.loads(lines[0]) + assert isinstance(document, dict) + assert set(document) == {"status", "source", "request", "items", "error"} + return document + + +def test_subprocess_environment_carries_no_credentials() -> None: + env = scrubbed_environment() + + leaked = sorted(name for name in env if _is_credential_name(name)) + assert leaked == [] + + +@pytest.mark.parametrize( + "scenario,status", + [ + ("ok", envelope.STATUS_OK), + ("no_format", envelope.STATUS_NO_FORMAT), + ("no_size", envelope.STATUS_NO_SIZE), + ("access_error", envelope.STATUS_ACCESS_ERROR), + ("partial_download", envelope.STATUS_PARTIAL_DOWNLOAD), + ], +) +def test_status_branch_emits_its_own_status_and_exit_code( + tmp_path: Path, scenario: str, status: str +) -> None: + args = BASE_ARGS + if scenario == "partial_download": + args = BASE_ARGS + ( + "--count", + "2", + "--download", + "--output-dir", + str(tmp_path / "downloads"), + ) + + result = run_scenario(tmp_path, scenario, *args) + document = single_document(result) + + assert document["status"] == status + assert result.returncode == envelope.STATUS_EXIT_CODES[status] + assert document["source"] == SOURCE_ID + + +def test_status_branch_echoes_the_normalized_request( + tmp_path: Path, +) -> None: + """``ok``/``no_format``/``no_size``/``partial_download`` echo the request.""" + + for scenario, args in ( + ("ok", BASE_ARGS), + ("no_format", BASE_ARGS), + ("no_size", BASE_ARGS), + ( + "partial_download", + BASE_ARGS + + ( + "--count", + "2", + "--download", + "--output-dir", + str(tmp_path / "downloads-echo"), + ), + ), + ): + result = run_scenario(tmp_path, scenario, *args) + document = single_document(result) + + assert document["request"] is not None, scenario + assert document["request"]["source"] == SOURCE_ID + assert document["request"]["topic"] == TOPIC + assert document["request"]["format"] == "png" + assert document["request"]["width"] == 100 + assert document["request"]["height"] == 100 + + +def test_branches_are_mutually_distinguishable() -> None: + branches = [ + envelope.STATUS_OK, + envelope.STATUS_NO_FORMAT, + envelope.STATUS_NO_SIZE, + envelope.STATUS_ACCESS_ERROR, + envelope.STATUS_INVALID_REQUEST, + envelope.STATUS_PARTIAL_DOWNLOAD, + ] + + codes = {status: envelope.STATUS_EXIT_CODES[status] for status in branches} + + assert len(set(branches)) == len(branches) + assert len(set(codes.values())) == len(branches), codes + + +def test_ok_branch_reports_the_verified_png_items(tmp_path: Path) -> None: + result = run_scenario(tmp_path, "ok", *BASE_ARGS) + document = single_document(result) + + assert document["status"] == envelope.STATUS_OK + assert document["error"] is None + assert document["items"] + item = document["items"][0] + assert item["format"] == "png" + assert item["width"] == 160 + assert item["height"] == 120 + assert item["source"] == SOURCE_ID + assert item["url"].startswith("https://") + + +def test_no_format_and_no_size_are_distinct_and_not_access_errors( + tmp_path: Path, +) -> None: + format_result = run_scenario(tmp_path, "no_format", *BASE_ARGS) + size_result = run_scenario(tmp_path, "no_size", *BASE_ARGS) + format_document = single_document(format_result) + size_document = single_document(size_result) + + assert format_document["status"] == envelope.STATUS_NO_FORMAT + assert size_document["status"] == envelope.STATUS_NO_SIZE + assert format_result.returncode == envelope.EXIT_NO_FORMAT + assert size_result.returncode == envelope.EXIT_NO_SIZE + assert format_document["error"]["code"] == "no_format_match" + assert size_document["error"]["code"] == "no_size_match" + assert format_document["error"]["message"] != size_document["error"]["message"] + for document in (format_document, size_document): + assert document["items"] == [] + assert document["source"] == SOURCE_ID + assert document["error"]["code"] != "source_access" + assert document["error"]["code"] != "source_unreachable" + + +def test_access_error_branch_is_not_reported_as_a_no_match(tmp_path: Path) -> None: + result = run_scenario(tmp_path, "access_error", *BASE_ARGS) + document = single_document(result) + + assert document["status"] == envelope.STATUS_ACCESS_ERROR + assert result.returncode == envelope.EXIT_ACCESS_ERROR + assert document["items"] == [] + assert document["source"] == SOURCE_ID + assert document["request"]["source"] == SOURCE_ID + assert document["error"]["code"] == "candidate_unreadable" + details = document["error"]["details"] + assert details["unreadable_candidates"] == 1 + assert details["candidates_inspected"] == 0 + assert details["failures"][0]["error"]["code"] == "source_unreachable" + assert details["failures"][0]["source"] == SOURCE_ID + + +def test_search_failure_branch_is_access_error_with_the_source_echoed( + tmp_path: Path, +) -> None: + # A source that cannot even be searched is an access error, never a + # "nothing matched" answer; the requested source id is still echoed. + result = run_scenario(tmp_path, "search_access_error", *BASE_ARGS) + document = single_document(result) + + assert document["status"] == envelope.STATUS_ACCESS_ERROR + assert result.returncode == envelope.EXIT_ACCESS_ERROR + assert document["items"] == [] + assert document["source"] == SOURCE_ID + assert document["error"]["code"] == "source_unreachable" + assert document["error"]["details"]["source_id"] == SOURCE_ID + + +def test_partial_download_branch_names_only_saved_files(tmp_path: Path) -> None: + output_dir = tmp_path / "downloads" + + result = run_scenario( + tmp_path, + "partial_download", + *BASE_ARGS, + "--count", + "2", + "--download", + "--output-dir", + str(output_dir), + ) + document = single_document(result) + + assert document["status"] == envelope.STATUS_PARTIAL_DOWNLOAD + assert result.returncode == envelope.EXIT_PARTIAL_DOWNLOAD + details = document["error"]["details"] + assert details["failed"] == 1 + assert details["saved"] == 1 + assert details["total"] == 2 + assert len(details["saved_paths"]) == 1 + saved_path = Path(details["saved_paths"][0]) + assert saved_path.is_file() + assert document["items"][0]["download"]["ok"] is True + assert document["source"] == SOURCE_ID + + +def test_unknown_source_branch_is_invalid_request_with_the_source_echoed() -> None: + result = run_cli( + "--topic", + TOPIC, + "--format", + "png", + "--width", + "100", + "--height", + "100", + "--source", + "no-such-source", + "--dry-run", + ) + document = single_document(result) + + assert document["status"] == envelope.STATUS_INVALID_REQUEST + assert result.returncode == envelope.EXIT_INVALID_REQUEST + assert document["source"] == "no-such-source" + assert document["error"]["code"] == "source_unknown" + assert SOURCE_ID in document["error"]["details"]["available_sources"] + + +def test_shipped_example_request_runs_and_parses() -> None: + expected = json.loads(EXAMPLE_REQUEST.read_text(encoding="utf-8")) + + result = run_cli("examples/input.json") + document = single_document(result) + + # The shipped example is a dry run, so it validates and echoes the request + # without contacting any source. + assert result.returncode == envelope.EXIT_OK + assert document["status"] == envelope.STATUS_DRY_RUN + assert document["source"] == expected["source"] + assert document["items"] == [] + assert document["error"] is None + assert document["request"]["topic"] == expected["topic"] + assert document["request"]["format"] == expected["format"] + assert document["request"]["width"] == expected["width"] + assert document["request"]["height"] == expected["height"] + assert document["request"]["count"] == expected["count"] + assert document["request"]["dry_run"] is True -- 2.54.0 From 2e8e70e765d8577d045d576ad5c11c2e3ec0cea5 Mon Sep 17 00:00:00 2001 From: Tool Backend Date: Tue, 3 Nov 2026 02:40:29 +0000 Subject: [PATCH 10/10] change: Implement 7: Document the tool-calling contract, statuses, exit codes and native-process usage Rewrote README.md as the delivered tool-calling contract for the calling AI agent and added tests/test_readme.py to keep it synchronized with the implementation. README.md now documents: - Native-process usage only: `python -m tools.example` with no Docker, container image or packaging stage, Python >= 3.11, no runtime third-party dependencies. - Every parameter exactly as produced by `--help`: positional `REQUEST.json`, `-t/--topic`, `--width`, `--height`, `-f/--format` (`png|ico`), `-s/--source` (default `wikimedia_commons`), `-c/--count` (1-50, default 10), `--download`, `-o/--output-dir`, `--resize`, `--dry-run`; width/height documented as minimums and the "document XOR flags" rule. - The JSON request document form used by `examples/input.json`: field table, types, defaults, optional `version`, and the document-level invalid_request rules. - The single-document JSON response schema: fixed keys `status`, `source`, `request`, `items`, `error`; item fields (actual format/width/height, `url`/`file_url`, `size_bytes`), download record, resize fragment and the verified-save failure record. - The fixed status vocabulary with its exit codes: `ok`=0, `dry_run`=0, `invalid_request`=2, `no_format`=3, `no_size`=4, `access_error`=5, `partial_download`=6, plus why no_match/outage are distinguishable. - Approved free sources (`wikimedia_commons`, `openverse`) with no API keys, tokens or registration, the approved HTTPS host allowlist, and the registry extension point (`SourceAdapter`, `SourceRegistry.register/unregister`, `build_registry()`/`default_registry()`, host allowlist extension). - Both decided behaviours: `--download` without `--output-dir` is an invalid request (invalid_request / exit 2), and partial saves are reported as `partial_download` (exit 6) with `download_incomplete` details while saved paths are still reported. - The real test entrypoint `pytest` plus the compileall/help/smoke commands. tests/test_readme.py asserts the README documents every long option the real parser accepts, the accepted formats, every status with its exit code from STATUS_EXIT_CODES, the response keys, every registered source id, the registry extension names, both product decisions, native-process wording, and the real pytest/compileall/help/smoke commands. Tests: Ran the facade check set with .venv/bin on PATH after `prepare_environment(component=tools, mode=restore)`: - `pytest -q` -> 163 passed, 1 skipped (pre-existing tests/test_live_source.py live-network skip: Wikimedia upload host returns HTTP 429 from this sandbox). - `python -m compileall -q tools tests` -> ok. - `python -m tools.example --help` -> exit 0. - `python -m tools.example examples/input.json` -> exit 0, one JSON dry_run envelope. - `pytest tests/test_readme.py -q` -> 5 passed. Also confirmed `git merge-base --is-ancestor origin/task/1-task HEAD` (target branch already integrated, zero diff), so no integration step was required. Progress: Delivered the documentation slice for issue 9. README.md was a two-line placeholder; it is now the full tool-calling contract: CLI parameters matching `--help` output, the JSON request-document form for examples/input.json, the single-document response schema, the fixed status vocabulary with exit codes (ok/dry_run=0, invalid_request=2, no_format=3, no_size=4, access_error=5, partial_download=6), credential-free sources and the registry extension point, the decided --download-without---output-dir invalid-request behaviour, partial_download reporting for incomplete saves, and plain native-process execution with the real pytest entrypoint. Added tests/test_readme.py (5 tests) so CI fails if documentation drifts from build_parser(), STATUS_EXIT_CODES, ACCEPTED_FORMATS or the default registry. No protected facade paths or source behaviour changed; existing obligations preserved (full suite green apart from the pre-existing live-source skip). Published by developer-agent through Tool Backend. Closes #9 --- .agent-work/ledger.json | 5 + README.md | 319 +++++++++++++++++++++++++++++++++++++++- tests/test_readme.py | 92 ++++++++++++ 3 files changed, 414 insertions(+), 2 deletions(-) create mode 100644 tests/test_readme.py diff --git a/.agent-work/ledger.json b/.agent-work/ledger.json index 123c0f3..449ebda 100644 --- a/.agent-work/ledger.json +++ b/.agent-work/ledger.json @@ -31,6 +31,11 @@ "events": [ "Added tests/test_status_branches.py (subprocess-driven checks for ok, no_format, no_size, access_error x2, invalid_request/unknown-source, partial_download: status + exit code + source/request echo + single JSON document, distinctness, saved-path truthfulness, shipped example run) and tests/test_live_source.py (credential-free live Commons png search with recorded observation and real-dimension/URL re-verification, explicit unresolved skip with observed reason when egress is unusable). Facade validate/test/smoke all succeed; target branch already integrated; no product or protected facade files touched." ] + }, + "projects/product-54354138-1#9": { + "events": [ + "Delivered the documentation slice for issue 9. README.md was a two-line placeholder; it is now the full tool-calling contract: CLI parameters matching `--help` output, the JSON request-document form for examples/input.json, the single-document response schema, the fixed status vocabulary with exit codes (ok/dry_run=0, invalid_request=2, no_format=3, no_size=4, access_error=5, partial_download=6), credential-free sources and the registry extension point, the decided --download-without---output-dir invalid-request behaviour, partial_download reporting for incomplete saves, and plain native-process execution with the real pytest entrypoint. Added tests/test_readme.py (5 tests) so CI fails if documentation drifts from build_parser(), STATUS_EXIT_CODES, ACCEPTED_FORMATS or the default registry. No protected facade paths or source behaviour changed; existing obligations preserved (full suite green apart from the pre-existing live-source skip)." + ] } } } diff --git a/README.md b/README.md index fb90130..f4142e5 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,318 @@ -# product-54354138-1 +# product-54354138-1 image toolkit -agent-intake-source: platform/product-requests#1 \ No newline at end of file +A plain native-process command-line tool for a calling AI agent. One invocation +accepts a request, searches an approved free image source, verifies every +candidate against its *downloaded bytes* and writes **exactly one +machine-readable JSON document** to stdout. The process exit code mirrors the +document's `status`, so success, "nothing matched", a broken request and an +unusable source are distinguishable without parsing prose. + +There is no Docker, no container image and no packaging step: the tool is run +directly as an executable process with the interpreter that is already present. + +## Contents + +- [Requirements](#requirements) +- [Running the tool](#running-the-tool) +- [Request parameters](#request-parameters) +- [JSON request document](#json-request-document) +- [JSON response document](#json-response-document) +- [Status vocabulary and exit codes](#status-vocabulary-and-exit-codes) +- [Image sources](#image-sources) +- [Download and resize behaviour](#download-and-resize-behaviour) +- [Tests and checks](#tests-and-checks) + +## Requirements + +- Python >= 3.11 (declared in `pyproject.toml`). +- No runtime third-party dependencies: the HTTPS client, PNG/ICO container + parsing, decoding, resampling and re-encoding all live in the standard + library and in `tools/`. +- No API keys, tokens or registration for any source (see + [Image sources](#image-sources)). + +There is nothing to install to *use* the tool from a checkout. Installing the +project (`pip install .`, `uv sync`, ...) only matters for the optional test +tooling. + +## Running the tool + +The tool is always invoked as a module of the working tree: + +```console +python -m tools.example --topic "red fox" --width 640 --height 480 --format png +``` + +A JSON request document can be passed instead: + +```console +python -m tools.example examples/input.json +``` + +`--help` is the authoritative parameter list; the table below is a summary: + +```console +python -m tools.example --help +``` + +Rules for the two invocation styles: + +- Either a request document **or** flags must be given. With neither, the run + fails with `invalid_request` (exit code 2). +- A request document cannot be combined with any flag; doing so is + `invalid_request` (exit code 2). + +## Request parameters + +| Parameter | Kind | Meaning | +| --- | --- | --- | +| `REQUEST.json` | positional | Path to a JSON request document holding the same fields as the flags. It cannot be combined with any of the flags. | +| `-t`, `--topic TEXT` | text | Search topic (required unless a request document is given). | +| `--width PX` | positive integer | Target width in pixels; only images at least this wide are accepted. | +| `--height PX` | positive integer | Target height in pixels; only images at least this tall are accepted. | +| `-f`, `--format FMT` | `png` or `ico` | Image format; required. Any other value is `invalid_request`. | +| `-s`, `--source ID` | source id | Image source id (default: `wikimedia_commons`); unknown ids are rejected with the list of available sources. | +| `-c`, `--count N` | integer 1-50 | How many candidates to request (default: `10`). | +| `--download` | flag | Download every matching image into the output directory. | +| `-o`, `--output-dir DIR` | path | Caller-specified download directory; required with `--download`. | +| `--resize` | flag | Downscale every downloaded image that is larger than the request in either dimension to exactly the requested width and height, re-encoded in the requested format; images are never scaled up. Without it the original bytes and dimensions are written. | +| `--dry-run` | flag | Validate the request and emit the envelope without any network access. | + +`--width` and `--height` are a **minimum**, not an exact match: an image that +is at least that large is usable because it can be downscaled, while a smaller +image would have to be scaled up and is therefore rejected. `--resize` decides +whether the *stored* file is downscaled; it never changes this filter. + +`--count` bounds how many matching images are reported. Because nothing a +source reports about a hit is trusted, the tool inspects a bounded window of up +to `count * 4` candidates (capped at 50) and stops as soon as `count` verified +images have been found. + +Short flags: `-t`/`--topic`, `-f`/`--format`, `-s`/`--source`, `-c`/`--count`, +`-o`/`--output-dir`, `-h`/`--help`. Every other parameter is long-form only. + +## JSON request document + +The document accepted as the positional argument (see `examples/input.json`) +is a single JSON **object** using the same names as the flags, in +`snake_case`: + +```json +{ + "version": 1, + "topic": "red fox in the snow", + "width": 640, + "height": 480, + "format": "png", + "source": "wikimedia_commons", + "count": 5, + "download": false, + "output_dir": "downloads", + "resize": false, + "dry_run": true +} +``` + +| Field | Type | Required | Notes | +| --- | --- | --- | --- | +| `topic` | string, non-empty | yes | Same as `--topic`. | +| `width` | integer > 0 or `null` | no | Minimum width; omitted/`null` means unconstrained. | +| `height` | integer > 0 or `null` | no | Minimum height; omitted/`null` means unconstrained. | +| `format` | `"png"` or `"ico"` | yes | Same as `--format`. | +| `source` | string | no | Defaults to `wikimedia_commons`. | +| `count` | integer 1-50 | no | Defaults to `10`. | +| `download` | boolean | no | Defaults to `false`. | +| `output_dir` | string, non-empty | required when `download` is `true` | Download directory. | +| `resize` | boolean | no | Defaults to `false`. | +| `dry_run` | boolean | no | Defaults to `false`. | +| `version` | integer | no | Optional document metadata; accepted and otherwise ignored. | + +Document rules that produce `invalid_request` (exit code 2): + +- the file cannot be read, is not valid JSON, or is not a JSON object; +- an unknown field is present (`error.code = "request_field"`, with + `details.unknown_fields` and `details.accepted_fields`); +- a field has the wrong type, is empty, or is out of range; +- `format` is outside `png|ico`; +- `download` is `true` while `output_dir` is absent or empty. + +## JSON response document + +Every run - success or failure - prints exactly one JSON document (one line) to +stdout, with this fixed set of top-level keys: + +```json +{ + "status": "ok", + "source": "wikimedia_commons", + "request": {"topic": "red fox", "width": 640, "height": 480, "format": "png", "source": "wikimedia_commons", "count": 1, "download": false, "output_dir": null, "resize": false, "dry_run": false}, + "items": [ + { + "source": "wikimedia_commons", + "url": "https://upload.wikimedia.org/wikipedia/commons/.../Fox.png", + "file_url": "https://upload.wikimedia.org/wikipedia/commons/.../Fox.png", + "format": "png", + "width": 1280, + "height": 853, + "size_bytes": 1489231, + "title": "File:Fox.png", + "page_url": "https://commons.wikimedia.org/wiki/File:Fox.png", + "mime_type": "image/png" + } + ], + "error": null +} +``` + +| Key | Type | Meaning | +| --- | --- | --- | +| `status` | string | One value of the fixed vocabulary below. | +| `source` | string or `null` | Source id the request selected, when known. | +| `request` | object or `null` | The validated request (every field, defaults filled in), `null` when the request could not be validated. | +| `items` | array | Verified images (empty unless `status` is `ok` or `partial_download`). | +| `error` | object or `null` | `{"code", "message", "details"}`; `details` is optional. | + +An item is only reported after its bytes were downloaded and parsed, so its +`format`, `width` and `height` are the **actual** values read from the payload +container, never what the source claimed. `url` and `file_url` are the same +direct HTTPS location; `file_url` is kept as an alias. `title`, `page_url` and +`mime_type` are present only when the source provides them. `size_bytes` is the +number of stored/fetched bytes. + +With `download` enabled every item additionally carries a `download` record: + +```json +{"ok": true, "path": "downloads/001-file-fox.png", "bytes": 1489231, "format": "png", "width": 1280, "height": 853} +``` + +With `resize` enabled the record also carries a `resize` fragment: +`{"requested": {...}, "from": {...}, "to": {...}, "format": "png", "applied": true}` +(`"applied": false, "reason": "already_fits"` when nothing had to change). + +A file whose stored bytes do not read back as the requested format at the +expected size is deleted and reported as `{"ok": false, "target": "...", +"error": {...}}` instead of being named as saved. + +## Status vocabulary and exit codes + +The status vocabulary is fixed and every branch reuses it; the exit code is +derived from the status. + +| `status` | Exit code | Meaning | +| --- | --- | --- | +| `ok` | 0 | At least one image really is the requested format and at least the requested width and height (all downloads completed when requested). | +| `dry_run` | 0 | The request was validated and echoed without contacting a source. | +| `invalid_request` | 2 | The request itself is unusable: bad or missing flags, unreadable/invalid document, unknown field, unsupported format, unknown source, or `download` without `output_dir`. | +| `no_format` | 3 | The source answered but offers nothing in the requested format for the topic. | +| `no_size` | 4 | The source has images of the requested format, but all of them are smaller than requested. | +| `access_error` | 5 | The source could not be searched, or not a single candidate could be downloaded; never reported as "no match". | +| `partial_download` | 6 | A download run where some images were saved and some could not be (see below). | + +The distinction is intentional and stable: + +- `no_format` / `no_size` describe the *topic*, not an outage, so they are not + errors of the source. +- A candidate whose bytes turn out to be a different format (JPEG, SVG, TIFF, + an HTML error page, ...) is a format rejection that feeds `no_format`, not an + access error. +- `access_error` means the source could not be used at all; its `error.code` + comes from the typed error (`source_unreachable`, `source_http_error`, + `source_invalid_response`, `source_host_not_allowed`, `candidate_unreadable`, + `access_error`). +- `invalid_request` always carries a machine-readable `error.code` + (`request_invalid`, `request_document`, `request_field`, + `request_format_unsupported`, `source_unknown`) plus `error.details`. + +## Image sources + +Two working free sources ship with the tool. Neither needs an API key, a token, +registration or a personal account; both are queried anonymously over HTTPS +with a stable identifying `User-Agent`. + +| Source id | Service | Approved hosts | +| --- | --- | --- | +| `wikimedia_commons` (default) | Wikimedia Commons | `commons.wikimedia.org`, `upload.wikimedia.org` | +| `openverse` | Openverse | `api.openverse.org` | + +Only those hosts on `https`/port 443 are contacted; any other URL is refused +with `source_host_not_allowed` rather than fetched. HTTP 429 and 5xx responses +are retried a bounded number of times with capped exponential backoff and then +reported as `access_error`; a failed or unknown source is never silently +degraded into an empty result. + +An unknown `--source` value fails with `invalid_request` and lists the +available ids: + +```json +{"error": {"code": "source_unknown", "message": "unknown image source 'nope'; available sources: openverse, wikimedia_commons", "details": {"source_id": "nope", "available_sources": ["openverse", "wikimedia_commons"]}}} +``` + +### Adding another source + +The registry in `tools/sources.py` is the extension point: + +- Implement the `SourceAdapter` protocol: a non-empty `source_id` string and + `search(topic, limit) -> list[SourceCandidate]`. +- Build `SourceCandidate` values whose `file_url` is an approved HTTPS URL and + whose `fetch_bytes()` returns the image bytes through `HTTPSClient`. +- Register the adapter with `SourceRegistry.register(adapter)` (a duplicate id + raises `DuplicateSourceError`; `unregister(source_id)` removes one) and wire + it into `build_registry()` / `default_registry()`, which is what the CLI uses. + `HTTPSClient(allowed_hosts=...)` and `validate_source_url()` extend the + approved-host allowlist for a new provider. +- The new id becomes selectable through `--source` / `"source"` immediately; + `SourceRegistry.get()` raises `UnknownSourceError` listing all registered ids + for anything else. + +## Download and resize behaviour + +Two product decisions are fixed and documented here: + +1. **`--download` without `--output-dir` is an invalid request.** There is no + hidden default directory; a download run without a caller-supplied + directory could only guess where files should go. Such a run fails with + `status: "invalid_request"`, exit code 2, and + `error.code: "request_field"`, `error.details.field: "output_dir"`. + The same rule applies to a request document with `"download": true` and no + `output_dir`. +2. **Partial saves are reported as `partial_download`.** When a download run + saves some images but fails to fetch or verify others, the status is + `partial_download` (exit code 6) - never `ok`. The document still lists every + successfully saved item, and `error.code: "download_incomplete"` carries + `details.saved`, `details.failed`, `details.total`, `details.saved_paths` + and `details.failures`. A run in which every image was saved is `ok` + (exit code 0); a run in which nothing could be fetched at all is + `access_error` (exit code 5). + +Files are written into `--output-dir` as `NNN-.`, created with +`mkdir -p` semantics. Without `--resize` the verified original bytes are +stored unchanged. With `--resize` each stored image is downscaled to exactly +the requested width and height (a dimension left open keeps its source value) +and re-encoded in the requested container format; an image already at or below +the requested size is written unchanged and the resize fragment reports +`"applied": false`. `--resize` never enlarges an image. A search without +`--download` never touches the filesystem. + +## Tests and checks + +The test suite entry point is `pytest` from the repository root: + +```console +pytest +``` + +Other repository checks used by CI: + +```console +python -m compileall tools tests +python -m tools.example --help +python -m tools.example examples/input.json +``` + +`examples/input.json` is a `--dry-run` request, so the smoke command validates +the contract and prints the envelope without any network access. + +Running the tests may require the declared dev dependency (`pytest>=8.0`) to be +installed, for example `pip install -e '.[dev]'` or `uv sync`. That install is +only for development; the tool itself runs as a plain process with no Docker or +container packaging at any stage. diff --git a/tests/test_readme.py b/tests/test_readme.py new file mode 100644 index 0000000..502a807 --- /dev/null +++ b/tests/test_readme.py @@ -0,0 +1,92 @@ +"""Keep README.md synchronized with the delivered CLI contract. + +The documentation is part of the product: the calling agent relies on the +parameter list, the status vocabulary and the exit codes stated in README.md. +These checks fail when the implementation moves on and the documentation is +left describing something the tool no longer does. +""" + +from __future__ import annotations + +from pathlib import Path + +from tools.envelope import STATUS_EXIT_CODES +from tools.errors import ACCEPTED_FORMATS +from tools.example import build_parser +from tools.sources import default_registry + +README = Path(__file__).resolve().parent.parent / "README.md" + + +def readme_text() -> str: + assert README.is_file(), "README.md is missing" + text = README.read_text(encoding="utf-8") + assert text.strip(), "README.md is empty" + return text + + +def long_options() -> list[str]: + """Return every ``--long`` option the real parser accepts.""" + + options: list[str] = [] + for action in build_parser()._actions: + for option in action.option_strings: + if option.startswith("--"): + options.append(option) + return sorted(set(options)) + + +def test_readme_documents_every_cli_parameter() -> None: + text = readme_text() + options = long_options() + assert {"--topic", "--width", "--height", "--format", "--source", "--count", + "--download", "--output-dir", "--resize", "--dry-run"} <= set(options) + missing = [option for option in options if option == "--help" and option not in text] + missing += [ + option + for option in options + if option != "--help" and f"`{option}" not in text + ] + assert not missing, f"README.md does not document parameters: {missing}" + # The positional request document and both invocation styles are documented. + assert "REQUEST.json" in text + assert "python -m tools.example examples/input.json" in text + # Format vocabulary in the README matches the accepted formats. + for image_format in ACCEPTED_FORMATS: + assert image_format in text + + +def test_readme_status_table_matches_exit_codes() -> None: + text = readme_text() + for status, exit_code in STATUS_EXIT_CODES.items(): + assert f"| `{status}` | {exit_code} |" in text, ( + f"README.md status table disagrees for {status!r} -> {exit_code}" + ) + + +def test_readme_documents_response_schema_and_sources() -> None: + text = readme_text() + for key in ('"status"', '"source"', '"request"', '"items"', '"error"'): + assert key in text, f"README.md does not document response key {key}" + for source_id in default_registry().available_sources(): + assert source_id in text, f"README.md does not document source {source_id!r}" + assert "SourceAdapter" in text and "SourceRegistry" in text + assert "API key" in text + + +def test_readme_documents_product_decisions() -> None: + text = readme_text() + # --download without --output-dir is an invalid request. + assert "--download" in text and "without `--output-dir` is an invalid request" in text + # Partial saves have their own status. + assert "partial_download" in text + assert "invalid_request" in text + # Native process only: no Docker or container packaging. + assert "Docker" in text and "container" in text + + +def test_readme_documents_the_real_test_entrypoint() -> None: + text = readme_text() + assert "\npytest\n" in text, "README.md must document the real pytest entrypoint" + assert "python -m compileall tools tests" in text + assert "python -m tools.example --help" in text -- 2.54.0