Files
79797644e1 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>
2026-05-21 10:36:03 +08:00

15 KiB
Raw Permalink Blame History

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 串行处理 recordsenrichment_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.pyanalyze / planjson 未导入问题(避免异常路径误报)
  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. M2P13-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 二次防线)
    • 实施:
      • 对最终 destinationlibrary_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
    • 实施:
      • 明确 failedskipped 互斥定义
      • 调整 summary 计数逻辑与输出文案
    • 交付物: 计数逻辑修复 + 测试
    • 验收标准:
      • 相同动作不会同时计入 failedskipped
      • 总数与明细可对账
  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.pyenrichment 并发行为)。
  7. 更新 tests/test_executor.py(汇总口径变化断言)。
  8. 更新 tests/test_reports.pyquality size 排序回归)。

9.3 文档更新

  1. README.md 已补充路径安全、执行防线、统计口径、并发行为说明。
  2. 本文档(codex_review.md)已补充实施结果与验证证据。