diff --git a/AGENTS.md b/AGENTS.md index cca53aa..2f4e6a6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,7 +1,20 @@ # Agent Instructions — Photo Analyzer -Read `CLAUDE.md` and `INTEGRATED_PIPELINE_CONCEPT.md` before implementation. Preserve -the established project invariants: never inspect or process content beneath any +`INTEGRATED_PIPELINE_CONCEPT.md` is the authoritative product and architecture +foundation for every user story. Before claiming or resuming implementation, read it +in full, then use it to interpret the story's scope, acceptance criteria, donor +strategy, architecture boundaries, safety invariants, fixture requirements, and test +obligations. Read `CLAUDE.md` as operational donor context after the concept. If an +issue, backlog Markdown file, or implementation idea conflicts with the concept, do +not silently follow the conflicting source: preserve the concept and stop for an +explicit decision when the conflict materially changes scope or behavior. + +Re-read the complete concept whenever context has been compacted/lost, the concept has +changed since the story began, or an implementation session is resumed without a +reliable record that the current version was read. A summary, prior familiarity, or +reading only the story's phase is not sufficient. + +Preserve the established project invariants: never inspect or process content beneath any `_IGNORE/` directory; write verified EXIF before Immich upload; use managed `immich-go` uploads rather than External Libraries; keep operations resume-safe; and treat the existing CLI implementations as primary donors rather than rewriting them. @@ -54,22 +67,26 @@ When asked to start, resume, or continue implementation, work from this isolated repository and use `work_item/scripts/work-item`. Gitea issues and their dependencies are the authoritative backlog. -1. Run `work_item/scripts/work-item claim`. -2. Run `work_item/scripts/work-item status` when resuming or whenever branch/state is +1. Read the current `INTEGRATED_PIPELINE_CONCEPT.md` completely and treat it as the + foundation for all decisions in the story. Then read `CLAUDE.md` for operational + donor context. +2. Run `work_item/scripts/work-item claim`. +3. Run `work_item/scripts/work-item status` when resuming or whenever branch/state is uncertain. Work only on the claimed story and its generated feature branch. -3. Read the entire issue, linked specification, dependencies, and acceptance criteria. -4. Inspect the legacy CLI donors before replacing applicable behavior. Update the donor +4. Read the entire issue, linked specification, dependencies, and acceptance criteria. + Reconcile them with the concept before designing or changing code. +5. Inspect the legacy CLI donors before replacing applicable behavior. Update the donor ledger and characterization tests required by the story. -5. Implement every acceptance criterion and its automated tests. -6. Run story-specific tests and the accumulated regression suite required by the epic. -7. Run `work_item/scripts/work-item submit --test ""` without +6. Implement every acceptance criterion and its automated tests. +7. Run story-specific tests and the accumulated regression suite required by the epic. +8. Run `work_item/scripts/work-item submit --test ""` without `--yes` and review the reported diff and file list. -8. If the changes are correct and safe, rerun with `--yes`. This commits, pushes, opens +9. If the changes are correct and safe, rerun with `--yes`. This commits, pushes, opens a pull request, records test evidence, and moves the issue to review. -9. After review and successful CI, run +10. After review and successful CI, run `work_item/scripts/work-item complete --merge`. This verifies the merge, closes the issue, returns to updated `main`, and removes the local feature branch. -10. Continue with the next eligible story by returning to step 1. +11. Continue with the next eligible story by returning to step 1. If genuinely blocked, run `work_item/scripts/work-item block --reason ""` and stop. Never skip to diff --git a/donor_ledger/README.md b/donor_ledger/README.md new file mode 100644 index 0000000..4f84ad4 --- /dev/null +++ b/donor_ledger/README.md @@ -0,0 +1,23 @@ +# Donor ledger + +`ledger.json` is the machine-readable migration inventory for the existing Photo +Analyzer and NSFW Tagger implementations. It is intentionally kept beside the live +donors until the archival gates in `INTEGRATED_PIPELINE_CONCEPT.md` are satisfied. + +Each row records the current source symbol, the intended treatment, its destination in +the integrated application, the reason for that decision, and the stable +characterization-test IDs protecting current behavior. `migration_status` remains +`inventoried` in US01-01: this story characterizes code but does not relocate it. + +Classification meanings: + +- `reuse`: preserve the implementation essentially unchanged; +- `extract`: move the coherent behavior behind a thin service/integration adapter; +- `refactor`: preserve the useful behavior while changing an incompatible boundary; +- `replace`: intentionally supersede unsafe or architecturally incompatible behavior. + +Run the story suite with: + +```bash +work_item/scripts/python -m unittest discover -s tests/characterization -v +``` diff --git a/donor_ledger/ledger.json b/donor_ledger/ledger.json new file mode 100644 index 0000000..67f296f --- /dev/null +++ b/donor_ledger/ledger.json @@ -0,0 +1,180 @@ +{ + "schema_version": 1, + "story": "US01-01", + "concept": "INTEGRATED_PIPELINE_CONCEPT.md", + "classifications": ["reuse", "extract", "refactor", "replace"], + "required_areas": [ + "discovery", "hashing", "imaging", "nsfw", "vision", "exif", + "database", "ui", "configuration", "logging", "cancellation", "error_behavior" + ], + "entries": [ + { + "id": "DONOR-001", "area": "discovery", + "sources": ["photo_analyzer.py::discover_photos", "photo_analyzer.py::SUPPORTED_EXTENSIONS"], + "classification": "extract", "target": "src/photo_pipeline/services/inventory.py", + "rationale": "Preserve sorted recursive discovery and supported formats behind the shared path policy.", + "intentional_changes": "All callers use stable asset IDs and one centralized boundary/exclusion policy.", + "test_ids": ["CHAR-001"], "migration_status": "inventoried" + }, + { + "id": "DONOR-002", "area": "discovery", + "sources": ["nsfwtag/scoring.py::discover_images", "nsfwtag/__init__.py::EXTS"], + "classification": "replace", "target": "src/photo_pipeline/services/inventory.py", + "rationale": "Its shallow/recursive switch and stable ordering are useful, but a second discovery policy and narrower formats conflict with the shared inventory.", + "intentional_changes": "Safety consumes canonical inventory assets rather than walking paths itself.", + "test_ids": ["CHAR-001"], "migration_status": "inventoried" + }, + { + "id": "DONOR-003", "area": "hashing", + "sources": ["photo_analyzer.py::_sha1_file", "photo_analyzer.py::_phash_image", "photo_analyzer.py::ensure_hashes"], + "classification": "refactor", "target": "src/photo_pipeline/services/inventory.py", + "rationale": "Retain streaming exact hashing and DCT pHash behavior while adding SHA-256, normalized pixels, versions, bounded workers, and stable IDs.", + "intentional_changes": "SHA-256 becomes byte identity; pHash is evidence rather than automatic fuzzy exclusion.", + "test_ids": ["CHAR-002"], "migration_status": "inventoried" + }, + { + "id": "DONOR-004", "area": "hashing", + "sources": ["photo_analyzer.py::cluster_duplicates", "photo_analyzer.py::mark_duplicates", "photo_analyzer.py::reconcile_moved"], + "classification": "refactor", "target": "src/photo_pipeline/services/duplicates.py", + "rationale": "Keep Hamming-distance evidence and reconciliation knowledge, but path-keyed union-find and automatic largest-file selection do not meet review and identity rules.", + "intentional_changes": "Exact matches may be recommended; fuzzy matches require durable human decisions and negative links.", + "test_ids": ["CHAR-002", "CHAR-003"], "migration_status": "inventoried" + }, + { + "id": "DONOR-005", "area": "imaging", + "sources": ["photo_analyzer.py::prepare_image"], + "classification": "extract", "target": "src/photo_pipeline/services/analysis.py", + "rationale": "The in-memory RGB conversion, long-edge resize, and JPEG normalization are proven analysis preparation behavior.", + "intentional_changes": "Orientation, color profile, pixel limits, and decoder version become explicit and tested.", + "test_ids": ["CHAR-002"], "migration_status": "inventoried" + }, + { + "id": "DONOR-006", "area": "imaging", + "sources": ["nsfwtag/server.py::serve_review", "nsfwtag/webapp.py::render_page"], + "classification": "refactor", "target": "src/photo_pipeline/services/thumbnails.py", + "rationale": "Preserve useful review presentation behavior, but arbitrary path image serving cannot be the integrated thumbnail boundary.", + "intentional_changes": "Managed oriented thumbnails are resolved only by asset ID and bounded size.", + "test_ids": ["CHAR-007"], "migration_status": "inventoried" + }, + { + "id": "DONOR-007", "area": "nsfw", + "sources": ["nsfwtag/scoring.py::score_images", "nsfwtag/__init__.py::MODEL_ID", "nsfwtag/__init__.py::DEFAULT_THRESHOLD"], + "classification": "extract", "target": "src/photo_pipeline/integrations/nsfw_model.py", + "rationale": "Retain the selected local model, batching, score interpretation, and error isolation behind an injectable integration.", + "intentional_changes": "SQLite is authoritative; CSV becomes optional export and only canonical assets are scored.", + "test_ids": ["CHAR-006"], "migration_status": "inventoried" + }, + { + "id": "DONOR-008", "area": "nsfw", + "sources": ["nsfwtag/__main__.py::main", "nsfw_tag.py::main"], + "classification": "refactor", "target": "src/photo_pipeline/services/safety.py", + "rationale": "Keep CLI compatibility temporarily while moving orchestration to durable jobs and explicit review decisions.", + "intentional_changes": "The API never invokes the legacy CLI and cloud analysis requires confirmed SFW state.", + "test_ids": ["CHAR-008"], "migration_status": "inventoried" + }, + { + "id": "DONOR-009", "area": "vision", + "sources": ["photo_analyzer.py::ANALYSIS_PROMPT", "photo_analyzer.py::analyze_image"], + "classification": "extract", "target": "src/photo_pipeline/integrations/vision.py", + "rationale": "Preserve provider-compatible image requests, prompt context, fenced-JSON handling, token accounting, and retry categories.", + "intentional_changes": "Typed validation, persisted prompt/model versions, bounded jittered retries, and stale-result fencing are added.", + "test_ids": ["CHAR-005"], "migration_status": "inventoried" + }, + { + "id": "DONOR-010", "area": "exif", + "sources": ["photo_analyzer.py::build_exif_caption_from_result", "photo_analyzer.py::write_exif", "photo_analyzer.py::read_existing_exif"], + "classification": "refactor", "target": "src/photo_pipeline/integrations/exiftool.py", + "rationale": "Retain additive caption/tag projection and exiftool knowledge while making ownership and read-back verification explicit.", + "intentional_changes": "Each metadata stage snapshots non-owned fields, verifies its projection, and refreshes post-write hashes.", + "test_ids": ["CHAR-004", "CHAR-009"], "migration_status": "inventoried" + }, + { + "id": "DONOR-011", "area": "exif", + "sources": ["nsfwtag/exif.py::write_keyword", "nsfwtag/exif.py::remove_keyword", "nsfwtag/exif.py::read_marks"], + "classification": "extract", "target": "src/photo_pipeline/integrations/exiftool.py", + "rationale": "The idempotent keyword commands and mutually exclusive review semantics are reusable in a shared adapter.", + "intentional_changes": "Both SFW and NSFW decisions are persisted and verified without removing unrelated metadata.", + "test_ids": ["CHAR-009"], "migration_status": "inventoried" + }, + { + "id": "DONOR-012", "area": "database", + "sources": ["photo_analyzer.py::SCHEMA", "photo_analyzer.py::get_db", "photo_analyzer.py::_migrate_schema"], + "classification": "refactor", "target": "src/photo_pipeline/models/", + "rationale": "Preserve historical schema and FTS migration knowledge while introducing SQLAlchemy repositories, Alembic, WAL, foreign keys, and stable asset identity.", + "intentional_changes": "Paths become occurrences, not primary identity; migrations are versioned and transactional where possible.", + "test_ids": ["CHAR-003"], "migration_status": "inventoried" + }, + { + "id": "DONOR-013", "area": "database", + "sources": ["webapp/query.py::search", "webapp/query.py::facets", "webapp/query.py::stats"], + "classification": "extract", "target": "src/photo_pipeline/repositories/assets.py", + "rationale": "FTS query sanitization, paging, facets, album labels, and stats are useful read-model behavior.", + "intentional_changes": "Repositories own SQL and return typed stable-ID projections.", + "test_ids": ["CHAR-003"], "migration_status": "inventoried" + }, + { + "id": "DONOR-014", "area": "ui", + "sources": ["nsfwtag/webapp.py::folder_tree_html", "nsfwtag/webapp.py::render_page", "nsfwtag/review.html"], + "classification": "refactor", "target": "frontend/js/views/safety.js", + "rationale": "Preserve folder navigation, score cards, review controls, lightbox interaction, and visual language.", + "intentional_changes": "Semantic static HTML/CSS/JS fetch JSON from /api/v1; Python no longer renders operational cards.", + "test_ids": ["CHAR-007"], "migration_status": "inventoried" + }, + { + "id": "DONOR-015", "area": "ui", + "sources": ["webapp/page.py::render_page", "webapp/server.py::serve"], + "classification": "replace", "target": "frontend/index.html", + "rationale": "The design is a donor, but server-generated application state and ad-hoc HTTP routing conflict with the application-shell/API architecture.", + "intentional_changes": "FastAPI serves a data-free shell and versioned JSON/SSE endpoints.", + "test_ids": ["CHAR-007"], "migration_status": "inventoried" + }, + { + "id": "DONOR-016", "area": "configuration", + "sources": ["photo_analyzer.py::load_env_file", "photo_analyzer.py::_env_int", "photo_analyzer.py::_env_bool"], + "classification": "replace", "target": "src/photo_pipeline/config.py", + "rationale": "Environment precedence is useful, but a manual untyped parser cannot validate the integrated app's paths, secrets, and worker settings.", + "intentional_changes": "Pydantic settings provide typed validation and secret references.", + "test_ids": ["CHAR-010"], "migration_status": "inventoried" + }, + { + "id": "DONOR-017", "area": "logging", + "sources": ["photo_analyzer.py::log_history", "photo_analyzer.py::_record_ratelimit"], + "classification": "refactor", "target": "src/photo_pipeline/jobs/events.py", + "rationale": "Per-item history, token evidence, and throttle diagnostics remain valuable operational events.", + "intentional_changes": "Structured durable events carry job, asset, attempt, and operation IDs with retention controls.", + "test_ids": ["CHAR-011"], "migration_status": "inventoried" + }, + { + "id": "DONOR-018", "area": "cancellation", + "sources": ["photo_analyzer.py::_handle_sigint", "webapp/runner.py::Runner"], + "classification": "replace", "target": "src/photo_pipeline/jobs/coordinator.py", + "rationale": "The cooperative stop intent is correct, but in-memory events and subprocess ownership do not survive restart or provide fencing.", + "intentional_changes": "Cancellation is a durable job transition checked between items and recovered by heartbeat/lease state.", + "test_ids": ["CHAR-012"], "migration_status": "inventoried" + }, + { + "id": "DONOR-019", "area": "error_behavior", + "sources": ["photo_analyzer.py::mark_error", "photo_analyzer.py::analyze_image", "photo_analyzer.py::write_exif"], + "classification": "refactor", "target": "src/photo_pipeline/services/analysis.py", + "rationale": "Keep item-level failures, retry categories, raw-response diagnostics, and explicit EXIF failure behavior.", + "intentional_changes": "Errors gain stable codes, attempts, retryability, and unknown/divergent states rather than broad status strings.", + "test_ids": ["CHAR-005", "CHAR-010"], "migration_status": "inventoried" + }, + { + "id": "DONOR-020", "area": "error_behavior", + "sources": ["nsfwtag/scoring.py::score_images", "nsfwtag/exif.py::_read_keywords", "webapp/runner.py::Runner"], + "classification": "refactor", "target": "src/photo_pipeline/services/safety.py", + "rationale": "Per-file inference isolation and tolerant metadata reads are useful, while process errors need durable domain outcomes.", + "intentional_changes": "Failures are persisted per attempt; unavailable integrations block precisely and never imply a safety decision.", + "test_ids": ["CHAR-006", "CHAR-010"], "migration_status": "inventoried" + }, + { + "id": "DONOR-021", "area": "configuration", + "sources": ["photo_analyzer.py::main", "nsfwtag/__main__.py::main"], + "classification": "refactor", "target": "src/photo_pipeline/__main__.py", + "rationale": "The current flags document supported workflows and compatibility needs, but long operations must become service-backed durable jobs.", + "intentional_changes": "Compatibility CLIs call shared services; API and worker management use typed commands.", + "test_ids": ["CHAR-008"], "migration_status": "inventoried" + } + ] +} diff --git a/tests/characterization/test_donors.py b/tests/characterization/test_donors.py new file mode 100644 index 0000000..a403489 --- /dev/null +++ b/tests/characterization/test_donors.py @@ -0,0 +1,295 @@ +from __future__ import annotations + +import base64 +import importlib +import json +import logging +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest import mock + +from PIL import Image + + +REPO = Path(__file__).resolve().parents[2] +FIXTURES = REPO / "tests" / "fixtures" / "characterization" + + +class DonorCharacterizationTests(unittest.TestCase): + """Golden contracts for donor behavior; these are not target-architecture tests.""" + + @classmethod + def setUpClass(cls): + cls.import_dir = tempfile.TemporaryDirectory(prefix="donor-import-") + old_cwd = Path.cwd() + sys.path.insert(0, str(REPO)) + try: + os.chdir(cls.import_dir.name) + cls.analyzer = importlib.import_module("photo_analyzer") + cls.nsfw_scoring = importlib.import_module("nsfwtag.scoring") + cls.nsfw_exif = importlib.import_module("nsfwtag.exif") + cls.nsfw_webapp = importlib.import_module("nsfwtag.webapp") + cls.web_query = importlib.import_module("webapp.query") + finally: + os.chdir(old_cwd) + cls.manifest = json.loads((FIXTURES / "manifest.json").read_text())["fixtures"] + cls.expected = json.loads((FIXTURES / "expected_outputs.json").read_text())["outputs"] + + @classmethod + def tearDownClass(cls): + for handler in list(logging.getLogger().handlers) + list( + logging.getLogger("history").handlers + ): + handler.close() + cls.import_dir.cleanup() + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory(prefix="donor-fixtures-") + self.root = Path(self.tmp.name) + + def tearDown(self): + self.analyzer._stop.clear() + self.tmp.cleanup() + + def _make_discovery_library(self) -> None: + recipes = {item["id"]: item for item in self.manifest} + colors = { + "DISC-ROOT-JPEG": (10, 20, 30), + "DISC-UPPER-JPEG": (40, 50, 60), + "DISC-NESTED-WEBP": (70, 80, 90), + "DISC-TIFF": (100, 110, 120), + } + sizes = { + "DISC-ROOT-JPEG": (12, 8), + "DISC-UPPER-JPEG": (8, 12), + "DISC-NESTED-WEBP": (9, 9), + "DISC-TIFF": (16, 4), + } + for fixture_id, color in colors.items(): + path = self.root / recipes[fixture_id]["relative_path"] + path.parent.mkdir(parents=True, exist_ok=True) + Image.new("RGB", sizes[fixture_id], color).save(path) + unsupported = self.root / recipes["DISC-UNSUPPORTED"]["relative_path"] + unsupported.write_text("not-an-image", encoding="utf-8") + + def _relative(self, paths) -> list[str]: + return [str(Path(path).relative_to(self.root)) for path in paths] + + def test_discovery_outputs(self): + self._make_discovery_library() + + analyzer_paths = self.analyzer.discover_photos(self.root) + nsfw_recursive = self.nsfw_scoring.discover_images(self.root, recursive=True) + nsfw_shallow = self.nsfw_scoring.discover_images(self.root, recursive=False) + + self.assertEqual( + self._relative(analyzer_paths), self.expected["analyzer_recursive_discovery"] + ) + self.assertEqual(self._relative(nsfw_recursive), self.expected["nsfw_recursive_discovery"]) + self.assertEqual(self._relative(nsfw_shallow), self.expected["nsfw_shallow_discovery"]) + + def test_image_preparation_and_hash_outputs(self): + raw_path = self.root / "hash.bin" + raw_path.write_bytes(b"photo-pipeline-donor") + self.assertEqual(self.analyzer._sha1_file(raw_path), self.expected["hash_bytes_sha1"]) + + gradient = Image.new("RGB", (40, 20)) + gradient.putdata( + [(x * 5 % 256, y * 11 % 256, (x + y) * 7 % 256) for y in range(20) for x in range(40)] + ) + image_path = self.root / "gradient.png" + gradient.save(image_path) + old_edge = self.analyzer.MAX_LONG_EDGE + try: + self.analyzer.MAX_LONG_EDGE = 16 + encoded, mime = self.analyzer.prepare_image(image_path) + finally: + self.analyzer.MAX_LONG_EDGE = old_edge + prepared_path = self.root / "prepared.jpg" + prepared_path.write_bytes(base64.b64decode(encoded)) + with Image.open(prepared_path) as prepared: + self.assertEqual(prepared.size, (16, 8)) + self.assertEqual(prepared.mode, "RGB") + self.assertEqual(mime, "image/jpeg") + phash = self.analyzer._phash_image(image_path) + self.assertRegex(phash or "", r"^[0-9a-f]{16}$") + self.assertEqual(phash, self.analyzer._phash_image(image_path)) + + def test_database_status_and_fts_outputs(self): + db_path = self.root / "characterization.sqlite" + conn = self.analyzer.get_db(str(db_path)) + self.addCleanup(conn.close) + photo_path = str(self.root / "lake.jpg") + self.analyzer.upsert_pending(conn, photo_path) + result = { + "description": "Three adults walk beside a lake.", + "tags": ["people", "lake", "summer"], + "people_count": 3, + "setting": "outdoor", + "time_of_day": "afternoon", + "season": "summer", + "mood": "relaxed", + "location_hint": "Como, Italy", + "approx_year": 2021, + } + self.analyzer.mark_analyzed(conn, photo_path, result, json.dumps(result)) + row = conn.execute("SELECT * FROM photos WHERE path = ?", (photo_path,)).fetchone() + self.assertEqual(row["status"], "analyzed") + self.assertEqual(json.loads(row["tags"]), result["tags"]) + found = self.web_query.search(conn, q="lake") + self.assertEqual(found["total"], 1) + self.assertEqual(found["rows"][0]["path"], photo_path) + self.analyzer.mark_exif_written(conn, photo_path) + self.assertEqual(conn.execute("SELECT status FROM photos").fetchone()[0], "exif_written") + + def test_caption_and_variant_outputs(self): + result = { + "description": "Three adults walk beside a lake.", + "tags": ["people", "lake", "summer"], + "mood": "relaxed", + "location_hint": "Como, Italy", + "approx_year": 2021, + } + self.assertEqual( + self.analyzer.build_exif_caption_from_result(result), self.expected["caption"] + ) + self.assertEqual( + self.analyzer._strip_variant_markers("IMG_0001-bearbeitet (1920x1080)"), + self.expected["variant_base"], + ) + + def test_vision_dry_run_output(self): + result, raw, tokens = self.analyzer.analyze_image( + None, self.root / "fixture.jpg", dry_run=True + ) + self.assertEqual(result["tags"], self.expected["dry_run_tags"]) + self.assertEqual(json.loads(raw), result) + self.assertEqual(tokens, {"prompt": 0, "completion": 0, "total": 0}) + + def test_nsfw_cache_output(self): + cache_path = self.root / "scores.csv" + values = self.expected["nsfw_cache"] + self.nsfw_scoring._save_cache(cache_path, values) + self.assertEqual(self.nsfw_scoring.load_cache(cache_path), values) + self.assertEqual( + cache_path.read_text(encoding="utf-8").splitlines(), + ["path,nsfw_score", "a.jpg,0.1250", "b.jpg,0.9876"], + ) + + def test_review_html_output(self): + logical = self.root / "album & one" / ".jpg" + page = self.nsfw_webapp.render_page([(logical, 0.8123), (logical, 0.7)], 0.6) + self.assertIn("<portrait>.jpg", page) + self.assertIn("album & one", page) + self.assertIn(f'data-score="{0.8123:.4f}"', page) + self.assertIn(self.expected["ui_total"], page) + self.assertIn(self.expected["ui_threshold"], page) + self.assertEqual(page.count('class="card"'), 1) + + def test_cli_entry_point_help(self): + env = os.environ.copy() + env["PYTHONPATH"] = str(REPO) + commands = [ + ([sys.executable, str(REPO / "photo_analyzer.py"), "--help"], "--group-variants"), + ([sys.executable, str(REPO / "nsfw_tag.py"), "--help"], "--review-min"), + ([sys.executable, "-m", "nsfwtag", "--help"], "--threshold"), + ] + for command, marker in commands: + with self.subTest(command=command): + result = subprocess.run( + command, + cwd=self.root, + env=env, + capture_output=True, + text=True, + timeout=30, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn(marker, result.stdout) + + def test_exif_command_contracts(self): + calls = [] + + def fake_run(command, **kwargs): + calls.append(command) + if "-json" in command: + return subprocess.CompletedProcess( + command, + 0, + stdout=json.dumps( + [{"ImageDescription": "User caption", "Keywords": ["family"]}] + ), + stderr="", + ) + return subprocess.CompletedProcess(command, 0, stdout="", stderr="") + + with mock.patch.object(self.analyzer.subprocess, "run", side_effect=fake_run): + self.assertTrue( + self.analyzer.write_exif("fixture.jpg", "AI caption", ["family", "lake"]) + ) + write_command = calls[-1] + self.assertIn("-overwrite_original", write_command) + self.assertIn("-ImageDescription=AI caption | User caption", write_command) + self.assertEqual(write_command.count("-Keywords=family"), 1) + self.assertEqual(write_command.count("-Keywords=lake"), 1) + + with mock.patch.object( + self.nsfw_exif.subprocess, + "run", + return_value=subprocess.CompletedProcess([], 0, stdout="", stderr=""), + ) as run: + self.assertTrue(self.nsfw_exif.write_keyword("fixture.jpg")) + command = run.call_args.args[0] + self.assertIn("-Keywords-=nsfw", command) + self.assertIn("-Keywords+=nsfw", command) + self.assertIn("-Subject-=nsfw", command) + self.assertIn("-Subject+=nsfw", command) + + def test_error_and_configuration_fallbacks(self): + with mock.patch.dict(os.environ, {"MAX_WORKERS": "not-an-int"}): + self.assertEqual(self.analyzer._env_int("MAX_WORKERS", 3), 3) + invalid = subprocess.CompletedProcess([], 0, stdout="not-json", stderr="") + with mock.patch.object(self.nsfw_exif.subprocess, "run", return_value=invalid): + self.assertEqual(self.nsfw_exif._read_keywords(["fixture.jpg"]), {}) + + def test_history_log_shape(self): + sink = mock.Mock() + result = { + "description": "Fixture description", + "tags": ["fixture"], + "mood": "calm", + "setting": "indoor", + "people_count": 0, + "location_hint": None, + "approx_year": None, + } + with mock.patch.object(self.analyzer.history_log, "info", sink): + self.analyzer.log_history( + "fixture.jpg", "analyzed", result=result, tokens={"total": 12} + ) + entry = json.loads(sink.call_args.args[0]) + self.assertEqual(entry["path"], "fixture.jpg") + self.assertEqual(entry["status"], "analyzed") + self.assertEqual(entry["tokens_total"], 12) + self.assertEqual(entry["tags"], ["fixture"]) + self.assertRegex(entry["ts"], r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}$") + + def test_cooperative_cancellation_contract(self): + self.analyzer._stop.clear() + fake_stderr = SimpleNamespace(write=mock.Mock(), flush=mock.Mock()) + with ( + mock.patch.object(self.analyzer.sys, "__stderr__", fake_stderr), + mock.patch.object(self.analyzer.log, "warning"), + ): + self.analyzer._handle_sigint(None, None) + self.assertTrue(self.analyzer._stop.is_set()) + fake_stderr.write.assert_called_once_with("\a") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/characterization/test_ledger.py b/tests/characterization/test_ledger.py new file mode 100644 index 0000000..f016cce --- /dev/null +++ b/tests/characterization/test_ledger.py @@ -0,0 +1,62 @@ +from __future__ import annotations + +import ast +import json +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parents[2] +LEDGER = REPO / "donor_ledger" / "ledger.json" +TRACEABILITY = REPO / "tests" / "characterization" / "test_traceability.json" +DONOR_TESTS = REPO / "tests" / "characterization" / "test_donors.py" + + +def source_symbols(path: Path) -> set[str]: + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + names = set() + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + names.add(node.name) + elif isinstance(node, (ast.Assign, ast.AnnAssign)): + targets = node.targets if isinstance(node, ast.Assign) else [node.target] + names.update(t.id for t in targets if isinstance(t, ast.Name)) + elif isinstance(node, ast.ImportFrom): + names.update(alias.asname or alias.name for alias in node.names) + return names + + +class DonorLedgerLintTests(unittest.TestCase): + def test_ledger_is_complete_and_resolvable(self): + ledger = json.loads(LEDGER.read_text(encoding="utf-8")) + traceability = json.loads(TRACEABILITY.read_text(encoding="utf-8")) + entries = ledger["entries"] + donor_test_symbols = source_symbols(DONOR_TESTS) + + self.assertEqual(ledger["story"], "US01-01") + self.assertEqual(len({entry["id"] for entry in entries}), len(entries)) + self.assertEqual(set(ledger["required_areas"]), {entry["area"] for entry in entries}) + self.assertFalse((REPO / "legacy_cli_archive").exists()) + + allowed = set(ledger["classifications"]) + for entry in entries: + with self.subTest(entry=entry["id"]): + self.assertIn(entry["classification"], allowed) + self.assertTrue(entry["rationale"].strip()) + self.assertTrue(entry["target"].strip()) + self.assertTrue(entry["test_ids"]) + self.assertEqual(entry["migration_status"], "inventoried") + for test_id in entry["test_ids"]: + self.assertIn(test_id, traceability) + class_name, method_name = traceability[test_id].split(".", 1) + self.assertEqual(class_name, "DonorCharacterizationTests") + self.assertIn(method_name, donor_test_symbols) + for source in entry["sources"]: + file_name, separator, symbol = source.partition("::") + source_path = REPO / file_name + self.assertTrue(source_path.is_file(), source) + if separator: + self.assertIn(symbol, source_symbols(source_path), source) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/characterization/test_traceability.json b/tests/characterization/test_traceability.json new file mode 100644 index 0000000..00fe0e6 --- /dev/null +++ b/tests/characterization/test_traceability.json @@ -0,0 +1,15 @@ +{ + "CHAR-001": "DonorCharacterizationTests.test_discovery_outputs", + "CHAR-002": "DonorCharacterizationTests.test_image_preparation_and_hash_outputs", + "CHAR-003": "DonorCharacterizationTests.test_database_status_and_fts_outputs", + "CHAR-004": "DonorCharacterizationTests.test_caption_and_variant_outputs", + "CHAR-005": "DonorCharacterizationTests.test_vision_dry_run_output", + "CHAR-006": "DonorCharacterizationTests.test_nsfw_cache_output", + "CHAR-007": "DonorCharacterizationTests.test_review_html_output", + "CHAR-008": "DonorCharacterizationTests.test_cli_entry_point_help", + "CHAR-009": "DonorCharacterizationTests.test_exif_command_contracts", + "CHAR-010": "DonorCharacterizationTests.test_error_and_configuration_fallbacks", + "CHAR-011": "DonorCharacterizationTests.test_history_log_shape", + "CHAR-012": "DonorCharacterizationTests.test_cooperative_cancellation_contract", + "LEDGER-001": "DonorLedgerLintTests.test_ledger_is_complete_and_resolvable" +} diff --git a/tests/fixtures/characterization/expected_outputs.json b/tests/fixtures/characterization/expected_outputs.json new file mode 100644 index 0000000..67908b5 --- /dev/null +++ b/tests/fixtures/characterization/expected_outputs.json @@ -0,0 +1,15 @@ +{ + "schema_version": 1, + "outputs": { + "analyzer_recursive_discovery": ["album/UPPER.JPG", "album/nested.webp", "root.jpg", "wide.tiff"], + "nsfw_recursive_discovery": ["album/UPPER.JPG", "album/nested.webp", "root.jpg"], + "nsfw_shallow_discovery": ["root.jpg"], + "hash_bytes_sha1": "6a961f2bf4e8cdcd04838175b8cc98d40d729fe4", + "variant_base": "IMG_0001", + "caption": "Three adults walk beside a lake. | Tags: people, lake, summer | Mood: relaxed | Location: Como, Italy | ~2021", + "dry_run_tags": ["dry-run", "test"], + "nsfw_cache": {"a.jpg": 0.125, "b.jpg": 0.9876}, + "ui_total": "1", + "ui_threshold": "0.60" + } +} diff --git a/tests/fixtures/characterization/manifest.json b/tests/fixtures/characterization/manifest.json new file mode 100644 index 0000000..b0f5f76 --- /dev/null +++ b/tests/fixtures/characterization/manifest.json @@ -0,0 +1,13 @@ +{ + "schema_version": 1, + "fixtures": [ + {"id": "DISC-ROOT-JPEG", "relative_path": "root.jpg", "recipe": "rgb:12x8:10,20,30"}, + {"id": "DISC-UPPER-JPEG", "relative_path": "album/UPPER.JPG", "recipe": "rgb:8x12:40,50,60"}, + {"id": "DISC-NESTED-WEBP", "relative_path": "album/nested.webp", "recipe": "rgb:9x9:70,80,90"}, + {"id": "DISC-TIFF", "relative_path": "wide.tiff", "recipe": "rgb:16x4:100,110,120"}, + {"id": "DISC-UNSUPPORTED", "relative_path": "notes.txt", "recipe": "text:not-an-image"}, + {"id": "HASH-BYTES", "relative_path": "hash.bin", "recipe": "bytes:photo-pipeline-donor"}, + {"id": "IMAGE-GRADIENT", "relative_path": "gradient.png", "recipe": "gradient:40x20"}, + {"id": "UI-ESCAPED", "relative_path": "album & one/.jpg", "recipe": "logical-only"} + ] +} diff --git a/work_item/.work-item.yml b/work_item/.work-item.yml index a9bc586..81cbff6 100644 --- a/work_item/.work-item.yml +++ b/work_item/.work-item.yml @@ -10,6 +10,7 @@ workflow: require_ci: false required_tests: - work_item/scripts/python -m unittest discover -s work_item/tests -v + - work_item/scripts/python -m unittest discover -s tests/characterization -v safety: max_file_bytes: 5000000 diff --git a/work_item/src/work_item/core.py b/work_item/src/work_item/core.py index ed8f0ad..4c755e2 100644 --- a/work_item/src/work_item/core.py +++ b/work_item/src/work_item/core.py @@ -317,7 +317,13 @@ class GitRepo: return branch def changed_paths(self) -> list[Path]: - output = self.git("status", "--porcelain=v1", "--untracked-files=all", "-z") + # Porcelain's leading status column is significant (an unstaged modification + # starts with a space). ``git()`` strips surrounding whitespace for ordinary + # scalar commands, so read this machine format directly and preserve it. + output = self.runner.run( + ("git", "status", "--porcelain=v1", "--untracked-files=all", "-z"), + cwd=self.root, + ).stdout if not output: return [] entries = output.split("\0") diff --git a/work_item/tests/test_core.py b/work_item/tests/test_core.py index 5dd2839..aed2476 100644 --- a/work_item/tests/test_core.py +++ b/work_item/tests/test_core.py @@ -354,6 +354,12 @@ class GitSafetyTests(RepositoryFixture): run("git", "mv", "README.md", "RENAMED.md", cwd=self.repo) self.assertEqual(git.changed_paths(), [Path("RENAMED.md")]) + def test_changed_paths_preserves_modified_and_untracked_filenames(self) -> None: + git = GitRepo(self.repo, self.config, Runner()) + (self.repo / "README.md").write_text("changed\n", encoding="utf-8") + (self.repo / "Another file.md").write_text("new\n", encoding="utf-8") + self.assertEqual(set(git.changed_paths()), {Path("README.md"), Path("Another file.md")}) + def test_diff_check_rejects_whitespace_errors(self) -> None: git = GitRepo(self.repo, self.config, Runner()) (self.repo / "README.md").write_text("seed\ntrailing whitespace \n", encoding="utf-8")