change: Implement 3: Complete the native-process CLI, JSON request handling, status vocabulary and exit codes
Repository Validate / repository-checks (pull_request) Successful in 3s
Repository Validate / repository-checks (pull_request) Successful in 3s
Review fix for issue #5: aligned the public CLI's size/resize semantics with the active product obligations instead of the previous exact-match behaviour. - `tools/example.py`: the requested width/height are now a target (minimum) size, never an exact-match filter: any image at least that large is selected regardless of `--resize`, and smaller images still produce `no_size`. `--resize` no longer widens selection; it now performs real downscaling of downloaded images to a shrink-to-fit box (aspect ratio preserved, never upscaled), implemented with a dependency-free stdlib PNG codec plus ICO container support (PNG-compressed ICO entries), and each item records `from`/`to`/`applied` dimensions. Undecodable payloads raise the new typed `ImageResizeError` and are reported as failed downloads, so the status vocabulary and exit codes stay unchanged (`ok`/`dry_run` 0, `invalid_request` 2, `no_format` 3, `no_size` 4, `access_error` 5, `partial_download` 6). Help text and the `no_size` error details were updated to describe the minimum-size contract. - `tools/errors.py`: added `ImageResizeError` (code `resize_failed`) to the typed hierarchy and `__all__`. - `tests/test_cli.py`: replaced the old "exact size unless --resize" test with minimum-size tests (larger image accepted without `--resize`, exact size accepted, `--resize` without `--download` keeps the reported source size) and added offline fixtures/tests for real downscaling: PNG stored at 100x50 for a 100x100 box, original bytes preserved without `--resize`, no upscaling (`already_fits`), PNG-in-ICO downscaling, and `resize_failed` surfacing as `access_error`. - `examples/input.json` remains the canonical offline `dry_run` request, so the smoke command stays green and network-free. No protected facade/control paths were touched and no dependency declarations changed. Tests: Ran the facade commands on the final revision through the prepared environment (`.venv` on PATH): `python -m compileall tools tests` → exit 0 (tools.validate); `pytest` → 83 passed (tools.test); `python -m tools.example examples/input.json` → exit 0 with one JSON document, status `dry_run` (tools.smoke); `python -m tools.example --help` → exit 0 (help-command acceptance). `tools/envelope.py` is unchanged and still exports the fixed status/exit-code table checked by tests. Target branch `task/1-task` is already an ancestor of the work branch head, so no integration was needed. Progress: 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. Published by developer-agent through Tool Backend. Closes #5
This commit was merged in pull request #13.
This commit is contained in:
+210
-2
@@ -3,8 +3,10 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import struct
|
||||
import subprocess
|
||||
import sys
|
||||
import zlib
|
||||
from pathlib import Path
|
||||
from typing import Any, Sequence
|
||||
|
||||
@@ -286,14 +288,40 @@ def test_no_size_when_only_smaller_images_exist(monkeypatch: pytest.MonkeyPatch,
|
||||
assert payload["status"] == envelope.STATUS_NO_SIZE
|
||||
|
||||
|
||||
def test_resize_accepts_larger_images(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None:
|
||||
def test_requested_size_is_a_minimum_not_an_exact_match(
|
||||
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)
|
||||
|
||||
assert code == envelope.EXIT_OK
|
||||
assert payload["status"] == envelope.STATUS_OK
|
||||
assert payload["request"]["resize"] is False
|
||||
assert [item["width"] for item in payload["items"]] == [400]
|
||||
assert [item["height"] for item in payload["items"]] == [300]
|
||||
|
||||
|
||||
def test_exact_size_is_accepted(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None:
|
||||
registry = FakeRegistry([FakeCandidate("fox", "https://example.test/a.png", "image/png", 100, 100)])
|
||||
|
||||
code, payload = run_main(monkeypatch, capsys, registry, *BASE_ARGS)
|
||||
|
||||
assert code == envelope.EXIT_OK
|
||||
assert len(payload["items"]) == 1
|
||||
|
||||
|
||||
def test_resize_without_download_keeps_reported_source_size(
|
||||
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
|
||||
assert payload["items"][0]["width"] == 400
|
||||
assert "download" not in payload["items"][0]
|
||||
|
||||
|
||||
def test_download_success_writes_files(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path) -> None:
|
||||
@@ -340,3 +368,183 @@ def test_search_access_error_maps_to_access_error(monkeypatch: pytest.MonkeyPatc
|
||||
assert code == envelope.EXIT_ACCESS_ERROR
|
||||
assert payload["status"] == envelope.STATUS_ACCESS_ERROR
|
||||
assert payload["error"]["code"] == "source_access"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Size filtering and real downscaling of downloaded images
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n"
|
||||
|
||||
|
||||
def png_bytes(width: int, height: int, rgba: tuple[int, int, int, int] = (255, 0, 0, 255)) -> bytes:
|
||||
"""Build a minimal, valid RGBA PNG without any third-party dependency."""
|
||||
|
||||
raw = bytearray()
|
||||
for _ in range(height):
|
||||
raw.append(0)
|
||||
raw += bytes(rgba) * width
|
||||
|
||||
def chunk(kind: bytes, data: bytes) -> bytes:
|
||||
crc = zlib.crc32(kind + data) & 0xFFFFFFFF
|
||||
return struct.pack(">I", len(data)) + kind + 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)))
|
||||
+ chunk(b"IEND", b"")
|
||||
)
|
||||
|
||||
|
||||
def png_size(payload: bytes) -> tuple[int, int]:
|
||||
assert payload.startswith(PNG_SIGNATURE)
|
||||
return struct.unpack(">II", payload[16:24])
|
||||
|
||||
|
||||
def ico_bytes(inner_png: bytes, width: int, height: int) -> bytes:
|
||||
header = struct.pack("<HHH", 0, 1, 1)
|
||||
entry = struct.pack(
|
||||
"<BBBBHHII",
|
||||
width % 256,
|
||||
height % 256,
|
||||
0,
|
||||
0,
|
||||
1,
|
||||
32,
|
||||
len(inner_png),
|
||||
6 + 16,
|
||||
)
|
||||
return header + entry + inner_png
|
||||
|
||||
|
||||
def ico_payload(payload: bytes) -> tuple[int, int, bytes]:
|
||||
reserved, kind, count = struct.unpack("<HHH", payload[:6])
|
||||
assert (reserved, kind, count) == (0, 1, 1)
|
||||
width, height, _colours, _reserved, _planes, _bits, size, offset = struct.unpack(
|
||||
"<BBBBHHII", payload[6:22]
|
||||
)
|
||||
return width or 256, height or 256, payload[offset : offset + size]
|
||||
|
||||
|
||||
DOWNLOAD_ARGS = ("--topic", "fox", "--format", "png", "--source", "fake")
|
||||
|
||||
|
||||
def test_download_resize_downscales_stored_image(
|
||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path
|
||||
) -> None:
|
||||
payload = png_bytes(200, 100)
|
||||
registry = FakeRegistry(
|
||||
[FakeCandidate("fox", "https://example.test/a.png", "image/png", 200, 100, payload=payload)]
|
||||
)
|
||||
|
||||
code, document = run_main(
|
||||
monkeypatch, capsys, registry, *BASE_ARGS, "--resize", "--download", "--output-dir", str(tmp_path)
|
||||
)
|
||||
|
||||
assert code == envelope.EXIT_OK
|
||||
download = document["items"][0]["download"]
|
||||
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}
|
||||
stored = Path(download["path"]).read_bytes()
|
||||
assert png_size(stored) == (100, 50)
|
||||
assert download["width"] == 100
|
||||
assert download["height"] == 50
|
||||
|
||||
|
||||
def test_download_without_resize_keeps_original_payload(
|
||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path
|
||||
) -> None:
|
||||
payload = png_bytes(200, 100)
|
||||
registry = FakeRegistry(
|
||||
[FakeCandidate("fox", "https://example.test/a.png", "image/png", 200, 100, payload=payload)]
|
||||
)
|
||||
|
||||
code, document = run_main(
|
||||
monkeypatch, capsys, registry, *BASE_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
|
||||
|
||||
|
||||
def test_resize_never_scales_up(
|
||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path
|
||||
) -> None:
|
||||
payload = png_bytes(50, 50)
|
||||
registry = FakeRegistry(
|
||||
[FakeCandidate("fox", "https://example.test/a.png", "image/png", 50, 50, payload=payload)]
|
||||
)
|
||||
|
||||
code, document = run_main(
|
||||
monkeypatch, capsys, registry, *DOWNLOAD_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
|
||||
|
||||
|
||||
def test_resize_downscales_png_stored_inside_ico(
|
||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path
|
||||
) -> None:
|
||||
payload = ico_bytes(png_bytes(100, 100), 100, 100)
|
||||
registry = FakeRegistry(
|
||||
[FakeCandidate("fox", "https://example.test/a.ico", "image/x-icon", 100, 100, payload=payload)]
|
||||
)
|
||||
|
||||
code, document = run_main(
|
||||
monkeypatch,
|
||||
capsys,
|
||||
registry,
|
||||
"--topic",
|
||||
"fox",
|
||||
"--format",
|
||||
"ico",
|
||||
"--source",
|
||||
"fake",
|
||||
"--width",
|
||||
"50",
|
||||
"--height",
|
||||
"50",
|
||||
"--resize",
|
||||
"--download",
|
||||
"--output-dir",
|
||||
str(tmp_path),
|
||||
)
|
||||
|
||||
assert code == envelope.EXIT_OK
|
||||
download = document["items"][0]["download"]
|
||||
assert download["resize"]["to"] == {"width": 50, "height": 50}
|
||||
width, height, inner = ico_payload(Path(download["path"]).read_bytes())
|
||||
assert (width, height) == (50, 50)
|
||||
assert png_size(inner) == (50, 50)
|
||||
|
||||
|
||||
def test_undecodable_payload_with_resize_fails_the_item(
|
||||
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path
|
||||
) -> None:
|
||||
registry = FakeRegistry(
|
||||
[
|
||||
FakeCandidate(
|
||||
"fox", "https://example.test/a.png", "image/png", 100, 100, payload=b"not-an-image"
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
code, document = run_main(
|
||||
monkeypatch, capsys, registry, *BASE_ARGS, "--resize", "--download", "--output-dir", str(tmp_path)
|
||||
)
|
||||
|
||||
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"
|
||||
|
||||
Reference in New Issue
Block a user