Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion integrations/cursor/imreview.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,22 @@ preflight passes, run targeted tests and low-cost static checks alongside the
source review.
Stop when every changed semantic path has a supported finding or an explicit
no-issue conclusion; do not add searches only for confidence.
After source review independently verifies its candidate findings, fetch a
bounded snapshot of existing PR conversation comments, review summaries, and
thread-aware inline review threads. Do not use existing feedback to skip source
paths. Classify every candidate as `new`, `duplicate`, `extends_existing`, or
`resolved_or_outdated`; suppress duplicates, and point extensions to the
existing thread instead of opening a parallel comment. If feedback cannot be
read, report that validation gap. For local/worktree reviews, mark feedback as
not applicable.

Do not wait for CI completion or resolved mergeability before the progress
update. Mark early findings as preliminary and continue the review. This update
is not a GitHub comment; do not post an interim review.

Before finalizing, call `validate_direct_review` with the pinned head SHA as
`evidence_head_sha` and classify `subtraction_signal`. Use `none` without a
`evidence_head_sha`, the feedback status and candidate dispositions, and
classify `subtraction_signal`. Use `none` without a
minimality proof when the diff does not add or expand a helper, class, fallback,
compatibility branch, or public behavior. Use `triggered` for those changes and
only then provide subtraction evidence.
Expand Down
11 changes: 10 additions & 1 deletion plugins/infermatrix-copilot/skills/imreview/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,20 @@ preflight passes, run targeted tests and low-cost static checks alongside the
source review.
Stop when every changed semantic path has a supported finding or an explicit
no-issue conclusion; do not add searches only for confidence.
After source review independently verifies its candidate findings, fetch a
bounded snapshot of existing PR conversation comments, review summaries, and
thread-aware inline review threads. Do not use existing feedback to skip source
paths. Classify every candidate as `new`, `duplicate`, `extends_existing`, or
`resolved_or_outdated`; suppress duplicates, and point extensions to the
existing thread instead of opening a parallel comment. If feedback cannot be
read, report that validation gap. For local/worktree reviews, mark feedback as
not applicable.
Do not wait for CI completion or resolved mergeability before the progress
update. Mark early findings as preliminary and continue the review. This update
is not a GitHub comment; do not post an interim review.
Before finalizing, call `validate_direct_review` with the pinned head SHA as
`evidence_head_sha` and classify `subtraction_signal`. Use `none` without a
`evidence_head_sha`, the feedback status and candidate dispositions, and
classify `subtraction_signal`. Use `none` without a
minimality proof when the diff does not add or expand a helper, class, fallback,
compatibility branch, or public behavior. Use `triggered` for those changes and
only then provide subtraction evidence.
Expand Down
83 changes: 81 additions & 2 deletions src/infermatrix_copilot/thin_mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ def _knowledge_root() -> Path:
"At native dependency boundaries, verify the pinned API contract and caller inputs; native reuse does not prove the caller's budget, ordering, adapter, or lifecycle correctness.",
"For feature-gated behavior, audit the enabled path independently; a safe disabled/default path limits blast radius but does not prove the new path correct.",
"Stop investigating when every changed semantic path has a supported finding or explicit no-issue conclusion; do not add searches only for confidence.",
"After candidate findings are evidence-verified, for PR targets fetch bounded conversation comments, review summaries, and thread-aware review threads. Keep source discovery independent: existing feedback is a final deduplication input, not a reason to skip changed semantic paths.",
"Classify every candidate finding as new, duplicate, extends_existing, or resolved_or_outdated. Suppress duplicates; for extensions, point to the existing thread instead of opening a parallel inline comment. Record unavailable PR feedback as a validation gap; use not_applicable only for local/worktree reviews.",
"Run subtraction only when the diff adds or expands a helper, class, fallback, compatibility branch, or public behavior; otherwise mark no subtraction signal.",
"Plan exactly one consolidated final review comment.",
]
Expand Down Expand Up @@ -217,6 +219,13 @@ def _knowledge_root() -> Path:
}
_SUBTRACTION_ACTIONS = {"DELETE", "DEFER", "INLINE", "MERGE", "MOVE"}
_SUBTRACTION_SIGNALS = {"none", "triggered"}
_FEEDBACK_STATUSES = {"checked", "unavailable", "not_applicable"}
_FINDING_DISPOSITIONS = {
"new",
"duplicate",
"extends_existing",
"resolved_or_outdated",
}
_EVIDENCE_HEAD_SHA = re.compile(r"[0-9a-f]{7,40}")


Expand Down Expand Up @@ -683,6 +692,8 @@ def _direct_completion_result(
minimality_proof: dict[str, str] | None = None,
final_comment_count: int = 1,
evidence_head_sha: str = "",
existing_feedback_status: str = "not_applicable",
finding_dispositions: list[dict[str, str]] | None = None,
) -> dict:
"""Mechanically gate Direct completion on subtraction classification.

Expand All @@ -698,6 +709,8 @@ def _direct_completion_result(
subtraction = subtraction or []
minimality_proof = minimality_proof or {}
evidence_head_sha = str(evidence_head_sha).strip().casefold()
existing_feedback_status = str(existing_feedback_status).strip().casefold()
finding_dispositions = finding_dispositions or []
missing: list[str] = []

if final_comment_count != 1:
Expand All @@ -713,6 +726,43 @@ def _direct_completion_result(
"validation result was read at"
)

if existing_feedback_status not in _FEEDBACK_STATUSES:
missing.append(
"existing_feedback_status must be 'checked', 'unavailable', or "
"'not_applicable'"
)

malformed_dispositions: list[int] = []
for index, item in enumerate(finding_dispositions):
if not isinstance(item, dict):
malformed_dispositions.append(index)
continue
anchor = str(item.get("anchor", "")).strip()
disposition = str(item.get("disposition", "")).strip().casefold()
existing_thread = str(item.get("existing_thread", "")).strip()
needs_thread = disposition in {
"duplicate",
"extends_existing",
"resolved_or_outdated",
}
if (
":" not in anchor
or disposition not in _FINDING_DISPOSITIONS
or (needs_thread and not existing_thread)
):
malformed_dispositions.append(index)
if malformed_dispositions:
missing.append(
"each finding disposition needs a path:line anchor, a valid "
"new/duplicate/extends_existing/resolved_or_outdated disposition, "
"and existing_thread for every non-new item "
f"(invalid indexes: {malformed_dispositions})"
)
if existing_feedback_status != "checked" and finding_dispositions:
missing.append(
"finding_dispositions require existing_feedback_status='checked'"
)

malformed_subtractions: list[int] = []
for index, item in enumerate(subtraction):
if not isinstance(item, dict):
Expand Down Expand Up @@ -773,9 +823,18 @@ def _direct_completion_result(
"subtraction_required": subtraction_signal == "triggered",
"subtraction_items": len(subtraction),
"minimality_proof": has_minimality_proof,
"existing_feedback_status": existing_feedback_status,
"finding_dispositions": len(finding_dispositions),
"duplicate_findings_suppressed": sum(
1
for item in finding_dispositions
if isinstance(item, dict)
and str(item.get("disposition", "")).strip().casefold()
in {"duplicate", "resolved_or_outdated"}
),
"missing": missing,
"next_action": (
"Return the single consolidated review comment."
"Return the single consolidated review comment with duplicate findings suppressed."
if complete
else "Classify the subtraction signal; only a triggered diff needs one bounded subtraction pass using the existing evidence packet."
),
Expand Down Expand Up @@ -852,7 +911,12 @@ def build_mcp(
"evidence at the frozen head SHA; fetch the PR head ref when the "
"local checkout holds another revision. Before treating a Direct "
"review as complete or posting its only final comment, call "
"validate_direct_review with that evidence_head_sha. Mark "
"validate_direct_review with that evidence_head_sha. After source "
"findings are independently verified, fetch bounded PR discussion "
"and thread-aware review feedback, classify every candidate, and "
"suppress duplicates. Pass existing_feedback_status=checked for "
"that PR path, unavailable when the fetch fails, or not_applicable "
"only for local/worktree reviews. Mark "
"subtraction_signal=none when the diff "
"does not add or expand a helper, class, fallback, compatibility "
"branch, or public behavior. Only subtraction_signal=triggered "
Expand Down Expand Up @@ -957,6 +1021,12 @@ def run() -> dict:
"completion_gate": {
"tool": "validate_direct_review",
"evidence_head_sha": "Required: the frozen head commit SHA every cited source file and validation result was read at; fetch the PR head ref when the local checkout holds another revision.",
"existing_feedback_status": {
"checked": "PR feedback was fetched after independent source verification and every candidate was classified.",
"unavailable": "PR feedback could not be fetched; report this validation gap.",
"not_applicable": "The target is a local/worktree review without a PR.",
},
"finding_dispositions": "For checked PR reviews: [{anchor, disposition, existing_thread?}] where disposition is new, duplicate, extends_existing, or resolved_or_outdated.",
"subtraction_signal": {
"none": "No helper/class/fallback/compatibility/public-behavior expansion; no subtraction evidence required.",
"triggered": "Require subtraction items or minimality_proof.",
Expand Down Expand Up @@ -1015,6 +1085,8 @@ def validate_direct_review(
minimality_proof: dict[str, str] | None = None,
final_comment_count: int = 1,
evidence_head_sha: str = "",
existing_feedback_status: str = "",
finding_dispositions: list[dict[str, str]] | None = None,
) -> dict:
"""Validate the Direct completion gate before the only final comment.

Expand All @@ -1025,6 +1097,11 @@ def validate_direct_review(
the frozen head commit every cited source file and validation result
was read at; a review whose evidence came from another revision is not
complete. A ``partial_review`` result is not a completed Direct review.
For PR reviews, fetch existing feedback only after independently
verifying candidate findings, then pass ``existing_feedback_status``
and classify each candidate in ``finding_dispositions``. Duplicate and
resolved/outdated findings must identify the existing thread and must
not be emitted as new comments.
"""
started = time.perf_counter()
result = _direct_completion_result(
Expand All @@ -1033,6 +1110,8 @@ def validate_direct_review(
minimality_proof=minimality_proof,
final_comment_count=final_comment_count,
evidence_head_sha=evidence_head_sha,
existing_feedback_status=existing_feedback_status,
finding_dispositions=finding_dispositions,
)
result.setdefault("diagnostics", {})["timing_ms"] = {
"validate_direct_review": int(
Expand Down
91 changes: 91 additions & 0 deletions test/test_thin_mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,14 @@ def test_direct_entrypoints_do_not_resolve_repo(monkeypatch):
and "fetch the PR head ref" in item
for item in review["first_review_checklist"]
)
assert any(
"existing feedback is a final deduplication input" in item
for item in review["first_review_checklist"]
)
assert any(
"extends_existing" in item and "Suppress duplicates" in item
for item in review["first_review_checklist"]
)
assert review["progress_update"] == {
"deadline_seconds": 60,
"channel": "host_conversation",
Expand All @@ -172,6 +180,12 @@ def test_direct_entrypoints_do_not_resolve_repo(monkeypatch):
assert review["completion_gate"] == {
"tool": "validate_direct_review",
"evidence_head_sha": "Required: the frozen head commit SHA every cited source file and validation result was read at; fetch the PR head ref when the local checkout holds another revision.",
"existing_feedback_status": {
"checked": "PR feedback was fetched after independent source verification and every candidate was classified.",
"unavailable": "PR feedback could not be fetched; report this validation gap.",
"not_applicable": "The target is a local/worktree review without a PR.",
},
"finding_dispositions": "For checked PR reviews: [{anchor, disposition, existing_thread?}] where disposition is new, duplicate, extends_existing, or resolved_or_outdated.",
"subtraction_signal": {
"none": "No helper/class/fallback/compatibility/public-behavior expansion; no subtraction evidence required.",
"triggered": "Require subtraction items or minimality_proof.",
Expand Down Expand Up @@ -406,6 +420,83 @@ def test_small_fix_accepts_no_subtraction_signal_without_full_proof():
assert result["evidence_head_sha"] == _HEAD_SHA


def test_pr_review_requires_feedback_status():
result = _direct_completion_result(
subtraction_signal="none",
evidence_head_sha=_HEAD_SHA,
existing_feedback_status="",
)

assert result["status"] == "partial_review"
assert result["publish_ready"] is False
assert result["missing"] == [
(
"existing_feedback_status must be 'checked', 'unavailable', or "
"'not_applicable'"
)
]


def test_checked_feedback_counts_and_suppresses_duplicate_dispositions():
result = _direct_completion_result(
subtraction_signal="none",
evidence_head_sha=_HEAD_SHA,
existing_feedback_status="checked",
finding_dispositions=[
{
"anchor": "src/runtime.py:42",
"disposition": "duplicate",
"existing_thread": "PRRT_duplicate",
},
{
"anchor": "src/runtime.py:91",
"disposition": "extends_existing",
"existing_thread": "PRRT_extension",
},
{
"anchor": "src/config.py:12",
"disposition": "new",
},
],
)

assert result["status"] == "complete"
assert result["finding_dispositions"] == 3
assert result["duplicate_findings_suppressed"] == 1


def test_non_new_disposition_requires_existing_thread():
result = _direct_completion_result(
subtraction_signal="none",
evidence_head_sha=_HEAD_SHA,
existing_feedback_status="checked",
finding_dispositions=[{
"anchor": "src/runtime.py:42",
"disposition": "resolved_or_outdated",
}],
)

assert result["status"] == "partial_review"
assert "existing_thread" in result["missing"][0]


def test_feedback_dispositions_require_checked_status():
result = _direct_completion_result(
subtraction_signal="none",
evidence_head_sha=_HEAD_SHA,
existing_feedback_status="unavailable",
finding_dispositions=[{
"anchor": "src/runtime.py:42",
"disposition": "new",
}],
)

assert result["status"] == "partial_review"
assert result["missing"] == [
"finding_dispositions require existing_feedback_status='checked'"
]


def test_triggered_subtraction_requires_evidence():
result = _direct_completion_result(
subtraction_signal="triggered",
Expand Down
Loading