chore: trim dead code, modularize CLI, and archive stale docs
Extract review-plan, report, quarantine, state, and config handlers into commands/ with shared cli_helpers; remove unused exceptions and duplicate plan summary wrappers. Archive superseded review markdown, sync docs to 517-test baseline, and fix empty series titles when only a quality tag remains. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
windyboy
co-authored by
Claude Sonnet 4.5
Cursor
Claude Sonnet 4.5
Cursor
parent
5f0b531269
commit
79797644e1
@@ -0,0 +1,127 @@
|
||||
# Refactor and Refinement Execution Plan
|
||||
|
||||
## Objective
|
||||
|
||||
Refactor the current `vlm` codebase to reduce structural technical debt while preserving the existing safety-first pipeline behavior. The plan focuses on improving CLI maintainability, strengthening stage boundaries, clarifying state semantics, enabling provider and strategy extensibility, and increasing testability and observability without changing the user-visible safety guarantees.
|
||||
|
||||
## Context and Scope
|
||||
|
||||
This plan is based on the verified review findings in the code analysis report, especially the structural issues around CLI size and repetitive error handling, weak dict/JSON contracts between stages, analyze-phase ordering coupling, duplicated sample-path rules, filesystem-dependent plan generation, configuration semantic overlap, review-state ambiguity, provider hard-coding, and the lack of unified state and strategy abstractions. Key source areas include `src/vlm/cli.py:158-603`, `src/vlm/commands/parse.py:66-155`, `src/vlm/io.py:85-249`, `src/vlm/commands/analyze.py:41-55`, `src/vlm/planner.py:34-40`, `src/vlm/planner.py:43-76`, `src/vlm/planner.py:354-358`, `src/vlm/enrichment.py:203-225`, `src/vlm/state.py:22-205`, `src/vlm/models.py:46-53`, and `src/vlm/models.py:79-85`.
|
||||
|
||||
## Assumptions
|
||||
|
||||
- Preserve the current pipeline order and safety behavior unless a change explicitly improves safety or determinism.
|
||||
- Avoid introducing breaking changes to the CLI surface in the first refactor wave.
|
||||
- Prefer incremental, reviewable changes that can ship independently.
|
||||
- Keep the artifact-driven workflow as the default, while making room for future hybrid or streaming modes.
|
||||
- Treat documentation, tests, and observability updates as first-class deliverables rather than afterthoughts.
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
### Phase 0: Baseline, guardrails, and dependency mapping
|
||||
|
||||
- [ ] Capture the current behavior baseline for the full workflow, including `scan`, `parse`, `enrich`, `analyze`, `plan`, and `execute`, so every later refactor can be compared against the existing safety model. This is necessary because the codebase relies on many cross-stage assumptions and the review identified several fragile boundaries.
|
||||
- [ ] Map the exact data flow between artifact files and in-memory models for `inventory`, `identities`, `analysis`, and `plan` outputs. This reduces the chance of accidental schema drift while refactoring the stage contracts.
|
||||
- [ ] Identify the minimum set of high-value integration paths to protect first: CLI startup, parse/enrich transition, analyze pairing, plan generation, execute rollback, and quarantine handling. These paths correspond to the highest-risk areas in the review.
|
||||
|
||||
### Phase 1: CLI decomposition and unified error handling
|
||||
|
||||
- [ ] Extract a shared command execution/error-wrapping layer from `src/vlm/cli.py:158-603` so repeated `try/except + echo + logger + exit` logic is centralized. This is needed to stop the CLI file from continuing to grow and to keep error behavior consistent.
|
||||
- [ ] Split non-core commands and support workflows into smaller command modules and keep the CLI module focused on registration and dispatch. This reduces the blast radius of future command additions and improves discoverability.
|
||||
- [ ] Standardize CLI-level error presentation so file, JSON, validation, and OS errors all follow one predictable response shape. This improves user experience and avoids duplicated branching.
|
||||
- [x] Update the CLI workflow description to match the actual supported flow, including `enrich`, so the user-facing guidance reflects the true pipeline. This is a low-cost refinement that removes user confusion.
|
||||
|
||||
### Phase 2: Stronger stage contracts and typed intermediate models
|
||||
|
||||
- [ ] Replace the most fragile dict-based stage boundaries with typed records or `TypedDict` models, starting with parse output and the io conversion layer in `src/vlm/commands/parse.py:66-155` and `src/vlm/io.py:85-249`. This directly addresses schema drift and makes refactors safer.
|
||||
- [x] Define explicit schemas for `identities.json`, `analysis.json`, and `plan.json`, and validate them on load/save. This adds a durable guardrail against silent data-shape changes.
|
||||
- [ ] Align enrichment output mutation with typed contracts so fields like `display_title`, `needs_review`, and review metadata have a single authoritative shape. This prevents inconsistent stage assumptions.
|
||||
- [x] Refactor `identities_to_analysis_input()` so it returns explicit identity-file pairs instead of relying on positional slicing and zipping in `src/vlm/commands/analyze.py:41-55`. This removes the fragile ordering dependency identified in the review.
|
||||
|
||||
### Phase 3: Shared rule extraction and deterministic planning
|
||||
|
||||
- [x] Extract the duplicated sample-path rule from `src/vlm/planner.py:34-40`, `src/vlm/duplicate_resolve.py:11-16`, and `src/vlm/plan_review.py:12-16` into a single shared helper. This reduces duplication and guarantees consistent classification behavior.
|
||||
- [x] Separate logical plan generation from live environment validation so plan output becomes reproducible and execute-time validation becomes an explicit pass. This addresses the filesystem-state coupling in `src/vlm/planner.py:43-76` and `src/vlm/planner.py:354-358`.
|
||||
- [x] Tag any environment-derived metadata in plan artifacts as snapshots rather than intrinsic plan facts. This makes the distinction between logical intent and runtime validation clear.
|
||||
- [ ] Review plan-related metadata and operation structures so they can support deterministic comparisons across runs. This is important for review tooling and regression analysis.
|
||||
|
||||
### Phase 4: Unified state model and review semantics
|
||||
|
||||
- [ ] Introduce a centralized state model that clearly separates processing, review, execution, and quarantine semantics. This directly addresses the spread of state concepts across `src/vlm/state.py:22-205`, `src/vlm/models.py:46-53`, and `src/vlm/models.py:79-85`.
|
||||
- [ ] Make `needs_review` a derived or secondary field rather than the main source of truth, and ensure `review_status` and `review_reason` are the primary review semantics. This removes the current overlap and reduces future workflow ambiguity.
|
||||
- [ ] Align execution and rollback state transitions with the centralized model so success, failure, rollback, and restore paths are all represented consistently.
|
||||
- [ ] Review persisted state files and transition logic for atomicity and consistency after the model changes are introduced.
|
||||
|
||||
### Phase 5: Provider extensibility and strategy abstractions
|
||||
|
||||
- [ ] Replace hard-coded provider assembly in `src/vlm/enrichment.py:203-225` with a registry or plugin-style registration mechanism. This enables new metadata sources without requiring direct edits to the core enrichment orchestration.
|
||||
- [ ] Introduce a strategy layer for naming, conflict handling, and keep/delete decisions so `Config` is no longer the only mechanism for behavior variation. This addresses the current static-config limitation in `src/vlm/config.py:13-148`.
|
||||
- [ ] Refactor media-type and operation-type dispatch toward registered handlers rather than expanding `if/elif` chains in the parser and executor. This makes future media types and operation kinds easier to add.
|
||||
- [ ] Keep the default built-in behavior intact while allowing new strategies to be added incrementally. This limits regression risk while improving extensibility.
|
||||
|
||||
### Phase 6: Error taxonomy and observability
|
||||
|
||||
- [ ] Define a small domain exception hierarchy for enrichment, planning, and execution safety failures, and update command-layer handling to use those typed errors. This makes failure handling more precise than broad exception catching.
|
||||
- [ ] Add structured logging or event fields for high-value workflow events such as cache hits, quarantine operations, rollback actions, and plan conflicts. This improves supportability and analysis quality.
|
||||
- [ ] Add stage-level timing and outcome metrics so long-running operations can be measured consistently. This is especially useful once the pipeline grows beyond small libraries.
|
||||
|
||||
### Phase 7: Testing expansion and validation coverage
|
||||
|
||||
- [ ] Add integration tests for cross-stage transitions, especially parse→enrich, analyze→plan, and plan→execute. These paths need stronger guarantees than isolated unit tests.
|
||||
- [ ] Add end-to-end tests that run a complete safe workflow against a controlled fixture library, including rollback and quarantine behavior. This validates the pipeline as a whole rather than one module at a time.
|
||||
- [ ] Add performance-oriented checks or benchmark fixtures for larger datasets so future changes can be evaluated against scaling regressions.
|
||||
- [ ] Extend property-based coverage where the new typed contracts or state transitions create meaningful invariants.
|
||||
|
||||
### Phase 8: UX refinement and future-mode readiness
|
||||
|
||||
- [ ] Refresh help text and workflow guidance after the CLI and pipeline changes are stable, so the documented flow stays aligned with actual behavior.
|
||||
- [ ] Evaluate whether a hybrid execution mode or a guided interactive mode should be introduced once the state model and contracts are stable. This is a future-facing refinement to improve usability without sacrificing safety.
|
||||
- [ ] Keep the artifact-first mode as the default until the alternative execution modes have matching safety guarantees and test coverage.
|
||||
|
||||
## Verification Criteria
|
||||
|
||||
- [ ] CLI startup and command registration still work, and repeated command-level error handling is no longer duplicated across the main CLI file.
|
||||
- [x] Stage artifact schemas are explicitly validated, and malformed inputs fail early with clear errors.
|
||||
- [x] Analyze no longer depends on positional assumptions between identities and video files.
|
||||
- [x] Sample-path classification produces one consistent result across planner, duplicate resolution, and review flows.
|
||||
- [x] Plan generation is reproducible for the same logical inputs, with live filesystem checks clearly separated as validation snapshots.
|
||||
- [ ] Review state semantics are no longer ambiguous, and `review_status` is the primary review source of truth.
|
||||
- [ ] Provider registration can be extended without editing the core orchestration logic.
|
||||
- [ ] Integration and E2E coverage exists for the main safe workflow and rollback/quarantine scenarios.
|
||||
- [ ] Structured logs or metrics expose workflow health and failure patterns.
|
||||
- [ ] The default user-visible pipeline still preserves the safety-first execution model.
|
||||
|
||||
## Potential Risks and Mitigations
|
||||
|
||||
1. **Risk: Refactor scope expands faster than the code can be stabilized**
|
||||
Mitigation: Keep the work split into independently shippable phases and require the baseline behavior to remain intact after each phase.
|
||||
|
||||
2. **Risk: Typed contracts introduce temporary friction in serialization/deserialization code**
|
||||
Mitigation: Introduce schema validation and typed records incrementally, starting with the highest-risk artifacts.
|
||||
|
||||
3. **Risk: State model changes cascade through planner, executor, and review flows**
|
||||
Mitigation: Centralize the new model first, then migrate consumers one by one while keeping compatibility adapters where needed.
|
||||
|
||||
4. **Risk: Provider and strategy abstraction can become too generic too early**
|
||||
Mitigation: Start with the current built-in cases as default registrations before allowing external extensibility.
|
||||
|
||||
5. **Risk: New tests may be slow or hard to maintain if they overuse large fixtures**
|
||||
Mitigation: Prefer focused fixtures for unit/integration coverage and reserve large datasets for targeted performance checks.
|
||||
|
||||
## Alternative Approaches
|
||||
|
||||
1. **Incremental refactor first**: Keep the current artifact-first architecture and only extract the most painful seams now. Trade-off: lowest regression risk, but slower progress on deeper extensibility issues.
|
||||
2. **Boundary-first refactor**: Prioritize typed contracts, state model, and deterministic planning before provider and strategy work. Trade-off: better long-term clarity, but requires more cross-module updates early.
|
||||
3. **Platform-style refactor**: Introduce registries, strategies, and structured observability as a broader platform layer. Trade-off: highest flexibility, but the largest immediate complexity increase.
|
||||
|
||||
## Status Tracking
|
||||
|
||||
- **Not Started**: Phase 0 baseline capture and dependency mapping
|
||||
- **Partially Completed**: Phase 1 CLI decomposition and unified error handling
|
||||
- **Partially Completed**: Phase 2 stronger stage contracts and typed intermediate models
|
||||
- **Partially Completed**: Phase 3 shared rule extraction and deterministic planning
|
||||
- **Not Started**: Phase 4 unified state model and review semantics
|
||||
- **Not Started**: Phase 5 provider extensibility and strategy abstractions
|
||||
- **Not Started**: Phase 6 error taxonomy and observability
|
||||
- **Not Started**: Phase 7 testing expansion and validation coverage
|
||||
- **Not Started**: Phase 8 UX refinement and future-mode readiness
|
||||
@@ -0,0 +1,102 @@
|
||||
# Review-Plan Output Refactor Plan
|
||||
|
||||
## Objective
|
||||
|
||||
在不修改 `plan.json` schema、不中断现有 CSV 手工审核流程的前提下，重构 `vlm review-plan` 的输出体验，让用户在终端中直接看到计划的核心内容和可审核的操作预览，减少必须打开 `plan.json` 才能继续操作的成本。
|
||||
|
||||
## Validated Baseline
|
||||
|
||||
- `review-plan` 当前只输出计划加载提示、风险统计、CSV 保存路径和前 5 条样例，没有输出完整计划预览，见 `src/vlm/cli.py:536-563`。
|
||||
- `ExecutionPlan` 已包含 `operations`、`summary`、`summary_by_reason`、`human_summary`、`metadata`，足以支撑更强的终端展示，见 `src/vlm/models.py:133-153`。
|
||||
- 计划保存时会把上述字段全部写入 JSON，因此无需改动 plan schema，见 `src/vlm/planner.py:591-625`。
|
||||
- CLI 中已经存在 fallback 计划摘要逻辑，可复用于 `review-plan`，见 `src/vlm/cli.py:976-987`。
|
||||
- `execute` 已经采用“计划概要 + 样例操作”的输出方式，可作为统一风格参考，见 `src/vlm/commands/execute.py:84-103` 和 `src/vlm/commands/execute.py:137-146`。
|
||||
- 当前测试只覆盖 summary 和 CSV 导出，未覆盖完整计划预览输出，见 `tests/test_cli_review_plan.py:31-88`。
|
||||
|
||||
## Recommended Approach
|
||||
|
||||
采用推荐方案：**抽离通用渲染层，并为 `review-plan` 提供受控预览输出**。
|
||||
|
||||
原因：
|
||||
|
||||
1. 只加 `human_summary` 无法解决“看不到计划内容”的核心问题。
|
||||
2. 直接打印全部 operations 会在大计划场景下严重刷屏。
|
||||
3. 抽离通用渲染层可以同时提升用户体验、结构清晰度和后续复用性。
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
|
||||
- 优化 `review-plan` 的终端输出结构。
|
||||
- 复用已有 `human_summary` / fallback summary。
|
||||
- 增加受控的操作预览输出。
|
||||
- 为计划展示提取可复用 helper。
|
||||
- 补充 CLI 测试，覆盖新增展示行为。
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- 修改 `ExecutionPlan` 数据模型。
|
||||
- 修改 `plan.json` schema。
|
||||
- 修改 review CSV 字段或 `apply-review` 工作流。
|
||||
- 引入交互式 TUI/Web 界面。
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
- [x] Task 1. [Status: Done] 重新定义 `review-plan` 的输出顺序为“计划概览 → 风险统计 → 操作预览 → CSV 路径”，优先展示决策信息，再展示审核细节，以替代当前仅有 summary 和 5 条样例的输出方式，现状见 `src/vlm/cli.py:546-563`。
|
||||
- [x] Task 2. [Status: Done] 在 `review-plan` 中优先输出 `ExecutionPlan.human_summary`，若为空则复用现有 fallback summary，避免重复设计摘要逻辑并统一跨命令体验，相关能力见 `src/vlm/planner.py:169-184`、`src/vlm/cli.py:976-987`。
|
||||
- [x] Task 3. [Status: Done] 提取统一的计划终端渲染 helper，负责 plan header、summary、reason 分布和 operation preview 的格式化输出，避免 CLI 命令函数继续承载大量展示细节，参考现有输出风格见 `src/vlm/commands/execute.py:84-103`。
|
||||
- [x] Task 4. [Status: Done] 设计受控预览机制，默认仅展示有限条操作并提示剩余数量，同时预留完整显示模式的扩展点，以兼顾可读性和信息完整性。
|
||||
- [x] Task 5. [Status: Done] 在预览输出中优先展示审核价值最高的字段，包括 `index`、`operation_type`、`risk_flags`、`source_path`、`destination_path`、`reason`，以便用户在不打开 JSON 的情况下完成多数审核判断，字段来源见 `src/vlm/plan_review.py:75-83` 和 `src/vlm/planner.py:606-620`。
|
||||
- [x] Task 6. [Status: Done] 保持 `ExecutionPlan` 模型、plan JSON schema 和 review CSV schema 完全兼容，将改动严格限制在输出层，降低对 `execute`、`report` 和 `apply-review` 的影响，相关结构见 `src/vlm/models.py:133-153` 和 `src/vlm/planner.py:591-625`。
|
||||
- [x] Task 7. [Status: Done] 扩展 `review-plan` CLI 测试，覆盖默认摘要输出、受控预览、完整显示模式、CSV 不变性和原有 summary 输出兼容性，弥补当前测试缺口，基线见 `tests/test_cli_review_plan.py:31-88`。
|
||||
- [x] Task 8. [Status: Done] 评估是否将 `execute` 的计划摘要展示逐步迁移到同一渲染 helper，减少跨命令输出风格分叉，参考现有入口见 `src/vlm/commands/execute.py:84-103` 和 `src/vlm/commands/execute.py:137-146`。
|
||||
- [x] Task 9. [Status: Done] 在最终验收中重点验证大计划场景下的可读性，确保默认输出足够简洁、重点清晰，并且不影响后续 `apply-review` 使用链路，相关流程见 `src/vlm/cli.py:589-643`。
|
||||
|
||||
## Verification Criteria
|
||||
|
||||
- [x] `vlm review-plan` 默认输出中包含计划摘要，而不只是风险计数。
|
||||
- [x] 默认输出中包含可读的操作预览，且预览字段足以支持人工初步审核。
|
||||
- [x] 大计划场景下默认输出不会无上限刷屏，并会提示仍有未展示操作。
|
||||
- [x] review CSV 的字段、写入逻辑与后续 `apply-review` 流程保持兼容，相关链路见 `src/vlm/plan_review.py:89-96` 和 `src/vlm/cli.py:621-643`。
|
||||
- [x] `plan.json` 的 schema、读写行为与现有字段保持不变，见 `src/vlm/planner.py:591-625` 和 `src/vlm/planner.py:628-671`。
|
||||
- [x] CLI 测试覆盖新增预览行为，并保留现有 summary/CSV 行为验证，基线见 `tests/test_cli_review_plan.py:31-88`。
|
||||
|
||||
## Risks and Mitigations
|
||||
|
||||
1. **默认输出过长，降低可读性**
|
||||
Mitigation: 使用默认限量预览，只展示高价值字段，并明确提示剩余条目数量。
|
||||
|
||||
2. **展示逻辑分散，后续难维护**
|
||||
Mitigation: 将计划渲染抽离为统一 helper，让 CLI 命令函数只负责流程编排与参数处理。
|
||||
|
||||
3. **改动误伤 CSV 手工审核链路**
|
||||
Mitigation: 将 CSV 视为稳定接口，不调整字段结构与导出逻辑，保持 `src/vlm/plan_review.py:89-96` 行为不变。
|
||||
|
||||
4. **CLI 输出测试过于脆弱**
|
||||
Mitigation: 测试聚焦结构性关键片段与核心字段，不对整段输出做过度刚性匹配。
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
1. **仅增加 `human_summary` 输出**
|
||||
优点：改动最小，交付最快。
|
||||
缺点：仍然看不到操作层内容。
|
||||
|
||||
2. **直接打印全部 operations**
|
||||
优点：实现简单，信息最完整。
|
||||
缺点：大计划会严重刷屏。
|
||||
|
||||
3. **抽离通用渲染层并提供受控预览**
|
||||
优点：用户体验、可维护性与复用性最平衡。
|
||||
缺点：实现成本略高于局部修补。
|
||||
结论：**推荐采用**。
|
||||
|
||||
## Recommended Outcome
|
||||
|
||||
推荐采用“仅重构展示层、不改数据层”的方案：
|
||||
|
||||
- 保持 `ExecutionPlan`、plan JSON、review CSV 全部兼容。
|
||||
- 为 `review-plan` 增加计划摘要与受控操作预览。
|
||||
- 把计划展示逻辑抽离为可复用渲染能力。
|
||||
- 用测试确保 CLI 可见行为稳定。
|
||||
|
||||
这样可以以最小风险解决当前“review 时看不到计划内容”的核心问题。
|
||||
@@ -0,0 +1,45 @@
|
||||
# Refactor Plan: Safety, Determinism, and Baseline Alignment
|
||||
|
||||
**Date:** 2026-04-07
|
||||
**Basis:** Verified repository state plus `REVIEW_REPORT.md`.
|
||||
**Status:** Completed.
|
||||
|
||||
## Objective
|
||||
|
||||
Refactor the codebase to resolve the safety and determinism issues identified during review, restore a truthful green baseline, and reconcile the review artifacts with the post-refactor state.
|
||||
|
||||
## Execution Outcome
|
||||
|
||||
- Full-suite baseline restored: `pytest -q` → **507 passed**.
|
||||
- The review-plan TUI is now a true optional runtime boundary via guarded Textual imports in `src/vlm/review_tui.py:16-30` and lazy CLI import/use in `src/vlm/cli.py:567-602`.
|
||||
- Move and rename execution now validate both source and destination paths against `library_root` in `src/vlm/executor.py:204-249`.
|
||||
- Duplicate handling is now explicit and deterministic through strict resolver errors in `src/vlm/duplicate_resolve.py:16-58` plus canonicalized planner matching and manual-review fallback in `src/vlm/planner.py:42-57` and `src/vlm/planner.py:151-203`.
|
||||
- Scanner behavior for non-zero `find` exits is now documented in code and deterministic in `src/vlm/scanner.py:158-214`.
|
||||
- Plan loading now crosses a validated typed boundary in `src/vlm/io.py:247-326`.
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
- [x] Task 1. [Status: Done] Re-established a reliable baseline by making the Textual review UI a true optional boundary. Evidence: `src/vlm/cli.py:567-602`, `src/vlm/review_tui.py:16-30`, `tests/test_cli_review_plan.py:237-343`.
|
||||
- [x] Task 2. [Status: Done] Added source-root validation for move and rename operations so execution checks both source and destination against `library_root`. Evidence: `src/vlm/executor.py:204-249`, `tests/test_path_safety.py:76-137`.
|
||||
- [x] Task 3. [Status: Done] Unified execution failure contracts so unsupported quarantine categories now return failed `OperationResult`s, and batch execution contains per-operation exceptions. Evidence: `src/vlm/quarantine.py:116-140`, `src/vlm/executor.py:108-130`, `tests/test_quarantine.py:84-110`, `tests/test_executor.py:951-1004`.
|
||||
- [x] Task 4. [Status: Done] Removed silent duplicate fallback behavior by raising explicit resolver errors and routing unresolved groups into manual review. Evidence: `src/vlm/duplicate_resolve.py:44-58`, `src/vlm/planner.py:187-203`, `tests/test_duplicate_resolve.py:223-237`, `tests/test_planner.py:619-669`.
|
||||
- [x] Task 5. [Status: Done] Introduced canonical path-normalization for duplicate-group matching. Evidence: `src/vlm/planner.py:42-57`, `src/vlm/planner.py:152-169`, `tests/test_planner.py:672-720`.
|
||||
- [x] Task 6. [Status: Done] Defined and implemented an explicit scanner contract for non-zero `find` exits with partial or empty stdout. Evidence: `src/vlm/scanner.py:158-214`, `tests/test_scanner.py:140-186`.
|
||||
- [x] Task 7. [Status: Done] Strengthened the plan I/O boundary with validated typed construction and canonical serialization helpers. Evidence: `src/vlm/io.py:247-326`, `tests/test_io.py:112-203`.
|
||||
- [x] Task 8. [Status: Done] Expanded regression coverage around the identified weak points. Evidence: `tests/test_cli_review_plan.py:237-343`, `tests/test_path_safety.py:76-137`, `tests/test_quarantine.py:84-110`, `tests/test_executor.py:951-1004`, `tests/test_duplicate_resolve.py:223-237`, `tests/test_planner.py:619-720`, `tests/test_scanner.py:140-186`, `tests/test_io.py:112-203`.
|
||||
- [x] Task 9. [Status: Done] Updated existing review artifacts to reflect the final baseline and current findings. Evidence: `REVIEW_REPORT.md`, `docs/TECHNICAL_REVIEW.md`.
|
||||
|
||||
## Verification Criteria
|
||||
|
||||
- [x] `pytest -q` passes, including the review-plan TUI tests in `tests/test_cli_review_plan.py:271-343`.
|
||||
- [x] Manual or crafted plans cannot move or rename sources outside `library_root`. Evidence: `src/vlm/executor.py:204-249`, `tests/test_path_safety.py:106-137`.
|
||||
- [x] Unsupported quarantine categories are recorded as failed results and do not abort later operations in the same execution batch. Evidence: `src/vlm/quarantine.py:116-140`, `src/vlm/executor.py:108-130`, `tests/test_executor.py:951-1004`.
|
||||
- [x] Duplicate resolution no longer silently selects index `0` for quality-data mismatch or unexpected low-level strategy input. Evidence: `src/vlm/duplicate_resolve.py:44-58`, `tests/test_duplicate_resolve.py:223-237`, `tests/test_planner.py:619-669`.
|
||||
- [x] Duplicate-group matching is stable across supported path-format variations and covered by planner tests. Evidence: `src/vlm/planner.py:42-57`, `tests/test_planner.py:672-720`.
|
||||
- [x] Scanner behavior for non-zero `find` exit is explicit, deterministic, and test-covered. Evidence: `src/vlm/scanner.py:158-214`, `tests/test_scanner.py:140-186`.
|
||||
- [x] Plan loading crosses one validated, typed boundary rather than propagating plain dicts after schema validation. Evidence: `src/vlm/io.py:247-326`, `tests/test_io.py:112-203`.
|
||||
- [x] Existing review documents reflect the actual test baseline and remaining findings.
|
||||
|
||||
## Post-plan Note
|
||||
|
||||
The refactor plan is complete. One low-priority packaging observation remains outside the implementation scope: `textual` is still listed in both `dev` and `tui` optional extras in `pyproject.toml:12-20`. The runtime optional-dependency bug itself is resolved via `src/vlm/cli.py:567-602` and `src/vlm/review_tui.py:16-30`.
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,48 @@
|
||||
> [!NOTE]
|
||||
> Status: Historical snapshot. Current refactor results and validated baseline are tracked in `CHANGELOG.md` (updated 2026-04-07).
|
||||
|
||||
# Post-Audit Fixes Implementation Plan (2026-02-13)
|
||||
|
||||
## Objective
|
||||
Address the insufficiencies identified in `VLM_PROJECT_AUDIT_REPORT.md`:
|
||||
1. **Transactional Integrity**: Introduce simple transaction logging for file operations.
|
||||
2. **Deep State Integration**: Ensure `vlm plan` respects "ignored" files in `state.json`.
|
||||
3. **Performance Optimization**: Use `ProcessPoolExecutor` for parallel `ffprobe` scanning.
|
||||
4. **Intelligence/Skill Updates**: Add risk warnings and developer test recipes.
|
||||
|
||||
## Tasks
|
||||
|
||||
### T1: Transaction Logging & Atomic State Updates
|
||||
- **File**: `src/vlm/transaction.py` (New), `src/vlm/executor.py`
|
||||
- **Action**:
|
||||
- Implement `TransactionLog` to save operation "intents" to `transaction.json`.
|
||||
- Modify `ExecutionEngine.execute_plan` to write intents before start and mark completions.
|
||||
- Integrate `StateManager` to update file status to `executed` or `quarantined` immediately after the filesystem move.
|
||||
- **Verification**: Run execution and crash/interrupt it, verify `transaction.json` state.
|
||||
|
||||
### T2: Deep State Integration in Planner
|
||||
- **File**: `src/vlm/planner.py`, `src/vlm/commands/plan.py`
|
||||
- **Action**:
|
||||
- Update `generate_plan` to filter files based on `state.json`.
|
||||
- If a file is marked as `ignored`, the plan should generate a `no-op` with reason "User marked as ignored in state".
|
||||
- **Verification**: Mark a file as ignored using `vlm state`, run `vlm plan`, check `plan.json`.
|
||||
|
||||
### T3: Scanner Performance Upgrade
|
||||
- **File**: `src/vlm/scanner.py`
|
||||
- **Action**:
|
||||
- Replace `ThreadPoolExecutor` with `ProcessPoolExecutor` in `scan_library`.
|
||||
- Ensure `_create_video_file` and its dependencies are picklable.
|
||||
- **Verification**: Run `vlm scan` on a large library, verify metadata is still correctly extracted.
|
||||
|
||||
### T4: Skill & Instruction Enhancements
|
||||
- **File**: `skills/vlm-expert/SKILL.md`, `skills/vlm-library-workflow/SKILL.md`
|
||||
- **Action**:
|
||||
- Add Risk Warning: "If quarantine operations > 20% of total, require secondary confirmation."
|
||||
- Add Developer Recipes: "How to run specific test suites for verification."
|
||||
- **Verification**: Check if Agent mentions risk warnings in high-volume plans.
|
||||
|
||||
## Execution Schedule
|
||||
1. **[x] T2** (Planner integration) - Completed.
|
||||
2. **[!] T3** (Scanner upgrade) - Attempted with ProcessPoolExecutor but deferred due to pickling issues in tests (mocks are not picklable). Reverted to ThreadPoolExecutor.
|
||||
3. **[x] T1** (Transactional engine) - Completed. Integrated TransactionLog and StateManager into ExecutionEngine.
|
||||
4. **[x] T4** (Skill updates) - Completed. Added risk warnings and developer recipes.
|
||||
@@ -0,0 +1,408 @@
|
||||
# 当前代码分析与改进建议
|
||||
|
||||
**日期**: 2026-04-01
|
||||
**范围**: `src/vlm/` 主流程代码、CLI 入口、测试与工程配置
|
||||
**验证方式**: 本地执行全量测试，结果为 **483 passed / 10.73s**
|
||||
|
||||
---
|
||||
|
||||
## 1. 结论摘要
|
||||
|
||||
当前仓库整体处于**中上水平**：架构主线清晰、测试覆盖较完整、执行链路明显偏向“安全优先”。项目并不是一个简单的批量重命名脚本，而是一个**分阶段、可审查、可回滚**的媒体库整理流水线。CLI 入口负责初始化配置与日志，随后将控制权分发到 `scan / parse / enrich / analyze / plan / execute` 等阶段模块。`pyproject.toml:18-19` `src/vlm/cli.py:79-155` `src/vlm/context.py:8-16`
|
||||
|
||||
项目最强的地方在于：
|
||||
|
||||
- 执行默认是 dry-run，真实修改必须显式确认。`src/vlm/cli.py:548-603` `src/vlm/executor.py:74-83`
|
||||
- 执行前有 plan，可人工复核；执行后有 rollback log，可逆操作。`src/vlm/commands/plan.py:68-104` `src/vlm/commands/execute.py:163-177` `src/vlm/executor.py:162-184`
|
||||
- 重复项优先进入 quarantine，而不是直接删除。`src/vlm/planner.py:114-158` `src/vlm/executor.py:234-262`
|
||||
|
||||
但也存在一些明显的工程问题：
|
||||
|
||||
1. `cli.py` 已经偏大，错误处理模式重复。`src/vlm/cli.py:158-603`
|
||||
2. 阶段间仍以大量 dict/JSON 作为契约，类型边界偏弱。`src/vlm/commands/parse.py:32-155` `src/vlm/enrichment.py:24-156` `src/vlm/io.py:85-249`
|
||||
3. 分析阶段对 identity 与 file 的顺序有隐式耦合。`src/vlm/commands/analyze.py:49-55` `src/vlm/io.py:181-249`
|
||||
4. 相同业务规则在多个模块重复实现。`src/vlm/planner.py:34-40` `src/vlm/duplicate_resolve.py:11-16` `src/vlm/plan_review.py:12-16`
|
||||
5. 部分 plan 结果依赖生成时的真实文件系统状态，降低了 plan 的可复现性。`src/vlm/planner.py:43-76` `src/vlm/planner.py:354-358` `src/vlm/planner.py:473-477`
|
||||
|
||||
---
|
||||
|
||||
## 2. 当前架构概览
|
||||
|
||||
### 2.1 总体分层
|
||||
|
||||
项目大致可以分为四层：
|
||||
|
||||
1. **CLI 与命令分发层**：负责参数解析、配置初始化、错误输出。`src/vlm/cli.py:79-155`
|
||||
2. **命令编排层**：负责组织每个阶段的输入、输出和控制台反馈。`src/vlm/commands/scan.py:14-96` `src/vlm/commands/parse.py:16-165` `src/vlm/commands/analyze.py:19-114` `src/vlm/commands/plan.py:14-109` `src/vlm/commands/execute.py:36-257`
|
||||
3. **领域核心层**：承载扫描、解析、增强、分析、规划、执行逻辑。`src/vlm/scanner.py:31-133` `src/vlm/parser.py:80-246` `src/vlm/enrichment.py:24-156` `src/vlm/analysis.py:12-123` `src/vlm/planner.py:79-190` `src/vlm/executor.py:26-184`
|
||||
4. **共享模型/基础设施层**：承载 `Config`、数据模型、I/O、状态、缓存等。`src/vlm/config.py:13-148` `src/vlm/models.py:13-185` `src/vlm/io.py:29-82` `src/vlm/state.py:22-205`
|
||||
|
||||
### 2.2 设计风格
|
||||
|
||||
当前实现采用的是**artifact-driven pipeline**：每个阶段把结果写入中间产物文件，而不是只在内存里串行传递。这种设计的直接收益是：
|
||||
|
||||
- 阶段可以单独重跑；
|
||||
- 中间结果可人工检查或编辑；
|
||||
- 出错定位更容易；
|
||||
- 更适合高风险文件整理任务。`src/vlm/scanner.py:527-593` `src/vlm/commands/parse.py:141-157` `src/vlm/io.py:60-82` `src/vlm/planner.py:597-677`
|
||||
|
||||
---
|
||||
|
||||
## 3. 核心流程分析
|
||||
|
||||
### 3.1 Scan
|
||||
|
||||
扫描阶段负责发现视频文件、分类、提取基础元数据，并可选复用旧 inventory 中的元数据缓存。目录遍历优先使用系统 `find`，不可用时回退到 Python 递归扫描。`src/vlm/commands/scan.py:47-75` `src/vlm/scanner.py:136-183`
|
||||
|
||||
当前分类逻辑完全依赖 `library_root` 下的**顶层目录名**。如果目录不在配置映射中，就会被归入 `other`。这意味着目录规范直接决定了后续 parse/plan 的可用性。`src/vlm/scanner.py:369-412` `src/vlm/config.py:26-30`
|
||||
|
||||
### 3.2 Parse
|
||||
|
||||
解析阶段按 `movie / series / anime / other` 分支工作：
|
||||
|
||||
- 电影：提取标题与年份；`src/vlm/parser.py:80-151`
|
||||
- 剧集：提取标题、季、集，支持多种命名模式；`src/vlm/parser.py:154-246`
|
||||
- anime/other：当前 v1 不自动组织，仅标记说明。`src/vlm/commands/parse.py:101-117`
|
||||
|
||||
该阶段还支持把 inventory 中的 `size / resolution / codec / bitrate` 嵌入 identity 记录，为后续重复项质量比较做准备。`src/vlm/commands/parse.py:23-29` `src/vlm/commands/parse.py:50-100`
|
||||
|
||||
### 3.3 Enrich
|
||||
|
||||
增强阶段会根据 title/year 调用 provider，并把结果缓存到 SQLite。增强内容包括：
|
||||
|
||||
- 中文/英文标题；
|
||||
- 外部评分与票数；
|
||||
- `needs_review` 决策；
|
||||
- `display_title`。`src/vlm/enrichment.py:24-156` `src/vlm/enrichment.py:251-356`
|
||||
|
||||
这里有一个非常关键的点：`plan` 阶段构建 identity 时优先读取 `display_title`。因此，**enrich 的结果会直接影响最终目录与文件命名**。`src/vlm/io.py:114-178`
|
||||
|
||||
### 3.4 Analyze
|
||||
|
||||
分析阶段做两件核心工作：
|
||||
|
||||
- 对剧集做缺集启发式检测；`src/vlm/analysis.py:12-67`
|
||||
- 对电影/剧集做重复项检测与质量比较。`src/vlm/analysis.py:70-123`
|
||||
|
||||
这里的完整性分析不是“官方元数据级别的完整性判断”，只是依据已有文件范围 `[min_episode, max_episode]` 找缺口。`src/vlm/analysis.py:13-21` `src/vlm/analysis.py:50-67`
|
||||
|
||||
### 3.5 Plan / Execute
|
||||
|
||||
`plan` 会把 `(VideoFile, Identity)` 转成 `FileOperation`，并结合 analysis 和状态信息做重复项保留、ignored 文件过滤、目录保留提示等。`src/vlm/commands/plan.py:36-66` `src/vlm/planner.py:79-190`
|
||||
|
||||
`execute` 负责真正落盘执行，并在 execute 模式下维护 transaction log、rollback log 和 state。`src/vlm/commands/execute.py:70-132` `src/vlm/executor.py:92-179` `src/vlm/state.py:105-205`
|
||||
|
||||
---
|
||||
|
||||
## 4. 优点总结
|
||||
|
||||
### 4.1 安全优先，适合真实文件整理
|
||||
|
||||
这是当前代码最成熟的能力：
|
||||
|
||||
- 默认 dry-run；`src/vlm/cli.py:555-559` `src/vlm/executor.py:78-83`
|
||||
- 支持回滚；`src/vlm/commands/execute.py:163-177` `src/vlm/executor.py:162-184`
|
||||
- 重复项优先 quarantine；`src/vlm/planner.py:148-157` `src/vlm/executor.py:234-262`
|
||||
- state 持久化写入采用临时文件 + `os.replace`，有原子替换保护。`src/vlm/state.py:82-102`
|
||||
|
||||
### 4.2 模块职责相对清晰
|
||||
|
||||
`scanner / parser / enrichment / analysis / planner / executor` 的分工符合业务阶段，也方便编写针对性测试。`src/vlm/scanner.py:31-133` `src/vlm/parser.py:80-246` `src/vlm/enrichment.py:24-156` `src/vlm/analysis.py:12-123` `src/vlm/planner.py:79-190` `src/vlm/executor.py:26-184`
|
||||
|
||||
### 4.3 数据模型可读性较好
|
||||
|
||||
`VideoFile`、`MovieIdentity`、`SeriesIdentity`、`FileOperation`、`ExecutionPlan`、`RollbackLog` 等模型把主流程概念表达得比较完整。`src/vlm/models.py:13-185`
|
||||
|
||||
### 4.4 测试覆盖明显比较扎实
|
||||
|
||||
测试目录包含 29 个测试文件，覆盖 parser、scanner、planner、executor、quarantine、CLI、state、reports 等模块；其中还包含 Hypothesis 属性测试。`pyproject.toml:28-35` `tests/test_scanner.py:27-260` `tests/test_enrichment.py:36-260` `tests/test_cli_state.py:58-240` `tests/test_executor.py:18-120` `tests/test_quarantine.py:13-120` `tests/test_planner.py:18-120` `tests/test_analysis_properties.py:1-80`
|
||||
|
||||
---
|
||||
|
||||
## 5. 发现的问题与解决方案
|
||||
|
||||
## 问题 1：`cli.py` 体积偏大，异常处理模式重复
|
||||
|
||||
### 现象
|
||||
|
||||
CLI 入口同时承担了：
|
||||
|
||||
- 配置加载；
|
||||
- 日志初始化；
|
||||
- 旧路径兼容；
|
||||
- 多个主命令注册；
|
||||
- 多处重复的 `try/except + click.echo + logger + sys.exit`。`src/vlm/cli.py:49-76` `src/vlm/cli.py:79-155` `src/vlm/cli.py:158-603`
|
||||
|
||||
### 影响
|
||||
|
||||
- 新增命令时容易继续复制粘贴错误处理；
|
||||
- 入口文件持续膨胀；
|
||||
- 行为一致性难以长期保证。
|
||||
|
||||
### 解决方案
|
||||
|
||||
**建议短期方案**：抽取统一的命令异常包装器，例如：
|
||||
|
||||
- 一个通用 `run_command()`；
|
||||
- 或一个 Click 装饰器，用于统一处理 `FileNotFoundError / JSONDecodeError / ValueError / OSError`；
|
||||
- CLI 仅保留参数定义和分发。
|
||||
|
||||
**建议中期方案**：继续下沉命令定义，把 `review-plan / quarantine / rollback / report` 这类非主链命令拆到独立模块中，再由 CLI 统一注册。`src/vlm/cli.py:452-620`
|
||||
|
||||
### 优先级
|
||||
|
||||
**高**。这是典型的结构性技术债，越晚处理，越容易固化。
|
||||
|
||||
---
|
||||
|
||||
## 问题 2：阶段间契约大量依赖裸 dict，类型边界偏弱
|
||||
|
||||
### 现象
|
||||
|
||||
尽管项目有 dataclass 模型，但在 `parse -> enrich -> analyze/plan` 之间，很多地方仍是 dict 直接读写，例如 parse 直接拼 JSON record，enrich 直接修改 record，I/O 层再把 record 组装回 dataclass。`src/vlm/commands/parse.py:66-155` `src/vlm/enrichment.py:66-156` `src/vlm/io.py:85-178` `src/vlm/io.py:181-249`
|
||||
|
||||
### 影响
|
||||
|
||||
- schema 漂移风险较高；
|
||||
- 某字段是否存在依赖运行时约定；
|
||||
- 对静态检查和重构不友好。
|
||||
|
||||
### 解决方案
|
||||
|
||||
**建议短期方案**：为 `identities.json`、`analysis.json`、`plan.json` 定义更明确的 schema 约束，并在 load/save 时做结构校验。`src/vlm/io.py:29-82` `src/vlm/planner.py:597-677`
|
||||
|
||||
**建议中期方案**：引入中间层 dataclass 或 `TypedDict`：
|
||||
|
||||
- `IdentityRecordMovie`
|
||||
- `IdentityRecordSeries`
|
||||
- `AnalysisDuplicateRecord`
|
||||
- `PlanRecord`
|
||||
|
||||
让 parse/enrich/analyze/plan 在边界上操作显式类型，而不是开放 dict。
|
||||
|
||||
### 优先级
|
||||
|
||||
**高**。这是影响长期可维护性的核心问题。
|
||||
|
||||
---
|
||||
|
||||
## 问题 3：分析阶段对输入顺序有隐式耦合
|
||||
|
||||
### 现象
|
||||
|
||||
`analyze_cmd()` 先拿到 `movie_identities / series_identities / video_files`，再通过切片和 `zip()` 组合成 `identity_file_pairs`。这要求 `io.identities_to_analysis_input()` 输出的 `video_files` 与两个 identity 列表保持严格顺序一致。`src/vlm/commands/analyze.py:41-55` `src/vlm/io.py:181-249`
|
||||
|
||||
### 影响
|
||||
|
||||
- 现在能工作，但逻辑脆弱；
|
||||
- 只要 `io.py` 内部重构顺序，重复项分析就可能悄悄出错；
|
||||
- 问题一旦发生，通常不是异常，而是“结果不对”。
|
||||
|
||||
### 解决方案
|
||||
|
||||
**建议直接改造**：让 `identities_to_analysis_input()` 直接返回 `list[tuple[Identity, VideoFile]]`，避免调用侧再切片配对。
|
||||
或者统一按 `path` 建索引后 join，而不是靠列表位置。`src/vlm/io.py:181-249`
|
||||
|
||||
### 优先级
|
||||
|
||||
**高**。这属于“看起来没问题，但很脆”的隐式约定。
|
||||
|
||||
---
|
||||
|
||||
## 问题 4：重复规则散落，`sample` 识别逻辑重复实现
|
||||
|
||||
### 现象
|
||||
|
||||
`sample` 文件识别规则在以下位置重复存在：
|
||||
|
||||
- `planner.py`; `src/vlm/planner.py:34-40`
|
||||
- `duplicate_resolve.py`; `src/vlm/duplicate_resolve.py:11-16`
|
||||
- `plan_review.py`; `src/vlm/plan_review.py:12-16`
|
||||
|
||||
### 影响
|
||||
|
||||
- 后续如果要更新 sample 识别规则，必须改三处；
|
||||
- 一旦其中一处漏改，会出现行为不一致；
|
||||
- 测试也会被迫覆盖重复逻辑。
|
||||
|
||||
### 解决方案
|
||||
|
||||
提取单一公共函数，例如放到 `utils.py` 或新增 `media_rules.py`：
|
||||
|
||||
- `is_sample_path(path: Path) -> bool`
|
||||
- 所有模块统一调用；
|
||||
- 增加一组针对 sample 识别的集中测试。
|
||||
|
||||
### 优先级
|
||||
|
||||
**中高**。改动不大，但收益稳定。
|
||||
|
||||
---
|
||||
|
||||
## 问题 5：plan 结果依赖实时文件系统状态，降低可复现性
|
||||
|
||||
### 现象
|
||||
|
||||
plan 阶段会：
|
||||
|
||||
- 分析哪些目录会被搬空；`src/vlm/planner.py:43-76`
|
||||
- 检查目标文件是否已存在；`src/vlm/planner.py:354-358` `src/vlm/planner.py:473-477`
|
||||
- 将这些事实写入 operation 和 metadata。`src/vlm/planner.py:159-190`
|
||||
|
||||
### 影响
|
||||
|
||||
- 同一份输入 identities，在不同时间点可能生成不同 plan；
|
||||
- plan 更像“当下环境快照”，而不是纯粹的逻辑结果；
|
||||
- 对复现、比对和离线审查不够友好。
|
||||
|
||||
### 解决方案
|
||||
|
||||
**建议短期方案**：保留当前行为，但把“实时文件系统检查结果”明确标记为环境信息，例如：
|
||||
|
||||
- `metadata.runtime_conflict_snapshot`
|
||||
- `metadata.directory_impact_snapshot`
|
||||
|
||||
**建议中期方案**：将 plan 拆成两步：
|
||||
|
||||
1. **logical plan**：只表达目标路径和操作意图；
|
||||
2. **validation pass**：在 execute 前单独检查冲突和目录影响。
|
||||
|
||||
这样 plan 更可复现，execute 前验证也更明确。`src/vlm/commands/execute.py:16-33`
|
||||
|
||||
### 优先级
|
||||
|
||||
**中高**。不是 bug，但会限制系统演进。
|
||||
|
||||
---
|
||||
|
||||
## 问题 6：配置语义有轻微串味
|
||||
|
||||
### 现象
|
||||
|
||||
扫描阶段并发控制使用的是 `config.enrichment_max_concurrency`。`src/vlm/scanner.py:76-77` `src/vlm/config.py:32-40`
|
||||
|
||||
### 影响
|
||||
|
||||
- 配置名称与用途不完全一致；
|
||||
- 用户理解成本增加；
|
||||
- 后面如果 enrich 和 scan 的并发需求不同，不易扩展。
|
||||
|
||||
### 解决方案
|
||||
|
||||
新增独立配置字段，例如：
|
||||
|
||||
- `scan_max_concurrency`
|
||||
- `enrichment_max_concurrency`
|
||||
|
||||
并保留兼容逻辑：若未设置 `scan_max_concurrency`，则回退到 `enrichment_max_concurrency`。
|
||||
|
||||
### 优先级
|
||||
|
||||
**中**。
|
||||
|
||||
---
|
||||
|
||||
## 问题 7：CLI 帮助文本与当前真实流程存在轻微偏差
|
||||
|
||||
### 现象
|
||||
|
||||
`main()` 顶部展示的 common workflow 没有包含 `enrich`，但实际项目已将 enrich 纳入正式流程。`src/vlm/cli.py:100-107` `src/vlm/cli.py:266-319`
|
||||
|
||||
### 影响
|
||||
|
||||
- 新用户可能低估 enrich 的作用；
|
||||
- 尤其在当前实现里，enrich 会影响最终命名路径。`src/vlm/io.py:114-178`
|
||||
|
||||
### 解决方案
|
||||
|
||||
更新 CLI 顶部说明，把推荐流程改为：
|
||||
|
||||
`scan -> parse -> enrich -> analyze -> plan -> execute`
|
||||
|
||||
### 优先级
|
||||
|
||||
**低到中**。实现很简单，但用户感知收益明显。
|
||||
|
||||
---
|
||||
|
||||
## 问题 8：review 状态模型存在语义重叠
|
||||
|
||||
### 现象
|
||||
|
||||
`MovieIdentity` 与 `SeriesIdentity` 注释已经明确指出 `needs_review` 与 `review_status` 存在语义交叉。`src/vlm/models.py:46-53` `src/vlm/models.py:79-85`
|
||||
|
||||
### 影响
|
||||
|
||||
- 状态转移规则不够清晰；
|
||||
- 后续手工审核、自动审批、拒绝逻辑继续增加时，复杂度会快速上升。
|
||||
|
||||
### 解决方案
|
||||
|
||||
建议把两者整理为更明确的状态机：
|
||||
|
||||
- `review_status`: `pending / approved / rejected`
|
||||
- `review_reason`: `low_confidence / missing_year / low_reputation / manual`
|
||||
- `needs_review` 作为派生字段，不持久化或不作为主状态源
|
||||
|
||||
### 优先级
|
||||
|
||||
**中**。建议在下一轮涉及审核工作流时一起处理。
|
||||
|
||||
---
|
||||
|
||||
## 6. 推荐改进顺序
|
||||
|
||||
### 第一优先级（建议优先处理）
|
||||
|
||||
1. **收缩 `cli.py`，统一异常处理**。`src/vlm/cli.py:158-603`
|
||||
2. **加强阶段间类型契约，减少裸 dict 传播**。`src/vlm/commands/parse.py:66-155` `src/vlm/io.py:85-249`
|
||||
3. **消除 analyze 阶段的顺序耦合**。`src/vlm/commands/analyze.py:49-55`
|
||||
|
||||
### 第二优先级（建议随后处理）
|
||||
|
||||
4. **提取统一的 sample/path 规则函数**。`src/vlm/planner.py:34-40` `src/vlm/duplicate_resolve.py:11-16` `src/vlm/plan_review.py:12-16`
|
||||
5. **把 plan 的逻辑结果与环境验证拆分**。`src/vlm/planner.py:43-76` `src/vlm/commands/execute.py:16-33`
|
||||
6. **拆分 scan/enrich 并发配置**。`src/vlm/scanner.py:76-77` `src/vlm/config.py:32-40`
|
||||
|
||||
### 第三优先级（体验与语义优化）
|
||||
|
||||
7. **修正 CLI workflow 文案**。`src/vlm/cli.py:100-107` `src/vlm/cli.py:266-319`
|
||||
8. **收敛 review 状态模型**。`src/vlm/models.py:46-53` `src/vlm/models.py:79-85`
|
||||
|
||||
---
|
||||
|
||||
## 7. 测试与工程状态
|
||||
|
||||
### 7.1 测试状态
|
||||
|
||||
当前测试状态良好，已覆盖：
|
||||
|
||||
- scanner；`tests/test_scanner.py:27-260`
|
||||
- enrichment；`tests/test_enrichment.py:36-260`
|
||||
- planner；`tests/test_planner.py:18-120`
|
||||
- executor；`tests/test_executor.py:18-120`
|
||||
- quarantine；`tests/test_quarantine.py:13-120`
|
||||
- CLI state；`tests/test_cli_state.py:58-240`
|
||||
- analysis property-based tests。`tests/test_analysis_properties.py:1-80`
|
||||
|
||||
### 7.2 工程配置
|
||||
|
||||
项目要求 Python 3.10+，显式运行依赖较少，当前主要依赖 `click` 与 `pyyaml`，测试框架使用 `pytest` 与 `hypothesis`。`pyproject.toml:1-19` `pyproject.toml:28-35`
|
||||
|
||||
这说明项目偏“标准库驱动 + 轻依赖”。优点是可维护性和部署都比较轻；不足是静态质量门禁配置（如 lint/type check）目前没有明显体现。`pyproject.toml:28-35`
|
||||
|
||||
---
|
||||
|
||||
## 8. 最终判断
|
||||
|
||||
这个仓库当前最值得肯定的是：它已经具备了一个**真实可用文件整理工具**应有的安全骨架。它的主要问题不是“代码不可用”，而是“随着功能继续增长，结构性技术债会逐步放大”。
|
||||
|
||||
因此，最合适的策略不是大规模推倒重来，而是：
|
||||
|
||||
- 继续保留当前 pipeline 主线；
|
||||
- 优先处理 CLI 入口膨胀、阶段契约脆弱、规则重复这些结构性问题；
|
||||
- 在不破坏安全模型的前提下，提高代码边界清晰度与可演进性。
|
||||
|
||||
如果按一句话总结：
|
||||
|
||||
> 当前代码已经有不错的产品化基础，下一步应从“功能正确”转向“边界稳固、结构可持续演进”。
|
||||
@@ -0,0 +1,313 @@
|
||||
> [!NOTE]
|
||||
> Status: Historical snapshot. Current refactor results and validated baseline are tracked in `CHANGELOG.md` (updated 2026-04-07).
|
||||
|
||||
# VLM 代码改进清单
|
||||
|
||||
本文档记录对 Video Library Manager (VLM) 项目的代码审查发现的问题及对应解决方案。排除 AI/OpenAPI 相关问题。
|
||||
|
||||
---
|
||||
|
||||
## 高优先级（确信度 ≥ 0.9）
|
||||
|
||||
### 1. 时间戳未统一使用 UTC
|
||||
|
||||
**问题描述**
|
||||
|
||||
多处使用 `datetime.now()` 未指定 timezone，与项目约定「timestamps in UTC」不一致，可能导致：
|
||||
- 序列化为 ISO 时缺少 `+00:00` 后缀
|
||||
- 多环境部署时依赖本地时区，行为不一致
|
||||
|
||||
**涉及文件**
|
||||
|
||||
| 文件 | 行号 |
|
||||
|------|------|
|
||||
| `planner.py` | 51 |
|
||||
| `executor.py` | 89, 119, 471 |
|
||||
| `state.py` | 104, 139, 173 |
|
||||
| `quarantine.py` | 61, 529 |
|
||||
|
||||
**确信度**: 0.95
|
||||
|
||||
**解决方案**
|
||||
|
||||
1. 在 `vlm/utils.py` 或现有模块中定义：
|
||||
|
||||
```python
|
||||
from datetime import datetime, timezone
|
||||
|
||||
def utc_now() -> datetime:
|
||||
"""Return current UTC time (timezone-aware)."""
|
||||
return datetime.now(timezone.utc)
|
||||
```
|
||||
|
||||
2. 全局替换所有 `datetime.now()` 为 `utc_now()` 或 `datetime.now(timezone.utc)`
|
||||
3. 在 `load_plan`、`load_rollback_log` 等反序列化时，对 naive datetime 做 `replace(tzinfo=timezone.utc)` 以保持向后兼容
|
||||
|
||||
---
|
||||
|
||||
### 2. 未使用的依赖 ffmpeg-python
|
||||
|
||||
**问题描述**
|
||||
|
||||
`pyproject.toml` 声明 `ffmpeg-python>=0.2.0`，但代码中未 import。scanner 使用 `subprocess` 直接调用 ffprobe。
|
||||
|
||||
**确信度**: 0.95
|
||||
|
||||
**解决方案**
|
||||
|
||||
从 `pyproject.toml` 的 dependencies 中移除 `ffmpeg-python`。若未来改用 ffmpeg-python 库再添加。
|
||||
|
||||
---
|
||||
|
||||
### 3. Parser 中 video extensions 硬编码
|
||||
|
||||
**问题描述**
|
||||
|
||||
`parser.py` 第 100、169 行使用固定扩展名列表 `['.mp4', '.mkv', ...]`，与 `config.video_extensions` 不一致。
|
||||
|
||||
- 用户在 config 中新增扩展（如 `.ts`），scan 能发现，但 parse 去扩展名时不会匹配
|
||||
- 如 `Movie (2020).ts` 可能得到错误的 title 解析
|
||||
|
||||
**确信度**: 0.9
|
||||
|
||||
**解决方案**
|
||||
|
||||
1. `parse_movie` / `parse_series` 增加可选参数 `extensions: list[str]`
|
||||
2. CLI parse 命令调用时传入 `config.video_extensions`
|
||||
3. 默认值使用与 config 相同的列表以保持向后兼容
|
||||
|
||||
---
|
||||
|
||||
### 4. 抽出统一的 I/O 层
|
||||
|
||||
**问题描述**
|
||||
|
||||
数据读取和转换分散在各 CLI 命令中，同一份 identities JSON 在 analyze、plan 等处有重复且略有不同的转换逻辑。新增字段时需多处同步，易遗漏。
|
||||
|
||||
**确信度**: 0.9
|
||||
|
||||
**解决方案**
|
||||
|
||||
新增 `vlm/io.py`，集中：
|
||||
|
||||
- `load_inventory_csv(path) -> list[VideoFile]`
|
||||
- `save_inventory_csv(files, path, library_root)`
|
||||
- `load_identities_json(path) -> dict`
|
||||
- `save_identities_json(data, path)`
|
||||
- `identities_to_plan_input(data) -> list[(VideoFile, Identity | None)]`
|
||||
- `identities_to_analysis_input(data) -> (list[MovieIdentity], list[SeriesIdentity], list[VideoFile])`
|
||||
|
||||
CLI 只调用这些函数，不再直接解析和构造 dataclass。
|
||||
|
||||
---
|
||||
|
||||
### 5. Analyze 阶段 VideoFile metadata 丢失
|
||||
|
||||
**问题描述**
|
||||
|
||||
`identities.json` 不含 `size_bytes`、`resolution`、`codec` 等，CLI 构造 `VideoFile` 时用 0 或 None，导致 `compare_quality()` 无法有效比较，duplicate 报告信息不足。
|
||||
|
||||
**确信度**: 0.9
|
||||
|
||||
**解决方案**
|
||||
|
||||
1. **方案 A**：analyze 命令同时接受 `--inventory`，从 inventory.csv 加载 metadata 并与 identities 按 path 合并
|
||||
2. **方案 B**：parse 输出时在 identities 中附带 size/resolution/codec（从 inventory 合并），避免 analyze 再读 inventory
|
||||
|
||||
---
|
||||
|
||||
## 中优先级（确信度 0.8–0.89）
|
||||
|
||||
### 6. 拆分 CLI 为 commands 子模块
|
||||
|
||||
**问题描述**
|
||||
|
||||
`cli.py` 约 1700 行，混合参数定义、业务逻辑、I/O、输出展示，维护和单测困难。
|
||||
|
||||
**确信度**: 0.85
|
||||
|
||||
**解决方案**
|
||||
|
||||
```
|
||||
src/vlm/
|
||||
cli.py # main、参数、ctx 传递、调用 commands
|
||||
commands/
|
||||
__init__.py
|
||||
scan.py # scan_cmd(ctx, ...)
|
||||
parse.py # parse_cmd(ctx, ...)
|
||||
enrich.py
|
||||
analyze.py
|
||||
plan.py
|
||||
execute.py
|
||||
report.py # 或按子命令拆分
|
||||
quarantine.py
|
||||
state.py
|
||||
config_cmd.py
|
||||
```
|
||||
|
||||
每个 `*_cmd` 接收 `ctx` 和参数，CLI 只做装饰与调用。单测可直接测 `*_cmd` 函数。
|
||||
|
||||
---
|
||||
|
||||
### 7. Provider last_request_count 非正式接口
|
||||
|
||||
**问题描述**
|
||||
|
||||
`enrichment.py` 使用 `getattr(provider, "last_request_count", 1)` 统计 API 调用，依赖实现细节。
|
||||
|
||||
**确信度**: 0.85
|
||||
|
||||
**解决方案**
|
||||
|
||||
1. 在 `providers/base.py` 的 `EnrichmentProvider` 协议中显式声明 `last_request_count: int` 属性
|
||||
2. TMDBProvider 确保实现该属性
|
||||
3. enrichment 通过协议访问，去掉 `getattr`
|
||||
|
||||
---
|
||||
|
||||
### 8. 异常捕获过宽
|
||||
|
||||
**问题描述**
|
||||
|
||||
约 20+ 处 `except Exception`，容易吞掉逻辑错误，难以区分可恢复错误与编程错误。
|
||||
|
||||
**涉及**：`cli.py`、`enrichment.py`、`executor.py`、`quarantine.py` 等。
|
||||
|
||||
**确信度**: 0.8
|
||||
|
||||
**解决方案**
|
||||
|
||||
- 针对预期异常（`FileNotFoundError`、`json.JSONDecodeError`、`ValueError`）分别处理
|
||||
- 保留顶层 `except Exception` 作为兜底，记录完整 traceback 后 `sys.exit(1)`
|
||||
- 避免在业务逻辑深处宽泛捕获
|
||||
|
||||
---
|
||||
|
||||
### 9. Enrichment 主循环过长
|
||||
|
||||
**问题描述**
|
||||
|
||||
`enrich_identities_data` 主循环约 80 行，混合迭代、缓存、API 调用、统计、payload 合并，可读性和可测性差。
|
||||
|
||||
**确信度**: 0.8
|
||||
|
||||
**解决方案**
|
||||
|
||||
拆分为：
|
||||
|
||||
- `_process_single_record(record, media_type, ...) -> None`
|
||||
- `_fetch_from_providers(record, media_type, providers, ...) -> tuple[dict, int, list, str]`
|
||||
- `_update_stats(stats, ...) -> None`
|
||||
- 主循环只负责迭代与调用上述函数
|
||||
|
||||
---
|
||||
|
||||
## 较低优先级（确信度 0.7–0.79）
|
||||
|
||||
### 10. Duplicate 检测 file_map 使用 filename 作为 key
|
||||
|
||||
**问题描述**
|
||||
|
||||
`analysis.py` 第 87 行：
|
||||
|
||||
```python
|
||||
file_map = {file.filename: file for file in files}
|
||||
```
|
||||
|
||||
同 filename 不同路径会互相覆盖（如 `/a/Movie.mkv` 与 `/b/Movie.mkv`），导致 identity 映射到错误 VideoFile。
|
||||
|
||||
**确信度**: 0.75
|
||||
|
||||
**解决方案**
|
||||
|
||||
- 使用 `str(file.path)` 作为 key
|
||||
- 确保 identity 与 VideoFile 的关联方式一致（如通过 path 或 (path, filename) 建立映射）
|
||||
|
||||
---
|
||||
|
||||
### 11. MovieIdentity / SeriesIdentity 字段重复
|
||||
|
||||
**问题描述**
|
||||
|
||||
两个 dataclass 有约 10 个共同 enrichment 字段，新增时需改两处，合并逻辑需分支处理。
|
||||
|
||||
**确信度**: 0.7
|
||||
|
||||
**解决方案**
|
||||
|
||||
- **方案 A**：抽取 `EnrichmentMixin` 基类，`MovieIdentity` / `SeriesIdentity` 继承
|
||||
- **方案 B（推荐）**：引入 `EnrichmentPayload` dataclass，两个 Identity 通过 `enrichment: EnrichmentPayload` 组合，侵入较小
|
||||
|
||||
---
|
||||
|
||||
### 12. Config 体积膨胀
|
||||
|
||||
**问题描述**
|
||||
|
||||
`Config` 约 50 个字段，enrichment/TMDB 相关占多数，职责混杂。
|
||||
|
||||
**确信度**: 0.7
|
||||
|
||||
**解决方案**
|
||||
|
||||
拆分 `EnrichmentConfig`、`TMDBConfig` 等子配置，通过嵌套或组合放入主 Config。
|
||||
|
||||
---
|
||||
|
||||
### 13. review_status 与 needs_review 语义重叠
|
||||
|
||||
**问题描述**
|
||||
|
||||
`review_status`（pending/approved/rejected）与 `needs_review`（bool）含义重叠，易混淆。
|
||||
|
||||
**确信度**: 0.75
|
||||
|
||||
**解决方案**
|
||||
|
||||
- 在 docstring 或文档中明确定义：`needs_review = (review_status == 'pending') and ...`
|
||||
- 或在模型中合并为单一状态枚举，避免两个字段语义交叉
|
||||
|
||||
---
|
||||
|
||||
## 低优先级（确信度 < 0.7）
|
||||
|
||||
### 14. config_init 与 ctx 一致性
|
||||
|
||||
**问题描述**
|
||||
|
||||
`config init` 未使用 `@pass_context`，与同组其他命令风格不一致，但当前不依赖 ctx，非功能性 bug。
|
||||
|
||||
**确信度**: 0.5
|
||||
|
||||
**解决方案**
|
||||
|
||||
若其他 config 子命令均用 `@pass_context`，可统一为 `config_init` 也接收 ctx 以保持风格一致；否则可保持现状。
|
||||
|
||||
---
|
||||
|
||||
### 15. CLI 延迟 import
|
||||
|
||||
**问题描述**
|
||||
|
||||
各命令在函数体内才 `import`，错误在首次执行该命令时才暴露，依赖关系不直观。
|
||||
|
||||
**确信度**: 0.6
|
||||
|
||||
**解决方案**
|
||||
|
||||
可接受；若希望启动时即发现依赖问题，可改为模块级 import，但会增加启动开销。
|
||||
|
||||
---
|
||||
|
||||
## 执行建议
|
||||
|
||||
| 阶段 | 项目 | 说明 |
|
||||
|------|------|------|
|
||||
| 第一批 | 1, 2, 3 | 改动小、风险低、收益明确 |
|
||||
| 第二批 | 4, 5 | 需要一定重构，与 I/O 设计相关 |
|
||||
| 第三批 | 6, 7, 8, 9 | 结构性改进，建议分步完成 |
|
||||
| 第四批 | 10–15 | 按需和档期安排 |
|
||||
|
||||
---
|
||||
|
||||
*文档生成日期：2025-02-10*
|
||||
@@ -0,0 +1,121 @@
|
||||
> [!NOTE]
|
||||
> Status: Historical snapshot. Current refactor results and validated baseline are tracked in `CHANGELOG.md` (updated 2026-04-07).
|
||||
|
||||
# Fix Plan (Verified Issues Only)
|
||||
|
||||
## Objective
|
||||
- Repair only verified defects from `REVIEW_REPORT.md`.
|
||||
- Keep behavior stable outside defect scope.
|
||||
- Ensure every fix is testable and reproducible.
|
||||
|
||||
## Definition of Done
|
||||
- `uv run --with pytest --with hypothesis pytest -q` passes.
|
||||
- No test imports use `src.vlm...`.
|
||||
- Timestamp handling outputs correct UTC instants.
|
||||
- Logging remains usable when file log path is unwritable.
|
||||
- Default home-based paths are resolved at runtime, not import time.
|
||||
|
||||
## Phase 0 - Baseline and guardrails
|
||||
- Run baseline suite and keep output:
|
||||
- `uv run --with pytest --with hypothesis pytest -q`
|
||||
- Record current known failures for before/after comparison.
|
||||
- Guardrails:
|
||||
- No unrelated refactor.
|
||||
- No behavior change outside listed findings.
|
||||
|
||||
## Phase 1 - Fix broken test import paths (P1)
|
||||
- Files:
|
||||
- `tests/test_executor.py`
|
||||
- `tests/test_quarantine.py`
|
||||
- Changes:
|
||||
- Replace `from src.vlm...` with `from vlm...`.
|
||||
- Verification:
|
||||
- `uv run --with pytest --with hypothesis pytest -q tests/test_executor.py tests/test_quarantine.py`
|
||||
- `uv run --with pytest --with hypothesis pytest -q`
|
||||
- Acceptance:
|
||||
- No `ModuleNotFoundError: No module named 'src'`.
|
||||
|
||||
## Phase 2 - Fix timezone relabeling bug (P2)
|
||||
- Files:
|
||||
- `src/vlm/scanner.py`
|
||||
- `src/vlm/reports.py`
|
||||
- Changes:
|
||||
- Use aware UTC timestamp creation for file mtimes.
|
||||
- Remove naive `replace(tzinfo=timezone.utc)` relabeling in export paths.
|
||||
- Enforce explicit conversion to UTC instant.
|
||||
- Tests:
|
||||
- Add/extend tests for naive timestamps under non-UTC local timezone assumptions.
|
||||
- Assert true instant conversion (not label swap).
|
||||
- Verification:
|
||||
- `uv run --with pytest --with hypothesis pytest -q tests/test_scanner.py tests/test_reports.py`
|
||||
- `uv run --with pytest --with hypothesis pytest -q`
|
||||
- Acceptance:
|
||||
- Timestamp tests pass and no relabel bug remains.
|
||||
|
||||
## Phase 3 - Remove dead `error_count` logic (P3)
|
||||
- File:
|
||||
- `src/vlm/scanner.py`
|
||||
- Preferred change:
|
||||
- Remove unused `error_count` and dead summary branch.
|
||||
- Alternative (if product requires count):
|
||||
- Increment and propagate count from scan exception paths.
|
||||
- Verification:
|
||||
- `uv run --with pytest --with hypothesis pytest -q tests/test_scanner.py`
|
||||
- Acceptance:
|
||||
- No dead state/branch for error counting.
|
||||
|
||||
## Phase 4 - Harden logging setup fallback (P3 risk)
|
||||
- File:
|
||||
- `src/vlm/logging_config.py`
|
||||
- Changes:
|
||||
- Wrap log-dir creation and file handler setup with `try/except (OSError, PermissionError)`.
|
||||
- Keep console logging active when file logging cannot initialize.
|
||||
- Emit one clear warning about fallback.
|
||||
- Tests:
|
||||
- Add/extend tests to simulate mkdir/file-handler failure.
|
||||
- Verify process continues (warning-only behavior).
|
||||
- Verification:
|
||||
- `uv run --with pytest --with hypothesis pytest -q tests/test_logging.py tests/test_cli.py`
|
||||
- `uv run --with pytest --with hypothesis pytest -q`
|
||||
- Acceptance:
|
||||
- No command abort when file log path is unwritable.
|
||||
|
||||
## Phase 5 - Resolve import-time home defaults (P3 new)
|
||||
- Files:
|
||||
- `src/vlm/logging_config.py`
|
||||
- `src/vlm/cli.py`
|
||||
- Changes:
|
||||
- Replace import-time `Path.home()` constants with runtime helper functions.
|
||||
- Resolve defaults at function/option execution time.
|
||||
- Tests:
|
||||
- Add/extend monkeypatch tests to prove runtime resolution.
|
||||
- Verification:
|
||||
- `uv run --with pytest --with hypothesis pytest -q tests/test_cli*.py tests/test_logging.py`
|
||||
- `uv run --with pytest --with hypothesis pytest -q`
|
||||
- Acceptance:
|
||||
- Defaults follow runtime environment changes in tests and execution.
|
||||
|
||||
## Documentation Updates (with code changes)
|
||||
- `README.md`
|
||||
- Add troubleshooting note for logging fallback to console-only.
|
||||
- Clarify UTC timestamp expectation in outputs/reports.
|
||||
- `REVIEW_REPORT.md`
|
||||
- Update each issue status (`open` -> `fixed`) with evidence.
|
||||
|
||||
## Execution Order
|
||||
1. Phase 1 (restore full test collection first).
|
||||
2. Phase 2 (timezone correctness).
|
||||
3. Phase 3 (dead logic cleanup).
|
||||
4. Phase 4 (logging resilience).
|
||||
5. Phase 5 (runtime defaults).
|
||||
6. Documentation + final full-suite validation.
|
||||
|
||||
## Final Validation Checklist
|
||||
- Run: `uv run --with pytest --with hypothesis pytest -q`
|
||||
- Spot-check CLI:
|
||||
- `vlm --help`
|
||||
- Confirm no regression in updated modules:
|
||||
- `src/vlm/scanner.py`
|
||||
- `src/vlm/reports.py`
|
||||
- `src/vlm/logging_config.py`
|
||||
- `src/vlm/cli.py`
|
||||
@@ -0,0 +1,93 @@
|
||||
# GEMINI.md
|
||||
|
||||
## Documentation Status
|
||||
- Synced with the post-refactor baseline on 2026-04-07 (source of truth: `CHANGELOG.md`).
|
||||
|
||||
This document provides a comprehensive overview of the Video Library Manager (VLM) project, intended to be used as instructional context for Gemini.
|
||||
|
||||
## Project Overview
|
||||
|
||||
The Video Library Manager (VLM) is a Python-based CLI tool designed for managing personal video collections. It emphasizes a "safety-first" and "human-in-the-loop" approach, ensuring that no file operations are performed without explicit user confirmation and that all actions are reversible through a rollback mechanism.
|
||||
|
||||
**Core Functionality:**
|
||||
|
||||
* **Scanning & Parsing:** Discovers video files, extracts metadata. Proactively detects `ffprobe` for video properties with graceful fallback to file-level metadata. Parses filenames for titles, years, seasons, and episodes, with specific support for Anime-style hyphenated numbering and release group stripping.
|
||||
* **Metadata Enrichment:** Augments local data with information from TMDB, including bilingual titles and reputation scores. It uses a local SQLite cache to improve performance.
|
||||
* **Analysis:** Detects duplicate files (with quality comparisons) and identifies gaps in TV series episodes.
|
||||
* **Planning & Execution:** Generates a reviewable JSON-based execution plan for file operations (move, rename, quarantine), validates move/rename sources and destinations against `library_root` during execution, and keeps execution failures isolated per operation.
|
||||
* **Plan Review Cycle:** Exports high-risk operations to CSV for manual confirmation (`review-plan`), offers an optional Textual TUI via `review-plan --tui`, and synchronizes user decisions back to the master plan (`apply-review`).
|
||||
* **Quarantine Management:** Safely isolates files for review, with full support for listing and restoration.
|
||||
* **Reporting:** Creates reports for inventory, duplicate files, and series completeness.
|
||||
* **State Management:** Tracks the status of files throughout the organization workflow.
|
||||
|
||||
**Technologies:**
|
||||
|
||||
* **Language:** Python 3.10+
|
||||
* **CLI Framework:** Click
|
||||
* **Configuration:** YAML
|
||||
* **Dependencies:** `pyyaml`, `click`
|
||||
* **Development:** `pytest` for testing, `hypothesis` for property-based testing.
|
||||
* **Package Management:** `uv` is mentioned in the documentation.
|
||||
|
||||
**Architecture:**
|
||||
|
||||
The project follows a modular structure located in the `src/vlm` directory.
|
||||
|
||||
* `cli.py`: The main entry point for the CLI, using Click.
|
||||
* `commands/*.py`: Implementation of the individual CLI commands (scan, parse, enrich, analyze, plan, execute/rollback).
|
||||
* `scanner.py`, `parser.py`, `enrichment.py`, `analysis.py`, `planner.py`, `executor.py`: Core logic for the different stages of the workflow.
|
||||
* `io.py`: Unified I/O layer for JSON and CSV handling, including validated typed plan loading.
|
||||
* `cache.py`: Local SQLite cache for TMDB metadata.
|
||||
* `context.py`: CLI context and state management for command execution.
|
||||
* `duplicate_resolve.py`: Logic for resolving duplicate files with explicit failure on unsupported or ambiguous inputs.
|
||||
* `logging_config.py`: Centralized logging configuration.
|
||||
* `plan_review.py`: Risk analysis and manual review generation for execution plans.
|
||||
* `quarantine.py`: Management of quarantined files (listing, adding, restoring).
|
||||
* `reports.py`: Generation of inventory, completeness, and duplicate reports.
|
||||
* `state.py`: File status tracking and persistence (reviewed, ignored, planned, etc.).
|
||||
* `providers/tmdb.py`: Client for interacting with the TMDB API.
|
||||
* `models.py`: Defines the data structures used throughout the application.
|
||||
* `config.py`: Manages application configuration from a YAML file.
|
||||
* `utils.py`: General utility functions (formatting, path handling).
|
||||
|
||||
## Building and Running
|
||||
|
||||
The project uses `uv` for dependency management.
|
||||
|
||||
**Installation:**
|
||||
|
||||
* Install dependencies: `uv pip install -e .`
|
||||
* Install development dependencies: `uv pip install -e ".[dev]"`
|
||||
* Install the optional Textual review UI: `uv pip install -e ".[tui]"`
|
||||
|
||||
**Running the application:**
|
||||
|
||||
The main entry point is the `vlm` command.
|
||||
|
||||
* Initialize configuration: `uv run vlm config init`
|
||||
* Scan the library: `uv run vlm scan`
|
||||
* Parse filenames: `uv run vlm parse`
|
||||
* Enrich metadata: `uv run vlm enrich`
|
||||
* Analyze the library: `uv run vlm analyze`
|
||||
* Generate a plan: `uv run vlm plan`
|
||||
* Review a plan: `uv run vlm review-plan`
|
||||
* Review a plan in the optional TUI: `uv run vlm review-plan --tui`
|
||||
* Apply edited review CSV decisions: `uv run vlm apply-review`
|
||||
* Execute the plan (dry-run): `uv run vlm execute`
|
||||
* Execute the plan (with confirmation): `uv run vlm execute --confirm`
|
||||
* Rollback the last execution: `uv run vlm rollback`
|
||||
* Generate reports: `uv run vlm report [inventory|completeness|duplicates|summary]`
|
||||
* Manage quarantine: `uv run vlm quarantine [list|add|restore]`
|
||||
* Manage file states: `uv run vlm state [show|set|query|clear]`
|
||||
|
||||
**Running tests:**
|
||||
|
||||
* Run all tests: `uv run pytest`
|
||||
|
||||
## Development Conventions
|
||||
|
||||
* **Code Style:** Adheres to PEP 8, uses 4-space indentation, and includes type hints for public functions.
|
||||
* **Testing:** Tests are located in the `tests/` directory and written using `pytest`. The project also uses `hypothesis` for property-based testing. New features should be accompanied by tests.
|
||||
* **Commits:** Commit messages should be clear and descriptive.
|
||||
* **Documentation:** The `README.md` file is very comprehensive and should be kept up-to-date.
|
||||
* **Safety:** A core principle is safety. Changes to the filesystem should be gated behind user confirmation (`--confirm`) and be reversible. The tool should never permanently delete files.
|
||||
@@ -0,0 +1,72 @@
|
||||
> [!NOTE]
|
||||
> Status: Historical snapshot. Current refactor results and validated baseline are tracked in `CHANGELOG.md` (updated 2026-04-07).
|
||||
|
||||
# VLM 改进实施计划（2026-02-13）
|
||||
|
||||
## 目标
|
||||
|
||||
基于 `IMPROVEMENT_RECOMMENDATIONS_2026-02-13.md` 落地高优先级修复，降低误解析与误搬运风险，并把“执行前人工复核”流程产品化。
|
||||
|
||||
## 范围
|
||||
|
||||
1. `parser`：修复 `XXxYY` 与分辨率串误匹配；优化标题规范化策略。
|
||||
2. `planner/config`：新增季集号阈值保护；默认排除 `Sample` 文件。
|
||||
3. `duplicate_resolve`：在保留策略中降低 `Sample` 优先级。
|
||||
4. `executor/cli`：dry-run 默认降噪，增加可选 `--verbose-ops`。
|
||||
5. `cli`：新增 `review-plan` 命令，输出风险复核 CSV。
|
||||
6. `skill`：补充 `vlm`/`uv run vlm` 回退与执行前风险闸门。
|
||||
|
||||
## 任务拆解
|
||||
|
||||
1. `T1` 配置扩展
|
||||
- 增加 `plan.max_season`、`plan.max_episode`、`plan.include_sample_files`。
|
||||
- 增加配置读写与校验。
|
||||
|
||||
2. `T2` 解析修复
|
||||
- 将 `(\d{1,2})x(\d{1,2})` 改为边界敏感匹配。
|
||||
- 增加针对 `1920x1080`/`1440x1080` 的回归测试。
|
||||
- 调整 `normalize_title` 为“仅清洗不强制 title-case”。
|
||||
|
||||
3. `T3` 规划闸门
|
||||
- 在 `_create_series_operation` 增加季集号阈值拦截。
|
||||
- 在 `_create_operation` 增加 `Sample` 排除（默认 no-op）。
|
||||
- 保持 `summary_by_reason` 可见拦截原因。
|
||||
|
||||
4. `T4` 重复保留策略
|
||||
- 在 `by_quality` / `by_reputation` 下对 `Sample` 降权。
|
||||
|
||||
5. `T5` 执行日志优化
|
||||
- dry-run 逐条操作日志默认改为 `DEBUG`。
|
||||
- CLI 增加 `--verbose-ops` 显式输出全量 dry-run 操作日志。
|
||||
|
||||
6. `T6` 计划体检命令
|
||||
- 新增 `vlm review-plan --input plan.json --output plan_manual_review.csv`。
|
||||
- 输出高风险汇总与明细（manual review / sample / 异常季集号 / conflict）。
|
||||
|
||||
7. `T7` Skill 文档更新
|
||||
- 增加命令回退策略（`vlm` 不可用时切换 `uv run vlm`）。
|
||||
- 固化执行前风险闸门流程。
|
||||
|
||||
8. `T8` 测试与验收
|
||||
- 补充 parser/planner/config/duplicate/cli/executor 相关测试。
|
||||
- 跑定向测试与全量回归。
|
||||
|
||||
## 执行状态
|
||||
|
||||
1. `[x]` `T1` 配置扩展完成。
|
||||
2. `[x]` `T2` 解析修复完成。
|
||||
3. `[x]` `T3` 规划闸门完成。
|
||||
4. `[x]` `T4` 重复保留策略 sample 降权完成。
|
||||
5. `[x]` `T5` dry-run 日志降噪 + `--verbose-ops` 完成。
|
||||
6. `[x]` `T6` `review-plan` 命令完成。
|
||||
7. `[x]` `T7` skill 文档更新完成。
|
||||
8. `[x]` `T8` 测试通过（定向 + 全量）。
|
||||
|
||||
## 验收标准
|
||||
|
||||
1. `1920x1080` 不再被解析为 `S20E10`。
|
||||
2. `season > max_season` 或 `episode > max_episode` 不生成 move/rename。
|
||||
3. 默认配置下 `Sample` 文件不进入搬运。
|
||||
4. `review-plan` 能生成 CSV 并输出风险统计。
|
||||
5. dry-run 默认日志显著降噪，开启 `--verbose-ops` 可恢复逐条日志。
|
||||
6. 新增与受影响测试通过。
|
||||
@@ -0,0 +1,231 @@
|
||||
> [!NOTE]
|
||||
> Status: Historical snapshot. Current refactor results and validated baseline are tracked in `CHANGELOG.md` (updated 2026-04-07).
|
||||
|
||||
# VLM 项目与 Skill 改进建议（2026-02-13）
|
||||
|
||||
## 1. 评估范围与依据
|
||||
|
||||
本建议基于以下真实运行结果与代码阅读：
|
||||
|
||||
1. 实际整理流程：`scan -> parse -> analyze -> plan -> execute(dry-run)`。
|
||||
2. 实际产物：`inventory.csv`、`identities.json`、`analysis.json`、`plan.json`、`plan_manual_review.csv`。
|
||||
3. 关键代码：`src/vlm/parser.py`、`src/vlm/planner.py`、`src/vlm/cli.py`、`src/vlm/executor.py`、`skills/vlm-library-workflow/SKILL.md`。
|
||||
|
||||
运行中观测到：
|
||||
|
||||
1. `plan.json` 总操作 `4710`，其中 `move=1623`、`quarantine=187`、`no-op=2900`。
|
||||
2. 人工复核候选 `516` 条，其中：
|
||||
1. `manual_review=349`
|
||||
2. `source_is_sample=92`
|
||||
3. `season>=20=75`
|
||||
4. `episode>=40=27`
|
||||
5. `truncated_title_pattern=8`
|
||||
|
||||
这说明当前系统“可用”，但在真实复杂片源命名下，仍存在高风险误解析与误搬运窗口。
|
||||
|
||||
## 2. 代码层改进（按优先级）
|
||||
|
||||
## P0（必须先做）
|
||||
|
||||
### 2.1 修复 `parse_series` 的 `XXxYY` 误匹配分辨率问题
|
||||
|
||||
现象：
|
||||
|
||||
1. `1440x1080`、`1920x1080` 会被局部匹配成 `40x10`、`20x10`，导致生成 `Season 40/S40E10`、`Season 20/S20E10` 这类错误目标路径。
|
||||
|
||||
根因位置：
|
||||
|
||||
1. `src/vlm/parser.py:197`，模式 `(\d{1,2})x(\d{1,2})` 过于宽松且允许子串匹配。
|
||||
|
||||
改进建议：
|
||||
|
||||
1. 改为边界敏感模式，例如 `(?<!\d)(\d{1,2})x(\d{1,2})(?!\d)`。
|
||||
2. 增加“分辨率语境排除”规则：若匹配附近存在 `1080/2160/720/480/p` 等特征，直接拒绝该候选。
|
||||
3. 对 `x` 模式降置信度或增加二次校验（例如 title 长度、是否含明显编码标签）。
|
||||
|
||||
验收标准：
|
||||
|
||||
1. `1920x1080`、`1440x1080` 不得触发季集解析。
|
||||
2. 新增回归测试覆盖上述文件名。
|
||||
|
||||
### 2.2 在规划阶段阻断明显异常季集号
|
||||
|
||||
现象：
|
||||
|
||||
1. 即便解析错误，`planner` 仍直接生成 `move` 操作，带来误改风险。
|
||||
|
||||
根因位置：
|
||||
|
||||
1. `src/vlm/planner.py:300-372`，仅检查 `needs_review/season is None/episodes empty`，未对极端季集号做保护。
|
||||
|
||||
改进建议：
|
||||
|
||||
1. 在 `_create_series_operation` 增加阈值守卫（例如 `season > 15` 或 `episode > 100` 时改为 `no-op` 并标记 `manual review`）。
|
||||
2. 阈值放入配置（如 `plan.max_season`, `plan.max_episode`），默认启用。
|
||||
|
||||
验收标准：
|
||||
|
||||
1. 出现异常季集号时，不得进入 `move/rename`。
|
||||
2. `plan.json` 的 `summary_by_reason` 可见明确拦截原因。
|
||||
|
||||
### 2.3 `Sample` 片段默认不应参与主文件整理
|
||||
|
||||
现象：
|
||||
|
||||
1. `Sample` 文件被移动为正片目标名，存在“样片覆盖正片语义”的风险。
|
||||
|
||||
根因位置：
|
||||
|
||||
1. 解析/规划阶段未区分样片；`duplicate_keep` 也未惩罚 sample。
|
||||
|
||||
改进建议：
|
||||
|
||||
1. 在扫描或规划阶段识别 `Sample`（路径段或文件名）。
|
||||
2. 默认行为：`no-op` 或优先 `quarantine`，并提供开关（如 `plan.include_sample_files=false`）。
|
||||
3. 在重复保留策略中将 sample 质量权重降到最低。
|
||||
|
||||
验收标准：
|
||||
|
||||
1. sample 不再成为默认保留/搬运目标。
|
||||
2. 增加针对 `.../Sample/...` 的单元测试与集成测试。
|
||||
|
||||
## P1（高价值）
|
||||
|
||||
### 2.4 优化标题规范化策略，避免语义损伤
|
||||
|
||||
现象：
|
||||
|
||||
1. `normalize_title()` 使用 `.title()` 会把 `CJ7 -> Cj7`、`R.I.P.D -> R I P D`，并可能对中日文混排标题产生副作用。
|
||||
|
||||
根因位置：
|
||||
|
||||
1. `src/vlm/parser.py:82-84`。
|
||||
|
||||
改进建议：
|
||||
|
||||
1. 默认不做 `.title()`，只做分隔符清洗与空白归一。
|
||||
2. 增加可选 title-case 开关，仅用于纯英文场景。
|
||||
|
||||
验收标准：
|
||||
|
||||
1. 缩写与数字混排标题大小写保持稳定。
|
||||
2. 新增包含中日文与缩写的测试用例。
|
||||
|
||||
### 2.5 降低 dry-run 日志噪音，提升可审阅性
|
||||
|
||||
现象：
|
||||
|
||||
1. dry-run 会输出大量逐条日志，审阅效率低。
|
||||
|
||||
根因位置：
|
||||
|
||||
1. `src/vlm/executor.py` dry-run 对每条操作记 `INFO`。
|
||||
2. `src/vlm/cli.py:771` 已做“Sample operations”，但日志层仍会刷屏。
|
||||
|
||||
改进建议：
|
||||
|
||||
1. 默认将逐条 dry-run 日志降为 `DEBUG`。
|
||||
2. CLI 增加 `--verbose-ops` 选项，按需打印全量操作。
|
||||
|
||||
验收标准：
|
||||
|
||||
1. 默认 dry-run 输出聚合摘要 + 少量样本。
|
||||
2. 显式开启 verbose 时仍可查看全量。
|
||||
|
||||
### 2.6 提供内建“计划体检”命令
|
||||
|
||||
改进建议：
|
||||
|
||||
1. 新增 `vlm review-plan --input plan.json --output plan_manual_review.csv`。
|
||||
2. 内建规则：异常季集号、sample、标题截断、高风险字符、手工审核项统计。
|
||||
|
||||
价值：
|
||||
|
||||
1. 将当前人工脚本化能力产品化，减少执行前漏检。
|
||||
|
||||
## P2（体验与可维护性）
|
||||
|
||||
### 2.7 丰富解析模式（尤其东亚命名）
|
||||
|
||||
建议新增模式：
|
||||
|
||||
1. `EP01` / `E01` 独立模式（season 缺失时可推断 `S01` 或标记低置信）。
|
||||
2. `第09話` / `第09集` 模式。
|
||||
3. `S01 Complete`、`END`、`SP` 特判策略。
|
||||
|
||||
### 2.8 用“真实语料回放测试”补齐回归
|
||||
|
||||
建议：
|
||||
|
||||
1. 在 `tests/fixtures/` 维护匿名化真实文件名样本。
|
||||
2. 新增端到端测试：`parse -> plan` 断言“异常项不得进入 move”。
|
||||
|
||||
## 3. Skill 描述改进建议（`vlm-library-workflow`）
|
||||
|
||||
当前 skill 已具备主流程与安全意识，但还可增强“执行前闸门”与“环境兼容性”。
|
||||
|
||||
### 3.1 增加命令回退策略（`vlm` 不存在时）
|
||||
|
||||
问题：
|
||||
|
||||
1. 当前文档默认 `vlm ...`，但实际环境可能仅支持 `uv run vlm ...`。
|
||||
|
||||
建议：
|
||||
|
||||
1. 在 `SKILL.md` 与 `references/command-recipes.md` 增加规则：
|
||||
1. 先试 `vlm --help`
|
||||
2. 失败则自动切换 `uv run vlm --help`
|
||||
|
||||
### 3.2 把“人工复核闸门”写入 Skill 强制流程
|
||||
|
||||
建议：
|
||||
|
||||
1. 在 `execute --confirm` 之前新增固定步骤：生成并汇报高风险清单。
|
||||
2. 若高风险计数 > 0，默认不执行确认，除非用户明确覆盖。
|
||||
|
||||
推荐规则（可直接写入 skill）：
|
||||
|
||||
1. `season>=20` 或 `episode>=40`
|
||||
2. `source_path` 包含 `Sample`
|
||||
3. `reason` 包含 `manual review`
|
||||
4. 标题截断模式（如路径中出现异常断裂片段）
|
||||
|
||||
### 3.3 在 Skill 中声明已知解析边界
|
||||
|
||||
建议：
|
||||
|
||||
1. 明确列出“当前 parser 对某些命名存在误判风险（如分辨率触发 `x` 模式）”。
|
||||
2. 遇到这些模式时，优先建议“复核并降级为 no-op”。
|
||||
|
||||
### 3.4 输出契约增加“风险摘要”
|
||||
|
||||
建议在 Output Contract 增加：
|
||||
|
||||
1. 高风险条目总数与分类计数。
|
||||
2. 前 5 条代表样本。
|
||||
3. 执行建议（继续/暂缓）。
|
||||
|
||||
## 4. 推荐实施顺序（两周节奏）
|
||||
|
||||
第 1 周（风险收敛）：
|
||||
|
||||
1. 修 `parse_series` 误匹配。
|
||||
2. `planner` 增加异常季集号保护。
|
||||
3. sample 文件默认降级处理。
|
||||
4. 增加对应单测。
|
||||
|
||||
第 2 周（体验提升）：
|
||||
|
||||
1. 加 `review-plan` 命令。
|
||||
2. 优化 dry-run 日志粒度。
|
||||
3. 迭代 `vlm-library-workflow` skill（执行前闸门 + 命令回退 + 风险摘要）。
|
||||
|
||||
## 5. 可直接转任务的条目
|
||||
|
||||
1. `parser`: 修复 `XXxYY` 与分辨率冲突，新增回归测试。
|
||||
2. `planner`: 增加 `max_season/max_episode` 风险闸门与配置项。
|
||||
3. `planner/duplicate`: sample 文件降权或默认不搬运。
|
||||
4. `cli`: 增加 `review-plan` 子命令。
|
||||
5. `executor/logging`: dry-run 默认聚合输出，逐条改为 debug。
|
||||
6. `skill`: 增加 `uv run vlm` fallback 与执行前人工复核强制步骤。
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
# Archived documentation (pre-2026-05-21 baseline)
|
||||
|
||||
These files are historical review, audit, and implementation plans from earlier refactors.
|
||||
They are **not** maintained as current project documentation.
|
||||
|
||||
**Canonical docs (use these instead):**
|
||||
|
||||
- `/README.md` — user guide and workflow
|
||||
- `/CHANGELOG.md` — release and refactor history
|
||||
- `/CLAUDE.md` / `/AGENTS.md` — agent/developer guidance
|
||||
- `/plans/2026-05-21-functional-code-simplification-plan-v1.md` — code simplification plan
|
||||
|
||||
Archived on 2026-05-21 as part of the functional code trim (Phase 2).
|
||||
@@ -0,0 +1,155 @@
|
||||
# Code & Documentation Review Report
|
||||
|
||||
**Date:** 2026-04-07
|
||||
**Reviewer:** Forge
|
||||
|
||||
## Scope
|
||||
|
||||
This report reflects the repository state after executing the review-report refactor plan and re-verifying the codebase against the updated implementation.
|
||||
|
||||
Primary verification inputs:
|
||||
- `src/vlm/cli.py:567-602`
|
||||
- `src/vlm/review_tui.py:16-30`
|
||||
- `src/vlm/executor.py:108-130`
|
||||
- `src/vlm/executor.py:204-249`
|
||||
- `src/vlm/quarantine.py:116-140`
|
||||
- `src/vlm/duplicate_resolve.py:16-58`
|
||||
- `src/vlm/planner.py:42-57`
|
||||
- `src/vlm/planner.py:151-203`
|
||||
- `src/vlm/scanner.py:158-214`
|
||||
- `src/vlm/io.py:247-326`
|
||||
- `tests/test_cli_review_plan.py:237-343`
|
||||
- `tests/test_path_safety.py:76-137`
|
||||
- `tests/test_executor.py:951-1004`
|
||||
- `tests/test_duplicate_resolve.py:223-237`
|
||||
- `tests/test_planner.py:619-720`
|
||||
- `tests/test_scanner.py:140-186`
|
||||
- `tests/test_io.py:112-203`
|
||||
- `pyproject.toml:12-20`
|
||||
|
||||
Validation baseline:
|
||||
- `pytest -q` → **507 passed**.
|
||||
|
||||
---
|
||||
|
||||
## Overall Score
|
||||
|
||||
## **9.0 / 10**
|
||||
|
||||
### Score breakdown
|
||||
- **Module boundaries / pipeline:** 8.8/10
|
||||
- **Execution safety (filesystem):** 9.0/10
|
||||
- **Planning / duplicate logic:** 9.0/10
|
||||
- **Data I/O & validation:** 9.0/10
|
||||
- **Error handling consistency:** 9.0/10
|
||||
- **Test signal:** 9.2/10
|
||||
- **Dependencies:** 8.5/10
|
||||
|
||||
---
|
||||
|
||||
## Verified strengths
|
||||
|
||||
1. **Pipeline and module boundaries remain clean and explicit** (scan → parse → analyze → plan → execute).
|
||||
- `src/vlm/commands/scan.py:14-97`
|
||||
- `src/vlm/commands/parse.py:17-166`
|
||||
- `src/vlm/commands/analyze.py:25-124`
|
||||
- `src/vlm/commands/plan.py:14-112`
|
||||
- `src/vlm/commands/execute.py:37-249`
|
||||
|
||||
2. **Execution guardrails are materially stronger than the earlier review baseline.**
|
||||
- `src/vlm/executor.py:108-130`
|
||||
- `src/vlm/executor.py:204-249`
|
||||
- `src/vlm/quarantine.py:116-140`
|
||||
|
||||
3. **Duplicate handling now favors explicit outcomes over silent fallback.**
|
||||
- `src/vlm/duplicate_resolve.py:16-58`
|
||||
- `src/vlm/planner.py:42-57`
|
||||
- `src/vlm/planner.py:151-203`
|
||||
|
||||
4. **Plan loading now crosses a validated typed boundary.**
|
||||
- `src/vlm/io.py:247-326`
|
||||
|
||||
5. **Testing coverage is broad and currently green.**
|
||||
- `tests/test_cli_review_plan.py:237-343`
|
||||
- `tests/test_path_safety.py:76-137`
|
||||
- `tests/test_executor.py:951-1004`
|
||||
- `tests/test_duplicate_resolve.py:223-237`
|
||||
- `tests/test_planner.py:619-720`
|
||||
- `tests/test_scanner.py:140-186`
|
||||
- `tests/test_io.py:112-203`
|
||||
|
||||
---
|
||||
|
||||
## Status of previously reported findings
|
||||
|
||||
### F1) Move/Rename source path is not constrained to `library_root`
|
||||
|
||||
**Status:** Resolved.
|
||||
Execution now rejects move/rename operations when either the source or destination escapes the configured root.
|
||||
Evidence: `src/vlm/executor.py:204-249`, `tests/test_path_safety.py:76-137`.
|
||||
|
||||
---
|
||||
|
||||
### F2) `by_quality` silently falls back to first item on quality-data mismatch
|
||||
|
||||
**Status:** Resolved.
|
||||
The resolver now raises `DuplicateResolutionError` for missing or misaligned quality data, and the planner converts unresolved groups into explicit manual-review no-ops with metadata.
|
||||
Evidence: `src/vlm/duplicate_resolve.py:44-58`, `src/vlm/planner.py:187-203`, `tests/test_duplicate_resolve.py:223-230`, `tests/test_planner.py:619-669`.
|
||||
|
||||
---
|
||||
|
||||
### F3) Duplicate resolution join relied on exact string path matches
|
||||
|
||||
**Status:** Resolved.
|
||||
Planner duplicate matching now canonicalizes incoming path keys before lookup, including quality-comparison entries.
|
||||
Evidence: `src/vlm/planner.py:42-57`, `src/vlm/planner.py:152-169`, `tests/test_planner.py:672-720`.
|
||||
|
||||
---
|
||||
|
||||
### F4) Quarantine category rejection raised exception while execute loop lacked per-op guard
|
||||
|
||||
**Status:** Resolved.
|
||||
Unsupported quarantine categories now return failed `OperationResult`s, and batch execution wraps each operation with defensive containment so later operations still run.
|
||||
Evidence: `src/vlm/quarantine.py:116-140`, `src/vlm/executor.py:108-130`, `tests/test_quarantine.py:84-110`, `tests/test_executor.py:951-1004`.
|
||||
|
||||
---
|
||||
|
||||
### F5) `find` non-zero exit still allowed stdout parsing without an explicit contract
|
||||
|
||||
**Status:** Resolved.
|
||||
Scanner behavior is now explicit: partial stdout is accepted with a warning, while a non-zero exit with no paths yields an empty deterministic result.
|
||||
Evidence: `src/vlm/scanner.py:158-214`, `tests/test_scanner.py:140-186`.
|
||||
|
||||
---
|
||||
|
||||
### F6) Optional dependency overlap (`textual` in both `dev` and `tui`)
|
||||
|
||||
**Status:** Still present as a low-priority packaging observation.
|
||||
`textual` remains listed in both optional extras in `pyproject.toml:12-20`, but the higher-severity runtime bug is fixed because the CLI now lazily imports the TUI and `review_tui` guards Textual imports.
|
||||
Evidence: `src/vlm/cli.py:567-602`, `src/vlm/review_tui.py:16-30`, `pyproject.toml:12-20`.
|
||||
|
||||
---
|
||||
|
||||
### F7) Plan JSON validation returned a dict-typed structure at the boundary
|
||||
|
||||
**Status:** Resolved.
|
||||
The I/O layer now validates the JSON record and then constructs an `ExecutionPlan` through a single typed factory/load path.
|
||||
Evidence: `src/vlm/io.py:247-326`, `tests/test_io.py:112-203`.
|
||||
|
||||
---
|
||||
|
||||
### F8) Unknown duplicate strategy defaulted to first item
|
||||
|
||||
**Status:** Resolved.
|
||||
Unexpected low-level strategy values now raise `DuplicateResolutionError` instead of silently keeping the first duplicate.
|
||||
Evidence: `src/vlm/duplicate_resolve.py:54-58`, `tests/test_duplicate_resolve.py:233-237`.
|
||||
|
||||
---
|
||||
|
||||
## Remaining recommendation
|
||||
|
||||
If desired, the next cleanup can be limited to dependency surface polish: remove or document the duplicated `textual` declaration in `pyproject.toml:12-20`. That is now a packaging clarity issue, not a runtime correctness issue.
|
||||
|
||||
## Closing
|
||||
|
||||
The refactor plan materially improved operational safety, duplicate-resolution determinism, scanner behavior, and plan-loading discipline. The repository is currently green at **507 passing tests**, and the substantive issues from the earlier review have been addressed.
|
||||
@@ -0,0 +1,98 @@
|
||||
> [!NOTE]
|
||||
> Status: Historical snapshot. Current refactor results and validated baseline are tracked in `CHANGELOG.md` (updated 2026-04-07).
|
||||
|
||||
# TMDB Enrichment 重构执行计划
|
||||
|
||||
## 1. 目标与范围
|
||||
- 目标：提升 `vlm enrich` 在 TMDB 场景下的正确性、稳定性、可观测性。
|
||||
- 范围：`src/vlm/providers/tmdb.py`、`src/vlm/enrichment.py`、`src/vlm/cli.py`、配置与测试。
|
||||
- 非目标：不改动 CLI 命令名和现有核心参数，不引入复杂依赖。
|
||||
|
||||
## 2. 设计原则
|
||||
- 简洁优先：保留现有调用链，避免过度抽象。
|
||||
- 统计真实：`api_calls` 仅统计真实外部请求。
|
||||
- 错误可解释：区分鉴权、限流、无匹配、服务异常。
|
||||
- 向后兼容：默认配置缺省时仍可运行，行为可预测。
|
||||
|
||||
## 3. 分阶段计划
|
||||
|
||||
### 阶段 0：基线确认
|
||||
- 记录当前测试基线：
|
||||
- `uv run pytest tests/test_enrichment.py tests/test_cli_enrich.py`
|
||||
- 记录当前运行基线：
|
||||
- `uv run vlm enrich --input identities.json --refresh-all`
|
||||
- 输出基线报告（用于对比重构前后变化）。
|
||||
|
||||
### 阶段 1：TMDB Provider 重构
|
||||
- 新增 TMDB HTTP 访问层（可内聚在 provider 文件内）：
|
||||
- 统一请求构建（query/header/timeout）。
|
||||
- 统一响应解析与错误分类。
|
||||
- 错误分类与策略：
|
||||
- `401/403`：鉴权失败，停止该条 provider 请求并记录原因。
|
||||
- `404`：资源缺失，返回无匹配。
|
||||
- `429`：指数退避重试（含上限）。
|
||||
- `5xx`：有限重试，最终记录失败。
|
||||
- 保留最小调用路径：`search -> details`。
|
||||
|
||||
### 阶段 2：Enrichment 统计与语义修复
|
||||
- 统一并明确统计口径：
|
||||
- `api_calls`：真实发起的远程请求次数。
|
||||
- `enriched`：获得有效 provider 或翻译结果的记录数。
|
||||
- `skipped`：未产生 enrich 结果的记录数。
|
||||
- 增加 skip/reason 聚合（建议键）：
|
||||
- `no_key`、`no_match`、`rate_limited`、`provider_error`、`invalid_input`。
|
||||
- 保持 `needs_review` 判定逻辑稳定且可解释。
|
||||
|
||||
### 阶段 3：CLI 进度与结果展示
|
||||
- TTY：保留 `click.progressbar`。
|
||||
- 非 TTY：保留分段文本进度（每 5% 或固定步进）。
|
||||
- 结束摘要补充原因分布：
|
||||
- 示例：`Skip reasons: no_key=4632 no_match=0 provider_error=0`
|
||||
|
||||
### 阶段 4：配置与文档
|
||||
- 配置补全（默认模板）：
|
||||
- `enrichment.api_keys.tmdb`
|
||||
- `enrichment.tmdb.language`（默认 `zh-CN`）
|
||||
- `enrichment.tmdb.region`（可选）
|
||||
- `enrichment.tmdb.include_adult`（默认 `false`）
|
||||
- README 增加：
|
||||
- key 配置示例。
|
||||
- 常见错误排查（401/429/0 enriched）。
|
||||
- 小样本验证流程。
|
||||
|
||||
### 阶段 5：测试与回归
|
||||
- Provider 测试：
|
||||
- 鉴权失败、限流重试、5xx 重试、无匹配。
|
||||
- Enrichment 测试：
|
||||
- 统计口径、skip reason 聚合、无 key 场景。
|
||||
- CLI 测试：
|
||||
- 非 TTY 进度输出、摘要 reason 输出。
|
||||
- 回归测试：
|
||||
- `uv run pytest` 全量通过。
|
||||
|
||||
## 4. 任务拆解（执行顺序）
|
||||
1. Task A：实现 TMDB 请求层与错误分类。`[已完成]`
|
||||
2. Task B：重构 `TMDBProvider.enrich()` 以接入请求层。`[已完成]`
|
||||
3. Task C：重构 enrichment 统计与 reason 聚合。`[已完成]`
|
||||
4. Task D：更新 CLI 输出（进度与摘要）。`[已完成]`
|
||||
5. Task E：补全配置模型与默认配置导出。`[已完成]`
|
||||
6. Task F：补充/修复测试并回归。`[已完成]`
|
||||
7. Task G：更新 README 与变更说明。`[待执行]`
|
||||
|
||||
## 5. 验收标准
|
||||
- 功能：
|
||||
- 有 key 时可正常 enrich，统计准确。
|
||||
- 无 key 时不误报 `api_calls`，输出原因可解释。
|
||||
- 质量：
|
||||
- 新增测试覆盖关键分支，相关测试通过。
|
||||
- 无破坏性 CLI 变更，现有命令仍可用。
|
||||
- 体验：
|
||||
- 非 TTY 场景有清晰进度和失败原因摘要。
|
||||
|
||||
## 6. 执行记录模板
|
||||
每个 Task 完成后记录以下内容：
|
||||
- 变更文件：
|
||||
- 关键改动：
|
||||
- 测试命令：
|
||||
- 测试结果：
|
||||
- 风险与后续：
|
||||
@@ -0,0 +1,78 @@
|
||||
> [!NOTE]
|
||||
> Status: Historical snapshot. Current refactor results and validated baseline are tracked in `CHANGELOG.md` (updated 2026-04-07).
|
||||
|
||||
# Video Library Manager (VLM) 深度审计报告
|
||||
|
||||
**报告版本**：1.0
|
||||
**日期**：2026-02-13
|
||||
**对象**：VLM 核心系统 + VLM Agent Skills
|
||||
|
||||
---
|
||||
|
||||
## 1. 项目架构审计 (Project Core Audit)
|
||||
|
||||
### 1.1 设计哲学评价
|
||||
VLM 的核心优势在于其**防御性组织流**。将文件操作推迟到最后一个环节，并提供完整的预览和回滚机制，这在同类工具中属于顶尖的安全设计。
|
||||
|
||||
* **优点**：
|
||||
* **分阶段解耦**：扫描、解析、富集、分析、规划、执行各阶段职责明确。
|
||||
* **元数据保护**：Schema v2 引入了视频元数据嵌入，确保去重逻辑的准确性。
|
||||
* **执行安全**：强制 Dry-run 和 `.quarantine` 隔离设计，避免了不可逆的数据丢失。
|
||||
|
||||
* **识别的不足**：
|
||||
* **事务性欠缺**：文件移动与状态更新非原子化，存在崩溃后状态不一致的风险。
|
||||
* **并发瓶颈**：ffprobe 扫描与 API 富集仍为串行，大规模整理效率较低。
|
||||
* **状态集成度**：`vlm state` 尚未完全作为 `vlm plan` 的强制前置过滤条件。
|
||||
|
||||
---
|
||||
|
||||
## 2. Skill 设计审计 (Agent Skills Audit)
|
||||
|
||||
项目目前包含两个核心 Skill：`vlm-expert`（领域专家）和 `vlm-library-workflow`（工作流指南）。
|
||||
|
||||
### 2.1 Skill 职责分工
|
||||
* **vlm-expert**：侧重于 API 参考、配置说明和开发者指南。它是 Agent 的“知识库”。
|
||||
* **vlm-library-workflow**：侧重于“最佳实践”和“命令食谱（Recipes）”。它是 Agent 的“操作手册”。
|
||||
|
||||
### 2.2 Skill 的实效性
|
||||
* **优点**：通过 `references/` 目录提供了详尽的 CLI 命令参考，使得 Agent 在执行任务时能够精准调用参数，减少了试错成本。
|
||||
* **优点**：集成了项目特有的安全规则（如：执行前必须 review plan.json），将人类的业务逻辑固化为 Agent 的强制守则。
|
||||
|
||||
---
|
||||
|
||||
## 3. 在项目中创建 Skill 的合理性分析 (The "Skill-in-Project" Strategy)
|
||||
|
||||
**结论：在项目中直接创建和维护 Skill 是极其合理的，且代表了下一代“AI 友好型”开发范式。**
|
||||
|
||||
### 3.1 为什么这种做法是合理的？
|
||||
1. **版本同步 (Version Alignment)**：
|
||||
Skill 与代码库在同一个 Git 仓库中。当 CLI 命令参数发生变化时，Skill 中的参考文档同步更新，避免了 Agent 使用过时的知识来操作新版本的代码。
|
||||
2. **上下文感知 (Context Awareness)**：
|
||||
Skill 可以直接引用项目中的脚本（如 `scripts/`）或特定路径。Agent 激活技能后，能立即感知到项目特有的辅助工具。
|
||||
3. **开箱即用 (Portability)**：
|
||||
新开发者（或新 Agent 实例）克隆项目后，不仅获得了代码，还获得了一套“如何操作该项目”的智能指令集。
|
||||
4. **业务逻辑固化**：
|
||||
将“安全第一”、“先 scan 后 parse”等业务流程写进 Skill 的指令中，比写在 README 中更有效，因为 Agent 会强制遵循这些指令。
|
||||
|
||||
---
|
||||
|
||||
## 4. 综合改进建议 (Integrated Recommendations)
|
||||
|
||||
### 4.1 项目层面 (Codebase)
|
||||
* **原子化操作**：在 `executor.py` 中引入简单的事务日志。
|
||||
* **状态闭环**：在 `planner.py` 中默认过滤掉 `state.json` 中标记为 `ignored` 的文件。
|
||||
* **性能优化**：实现 `ProcessPoolExecutor` 来加速 `vlm scan` 中的 ffprobe 过程。
|
||||
|
||||
### 4.2 Skill 层面 (Intelligence)
|
||||
* **引入风险预警指令**：在 Skill 中增加一条指令：*“当计划中包含超过 20% 的删除/隔离操作时，必须要求用户进行二次确认。”*
|
||||
* **自动化测试集成**：在 Skill 中增加如何运行定向测试的 Recipe，引导 Agent 在修改代码后立即验证。
|
||||
|
||||
---
|
||||
|
||||
## 5. 总结
|
||||
|
||||
VLM 项目通过 **代码 (Python)** + **知识 (Skill)** 的双重驱动，构建了一个安全且智能的视频管理系统。在项目中维护 Skill 不仅合理，更是提升 Agent 协作效率的最佳实践。建议继续保持这种“代码与指令同源”的开发模式。
|
||||
|
||||
---
|
||||
**审计人**：Gemini CLI Agent
|
||||
**结论**：项目设计合理，建议推进性能优化与状态深度集成。
|
||||
@@ -0,0 +1,348 @@
|
||||
> [!NOTE]
|
||||
> Status: Historical snapshot. Current refactor results and validated baseline are tracked in `CHANGELOG.md` (updated 2026-04-07).
|
||||
|
||||
# Codex 架构复核报告
|
||||
|
||||
评审日期: 2026-02-13
|
||||
评审范围: `ARCHITECTURE_REVIEW.md` 逐条核验 + 全项目补充审查
|
||||
评审基线: 当前工作区代码（含 `src/vlm/`、`tests/`）
|
||||
|
||||
## 1. 结论摘要
|
||||
|
||||
- `ARCHITECTURE_REVIEW.md` 中有一部分问题判断准确，但存在明显“已被代码修复却仍判定为问题”的过期结论。
|
||||
- 当前项目确实仍有若干高优先级问题，且有至少 2 个关键问题未被该报告指出。
|
||||
- 全量测试当前通过（整改后为 `460 passed`），且已补齐异常路径与安全边界回归用例。
|
||||
|
||||
## 2. 对 ARCHITECTURE_REVIEW.md 的逐项核验
|
||||
|
||||
### 2.1 判定为“真实存在”的问题
|
||||
|
||||
1. 无统一 I/O 入口（部分命令仍在 CLI 内重复读写/解析 CSV/JSON）
|
||||
- 证据: `src/vlm/cli.py:220`, `src/vlm/cli.py:1628`, `src/vlm/cli.py:1496`
|
||||
- 影响: 解析逻辑重复，行为漂移风险，维护成本高
|
||||
|
||||
2. State 未接入主流程（scan/parse/enrich/analyze/plan/execute 未自动更新状态）
|
||||
- 证据: `StateManager` 仅用于 `vlm state *`
|
||||
- 代码位置: `src/vlm/cli.py:1731`, `src/vlm/cli.py:1797`, `src/vlm/cli.py:1849`
|
||||
|
||||
3. State 持久化非原子写
|
||||
- 证据: 直接 `open(..., "w")` 覆盖写
|
||||
- 代码位置: `src/vlm/state.py:79`
|
||||
- 风险: 崩溃/断电时 state 文件损坏
|
||||
|
||||
4. State 路径键未 canonicalize
|
||||
- 证据: `str(file_path)` 直接作为 key，未 `resolve()`
|
||||
- 代码位置: `src/vlm/state.py:117`, `src/vlm/state.py:139`
|
||||
- 影响: 软链接/相对路径导致同一文件多状态
|
||||
|
||||
5. enrichment 串行处理，配置中的并发参数未生效
|
||||
- 证据: 双层 for 串行处理 records；`enrichment_max_concurrency` 未用于调度
|
||||
- 代码位置: `src/vlm/enrichment.py:67`, `src/vlm/config.py:35`
|
||||
|
||||
6. scan 的 ffprobe 仍串行
|
||||
- 证据: 文件逐个 `_create_video_file`，其中 `extract_metadata` 为阻塞调用
|
||||
- 代码位置: `src/vlm/scanner.py:272`
|
||||
|
||||
### 2.2 判定为“部分成立”的问题
|
||||
|
||||
1. “Analyze 阶段丢失 metadata，导致 by_quality 失效”
|
||||
- 现状: 已支持 v2 `video_metadata`，且 analyze 支持合并 inventory
|
||||
- 证据: `src/vlm/io.py:64`, `src/vlm/io.py:179`, `src/vlm/commands/analyze.py:31`
|
||||
- 结论: 对 v1 输入仍可能缺质量字段；对 v2/带 inventory 路径已修复
|
||||
|
||||
2. “路径验证缺失，路径会陈旧”
|
||||
- 现状: execute 时会检查 source 是否存在
|
||||
- 证据: `src/vlm/executor.py:251`
|
||||
- 结论: “陈旧路径无保护”不准确；但仍有更深层路径安全问题（见第 3 节）
|
||||
|
||||
3. “回滚前无验证 / 非幂等”
|
||||
- 现状: rollback 为 best-effort，且会判断目标是否存在再回滚
|
||||
- 证据: `src/vlm/executor.py:523`
|
||||
- 结论: 作为增强建议成立，但不构成当前实现缺陷性 bug
|
||||
|
||||
### 2.3 判定为“不成立或已过时”的问题
|
||||
|
||||
1. “Quarantine 非原子，可能产生孤儿文件”
|
||||
- 现状: 已实现两阶段提交（pending -> move -> committed）并含恢复逻辑
|
||||
- 证据: `src/vlm/quarantine.py:190`, `src/vlm/quarantine.py:286`, `src/vlm/quarantine.py:490`
|
||||
- 结论: 该条为过时结论
|
||||
|
||||
2. “缺少 schema versioning”
|
||||
- 现状: 多个核心产物已有版本字段
|
||||
- 证据:
|
||||
- inventory 注释版本: `src/vlm/scanner.py:519`
|
||||
- identities 版本: `src/vlm/cli.py:351`
|
||||
- analysis 版本: `src/vlm/commands/analyze.py:100`
|
||||
- plan 版本: `src/vlm/planner.py:430`
|
||||
- rollback 版本: `src/vlm/executor.py:366`
|
||||
- 结论: 不成立
|
||||
|
||||
3. “配置校验时机过晚”
|
||||
- 现状: CLI 初始化时已执行 `validate_config`
|
||||
- 证据: `src/vlm/cli.py:79`
|
||||
- 结论: 不成立
|
||||
|
||||
4. “io.py 引入 scanner 导致循环依赖风险（高）”
|
||||
- 现状: 当前依赖是单向的，未形成循环
|
||||
- 证据: `src/vlm/io.py:13`
|
||||
- 结论: 现阶段仅属架构偏好建议，不是实际问题
|
||||
|
||||
## 3. 报告遗漏但实际存在的问题（重点）
|
||||
|
||||
### 3.1 高优先级: `analyze` / `plan` 的 JSON 错误分支存在 NameError
|
||||
|
||||
- 问题: `except json.JSONDecodeError` 使用了未导入的 `json`
|
||||
- 位置: `src/vlm/cli.py:628`, `src/vlm/cli.py:678`
|
||||
- 影响: 输入非法 JSON 时，用户得到 `NameError` 而不是清晰报错，故障定位困难
|
||||
- 复现: 通过 `CliRunner` 对无效 JSON 调用 `vlm analyze` / `vlm plan`，异常为 `name 'json' is not defined`
|
||||
|
||||
### 3.2 高优先级: 目标路径缺少边界约束，存在路径穿越风险
|
||||
|
||||
- 问题链路:
|
||||
- `display_title` 可由外部 enrichment 数据参与构造，未过滤路径分隔符
|
||||
- planner 直接模板拼接 destination
|
||||
- 未校验 destination 必须位于 `library_root` 内
|
||||
- 位置:
|
||||
- `src/vlm/enrichment.py:509`
|
||||
- `src/vlm/io.py:81`, `src/vlm/io.py:103`
|
||||
- `src/vlm/planner.py:212`, `src/vlm/planner.py:228`
|
||||
- 风险: 特殊标题/模板可把文件移动到库目录外
|
||||
|
||||
### 3.3 中优先级: 执行汇总统计口径重叠
|
||||
|
||||
- 问题: conflict 既计入 `failed` 又计入 `skipped`
|
||||
- 位置: `src/vlm/executor.py:318`
|
||||
- 影响: 汇总数据解释歧义，可能误导用户决策
|
||||
|
||||
### 3.4 中优先级: duplicate report 的排序信息被削弱
|
||||
|
||||
- 问题: `report duplicates` 重建 `VideoFile` 时将 `size_bytes` 固定为 0；而文本报告按 `group.files` 最大 size 排序
|
||||
- 位置: `src/vlm/cli.py:1536`, `src/vlm/reports.py:338`
|
||||
- 影响: 重复组排序质量下降（与真实文件体积不一致）
|
||||
|
||||
## 4. 设计层观察（非缺陷但建议）
|
||||
|
||||
1. 当前“读多写少 + 人工确认”主流程设计合理，安全性基线较好。
|
||||
2. `Quarantine` 两阶段提交是亮点，但 manifest 写入仍非原子文件替换，可继续增强。
|
||||
3. `io.py` 已初步形成统一入口，但 CLI 层未完全收敛，建议继续去重。
|
||||
|
||||
## 5. 建议修复优先级
|
||||
|
||||
### P0（立即）
|
||||
|
||||
1. 修复 `cli.py` 中 `analyze` / `plan` 的 `json` 未导入问题（避免异常路径误报）
|
||||
2. 增加 destination 边界校验（`destination.resolve().is_relative_to(library_root.resolve())` 等）
|
||||
3. 对标题/模板输入做路径片段净化（移除 `/`, `\\`, `..` 等）
|
||||
|
||||
### P1（近期）
|
||||
|
||||
1. State 写入改为原子落盘（临时文件 + `os.replace`）
|
||||
2. State key 统一 canonical path
|
||||
3. 收敛 CLI 里的重复 CSV/JSON 解析到 `io.py`
|
||||
|
||||
### P2（中期）
|
||||
|
||||
1. 修正执行汇总口径（`failed/skipped` 去重）
|
||||
2. 修正 duplicate report 的 size 数据来源
|
||||
3. 为 enrichment / ffprobe 引入受控并发，真正使用 `enrichment_max_concurrency`
|
||||
|
||||
## 6. 测试与证据
|
||||
|
||||
- 全量测试: `uv run pytest -q` -> `460 passed`
|
||||
- 定向回归: `tests/test_cli_json_errors.py`, `tests/test_path_safety.py`, `tests/test_state.py`, `tests/test_enrichment.py`, `tests/test_scanner.py`, `tests/test_executor.py`, `tests/test_reports.py` -> 全部通过
|
||||
- 额外手工复现:
|
||||
- 无效 JSON 调用 `vlm analyze` / `vlm plan`，触发 `NameError`（实锤缺陷）
|
||||
|
||||
## 7. 最终判断
|
||||
|
||||
- `ARCHITECTURE_REVIEW.md` 的总体方向是有价值的，但包含多个已失效结论。
|
||||
- 当前项目最需要优先处理的，不是报告里强调的旧问题，而是：
|
||||
1. CLI 异常路径 NameError
|
||||
2. 路径安全边界与标题净化
|
||||
3. State 原子写与 canonical key
|
||||
|
||||
## 8. 详细整改计划（按阶段推进）
|
||||
|
||||
### 8.1 目标与范围
|
||||
|
||||
- 目标: 在不破坏现有 CLI 行为的前提下，完成高风险缺陷修复、核心一致性改造与性能增强。
|
||||
- 范围: `src/vlm/cli.py`, `src/vlm/planner.py`, `src/vlm/io.py`, `src/vlm/enrichment.py`, `src/vlm/state.py`, `src/vlm/executor.py`, `src/vlm/reports.py`, `tests/` 对应模块。
|
||||
- 非目标: 本轮不做大规模架构重写（例如全面重构命令框架、替换数据模型体系）。
|
||||
|
||||
### 8.2 里程碑
|
||||
|
||||
1. M1（P0，1-2 天）: 修复线上风险最高的功能正确性与安全边界问题。
|
||||
2. M2（P1，3-5 天）: 完成 state 持久化可靠性与 I/O 收敛改造。
|
||||
3. M3（P2，5-8 天）: 修复统计口径、报告准确性与可控并发能力。
|
||||
4. M4（回归与发布，1-2 天）: 全量回归、文档更新、发布前检查。
|
||||
|
||||
### 8.3 任务清单（任务-交付物-验收标准）
|
||||
|
||||
#### M1 / P0（必须先做）
|
||||
|
||||
1. 任务 T1: 修复 `analyze/plan` 的 JSON 异常分支 NameError
|
||||
- 修改点: `src/vlm/cli.py`
|
||||
- 实施:
|
||||
- 在模块级引入 `json`，或将异常类型改为已存在解析函数抛出的统一异常类型
|
||||
- 保证错误信息仍是用户可读文案（非 traceback）
|
||||
- 交付物: 代码修复 + 新增/更新 CLI 测试
|
||||
- 验收标准:
|
||||
- 输入非法 JSON 时返回非 0 退出码
|
||||
- stderr 包含明确的 JSON 解析错误提示
|
||||
- 不出现 `NameError`
|
||||
|
||||
2. 任务 T2: 增加 destination 路径边界校验
|
||||
- 修改点: `src/vlm/planner.py`（必要时补充 `src/vlm/executor.py` 二次防线）
|
||||
- 实施:
|
||||
- 对最终 `destination` 与 `library_root` 做 canonical 关系校验
|
||||
- 不在库内时直接拒绝生成/执行计划
|
||||
- 交付物: 边界校验逻辑 + 错误提示 + 测试
|
||||
- 验收标准:
|
||||
- 包含 `../`、绝对路径逃逸等输入均被拦截
|
||||
- 合法路径不受影响
|
||||
|
||||
3. 任务 T3: 标题与模板片段净化（防路径注入）
|
||||
- 修改点: `src/vlm/enrichment.py`, `src/vlm/io.py`, `src/vlm/planner.py`
|
||||
- 实施:
|
||||
- 增加统一 `sanitize_path_component()`（禁止 `/`, `\\`, `..`, 控制字符）
|
||||
- 所有参与目标路径拼接的字段都经过净化
|
||||
- 交付物: 公共净化函数 + 调用点替换 + 回归测试
|
||||
- 验收标准:
|
||||
- 恶意标题不会影响目录层级
|
||||
- 原有正常标题生成路径稳定
|
||||
|
||||
#### M2 / P1（稳定性改造）
|
||||
|
||||
1. 任务 T4: State 持久化改为原子写
|
||||
- 修改点: `src/vlm/state.py`
|
||||
- 实施:
|
||||
- 使用临时文件写入 + `os.replace`
|
||||
- 明确 fsync 策略（至少文件级）
|
||||
- 交付物: 原子写实现 + 崩溃场景单测（可用 mock）
|
||||
- 验收标准:
|
||||
- 任意写入中断不会留下半截 JSON
|
||||
- 反复读写结果一致
|
||||
|
||||
2. 任务 T5: State key 统一 canonical path
|
||||
- 修改点: `src/vlm/state.py`
|
||||
- 实施:
|
||||
- key 写入前统一 `resolve()`（并处理不存在路径的兼容逻辑）
|
||||
- 读取时保留向后兼容迁移（旧 key 可识别）
|
||||
- 交付物: key 规范化 + 兼容迁移逻辑 + 测试
|
||||
- 验收标准:
|
||||
- 相对路径/软链接指向同一文件时状态一致
|
||||
- 旧 state 文件可被平滑读取
|
||||
|
||||
3. 任务 T6: 收敛 CLI 里重复 CSV/JSON 读写逻辑
|
||||
- 修改点: `src/vlm/cli.py`, `src/vlm/io.py`
|
||||
- 实施:
|
||||
- 提炼并复用 `io.py` 的统一加载/保存接口
|
||||
- CLI 仅负责参数与错误展示
|
||||
- 交付物: 去重后的调用链 + 回归测试
|
||||
- 验收标准:
|
||||
- CLI 不再手写重复解析分支
|
||||
- 解析行为在各命令一致
|
||||
|
||||
#### M3 / P2（准确性与性能）
|
||||
|
||||
1. 任务 T7: 修正执行汇总统计口径重叠
|
||||
- 修改点: `src/vlm/executor.py`
|
||||
- 实施:
|
||||
- 明确 `failed` 与 `skipped` 互斥定义
|
||||
- 调整 summary 计数逻辑与输出文案
|
||||
- 交付物: 计数逻辑修复 + 测试
|
||||
- 验收标准:
|
||||
- 相同动作不会同时计入 `failed` 与 `skipped`
|
||||
- 总数与明细可对账
|
||||
|
||||
2. 任务 T8: 修正 duplicate report 的 size 失真
|
||||
- 修改点: `src/vlm/cli.py`, `src/vlm/reports.py`
|
||||
- 实施:
|
||||
- `report duplicates` 重建模型时保留真实 `size_bytes`
|
||||
- 报告排序按真实值生效
|
||||
- 交付物: 修复 + 测试
|
||||
- 验收标准:
|
||||
- 重复组排序与真实文件大小一致
|
||||
- 旧输入格式仍可正常输出报告
|
||||
|
||||
3. 任务 T9: 引入受控并发（enrichment + ffprobe）
|
||||
- 修改点: `src/vlm/enrichment.py`, `src/vlm/scanner.py`, `src/vlm/config.py`
|
||||
- 实施:
|
||||
- 让 `enrichment_max_concurrency` 真正参与调度
|
||||
- 为 ffprobe 引入线程池/进程池并控制并发上限
|
||||
- 维持输出稳定顺序（必要时按输入索引回排）
|
||||
- 交付物: 并发实现 + 性能对比数据 + 回归测试
|
||||
- 验收标准:
|
||||
- 在中等规模样本上性能显著提升
|
||||
- 结果内容与串行版本一致
|
||||
|
||||
### 8.4 测试任务映射
|
||||
|
||||
1. T1 对应测试
|
||||
- `tests/test_cli_parse.py` 新增: 非法 JSON 输入到 `analyze/plan` 的错误断言
|
||||
|
||||
2. T2/T3 对应测试
|
||||
- 新增 `tests/test_path_safety.py`:
|
||||
- 标题含 `../`、`/`、`\\`、控制字符
|
||||
- 模板路径逃逸
|
||||
- `destination` 越界拒绝
|
||||
|
||||
3. T4/T5 对应测试
|
||||
- `tests/test_cli_state.py` 或新增 `tests/test_state_atomic.py`:
|
||||
- 原子写完整性
|
||||
- canonical key 去重
|
||||
- 旧格式兼容
|
||||
|
||||
4. T6 对应测试
|
||||
- `tests/test_cli_*.py` 补充接口一致性断言，验证不同命令对同类输入行为一致
|
||||
|
||||
5. T7/T8 对应测试
|
||||
- `tests/test_executor.py`: summary 口径互斥
|
||||
- `tests/test_duplicate_quality_metadata.py` 或新增 `tests/test_reports_duplicates.py`: size 排序正确性
|
||||
|
||||
6. T9 对应测试
|
||||
- 新增 `tests/test_enrichment_concurrency.py`, `tests/test_scanner_concurrency.py`（含顺序稳定性与结果一致性）
|
||||
|
||||
### 8.5 执行顺序与依赖关系
|
||||
|
||||
1. 先做 T1/T2/T3（阻断高风险故障）。
|
||||
2. 再做 T4/T5（保证持久化可靠性），T6 可与 T5 并行。
|
||||
3. 最后执行 T7/T8/T9（不影响主流程安全前提下优化准确性和性能）。
|
||||
4. 每个里程碑结束后运行一次全量 `uv run pytest -q`，M4 前补充一次端到端 CLI 手工回归。
|
||||
|
||||
### 8.6 Definition of Done（完成标准）
|
||||
|
||||
1. 所有 P0/P1 任务完成并合并，P2 至少完成 T7/T8。
|
||||
2. 新增测试全部通过，且全量测试保持通过。
|
||||
3. 风险点有对应文档说明（错误码、限制行为、兼容策略）。
|
||||
4. `README.md` 或对应命令帮助文档完成必要更新。
|
||||
|
||||
## 9. 实施结果（本轮已完成）
|
||||
|
||||
### 9.1 任务完成状态
|
||||
|
||||
1. T1 完成: 修复 `analyze/plan` JSON 异常路径 `NameError`（模块级引入 `json`，并新增 CLI 回归测试）。
|
||||
2. T2 完成: planner 增加 destination 边界校验；executor 增加二次防线。
|
||||
3. T3 完成: 新增统一路径片段净化函数并用于标题生成与路径模板链路。
|
||||
4. T4 完成: state 存储改为临时文件 + `fsync` + `os.replace` 原子落盘。
|
||||
5. T5 完成: state key 与 `FileState.file_path` 统一 canonical path。
|
||||
6. T6 完成: 收敛 CLI 重复 I/O 逻辑，`parse/report` 相关命令复用 `io.py`。
|
||||
7. T7 完成: 执行汇总统计口径修正，`failed/skipped` 去重。
|
||||
8. T8 完成: duplicate report 重建 `VideoFile` 时保留真实 size，并修复排序依据。
|
||||
9. T9 完成: enrichment 与 scan(ffprobe) 引入受控并发，并保持结果顺序稳定。
|
||||
|
||||
### 9.2 新增/更新测试
|
||||
|
||||
1. 新增 `tests/test_cli_json_errors.py`（覆盖 analyze/plan 非法 JSON）。
|
||||
2. 新增 `tests/test_path_safety.py`（路径净化、越界拦截、执行二次防线）。
|
||||
3. 新增 `tests/test_config_concurrency.py`（并发参数校验）。
|
||||
4. 更新 `tests/test_state.py`（原子写与 canonical key）。
|
||||
5. 更新 `tests/test_scanner.py`（扫描并发行为）。
|
||||
6. 更新 `tests/test_enrichment.py`（enrichment 并发行为）。
|
||||
7. 更新 `tests/test_executor.py`（汇总口径变化断言）。
|
||||
8. 更新 `tests/test_reports.py`（quality size 排序回归）。
|
||||
|
||||
### 9.3 文档更新
|
||||
|
||||
1. `README.md` 已补充路径安全、执行防线、统计口径、并发行为说明。
|
||||
2. 本文档（`codex_review.md`）已补充实施结果与验证证据。
|
||||
Reference in New Issue
Block a user