mirror of
https://github.com/agentscope-ai/ReMe.git
synced 2026-09-08 22:21:15 +00:00
fix: preserve resource image detail and linked daily ownership
This commit is contained in:
parent
552940211f
commit
1ca37437ad
4 changed files with 224 additions and 17 deletions
|
|
@ -158,19 +158,26 @@ def _normalize_image_bytes(
|
|||
needs_orientation = orientation in range(2, 9)
|
||||
if not needs_resize and not needs_convert and not needs_orientation:
|
||||
return None, source_mime, source_mime
|
||||
resize_frame = None
|
||||
try:
|
||||
frame = image
|
||||
if needs_resize:
|
||||
# Resize the decoded source before color conversion so large
|
||||
# non-JPEG images do not require a second full-size frame.
|
||||
# Pillow forces NEAREST for palette and bilevel images, even
|
||||
# when LANCZOS is requested. Expand these modes within the
|
||||
# checked pixel budget so resizing retains fine strokes and
|
||||
# palette transparency. Other modes resize before conversion.
|
||||
if image.mode in ("P", "1"):
|
||||
resize_frame = image.convert("RGBA" if image.mode == "P" else "L")
|
||||
frame = resize_frame
|
||||
# Pillow 10 cannot apply LANCZOS directly to 16-bit integer
|
||||
# modes; NEAREST keeps that path bounded without a full-size
|
||||
# RGB conversion first.
|
||||
resize_filter = image_module.Resampling.LANCZOS
|
||||
if image.mode.startswith("I;16"):
|
||||
if frame.mode.startswith("I;16"):
|
||||
resize_filter = image_module.Resampling.NEAREST
|
||||
image.thumbnail((MAX_IMAGE_REQUEST_DIMENSION, MAX_IMAGE_REQUEST_DIMENSION), resize_filter)
|
||||
has_alpha = image.mode in ("RGBA", "LA", "P")
|
||||
frame = image.convert("RGBA" if has_alpha else "RGB")
|
||||
frame.thumbnail((MAX_IMAGE_REQUEST_DIMENSION, MAX_IMAGE_REQUEST_DIMENSION), resize_filter)
|
||||
has_alpha = frame.mode in ("RGBA", "LA", "P")
|
||||
frame = frame.convert("RGBA" if has_alpha else "RGB")
|
||||
try:
|
||||
buffer = io.BytesIO()
|
||||
if frame.mode == "RGBA":
|
||||
|
|
@ -182,6 +189,9 @@ def _normalize_image_bytes(
|
|||
frame.close()
|
||||
except Exception as exc: # pylint: disable=broad-except
|
||||
raise RuntimeError(f"Failed to convert/resize image ({suffix or 'unknown suffix'}): {exc}") from exc
|
||||
finally:
|
||||
if resize_frame is not None:
|
||||
resize_frame.close()
|
||||
|
||||
|
||||
def _build_image_request_payload(
|
||||
|
|
|
|||
|
|
@ -245,11 +245,18 @@ class BaseAutoResourceStep(BaseStep):
|
|||
raise ValueError(f"invalid daily_dir {daily_dir!r}: {error or 'cannot resolve path'}")
|
||||
if not daily_root.is_dir():
|
||||
return []
|
||||
return sorted(
|
||||
entry.name
|
||||
for entry in daily_root.iterdir()
|
||||
if _DATE_RE.fullmatch(entry.name) and entry.is_dir() and not entry.is_symlink()
|
||||
)
|
||||
days = []
|
||||
for entry in daily_root.iterdir():
|
||||
if not _DATE_RE.fullmatch(entry.name):
|
||||
continue
|
||||
try:
|
||||
resolved, path_error = resolve_path(workspace, f"{daily_dir}/{entry.name}")
|
||||
if not path_error and resolved is not None and resolved.is_dir():
|
||||
days.append(entry.name)
|
||||
except (OSError, RuntimeError):
|
||||
# Broken or cyclic links must not prevent lookup in other days.
|
||||
continue
|
||||
return sorted(days)
|
||||
|
||||
async def _find_loose_resource_day(self, file_path: str) -> str | None:
|
||||
"""Find the single daily-card owner for a root-level resource."""
|
||||
|
|
|
|||
|
|
@ -237,15 +237,81 @@ def test_image_preprocessing_resizes_16_bit_tiff_before_rgb_conversion():
|
|||
assert max(sent.size) <= 2048
|
||||
|
||||
|
||||
@pytest.mark.parametrize(("mode", "transparent"), [("P", False), ("1", False), ("P", True)])
|
||||
def test_image_preprocessing_retains_thin_strokes_and_palette_alpha(mode, transparent):
|
||||
"""Filtered downscaling retains lines that nearest-neighbor sampling drops."""
|
||||
image = Image.new(mode, (4096, 256), 1 if mode == "1" else 0)
|
||||
if mode == "P":
|
||||
image.putpalette([255, 255, 255, 0, 0, 0] + [0] * 762)
|
||||
if transparent:
|
||||
image.info["transparency"] = 0
|
||||
image.paste(0 if mode == "1" else 1, (0, 0, 1, 256))
|
||||
buffer = io.BytesIO()
|
||||
image.save(buffer, format="PNG")
|
||||
image.close()
|
||||
|
||||
payload = _build_image_request_payload(buffer.getvalue(), ".png")
|
||||
|
||||
assert payload["source_mime"] == "image/png"
|
||||
with Image.open(io.BytesIO(base64.b64decode(payload["data_b64"]))) as sent:
|
||||
assert sent.size == (2048, 128)
|
||||
if transparent:
|
||||
assert payload["mime"] == "image/png"
|
||||
assert sent.mode == "RGBA"
|
||||
alpha_min, alpha_max = sent.getextrema()[3]
|
||||
assert alpha_min == 0
|
||||
assert 0 < alpha_max < 255
|
||||
else:
|
||||
assert sent.getextrema()[0][0] < 240
|
||||
assert sent.getpixel((sent.width - 1, 0))[:3] == (255, 255, 255)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("mode", ["P", "1"])
|
||||
@pytest.mark.parametrize("failing_method", ["thumbnail", "save"])
|
||||
def test_image_preprocessing_closes_expanded_frames_on_failure(mode, failing_method):
|
||||
"""Failure after mode expansion releases the temporary resize frame."""
|
||||
image = Image.new(mode, (2049, 2))
|
||||
buffer = io.BytesIO()
|
||||
image.save(buffer, format="PNG")
|
||||
image.close()
|
||||
converted_frames = []
|
||||
original_convert = Image.Image.convert
|
||||
|
||||
def record_convert(frame, *args, **kwargs):
|
||||
converted = original_convert(frame, *args, **kwargs)
|
||||
if frame.mode == mode:
|
||||
converted_frames.append(converted)
|
||||
return converted
|
||||
|
||||
with (
|
||||
patch.object(Image.Image, "convert", new=record_convert),
|
||||
patch.object(Image.Image, failing_method, side_effect=OSError(f"{failing_method} failed")),
|
||||
pytest.raises(RuntimeError, match="Failed to convert/resize image"),
|
||||
):
|
||||
_build_image_request_payload(buffer.getvalue(), ".png")
|
||||
|
||||
assert len(converted_frames) == 1
|
||||
for frame in converted_frames:
|
||||
with pytest.raises(ValueError, match="closed image"):
|
||||
frame.getpixel((0, 0))
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("source_size", "request_size"),
|
||||
[((3000, 1000), (683, 2048)), ((4096, 1024), (512, 2048))],
|
||||
ids=["resize-and-rotate", "decoder-downsample-and-rotate"],
|
||||
)
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_image_applies_exif_orientation_before_resizing(auto_resource_env):
|
||||
async def test_auto_image_applies_exif_orientation_before_resizing(source_size, request_size, auto_resource_env):
|
||||
"""A rotated phone JPEG is normalized upright for the VLM without touching its source."""
|
||||
env = auto_resource_env
|
||||
image = Image.new("RGB", (3000, 1000), (40, 80, 120))
|
||||
image = Image.new("RGB", source_size, (255, 0, 0))
|
||||
image.paste((0, 0, 255), (image.width // 2, 0, image.width, image.height))
|
||||
exif = Image.Exif()
|
||||
exif[274] = 6
|
||||
buffer = io.BytesIO()
|
||||
image.save(buffer, format="JPEG", exif=exif)
|
||||
image.close()
|
||||
source = env.write_binary("resource/2026-01-01/phone.jpg", buffer.getvalue())
|
||||
stored_bytes = source.read_bytes()
|
||||
model = _FakeVisionModel(_caption_json("upright-phone-photo", "Upright", "An upright phone photo."))
|
||||
|
|
@ -256,8 +322,12 @@ async def test_auto_image_applies_exif_orientation_before_resizing(auto_resource
|
|||
data_block = model.calls[0][0].content[1]
|
||||
assert data_block.source.media_type == "image/jpeg"
|
||||
with Image.open(io.BytesIO(base64.b64decode(data_block.source.data))) as sent:
|
||||
assert sent.size == (683, 2048)
|
||||
assert sent.size == request_size
|
||||
assert sent.getexif().get(274) is None
|
||||
top = sent.getpixel((sent.width // 2, sent.height // 4))
|
||||
bottom = sent.getpixel((sent.width // 2, 3 * sent.height // 4))
|
||||
assert top[0] > 240 and top[2] < 15
|
||||
assert bottom[2] > 240 and bottom[0] < 15
|
||||
assert source.read_bytes() == stored_bytes
|
||||
assert (env.workspace / "daily/2026-01-01/upright-phone-photo.md").is_file()
|
||||
|
||||
|
|
|
|||
|
|
@ -1,13 +1,18 @@
|
|||
"""Regression tests for the safety findings from the Auto Resource PR review."""
|
||||
|
||||
from unittest.mock import patch
|
||||
from pathlib import Path
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import frontmatter
|
||||
import pytest
|
||||
|
||||
from reme.components import R
|
||||
from reme.steps.evolve.auto_resource import AutoResourceStep
|
||||
from reme.steps.evolve.auto_text_resource import AutoTextResourceStep
|
||||
from reme.steps.evolve.base_auto_resource import BaseAutoResourceStep
|
||||
|
||||
from .auto_resource_test_support import (
|
||||
FakeAgentWrapper,
|
||||
FakeVisionModel,
|
||||
StructuredVisionModel,
|
||||
caption_json,
|
||||
|
|
@ -20,6 +25,16 @@ pytest_plugins = ("unit.auto_resource_test_plugin",)
|
|||
pytestmark = pytest.mark.asyncio
|
||||
|
||||
|
||||
def _link_daily_directory(workspace: Path, day: str, target: Path) -> None:
|
||||
"""Expose a fixture directory through the supported daily layout."""
|
||||
link = workspace / "daily" / day
|
||||
link.parent.mkdir(parents=True, exist_ok=True)
|
||||
try:
|
||||
link.symlink_to(target, target_is_directory=True)
|
||||
except OSError as exc:
|
||||
pytest.skip(f"symlinks unavailable: {exc}")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("routed", [False, True], ids=["image", "unified-router"])
|
||||
async def test_image_rejects_paths_outside_the_resource_tree(routed, auto_resource_env, tmp_path):
|
||||
"""Traversal and external paths fail before image reads."""
|
||||
|
|
@ -174,9 +189,14 @@ async def test_blank_plain_caption_does_not_create_or_overwrite_note(routed, pla
|
|||
|
||||
|
||||
@pytest.mark.parametrize("routed", [False, True], ids=["image", "unified-router"])
|
||||
async def test_loose_root_image_keeps_original_daily_card_across_days(routed, auto_resource_env):
|
||||
@pytest.mark.parametrize("linked_day", [False, True], ids=["directory", "internal-symlink"])
|
||||
async def test_loose_root_image_keeps_original_daily_card_across_days(routed, linked_day, auto_resource_env):
|
||||
"""Later updates and deletion keep a loose resource's first daily-card ownership."""
|
||||
env = auto_resource_env
|
||||
if linked_day:
|
||||
archive = env.workspace / "archive-day"
|
||||
archive.mkdir()
|
||||
_link_daily_directory(env.workspace, "2026-01-01", archive)
|
||||
source = env.write_binary("resource/photo.png", image_bytes(color=(200, 30, 30)))
|
||||
initial_model = FakeVisionModel(caption_json("original-card", "Original", "first-day caption"))
|
||||
with patch.object(BaseAutoResourceStep, "_today", return_value="2026-01-01"):
|
||||
|
|
@ -261,9 +281,14 @@ async def test_loose_root_image_keeps_original_daily_card_across_days(routed, au
|
|||
|
||||
|
||||
@pytest.mark.parametrize("routed", [False, True], ids=["image", "unified-router"])
|
||||
async def test_loose_root_image_duplicate_daily_owners_fail_closed(routed, auto_resource_env):
|
||||
@pytest.mark.parametrize("linked_day", [False, True], ids=["directory", "internal-symlink"])
|
||||
async def test_loose_root_image_duplicate_daily_owners_fail_closed(routed, linked_day, auto_resource_env):
|
||||
"""Ambiguous exact ownership is reported without reading the image model or changing notes."""
|
||||
env = auto_resource_env
|
||||
if linked_day:
|
||||
archive = env.workspace / "archive-day"
|
||||
archive.mkdir()
|
||||
_link_daily_directory(env.workspace, "2026-01-01", archive)
|
||||
source = env.write_binary("resource/duplicate.png", image_bytes())
|
||||
first_note = env.write_note("daily/2026-01-01/first.md", "[[resource/duplicate.png]]", body="first owner")
|
||||
second_note = env.write_note("daily/2026-01-02/second.md", "[[resource/duplicate.png]]", body="second owner")
|
||||
|
|
@ -288,3 +313,98 @@ async def test_loose_root_image_duplicate_daily_owners_fail_closed(routed, auto_
|
|||
assert not (env.workspace / "daily/2026-01-01.md").exists()
|
||||
assert not (env.workspace / "daily/2026-01-02.md").exists()
|
||||
assert not (env.workspace / "daily/2026-01-03.md").exists()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("routed", [False, True], ids=["image", "unified-router"])
|
||||
async def test_loose_root_lookup_ignores_unsafe_daily_links(routed, auto_resource_env, tmp_path):
|
||||
"""Outside, missing, and cyclic directories cannot expose notes or block a safe owner."""
|
||||
env = auto_resource_env
|
||||
outside = write_note(tmp_path / "outside-day/claim.md", "[[resource/photo.png]]", body="private outside note")
|
||||
outside_before = outside.read_bytes()
|
||||
_link_daily_directory(env.workspace, "2025-12-01", outside.parent)
|
||||
_link_daily_directory(env.workspace, "2025-12-02", env.workspace / "missing-day")
|
||||
_link_daily_directory(env.workspace, "2025-12-03", env.workspace / "daily/2025-12-03")
|
||||
source = env.write_binary("resource/photo.png", image_bytes())
|
||||
owned_note = env.write_note("daily/2026-01-01/original.md", "[[resource/photo.png]]")
|
||||
model = FakeVisionModel(caption_json("original", "Updated", "Updated inside workspace."))
|
||||
original_read_text = Path.read_text
|
||||
outside_reads = []
|
||||
|
||||
def guarded_read_text(path, *args, **kwargs):
|
||||
if path.resolve() == outside.resolve():
|
||||
outside_reads.append(path)
|
||||
raise AssertionError("cross-date lookup read a note outside the workspace")
|
||||
return original_read_text(path, *args, **kwargs)
|
||||
|
||||
with patch.object(Path, "read_text", guarded_read_text):
|
||||
with patch.object(BaseAutoResourceStep, "_today", return_value="2026-01-02"):
|
||||
updated = await env.run(env.processor(model, routed=routed), [{"change": "modified", "path": str(source)}])
|
||||
assert updated.success is True
|
||||
assert updated.metadata["results"][0]["metadata"]["path"] == "daily/2026-01-01/original.md"
|
||||
assert "Updated inside workspace." in owned_note.read_text(encoding="utf-8")
|
||||
source.unlink()
|
||||
with patch.object(BaseAutoResourceStep, "_today", return_value="2026-01-03"):
|
||||
deleted = await env.run(env.processor(model, routed=routed), [{"change": "deleted", "path": str(source)}])
|
||||
|
||||
assert deleted.success is True
|
||||
assert deleted.metadata["results"][0]["metadata"]["action"] == "deleted"
|
||||
assert not owned_note.exists()
|
||||
assert not outside_reads
|
||||
assert outside.read_bytes() == outside_before
|
||||
assert len(model.calls) == 1
|
||||
assert not (env.workspace / "daily/2026-01-02").exists()
|
||||
assert not (env.workspace / "daily/2026-01-03").exists()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("routed", [False, True], ids=["text", "unified-router"])
|
||||
async def test_loose_root_text_updates_and_deletes_original_daily_card(routed, auto_resource_env):
|
||||
"""Text processing also uses exact cross-date ownership for its prompt and lifecycle."""
|
||||
env = auto_resource_env
|
||||
source = env.write_binary("resource/report.txt", b"Updated report text.")
|
||||
note_rel = "daily/2026-01-01/original.md"
|
||||
owned_note = env.write_note(note_rel, "[[resource/report.txt]]", body="Original report text.")
|
||||
unrelated = env.write_note("daily/2026-01-02/report.md", "[[resource/other.txt]]", body="Keep unrelated report.")
|
||||
unrelated_before = unrelated.read_bytes()
|
||||
wrapper = FakeAgentWrapper()
|
||||
|
||||
async def update_note(inputs, **kwargs):
|
||||
assert "Date: 2026-01-01" in inputs
|
||||
assert f"Target note path: {note_rel}" in inputs
|
||||
assert "Updated report text." in inputs
|
||||
assert "read" in kwargs["job_tools"]
|
||||
response = await env.app_context.jobs["write"](
|
||||
path=note_rel,
|
||||
name="suggested-rename",
|
||||
description="Updated report",
|
||||
content="Updated report text.",
|
||||
metadata={"source_resource": "[[resource/report.txt]]"},
|
||||
)
|
||||
assert response.success is True
|
||||
return {"result": "Updated original report."}
|
||||
|
||||
env.app_context.registry = R
|
||||
step_cls = AutoResourceStep if routed else AutoTextResourceStep
|
||||
step = step_cls(app_context=env.app_context, file_store=env.file_store, agent_wrapper=wrapper, language="en")
|
||||
with patch.object(wrapper, "reply", new=AsyncMock(side_effect=update_note)) as reply:
|
||||
with patch.object(BaseAutoResourceStep, "_today", return_value="2026-01-02"):
|
||||
updated = await env.run(step, [{"change": "modified", "path": str(source)}])
|
||||
assert updated.success is True
|
||||
result = updated.metadata["results"][0]["metadata"]
|
||||
assert result["path"] == note_rel
|
||||
assert result["created"] is False
|
||||
assert result["action"] == "modified"
|
||||
assert result["index"]["date"] == "2026-01-01"
|
||||
assert "Updated report text." in owned_note.read_text(encoding="utf-8")
|
||||
source.unlink()
|
||||
with patch.object(BaseAutoResourceStep, "_today", return_value="2026-01-03"):
|
||||
deleted = await env.run(step, [{"change": "deleted", "path": str(source)}])
|
||||
reply.assert_awaited_once()
|
||||
|
||||
assert deleted.success is True
|
||||
assert deleted.metadata["results"][0]["metadata"]["path"] == note_rel
|
||||
assert deleted.metadata["results"][0]["metadata"]["action"] == "deleted"
|
||||
assert deleted.metadata["results"][0]["metadata"]["index"]["date"] == "2026-01-01"
|
||||
assert not owned_note.exists()
|
||||
assert unrelated.read_bytes() == unrelated_before
|
||||
assert list((env.workspace / "daily/2026-01-02").glob("*.md")) == [unrelated]
|
||||
assert not (env.workspace / "daily/2026-01-03").exists()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue