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
35 changes: 35 additions & 0 deletions src/envelope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ pub struct Counts {
pub warn: u32,
pub error: u32,
pub suppressed: u32,
/// Findings the model reported that did not cite a changed line and were
/// dropped. Nonzero values are a model-quality signal worth tracking.
#[serde(default)]
pub ungrounded: u32,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down Expand Up @@ -104,6 +108,9 @@ pub struct Envelope {
pub gate: Gate,
pub model_used: String,
pub usage: Usage,
/// Wall-clock duration of the review engine run in milliseconds.
#[serde(default)]
pub duration_ms: u64,
pub base_sha: Option<String>,
pub head_sha: Option<String>,
pub since_sha: Option<String>,
Expand Down Expand Up @@ -164,6 +171,33 @@ pub fn fail_closed_finding(detail: &str) -> Finding {
}
}

/// The synthetic finding emitted when the model narrated merge-relevant risk
/// in its summary while reporting zero structured findings. The contradiction
/// means the output cannot be trusted as a pass; the narration is preserved so
/// the concern is not silently dropped. Uses OPERATIONAL_PATH: a malicious
/// diff can induce this shape via prompt injection, so it never bypasses the
/// gate.
pub fn narrated_risk_finding(summary: &str) -> Finding {
let quoted: String = summary.lines().map(|l| format!("> {l}\n")).collect();
Finding {
path: OPERATIONAL_PATH.to_string(),
line: 1,
end_line: None,
severity: Severity::Error,
kind: Kind::Uncertainty,
confidence: 1.0,
title: "Model narrated risk without structured findings".to_string(),
body: format!(
"The model's summary describes merge-relevant risk but it reported no \
structured findings, so the review cannot be trusted as a pass. Postil is \
failing closed instead of posting a clean status above contradictory prose.\n\n\
Narrated summary:\n\n{quoted}\n\
Re-run the review; if the contradiction persists, inspect the areas the \
summary names and address them or record findings manually."
),
}
}

/// The synthetic finding emitted when the provider could not be reached at all.
pub fn provider_error_finding(detail: &str) -> Finding {
Finding {
Expand Down Expand Up @@ -236,6 +270,7 @@ mod tests {
},
model_used: "m".into(),
usage: Usage::default(),
duration_ms: 0,
base_sha: None,
head_sha: None,
since_sha: None,
Expand Down
4 changes: 3 additions & 1 deletion src/forge/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,12 @@ impl Forge for GitHub {
"line": f.line,
"side": "RIGHT",
"body": format!(
"**{}** ({} / {} confidence)\n\n{}",
"{} **{}**\n`{}` · confidence {} · kind: {}\n\n{}",
super::severity_icon(f.severity),
f.title,
f.severity.as_str(),
super::format_confidence(f.confidence),
f.kind.as_str(),
f.body
),
});
Expand Down
45 changes: 36 additions & 9 deletions src/forge/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,28 @@ pub mod gitlab;

use anyhow::Result;

use crate::envelope::{Envelope, Finding};
use crate::envelope::{Envelope, Finding, Severity};

/// Base URL for the brand status icons rendered in PR comments and check
/// summaries. The four icons (error, warn, info, pass) are served by the
/// marketing site and mirror the product-page statusline.
pub const STATUS_ICON_BASE: &str = "https://postil.dev/status";

/// Markdown `<img>` for a named status icon, sized to sit inline with text.
pub fn icon_md(name: &str) -> String {
format!(
"<img src=\"{STATUS_ICON_BASE}/{name}.svg\" width=\"14\" height=\"14\" \
alt=\"{name}\" align=\"text-bottom\">"
)
}

pub fn severity_icon(severity: Severity) -> String {
icon_md(match severity {
Severity::Error => "error",
Severity::Warn => "warn",
Severity::Info => "info",
})
}

#[derive(Debug, Clone)]
pub struct PrMeta {
Expand Down Expand Up @@ -69,29 +90,33 @@ pub fn check_title(envelope: &Envelope) -> String {
pub fn check_summary(envelope: &Envelope) -> String {
let mut s = String::new();
if envelope.silent {
s.push_str(
"Postil reviewed this change and found nothing that affects the merge decision.\n",
);
s.push_str(&format!(
"{} Postil reviewed this change and found nothing that affects the merge \
decision.\n",
icon_md("pass")
));
} else {
if !envelope.summary.is_empty() {
s.push_str(&envelope.summary);
s.push_str("\n\n");
}
for f in &envelope.findings {
s.push_str(&format!(
"- **{}** `{}:{}` [{}/{}] {}\n",
"- {} **{}** `{}:{}` — {} · confidence {} · kind: {}\n",
severity_icon(f.severity),
f.severity.as_str(),
f.path,
f.line,
f.kind.as_str(),
f.title,
format_confidence(f.confidence),
f.title
f.kind.as_str(),
));
}
}
if !envelope.resolved.is_empty() {
s.push_str(&format!(
"\n{} finding(s) from the previous review resolved.\n",
"\n{} {} finding(s) from the previous review resolved.\n",
icon_md("pass"),
envelope.resolved.len()
));
}
Expand All @@ -105,6 +130,8 @@ pub fn check_summary(envelope: &Envelope) -> String {
s
}

/// Confidence rendered as the product statusline shows it: a bare decimal
/// probability ("0.91"), not a percentage.
pub fn format_confidence(c: f64) -> String {
format!("{:.0}%", c * 100.0)
format!("{:.2}", c)
}
41 changes: 36 additions & 5 deletions src/llm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ impl LlmClient {
) -> Result<ModelReview> {
let mut usage = Usage::default();
let content = self.chat(model, system, user, &mut usage).await?;
match parse_review(&content) {
Ok(raw) => Ok(into_review(raw, model, usage)),
let raw = match parse_review(&content) {
Ok(raw) => raw,
Err(parse_err) => {
// One repair attempt: ask the same model to fix its own JSON.
let repair_user = format!(
Expand All @@ -198,11 +198,42 @@ impl LlmClient {
)
.await
.context("JSON repair call failed")?;
let raw = parse_review(&repaired)
.map_err(|e| anyhow!("model output invalid after repair: {e}"))?;
Ok(into_review(raw, model, usage))
parse_review(&repaired)
.map_err(|e| anyhow!("model output invalid after repair: {e}"))?
}
};
let mut review = into_review(raw, model, usage);

// Semantic consistency retry: a summary that narrates risk next to an
// empty findings array is the contract violation behind "clean status,
// scary prose" reviews. Give the model one chance to either structure
// the risk or retract it; if the contradiction survives, the caller
// fails the review closed.
if review.findings.is_empty() && !review.summary.is_empty() {
let retry_user = format!(
"{user}\n\n[Your previous response]\n{content}\n\n[Correction] Your summary \
describes merge-relevant risk but `findings` is empty, which is invalid. \
Either report each risk as a structured finding citing its exact new-file \
line from the diff above, or — if nothing is actually merge-relevant — \
return exactly {{\"summary\": \"\", \"findings\": []}}."
);
let mut retry_usage = usage;
if let Ok(retried) = self
.chat(model, system, &retry_user, &mut retry_usage)
.await
&& let Ok(retried_raw) = parse_review(&retried)
{
let candidate = into_review(retried_raw, model, retry_usage);
let still_contradictory =
candidate.findings.is_empty() && !candidate.summary.is_empty();
if still_contradictory {
review.usage = retry_usage;
} else {
review = candidate;
}
}
}
Ok(review)
}

async fn chat(
Expand Down
39 changes: 24 additions & 15 deletions src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub fn print_pretty(envelope: &Envelope) {
if envelope.silent {
out.push_str(&paint(
color,
"postil: no merge-relevant findings. Staying silent.\n",
"✓ postil: no merge-relevant findings. Staying silent.\n",
Paint::Green,
));
} else {
Expand All @@ -28,46 +28,55 @@ pub fn print_pretty(envelope: &Envelope) {
}
for f in &envelope.findings {
let (glyph, p) = match f.severity {
Severity::Error => ("error", Paint::Red),
Severity::Warn => ("warn ", Paint::Yellow),
Severity::Info => ("info ", Paint::Blue),
Severity::Error => ("✕ error", Paint::Red),
Severity::Warn => ("▲ warn ", Paint::Yellow),
Severity::Info => ("ℹ info ", Paint::Blue),
};
out.push_str(&format!(
"{} {}:{} {}\n",
"{} {}:{}\n",
paint(color, glyph, p),
f.path,
f.line,
f.title
f.line
));
out.push_str(&format!(" {}\n", f.title));
for line in f.body.lines() {
out.push_str(&format!(" {line}\n"));
out.push_str(&format!(" {line}\n"));
}
out.push_str(&format!(
" kind: {} confidence: {:.0}%\n\n",
f.kind.as_str(),
f.confidence * 100.0
" (confidence {:.2}, kind: {})\n\n",
f.confidence,
f.kind.as_str()
));
}
let n = envelope.findings.len();
let mut tally = format!("{n} finding{}", if n == 1 { "" } else { "s" });
if envelope.counts.suppressed > 0 {
tally.push_str(&format!(
" · {} suppressed by policy",
envelope.counts.suppressed
));
}
out.push_str(&format!("{tally}\n"));
}
if !envelope.resolved.is_empty() {
out.push_str(&format!(
"{} finding(s) from the previous review resolved.\n",
"✓ {} finding(s) from the previous review resolved.\n",
envelope.resolved.len()
));
}
if envelope.counts.suppressed > 0 {
if envelope.silent && envelope.counts.suppressed > 0 {
out.push_str(&format!(
"{} finding(s) suppressed by policy.\n",
envelope.counts.suppressed
));
}
let gate = if envelope.gate.failing {
paint(color, "gate: FAILING", Paint::Red)
paint(color, "gate: failing", Paint::Red)
} else {
paint(color, "gate: passing", Paint::Green)
};
out.push_str(&format!(
"{gate} (failOn: {}) model: {}\n",
"{gate} (fail-on: {}) model: {}\n",
envelope.gate.fail_on, envelope.model_used
));
eprint!("{out}");
Expand Down
1 change: 1 addition & 0 deletions src/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ mod tests {
},
model_used: "m".into(),
usage: Usage::default(),
duration_ms: 0,
base_sha: None,
head_sha: None,
since_sha: None,
Expand Down
7 changes: 6 additions & 1 deletion src/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ pub fn system_prompt(cfg: &Config) -> String {
\"findings\": [{\"path\": \"file path from the diff\", \"line\": <new-file line>,\n \
\"endLine\": <optional>, \"severity\": \"info|warn|error\",\n \
\"kind\": \"risk|humanEscalation|guardrail|uncertainty\", \"confidence\": <0..1>,\n \
\"title\": \"short imperative title\", \"body\": \"specific, evidence-based markdown\"}]}\n",
\"title\": \"short imperative title\", \"body\": \"specific, evidence-based markdown\"}]}\n\
\n\
The summary and findings must agree. Every risk the summary mentions MUST appear as \
a structured finding with its diff line; if findings is empty, summary MUST be the \
empty string. A summary that narrates problems alongside an empty findings array is \
invalid output and will fail the review.\n",
);
p
}
Expand Down
17 changes: 16 additions & 1 deletion src/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ async fn review_diff(
None => Vec::new(),
};

let review_started = std::time::Instant::now();
let parsed = diff::parse(diff_text);
let index = DiffIndex::build(&parsed);
let incremental = args.since_sha.is_some();
Expand All @@ -242,6 +243,7 @@ async fn review_diff(
let mut model_used = "none (empty diff)".to_string();
let mut usage = Usage::default();
let mut suppressed = 0u32;
let mut ungrounded = 0u32;
let mut findings: Vec<Finding> = Vec::new();

if !cfg.enabled {
Expand All @@ -265,15 +267,25 @@ async fn review_diff(
let client = LlmClient::from_env(cfg)?;
match client.review(cfg, &system, &user).await {
Ok(model_review) => {
let raw_findings = model_review.findings.len();
let outcome = filter::apply(cfg, &index, model_review.findings)?;
model_used = model_review.model_used;
usage = model_review.usage;
suppressed = outcome.suppressed;
ungrounded = outcome.ungrounded;
if outcome.all_ungrounded {
findings = vec![fail_closed_finding(&format!(
"model reported {} finding(s), none grounded in the diff",
outcome.ungrounded
))];
} else if raw_findings == 0 && !model_review.summary.trim().is_empty() {
// Risk narrated in prose with zero structured findings
// (post-retry). Passing this as clean is the predecessor
// product's worst failure mode; fail closed instead and
// carry the narration into the finding so it is not lost.
findings = vec![crate::envelope::narrated_risk_finding(
&model_review.summary,
)];
} else {
summary = model_review.summary;
findings = outcome.kept;
Expand Down Expand Up @@ -329,7 +341,8 @@ async fn review_diff(
&& !(advisory_on_error && f.path == crate::envelope::PROVIDER_PATH)
});
let silent = findings.is_empty();
let counts = Envelope::counts_of(&findings, suppressed);
let mut counts = Envelope::counts_of(&findings, suppressed);
counts.ungrounded = ungrounded;
let buckets = Envelope::buckets_of(&findings);

Ok(Envelope {
Expand All @@ -346,6 +359,7 @@ async fn review_diff(
},
model_used,
usage,
duration_ms: review_started.elapsed().as_millis() as u64,
base_sha: meta.map(|m| m.base_sha.clone()),
head_sha,
since_sha: args.since_sha.clone(),
Expand Down Expand Up @@ -424,6 +438,7 @@ fn error_envelope(cfg: &Config, err: &anyhow::Error, head_sha: &str, meta: &PrMe
},
model_used: cfg.model_chain().join(" -> "),
usage: Usage::default(),
duration_ms: 0,
base_sha: Some(meta.base_sha.clone()),
head_sha: Some(head_sha.to_string()),
since_sha: None,
Expand Down
1 change: 1 addition & 0 deletions src/sarif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ mod tests {
},
model_used: "m".into(),
usage: Usage::default(),
duration_ms: 0,
base_sha: None,
head_sha: None,
since_sha: None,
Expand Down
Loading
Loading