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
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "postil-cli"
version = "0.8.20"
version = "0.8.21"
edition = "2024"
description = "Postil: a low-noise AI review gate. Silent on clean PRs, hard gate on real risk."
license = "Apache-2.0"
Expand Down
2 changes: 1 addition & 1 deletion bench/baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"schemaVersion": 1,
"corpus": {
"fixtureCorpusSha256": "8e4c2cb9ad5a7efdfe6a875566d20133e905155b6f693a873595adf6c069e065",
"evaluatorSha256": "e1c5b005a5e516b79866b4c6e98df54e806d424b923b8c9797e3930ff3026874"
"evaluatorSha256": "ebc72a8d7d60c08bf79759a0dfc5dbf821a9e6f79f4503c08ea9660dddece4d7"
},
"profiles": {
"z-ai/glm-5.2": {
Expand Down
27 changes: 27 additions & 0 deletions bench/src/live.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
runLive,
resolveLiveTimeoutOverrides,
scorerOperationalFailure,
targetSuppressionReasons,
validateLiveRunId,
} from "./live";

Expand Down Expand Up @@ -49,6 +50,32 @@ async function onlyCaseAttempt(runRoot: string): Promise<string> {
}

describe("live benchmark review mode", () => {
test("reports why an authored target was suppressed", () => {
const truth = {
clean: false,
path: "src/api/orders/route.ts",
startLine: 31,
endLine: 31,
severity: "error",
};
const finding = {
path: truth.path,
line: truth.startLine,
severity: "warn" as const,
confidence: 0.8,
kind: "risk" as const,
title: "Restore the response field",
body: "The response field is missing.",
};

expect(targetSuppressionReasons({
suppressedFindings: [
{ finding, reason: "repositoryClaimUnsupported" },
{ finding: { ...finding, line: 40 }, reason: "belowThreshold" },
],
}, truth)).toEqual(["repositoryClaimUnsupported"]);
});

test("forwards explicit timeout overrides to the isolated child and records them", async () => {
const root = await mkdtemp(join(tmpdir(), "postil-live-timeout-forwarding-"));
const names = [
Expand Down
35 changes: 30 additions & 5 deletions bench/src/live.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ export interface LiveCaseResult {
observedProviderCostUsdDecimal: string | null;
costAccountingComplete: boolean;
reviewCoverage: Envelope["reviewCoverage"] | null;
/** Suppression reasons for findings that overlapped the authored target.
* Diagnostic only: suppressed findings never count as detections. */
suppressedTargetReasons?: string[];
exitCode: number | undefined;
error?: string;
/** Captured stderr of the binary run. Used internally to classify transient
Expand Down Expand Up @@ -541,6 +544,7 @@ async function runLiveCase(
observedProviderCostUsdDecimal: null,
costAccountingComplete: false,
reviewCoverage: null,
suppressedTargetReasons: [],
exitCode: undefined,
};

Expand Down Expand Up @@ -824,10 +828,9 @@ function scoreCase(base: LiveCaseResult, truth: GroundTruth, env: Envelope): Liv
return base;
}

const match = findings.find((finding) =>
finding.path === truth.path &&
Math.min(finding.line, finding.endLine ?? finding.line) <= truth.endLine! &&
Math.max(finding.line, finding.endLine ?? finding.line) >= truth.startLine!);
base.suppressedTargetReasons = targetSuppressionReasons(env, truth);

const match = findings.find((finding) => findingMatchesTruth(finding, truth));
base.detected = Boolean(match);
if (match) {
base.foundSeverity = match.severity;
Expand All @@ -842,6 +845,25 @@ function scoreCase(base: LiveCaseResult, truth: GroundTruth, env: Envelope): Liv
return base;
}

function findingMatchesTruth(
finding: Pick<Envelope["findings"][number], "path" | "line" | "endLine">,
truth: GroundTruth,
): boolean {
return finding.path === truth.path &&
Math.min(finding.line, finding.endLine ?? finding.line) <= truth.endLine! &&
Math.max(finding.line, finding.endLine ?? finding.line) >= truth.startLine!;
}

export function targetSuppressionReasons(
env: Pick<Envelope, "suppressedFindings">,
truth: GroundTruth,
): string[] {
if (truth.clean) return [];
return env.suppressedFindings
.filter(({ finding }) => findingMatchesTruth(finding, truth))
.map(({ reason }) => reason);
}

function groundTruthOf(c: ReturnType<typeof benchmarkCase.parse>): GroundTruth {
const gt = c.groundTruth.findings[0];
if (!gt) return { clean: true, path: null, startLine: null, endLine: null, severity: null };
Expand Down Expand Up @@ -1013,7 +1035,10 @@ export function formatLiveReport(report: LiveReport): string {
? `${r.truthSeverity}->${r.foundSeverity}${severityTag(r)} conf=${fmt(r.confidence)}`
: `${r.truthSeverity}`;
const fp = r.falsePositives > 0 ? ` +${r.falsePositives} FP` : "";
lines.push(`${tag} ${r.id}: ${sev}${fp}`);
const suppressed = r.detected || !r.suppressedTargetReasons?.length
? ""
: ` suppressed-target=${r.suppressedTargetReasons.join(",")}`;
lines.push(`${tag} ${r.id}: ${sev}${fp}${suppressed}`);
}
}
return lines.join("\n");
Expand Down
13 changes: 6 additions & 7 deletions src/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ pub fn review_contract(cfg: &Config) -> String {
`.postil/change-metadata`. Findings citing other lines are discarded as \
ungrounded.\n\
\n\
`repositoryContext` is optional and defaults to a diff-local finding. Omit it whenever \
the exact cited changed line is sufficient to establish the defect, even if unchanged \
code elsewhere could also be relevant. Include it only when the conclusion depends on \
`repositoryContext` is optional. Omit it for bugs the cited line establishes, including \
removed fields, bypassed guards, boundary errors, or lifecycle defects; caller or consumer \
impact alone doesn't require it. Include it only when the conclusion depends on \
repository-wide evidence, using `claim: absence` for a construct missing from the \
complete reviewed head or `claim: mismatch` for a repository target whose expected \
value is not established by the cited changed line. When included, name the target in \
Expand Down Expand Up @@ -547,12 +547,11 @@ mod tests {
#[test]
fn generator_omits_repository_claims_for_diff_local_conclusions() {
let prompt = system_prompt(&Config::default(), trusted_date());
assert!(
prompt.contains("`repositoryContext` is optional and defaults to a diff-local finding")
);
assert!(prompt.contains("`repositoryContext` is optional"));
assert!(prompt.contains(
"Omit it whenever the exact cited changed line is sufficient to establish the defect"
"Omit it for bugs the cited line establishes, including removed fields, bypassed guards, boundary errors, or lifecycle defects"
));
assert!(prompt.contains("caller or consumer impact alone doesn't require it"));
assert!(
prompt.contains(
"Include it only when the conclusion depends on repository-wide evidence"
Expand Down
25 changes: 24 additions & 1 deletion src/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ fn review_batch_validation_reason(
if let Some(category) = crate::envelope::publication_evidence_boundary_category(finding) {
let repair = match category {
"delegatedEvidenceCollection" => {
"do not delegate repository inspection to a human; declare the exact absence or mismatch through `repositoryContext` so Postil can search the complete reviewed head, or retract the finding"
"if the cited changed line already establishes the defect, state it directly and omit `repositoryContext`; otherwise do not delegate repository inspection to a human, declare the exact absence or mismatch through `repositoryContext` so Postil can search the complete reviewed head, or retract the finding"
}
"reviewArtifactPhrase" | "reviewArtifactBoundary" => {
"remove review-process terms such as `diff`, `patch`, `PR`, `MR`, `review input`, and `provided context`, state the defect directly from the concrete repository construct with an actionable fix, and retract the finding if the supplied evidence is insufficient"
Expand Down Expand Up @@ -4195,6 +4195,29 @@ mod tests {
assert_eq!(failure.safe_detail(), "reviewArtifactPhrase=1");
}

#[test]
fn delegated_evidence_repair_preserves_directly_established_defects() {
let annotated = "### src/lib.rs\n@@ fixture @@\n 7 + changed();\n";
let mut finding = finding(
"src/lib.rs",
7,
"Verify that `applyBulkEdit` internally enforces the permission check.",
);
finding.evidence = Some("changed();".to_string());

let reason = review_batch_validation_reason(&finding, annotated, None).unwrap();

assert_eq!(reason.category, "delegatedEvidenceCollection");
assert!(reason.repair_detail.contains(
"if the cited changed line already establishes the defect, state it directly and omit `repositoryContext`"
));
assert!(
reason
.repair_detail
.contains("declare the exact absence or mismatch through `repositoryContext`")
);
}

#[test]
fn all_ungrounded_failure_uses_public_evidence_validation_language() {
let finding = ungrounded_findings_failure(3);
Expand Down