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
Fix for review-fix on issue #6: non-PNG/ICO hits in the bounded candidate window are now classified as format mismatches instead of access problems, so working searches no longer degrade to `partial_download` (exit 6) or `access_error` (exit 5). Changes: - `tools/images.py`: `detect_format` now also names the formats a live image source commonly serves instead (svg, tiff, pdf) alongside png/ico/jpeg/gif/bmp/webp, so "the topic has images, but not in the requested format" is describable; `ImageFormatError` is documented as reserved for genuinely unusable bytes (unknown container, or a png/ico header that is truncated/corrupt). - `tools/example.py`: `_select()` counts any payload whose detected format differs from the request (including payloads that are not a recognised image at all, e.g. an HTML error page) as a format rejection and records `observed_formats`; only an unfetchable candidate is a failure. `Selection` gained `other_formats`. `_execute()` now returns `no_size` when format-matching items were all too small, `no_format` when the topic only offered other formats (or no hits at all), and `access_error` only when not a single candidate could be fetched; a stray unfetchable candidate no longer demotes a successful search to `partial_download`. Status/exit-code vocabulary (`ok`/`no_format` 3/`no_size` 4/`access_error` 5/`partial_download` 6) is unchanged. - `tests/test_images.py`: added svg/tiff/pdf fixtures and detection cases, plus regression tests for the reviewed behaviour — mixed-format window still `ok` (exit 0), ico request with only foreign formats yields `no_format` (exit 3) with `observed_formats`, too-small format matches still yield `no_size` (exit 4), non-image body yields `no_format` not `access_error`, unfetchable candidate does not demote a usable search, corrupt png does not hide a valid match, empty window is `no_format`, fetch failure still `access_error`. - `tests/test_cli.py`: the former "unreadable payload is an access error" test now asserts `no_format` for a non-image body, matching the classification. Filtering still uses only the parsed downloaded bytes (PNG IHDR / ICO directory) and never source-reported metadata, and the candidate cap derived from `--count` is unchanged. Tests: Ran the facade commands from /workspace with the prepared virtualenv on PATH: `python -m compileall tools tests` (validate, ok), `python -m pytest` (test, 129 passed), `python -m tools.example examples/input.json` (smoke, exit 0, dry_run envelope) and `python -m tools.example --help` (acceptance, exit 0). Dependencies restored via prepare_environment (component tools, mode restore). Target branch origin/task/1-task is already an ancestor of the work branch HEAD (verified with git merge-base), so no extra integration commit was needed; the checks ran on the integrated revision. Progress: 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. Published by developer-agent through Tool Backend. Closes #6
This commit was merged in pull request #14.
This commit is contained in:
+64
-49
@@ -17,7 +17,7 @@ import re
|
||||
import struct
|
||||
import sys
|
||||
import zlib
|
||||
from dataclasses import dataclass
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
from typing import Any, Mapping, Sequence
|
||||
|
||||
@@ -386,11 +386,19 @@ class KeptImage:
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class Selection:
|
||||
"""Outcome of inspecting a bounded window of downloaded candidates."""
|
||||
"""Outcome of inspecting a bounded window of downloaded candidates.
|
||||
|
||||
``format_rejects`` counts candidates whose real payload is *not* the
|
||||
requested format -- including payloads that are not a recognised image at
|
||||
all. ``size_rejects`` counts candidates of the requested format that are
|
||||
too small. ``failures`` only holds candidates whose bytes could not be
|
||||
fetched at all, because that is the one situation worth an access error.
|
||||
"""
|
||||
|
||||
kept: tuple[KeptImage, ...] = ()
|
||||
format_rejects: int = 0
|
||||
size_rejects: int = 0
|
||||
other_formats: Mapping[str, int] = field(default_factory=dict)
|
||||
failures: tuple[dict[str, Any], ...] = ()
|
||||
|
||||
@property
|
||||
@@ -400,16 +408,28 @@ class Selection:
|
||||
return len(self.kept) + self.format_rejects + self.size_rejects
|
||||
|
||||
|
||||
#: Bucket name for a downloaded payload that is not a recognised image at all.
|
||||
UNKNOWN_FORMAT = "unknown"
|
||||
|
||||
|
||||
def _count_other_format(other_formats: dict[str, int], detected: str | None) -> None:
|
||||
"""Record which other format a rejected candidate really was."""
|
||||
|
||||
key = detected or UNKNOWN_FORMAT
|
||||
other_formats[key] = other_formats.get(key, 0) + 1
|
||||
|
||||
|
||||
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.
|
||||
other-format bytes -- a JPEG, an SVG, a TIFF, an HTML error page, anything
|
||||
that is not a real ``png``/``ico`` payload -- counts as a *format rejection*
|
||||
so the caller gets ``no_format`` instead of an access error, and a candidate
|
||||
of the wanted format that is too small counts as a size rejection. Only a
|
||||
candidate whose bytes cannot be fetched at all is recorded as a failure.
|
||||
"""
|
||||
|
||||
wanted_format = str(request["format"])
|
||||
@@ -417,6 +437,7 @@ def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -
|
||||
kept: list[KeptImage] = []
|
||||
format_rejects = 0
|
||||
size_rejects = 0
|
||||
other_formats: dict[str, int] = {}
|
||||
failures: list[dict[str, Any]] = []
|
||||
|
||||
for candidate in candidates:
|
||||
@@ -433,24 +454,19 @@ def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -
|
||||
)
|
||||
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:
|
||||
# Whatever these bytes are (another image format, or no recognisable
|
||||
# image at all), they are not the requested format: that is a format
|
||||
# verdict about the topic, never an access problem.
|
||||
format_rejects += 1
|
||||
_count_other_format(other_formats, detected)
|
||||
continue
|
||||
try:
|
||||
info = images.inspect_image(payload)
|
||||
except ImageFormatError as error:
|
||||
# The signature claims the requested format but the header is
|
||||
# corrupt, so the payload itself is unusable rather than merely the
|
||||
# wrong format.
|
||||
failures.append(_failure(candidate, error.to_dict()))
|
||||
continue
|
||||
if not _meets_minimum_size(info, request):
|
||||
@@ -462,6 +478,7 @@ def _select(candidates: Sequence[SourceCandidate], request: Mapping[str, Any]) -
|
||||
kept=tuple(kept),
|
||||
format_rejects=format_rejects,
|
||||
size_rejects=size_rejects,
|
||||
other_formats=dict(other_formats),
|
||||
failures=tuple(failures),
|
||||
)
|
||||
|
||||
@@ -881,8 +898,14 @@ def _execute(argv: Sequence[str]) -> Response:
|
||||
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".
|
||||
* ``access_error`` -- the source could not be searched, or not a single
|
||||
candidate could be fetched, which is never reported as a "no match".
|
||||
|
||||
A candidate whose downloaded bytes are a different format (a JPEG, an SVG,
|
||||
an HTML error page, ...) is a format mismatch about the topic, not an access
|
||||
problem: it feeds ``no_format`` instead of turning a working search into an
|
||||
error, and a stray unfetchable candidate never demotes a usable answer to
|
||||
``partial_download``.
|
||||
"""
|
||||
|
||||
request, source, registry = _request_from(argv)
|
||||
@@ -904,17 +927,11 @@ def _execute(argv: Sequence[str]) -> Response:
|
||||
"candidate_count": len(candidates),
|
||||
"candidates_inspected": selection.inspected,
|
||||
"format_matches": selection.size_rejects,
|
||||
"other_format_candidates": selection.format_rejects,
|
||||
"observed_formats": dict(selection.other_formats),
|
||||
"size_matches": 0,
|
||||
"unreadable_candidates": len(unreadable),
|
||||
}
|
||||
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,
|
||||
@@ -924,31 +941,29 @@ def _execute(argv: Sequence[str]) -> Response:
|
||||
details=details,
|
||||
),
|
||||
)
|
||||
return builder.build(
|
||||
STATUS_NO_FORMAT,
|
||||
if selection.format_rejects or not unreadable:
|
||||
# Either the source really had no images in the requested format, or
|
||||
# it had no hits at all; both answer the topic, not an outage.
|
||||
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,
|
||||
),
|
||||
)
|
||||
return builder.access_error(
|
||||
error=error_fragment(
|
||||
"no_format_match",
|
||||
f"the source returned no {request['format']} images for {request['topic']!r}",
|
||||
details=details,
|
||||
),
|
||||
"candidate_unreadable",
|
||||
"none of the candidates could be downloaded",
|
||||
details={**details, "failures": unreadable},
|
||||
)
|
||||
)
|
||||
|
||||
if not request["download"]:
|
||||
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,
|
||||
},
|
||||
),
|
||||
)
|
||||
# Every reported item was fetched and verified against its real bytes, so
|
||||
# an unfetchable candidate is simply skipped: it must not demote an
|
||||
# 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)
|
||||
|
||||
Reference in New Issue
Block a user