diff --git a/doc/architecture/SPEC/contract.md b/doc/architecture/SPEC/contract.md index 23b2a42d..3625bfc9 100644 --- a/doc/architecture/SPEC/contract.md +++ b/doc/architecture/SPEC/contract.md @@ -1,6 +1,6 @@ # contract.py —— 规范 - + `LOC ~185 · 旧版跨仓库契约兼容层 · refactor-status: compatibility-shim` @@ -22,8 +22,8 @@ revision、supported repositories，以及 `supports_expected_head`、 `supports_idempotent_strict_start`、`supports_knowledge_curation`、 `max_strict_workers`。其余兼容导出包括 `build_review_result(run_dir) -> dict`（结构化评审 结果：`contract_version`、`run_id`、`state`、`reviewed_head_sha`、`verdict`、 -`summary_markdown`、`comments`、`stale`/`expected_head_sha`/ -`actual_head_sha`、`diagnostics`）；`unknown_run_result(run_id)`（显式 +`summary_markdown`、`comments`、`finding_dispositions`、 +`stale`/`expected_head_sha`/`actual_head_sha`、`diagnostics`）；`unknown_run_result(run_id)`（显式 `state: unknown`，绝不抛错 —— 丢响应和还在跑必须可区分）； `sanitize_comments` 与 `COMMENT_FIELDS`；以及从 `direct_routing` 再导出的 `direct_knowledge_routes` / `direct_execution_budget` / @@ -46,6 +46,11 @@ revision、supported repositories，以及 `supports_expected_head`、 - `sanitize_comments` 按 `COMMENT_FIELDS` 白名单 （file/line/severity/comment/evidence/suggestion）——内部记账键 （`_verified`、`corroborated_by` 等）绝不泄进消费方输出。 +- `comments` 是**终局集合**，不是候选集合：评审自己的取舍已在评审步 + 应用完毕（见 `engine/steps/review` 页）。`finding_dispositions` 是这件事的 + 凭据——每个候选一条 `{anchor, disposition, declared}`，按 + `DISPOSITION_FIELDS` 白名单投影，被撤下的评论正文本身绝不过界。 + 消费方因此无需把散文里的取舍和手上的列表对账（#141）。 - 本模块自身保持仓库中立；仓库专属的 Direct 路由表住在 `direct_routing.py`（见其页）。 - 能力身份的权威实现在 `sdk.v1.get_capabilities`；这里不维护第二份版本或 diff --git a/doc/architecture/SPEC/engine/steps/review.md b/doc/architecture/SPEC/engine/steps/review.md index f8c826b5..be034161 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` @@ -30,6 +30,16 @@ ## 不变量 - patch 门：廉价摘要**常开**，只有触发时才跑 LLM 评审；**fail-closed**（**C6**）； 高风险模块来自 adapter，settings 只作兜底（**A5**）。 +- **取舍是字段，不是句子**：每条候选评论带 `disposition` + （`publish` | `excluded` | `duplicate` | `resolved` | `no_issue`）。 + `finalize_review_dispositions` 在**评论预算之前**执行——被撤下的候选 + 因此占不掉八个可发布名额——也在**任何渲染器之前**执行，于是 summary、 + 正文、行内评论三者都由同一个终局集合渲染。缺字段=发布（该字段是增量的）； + 无法识别的取值也发布，但记进 `finding_dispositions.declared`， + 让异常可见而不是静默丢掉真实发现。`no_issue` 走 summary 的 + 「Checked, no defect found」注记：保留证据，去掉诉求，不带优先级。 + 在此之前取舍只活在模型的 `summary` 散文里，而发布读的是列表，于是 + summary 说要丢弃的请求照样行内发出（#141）。 - **`review_verdict` 是发布的 state 字段，不是散文**：由 `_review_verdict(review_comments, pr_state)` 计算（**与渲染器同一个 helper，绝不第二份校准规则**），随 `review_text`/`review_summary`/ diff --git a/doc/architecture/SPEC/sdk.md b/doc/architecture/SPEC/sdk.md index 0057a656..be4bcc67 100644 --- a/doc/architecture/SPEC/sdk.md +++ b/doc/architecture/SPEC/sdk.md @@ -1,6 +1,6 @@ # sdk/ —— 规范 - + `Python SDK v1 · 跨仓库唯一 typed 边界 · refactor-status: ok` @@ -76,7 +76,8 @@ push、开 PR 或 schedule。ReviewBot 必须向 `KnowledgeCurator` 传 dedicated work checkout，并继续拥有重试、artifact 与 fork publication；SDK 也绝不写 packaged knowledge tree。 -- SDK、Direct、Strict、Quality API 版本常量均为 `1.0.0`，Knowledge 为 `1.1.0` +- SDK、Direct、Quality API 版本常量为 `1.0.0`；Strict 为 `1.1.0` + （结果新增 `finding_dispositions`，#141），Knowledge 为 `1.1.0` （1.1 向后兼容地新增 `catalog_entries()` / `KnowledgeCatalogEntry` 与容量预检）， distribution 为 `0.2.0`；`Capabilities.knowledge_api_version` 与 `supports_knowledge_curation` 组成 ReviewBot 的 paired-release 握手，避免只按 diff --git a/src/infermatrix_copilot/contract.py b/src/infermatrix_copilot/contract.py index aabfa9b3..d9061813 100644 --- a/src/infermatrix_copilot/contract.py +++ b/src/infermatrix_copilot/contract.py @@ -62,6 +62,7 @@ "direct_mandatory_review_guides", "direct_review_plan", "sanitize_comments", + "sanitize_dispositions", "unknown_run_result", "unknown_quality_result", ] @@ -74,6 +75,12 @@ "file", "line", "severity", "comment", "evidence", "suggestion", }) +# One record per candidate the review considered: where it was anchored, and +# what the review decided to do with it. +DISPOSITION_FIELDS: frozenset[str] = frozenset({ + "anchor", "disposition", "declared", +}) + # Trace events the assembler surfaces as diagnostics. _DIAGNOSTIC_EVENTS = ("review_plan", "capability_gap", "expected_head_mismatch", "anchor_resolution", "diff_fallback") @@ -119,6 +126,20 @@ def _state_updates(run_dir: Path) -> dict: return merged +def sanitize_dispositions(records: Any) -> list[dict]: + """Project disposition records onto `DISPOSITION_FIELDS`. + + Same allow-list discipline as `sanitize_comments`: the withheld prose + itself never crosses the boundary, only the anchor and what was decided. + """ + out: list[dict] = [] + for record in records if isinstance(records, list) else []: + if isinstance(record, dict): + out.append({k: v for k, v in record.items() + if k in DISPOSITION_FIELDS}) + return out + + def sanitize_comments(comments: Any) -> list[dict]: """Project review comments onto `COMMENT_FIELDS`, dropping everything else.""" out: list[dict] = [] @@ -170,6 +191,12 @@ def build_review_result(run_dir: Path | str) -> dict[str, Any]: "verdict": str(updates.get("review_verdict") or ""), "summary_markdown": str(updates.get("review_summary") or ""), "comments": sanitize_comments(updates.get("review_comments")), + # Every candidate this review considered and what became of it. The + # published `comments` above are the finalized set; this is the proof, + # so a consumer never has to reconcile the summary's prose against the + # list it was handed (#141). + "finding_dispositions": sanitize_dispositions( + updates.get("review_finding_dispositions")), "stale": bool(mismatch), "expected_head_sha": str((mismatch or {}).get("expected") or ""), "actual_head_sha": str((mismatch or {}).get("actual") or ""), @@ -234,6 +261,7 @@ def unknown_run_result(run_id: str) -> dict[str, Any]: return {"contract_version": STRICT_API_VERSION, "run_id": run_id, "state": "unknown", "note": "", "reviewed_head_sha": "", "verdict": "", "summary_markdown": "", "comments": [], + "finding_dispositions": [], "stale": False, "expected_head_sha": "", "actual_head_sha": "", "diagnostics": {}} diff --git a/src/infermatrix_copilot/engine/steps/review/prompts.py b/src/infermatrix_copilot/engine/steps/review/prompts.py index 6e499049..fbc7922a 100644 --- a/src/infermatrix_copilot/engine/steps/review/prompts.py +++ b/src/infermatrix_copilot/engine/steps/review/prompts.py @@ -252,6 +252,17 @@ diffs collapse per-file sweep notes into grouped lines; evidence: at most 3 quoted \ lines per comment (quote the ONE decisive line, not the region). If running long, \ drop the least decisive findings lines first — never a comment. +- SELECTION IS A FIELD, NOT A SENTENCE. Every comment carries `disposition`. Use \ +`publish` for a finding you are actually raising; `excluded` for a candidate you \ +decided against; `duplicate` for one you consolidated into another comment here; \ +`resolved` for a concern this change already answers; `no_issue` for a check that \ +found the code correct. Only `publish` comments are published — the other four are \ +kept as the review's record of what it decided. A decision written only as prose in \ +`summary` is NOT applied: if you say a request should be dropped, mark that comment \ +`excluded` or it ships to the contributor anyway, contradicting your own summary. +- A CONFIRMATION IS NOT A FINDING. "I checked X and it is correct" is `no_issue`, \ +never a severity-carrying comment. A finding asks the contributor to do something; if \ +there is nothing to do, there is nothing to rank. - Only if the sweep truly surfaces nothing that belongs in this PR: empty review_comments \ with a one-line summary.""" diff --git a/src/infermatrix_copilot/engine/steps/review/steps.py b/src/infermatrix_copilot/engine/steps/review/steps.py index 9838e238..dac07a09 100644 --- a/src/infermatrix_copilot/engine/steps/review/steps.py +++ b/src/infermatrix_copilot/engine/steps/review/steps.py @@ -44,6 +44,8 @@ _render_review_md, _render_review_summary, _review_verdict, + disposition_records, + finalize_review_dispositions, _same_finding, _sweep_targets, ) @@ -687,7 +689,8 @@ async def _review_diff(ctx: StepContext) -> StepResult: "grounded, specific, useful findings.", guidance=guidance, expected="review_comments with file/line/anchor_snippet/severity/comment/" - "evidence; APPROVE-equivalent = empty review_comments with a summary.", + "evidence/disposition; APPROVE-equivalent = empty " + "review_comments with a summary.", evidence={"pr_diff": str(diff), "pr_context": ctx.state.get("pr_context", ""), "gate_report": ctx.state.get("gate_report", ""), @@ -698,7 +701,9 @@ async def _review_diff(ctx: StepContext) -> StepResult: "list of {file, line, anchor_snippet, severity: " "blocker|major|minor|nit, comment, evidence, " "suggestion: optional replacement code for the " - "cited lines — the concrete edit, no prose}"}, + "cited lines — the concrete edit, no prose, " + "disposition: publish|excluded|duplicate|resolved|" + "no_issue — only publish is published}"}, extra_tools={**_gh_read_tools(_repo_path(ctx)), **review_repo_tools(_repo_path(ctx))}, ) @@ -850,6 +855,15 @@ async def _review_diff(ctx: StepContext) -> StepResult: # major/blocker) or for individually verify-confirmed cut comments — a # quiet PR with an unverified tail stays terse (a thin-GT val item # rendering 12 findings lost 3/3 on noise). + # Apply the review's OWN selection before anything else reads the list. + # It runs ahead of the budget below so a withheld candidate cannot occupy + # one of the eight publishable slots, and ahead of every renderer so the + # summary, the body and the inline comments are all made from the same + # finalized set (#141). + all_candidates = list(output.get("review_comments") or []) + published, withheld = finalize_review_dispositions(output) + output["review_comments"] = published + output["_withheld_findings"] = withheld comments = sorted(output.get("review_comments") or [], key=lambda c: (_SEVERITY_ORDER.get( str(c.get("severity", "minor")).lower(), 2), @@ -857,6 +871,20 @@ async def _review_diff(ctx: StepContext) -> StepResult: for c in comments: c.pop("corroborated_by", None) output["review_comments"] = comments[:8] + # Recorded AFTER the cut: a candidate the review chose to publish but the + # budget dropped is `over_budget`, never `excluded`. A consumer checking + # that no withheld finding was published must not trip over a healthy + # review whose ninth comment simply did not fit. + output["_finding_dispositions"] = disposition_records( + all_candidates, output["review_comments"]) + if withheld: + ctx.trace.record( + "finding_dispositions", + withheld=len(withheld), + published=len(output["review_comments"]), + dispositions=[r["disposition"] + for r in output["_finding_dispositions"]], + ) rich = sum(1 for c in comments[:8] if _SEVERITY_ORDER.get(str(c.get("severity", "minor")).lower(), 2) <= _SEVERITY_ORDER["major"]) >= 2 @@ -909,6 +937,9 @@ async def _review_diff(ctx: StepContext) -> StepResult: "review_summary": review_summary, "review_comments": review_comments, "review_verdict": review_verdict, + # The audit trail crosses the contract with the result: a + # consumer can prove the published set IS the finalized set. + "review_finding_dispositions": output.get("_finding_dispositions") or [], }) depth_note = f"; depth={plan.depth} via {plan.planner}" if plan else "" result.summary = (f"review produced ({len(output.get('review_comments') or [])} " diff --git a/src/infermatrix_copilot/engine/steps/review/utils.py b/src/infermatrix_copilot/engine/steps/review/utils.py index cec6878f..13258a29 100644 --- a/src/infermatrix_copilot/engine/steps/review/utils.py +++ b/src/infermatrix_copilot/engine/steps/review/utils.py @@ -478,6 +478,104 @@ def _prefix_rank(line: str) -> int: return parts +# One review emits candidates AND the decisions it took about them. Before +# this existed the decisions lived only in the model's `summary` prose while +# publication read `review_comments`, so a request the summary said to drop +# was still posted inline, and a confirmation that nothing was wrong was +# posted with a severity (JiusiServe/InferMatrixCopilot#141). Selection is +# data now: every output form renders from the set this function returns. +PUBLISH_DISPOSITION = "publish" +WITHHOLDING_DISPOSITIONS = frozenset({ + "excluded", # the review decided against raising it + "duplicate", # consolidated into another finding in this same review + "resolved", # the change already answers it; nothing is being asked + "no_issue", # checked and found correct — a negative check, not a defect +}) + + +def _disposition_of(comment: dict) -> str: + return str(comment.get("disposition") or "").strip().lower() + + +def finalize_review_dispositions(output: dict) -> tuple[list[dict], list[dict]]: + """Split candidates into what publishes and what was withheld. + + Runs BEFORE the comment budget, so a withheld candidate cannot occupy one + of the eight publishable slots. + + An absent disposition publishes: the field is additive and most runs will + not carry one. An UNRECOGNIZED value also publishes rather than being + dropped — losing a real finding to a typo is worse than publishing one + whose bookkeeping we cannot read — but it is recorded so the anomaly is + visible instead of silent. Only the four explicit withholding values + withhold, which is why this is not a keyword filter over prose. + """ + published: list[dict] = [] + withheld: list[dict] = [] + for comment in output.get("review_comments") or []: + if not isinstance(comment, dict): + continue + if _disposition_of(comment) in WITHHOLDING_DISPOSITIONS: + withheld.append(comment) + else: + published.append(comment) + return published, withheld + + +# Not offered to the model: a candidate the review chose to publish, cut by +# the deterministic comment budget rather than by any review decision. A +# consumer must be able to tell "we decided against this" from "it did not +# fit", or the count of publish records and of published comments disagree +# for a perfectly healthy review. +OVER_BUDGET_DISPOSITION = "over_budget" + + +def disposition_records(candidates: list[dict], + published: list[dict]) -> list[dict]: + """The audit trail: one record per candidate, whatever became of it. + + Published as `finding_dispositions` so a consumer can prove the published + set is the finalized set, without carrying the withheld prose itself. + """ + survived = {id(comment) for comment in published} + records = [] + for comment in candidates: + if not isinstance(comment, dict): + continue + declared = _disposition_of(comment) + if declared in WITHHOLDING_DISPOSITIONS: + disposition = declared + elif id(comment) in survived: + disposition = PUBLISH_DISPOSITION + else: + disposition = OVER_BUDGET_DISPOSITION + records.append({ + "anchor": _anchor(comment), + "disposition": disposition, + "declared": declared, + }) + return records + + +def negative_check_notes(withheld: list[dict]) -> list[str]: + """`no_issue` candidates as concise evidence notes. + + #141 asks that a statement confirming no problem exists never carry a + finding priority or request contributor action, while allowing the check + itself to remain visible. A note keeps the evidence and drops the ask. + """ + notes = [] + for comment in withheld: + if _disposition_of(comment) != "no_issue": + continue + text = str(comment.get("comment") or "").strip() + if not text: + continue + anchor = _anchor(comment) + notes.append(f"`{anchor}` — {text}" if anchor else text) + return notes + + def _render_review_summary(output: dict, pr_state: str = "") -> str: """Render a concise review body; individual findings are posted inline.""" comments = output.get("review_comments") or [] @@ -489,6 +587,13 @@ def _render_review_summary(output: dict, pr_state: str = "") -> str: parts.append(f"{len(comments)} actionable finding(s).") else: parts.append("No actionable findings.") + # A negative check keeps its evidence and loses its ask: it is reported + # here as a note, never among the findings where it would carry a + # severity and read as work to do (#141). + notes = negative_check_notes(output.get("_withheld_findings") or []) + if notes: + parts.append("**Checked, no defect found:**\n" + + "\n".join(f"- {note}" for note in notes)) parts.append(f"**Verdict:** {_review_verdict(comments, pr_state)}") return "\n\n".join(parts) diff --git a/src/infermatrix_copilot/sdk/v1/models.py b/src/infermatrix_copilot/sdk/v1/models.py index 399d8fe1..90c84c6a 100644 --- a/src/infermatrix_copilot/sdk/v1/models.py +++ b/src/infermatrix_copilot/sdk/v1/models.py @@ -7,7 +7,7 @@ SDK_API_VERSION = "1.0.0" DIRECT_API_VERSION = "1.0.0" -STRICT_API_VERSION = "1.0.0" +STRICT_API_VERSION = "1.1.0" # + finding_dispositions (#141) QUALITY_API_VERSION = "1.0.0" KNOWLEDGE_API_VERSION = "1.1.0" diff --git a/test/test_review_step.py b/test/test_review_step.py index 3412a97a..7deb58ff 100644 --- a/test/test_review_step.py +++ b/test/test_review_step.py @@ -242,3 +242,168 @@ def test_review_without_snippets_is_untouched(settings, trace, tmp_path, git_rep _ctx(settings, trace, tmp_path, state, llm=llm))) comment = result.outputs["review_comments"][0] assert comment["line"] == 2 and "_anchor_unverified" not in comment + + +def _dispositions(result): + return {r["anchor"]: r["disposition"] + for r in result.outputs["state_updates"]["review_finding_dispositions"]} + + +def test_an_excluded_candidate_reaches_no_published_surface( + settings, trace, tmp_path, git_repo): + """vllm-project/vllm-omni#6959: the summary said to drop the repo-id + validation request because the evidence packet already covered it, and the + request was published inline as a P2 anyway. The decision lived in prose + while publication read the list.""" + diff = ("diff --git a/mod_a.py b/mod_a.py\n--- a/mod_a.py\n+++ b/mod_a.py\n" + "@@ -1,1 +1,3 @@\n A = 1\n+B = 2\n+C = 3\n") + llm = ScriptedLLM([_contract_reply([ + {"file": "mod_a.py", "line": 2, "severity": "major", + "comment": "lock scope is too narrow", "evidence": "read mod_a.py", + "disposition": "publish"}, + {"file": "mod_a.py", "line": 3, "severity": "minor", + "comment": "validate the repo id before use", + "evidence": "read mod_a.py", "disposition": "excluded"}, + ])]) + state = {"diff_text": diff, "task_spec": {"pr": 6959}, + "repo_path": str(git_repo)} + + result = asyncio.run(_registry().get("agent.review_diff").handler( + _ctx(settings, trace, tmp_path, state, llm=llm))) + + assert result.ok + published = result.outputs["review_comments"] + assert [c["comment"] for c in published] == ["lock scope is too narrow"] + # Not in the inline comments, not in the body, not in the summary. + assert "validate the repo id" not in result.outputs["review_text"] + assert "validate the repo id" not in result.outputs["review_summary"] + assert _dispositions(result)["mod_a.py:3"] == "excluded" + + +def test_a_no_issue_confirmation_never_carries_a_finding_priority( + settings, trace, tmp_path, git_repo): + """vllm-project/vllm-omni#6959 again: a confirmation that the inspected + SPDX headers left no stale comments was published as a P2 finding. It + reports no defect and asks for nothing, so it cannot be ranked — but the + check itself is worth keeping, as a note.""" + diff = ("diff --git a/mod_a.py b/mod_a.py\n--- a/mod_a.py\n+++ b/mod_a.py\n" + "@@ -1,1 +1,3 @@\n A = 1\n+B = 2\n+C = 3\n") + llm = ScriptedLLM([_contract_reply([ + {"file": "mod_a.py", "line": 2, "severity": "minor", + "comment": "the SPDX headers leave no stale docstrings", + "evidence": "read mod_a.py", "disposition": "no_issue"}, + ])]) + state = {"diff_text": diff, "task_spec": {"pr": 6959}, + "repo_path": str(git_repo)} + + result = asyncio.run(_registry().get("agent.review_diff").handler( + _ctx(settings, trace, tmp_path, state, llm=llm))) + + assert result.ok + assert result.outputs["review_comments"] == [] + summary = result.outputs["review_summary"] + assert "Checked, no defect found:" in summary + assert "no stale docstrings" in summary + # A review that collected observations but has nothing to ask for is a + # valid zero-finding result, not a REQUEST CHANGES. + assert summary.rstrip().endswith("**Verdict:** APPROVE") + + +def test_withheld_candidates_do_not_consume_the_comment_budget( + settings, trace, tmp_path, git_repo): + """Finalization runs before the eight-comment cut, so an excluded + candidate cannot push a real finding out of the published set.""" + diff = ("diff --git a/mod_a.py b/mod_a.py\n--- a/mod_a.py\n+++ b/mod_a.py\n" + "@@ -1,1 +1,12 @@\n A = 1\n" + "".join( + f"+L{n} = {n}\n" for n in range(2, 13))) + comments = [ + {"file": "mod_a.py", "line": n, "severity": "nit", + "comment": f"withheld {n}", "evidence": "e", "disposition": "duplicate"} + for n in range(2, 10) + ] + [ + {"file": "mod_a.py", "line": 11, "severity": "nit", + "comment": "the surviving finding", "evidence": "e"}, + ] + llm = ScriptedLLM([_contract_reply(comments)]) + state = {"diff_text": diff, "task_spec": {"pr": 1}, + "repo_path": str(git_repo)} + + result = asyncio.run(_registry().get("agent.review_diff").handler( + _ctx(settings, trace, tmp_path, state, llm=llm))) + + assert result.ok + published = [c["comment"] for c in result.outputs["review_comments"]] + assert published == ["the surviving finding"] + + +def test_a_review_without_dispositions_publishes_exactly_as_before( + settings, trace, tmp_path, git_repo): + """The field is additive: an older run that carries none is unchanged.""" + diff = ("diff --git a/mod_a.py b/mod_a.py\n--- a/mod_a.py\n+++ b/mod_a.py\n" + "@@ -1,1 +1,2 @@\n A = 1\n+B = 2\n") + llm = ScriptedLLM([_contract_reply([ + {"file": "mod_a.py", "line": 2, "severity": "major", + "comment": "unguarded index", "evidence": "e"}, + ])]) + state = {"diff_text": diff, "task_spec": {"pr": 1}, + "repo_path": str(git_repo)} + + result = asyncio.run(_registry().get("agent.review_diff").handler( + _ctx(settings, trace, tmp_path, state, llm=llm))) + + assert result.ok + assert [c["comment"] for c in result.outputs["review_comments"]] == [ + "unguarded index"] + assert _dispositions(result) == {"mod_a.py:2": "publish"} + + +def test_an_unreadable_disposition_publishes_and_is_recorded( + settings, trace, tmp_path, git_repo): + """Losing a real finding to a typo is worse than publishing one whose + bookkeeping we cannot read; the declared value is kept so it is visible.""" + diff = ("diff --git a/mod_a.py b/mod_a.py\n--- a/mod_a.py\n+++ b/mod_a.py\n" + "@@ -1,1 +1,2 @@\n A = 1\n+B = 2\n") + llm = ScriptedLLM([_contract_reply([ + {"file": "mod_a.py", "line": 2, "severity": "major", + "comment": "unguarded index", "evidence": "e", "disposition": "drop"}, + ])]) + state = {"diff_text": diff, "task_spec": {"pr": 1}, + "repo_path": str(git_repo)} + + result = asyncio.run(_registry().get("agent.review_diff").handler( + _ctx(settings, trace, tmp_path, state, llm=llm))) + + assert result.ok + assert len(result.outputs["review_comments"]) == 1 + record = result.outputs["state_updates"]["review_finding_dispositions"][0] + assert record["disposition"] == "publish" + assert record["declared"] == "drop" + + +def test_a_budget_cut_is_not_recorded_as_a_review_decision( + settings, trace, tmp_path, git_repo): + """Nine publishable findings, eight slots. The ninth was not withheld by + the review, so a consumer checking that nothing withheld got published + must not trip over it.""" + diff = ("diff --git a/mod_a.py b/mod_a.py\n--- a/mod_a.py\n+++ b/mod_a.py\n" + "@@ -1,1 +1,11 @@\n A = 1\n" + "".join( + f"+L{n} = {n}\n" for n in range(2, 12))) + llm = ScriptedLLM([_contract_reply([ + {"file": "mod_a.py", "line": n, "severity": "nit", + "comment": f"finding {n}", "evidence": "e"} + for n in range(2, 11) + ])]) + state = {"diff_text": diff, "task_spec": {"pr": 1}, + "repo_path": str(git_repo)} + + result = asyncio.run(_registry().get("agent.review_diff").handler( + _ctx(settings, trace, tmp_path, state, llm=llm))) + + assert result.ok + assert len(result.outputs["review_comments"]) == 8 + recorded = [r["disposition"] + for r in result.outputs["state_updates"][ + "review_finding_dispositions"]] + assert recorded.count("publish") == 8 + assert recorded.count("over_budget") == 1 + assert "excluded" not in recorded diff --git a/test/test_sdk_v1.py b/test/test_sdk_v1.py index cd284962..270f4b51 100644 --- a/test/test_sdk_v1.py +++ b/test/test_sdk_v1.py @@ -348,6 +348,6 @@ def test_strict_runtime_public_capabilities_smoke(tmp_path): caps = runtime.capabilities() assert caps.distribution_version == "0.2.0" - assert caps.strict_api_version == "1.0.0" + assert caps.strict_api_version == "1.1.0" assert caps.knowledge_api_version == KNOWLEDGE_API_VERSION assert caps.supports_knowledge_curation is True