Skip to content

#235: detector precision — stop mis-flagging correct external reviews #237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
seungpyoson merged 16 commits into from
Jun 25, 2026
Merged

#235: detector precision — stop mis-flagging correct external reviews #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
9627c33
fix(review): tighten 3 review-quality detectors to stop mis-flagging …
Jun 21, 2026
3112e08
fix(review): parametrize root2 corpus over plugin copies to restore c…
Jun 21, 2026
e7e0224
fix(review): bound CONCRETE_FINDING_CODE_LOCUS quantifiers to clear S…
Jun 21, 2026
070e52d
fix(review): resolve two detector bugs from PR #237 review (multi-dot…
Jun 21, 2026
a2a7be1
fix(review): pattern-level hardening of shallow_output + permission_b…
Jun 21, 2026
a2d77aa
refactor(review): split detector regexes under SonarCloud complexity …
Jun 22, 2026
a5c2868
refactor(review): split remaining detector regexes strictly under com…
Jun 22, 2026
8988e37
fix(relay): revert spoofable not_reviewed foreign-path suppression
Jun 22, 2026
b75e10d
fix(relay): revert leaky permission-praise suppressor; complete detec…
Jun 22, 2026
1365a1e
Merge main into fix/232-root2-discard (#237)
Jun 24, 2026
32cd266
fix: ground concise review loci in selected source
Jun 24, 2026
83aabfa
test: cover agy review prompt copy
Jun 24, 2026
0168a25
fix: require source grounding for concise concrete reviews
Jun 24, 2026
5b676a1
revert: drop unrequested defect-cue lexicon expansion
Jun 24, 2026
1e4aab7
revert: drop blocked concise-review source-grounding (#235)
Jun 25, 2026
b6f2524
fix(review): remove spoofable conciseConcreteReview exemption (#235)
Jun 25, 2026
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
14 changes: 13 additions & 1 deletion plugins/agy/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
14 changes: 13 additions & 1 deletion plugins/api-reviewers/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
14 changes: 13 additions & 1 deletion plugins/claude/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
14 changes: 13 additions & 1 deletion plugins/gemini/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
14 changes: 13 additions & 1 deletion plugins/grok/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
14 changes: 13 additions & 1 deletion plugins/kimi/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
14 changes: 13 additions & 1 deletion relay/relay-agy/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
14 changes: 13 additions & 1 deletion relay/relay-gemini/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
14 changes: 13 additions & 1 deletion relay/relay-grok/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
14 changes: 13 additions & 1 deletion relay/relay-kimi/scripts/lib/review-prompt.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,12 @@ function isSelectedSourceInspectionMechanicsDiscussionLine(lower) {
]);
}

// A could-not-inspect line is suppressed ONLY when the reviewer EXPLICITLY marks it
// out of scope. Inferring out-of-scope from "a foreign file is named + the selected
// source was (claimed) inspected" was tried (PR #237) and reverted: any text-based
// inspection signal is spoofable, and the only non-spoofable alternative (named file
// is provably not in the packet) silently suppresses a no-engagement APPROVE, which
// breaks the fail-toward-flagging invariant. The explicit marker is the contract.
function isOutOfScopeInspectionGapLine(lower) {
if (!includesAny(lower, ["could not inspect", "unable to inspect", "not inspected", "not reviewed"])) return false;
return includesAny(lower, [
Expand Down Expand Up @@ -1152,7 +1158,8 @@ function semanticFailureReasons(text, looksShallow, selectedSource = null) {
const semanticText = semanticLines
.filter((line) => {
const lower = unmarkReviewText(line).toLowerCase();
return !isOutOfScopeInspectionGapLine(lower) && !isPriorReviewCommentsGapLine(lower);
return !isOutOfScopeInspectionGapLine(lower)
&& !isPriorReviewCommentsGapLine(lower);
})
.join("\n")
.toLowerCase();
Expand Down Expand Up @@ -1253,6 +1260,11 @@ function isNegatedPermissionBlockLine(line) {
);
}

// Used only by the tiny-source concise-review exemption (conciseTinyReview). This is a best-effort
// signal, NOT a security gate: a text inspection claim is spoofable, so it must never be used to
// SUPPRESS a not_reviewed flag (that was the reverted PR #237 foreign-path branch). Here it only
// grants a length exemption to an already-structured review of a <512-byte source, where a false
// claim buys nothing material.
function mentionsSelectedSourceInspection(lowerText, selectedSource) {
if (!includesAny(lowerText, SELECTED_SOURCE_INSPECTION_VERBS)) return false;
return mentionsSelectedSourcePath(lowerText, selectedSource);
Expand Down
Loading
Loading