Files
vrsub/docs/代码审查问题跟踪.md
T

79 lines
10 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 代码审查问题跟踪
本文件跟踪本次项目审查发现的问题、修复范围与验证结果。状态:待处理 / 处理中 / 已修复。只有完成实现与相关验证后才标记已修复;代码修改不代表已部署。
全部审查问题的修复统一在 `fix/review-improvements` 分支推进,验证完成后由用户合并到 `master`
## 缺陷清单
| ID | 优先级 | 问题 | 状态 | 验收标准 |
| --- | --- | --- | --- | --- |
| R01 | P0 | 普通删除接口可能删除批量源视频目录;清理逻辑从输入路径推导删除范围 | 已修复 | 普通接口拒绝单独删除批量 run;手动与自动清理只删除该上传任务的私有目录;视频、旁挂字幕及其他任务文件不受影响 |
| R02 | P1 | 自适应线程池限流后可能扩容;退出标记位于积压队列尾部,缩容不及时 | 已修复 | 限流不增加并发;降低目标后不再超额提交;真实积压队列验证 |
| R03 | P1 | 最后节点执行时暂停再继续会覆盖有效下载 URI;批量缺产物仍清理并完成 | 已修复 | 收尾幂等,恢复后全部必需产物可下载;缺产物不清理工作空间 |
| R04 | P1 | 环形 DAG 可通过保存校验,执行失败后仍在 QUEUED 堵塞队列 | 待处理 | 保存/发布拒绝无效 DAG;历史无效任务进入 FAILED,不阻塞后续任务 |
| R05 | P1 | 翻译按固定四行解析 SRT;补齐行数不能保证文本与时间轴对应 | 待处理 | 合法多行 cue 正确解析;按稳定 ID 回填译文并校验缺失、重复项 |
| R06 | P1 | OCR 跨空白帧合并相同字幕;临时请求失败被永久存为无文字 | 待处理 | 空白帧结束当前字幕段;无文字和可重试失败分开存档 |
| R07 | P2 | 抽帧/分块目录残留污染重跑;OCR/过滤存档无输入和参数指纹 | 待处理 | 输出减少时无旧文件混入;输入/有效参数变化时断点失效 |
| R08 | P2 | 批量任务未固定工作流版本,恢复收尾可能使用新版本定义;创建任务与明细不原子 | 待处理 | 执行与收尾使用固定版本;工作线程只看到完整的已创建任务 |
## 优化与维护清单
下列项目为优化建议,收益需按真实负载验证,尚未承诺实施顺序。
| ID | 项目 | 状态 | 验收方向 |
| --- | --- | --- | --- |
| O01 | Whisper 模型缓存及上传/批量共享 GPU 并发额度 | 待处理 | 相同配置复用模型;初始化线程安全;显存有界 |
| O02 | Whisper 分块、LLM 翻译批次断点 | 待处理 | 中断后只补未完成部分;输入与参数身份匹配 |
| O03 | VAD 流式读取、向量化 RMS | 待处理 | 与现有 RMS 结果一致;内存不随全片样本数增长;记录性能对比 |
| O04 | LLM 公共客户端、连接复用、统一限流重试 | 待处理 | 网络故障处理一致;提示词保留在各节点;避免重复计费与无界重试 |
| O05 | 字幕纠错批量化、减少重复上下文生成 | 待处理 | 保留目标 ID;固定输入上下文;真实字幕质量验证 |
| O06 | 合并媒体探测、减少 OCR 小文件,评估分段抽帧与 OCR 重叠执行 | 待处理 | 保持时间轴和落盘协议,测量 I/O 与吞吐变化 |
| O07 | 统一 SRT 解析/序列化/时间戳,移除生产代码对 tests 包的依赖 | 待处理 | 正式安装包可运行;多行 cue、空字幕、时间精度回归通过 |
| O08 | 统一运行状态、工作空间与 JSONL 存档公共能力 | 待处理 | 状态条件更新、文件归属明确、存档损坏可恢复 |
| O09 | 后台线程停止与窗口统计可靠性 | 待处理 | 停止等待可中断;不丢弃仍存活线程;并发统计一致 |
| O10 | 批量目录索引、聚合查询、数据库索引与轮询 | 待处理 | 避免每视频重复扫描目录;GET 不刷新状态更新时间;相关查询有界 |
| O11 | Whisper 解码耗时统计 | 待处理 | 消费 segments 生成器后计时,使用实际块时长计算实时倍率 |
| O12 | 校正测试断言与文档漂移 | 待处理 | 成功验证包括产物可读/可下载;对齐测试可在正确结果下通过;文档与模型/断点实现一致 |
## R01 修复记录
- 根因:普通任务删除接口未区分 `source=batch`,直接递归删除 `Path(input_uri).parent`;自动清理器也使用输入路径推导删除目录。
- 处理约定:普通删除接口拒绝批量 run(422),提示通过批量任务入口删除;上传任务只清理 `<storage>/uploads/<run_id>``<storage>/runs/<run_id>`。批量任务已有专用删除入口,保留源视频和视频旁成品。
- 测试要求:使用 `testdata/` 真实视频与字幕副本、临时数据库及私有存储;覆盖批量任务拒绝删除、正常上传清理、历史外部输入路径保护和自动清理路径保护。
- 实现:[普通删除接口](../src/wov_app/routers/apps.py) 在删除前拒绝批量 run[自动清理器](../src/wov_app/maintenance.py) 和普通接口均按私有目录布局定位清理范围。
- 回归测试:[test_run_deletion_safety.py](../tests/test_run_deletion_safety.py),覆盖五种状态的批量 run、外部输入路径与旁挂字幕保护、其他任务目录保护。
- TDD 红:`uv run pytest tests/test_run_deletion_safety.py -q --tb=short`7 failed;批量 run 被错误删除,两个外部路径用例复现媒体文件丢失。
- TDD 绿及相关回归:`uv run pytest tests/test_run_deletion_safety.py tests/test_apps_api.py tests/test_batch.py tests/test_maintenance.py -q`73 passed7.73 秒;仅有既存 Starlette/httpx 弃用警告。
- 状态:修复及验证完成,纳入 `fix/review-improvements` 分支;未部署。运行中的旧进程需加载新代码后才能获得保护。
- 后续边界:普通列表仍会显示批量 run,但点击删除会收到批量入口提示;运行中删除的统一取消/状态协调属于 O08,未在本次展开。
## R02 修复记录
- 根因:`report_failure()` 用降低后的最大值直接调用扩缩容,导致当前 1 并发扩为 19;缩容哨兵追加在 FIFO 积压队列尾部,存量任务仍按旧并发执行。
- 实现:[adaptive_pool.py](../nodes/adaptive_pool.py) 改为标准 `ThreadPoolExecutor` 复用线程,map 按目标额度有界提交并串行收集结果。移除自建工作队列与退出哨兵,限流更新和提交共享锁。
- 限流语义:只保持或降低当前额度;已发出的请求允许完成,后续提交遵守新额度;错误窗口不扩容,干净窗口逐步恢复上限。有效上限跨重试 map 保留,每批重新统计窗口与真实单任务耗时。
- 兼容行为:map 按输入顺序返回结果,worker 异常仍作为结果返回;cancel 仍只抑制进度回调,OCR 自行检测暂停。线程执行器在 map 结束时回收。
- TDD 红:3 个新增复现测试全部失败,分别观测到限流后目标 19、缩容后后续实际并发 4、错误窗口仍增加额度。
- TDD 绿及调用方回归:`uv run pytest tests/test_adaptive_pool.py tests/test_llm_filter.py tests/test_ocr_flow.py tests/test_subtitle_ocr_order_threading.py -q --tb=short`85 passed51.60 秒;包含真实 14236 帧顺序数据、OCR 暂停、过滤限流重试。
- 补充重试额度用例后:`uv run pytest tests/test_adaptive_pool.py -q`22 passed0.27 秒。连续限流后的第二轮 map 保持 1 并发,进度序号重新从 1 开始。`git diff --check` 通过。
- 状态:修复及验证完成,纳入 `fix/review-improvements` 分支;未部署。尚未对真实服务配额下的吞吐做性能结论;已发出请求的终止依赖节点自身超时。
## R03 修复记录
- 根因:最终产物原地改名后,节点输出 URI 未同步,恢复时旧路径覆盖有效最终记录;批量放置对缺失产物直接跳过,随后仍清理并标记完成。
- 实现:[scheduler.py](../src/wov_app/scheduler.py) 保留节点原文件,把命名成品复制至 run 的 finals 别名目录;时间戳固定取 run 创建时间,重复收尾路径稳定。缺少最终引用或文件时进入 FAILED。
- 兼容:旧版本已经改名但最终别名记录仍有效时复用该路径,不再覆盖为失效节点 URI;原始与最终文件均丢失时明确报错。
- 文件操作:[storage.py](../src/wov_app/storage.py) 提供同目录临时文件复制和原子替换,失败清理临时文件、保留源及原目标。[batch.py](../src/wov_app/batch.py) 先预检全部必需成品,复制成功后才清理 run 和工作空间;中途失败保留断点供重试。
- TDD 红:新增/强化的 8 个场景失败,覆盖暂停恢复链接、旧改名记录、多别名同源、缺少输出引用/文件、批量缺产物清理、节点原始 URI 保留及复制故障。
- 相关回归:`uv run pytest tests/test_finalization.py tests/test_scheduler.py tests/test_batch.py tests/test_apps_api.py tests/test_db.py -q --tb=short`97 passed3.22 秒。
- 补充批量复制故障及安全回归:`uv run pytest tests/test_finalization.py tests/test_maintenance.py tests/test_run_deletion_safety.py -q --tb=short`22 passed,5.83 秒。真实视频/字幕验证写满故障后旧成品和工作空间完整,故障解除可成功放置并清理;API 下载验证原始文件与两个最终别名均可读取。仅有既存 Starlette/httpx 弃用警告。
- 状态:修复及验证完成,纳入 `fix/review-improvements` 分支;未部署。原子性为单文件级,多文件中途失败允许部分完整成品已经更新,重试会重新放置;不承诺断电持久性。
- 剩余边界:批量固定工作流版本仍由 R08 跟踪;此修复不会自动重建已经丢失的历史成品。
## 审查基线
- 修复前全套测试:`uv run pytest`369 passed、6 skipped76.75 秒。
- 隔离复现已确认:源视频目录误删、限流后 1 → 19 并发、队列积压时缩容滞后、暂停恢复后成品 URI 失效、环形 DAG 阻塞队首、多行 SRT 损坏、OCR 跨空白合并、重用抽帧目录留下旧尾帧。
- 本文件中的“已修复”只表示当前工作区实现及验证完成;部署状态需另行记录。