open-standards/joule-code/joule-code-rs/crates/joule-code-review/src/lib.rs

//! **Two-stage review as a signed artifact** — the methodology layer's review
//! discipline, made checkable instead of promised.
//!
//! The converged agentic-dev practice reviews every change twice: once for
//! **spec compliance** (did it do everything it was supposed to and nothing
//! else) and once for **quality** (does it meet the bar), with fresh
//! reviewers per round so no reviewer inherits the previous round's fatigue
//! or sunk cost. All of that lives in prompts today — nothing stops an agent
//! from approving its own work, and "it was reviewed" is testimony.
//!
//! This crate makes the discipline structural, in the family's house style:
//!
//! - a [`Review`] is a signed, content-addressed verdict by one reviewer on
//!   one coding op, at one [`ReviewStage`], in one numbered round;
//!   `RequestChanges` without findings is malformed — a rejection must say
//!   why;
//! - **separation of duties is enforced, not advised**: a review whose
//!   reviewer key equals the op's author key fails verification — the
//!   competing-mandates rule the methodology people state as a guideline is
//!   a signature check here;
//! - [`approval`] is the deterministic audit: an op is approved iff both
//!   stages' **latest round** is an approval from a verified, independent
//!   reviewer. Earlier rejected rounds stay on the record — the history of
//!   how many rounds it took is part of the artifact, never overwritten.
//!
//! What this crate does not claim: that the reviewer had fresh context, or
//! good judgment. Those stay operational. What it proves: who reviewed what,
//! at which stage, in what order, with what verdict — and that none of them
//! was the author grading their own homework.

use ed25519_dalek::{Signature, Signer, SigningKey, Verifier, VerifyingKey};
use joule_code_cargo::SignedOp;
use joule_code_schema::ContentId;
use serde::{Deserialize, Serialize};

/// Signature algorithm id for Ed25519 (matches `joule-code-schema`).
pub const ALG_ED25519: u8 = 0x01;

/// Errors from review construction, signing, or verification.
#[derive(Debug, thiserror::Error)]
pub enum ReviewError {
    #[error("unsupported algorithm id {0:#04x}")]
    BadAlg(u8),
    #[error("a spec-compliance review must cite the spec it checked against")]
    MissingSpec,
    #[error("request-changes must carry at least one finding")]
    NoFindings,
    #[error("an approval must not carry findings (write them, then request changes)")]
    ApproveWithFindings,
    #[error("the reviewer is the op's author — self-review is structurally void")]
    SelfReview,
    #[error("signature verification failed")]
    BadSignature,
    #[error("schema: {0}")]
    Schema(#[from] joule_code_schema::SchemaError),
    #[error("cargo: {0}")]
    Cargo(#[from] joule_code_cargo::CargoError),
}

/// The two stages, in the order they gate.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum ReviewStage {
    /// Did the op implement what was specified — everything, and nothing else.
    SpecCompliance,
    /// Does the change meet the quality bar.
    Quality,
}

/// The verdict of one review round.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum ReviewVerdict {
    Approve,
    RequestChanges,
}

/// One signed review round (unsigned form).
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Review {
    /// Signature algorithm id (`0x01` = Ed25519).
    pub alg: u8,
    /// The op under review.
    pub op_id: ContentId,
    /// Which gate this round belongs to.
    pub stage: ReviewStage,
    /// The spec checked against — required for [`ReviewStage::SpecCompliance`].
    pub spec_id: Option<ContentId>,
    /// The verdict.
    pub verdict: ReviewVerdict,
    /// Why changes are requested. Non-empty iff `RequestChanges`.
    pub findings: Vec<String>,
    /// Round number within this op+stage, starting at 1. Later rounds
    /// supersede earlier ones; earlier ones remain on the record.
    pub round: u32,
    /// The reviewer's Ed25519 verifying key.
    pub reviewer: [u8; 32],
}

impl Review {
    /// Structural validation.
    pub fn validate(&self) -> Result<(), ReviewError> {
        if self.alg != ALG_ED25519 {
            return Err(ReviewError::BadAlg(self.alg));
        }
        if self.stage == ReviewStage::SpecCompliance && self.spec_id.is_none() {
            return Err(ReviewError::MissingSpec);
        }
        match self.verdict {
            ReviewVerdict::RequestChanges if self.findings.is_empty() => {
                Err(ReviewError::NoFindings)
            }
            ReviewVerdict::Approve if !self.findings.is_empty() => {
                Err(ReviewError::ApproveWithFindings)
            }
            _ => Ok(()),
        }
    }

    /// The review id: content address of the canonical form.
    pub fn review_id(&self) -> Result<ContentId, ReviewError> {
        self.validate()?;
        Ok(ContentId::of_canonical(self)?)
    }

    /// Preimage: `review_id ‖ op_id` — the verdict is bound to exactly this op.
    fn preimage(&self) -> Result<Vec<u8>, ReviewError> {
        let id = self.review_id()?;
        let mut pre = id.as_str().as_bytes().to_vec();
        pre.extend_from_slice(self.op_id.as_str().as_bytes());
        Ok(pre)
    }

    /// Sign with the reviewer's key.
    pub fn sign(self, key: &SigningKey) -> Result<SignedReview, ReviewError> {
        let sig = key.sign(&self.preimage()?);
        Ok(SignedReview { review: self, sig: sig.to_bytes().to_vec() })
    }
}

/// A review plus its reviewer's signature.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct SignedReview {
    pub review: Review,
    /// Ed25519 signature (64 bytes) over `review_id ‖ op_id`.
    pub sig: Vec<u8>,
}

impl SignedReview {
    /// Verify structure and signature, and — against the op — enforce the
    /// separation of duties: the reviewer key must differ from the op's
    /// author key, or the review is void no matter what it says.
    pub fn verify(&self, op: &SignedOp) -> Result<ContentId, ReviewError> {
        let op_id = op.op.op_id()?;
        if self.review.op_id != op_id {
            return Err(ReviewError::BadSignature);
        }
        if self.review.reviewer == op.op.author {
            return Err(ReviewError::SelfReview);
        }
        let pre = self.review.preimage()?;
        let key =
            VerifyingKey::from_bytes(&self.review.reviewer).map_err(|_| ReviewError::BadSignature)?;
        let sig_bytes: [u8; 64] =
            self.sig.as_slice().try_into().map_err(|_| ReviewError::BadSignature)?;
        key.verify(&pre, &Signature::from_bytes(&sig_bytes))
            .map_err(|_| ReviewError::BadSignature)?;
        self.review.review_id()
    }
}

/// The deterministic approval audit for one op.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Approval {
    /// The latest verified round per stage, if any: (round, verdict).
    pub spec_compliance: Option<(u32, ReviewVerdict)>,
    pub quality: Option<(u32, ReviewVerdict)>,
    /// Reviews that failed verification (bad signature, wrong op,
    /// self-review) — on the record, never silently dropped.
    pub void: usize,
    /// Total verified rounds across both stages — the cost-of-convergence
    /// number (how many rounds it took is part of the story).
    pub rounds: usize,
}

impl Approval {
    /// Approved iff both stages' latest verified round is an approval.
    pub fn is_approved(&self) -> bool {
        matches!(self.spec_compliance, Some((_, ReviewVerdict::Approve)))
            && matches!(self.quality, Some((_, ReviewVerdict::Approve)))
    }
}

/// Audit an op against its review record.
pub fn approval(op: &SignedOp, reviews: &[SignedReview]) -> Approval {
    let mut spec: Option<(u32, ReviewVerdict)> = None;
    let mut quality: Option<(u32, ReviewVerdict)> = None;
    let mut void = 0usize;
    let mut rounds = 0usize;

    for signed in reviews {
        if signed.verify(op).is_err() {
            void += 1;
            continue;
        }
        rounds += 1;
        let slot = match signed.review.stage {
            ReviewStage::SpecCompliance => &mut spec,
            ReviewStage::Quality => &mut quality,
        };
        let supersedes = slot.map(|(round, _)| signed.review.round > round).unwrap_or(true);
        if supersedes {
            *slot = Some((signed.review.round, signed.review.verdict));
        }
    }

    Approval { spec_compliance: spec, quality, void, rounds }
}

#[cfg(test)]
mod tests {
    use super::*;
    use joule_code_cargo::{Anchor, CodingOp, Edit};
    use joule_code_schema::Tier;

    fn sk(s: u8) -> SigningKey {
        SigningKey::from_bytes(&[s; 32])
    }

    fn op_fixture(author: &SigningKey) -> SignedOp {
        CodingOp {
            alg: 0x01,
            prior: ContentId::of_bytes(b"tree"),
            edits: vec![Edit {
                path: "src/lib.rs".into(),
                anchor: Anchor::ByteRange { start: 0, end: 0 },
                replacement: b"fn f() {}".to_vec(),
            }],
            author: author.verifying_key().to_bytes(),
            intent: "add f".into(),
            tier: Tier::L3,
        }
        .sign(author)
        .expect("valid op signs")
    }

    fn review(
        op: &SignedOp,
        stage: ReviewStage,
        verdict: ReviewVerdict,
        findings: Vec<String>,
        round: u32,
        reviewer: &SigningKey,
    ) -> SignedReview {
        Review {
            alg: ALG_ED25519,
            op_id: op.op.op_id().unwrap(),
            stage,
            spec_id: Some(ContentId::of_bytes(b"spec")),
            verdict,
            findings,
            round,
            reviewer: reviewer.verifying_key().to_bytes(),
        }
        .sign(reviewer)
        .expect("valid review signs")
    }

    #[test]
    fn self_review_is_structurally_void() {
        let author = sk(1);
        let op = op_fixture(&author);
        let own = review(&op, ReviewStage::Quality, ReviewVerdict::Approve, vec![], 1, &author);
        assert!(matches!(own.verify(&op), Err(ReviewError::SelfReview)));
        // and the audit refuses to count it
        let a = approval(&op, &[own]);
        assert_eq!(a.void, 1);
        assert!(!a.is_approved());
    }

    #[test]
    fn approval_requires_both_stages() {
        let author = sk(1);
        let (r1, r2) = (sk(2), sk(3));
        let op = op_fixture(&author);
        let spec_ok =
            review(&op, ReviewStage::SpecCompliance, ReviewVerdict::Approve, vec![], 1, &r1);
        // spec compliance alone is not approval
        assert!(!approval(&op, std::slice::from_ref(&spec_ok)).is_approved());
        let quality_ok = review(&op, ReviewStage::Quality, ReviewVerdict::Approve, vec![], 1, &r2);
        assert!(approval(&op, &[spec_ok, quality_ok]).is_approved());
    }

    #[test]
    fn later_rounds_supersede_and_history_stays_on_record() {
        let author = sk(1);
        let (r1, r2, r3) = (sk(2), sk(3), sk(4));
        let op = op_fixture(&author);
        let rejected = review(
            &op,
            ReviewStage::Quality,
            ReviewVerdict::RequestChanges,
            vec!["unwrap() in the hot path".into()],
            1,
            &r1,
        );
        let fixed = review(&op, ReviewStage::Quality, ReviewVerdict::Approve, vec![], 2, &r2);
        let spec_ok =
            review(&op, ReviewStage::SpecCompliance, ReviewVerdict::Approve, vec![], 1, &r3);
        let a = approval(&op, &[rejected, fixed, spec_ok]);
        assert!(a.is_approved());
        assert_eq!(a.rounds, 3); // the cost of convergence is part of the artifact
        assert_eq!(a.quality, Some((2, ReviewVerdict::Approve)));
    }

    #[test]
    fn a_rejection_must_say_why_and_an_approval_must_not_nag() {
        let author = sk(1);
        let reviewer = sk(2);
        let op = op_fixture(&author);
        let mute = Review {
            alg: ALG_ED25519,
            op_id: op.op.op_id().unwrap(),
            stage: ReviewStage::Quality,
            spec_id: None,
            verdict: ReviewVerdict::RequestChanges,
            findings: vec![],
            round: 1,
            reviewer: reviewer.verifying_key().to_bytes(),
        };
        assert!(matches!(mute.validate(), Err(ReviewError::NoFindings)));
        let nag = Review {
            verdict: ReviewVerdict::Approve,
            findings: vec!["fine I guess".into()],
            ..mute
        };
        assert!(matches!(nag.validate(), Err(ReviewError::ApproveWithFindings)));
    }

    #[test]
    fn spec_compliance_must_cite_a_spec() {
        let author = sk(1);
        let reviewer = sk(2);
        let op = op_fixture(&author);
        let unanchored = Review {
            alg: ALG_ED25519,
            op_id: op.op.op_id().unwrap(),
            stage: ReviewStage::SpecCompliance,
            spec_id: None,
            verdict: ReviewVerdict::Approve,
            findings: vec![],
            round: 1,
            reviewer: reviewer.verifying_key().to_bytes(),
        };
        assert!(matches!(unanchored.validate(), Err(ReviewError::MissingSpec)));
    }

    #[test]
    fn a_review_for_a_different_op_does_not_transfer() {
        let author = sk(1);
        let reviewer = sk(2);
        let op_a = op_fixture(&author);
        let mut other = op_a.op.clone();
        other.intent = "different op".into();
        let op_b = other.sign(&author).unwrap();
        let for_b = review(&op_b, ReviewStage::Quality, ReviewVerdict::Approve, vec![], 1, &reviewer);
        assert!(for_b.verify(&op_a).is_err());
        assert_eq!(approval(&op_a, &[for_b]).void, 1);
    }
}