diff --git a/CHANGELOG.md b/CHANGELOG.md index 35c3e237..358494a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ 格式遵循 [Keep a Changelog](https://keepachangelog.com/zh-CN/1.1.0/),版本号遵循语义化版本。 发版步骤见 [docs/release-process.md](docs/release-process.md)。 +## [Unreleased] + +### Fixed + +- 章节目录:模型按主题弧线而非时间序返回章节时(每章时间锚点正确),章节层不再整体失败,改为按时间排序归一化(排序改变顺序时记入日志,含错位数与 LIS 统计)。此前此类输出会导致整个视频无章节目录(生产事故:BV1fcHD6cEw3)。 +- 章节生成失败原因现在落盘到任务状态的 llm_status.json(chapters_error 字段),不再只出现在服务日志里;补跑成功后旧失败原因自动清除。 + ## [1.0.0] - 2026-10-10 首个版本基线,不追认历史。本条概括基线时点的对外能力与近期变更。 diff --git a/docs/sessions/chapters-sort-261015/progress/vta-261015-chapters-sort-progress.md b/docs/sessions/chapters-sort-261015/progress/vta-261015-chapters-sort-progress.md new file mode 100644 index 00000000..e2c48cc6 --- /dev/null +++ b/docs/sessions/chapters-sort-261015/progress/vta-261015-chapters-sort-progress.md @@ -0,0 +1,41 @@ +# 进度存档:章节生成 start_seg 乱序排序归一化 + chapters_error 落盘 + +Task-Id: VideoTranscriptAPI-20261015-01(dispatch dlg-20261010-101342-e71d15) + +## 里程碑 1:A 排序归一化 + +- 当前阶段:repairing(A 完成并测试通过,B 未开工) +- 本段结论:`_validate_and_normalize_start_segs` 去重后非严格递增不再判 FAILED,改为按 + start_seg 排序归一化;排序确实改变顺序时打 warning(含被移动章节数 moved、LIS 长度, + 生产样本实测 moved 6/17、LIS 14/17)。排序发生在 `_derive_end_segs` 之前,区间推导 + 自洽;已升序输入无排序、无新 warning,行为与 HEAD 一致。生产 attempt 2 样本序列 + (17 章)固化为测试输入 `tests/unit/test_chapters_processor.py::TestStartSegSortNormalization`。 +- 关键决策与已否决方案:按卡面锁定决策执行(无条件排序、无 LIS 阈值、无碎章否决、 + prompt 不改、不新增端点);统计口径选择「moved=逐位与排序结果比较的位移数」+ + LIS 长度双口径,与卡面「错位 3/17」(= 17 - LIS 口径)并存不冲突。 +- 下一步唯一动作:实现 B(save_llm_status 增加 chapters_error 参数 + GENERATED 显式 + 清空)并接线 llm_ops,配套 test_cache_manager / test_chapters_pipeline_wiring 测试。 + +## 里程碑 2:B chapters_error 落盘 + llm_ops 接线 + +- 当前阶段:repairing(A/B 实现与测试均完成,剩 CHANGELOG 与全量 make test) +- 本段结论:`save_llm_status` 新增 `chapters_error` 参数(None 不更新,非 None 写入); + 合并逻辑新增「chapters_status=generated ⇒ 强制 pop chapters_error」不变式,集中在 + cache_manager 一处,caller 误传也无法制造「已生成却显示旧错误」。llm_ops 提取 + chapters_error(顶层优先、stats 兜底)传入 save_llm_status;未触碰层(None)保留旧值 + 语义不变。契约测试 4 个(FAILED 落盘 / 补跑清除 / 误传仍清除 / 未触碰保留)+ 接线 + 测试 3 个(FAILED 传递 / stats 兜底 / GENERATED 传 None)。 +- 关键决策与已否决方案:GENERATED 清空放 save_llm_status 内部而非 caller 侧——不变式 + 单点锁定,任何 caller 都无法绕过(锁定决策 4);skipped_*/disabled 状态不清旧 + error(轴表只约束 GENERATED 清除,扩大清空范围属超卡面行为变更,报告里备注)。 +- 下一步唯一动作:写 CHANGELOG Fixed 条目,跑 make test 全量门禁,写报告。 + +## 里程碑 3:收尾(CHANGELOG + 全量门禁) + +- 当前阶段:验收中(A/B 已提交并各自红验通过,CHANGELOG 已写,全量门禁已过) +- 本段结论:CHANGELOG 新增 [Unreleased] Fixed 两条(排序归一化、chapters_error 落盘)。 + make test 全量门禁 exit=0(TMPDIR=/tmp;默认 TMP 下 test_unix_domain_socket_is_not_blocked + 因本 worktree 根路径 88 字符致 mkdtemp socket 路径超 AF_UNIX 108 字节限制而失败, + 该测试文件自 2026-10-02 d9f2e2c6 未变,属环境性继承红,与本次改动无关)。 +- 关键决策与已否决方案:无 +- 下一步唯一动作:写 report.md 交验收。 diff --git a/docs/sessions/chapters-sort-261015/reviews/VideoTranscriptAPI-20261015-01-verdict.md b/docs/sessions/chapters-sort-261015/reviews/VideoTranscriptAPI-20261015-01-verdict.md new file mode 100644 index 00000000..2fd21d10 --- /dev/null +++ b/docs/sessions/chapters-sort-261015/reviews/VideoTranscriptAPI-20261015-01-verdict.md @@ -0,0 +1,49 @@ +# 独立 Review Verdict:章节排序归一化与 chapters_error 落盘 + +审查对象:`30466452..819b6f8435d191ba3fad9734f5d757d28295521e`(冻结 H0) +风险档位:personal +结论:**继续**(p2-only,不阻断交付) +failure-visibility: p2-only + +## Findings + +### P2 — 两个单调用统计 helper 没有第二消费者 + +- 位置:`src/video_transcript_api/llm/processors/chapters_processor.py:631`、`src/video_transcript_api/llm/processors/chapters_processor.py:642`、调用点 `src/video_transcript_api/llm/processors/chapters_processor.py:787`。 +- 违反:卡面反熵条款要求对新增符号检查第二消费者;全局约束也要求新增 helper 有已发生的失败或第二调用方。`rg` 全仓结果显示 `_count_moved_entries` 与 `_longest_increasing_subsequence` 各只有该归一化分支一个生产调用点,未发现既有失败需要这层抽象。 +- 影响:不影响归一化结果或 warning 数值;增加两个仅服务一个分支的符号与维护面。作为低风险清理项,不阻断交付;可接受不修,若修则可把统计内联到该分支。 + +## P1 判定 + +无 P1 finding,因此无需执行 P1 两问。唯一 finding 为 P2,personal 风险下不阻断交付。 + +## I1–I10 不变式核对 + +- **I1 通过**:归一化排序在 `src/video_transcript_api/llm/processors/chapters_processor.py:795`,区间推导在 `src/video_transcript_api/llm/processors/chapters_processor.py:1328`;生产样本测试还断言排序后区间连续且末章覆盖最后下标(`tests/unit/test_chapters_processor.py:1040`)。 +- **I2 通过**:排序移动完整字典记录,仅以 `start_seg` 为键(`src/video_transcript_api/llm/processors/chapters_processor.py:779`、`src/video_transcript_api/llm/processors/chapters_processor.py:795`);生产样本测试比较完整 `(title, gist, start_seg)` 集合(`tests/unit/test_chapters_processor.py:1033`)。 +- **I3 通过**:仅 `is_sorted` 为假时计算统计、告警并排序(`src/video_transcript_api/llm/processors/chapters_processor.py:782`);已排序输入测试断言顺序不变、只调用一次且没有排序 warning(`tests/unit/test_chapters_processor.py:1077`)。 +- **I4 通过**:乱序 warning 同时记录 moved 数与 LIS 长度(`src/video_transcript_api/llm/processors/chapters_processor.py:789`);生产序列测试断言 `moved 6/17`、`LIS 14/17`(`tests/unit/test_chapters_processor.py:1056`)。 +- **I5 通过**:幸存下标成员校验仍在排序前执行(`src/video_transcript_api/llm/processors/chapters_processor.py:742`);去重保留首次(`src/video_transcript_api/llm/processors/chapters_processor.py:771`);title/gist 非空校验(`src/video_transcript_api/llm/processors/chapters_processor.py:749`、`src/video_transcript_api/llm/processors/chapters_processor.py:756`)、首章钳位(`src/video_transcript_api/llm/processors/chapters_processor.py:799`)和章节数量边界(`src/video_transcript_api/llm/processors/chapters_processor.py:1314`)均保留。测试继续覆盖越界、重复、钳位与空 title/gist(`tests/unit/test_chapters_processor.py:890`、`tests/unit/test_chapters_processor.py:970`、`tests/unit/test_chapters_processor.py:1120`)。 +- **I6 通过**:指纹仍基于输入 segments 而非章节数组顺序(`src/video_transcript_api/llm/processors/chapters_processor.py:283`、`src/video_transcript_api/llm/processors/chapters_processor.py:1133`);排序后仍先推导 end/time,再合并相邻同名章(`src/video_transcript_api/llm/processors/chapters_processor.py:1328`、`src/video_transcript_api/llm/processors/chapters_processor.py:1332`)。内嵌章节头按 `start_seg` 对齐原始 dialog 下标(`src/video_transcript_api/utils/rendering/dialog_renderer.py:439`、`src/video_transcript_api/utils/rendering/dialog_renderer.py:525`);笔记切片按闭区间下标取原始 segments(`src/video_transcript_api/llm/processors/notes_processor.py:188`、`src/video_transcript_api/llm/processors/notes_processor.py:272`)。生产样本的配对与区间测试通过。 +- **I7 通过**:Coordinator 把 `ChaptersResult.error` 放入 stats(`src/video_transcript_api/llm/coordinator.py:332`、`src/video_transcript_api/llm/coordinator.py:388`);llm_ops 顶层优先、stats 兜底后传给状态写入(`src/video_transcript_api/api/services/llm_ops.py:1990`、`src/video_transcript_api/api/services/llm_ops.py:2489`);CacheManager 将非 None 值写入合并状态(`src/video_transcript_api/cache/cache_manager.py:2062`)。测试读取实际写出的 `llm_status.json` 并断言失败状态与错误字段(`tests/unit/test_cache_manager.py:1797`)。 +- **I8 通过**:CacheManager 在写入 error 后按 `chapters_status == "generated"` 强制删除该字段(`src/video_transcript_api/cache/cache_manager.py:2064`、`src/video_transcript_api/cache/cache_manager.py:2070`);测试覆盖 FAILED 补跑清除及 caller 误传仍清除(`tests/unit/test_cache_manager.py:1810`、`tests/unit/test_cache_manager.py:1824`)。 +- **I9 通过**:未触碰章节层时 `chapters_status` 与 `chapters_error` 都从撤销前快照回填(`src/video_transcript_api/api/services/llm_ops.py:2396`、`src/video_transcript_api/api/services/llm_ops.py:2410`);resummarize 接线测试断言两者均保留(`tests/unit/test_chapters_pipeline_wiring.py:282`)。generate_notes 只更新 `notes_status`(`src/video_transcript_api/api/services/llm_ops.py:473`),CacheManager 的 None 合并语义保留旧 error,且有实际文件断言(`tests/unit/test_cache_manager.py:1842`)。 +- **I10 通过**:字段仍只在非 None 时更新,并且读改写全程处于 `media_lock` 内(`src/video_transcript_api/cache/cache_manager.py:2031`、`src/video_transcript_api/cache/cache_manager.py:2054`);未触碰测试验证旧状态与 error 均保留(`tests/unit/test_cache_manager.py:1842`)。 + +## 必查项与验证 + +- 测试断言抽查:新增排序测试用具体的序列、配对、区间、warning 数值断言;落盘测试读取实际 JSON。未发现恒真断言。 +- 静默失败候选扫描:新增生产行未命中吞异常、异常后返回空值/成功等候选;扫描唯一命中 `if not is_sorted` 是正常排序分支。 +- 新符号检查:两个统计 helper 只有一个生产调用方;已作为上方 P2 记录。 +- `git diff --check 30466452..819b6f84`:通过,退出码 0。 +- `uv run pytest tests/unit/test_chapters_pipeline_wiring.py tests/unit/test_cache_manager.py tests/unit/test_chapters_processor.py -p no:warnings`:`261 passed in 2.68s`。 +- 主干 CI 基线:卡面记录 `gh api request failed`;继承红未能判定。本轮本地 Narrow-Verify 未观察到新红。 +- OCR 前置扫描:`skipped`。OCR 工具可用,但卡面禁止有费用的网络调用,本轮未请求外部模型;该 skipped 不等同于扫过且干净。 + +## 下一轮输入 + +无。审查时远端实现分支仍指向冻结 H0 `819b6f8435d191ba3fad9734f5d757d28295521e`,未发现 H0 之后的新提交。 + +## Backlog + +无其他与本轮 diff 相关的存量问题。 diff --git a/src/video_transcript_api/api/services/llm_ops.py b/src/video_transcript_api/api/services/llm_ops.py index 3551cd95..c6d709a5 100644 --- a/src/video_transcript_api/api/services/llm_ops.py +++ b/src/video_transcript_api/api/services/llm_ops.py @@ -1983,6 +1983,13 @@ def _save_llm_results( or stats.get("chapters_source_kind") or "none" ) + # chapters_error:章节层失败原因原文(协调器把它写进 stats["chapters_error"], + # GENERATED 时为 None、FAILED 时为 ChaptersResult.error)。顶层优先、stats 兼底, + # 两者都没有则视为本轮无失败信息(None → save_llm_status 合并语义不更新; + # generated 状态下的旧错误清除由 save_llm_status 内部强制完成)。 + chapters_error = result_dict.get("chapters_error") + if chapters_error is None: + chapters_error = stats.get("chapters_error") # 保存 LLM 模型配置到数据库 if models_used: @@ -2391,6 +2398,20 @@ def _save_llm_results( if effective_chapters_status is not None else old_llm_status.get("chapters_status") ) + # chapters_error 回填(与上面 final_* 同构,三分支语义): + # 1) 本轮尝试章节层(effective 非 None)→ 用本轮值;GENERATED 时本轮值 + # 为 None,旧错误清除由 cache_manager 内部 GENERATED⇒pop 强制完成, + # 此处不变动该不变式; + # 2) 本轮未触碰章节层(resummarize / generate_notes 等)→ 回填旧值: + # 上面 write-ahead 撤销已删除磁盘旧状态文件,此处若传 None, + # save_llm_status 的合并语义无旧值可保,历史失败诊断会被静默丢失, + # 违反「FAILED 时 llm_status.json 含 chapters_error」的落盘契约; + # 3) 旧值缺失(存量数据无 chapters_error 字段)→ None,与现状一致。 + final_chapters_error = ( + chapters_error + if effective_chapters_status is not None + else old_llm_status.get("chapters_error") + ) if calibrated_saved or summary_saved: def record_saved_artifact(layer: str, filename: str) -> Optional[dict]: @@ -2465,6 +2486,7 @@ def record_saved_artifact(layer: str, filename: str) -> Optional[dict]: "calibration_stats": final_calibration_stats, "summary_status": final_summary_status, "chapters_status": final_chapters_status, + "chapters_error": final_chapters_error, } if final_artifact_provenance or old_artifact_provenance is not None: status_updates["artifact_provenance"] = final_artifact_provenance diff --git a/src/video_transcript_api/cache/cache_manager.py b/src/video_transcript_api/cache/cache_manager.py index 7afdfecf..b930da73 100644 --- a/src/video_transcript_api/cache/cache_manager.py +++ b/src/video_transcript_api/cache/cache_manager.py @@ -1998,6 +1998,7 @@ def save_llm_status( chapters_status: Optional[str] = None, notes_status: Optional[str] = None, artifact_provenance: Optional[Dict[str, Any]] = None, + chapters_error: Optional[str] = None, ) -> Dict[str, Any]: """写入/合并 llm_status.json("诚实状态模型"统一落盘文件)。 @@ -2019,6 +2020,9 @@ def save_llm_status( skipped_no_timeline/failed/pending/disabled),None 表示不更新 artifact_provenance: 校对/总结来源记录;None 表示不更新,空字典表示清除 整个可选记录,非空字典在同一次原子状态写入中替换旧记录 + chapters_error: 章节层失败原因原文(ChaptersResult.error),None 表示 + 不更新;此外 chapters_status=generated 时无论是否传入都会强制 + 清空旧 chapters_error(见下方合并逻辑注释) Returns: dict: 锁内完成写入后的完整合并快照 @@ -2055,6 +2059,16 @@ def save_llm_status( existing['summary_status'] = summary_status if chapters_status is not None: existing['chapters_status'] = chapters_status + if chapters_error is not None: + existing['chapters_error'] = chapters_error + # GENERATED 蕴含本轮无失败:显式清空历史失败原因。不能只靠 + # 「非 None 才更新」的合并语义——从 FAILED 补跑 GENERATED 时 + # 本轮 error 为 None,旧失败原因会残留,制造「已生成却显示 + # 旧错误」的静默错。放在 error 写入之后:即使 caller 在 + # generated 状态下误传了 error 也会被清掉,不变式 + # 「generated ⇒ 无 chapters_error」集中锁在这一处。 + if chapters_status == "generated": + existing.pop('chapters_error', None) if notes_status is not None: existing['notes_status'] = notes_status if artifact_provenance is not None: diff --git a/src/video_transcript_api/llm/processors/chapters_processor.py b/src/video_transcript_api/llm/processors/chapters_processor.py index 56eb4577..5adb46e8 100644 --- a/src/video_transcript_api/llm/processors/chapters_processor.py +++ b/src/video_transcript_api/llm/processors/chapters_processor.py @@ -15,6 +15,7 @@ 密度、同名合并)都在本地代码里做,不依赖 LLM 自我把关。 """ +import bisect import hashlib import json import math @@ -627,6 +628,33 @@ def _build_retry_hint(validation_error: str) -> str: return flattened[:_RETRY_HINT_TRUNCATE_TO] + "…" +def _count_moved_entries(chapters: List[Dict[str, Any]]) -> int: + """统计按 start_seg 排序后位置会变化的章节条数(逐位与排序结果比较)。 + + 仅用于乱序归一化时的观测统计(进 warning),不参与任何判定。 + """ + sorted_segs = sorted(c["start_seg"] for c in chapters) + return sum( + 1 for c, target in zip(chapters, sorted_segs) if c["start_seg"] != target + ) + + +def _longest_increasing_subsequence(values: List[int]) -> int: + """严格递增最长子序列(LIS)长度,O(n log n) patience 实现。 + + 调用方传入的 start_seg 已去重(值互异),严格递增与非降等价。 + 仅用于乱序归一化时的观测统计(进 warning),不参与任何判定。 + """ + tails: List[int] = [] + for value in values: + pos = bisect.bisect_left(tails, value) + if pos == len(tails): + tails.append(value) + else: + tails[pos] = value + return len(tails) + + def _validate_and_normalize_start_segs( raw_chapters: Any, survived_indices: List[int], @@ -641,8 +669,14 @@ def _validate_and_normalize_start_segs( 下标可能不连续(中间的下标对应被过滤掉的条目),区间校验会放行一个 "数值上在范围内、但实际已被过滤掉"的下标(见 `Chapter` 类文档的锚点 偏移问题),必须严格按幸存下标集合的成员关系校验。 - 2. 按 start_seg 去重(保留首次出现),去重后的序列必须严格递增 - 3. 去重后若首项 start_seg 不是幸存下标里最小的那个,钳制为最小幸存下标 + 2. 按 start_seg 去重(保留首次出现);去重后若非严格递增,按 start_seg + 排序归一化(打 warning 留观测数据)而**不**判失败——数组的排列顺序 + 只是 start_seg 取值集合的冗余表示,模型按主题弧线而非时间序排列数组 + 时,各章 (title, gist, start_seg) 的配对与锚点仍然正确,整体判死会 + 把可救数据全部丢弃(生产事故:BV1fcHD6cEw3 两次因乱序 FAILED,用户 + 视图页无章节目录)。排序确实改变顺序时打 warning,内容含被移动章节 + 数与 LIS 长度,为未来评估「排序后 gist-区间错配」是否出现留样本。 + 3. 排序后若首项 start_seg 不是幸存下标里最小的那个,钳制为最小幸存下标 (覆盖开头;这是自动修正,不算校验失败)——同样不能再硬编码钳制为字面 量 0:0 本身就可能已被过滤掉,不在幸存下标集合里。 4. title、gist 必须是非空字符串(strip 后非空)——曾经缺失/非字符串/空白值 @@ -741,11 +775,24 @@ def _validate_and_normalize_start_segs( seen.add(item["start_seg"]) deduped.append(item) - # 去重后必须严格递增 - for prev, curr in zip(deduped, deduped[1:]): - if curr["start_seg"] <= prev["start_seg"]: - seq = [d["start_seg"] for d in deduped] - return None, f"start_seg sequence not strictly increasing after dedup: {seq}" + # 去重后非严格递增:排序归一化,不判失败(理由见 docstring 第 2 条)。 + # 排序必须稳定且只按 start_seg 比较键——值互异(已去重),sorted 的结果 + # 唯一确定;(title, gist, start_seg) 整体随元素移动,配对不变。已升序 + # 的输入不排序、不打 warning,行为与归一化之前完全一致。 + is_sorted = all( + curr["start_seg"] > prev["start_seg"] + for prev, curr in zip(deduped, deduped[1:]) + ) + if not is_sorted: + moved = _count_moved_entries(deduped) + lis_len = _longest_increasing_subsequence([d["start_seg"] for d in deduped]) + logger.warning( + f"[CHAPTERS] start_seg sequence not strictly increasing after dedup; " + f"normalizing by sorting: moved {moved}/{len(deduped)} chapters, " + f"LIS {lis_len}/{len(deduped)}, " + f"before {[d['start_seg'] for d in deduped]}" + ) + deduped.sort(key=lambda c: c["start_seg"]) # 首项若不是最小幸存下标,钳制为最小幸存下标(覆盖开头,不算失败)。 # 不能钳制为字面量 0——0 本身可能已被入口过滤掉、不在幸存下标集合里。 diff --git a/tests/unit/test_cache_manager.py b/tests/unit/test_cache_manager.py index 6f6dbd51..9dccfc92 100644 --- a/tests/unit/test_cache_manager.py +++ b/tests/unit/test_cache_manager.py @@ -1775,6 +1775,87 @@ def test_corrupted_status_file_does_not_crash_get_cache(self, cm, cache_dir): assert "llm_status" not in result +class TestSaveLLMStatusChaptersError: + """chapters_error persistence contract (production incident + task_1f8a978a541f40ba8ffe47b0d01e15cf): the chapters failure reason used + to live only in app.log, invisible in llm_status.json. It must now be + persisted on FAILED, and explicitly cleared when a later re-run reaches + GENERATED (a stale error next to a generated status would be a new silent + contradiction).""" + + def _write(self, cm, **kwargs): + return cm.save_llm_status( + platform="youtube", media_id="vid1", + use_speaker_recognition=False, **kwargs, + ) + + def _status_data(self, cache_dir): + files = list(cache_dir.rglob("llm_status.json")) + assert len(files) == 1 + return json.loads(files[0].read_text(encoding="utf-8")) + + def test_failed_status_persists_chapters_error(self, cm, cache_dir): + _save_sample_capswriter(cm) + self._write( + cm, + chapters_status="failed", + chapters_error="Semantic validation failed after retry: ...", + ) + data = self._status_data(cache_dir) + assert data["chapters_status"] == "failed" + assert data["chapters_error"] == ( + "Semantic validation failed after retry: ..." + ) + + def test_generated_after_failed_clears_stale_error(self, cm, cache_dir): + """Re-run reaching GENERATED must not leave the old failure reason in + llm_status.json (jq must read null / key absent).""" + _save_sample_capswriter(cm) + self._write( + cm, + chapters_status="failed", + chapters_error="Semantic validation failed after retry: ...", + ) + self._write(cm, chapters_status="generated") # re-run, no error passed + data = self._status_data(cache_dir) + assert data["chapters_status"] == "generated" + assert "chapters_error" not in data + + def test_generated_forces_clear_even_if_error_passed(self, cm, cache_dir): + """The invariant lives in save_llm_status: a caller mistakenly passing + an error alongside generated status cannot produce the contradiction.""" + _save_sample_capswriter(cm) + self._write( + cm, + chapters_status="failed", + chapters_error="Semantic validation failed after retry: ...", + ) + self._write( + cm, + chapters_status="generated", + chapters_error="stale mistake", + ) + data = self._status_data(cache_dir) + assert data["chapters_status"] == "generated" + assert "chapters_error" not in data + + def test_untouched_layers_preserve_status_and_error(self, cm, cache_dir): + """A call that touches neither chapters field (e.g. recalibrate-only) + must preserve both the old status and the old error unchanged.""" + _save_sample_capswriter(cm) + self._write( + cm, + chapters_status="failed", + chapters_error="Semantic validation failed after retry: ...", + ) + self._write(cm, summary_status="generated") + data = self._status_data(cache_dir) + assert data["chapters_status"] == "failed" + assert data["chapters_error"] == ( + "Semantic validation failed after retry: ..." + ) + + class TestInvalidateLLMStatus: """Tests for invalidate_llm_status: the write-ahead revocation primitive _save_llm_results (llm_ops.py, S1 PR3 review hardening) calls before diff --git a/tests/unit/test_chapters_pipeline_wiring.py b/tests/unit/test_chapters_pipeline_wiring.py index e0257700..7e0cd690 100644 --- a/tests/unit/test_chapters_pipeline_wiring.py +++ b/tests/unit/test_chapters_pipeline_wiring.py @@ -186,6 +186,137 @@ def test_generated_writes_chapters_file_and_status(self, monkeypatch): == ChaptersStatus.GENERATED ) + assert result["chapters_status"] == ChaptersStatus.GENERATED + assert ( + mock_cm.save_llm_status.call_args.kwargs["chapters_status"] + == ChaptersStatus.GENERATED + ) + + def test_failed_passes_chapters_error_to_save_llm_status(self, monkeypatch): + """FAILED layer must carry ChaptersResult.error into save_llm_status so + the failure reason lands in llm_status.json (not only in app.log). + Production incident task_1f8a978a: the reason was invisible on disk.""" + mock_cm = self._patch_cm(monkeypatch) + llm_ops._save_llm_results( + task_id="t-cherr-failed", + platform="youtube", + media_id="m1", + use_speaker_recognition=False, + result_dict=self._base_result( + chapters_status=ChaptersStatus.FAILED, + chapters=[], + chapters_error="Semantic validation failed after retry: boom", + ), + calibrate_only=False, + processing_options=normalize_processing_options( + {"chapters": True, "summarize": True} + ), + ) + kwargs = mock_cm.save_llm_status.call_args.kwargs + assert kwargs["chapters_status"] == ChaptersStatus.FAILED + assert kwargs["chapters_error"] == ( + "Semantic validation failed after retry: boom" + ) + + def test_chapters_error_falls_back_to_stats(self, monkeypatch): + """Coordinator writes the error into stats["chapters_error"]; a result_dict + without a top-level key must still pick it up from stats.""" + mock_cm = self._patch_cm(monkeypatch) + llm_ops._save_llm_results( + task_id="t-cherr-stats", + platform="youtube", + media_id="m1", + use_speaker_recognition=False, + result_dict=self._base_result( + chapters_status=ChaptersStatus.FAILED, + chapters=[], + stats={ + "calibration_status": "full", + "chapters_error": "from coordinator stats", + }, + ), + calibrate_only=False, + processing_options=normalize_processing_options( + {"chapters": True, "summarize": True} + ), + ) + kwargs = mock_cm.save_llm_status.call_args.kwargs + assert kwargs["chapters_error"] == "from coordinator stats" + + def test_generated_passes_none_error(self, monkeypatch): + """GENERATED round carries no error (chapters_error=None); clearing the + stale failure reason is enforced inside save_llm_status, keyed on the + generated status itself.""" + mock_cm = self._patch_cm(monkeypatch) + llm_ops._save_llm_results( + task_id="t-cherr-gen", + platform="youtube", + media_id="m1", + use_speaker_recognition=False, + result_dict=self._base_result( + chapters_status=ChaptersStatus.GENERATED, + chapters=[ + { + "index": 0, + "title": "Intro", + "gist": "Hello", + "start_seg": 0, + "end_seg": 2, + "start_time": 0.0, + "end_time": 10.0, + } + ], + chapters_fingerprint="fp1", + chapters_segment_count=3, + chapters_source_kind="segments", + ), + calibrate_only=False, + processing_options=normalize_processing_options( + {"chapters": True, "summarize": True} + ), + ) + kwargs = mock_cm.save_llm_status.call_args.kwargs + assert kwargs["chapters_status"] == ChaptersStatus.GENERATED + assert kwargs["chapters_error"] is None + + def test_untouched_layer_refills_stale_chapters_error(self, monkeypatch): + """Layer-untouched round (resummarize / generate_notes) must refill the + historical chapters_error. invalidate_llm_status is write-ahead: the old + llm_status.json is already gone when save_llm_status merges, so passing + the raw round value (None) silently drops the persisted failure + diagnostic while the refilled chapters_status stays FAILED.""" + old = { + "chapters_status": ChaptersStatus.FAILED, + "chapters_error": "stale semantic validation failure", + } + snapshot = { + "file_path": "/tmp/cache/x", + "llm_chapters": {"chapters": [{"title": "Old"}]}, + "llm_status": old, + } + mock_cm = self._patch_cm(monkeypatch, snapshot=snapshot, old_status=old) + result = llm_ops._save_llm_results( + task_id="t-cherr-refill", + platform="youtube", + media_id="m1", + use_speaker_recognition=False, + result_dict=self._base_result( + chapters_status=None, + chapters=[], + ), + calibrate_only=False, + processing_options={ + "calibrate": False, + "summarize": True, + "infer_speaker_names": False, + "chapters": False, + }, + ) + assert result["chapters_status"] is None + kwargs = mock_cm.save_llm_status.call_args.kwargs + assert kwargs["chapters_status"] == ChaptersStatus.FAILED + assert kwargs["chapters_error"] == "stale semantic validation failure" + def test_skipped_does_not_write_chapters_file(self, monkeypatch): mock_cm = self._patch_cm(monkeypatch) llm_ops._save_llm_results( diff --git a/tests/unit/test_chapters_processor.py b/tests/unit/test_chapters_processor.py index 2797c09f..342d842a 100644 --- a/tests/unit/test_chapters_processor.py +++ b/tests/unit/test_chapters_processor.py @@ -2,7 +2,9 @@ Covers the full processing pipeline: gating (short / no-timeline / too-long), happy-path generation, semantic-validation retry (out-of-range index, -duplicate index, non-increasing index), first-index clamping, structural +duplicate index), start_seg sort normalization (non-increasing order is +sorted into shape instead of failing; sort stats warning), first-index +clamping, structural validation (chapter count bounds), density warning, adjacent-same-title merging, None-time tolerance, "HH:MM:SS" string time parsing, fingerprint stability, and exhausted-retry failure. @@ -881,6 +883,10 @@ def test_selected_models_override_config(self): class TestSemanticRetry(ChaptersProcessorTestBase): + """Semantic-validation retry: out-of-range index, duplicate index (dedup), + blank title/gist. NOTE: non-increasing ORDER is no longer a validation + failure -- it is normalized by sorting (see TestStartSegSortNormalization).""" + def test_out_of_range_index_retries_then_succeeds(self): segments = make_segments(10) bad = mock_llm_response([ @@ -902,31 +908,27 @@ def test_out_of_range_index_retries_then_succeeds(self): retry_call_kwargs = self.llm_client.call.call_args_list[1][1] self.assertIn("out of range", retry_call_kwargs["user_prompt"].lower()) - def test_duplicate_index_retry_still_bad_fails(self): - """A literal duplicate start_seg that, even after dedup (keep-first), still - leaves a non-increasing tail must fail -- and stay failed if the retry - repeats the same mistake.""" + def test_duplicate_index_dedup_then_sort_normalized(self): + """A literal duplicate start_seg is dropped by dedup (keep-first); if the + remaining sequence is non-increasing it is sorted into order instead of + failing -- no retry needed (sort normalization, not a validation error).""" segments = make_segments(10) - bad1 = mock_llm_response([ + raw = mock_llm_response([ {"title": "A", "gist": "g", "start_seg": 0}, {"title": "B", "gist": "g", "start_seg": 5}, - {"title": "B2", "gist": "g", "start_seg": 5}, # duplicate of B's start_seg - {"title": "C", "gist": "g", "start_seg": 3}, # dedup -> [0,5,3]: not increasing - ]) - bad2 = mock_llm_response([ - {"title": "A", "gist": "g", "start_seg": 0}, - {"title": "B", "gist": "g", "start_seg": 6}, - {"title": "B2", "gist": "g", "start_seg": 6}, # duplicate again - {"title": "C", "gist": "g", "start_seg": 4}, # dedup -> [0,6,4]: still not increasing + {"title": "B2", "gist": "g", "start_seg": 5}, # duplicate of B's start_seg, dropped + {"title": "C", "gist": "g", "start_seg": 3}, # dedup -> [0,5,3]: sorted -> [0,3,5] ]) - self.llm_client.call.side_effect = [bad1, bad2] + self.llm_client.call.return_value = raw result = self.processor.process(segments=segments, title="T") - self.assertEqual(result.status, ChaptersStatus.FAILED) - self.assertIsNotNone(result.error) - self.assertEqual(self.llm_client.call.call_count, 2) - self.assertEqual(result.chapters, []) + self.assertEqual(result.status, ChaptersStatus.GENERATED) + self.assertEqual(self.llm_client.call.call_count, 1) # no retry + self.assertEqual([c.start_seg for c in result.chapters], [0, 3, 5]) + # (title, gist, start_seg) pairing must survive the sort + self.assertEqual(result.chapters[1].title, "C") + self.assertEqual(result.chapters[2].title, "B") def test_pure_duplicate_collapses_without_retry(self): """A duplicate start_seg that fully resolves to an increasing sequence after @@ -945,22 +947,25 @@ def test_pure_duplicate_collapses_without_retry(self): self.assertEqual(len(result.chapters), 2) self.llm_client.call.assert_called_once() - def test_non_increasing_retries_then_succeeds(self): + def test_non_increasing_sorted_without_retry(self): + """Non-increasing start_seg order is normalized by sorting (single LLM + call, no retry) -- the sequence is a redundant representation of the + start_seg value set, not a semantic violation.""" segments = make_segments(10) - bad = mock_llm_response([ + raw = mock_llm_response([ {"title": "A", "gist": "g", "start_seg": 5}, {"title": "B", "gist": "g", "start_seg": 2}, # decreasing ]) - good = mock_llm_response([ - {"title": "A", "gist": "g", "start_seg": 0}, - {"title": "B", "gist": "g", "start_seg": 5}, - ]) - self.llm_client.call.side_effect = [bad, good] + self.llm_client.call.return_value = raw result = self.processor.process(segments=segments, title="T") self.assertEqual(result.status, ChaptersStatus.GENERATED) - self.assertEqual(self.llm_client.call.call_count, 2) + self.assertEqual(self.llm_client.call.call_count, 1) + # [5, 2] sorts to [2, 5]; first chapter is then clamped 2 -> 0 + self.assertEqual([c.start_seg for c in result.chapters], [0, 5]) + self.assertEqual(result.chapters[0].title, "B") + self.assertEqual(result.chapters[1].title, "A") def test_first_index_above_zero_is_clamped_without_retry(self): segments = make_segments(10) @@ -988,6 +993,130 @@ def test_illegal_json_retry_exhausted_at_llm_layer_fails(self): self.assertEqual(result.chapters, []) +class TestStartSegSortNormalization(ChaptersProcessorTestBase): + """Non-increasing start_seg order is normalized by sorting instead of failing. + + Regression tests for the production incident on BV1fcHD6cEw3 (task + task_1f8a978a541f40ba8ffe47b0d01e15cf): the fallback model returned + chapters ordered by topic arc instead of time; every start_seg anchor was + correct but the whole layer was judged FAILED twice, leaving the user + without a chapter list. The array order is a redundant representation of + the start_seg value set, so sorting is a safe salvage. + """ + + # Real attempt-2 start_seg sequence from the production incident (17 + # chapters, all unique and within surviving indices). + PRODUCTION_SEQUENCE = [ + 0, 22, 41, 50, 90, 155, 168, 222, 343, 371, 306, 378, 361, 303, + 436, 464, 486, + ] + + def _production_chapters(self): + return [ + {"title": f"Chapter {seg}", "gist": f"gist {i}", "start_seg": seg} + for i, seg in enumerate(self.PRODUCTION_SEQUENCE) + ] + + def test_production_out_of_order_sample_normalized(self): + segments = make_segments(500, text_repeat=1) + self.llm_client.call.return_value = mock_llm_response( + self._production_chapters() + ) + + result = self.processor.process(segments=segments, title="T") + + self.assertEqual(result.status, ChaptersStatus.GENERATED) + self.assertEqual(self.llm_client.call.call_count, 1) # no retry + self.assertEqual(len(result.chapters), 17) + starts = [c.start_seg for c in result.chapters] + self.assertEqual(starts, sorted(starts)) + # (title, gist, start_seg) pairing must survive the sort + self.assertEqual( + {(c.title, c.gist, c.start_seg) for c in result.chapters}, + {(d["title"], d["gist"], d["start_seg"]) + for d in self._production_chapters()}, + ) + + def test_production_sample_derives_consistent_end_segs(self): + """After sorting, _derive_end_segs must produce contiguous non-overlapping + intervals covering the whole timeline (surviving indices are contiguous + here, so end_seg = next start_seg - 1, last end = last index).""" + segments = make_segments(500, text_repeat=1) + self.llm_client.call.return_value = mock_llm_response( + self._production_chapters() + ) + + result = self.processor.process(segments=segments, title="T") + + chapters = result.chapters + for prev, curr in zip(chapters, chapters[1:]): + self.assertEqual(prev.end_seg + 1, curr.start_seg) # contiguous + self.assertEqual(chapters[-1].end_seg, 499) # last surviving index + + def test_out_of_order_logs_moved_and_lis_stats(self): + segments = make_segments(500, text_repeat=1) + self.llm_client.call.return_value = mock_llm_response( + self._production_chapters() + ) + + with patch( + "video_transcript_api.llm.processors.chapters_processor.logger" + ) as mock_logger: + result = self.processor.process(segments=segments, title="T") + + self.assertEqual(result.status, ChaptersStatus.GENERATED) + warning_messages = [ + str(call.args[0]) for call in mock_logger.warning.call_args_list + ] + sort_warnings = [m for m in warning_messages if "normalizing by sorting" in m] + self.assertEqual(len(sort_warnings), 1) + # 6 of 17 positions change; LIS of the production sequence is 14/17 + self.assertIn("moved 6/17", sort_warnings[0]) + self.assertIn("LIS 14/17", sort_warnings[0]) + + def test_sorted_input_keeps_behavior_unchanged(self): + """Already-sorted input: no sort warning, original order kept, no retry. + Behavior must be identical to pre-normalization HEAD.""" + segments = make_segments(10) + self.llm_client.call.return_value = mock_llm_response([ + {"title": "A", "gist": "g", "start_seg": 0}, + {"title": "B", "gist": "g", "start_seg": 5}, + {"title": "C", "gist": "g", "start_seg": 8}, + ]) + + with patch( + "video_transcript_api.llm.processors.chapters_processor.logger" + ) as mock_logger: + result = self.processor.process(segments=segments, title="T") + + self.assertEqual(result.status, ChaptersStatus.GENERATED) + self.assertEqual(self.llm_client.call.call_count, 1) + self.assertEqual([c.start_seg for c in result.chapters], [0, 5, 8]) + warning_messages = [ + str(call.args[0]) for call in mock_logger.warning.call_args_list + ] + self.assertFalse( + any("normalizing by sorting" in m for m in warning_messages), + f"sorted input must not trigger the sort warning, got: {warning_messages}", + ) + + def test_sort_then_clamp_first_chapter(self): + """Sorting happens before first-index clamping: after sorting, if the first + chapter still starts above the smallest surviving index, it is clamped.""" + segments = make_segments(10) + self.llm_client.call.return_value = mock_llm_response([ + {"title": "A", "gist": "g", "start_seg": 5}, + {"title": "B", "gist": "g", "start_seg": 2}, + {"title": "C", "gist": "g", "start_seg": 8}, + ]) + + result = self.processor.process(segments=segments, title="T") + + self.assertEqual(result.status, ChaptersStatus.GENERATED) + self.assertEqual([c.start_seg for c in result.chapters], [0, 5, 8]) + self.assertEqual(result.chapters[0].title, "B") # clamped from 2 to 0 + + class TestTitleGistValidation(ChaptersProcessorTestBase): """title/gist must be non-empty strings after stripping -- a missing/non-string/ blank value used to be silently coerced to "" and still became a chapter. Now