mirror of
https://github.com/wiltodelta/remove-ai-watermarks.git
synced 2026-08-10 08:00:32 +02:00
Make InvisibleOptions engine knobs only and pin the forwarding
`InvisibleOptions` promises in its docstring that every default mirrors
`InvisibleEngine`. Two fields made that promise cost something to keep: `force` is
not an engine parameter at all, and `controlnet_scale` was a third spelling of the
engine's `controlnet_conditioning_scale`. The mirror test carried an exception
table for each. This removes both, so the comparison needs no exceptions -- a field
that needs one is a field that belongs somewhere else.
`force` decides WHETHER the engine runs, which is settled before it is built, so it
joins `backend` and `sensitivity` as a parameter of `remove_all` and `remove_batch`
and is threaded to `_run_invisible` as its own argument. `controlnet_scale` takes
the engine's own name; the click option stays `--controlnet-scale` and is now
translated exactly once instead of at three forwarding sites.
Safe to do today: both symbols landed after 0.25.0 and have never been published.
The forwarding turned out to be the weaker half. A defaults comparison cannot see a
hardcoded literal at the seam, and `_run_invisible` passed the entire suite with
`controlnet_conditioning_scale` pinned to a constant. Each of the two knobs also
reaches the engine through TWO paths -- `remove_all` versus `remove_batch(mode="all")`
for `force`, `_run_invisible` versus `_batch_engine` for the scale -- and guarding one
left the other free to hardcode with a green suite. So:
* `test_every_field_arrives_at_the_engine_with_the_caller_s_value` drives the real
seam with all 13 fields set off their defaults; mutating any one of them to its
default fails it.
* `test_force_reaches_the_scrub_gate_in_every_scrubbing_mode` and
`test_batch_controlnet_scale_flows_to_the_cached_engine` are parametrized over
both modes, so neither path can be pinned alone.
Also fixes an order-dependent test surfaced by the added tests reshuffling the xdist
shards. `test_visible_path_decodes_file_once` counted every `image_io.imread` in the
process, but the Gemini engine loads its own bundled capture assets on first
construction, so the count was 3 on a cold engine and 1 on a warm one and the test
passed only when an earlier test happened to build the engine first. It now counts
decodes of the SOURCE, which is the invariant it exists for, and still fails when the
shared decode is broken. The production path was never wrong: the source bitmap is
decoded exactly once.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
13095fb45c
commit
4a896cd4b5
+103
-9
@@ -196,10 +196,12 @@ class TestRemoveVisibleOutputPath:
|
||||
|
||||
|
||||
class TestInvisibleOptionsMirrorTheEngine:
|
||||
"""``InvisibleOptions`` forwards to ``InvisibleEngine`` and promises every default
|
||||
mirrors it. Compare the signatures field by field rather than pinning the values we
|
||||
happen to know about, so the next field added on one side and not the other fails
|
||||
here. See the incident record in ``docs/module-internals.md``."""
|
||||
"""``InvisibleOptions`` forwards to ``InvisibleEngine`` and promises every NAME and
|
||||
default mirrors it. Compare the signatures field by field rather than pinning the
|
||||
values we happen to know about, so the next field added on one side and not the
|
||||
other fails here. The comparison deliberately has no exception table: a field that
|
||||
needs one is a field that belongs somewhere else, which is why ``force`` is a
|
||||
parameter of ``remove_all``. See the incident record in ``docs/module-internals.md``."""
|
||||
|
||||
def test_every_default_matches_the_engine(self):
|
||||
import dataclasses
|
||||
@@ -213,12 +215,67 @@ class TestInvisibleOptionsMirrorTheEngine:
|
||||
for name, p in inspect.signature(method).parameters.items()
|
||||
if p.default is not inspect.Parameter.empty
|
||||
}
|
||||
# ``force`` is a pipeline decision made before the engine runs, so it has no
|
||||
# engine counterpart; ``remove_watermark`` spells the controlnet knob out in full.
|
||||
renamed = {"controlnet_scale": "controlnet_conditioning_scale"}
|
||||
options = {f.name: f.default for f in dataclasses.fields(api.InvisibleOptions) if f.name != "force"}
|
||||
options = {f.name: f.default for f in dataclasses.fields(api.InvisibleOptions)}
|
||||
|
||||
assert options == {name: engine.get(renamed.get(name, name), "<not an engine parameter>") for name in options}
|
||||
assert options == {name: engine.get(name, "<not an engine parameter>") for name in options}
|
||||
|
||||
@pytest.mark.skipif(not CHATGPT.exists(), reason="sample image not present")
|
||||
def test_every_field_arrives_at_the_engine_with_the_caller_s_value(self, monkeypatch, tmp_path):
|
||||
"""A defaults comparison is not a forwarding test. `_run_invisible` hands each
|
||||
field to one of TWO engine callables by hand, and a hardcoded literal there is
|
||||
invisible to the mirror above -- `controlnet_conditioning_scale` shipped that way
|
||||
and the whole suite stayed green. Drive the real seam with every field set OFF
|
||||
its default and assert the caller's value arrives, whichever callable takes it."""
|
||||
import dataclasses
|
||||
|
||||
from remove_ai_watermarks import invisible_engine
|
||||
|
||||
# Every value differs from the default, so a hardcoded default cannot pass.
|
||||
opts = api.InvisibleOptions(
|
||||
strength=0.42,
|
||||
pipeline="sdxl-zimage",
|
||||
seed=7,
|
||||
hf_token="token",
|
||||
humanize=0.3,
|
||||
unsharp=0.2,
|
||||
adaptive_polish=True,
|
||||
max_resolution=1536,
|
||||
controlnet_conditioning_scale=0.65,
|
||||
cpu_offload=True,
|
||||
tile=True,
|
||||
tile_size=768,
|
||||
tile_overlap=64,
|
||||
)
|
||||
seen: dict[str, object] = {}
|
||||
|
||||
class FakeEngine:
|
||||
def __init__(self, **kwargs):
|
||||
seen.update(kwargs)
|
||||
|
||||
def remove_watermark(self, **kwargs):
|
||||
seen.update(kwargs)
|
||||
|
||||
monkeypatch.setattr(invisible_engine, "is_available", lambda: True)
|
||||
monkeypatch.setattr(invisible_engine, "InvisibleEngine", FakeEngine)
|
||||
api._run_invisible(
|
||||
CHATGPT,
|
||||
CHATGPT,
|
||||
tmp_path / "out.png",
|
||||
opts,
|
||||
None,
|
||||
lambda _stage, _detail: None,
|
||||
api._SourceEvidence(CHATGPT),
|
||||
True,
|
||||
)
|
||||
|
||||
missing = {f.name: getattr(opts, f.name) for f in dataclasses.fields(opts) if f.name not in seen}
|
||||
assert not missing, f"never forwarded to the engine: {missing}"
|
||||
wrong = {
|
||||
f.name: (getattr(opts, f.name), seen[f.name])
|
||||
for f in dataclasses.fields(opts)
|
||||
if seen[f.name] != getattr(opts, f.name)
|
||||
}
|
||||
assert not wrong, f"forwarded a value the caller did not pass (want, got): {wrong}"
|
||||
|
||||
|
||||
class TestRemoveAllLibrary:
|
||||
@@ -381,6 +438,43 @@ class TestRemoveBatchLibrary:
|
||||
assert summary.processed == 0
|
||||
assert summary.failed == 1
|
||||
|
||||
@pytest.mark.parametrize("mode", ["all", "invisible"])
|
||||
@pytest.mark.parametrize(("force", "expected"), [(True, "removed"), (False, "no-signal")])
|
||||
def test_force_reaches_the_scrub_gate_in_every_scrubbing_mode(self, monkeypatch, tmp_path, mode, force, expected):
|
||||
"""``force`` reaches the gate through a DIFFERENT seam per mode: ``all`` re-enters
|
||||
``remove_all``, ``invisible`` calls ``_run_invisible`` directly. Only the second
|
||||
was guarded, and pinning the first to False passed the whole suite while every
|
||||
output kept its watermark and the run still reported the files as processed."""
|
||||
from remove_ai_watermarks import api, image_io, invisible_engine
|
||||
|
||||
src = tmp_path / "in"
|
||||
src.mkdir()
|
||||
for i in range(2):
|
||||
image_io.imwrite(src / f"img{i}.png", np.full((64, 64, 3), 120, np.uint8))
|
||||
scrubbed: list[Path] = []
|
||||
|
||||
class FakeEngine:
|
||||
def remove_watermark(self, **kwargs):
|
||||
scrubbed.append(kwargs["image_path"])
|
||||
image_io.imwrite(kwargs["output_path"], np.full((64, 64, 3), 120, np.uint8))
|
||||
|
||||
monkeypatch.setattr(invisible_engine, "is_available", lambda: True)
|
||||
events: list[tuple[str, str]] = []
|
||||
|
||||
summary = api.remove_batch(
|
||||
src,
|
||||
tmp_path / "out",
|
||||
mode=mode,
|
||||
backend="cv2",
|
||||
force=force,
|
||||
engine=FakeEngine(),
|
||||
progress=lambda _p, stage, detail: events.append((stage, detail)),
|
||||
)
|
||||
|
||||
assert summary.processed == 2
|
||||
assert ("invisible", expected) in events
|
||||
assert len(scrubbed) == (2 if force else 0)
|
||||
|
||||
|
||||
class TestSourceEvidenceHolder:
|
||||
"""One metadata extraction per source file, per call.
|
||||
|
||||
@@ -485,6 +485,26 @@ class TestAllCommand:
|
||||
assert result.exit_code == 0, result.output
|
||||
assert mock_cls.call_args.kwargs["cpu_offload"] is True
|
||||
|
||||
def test_all_controlnet_scale_flows_to_engine(self, runner, sample_png, tmp_path):
|
||||
"""The click option is `--controlnet-scale`, the engine parameter is
|
||||
`controlnet_conditioning_scale`, and `InvisibleOptions` now uses the engine's
|
||||
spelling so the translation happens once. Pin the value, not just the name:
|
||||
hardcoding the constant in `_run_invisible` passed the whole suite before."""
|
||||
mock_cls, _mock_engine = _mock_invisible_engine()
|
||||
output = tmp_path / "clean.png"
|
||||
with (
|
||||
patch("remove_ai_watermarks.cli.InvisibleEngine", mock_cls, create=True),
|
||||
patch("remove_ai_watermarks.invisible_engine.InvisibleEngine", mock_cls),
|
||||
patch("remove_ai_watermarks.invisible_engine.is_available", return_value=True),
|
||||
):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["all", str(sample_png), "-o", str(output), "--controlnet-scale", "0.65", "--force"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert mock_cls.call_args.kwargs["controlnet_conditioning_scale"] == pytest.approx(0.65)
|
||||
|
||||
def test_all_nonexistent_file(self, runner):
|
||||
result = runner.invoke(main, ["all", "/nonexistent/file.png"])
|
||||
assert result.exit_code != 0
|
||||
@@ -802,6 +822,26 @@ class TestBatchCommand:
|
||||
assert result.exit_code == 0, result.output
|
||||
assert mock_cls.call_args.kwargs["cpu_offload"] is True
|
||||
|
||||
@pytest.mark.parametrize("mode", ["invisible", "all"])
|
||||
def test_batch_controlnet_scale_flows_to_the_cached_engine(self, runner, tmp_path, mode):
|
||||
"""`_batch_engine` is a SECOND engine-construction site: `all` builds its engine
|
||||
inside `_run_invisible`, batch prebuilds one for the whole directory. Pinning the
|
||||
value on the `all` path alone leaves this one free to hardcode a constant."""
|
||||
input_dir = _make_batch_dir(tmp_path)
|
||||
output_dir = tmp_path / "output"
|
||||
mock_cls, _mock_engine = _mock_invisible_engine()
|
||||
args = ["batch", str(input_dir), "-o", str(output_dir), "--mode", mode]
|
||||
with (
|
||||
patch("remove_ai_watermarks.cli.InvisibleEngine", mock_cls, create=True),
|
||||
patch("remove_ai_watermarks.invisible_engine.InvisibleEngine", mock_cls),
|
||||
patch("remove_ai_watermarks.cli.invisible_available", return_value=True, create=True),
|
||||
patch("remove_ai_watermarks.invisible_engine.is_available", return_value=True),
|
||||
):
|
||||
result = runner.invoke(main, [*args, "--controlnet-scale", "0.65", "--force"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert mock_cls.call_args.kwargs["controlnet_conditioning_scale"] == pytest.approx(0.65)
|
||||
|
||||
def test_batch_invisible_skips_no_signal_and_copies_through(self, runner, tmp_path):
|
||||
"""P0#5: batch invisible mode skips the scrub on signal-less images (no
|
||||
--force) and copies the input through, so the output dir is complete with the
|
||||
|
||||
@@ -782,13 +782,19 @@ class TestIdentifyVisibleTextMarks:
|
||||
"""The web path identify(check_visible=True, check_invisible=False) must
|
||||
decode the image exactly once and share the array across the sparkle +
|
||||
text-mark detectors. Two decodes of the same bitmap spiked memory on the
|
||||
small web worker (the OOM the decode-once refactor addresses)."""
|
||||
small web worker (the OOM the decode-once refactor addresses).
|
||||
|
||||
Count decodes OF THE SOURCE, not every ``imread`` in the process: the Gemini
|
||||
engine loads its own bundled capture assets on first construction, so a
|
||||
process-wide count was 3 on a cold engine and 1 on a warm one, and the test
|
||||
passed only when some earlier test happened to build the engine first."""
|
||||
import remove_ai_watermarks.image_io as image_io
|
||||
|
||||
real_imread = image_io.imread
|
||||
with patch.object(image_io, "imread", side_effect=real_imread) as mock_imread:
|
||||
identify(tmp_clean_png, check_visible=True, check_invisible=False)
|
||||
assert mock_imread.call_count == 1
|
||||
source_decodes = [c for c in mock_imread.call_args_list if Path(c.args[0]) == Path(tmp_clean_png)]
|
||||
assert len(source_decodes) == 1
|
||||
|
||||
def test_missing_pixel_extra_preserves_metadata_verdict(self, tmp_png_with_ai_metadata: Path):
|
||||
import remove_ai_watermarks.image_io as image_io
|
||||
|
||||
Reference in New Issue
Block a user