change: Implement 4: Filter candidates by real format and minimum size from downloaded bytes
Repository Validate / repository-checks (pull_request) Successful in 3s
Repository Validate / repository-checks (pull_request) Successful in 3s
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
This commit is contained in:
+219
-60
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user