fix: address whole-project code review (visible all/batch, engine consolidation, I/O)

Nine findings from a high-effort project-wide review, fixed and verified
(571 passed, ruff/pyright clean):

Correctness:
- all/batch now remove Doubao/Jimeng/Samsung visible text marks: the visible
  step routes through the registry (new cli._remove_visible_auto) instead of a
  hardcoded GeminiEngine, so they no longer leave the wordmark intact.
- batch always reads the original source (dropped the out_path-reuse that
  re-processed already-cleaned outputs on a re-run).
- img2img_runner only retries the diffusion call on the deprecated-callback
  TypeError; any other TypeError now propagates instead of double-running.
- gemini detect/remove and the reverse-alpha engines normalize channels via a
  new image_io.to_bgr, fixing a grayscale/BGRA crash in the FP-gate path.
- _png_late_metadata advances its cursor by the clamped length, so a malformed
  chunk length no longer aborts the late AI-label scan.

Cleanup / efficiency:
- Consolidate the ~90%-identical Doubao/Jimeng/Samsung engines into a shared
  config-driven _text_mark_engine.TextMarkEngine base; each engine is now a thin
  subclass (TextMarkConfig + test shims). Behavior is byte-exact (the three
  engine test suites pass unchanged). Registry adapters collapse to one
  _text_mark(...) row each. Gemini stays a separate engine.
- scan_head is memoized per (path, size, mtime), so identify() reads the file
  head once instead of ~8 times.
- invisible_engine post-processing decodes/encodes the output once (chained in
  memory) instead of 2-4 times across stages.
- Remove the orphaned get_model_id_for_profile (+ CONTROLNET_PROFILE); derive
  the --strength help from the strength constants (strength_default_help) so it
  cannot drift; share the --pipeline/--strength click options; simplify the
  retired --auto resolver.

Net -835 lines. Tests added for the registry-routed visible pass, to_bgr,
the polish/model/guidance wiring, and strength_default_help. CLAUDE.md updated
for the new base module, the engine/registry changes, image_io.to_bgr, and the
scan_head cache.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Victor Kuznetsov
2026-06-09 13:21:13 -07:00
co-authored by Claude Opus 4.8
parent b1189549b8
commit 2fcd00ced0
17 changed files with 777 additions and 1264 deletions
+15
View File
@@ -374,6 +374,21 @@ class TestAllCommand:
result = runner.invoke(main, ["all", "/nonexistent/file.png"])
assert result.exit_code != 0
def test_all_visible_step_uses_registry(self, runner, sample_png, tmp_path):
"""Regression (#1): the `all` visible step must route through the registry
(best_auto_mark), so Doubao/Jimeng/Samsung text marks are handled -- not just
the Gemini sparkle via a hardcoded GeminiEngine."""
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.watermark_registry.best_auto_mark", return_value=None) as mock_best,
):
result = runner.invoke(main, ["all", str(sample_png), "-o", str(output)])
assert result.exit_code == 0, result.output
mock_best.assert_called() # the registry auto-detector drove the visible pass
def test_all_preserves_rgba_across_invisible_step(self, runner, tmp_path):
"""Regression: ``all`` must keep transparency even when the invisible
step writes a 3-channel result (as the real diffusion engine does).
+13
View File
@@ -124,6 +124,19 @@ class TestGeminiEngine:
result = self.engine.remove_watermark_custom(image, (10, 10, 48, 48))
assert result.shape == image.shape
def test_detect_on_grayscale_does_not_crash(self):
# A 2D grayscale array reaching detect_watermark (registry adapter / library
# API) must not crash the FP-gate's axis=2 reduction; it is normalized to BGR.
gray = np.full((300, 300), 100, dtype=np.uint8)
result = self.engine.detect_watermark(gray)
assert result is not None
def test_remove_on_bgra_returns_3_channel(self):
bgra = np.zeros((300, 300, 4), dtype=np.uint8)
bgra[..., 3] = 255
result = self.engine.remove_watermark(bgra)
assert result.shape == (300, 300, 3)
def test_remove_watermark_custom_large_region(self, tmp_image_path):
image = cv2.imread(str(tmp_image_path), cv2.IMREAD_COLOR)
result = self.engine.remove_watermark_custom(image, (10, 10, 96, 96))
+26
View File
@@ -59,6 +59,32 @@ class TestUnicodeRoundTrip:
assert np.array_equal(out, src)
class TestToBgr:
def test_grayscale_2d_promoted_to_bgr(self) -> None:
gray = np.full((4, 5), 120, dtype=np.uint8)
out = image_io.to_bgr(gray)
assert out.shape == (4, 5, 3)
# GRAY2BGR replicates the channel, so all three match the source.
assert np.array_equal(out[..., 0], gray)
assert np.array_equal(out[..., 0], out[..., 2])
def test_single_channel_3d_promoted(self) -> None:
gray = np.full((4, 5, 1), 7, dtype=np.uint8)
assert image_io.to_bgr(gray).shape == (4, 5, 3)
def test_bgra_dropped_to_bgr(self) -> None:
bgra = np.zeros((4, 5, 4), dtype=np.uint8)
bgra[..., :3] = (10, 120, 240)
out = image_io.to_bgr(bgra)
assert out.shape == (4, 5, 3)
assert np.array_equal(out, bgra[..., :3])
def test_bgr_returned_unchanged(self) -> None:
bgr = _make_bgr()
out = image_io.to_bgr(bgr)
assert out is bgr # 3-channel: no copy
class TestFailureSemantics:
def test_missing_file_returns_none(self, tmp_path: Path) -> None:
assert image_io.imread(tmp_path / "does-not-exist-不存在.png") is None
+7 -13
View File
@@ -20,7 +20,6 @@ from remove_ai_watermarks.noai.watermark_profiles import (
GEMINI_STRENGTH,
OPENAI_STRENGTH,
UNKNOWN_STRENGTH,
get_model_id_for_profile,
normalize_profile,
resolve_strength,
strength_default_help,
@@ -111,24 +110,19 @@ class TestMpsErrorDetection:
class TestModelProfiles:
"""Tests for watermark_profiles.py."""
"""Tests for watermark_profiles.py profile-name normalization."""
def test_sdxl_profile(self):
assert get_model_id_for_profile("sdxl") == "stabilityai/stable-diffusion-xl-base-1.0"
def test_canonical_profiles_unchanged(self):
assert normalize_profile("sdxl") == "sdxl"
assert normalize_profile("controlnet") == "controlnet"
def test_default_alias_resolves_to_sdxl(self):
# "default" is the legacy alias for "sdxl" (back-compat for existing scripts).
assert get_model_id_for_profile("default") == "stabilityai/stable-diffusion-xl-base-1.0"
assert normalize_profile("default") == "sdxl"
assert normalize_profile("controlnet") == "controlnet"
def test_controlnet_profile(self):
# controlnet shares the SDXL base checkpoint (the ControlNet is an add-on).
assert get_model_id_for_profile("controlnet") == "stabilityai/stable-diffusion-xl-base-1.0"
def test_unknown_profile_raises(self):
with pytest.raises(ValueError, match="Unknown model profile"):
get_model_id_for_profile("nonexistent")
def test_normalize_is_case_and_whitespace_insensitive(self):
assert normalize_profile(" Default ") == "sdxl"
assert normalize_profile("CONTROLNET") == "controlnet"
class TestResolveStrength: