From 2c1c63a27554b80537db5c052511d4f41a2a7069 Mon Sep 17 00:00:00 2001 From: tzhouam Date: Tue, 25 Aug 2026 13:44:24 +0800 Subject: [PATCH] review: bugfix-PR regression-coverage checklist (both modes) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #93 asks the reviewer to make regression coverage a formal duty on bugfix PRs. Phase 1, data-plane only, one duty stated once per mode: - Strict: a "Bugfix PR regression coverage" section in the injected checklist page (repos/vllm-omni/review/guides/strict-review-checklist.md) — pin the ORIGINAL failure path (failing pre-fix, root-cause-aligned, happy path does not count), check same-class entrypoints/boundaries, and conclude with the covering test or an explicit blocking/non-blocking finding. Fenced "only when the PR is a bugfix", so non-bugfix reviews are unaffected. Page stays inside its 7,000-char injection budget (6,991 after the edit — the next section added here must trim first). - Direct: one conditional item in _DIRECT_REVIEW_CHECKLIST with the same duty, guarded by the same bugfix condition. Guardrail test pins the Direct item; _index.md description extended; SPEC page re-verified. Knowledge validators, wiki lint, and the vllm-omni release audit (enforce, CLEAN) all pass. Per the house eval rule this is the cheap reversible trial route — checklist data, no prompt/pipeline changes; measurement before any Phase 2 (planner bugfix signal + dedicated manifest key). Refs #93. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0165QsXAeLVPrKfj8Yk5zns3 --- doc/architecture/SPEC/thin_mcp_server.md | 2 +- knowledge/repos/vllm-omni/review/guides/_index.md | 2 +- .../review/guides/strict-review-checklist.md | 11 ++++++++++- src/infermatrix_copilot/thin_mcp_server.py | 1 + test/test_thin_mcp_server.py | 4 ++++ 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/doc/architecture/SPEC/thin_mcp_server.md b/doc/architecture/SPEC/thin_mcp_server.md index 483d75ba..f893edc5 100644 --- a/doc/architecture/SPEC/thin_mcp_server.md +++ b/doc/architecture/SPEC/thin_mcp_server.md @@ -1,6 +1,6 @@ # thin_mcp_server.py —— 规范 - + `LOC ~1286 · 默认 MCP：Direct 路由 + Strict 入口 · refactor-status: oversized` diff --git a/knowledge/repos/vllm-omni/review/guides/_index.md b/knowledge/repos/vllm-omni/review/guides/_index.md index 30d78f72..e909604b 100644 --- a/knowledge/repos/vllm-omni/review/guides/_index.md +++ b/knowledge/repos/vllm-omni/review/guides/_index.md @@ -13,5 +13,5 @@ sources: [] |---|---| | 按 PR 描述先路由到精确 owner/model 代码地图，再用 changed files 校验范围 | [PR intent maintainer routing](maintainer-pattern-routing.md) | | 审查模型适配 PR 的完整性 | [model adaptation guardrails](model-adaptation-guardrails.md) | -| Strict 审查注入的触发式检查单（streaming 生命周期、平台默认值影响面、依赖下界、CI marker 选择、测试有效性） | [strict review checklist](strict-review-checklist.md) | +| Strict 审查注入的触发式检查单（streaming 生命周期、平台默认值影响面、依赖下界、CI marker 选择、测试有效性、bugfix 回归覆盖） | [strict review checklist](strict-review-checklist.md) | | 判断新模型验证是否证明语义正确 | [model validation](model-validation.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 293424c0..5641d42d 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-15 +updated: 2026-08-25 type: guide tags: [vllm-omni, review] sources: [] @@ -114,6 +114,15 @@ Train-distilled trigger→check lines (20-PR campaign + teacher traces semantic merge conflict here (double resample) that invalidates the PR's own WER numbers; on any merge commit, re-verify reported numbers at HEAD. +## Bugfix PR regression coverage + +Only when the PR is a bugfix (title/labels/linked issue): demand a regression +test pinning the ORIGINAL failure path — failing before the fix, matching the +root cause, not a happy path (a test that cannot fail counts as missing). +Check same-class entrypoints and boundaries; an accepted gap must state reason +and residual risk. Conclude by naming the covering test/case or filing an +explicit blocking / non-blocking finding — never silence. + ## Process norms maintainers enforce - A PR fixing one of N problems in a linked issue: one comment names the diff --git a/src/infermatrix_copilot/thin_mcp_server.py b/src/infermatrix_copilot/thin_mcp_server.py index 829138d8..ab376d4f 100644 --- a/src/infermatrix_copilot/thin_mcp_server.py +++ b/src/infermatrix_copilot/thin_mcp_server.py @@ -184,6 +184,7 @@ def _knowledge_root() -> Path: # unassisted baseline caught and Direct missed, and neither belongs to any component # owner, so no knowledge route will surface them. "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.", "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/test/test_thin_mcp_server.py b/test/test_thin_mcp_server.py index 96e0dbe3..776d77a8 100644 --- a/test/test_thin_mcp_server.py +++ b/test/test_thin_mcp_server.py @@ -123,6 +123,10 @@ def test_direct_entrypoints_do_not_resolve_repo(monkeypatch): "subtraction" in item for item in review["first_review_checklist"] ) + assert any( + "pins the original failure path" in item + for item in review["first_review_checklist"] + ) assert any( "consumers, trust boundaries, and lifecycle ownership" in item for item in review["first_review_checklist"]