Nirav PatelandClaude Fable 5
e32c600215
- push reviews now fetch ONE compare diff (before...after) instead of
slicing to the first 20 commits under a head-sha status; new-branch
pushes (zero before-sha) fall back to the head commit diff
- truncated diffs can no longer yield a clean pass: verdict capped at
warn and status description prefixed 'partial review (diff truncated)'
- any diff-fetch failure (throw / empty body on a non-empty push or PR)
short-circuits to status 'warning' ('review unavailable — diff fetch
failed (not blocking)') with a ledger row; a diffless prompt is never
sent to the model
- accepted jobs persist to sqlite (queue_jobs) before the pending status
posts; startup re-enqueues rows that never reached a final state, so
restarts no longer strand shas at 'pending'
- 403 on status/comment posts self-heals: admin-scoped token (config
admin_token) adds shre-reviewer as collaborator (write), retries once
- scripts/wire-repos.mjs: idempotent estate-wide webhook + collaborator
wiring over GET /repos/search
- tests: 32 -> 51 (truncation cap, persisted-queue reconciliation,
collab-retry stubs, fetchDiff routing)
Co-Authored-By: Claude Fable 5 <[email protected]>
81 lines
3.3 KiB
JavaScript
81 lines
3.3 KiB
JavaScript
// Verdict derivation. Posture: BLOCK on confirmed critical/high findings in
|
|
// correctness/security; everything else is warn-or-pass. Human override =
|
|
// admin merge (branch protection does not apply to admins).
|
|
|
|
const BLOCKING_SEVERITIES = new Set(['critical', 'high']);
|
|
const BLOCKING_AXES = new Set(['correctness', 'security']);
|
|
|
|
export function isBlockingFinding(f) {
|
|
if (!f || typeof f !== 'object') return false;
|
|
const sev = String(f.severity || '').toLowerCase();
|
|
const conf = String(f.confidence || '').toLowerCase();
|
|
if (!BLOCKING_SEVERITIES.has(sev)) return false;
|
|
if (conf !== 'confirmed') return false;
|
|
// If the model tagged an axis, only correctness/security block.
|
|
// Untagged critical+confirmed findings block too (conservative).
|
|
const axis = f.axis ? String(f.axis).toLowerCase() : null;
|
|
return axis === null || BLOCKING_AXES.has(axis);
|
|
}
|
|
|
|
export function deriveVerdict(review) {
|
|
const findings = Array.isArray(review?.findings) ? review.findings : [];
|
|
if (findings.some(isBlockingFinding)) return 'fail';
|
|
const hasWarn = findings.some(f => {
|
|
const sev = String(f?.severity || '').toLowerCase();
|
|
return sev === 'critical' || sev === 'high' || sev === 'medium';
|
|
});
|
|
if (hasWarn) return 'warn';
|
|
// trust the model's verdict if it said warn without matching findings
|
|
if (review?.verdict === 'warn') return 'warn';
|
|
return 'pass';
|
|
}
|
|
|
|
export function normalizeReview(raw) {
|
|
if (!raw || typeof raw !== 'object') return null;
|
|
const axesNames = ['correctness', 'security', 'tests_coverage', 'design_simplicity',
|
|
'performance', 'style', 'breaking_changes', 'docs'];
|
|
const axes = {};
|
|
for (const name of axesNames) {
|
|
const a = raw.axes?.[name];
|
|
axes[name] = {
|
|
score: clampScore(a?.score),
|
|
rationale: typeof a?.rationale === 'string' ? a.rationale.slice(0, 500) : ''
|
|
};
|
|
}
|
|
const findings = (Array.isArray(raw.findings) ? raw.findings : []).map(f => ({
|
|
title: String(f?.title ?? 'untitled').slice(0, 200),
|
|
file: String(f?.file ?? '').slice(0, 300),
|
|
line: Number.isFinite(Number(f?.line)) ? Number(f.line) : null,
|
|
severity: pick(String(f?.severity || '').toLowerCase(), ['critical', 'high', 'medium', 'low'], 'low'),
|
|
confidence: pick(String(f?.confidence || '').toLowerCase(), ['confirmed', 'plausible'], 'plausible'),
|
|
axis: f?.axis ? String(f.axis).toLowerCase() : null,
|
|
detail: String(f?.detail ?? '').slice(0, 2000)
|
|
}));
|
|
const review = {
|
|
axes,
|
|
overall: clampScore(raw.overall),
|
|
grade: pick(String(raw.grade || '').toUpperCase(), ['A', 'B', 'C', 'D', 'F'], 'C'),
|
|
findings,
|
|
verdict: pick(String(raw.verdict || '').toLowerCase(), ['pass', 'warn', 'fail'], 'pass')
|
|
};
|
|
review.verdict = deriveVerdict(review); // service-side derivation is authoritative
|
|
return review;
|
|
}
|
|
|
|
// A truncated diff means part of the change was never reviewed: a clean
|
|
// 'pass' cannot be claimed. Cap at 'warn'; a confirmed-critical 'fail' stands.
|
|
export function applyTruncationCap(review, truncated) {
|
|
if (truncated && review && review.verdict === 'pass') review.verdict = 'warn';
|
|
return review;
|
|
}
|
|
|
|
function clampScore(v) {
|
|
const n = Number(v);
|
|
if (!Number.isFinite(n)) return 0;
|
|
return Math.max(0, Math.min(100, Math.round(n)));
|
|
}
|
|
|
|
function pick(v, allowed, fallback) {
|
|
return allowed.includes(v) ? v : fallback;
|
|
}
|