Files
dl-organizer/FIX_PLAN.md
T
6f0df5a774 release: v0.2.0 repository hygiene, CI, and docs sync
Stop tracking personal workflow artifacts at repo root, add CI and MIT
license, align README and agent skills with artifacts/ defaults, and
enable Ruff in dev/CI so releases are verifiable without local-only runs.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-06-01 15:32:03 +08:00

447 lines
15 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# VLM Fix Plan
**Derived from:** [`ISSUES_REPORT.md`](ISSUES_REPORT.md)
**Created:** 2026-06-01
**Verified baseline:** `uv run pytest -q` → 517 passed; 36 tracked workflow artifacts (~34 MB) at repo root
This plan turns the issues report into an ordered, shippable sequence of work. Each phase ends with verifiable acceptance criteria before starting the next.
---
## Goals
1. **Stop leaking personal library data** via Git (P0).
2. **Make quality enforceable** with CI and clear OSS metadata (P1).
3. **Align all human and agent docs** with `artifacts/` defaults and the full review cycle (P1P2).
4. **Defer structural refactors** until they ride on feature work (P2P3).
## Non-goals (this plan)
- Anime parsing (ISSUE-012), quarantine category expansion (ISSUE-013), configurable parser patterns (ISSUE-014).
- Splitting `planner.py` / `plan_review.py` (ISSUE-010) unless already editing those areas.
- Full `cli.py` slim-down (ISSUE-006) — optional stretch after Phase 3.
- Git history rewrite (`git filter-repo`) unless the remote was public and retention policy requires it.
## Principles
- **One concern per PR** where possible (hygiene, CI, license, docs).
- **No behavior changes** in Phases 13 unless required for tests or docs accuracy.
- **Synthetic fixtures only** under `tests/` — never commit real library paths again.
- **CHANGELOG** is the canonical record for test counts and release notes.
---
## Phase overview
| Phase | Focus | Issues | Est. effort | Depends on |
|-------|--------|--------|-------------|------------|
| **0** | Repository hygiene | ISSUE-001 | 12 h | — |
| **1** | CI + release metadata | ISSUE-002, ISSUE-003, ISSUE-005 | 24 h | Phase 0 |
| **2** | Documentation & skills sync | ISSUE-004, ISSUE-009 | 35 h | Phase 1 (optional: parallel after 0) |
| **3** | Operator & style guidance | ISSUE-007, ISSUE-008 | 23 h | Phase 1 |
| **4** | DX tooling (stretch) | ISSUE-011 | 23 h | Phase 1 |
| **5** | Maintainability (opportunistic) | ISSUE-006, ISSUE-010 | As needed | Anytime |
```mermaid
flowchart LR
P0[Phase 0 Hygiene] --> P1[Phase 1 CI and License]
P0 --> P2[Phase 2 Docs]
P1 --> P2
P1 --> P3[Phase 3 Ops docs]
P1 --> P4[Phase 4 Ruff optional]
P5[Phase 5 CLI split] -.->|when touching CLI| P1
```
---
## Phase 0 — Repository hygiene (P0)
**Issue:** ISSUE-001
**Risk if skipped:** Privacy exposure, bloated clones, agents/users picking stale root `plan.json` / `identities.json`.
### Tasks
1. **Expand `.gitignore`** (repository root only — use leading `/`):
```gitignore
# Legacy root-level workflow outputs (defaults are under artifacts/)
/inventory*.csv
/identities*.json
/analysis*.json
/plan*.json
/plan*_manual_review.csv
```
Keep existing `artifacts/` and `runs/` entries.
2. **Stop tracking workflow artifacts** (keep files on disk for local use if desired):
```bash
git ls-files '*.json' '*.csv' \
| grep -v -E 'mcp\.json|markdownlint' \
| xargs git rm --cached
```
Do **not** remove `.kiro/settings/mcp.json` or `.markdownlint.json`.
3. **Add minimal synthetic fixtures** (only if a future test needs committed samples):
- `tests/fixtures/inventory_mini.csv` — 23 rows, fake paths (`/tmp/vlm-fixture/...`).
- `tests/fixtures/identities_mini.json` — v1 or v2 schema, 1 movie + 1 series.
- `tests/fixtures/plan_mini.json` — 12 `FileOperation` entries, no real paths.
Current tests use `tmp_path` and `artifacts/` defaults; **fixtures are optional** unless you add integration tests that read committed files.
4. **README — Artifacts section** (short):
- Default outputs: `artifacts/inventory.csv`, `identities.json`, `analysis.json`, `plan.json`, `plan_manual_review.csv`.
- Do not commit generated CSV/JSON at repo root or under `artifacts/`.
- `workspace_dir` / `--output` overrides still write locally; same rule applies.
5. **CHANGELOG** entry: “Remove tracked personal workflow artifacts from Git; expand root `.gitignore`.”
### Acceptance criteria
- [ ] `git ls-files '*.json' '*.csv'` lists only `.kiro/settings/mcp.json` and `.markdownlint.json` (or project-intentional config JSON).
- [ ] `du` of tracked workflow artifacts → ~0 (was ~34 MB).
- [ ] `uv run pytest -q` still passes (517).
- [ ] Fresh clone + `vlm scan` writes under `artifacts/`, not repo root.
### PR suggestion
- **Title:** `[P0] Stop tracking root workflow artifacts and expand .gitignore`
- **Label:** `priority:p0`, `type:security`
### History rewrite (optional, separate decision)
- Use `git filter-repo` **only if** the remote was public and policy requires purging paths from history.
- Coordinate with anyone who cloned the repo; force-push is disruptive.
- Default for private solo repos: **cached remove + ignore is enough** going forward.
---
## Phase 1 — CI, license, versioning (P1)
**Issues:** ISSUE-002, ISSUE-003, ISSUE-005
### 1A — Continuous integration (ISSUE-002)
**File:** `.github/workflows/test.yml`
```yaml
name: Tests
on:
push:
branches: [main, master]
pull_request:
jobs:
test:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.10", "3.11", "3.12"]
steps:
- uses: actions/checkout@v4
- uses: astral-sh/setup-uv@v4
- run: uv pip install -e ".[dev]"
- run: uv run pytest -q
```
**Tasks**
1. Add workflow above (adjust default branch name if not `main`).
2. Pin `uv` / use `setup-uv` with cache if desired (optional follow-up).
3. README: optional CI badge after first green run.
**Acceptance criteria**
- [ ] PR and push to default branch run full suite.
- [ ] Failure blocks merge (branch protection — configure on GitHub).
**PR:** `[P1] Add GitHub Actions workflow for pytest`
---
### 1B — License and project metadata (ISSUE-003)
**Tasks**
1. Choose license (recommend **MIT** for a personal CLI tool unless you need patent grant → Apache-2.0).
2. Add `LICENSE` with copyright year and holder name.
3. Update `pyproject.toml`:
```toml
license = { text = "MIT" }
# or license = "MIT" depending on PEP 621 style used
authors = [
{ name = "Your Name", email = "you@example.com" },
]
[project.urls]
Homepage = "https://github.com/<owner>/dl-organizer"
Repository = "https://github.com/<owner>/dl-organizer"
Issues = "https://github.com/<owner>/dl-organizer/issues"
```
4. Replace README placeholders:
- License section → link to `LICENSE`.
- Remove `[Add your license here]`.
- Fix `yourusername/video-library-manager` URLs to real repo path (`dl-organizer` or rename consistently).
**Acceptance criteria**
- [ ] `LICENSE` present; README and `pyproject.toml` agree.
- [ ] No placeholder GitHub URLs in README.
**PR:** `[P1] Add LICENSE and fix packaging metadata`
---
### 1C — Versioning policy (ISSUE-005)
**Tasks**
1. Add `CONTRIBUTING.md` (or README subsection) — **semver** for this project:
- **PATCH:** fixes, docs-only.
- **MINOR:** new commands, backward-compatible behavior.
- **MAJOR:** breaking config/schema/CLI contracts.
2. Bump `version` in `pyproject.toml` to **`0.2.0`** when Phase 0 + 1 land (signals “repo is safe to share”).
3. CHANGELOG section `## 0.2.0` summarizing hygiene + CI + license.
**Acceptance criteria**
- [ ] Version documented; `0.2.0` tagged or noted in CHANGELOG when releasing.
**PR:** Can merge with 1B or as a one-line bump in the hygiene PR.
---
## Phase 2 — Documentation and skills sync (P1/P2)
**Issues:** ISSUE-004, ISSUE-009
### Single source of truth
| Topic | Canonical location |
|-------|---------------------|
| Test count / release notes | `CHANGELOG.md` (latest entry) |
| Agent + contributor commands | `CLAUDE.md`, `AGENTS.md` (keep in sync) |
| End-user workflow | `README.md` (link to skills for agents) |
| Cursor/Codex skill | `skills/vlm-library-workflow/` |
Add to README top or Development section:
> **Documentation status:** See latest entry in [`CHANGELOG.md`](CHANGELOG.md) (currently 517 tests, 2026-05-21).
### README fixes (ISSUE-009)
1. **Development / testing** — standardize on:
```bash
uv run pytest -q
uv run pytest tests/test_scanner.py -v
```
2. **Coverage** — either:
- Add `pytest-cov` to `[project.optional-dependencies]` `dev`, and document `uv run pytest --cov=vlm`, **or**
- Remove `--cov=vlm` from README if coverage is not supported.
3. **Project structure tree** — add missing modules:
- `cli_helpers.py`, `plan_review.py`, `plan_render.py`, `plan_structure_preview.py`
- `review_display.py`, `review_tui.py`, `transaction.py`
- `commands/review_plan.py`, `report.py`, `quarantine_cmd.py`, `state_cmd.py`, `config_cmd.py`
4. **Remove broken skill reference** — delete or replace `activate_skill vlm-expert` with `vlm-library-workflow` (skill lives under `skills/vlm-library-workflow/`).
5. **Workflow section** — ensure human-in-the-loop steps are visible:
```text
scan → parse → enrich → analyze → plan → review-plan → apply-review → execute (dry-run) → execute --confirm
```
### Skills update (`skills/vlm-library-workflow/`)
1. Set **Documentation Status** to **2026-06-01** (or current release date).
2. Replace all root paths with `artifacts/` defaults matching `cli.py` help strings.
3. Update **Workflow Order** to include:
- `vlm review-plan` (and `--tui` as optional)
- `vlm apply-review` before `execute --confirm`
4. Sync `references/command-recipes.md`, `workflow.md`, `cli-reference.md` in the same pass.
5. Mention: never commit `artifacts/` or root CSV/JSON outputs.
### Agent doc alignment (ISSUE-004)
- `CLAUDE.md` / `AGENTS.md`: confirm all example commands use `uv run` and `artifacts/` paths.
- Remove duplicate long command lists from README if desired — link to `CLAUDE.md` instead (optional trim).
### Acceptance criteria
- [ ] Grep for root `inventory.csv` in `skills/` and active docs → 0 (except “do not use at root” warnings).
- [ ] Skill workflow includes `review-plan` and `apply-review`.
- [ ] README tree matches `src/vlm/*.py` layout.
- [ ] No reference to non-existent `vlm-expert` skill.
**PR:** `[P1] Sync README and vlm-library-workflow to artifacts/ baseline`
---
## Phase 3 — Operations and import consistency (P2)
**Issues:** ISSUE-007, ISSUE-008
### 3A — Import style (ISSUE-007)
**Decision:** Absolute imports `from vlm.*` for package code (match `planner.py`, `commands/`).
**Tasks**
1. Convert `executor.py` relative imports to absolute.
2. Add one line to `AGENTS.md`: “Use `from vlm.module import ...` in `src/vlm/`; no new relative imports.”
3. Run full test suite.
**Acceptance criteria**
- [ ] `executor.py` uses `vlm.*` only.
- [ ] AGENTS.md documents the rule.
**PR:** `[P2] Standardize executor imports to vlm.*`
---
### 3B — Enrichment operations (ISSUE-008)
**Tasks**
1. README subsection **Enrichment / TMDB**:
- API key in `~/.vlm/config.yaml` — do not commit config or paste keys in issues.
- `enrichment.max_concurrency` default (6): used by scanner ffprobe pool and enrichment workers.
- TMDB rate limits: link to TMDB API terms; recommend starting with concurrency 24 on large libraries.
- No built-in exponential backoff today — expect transient failures on bulk runs.
2. Optional follow-up (separate issue): shared retry helper in `providers/`.
**Acceptance criteria**
- [ ] Operator can read README before `vlm enrich` on 1000+ titles without guessing concurrency impact.
**PR:** `[P2] Document TMDB concurrency and secrets handling`
---
## Phase 4 — Developer experience (P3, stretch)
**Issue:** ISSUE-011
**Depends on:** Phase 1 CI
### Tasks
1. Add `[tool.ruff]` to `pyproject.toml` — target `src/vlm`, `tests`; select sensible rules (E, F, I).
2. Add Ruff to `dev` optional deps; CI step: `uv run ruff check src tests`.
3. Fix or noqa only clear violations in one pass (avoid huge unrelated diff).
4. Optional: `mypy` on `src/vlm` with gradual typing — defer if noisy.
**Acceptance criteria**
- [ ] CI fails on new Ruff violations in `src/` and `tests/`.
**PR:** `[P3] Add Ruff lint to dev deps and CI`
---
## Phase 5 — Maintainability (opportunistic)
**Issues:** ISSUE-006, ISSUE-010, ISSUE-016
| Trigger | Action |
|---------|--------|
| New CLI command | Register in `commands/`; add thin `@click` wrapper in `cli.py` only |
| Touch duplicate logic | Extract `planner_duplicates.py` from `planner.py` |
| Touch review CSV | Extract `plan_review_csv.py` from `plan_review.py` |
| Touch executor errors | Ensure CLI prints every `OperationResult.error_message` (ISSUE-016) |
**Target:** `cli.py` ~200 lines registration-only — not a blocking milestone.
---
## Known limitations (document only)
Track as GitHub issues only if you plan work; otherwise leave in README **Limitations**:
| ID | Item |
|----|------|
| ISSUE-012 | Anime scanned, not parsed |
| ISSUE-013 | Quarantine: movie/series only |
| ISSUE-014 | Parser patterns hardcoded |
| ISSUE-015 | Rollback best-effort — reinforce in `execute` output |
---
## Strengths — regression checklist
After each phase, confirm these still hold (from ISSUES_REPORT STR-001005):
- [ ] `vlm execute` defaults to dry-run; `--confirm` required for writes.
- [ ] Quarantine used instead of delete; `library_root` boundary checks on execute.
- [ ] `review-plan` → CSV → `apply-review` path documented and tested.
- [ ] Duplicate strategies fail explicitly (`DuplicateResolutionError` / manual review).
- [ ] `ExecutionPlan` validated on load/save in `io.py`.
- [ ] `tests/test_path_safety.py` and CLI integration tests still pass.
---
## Suggested PR sequence
| # | Branch / PR | Phase |
|---|-------------|-------|
| 1 | `fix/p0-untrack-artifacts` | 0 |
| 2 | `fix/p1-ci` | 1A |
| 3 | `fix/p1-license-metadata` | 1B + 1C |
| 4 | `fix/p1-docs-skills` | 2 |
| 5 | `fix/p2-imports` | 3A |
| 6 | `fix/p2-enrichment-docs` | 3B |
| 7 | `fix/p3-ruff` | 4 (optional) |
Merge **1 before any public push**. Phases 2 and 3 can run in parallel after CI exists.
---
## Final verification (release `0.2.0`)
Run once before tagging:
```bash
git ls-files '*.json' '*.csv' # expect only config JSON
uv run pytest -q # 517 passed
uv run vlm --help
# Optional: uv run ruff check src tests
```
Update `CHANGELOG.md` with `## 0.2.0` bullet list: artifact hygiene, CI, license, docs/skills sync.
---
## GitHub issue mapping
| Issue ID | Suggested title | Phase |
|----------|-----------------|-------|
| ISSUE-001 | `[P0] Remove committed library artifacts from Git` | 0 |
| ISSUE-002 | `[P1] Add CI workflow for pytest` | 1A |
| ISSUE-003 | `[P1] Add LICENSE and fix pyproject/README metadata` | 1B |
| ISSUE-004 | `[P1] Sync documentation and agent skills to artifacts/ baseline` | 2 |
| ISSUE-005 | `[P1] Document versioning and bump to 0.2.0` | 1C |
| ISSUE-006 | `[P2] Slim cli.py to registration-only pattern` | 5 |
| ISSUE-007 | `[P2] Standardize import style to vlm.*` | 3A |
| ISSUE-008 | `[P2] Document TMDB rate limits and concurrency` | 3B |
| ISSUE-009 | (merge into ISSUE-004 PR) | 2 |
| ISSUE-010 | `[P2] Split planner/plan_review when touched` | 5 |
| ISSUE-011 | `[P3] Add Ruff (and optional mypy) in CI` | 4 |
---
## References
- [`ISSUES_REPORT.md`](ISSUES_REPORT.md) — full issue descriptions and evidence
- [`CHANGELOG.md`](CHANGELOG.md) — canonical test count and release history
- [`CLAUDE.md`](CLAUDE.md) / [`AGENTS.md`](AGENTS.md) — contributor commands