From dc0edd5b21b415510065291ed5a37df601882844 Mon Sep 17 00:00:00 2001 From: tzhouam Date: Thu, 10 Sep 2026 17:15:52 +0800 Subject: [PATCH] Make test evidence answerable against the CI definition A PR body that says tests pass is not evidence that the guard cases CI runs for the changed feature were executed. The lane selects by marker, so running a test file directly can prove nothing about what the lane collects. REV-3a makes that comparison a gate: find the guard steps from the pipeline definition at the frozen head, compare their commands with what the PR reports running by selector rather than by literal text, and treat a missing, unrun or mismatched case as a blocking finding. The rule reaches every review channel it applies to. Direct consumers get it through the first-review checklist; the Strict reviewer gets the repo-shaped form through the injected checklist page. That injection had no room left. The page was 6,991 characters against a 7,000-character cap, so anything appended would have been cut off mid-sentence and the gate would have looked present while never reaching the model. The cap moves to 8,000, and the three places that documented it as 4k or 7k now agree with the code. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JCk5xJGRyydouK7tuhs8kk --- doc/architecture/SPEC/direct_routing.md | 2 +- doc/architecture/SPEC/engine/steps/review.md | 5 +++-- doc/features/review-recall.md | 2 +- doc/knowledge/writing.md | 2 +- knowledge/general/review/_index.md | 4 ++-- knowledge/general/review/rules.md | 22 +++++++++++++++++-- .../review/guides/strict-review-checklist.md | 11 ++++++++-- knowledge/tools/check_wiki_lint.py | 2 +- src/infermatrix_copilot/direct_routing.py | 1 + .../engine/steps/review/steps.py | 5 ++++- test/test_review_step.py | 8 ++++++- 11 files changed, 50 insertions(+), 14 deletions(-) diff --git a/doc/architecture/SPEC/direct_routing.md b/doc/architecture/SPEC/direct_routing.md index 8565756e4..9207ea0a7 100644 --- a/doc/architecture/SPEC/direct_routing.md +++ b/doc/architecture/SPEC/direct_routing.md @@ -1,6 +1,6 @@ # direct_routing.py —— 规范 - + `LOC ~917 · Direct 模式完整策略包与知识路由 · refactor-status: known-debt` diff --git a/doc/architecture/SPEC/engine/steps/review.md b/doc/architecture/SPEC/engine/steps/review.md index bcfc3405f..3ec00d269 100644 --- a/doc/architecture/SPEC/engine/steps/review.md +++ b/doc/architecture/SPEC/engine/steps/review.md @@ -1,6 +1,6 @@ # engine/steps/review/ —— 规范 - + `LOC ~900（6 个文件） · step 库（评审） · refactor-status: ok` @@ -40,7 +40,8 @@ `_EXPECTED_PLANNER_CAUSES`（`unavailable` / `rejected_depth:`）时记 trace 缺口 —— 配置了的 planner 答非所问（transport / 空回复 / 不可解析） 与"没配"和"守卫在履职"是三件事（见 `review/planner.md` 的 5 种 cause）。 -- 评审：领域 checklist 由 profile 的 `review.md` 扩展；`_sweep_targets` 以 +- 评审：领域 checklist 由 adapter 的 `knowledge.review_checklist` 或 profile 的 + `review.md` 扩展，**注入上限 8k 字符**（页面写满上限时新增门禁会被静默截断）；`_sweep_targets` 以 `repo.language` 为键，**诚实降级**；裁决自洽（任何 ≥minor 的评论 ⇒ REQUEST CHANGES）； 确定性的按严重度排序的评论上限。 - prompt 是仓库中立的（**A5**）。 diff --git a/doc/features/review-recall.md b/doc/features/review-recall.md index abf7f69dd..addb5b585 100644 --- a/doc/features/review-recall.md +++ b/doc/features/review-recall.md @@ -117,7 +117,7 @@ Validated 账本改为**排序**（`[resolved]`/`[claim-*]` 在前）并封顶 1 **旧路径原封不动**。离线测试覆盖新的 planner 规则、文档 pass 选择、 第二轮（触发/跳过/近重复）、溢出门、账本排序、锚点回退、循环追问， 以及每一个新工具（含 calc 的沙箱）。知识 checklist 页按 teacher 蒸馏出的仓库先验重写 -（注入上限 4k→7k 字符）。 +（注入上限 4k→8k 字符）。 ## 实测结果（在各道门之后补记） diff --git a/doc/knowledge/writing.md b/doc/knowledge/writing.md index faa6b2e1d..e69d27f1a 100644 --- a/doc/knowledge/writing.md +++ b/doc/knowledge/writing.md @@ -94,7 +94,7 @@ baseline、catalog、source map、SHA pin 和 manifest，不做 owner 归属判 - `briefing_docs_extra` —— 所有档位都加载的扩展切片，vllm-omni 目前是 `repos/vllm-omni/review/guides/maintainer-pattern-routing.md`； - 强模型额外的 `performance_briefing_docs`（精简版评审模式）； - - `review_checklist` —— 单页仓库评审清单，前 4k 字符注入 Strict reviewer 的 + - `review_checklist` —— 单页仓库评审清单，前 8k 字符注入 Strict reviewer 的 system prompt，vllm-omni 指向 `repos/vllm-omni/review/guides/strict-review-checklist.md`。 - **按需** —— `doc_search` / `doc_read` 工具可递归到达 `general/` 里每一页更深的 diff --git a/knowledge/general/review/_index.md b/knowledge/general/review/_index.md index dca9e9d98..f5bf3ef1f 100644 --- a/knowledge/general/review/_index.md +++ b/knowledge/general/review/_index.md @@ -1,7 +1,7 @@ --- title: "代码审查" created: 2026-07-10 -updated: 2026-07-30 +updated: 2026-09-10 type: index tags: [general, review] sources: [] @@ -22,6 +22,6 @@ sources: [] | 遇到什么 | 查看哪里 | 说明 | |---|---|---| -| 检查同族实现覆盖和条件分支对称性 | [通用设计审查规则](rules.md) | 由代码结构触发的跨仓库设计门禁 | +| 检查同族实现覆盖、条件分支对称性，或 PR 测试证据是否对齐 CI | [通用设计审查规则](rules.md) | 由代码结构和测试证据触发的跨仓库门禁 | | 从维护者视角审查当前改动，或只拿到 PR 链接 | [独立审查执行合同](guides/review-execution-contract.md) | 一次 Direct 审查复用同一证据，同时完成正确性和设计减法，只输出一篇评论 | | 查找其他审查专项 | [guides 索引](guides/_index.md) | 只继续读取当前风险命中的专项 | diff --git a/knowledge/general/review/rules.md b/knowledge/general/review/rules.md index 6af4b6e7e..c33cad5a6 100644 --- a/knowledge/general/review/rules.md +++ b/knowledge/general/review/rules.md @@ -1,10 +1,10 @@ --- title: "通用设计审查规则" created: 2026-07-30 -updated: 2026-07-31 +updated: 2026-09-10 type: rule tags: [general, review] -sources: ["InferMatrixCopilot Issue #17", "InferMatrixCopilot Issue #24", "vllm-project/vllm-omni PR #5394", "zuiho-kai/claude-workflow-starter@c217fc6"] +sources: ["InferMatrixCopilot Issue #17", "InferMatrixCopilot Issue #24", "vllm-project/vllm-omni PR #5394", "zuiho-kai/claude-workflow-starter@c217fc6", "owner review-rule request 2026-09-10", "JiusiServe/omni-reviewbot PR #56"] confidence: high --- @@ -78,5 +78,23 @@ confidence: high - 验收：10 分钟内给用户 actionable findings 或明确的 `partial review`；更深验证作为 后续可选任务，不阻塞本轮答复。 ^[InferMatrixCopilot Issue #24] +## 测试证据 + +### REV-3a — PR 的测试证据必须对齐 CI 实际执行的命令 + +- 触发：PR 描述声称已验证（Test Plan / Test Result、贴出命令或结果），或改动落在有 + CI 看护用例的特性、模型或硬件路径上。 +- 强制：以冻结 head 的 CI 定义为准反查看护 step —— pipeline step 的 `commands` 是 CI + 真正执行的命令，`source_file_dependencies` 声明它看护哪些改动路径，marker 语义以 + `pyproject.toml` 为准。逐条比对 lane 命令与 PR 报告的命令是否选中同一组用例：test + target、`-m` 表达式、run-level 或等价门必须一致。 +- 禁止：把"跑过某个测试文件"当成该用例在 lane 内被执行的证据 —— marker 会把它 + deselect；用 `timeout`、`--cov*`、容器或环境前缀的字面差异判定不一致；PR 完全没有 + 测试证据时保持沉默。 +- 验收：审查记录列出命中的看护 step、其 lane 命令、PR 报告的命令和等价结论。缺测试 + 证据、看护 step 未执行或 selector 不一致时给出阻断级 finding，点名 step label 与两条 + 命令；只跑子集时必须写明未覆盖部分。 + ^[owner review-rule request 2026-09-10] ^[JiusiServe/omni-reviewbot PR #56] + 具体审查执行顺序见[独立审查执行合同](guides/review-execution-contract.md)，输入、输出与 边界矩阵写法见[Reviewer Lens Contracts](guides/reviewer-lens-contracts.md)。 diff --git a/knowledge/repos/vllm-omni/review/guides/strict-review-checklist.md b/knowledge/repos/vllm-omni/review/guides/strict-review-checklist.md index 5641d42d0..bcc8473e5 100644 --- a/knowledge/repos/vllm-omni/review/guides/strict-review-checklist.md +++ b/knowledge/repos/vllm-omni/review/guides/strict-review-checklist.md @@ -1,7 +1,7 @@ --- title: "Strict 审查触发式检查单" created: 2026-08-12 -updated: 2026-08-25 +updated: 2026-09-10 type: guide tags: [vllm-omni, review] sources: [] @@ -10,7 +10,7 @@ sources: [] # Strict 审查触发式检查单 Train-distilled trigger→check lines (20-PR campaign + teacher traces -2026-08-15). The Strict reviewer injects the first 7,000 chars +2026-08-15). The Strict reviewer injects the first 8,000 chars (`knowledge.review_checklist`); keep the core inside that budget. ## Streaming / TTS serving @@ -105,6 +105,13 @@ Train-distilled trigger→check lines (20-PR campaign + teacher traces loads that config; propose the single most plausible in-diff mechanism. - L2 covers CPU/mock plumbing only; serving/entrypoints tests may not collect under the installed vLLM — classify as environment, not defect. +- PR body vs lane: a `.buildkite/**` step's `commands` are what CI runs and + its `source_file_dependencies` say which changed paths it guards. Compare + the Test Plan/Test Result against them by SELECTOR (targets, `-m` + expression, run-level), never by literal text — `timeout`, `--cov*` and + container prefixes differ by design. A guard step never run, a command + that drops the lane's marker, or no test evidence at all is a blocking + finding naming the step label and both commands. ## Benchmarks / audio numerics diff --git a/knowledge/tools/check_wiki_lint.py b/knowledge/tools/check_wiki_lint.py index 3ea4e928e..ea752c1ad 100644 --- a/knowledge/tools/check_wiki_lint.py +++ b/knowledge/tools/check_wiki_lint.py @@ -34,7 +34,7 @@ "briefing_docs_extra", "performance_briefing_docs", # single knowledge page injected as the Strict reviewer's repo-specific - # checklist (first 4k chars); resolved relative to the knowledge root + # checklist (first 8k chars); resolved relative to the knowledge root "review_checklist", } DATE = re.compile(r"^\d{4}-\d{2}-\d{2}$") diff --git a/src/infermatrix_copilot/direct_routing.py b/src/infermatrix_copilot/direct_routing.py index 31a503b57..93e1be538 100644 --- a/src/infermatrix_copilot/direct_routing.py +++ b/src/infermatrix_copilot/direct_routing.py @@ -182,6 +182,7 @@ def _knowledge_root() -> Path: "After preflight passes, run targeted tests and low-cost static checks alongside source review.", "When the diff adds or changes a test, check the assertions bind to real behavior and not to values the fixture, mock, or fake injected.", "When the PR is a bugfix (title, labels, or linked issue), require a regression test that pins the original failure path; happy-path-only additions do not count, and a missing pin becomes an explicit blocking or non-blocking finding, never silence.", + "Check the PR body's Test Plan and Test Result against the CI definition at the frozen head: name the guard steps whose declared source dependencies cover the changed paths, then compare their commands with what the PR reports running by selector — test targets, marker expression, run level — not by literal text. A guard step never run, a dropped marker, or absent test evidence is a blocking finding naming the step and both commands.", "When the diff passes a new argument to a dependency, check it against the lowest version the project's own constraints still permit, not the version installed here.", "For resource or cache changes, trace budget measurement through reservation and physical consumption, including warmup/profile/activation ordering and low-resource behavior.", "For runtime changes, trace exception propagation, partial-allocation cleanup, cancellation, timeout, shutdown, and concurrent scheduling to the terminal user-visible signal.", diff --git a/src/infermatrix_copilot/engine/steps/review/steps.py b/src/infermatrix_copilot/engine/steps/review/steps.py index e17003f0c..9838e2386 100644 --- a/src/infermatrix_copilot/engine/steps/review/steps.py +++ b/src/infermatrix_copilot/engine/steps/review/steps.py @@ -672,8 +672,11 @@ async def _review_diff(ctx: StepContext) -> StepResult: for review_md in candidates: try: if review_md.exists() and ctx.settings.profile_briefing_enabled: + # budget: the largest checklist in the tree sat at + # 6,991 chars, flush against the old 7k cap, so a + # newly added gate was silently cut off mid-page guidance += ("\n\n## Repo-specific review checklist\n" - + review_md.read_text(encoding="utf-8")[:7_000]) + + review_md.read_text(encoding="utf-8")[:8_000]) break except OSError: continue diff --git a/test/test_review_step.py b/test/test_review_step.py index ee79f2dbe..3412a97ab 100644 --- a/test/test_review_step.py +++ b/test/test_review_step.py @@ -107,8 +107,13 @@ def test_review_checklist_resolves_from_adapter_knowledge(settings, trace, escapes the knowledge root is ignored.""" kroot = tmp_path / "knowledge" (kroot / "repos" / "repo_x").mkdir(parents=True) + # The marker sits past the old 7k cap, which the vllm-omni checklist was + # already flush against, and the tail sits past the current one. (kroot / "repos" / "repo_x" / "checklist.md").write_text( - "REPO-X-CHECKLIST-MARKER: check the frobnicator") + "filler line\n" * 600 + + "REPO-X-CHECKLIST-MARKER: check the frobnicator\n" + + "pad\n" * 300 + + "REPO-X-BEYOND-BUDGET") settings.knowledge_dir = kroot adir = settings.adapters_dir / "repo_x" adir.mkdir(parents=True) @@ -126,6 +131,7 @@ def test_review_checklist_resolves_from_adapter_knowledge(settings, trace, assert result.ok, result.summary prompt = llm.calls[0]["messages"][0]["content"] assert "REPO-X-CHECKLIST-MARKER" in prompt + assert "REPO-X-BEYOND-BUDGET" not in prompt # escape guard: a traversal path is ignored, not read (adir / "manifest.yaml").write_text(