docs: address Codex review of WP4 plan (salt key compat, store/protocol, INFRA path)
This commit is contained in:
@@ -65,22 +65,22 @@ Expected: FAIL(`_build_key() takes 3 positional arguments but 4 were given`)
|
||||
- [ ] **Step 3: redis_cache 加 salt 维度**
|
||||
|
||||
`adapters/redis_cache.py`:
|
||||
- `_build_key(self, model, messages, cache_salt: str | None = None)`:payload 字典加 `"salt": cache_salt`:
|
||||
- `_build_key(self, model, messages, cache_salt: str | None = None)`:**仅当 `cache_salt is not None` 才加入 `salt` 字段**(默认 None 时 payload 结构与现状一字节不差,旧缓存键不失效):
|
||||
```python
|
||||
payload = json.dumps(
|
||||
{"model": model, "messages": messages, "salt": cache_salt},
|
||||
sort_keys=True,
|
||||
ensure_ascii=False,
|
||||
)
|
||||
key_obj: dict = {"model": model, "messages": messages}
|
||||
if cache_salt is not None:
|
||||
key_obj["salt"] = cache_salt
|
||||
payload = json.dumps(key_obj, sort_keys=True, ensure_ascii=False)
|
||||
```
|
||||
- `get(self, model, messages, cache_salt: str | None = None)`:`key = self._build_key(model, messages, cache_salt)`
|
||||
- `set(self, model, messages, response, cache_salt: str | None = None)`:同样传 `cache_salt`
|
||||
|
||||
- [ ] **Step 4: Protocol + chat 透传 salt**
|
||||
|
||||
`core/protocols.py` `LLMProvider.chat` 签名加 `cache_salt: str | None = None`(keyword-only,放 `parent_call_id` 后)。
|
||||
`adapters/llm.py` `chat` 签名加同参;L299 改 `await self._cache.get(self._model, messages, cache_salt)`;L373 改 `await self._cache.set(self._model, messages, response, cache_salt)`。
|
||||
grep `adapters/` 找到转发 `chat` 的 VLM/包装类(如有),同步加 `cache_salt` 透传。
|
||||
`core/protocols.py` `LLMProvider.chat`(L22)签名加 `cache_salt: str | None = None`(keyword-only,放 `parent_call_id` 后)。
|
||||
`adapters/llm.py` `chat`(L271)签名加同参;L299 改 `await self._cache.get(self._model, messages, cache_salt)`;L373 改 `await self._cache.set(self._model, messages, response, cache_salt)`。
|
||||
`adapters/redis_cache.py` `get`(L50)/`set`(L73)签名各加 `cache_salt: str | None = None`,内部 `_build_key(model, messages, cache_salt)`。
|
||||
**VLM 转发**:`core/protocols.py` `VLMProvider.chat_with_images`(L35)与 `adapters/vlm.py` `GovernedVLMClient.chat_with_images`(L32)签名加 keyword-only `cache_salt: str | None = None`,L53 转发 `self._llm.chat(..., cache_salt=cache_salt)`。
|
||||
|
||||
- [ ] **Step 5: 运行测试确认通过 + 回归**
|
||||
|
||||
@@ -101,9 +101,10 @@ git commit -m "feat: add cache_salt dimension to LLM response cache"
|
||||
## Task 2: TTL 语义修正(≤0 报错)
|
||||
|
||||
**Files:**
|
||||
- Modify: `main.py:93`
|
||||
- Modify: `adapters/redis_cache.py`(新增 `_resolve_cache_ttl`)
|
||||
- Modify: `main.py:93`、`app/harness/video_split_cli.py:257`(两处 TTL 入口共用)
|
||||
- Modify: `.env:65`、`.env.example:49`
|
||||
- Test: `tests/unit/test_governed_llm.py` 或新增 `tests/unit/test_infra_settings.py`
|
||||
- Test: 新增 `tests/unit/test_infra_settings.py`
|
||||
|
||||
- [ ] **Step 1: 写失败测试**
|
||||
|
||||
@@ -112,9 +113,9 @@ git commit -m "feat: add cache_salt dimension to LLM response cache"
|
||||
import pytest
|
||||
|
||||
|
||||
def test_redis_cache_ttl_zero_rejected(monkeypatch):
|
||||
def test_redis_cache_ttl_zero_rejected():
|
||||
"""REDIS_CACHE_TTL<=0 必须启动即报错,消灭'0=永不过期'隐式语义。"""
|
||||
from main import _resolve_cache_ttl # Step 3 抽出的纯函数
|
||||
from adapters.redis_cache import _resolve_cache_ttl # Step 3 抽出的纯函数
|
||||
|
||||
with pytest.raises(ValueError, match="REDIS_CACHE_TTL"):
|
||||
_resolve_cache_ttl(0)
|
||||
@@ -128,9 +129,9 @@ def test_redis_cache_ttl_zero_rejected(monkeypatch):
|
||||
Run: `conda run -n Video-Tree-TRM python -m pytest tests/unit/test_infra_settings.py -v`
|
||||
Expected: FAIL(`cannot import name '_resolve_cache_ttl'`)。
|
||||
|
||||
- [ ] **Step 3: 抽出并实现 fail-loud**
|
||||
- [ ] **Step 3: 抽出 fail-loud 并替换两处 TTL 入口**
|
||||
|
||||
`main.py`:新增纯函数并替换 L93 逻辑:
|
||||
`adapters/redis_cache.py` 新增模块级纯函数(配置校验贴近 cache 实现、避免 app→main 依赖):
|
||||
```python
|
||||
def _resolve_cache_ttl(ttl: int) -> int:
|
||||
"""校验 Redis 缓存 TTL:必须为正整数(消灭 0=永不过期 的隐式语义)。"""
|
||||
@@ -141,7 +142,8 @@ def _resolve_cache_ttl(ttl: int) -> int:
|
||||
)
|
||||
return ttl
|
||||
```
|
||||
L93 改为 `ttl_s = _resolve_cache_ttl(settings.redis_cache_ttl)`(删除 `if >0 else None` 分支;`RedisResponseCache` 的 `ttl_s` 从此恒为正)。
|
||||
`main.py:93` 改为 `ttl_s = _resolve_cache_ttl(settings.redis_cache_ttl)`(import 上述函数,删 `if >0 else None`)。
|
||||
`app/harness/video_split_cli.py:257`(`_build_redis_cache` 内)同样从 `ttl_s = ... if > 0 else None` 改为 `ttl_s = _resolve_cache_ttl(settings.redis_cache_ttl)`——消灭第二条 `0=永不过期` 入口。
|
||||
|
||||
- [ ] **Step 4: 改 .env / .env.example**
|
||||
|
||||
@@ -362,9 +364,13 @@ async def test_baseline_infra_error_not_cached(tmp_path):
|
||||
Run: `conda run -n Video-Tree-TRM python -m pytest tests/unit/test_harness_validate.py -k baseline_infra_error_not_cached -v`
|
||||
Expected: FAIL(当前 INFRA unit 的对错被 put 进缓存)。
|
||||
|
||||
- [ ] **Step 3: 实现 INFRA 隔离**
|
||||
- [ ] **Step 3: 实现 INFRA 隔离(精确数据路径)**
|
||||
|
||||
`_resolve_baseline_block`:miss 跑完后,识别 INFRA 题(从 db 读 stop_reason∈{error,parse_error} 或经 run_inference 结果),其所属 unit:不 `baseline_cache.put`、不计入返回的 `b_units`(从块 unit 集合排除,使配对与 W/L 不含它)。护栏所需的 `errors_inc` 在 put 之前累计并检查(护栏前置)。具体实现按 Step 前"保真前置"要求对齐现场与 TRM4。
|
||||
**数据路径**(Codex 审指出:当前 `_load_run_rows`(validate.py:212)只查 `question_id,prediction,answer,steps_json`,拿不到 per-unit stop_reason,只有汇总 `stop_reason_counts` 无法定位哪个 unit 是 INFRA):
|
||||
1. `_load_run_rows`(validate.py:194-212)的 SELECT 增加 `stop_reason` 列;`_candidate_correctness_from_db` 相应可返回每题 stop_reason(或新增 `_infra_question_ids_from_db(log, run_id, questions)` 返回 stop_reason∈{"error","parse_error"} 的 qid 集)。
|
||||
2. `_resolve_baseline_block`:miss 跑完后,计算本块 INFRA unit 集(unit 内**任一题** stop_reason∈{"error","parse_error"})。对这些 unit:**不 `baseline_cache.put`**、**不计入 `b_units`**、并从返回给调用方的有效 unit 集中排除。护栏所需 `errors_inc` 在 put 之前累计并调 `_check_infra_guard`(护栏前置于缓存写入)。
|
||||
3. **贯穿调用方**(validate.py:575-600):`_resolve_baseline_block` 返回"有效 unit 子集 `valid_unit_chunk`",后续 `_run_candidate_block`、`unit_correctness_view`、`pair_block`、`_build_evidence_rows` 全部用 `valid_unit_chunk`(而非原始 `unit_chunk`),确保配对 `unit_ids` 与基线侧一致、不含 INFRA unit。
|
||||
4. 保真:非 INFRA unit 的配对翻转、基线快照复用、unit 折叠逻辑一字不改;仅"把 INFRA unit 从本块整体剔除"。按 Step 前"保真前置"逐行比对 TRM4。
|
||||
|
||||
- [ ] **Step 4: 运行测试确认通过 + 保真回归**
|
||||
|
||||
@@ -505,9 +511,9 @@ git commit -m "fix: read-only baseline queries skip _runs upsert (register_run f
|
||||
|
||||
- [ ] **Step 1: 写失败测试**
|
||||
|
||||
在 `tests/unit/test_harness_observation.py` 追加(验证 slow_candidate 与 final 可区分):
|
||||
`read_dual_metric` 真实签名为 `(db_path, *, run_id)`(无 epoch 参数,observation.py:164),故 observation 层 write/read 对任意 version_kind 都已支持——真正要验证的是 **runner 慢更新第二次写出的是 `slow_candidate` 而非 `final`**。在 `tests/unit/test_harness_observation.py` 追加 observation 层可区分性基线测试:
|
||||
```python
|
||||
def test_slow_candidate_kind_distinct_from_final(tmp_path):
|
||||
def test_dual_metric_kinds_distinguishable(tmp_path):
|
||||
from app.harness.observation import write_dual_metric, read_dual_metric
|
||||
|
||||
db = str(tmp_path / "h.db")
|
||||
@@ -517,16 +523,16 @@ def test_slow_candidate_kind_distinct_from_final(tmp_path):
|
||||
write_dual_metric(db, run_id="r", epoch=1, version_kind="slow_candidate",
|
||||
skills_version="v2", prompts_version="v2", pool="val",
|
||||
hard_acc=0.6, soft_score=None, mixed_score=None)
|
||||
rows = read_dual_metric(db, run_id="r", epoch=1)
|
||||
kinds = {r["version_kind"] for r in rows}
|
||||
assert "final" in kinds and "slow_candidate" in kinds
|
||||
rows = read_dual_metric(db, run_id="r") # 真实签名无 epoch,返回该 run 全部行
|
||||
kinds = {row["version_kind"] for row in rows if row["epoch"] == 1}
|
||||
assert kinds == {"final", "slow_candidate"}
|
||||
```
|
||||
> 依 `test_harness_observation.py` 现有 read_dual_metric 签名;实现前读对齐。
|
||||
> 实现前读 `test_harness_observation.py` 与 `write_dual_metric`/`read_dual_metric`(observation.py:119/164)确认参数名(epoch/pool/hard_acc 等)与返回行字段。真正的 `slow_candidate` 语义由 Step 3 改 runner 调用点落地。
|
||||
|
||||
- [ ] **Step 2: 运行确认失败/通过基线**
|
||||
- [ ] **Step 2: 运行基线(observation 层已支持任意 kind)**
|
||||
|
||||
Run: `conda run -n Video-Tree-TRM python -m pytest tests/unit/test_harness_observation.py -k slow_candidate -v`
|
||||
Expected: 若 read_dual_metric 已支持则 PASS 基线;重点是 Step 3 改 runner 调用点。
|
||||
Run: `conda run -n Video-Tree-TRM python -m pytest tests/unit/test_harness_observation.py -k dual_metric_kinds -v`
|
||||
Expected: PASS(observation 层本就接受任意 version_kind);本 task 的实质改动在 Step 3 的 runner 调用点。
|
||||
|
||||
- [ ] **Step 3: 改 runner 慢更新第二次写为 slow_candidate**
|
||||
|
||||
@@ -549,31 +555,38 @@ git commit -m "fix: slow-update R2 dual_metric uses slow_candidate kind (not fin
|
||||
## Task 10: 离线 --retry-uncertain + docstring 修正
|
||||
|
||||
**Files:**
|
||||
- Modify: `app/harness/video_split_cli.py`(build_arg_parser + run_pipeline)
|
||||
- Modify: `app/harness/baseline_diagnosis.py`(done 集排除 uncertain + docstring)
|
||||
- Test: `tests/unit/test_video_split_cli.py` 或 `test_baseline_diagnosis`
|
||||
- Modify: `core/evolution/protocols.py:130`(`DiagnosisSignalStore.done_question_ids` 加参数)
|
||||
- Modify: `adapters/baseline_diagnosis_store.py:109`(SqliteDiagnosisSignalStore 实现 + SQL)
|
||||
- Modify: `app/harness/baseline_diagnosis.py:96`(透传 + docstring)
|
||||
- Modify: `app/harness/video_split_cli.py`(build_arg_parser + run_pipeline 透传)
|
||||
- Test: `tests/unit/`(对 SqliteDiagnosisSignalStore.done_question_ids)
|
||||
|
||||
- [ ] **Step 1: 写失败测试**
|
||||
|
||||
定位 `baseline_diagnosis` 的 done_question_ids 计算函数(`app/harness/baseline_diagnosis.py`,grep `done_question_ids`),追加测试:`retry_uncertain=True` 时 tier=uncertain 的已落库题不计入 done(会被重诊)。
|
||||
`done_question_ids` 的真实定义在 Protocol `core/evolution/protocols.py:130` 与实现 `adapters/baseline_diagnosis_store.py:109`(不在 baseline_diagnosis.py)。对 SqliteDiagnosisSignalStore 追加测试:
|
||||
```python
|
||||
def test_retry_uncertain_excludes_uncertain_from_done(...):
|
||||
# 构造 baseline_diagnosis 表含 tier=T2 与 tier=uncertain 行;
|
||||
# done_ids(retry_uncertain=False) 含 uncertain 题;
|
||||
# done_ids(retry_uncertain=True) 不含 uncertain 题
|
||||
...
|
||||
def test_done_question_ids_retry_uncertain_excludes(tmp_path):
|
||||
# 构造 baseline_diagnosis 表含 tier=T2 行与 tier=uncertain 行(同 run+fingerprint);
|
||||
from adapters.baseline_diagnosis_store import SqliteDiagnosisSignalStore
|
||||
store = SqliteDiagnosisSignalStore(str(tmp_path / "h.db"))
|
||||
# ... upsert 一条 T2、一条 uncertain ...
|
||||
assert "<uncertain_qid>" in store.done_question_ids("run", "fp") # 默认含
|
||||
assert "<uncertain_qid>" not in store.done_question_ids("run", "fp", retry_uncertain=True) # 排除
|
||||
assert "<t2_qid>" in store.done_question_ids("run", "fp", retry_uncertain=True) # T2 仍算完成
|
||||
```
|
||||
> 实现前读 `baseline_diagnosis.py` 的 done 集查询函数签名对齐。
|
||||
> 实现前读 `adapters/baseline_diagnosis_store.py` 的 upsert/表结构与 `done_question_ids` SQL 对齐构造。
|
||||
|
||||
- [ ] **Step 2: 运行确认失败**
|
||||
|
||||
Run: `conda run -n Video-Tree-TRM python -m pytest -k retry_uncertain -v`
|
||||
Expected: FAIL(无该参数)。
|
||||
Run: `conda run -n Video-Tree-TRM python -m pytest -k done_question_ids_retry_uncertain -v`
|
||||
Expected: FAIL(`unexpected keyword argument 'retry_uncertain'`)。
|
||||
|
||||
- [ ] **Step 3: 实现 retry_uncertain**
|
||||
- [ ] **Step 3: 实现 retry_uncertain(贯穿 Protocol→Adapter→编排→CLI)**
|
||||
|
||||
`app/harness/baseline_diagnosis.py`:done 集查询函数加 `retry_uncertain: bool = False`,True 时 `WHERE ... AND tier != 'uncertain'`(uncertain 题不算完成,会被重诊)。修正模块 docstring 把"崩溃最多丢正在写的一行/逐行落库"表述改为实际的"run 末批量落库(Phase 3),崩溃丢本次 run 全部未落库结果,靠 Redis 缓存缓解重烧"。
|
||||
`app/harness/video_split_cli.py`:`build_arg_parser` 加 `--retry-uncertain`(store_true);`run_pipeline` 透传到 `run_baseline_diagnosis` 的 done 集计算。
|
||||
- `core/evolution/protocols.py:130` `DiagnosisSignalStore.done_question_ids` 签名加 `retry_uncertain: bool = False`(keyword)。
|
||||
- `adapters/baseline_diagnosis_store.py:109` 实现同签名;SQL 在 `retry_uncertain=True` 时追加 `AND tier != 'uncertain'`(uncertain 题不算完成,会被重诊)。
|
||||
- `app/harness/baseline_diagnosis.py:96` `done = store.done_question_ids(baseline_run_id, diag_fingerprint)` 透传 `retry_uncertain`;修正模块 docstring 把"崩溃最多丢正在写的一行/逐行落库"改为实际的"run 末批量落库(Phase 3),崩溃丢本次 run 未落库结果,靠 Redis 缓存缓解重烧"。
|
||||
- `app/harness/video_split_cli.py`:`build_arg_parser` 加 `--retry-uncertain`(store_true);`run_pipeline` 透传到 `run_baseline_diagnosis`。
|
||||
|
||||
- [ ] **Step 4: 运行测试确认通过 + 回归**
|
||||
|
||||
|
||||
Reference in New Issue
Block a user