diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 57c13cc..f2c4c72 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,33 +1,154 @@ -# Contributing +# Contributing to Video Library Manager (VLM) -## Versioning +Thanks for helping improve VLM. This guide is for humans patching the CLI and library code—not for running the tool on your own media collection (see [README.md](README.md) for that). -This project follows [Semantic Versioning](https://semver.org/): +## What fits here -- **PATCH** — bug fixes, documentation-only changes -- **MINOR** — backward-compatible features (new commands, options, schema fields) -- **MAJOR** — breaking changes to CLI contracts, config keys, or artifact schemas +Good contributions: -Release notes and test baselines are recorded in [`CHANGELOG.md`](CHANGELOG.md). +- Bug fixes and regressions in `src/vlm/` +- CLI UX, validation messages, and safer defaults +- Tests and docs that match current behavior +- Performance or correctness in scan/parse/plan/execute paths + +Out of scope unless discussed first: + +- Breaking changes to artifact JSON schemas or default CLI behavior without a migration note in the PR +- Features that permanently delete files (use quarantine; see safety rules below) +- Committing generated workflow output or personal library data + +## Prerequisites + +- **Python 3.10+** (CI also runs 3.11 and 3.12) +- **[uv](https://github.com/astral-sh/uv)** for installs and `uv run …` +- **Git** +- Optional for manual end-to-end checks: `ffprobe` on `PATH` (metadata extraction degrades without it) ## Development setup ```bash +git clone +cd dl-organizer + +# Editable install + dev tools (pytest, hypothesis, ruff, pytest-cov) uv pip install -e ".[dev]" + +# Optional: Textual UI for `vlm review-plan --tui` only +uv pip install -e ".[tui]" +``` + +Verify the environment: + +```bash +uv run vlm --help +uv run ruff check src tests uv run pytest -q ``` -Optional Textual UI: `uv pip install -e ".[tui]"`. +Expect **517** tests to pass on the current baseline (`uv run pytest -q` → `517 passed`). If you change behavior, update or add tests—do not weaken assertions to green the suite. -## Pull requests +During iteration, run a narrow file instead of the full suite: -1. Write or extend tests for behavior changes. -2. Run `uv run pytest -q` locally (CI runs the same). -3. Keep commits focused; use imperative subject lines. -4. Do not commit personal library paths, workflow artifacts (`artifacts/`, root CSV/JSON), or API keys from `~/.vlm/config.yaml`. +```bash +uv run pytest tests/test_planner.py -q +uv run pytest tests/test_cli_scan.py::test_scan_writes_inventory -q +``` + +## Where to change things + +| Area | Location | Tests often in | +|------|----------|----------------| +| CLI registration | `src/vlm/cli.py` | `tests/test_cli_*.py` | +| Command implementation | `src/vlm/commands/` | matching `tests/test_cli_*.py` | +| Scan / inventory | `scanner.py` | `test_scanner.py`, `test_cli_scan.py` | +| Parse / identities | `parser.py`, `io.py` | `test_parser.py`, `test_io.py` | +| Enrich / TMDB | `enrichment.py`, `providers/` | `test_enrichment.py` | +| Analysis / duplicates | `analysis.py`, `duplicate_resolve.py` | `test_analysis*.py`, `test_duplicate_*.py` | +| Plan / review | `planner.py`, `plan_review.py`, `review_tui.py` | `test_planner.py`, `test_plan_review.py` | +| Execute / rollback | `executor.py`, `transaction.py` | `test_executor.py`, `test_cli_rollback.py` | +| Config | `config.py` | `test_config.py` | +| Shared models | `models.py` | domain tests that construct fixtures | + +Agent-oriented repo notes live in [AGENTS.md](AGENTS.md). Architecture and workflow stages are summarized in [CLAUDE.md](CLAUDE.md). + +## Safety rules (do not regress) + +VLM is built around reversible, reviewed file operations: + +1. **No permanent deletes** in normal workflows—duplicates go to quarantine. +2. **`vlm execute` is dry-run by default**; real moves need `--confirm` and rollback logging. +3. **Paths must stay under `library_root`** for move/rename execution. +4. **Default pipeline is read-first**: scan → parse → (enrich) → analyze → plan → review → execute. Do not hide destructive steps behind implicit flags. + +If your change touches `executor.py`, `planner.py`, or `quarantine.py`, add or extend tests for failure paths and library-root checks. + +## Artifacts and schemas + +Generated files belong under `artifacts/` (or `workspace_dir` in config)—**never commit** them, root-level `inventory.csv` / `identities.json` / `plan.json`, or copies of `~/.vlm/config.yaml`. + +| Artifact | Notes for contributors | +|----------|-------------------------| +| `identities.json` | v1 without `--inventory`; v2 embeds video metadata for quality-aware duplicates | +| `analysis.json` | Output of analyze; input to `vlm plan --analysis` | +| `plan.json` | Validated typed `ExecutionPlan`; review CSV syncs via `apply-review` | + +Loading/saving goes through `io.py` with schema validation. If you add or rename JSON fields, update loaders, tests, and call out **backward compatibility** in the PR (v1 identities must still load). ## Code style -- Python 3.10+, 4-space indentation, type hints on public APIs. -- Use absolute imports in package code: `from vlm.module import ...` (not relative imports in new code). -- `uv run ruff check src tests` when Ruff is installed via `[dev]`. +- Python 3.10+, 4 spaces, PEP 8 naming (`snake_case` / `PascalCase`). +- Type hints on public functions and non-trivial internal APIs. +- Absolute imports in package code: `from vlm.module import …` (no new relative imports in `src/vlm/`). +- Ruff: `E`, `F`, `I` (see `pyproject.toml`). CI runs `uv run ruff check src tests`. +- Prefer small, focused modules; put CLI wiring in `commands/`, not bloated `cli.py`. + +## Testing expectations + +| Change type | Minimum bar | +|-------------|-------------| +| Pure helper / module logic | Unit test in `tests/test_.py` | +| CLI flag or error message | `CliRunner` test in `tests/test_cli_*.py` | +| Invariants across inputs | Consider `hypothesis` (existing pattern in `test_*_properties.py`) | +| Config key or default | `test_config.py` + note in PR if users must edit `config.yaml` | + +Property tests use Hypothesis with `max_examples = 100` in `pyproject.toml`. + +Coverage is optional locally: `uv run pytest --cov=vlm --cov-report=term-missing` (requires `[dev]`). + +## Pull request checklist + +Before opening a PR: + +- [ ] Branch is up to date with `main` (or target branch) +- [ ] `uv run ruff check src tests` passes +- [ ] `uv run pytest -q` passes (full suite) +- [ ] New/changed behavior has tests; CLI changes have at least one failure-path test if applicable +- [ ] No secrets, personal paths, or generated artifacts in the diff +- [ ] PR description: **what**, **why**, and **how you tested** (paste `pytest` summary or targeted command) +- [ ] Breaking CLI, config, or schema changes: migration steps and suggested `CHANGELOG.md` entry (maintainers may edit release notes) + +Commit messages: short imperative subject (`fix planner conflict when destination exists`). Keep commits focused; split refactors from behavior changes when possible. + +## CI + +GitHub Actions (`.github/workflows/test.yml`) on push/PR to `main` / `master`: + +1. `uv pip install -e ".[dev]"` +2. `uv run ruff check src tests` +3. `uv run pytest -q` on Python **3.10, 3.11, 3.12** + +Match CI locally before requesting review. + +## Versioning and releases + +[Semantic Versioning](https://semver.org/): + +- **PATCH** — bug fixes, docs-only +- **MINOR** — backward-compatible features (commands, options, new optional JSON fields) +- **MAJOR** — breaking CLI contracts, config keys, or artifact schemas + +Release history: [CHANGELOG.md](CHANGELOG.md). Package version: `pyproject.toml` → `[project].version`. + +## Questions + +Open an issue for bugs or design questions. For large features, describe the safety impact (especially execute/quarantine) before a big PR so review stays tractable.