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

//! The **independent reviewer agent**: a separate principal that judges a
//! coding op against its spec and produces signed two-stage reviews.
//!
//! This is the other half of the review gate. The gate
//! (`joule-code-review`) checks that an op carries an independent two-stage
//! approval; this crate is something that *produces* one — a reviewer that is
//! a genuinely distinct principal (its own signing key) making a real,
//! falsifiable judgment, not a rubber stamp.
//!
//! Two invariants make it honest:
//!
//! - **Independence is enforced at the source.** [`Reviewer::review`] refuses
//!   to review an op it authored (`op.author == reviewer key`) — a principal
//!   cannot review its own work, whether that principal is a human or an
//!   agent. Separation of duties is not left to the audit to catch; the
//!   reviewer won't sign in the first place.
//! - **The judgment can fail.** A reviewer that always approves is a stamp,
//!   not a reviewer. The structural checks here genuinely request changes —
//!   an op that cites no clause, cites a *non-goal*, changes nothing, or ships
//!   an unfinished marker (`todo!()`, `unimplemented!()`, `FIXME`) is sent
//!   back with findings.
//!
//! This is the **structural** reviewer — the cheapest sufficient tier of
//! review: deterministic, reproducible, model-free. It checks the things a
//! machine can check for certain. A semantic reviewer (does the code actually
//! *mean* what the clause asks?) is the escalation tier above it — a model
//! judgment by another distinct principal — and composes the same way: it
//! signs reviews with its own key, and the gate counts them identically.
//!
//! Whoever the reviewer principal is — agent or human — its key is the trace.

use ed25519_dalek::SigningKey;
use joule_code_cargo::SignedOp;
use joule_code_review::{Review, ReviewStage, ReviewVerdict, SignedReview, ALG_ED25519};
use joule_code_spec::{ClauseKind, SignedSpec};

pub mod cascade;
pub mod llm;

/// Errors a reviewer can raise (distinct from a *request-changes* verdict,
/// which is a normal review outcome, not an error).
#[derive(Debug, thiserror::Error)]
pub enum ReviewerError {
    /// The reviewer was asked to review an op it authored — self-review is
    /// refused at the source, regardless of the principal's species.
    #[error("a reviewer must not review its own op (self-review)")]
    SelfReview,
    #[error("spec: {0}")]
    Spec(#[from] joule_code_spec::SpecError),
    #[error("review: {0}")]
    Review(#[from] joule_code_review::ReviewError),
    #[error("cargo: {0}")]
    Cargo(#[from] joule_code_cargo::CargoError),
}

/// Unfinished-work markers a quality review refuses to pass.
const UNFINISHED_MARKERS: &[&str] = &["todo!(", "unimplemented!(", "unreachable!(", "FIXME", "XXX"];

/// The clause(s) an op claims, parsed from a leading `Cn:` tag in its intent
/// (`"C1: make todos persist"` → `["C1"]`) — the same convention the editor
/// uses. The reviewer checks the claim against the spec; it does not invent
/// one. An untagged intent claims nothing (`[]`), which spec-compliance then
/// sends back.
pub fn claimed_clauses(intent: &str) -> Vec<String> {
    let Some((head, _)) = intent.split_once(':') else { return vec![] };
    let id = head.trim();
    if !id.is_empty() && id.len() <= 32 && id.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') {
        vec![id.to_string()]
    } else {
        vec![]
    }
}

/// An independent reviewer principal. Holds the signing key that *is* its
/// identity and the trace of everything it approves or rejects.
pub struct Reviewer {
    key: SigningKey,
}

impl Reviewer {
    /// A reviewer with the given signing identity. The caller is responsible
    /// for this key being a *different* principal than the authors it reviews
    /// (the reviewer also refuses self-review defensively).
    pub fn new(key: SigningKey) -> Self {
        Self { key }
    }

    /// This reviewer's principal — its verifying key.
    pub fn principal(&self) -> [u8; 32] {
        self.key.verifying_key().to_bytes()
    }

    /// Judge an op against its spec, producing signed two-stage reviews.
    ///
    /// `claimed_clauses` are the clause ids the op claims to implement (the
    /// citation, or the author's `Cn:` tag). The reviewer checks them against
    /// the spec; it does not take the author's word for what the op does.
    ///
    /// Returns one or two reviews: always a **spec-compliance** review, and —
    /// only if that passes — a **quality** review (you do not quality-review
    /// work that does not yet do what was asked). A reviewer that authored the
    /// op refuses with [`ReviewerError::SelfReview`].
    pub fn review(
        &self,
        op: &SignedOp,
        spec: &SignedSpec,
        claimed_clauses: &[String],
    ) -> Result<Vec<SignedReview>, ReviewerError> {
        if op.op.author == self.principal() {
            return Err(ReviewerError::SelfReview);
        }
        let op_id = op.op.op_id()?;
        let spec_id = spec.verify()?;

        let mut out = Vec::new();
        let compliance = self.judge_compliance(op, spec, claimed_clauses);
        let compliance_passed = compliance.0 == ReviewVerdict::Approve;
        out.push(self.sign_review(
            op_id.clone(),
            ReviewStage::SpecCompliance,
            Some(spec_id.clone()),
            compliance,
        )?);

        if compliance_passed {
            let quality = self.judge_quality(op);
            out.push(self.sign_review(op_id, ReviewStage::Quality, None, quality)?);
        }
        Ok(out)
    }

    /// Spec-compliance: does the op implement a real requirement/constraint,
    /// and nothing it shouldn't? Deterministic, and able to fail.
    fn judge_compliance(
        &self,
        op: &SignedOp,
        spec: &SignedSpec,
        claimed: &[String],
    ) -> (ReviewVerdict, Vec<String>) {
        let mut findings = Vec::new();
        if claimed.is_empty() {
            findings.push("op claims no spec clause; spec-compliance cannot be verified".into());
        }
        for id in claimed {
            match spec.spec.clauses.iter().find(|c| &c.id == id) {
                None => findings.push(format!("cites clause {id:?}, which the spec does not contain")),
                Some(clause) if clause.kind == ClauseKind::NonGoal => {
                    findings.push(format!(
                        "cites non-goal {id:?} ({:?}) — implementing an out-of-scope item",
                        clause.text
                    ));
                }
                Some(_) => {}
            }
        }
        if op.op.edits.is_empty() {
            findings.push("op makes no edits — it implements nothing".into());
        }
        if findings.is_empty() {
            (ReviewVerdict::Approve, vec![])
        } else {
            (ReviewVerdict::RequestChanges, findings)
        }
    }

    /// Quality: is the change finished enough to ship? Scans the written bytes
    /// for unfinished-work markers and trivial no-ops. Deterministic, and able
    /// to fail.
    fn judge_quality(&self, op: &SignedOp) -> (ReviewVerdict, Vec<String>) {
        let mut findings = Vec::new();
        let total_written: usize = op.op.edits.iter().map(|e| e.replacement.len()).sum();
        if total_written == 0 {
            findings.push("change writes no content (all replacements empty)".into());
        }
        for edit in &op.op.edits {
            let text = String::from_utf8_lossy(&edit.replacement);
            for marker in UNFINISHED_MARKERS {
                if text.contains(marker) {
                    findings.push(format!("{}: ships an unfinished marker `{marker}`", edit.path));
                }
            }
        }
        if findings.is_empty() {
            (ReviewVerdict::Approve, vec![])
        } else {
            (ReviewVerdict::RequestChanges, findings)
        }
    }

    /// Sign a **request-changes** review at `stage` with `findings` — used to
    /// emit a rejection from a learned review skill (a defect a model taught
    /// the cheap tier to catch). Refuses self-review and requires non-empty
    /// findings, like any rejection. `spec` supplies the id for a
    /// spec-compliance stage.
    pub fn reject(
        &self,
        op: &SignedOp,
        spec: &SignedSpec,
        stage: ReviewStage,
        findings: Vec<String>,
    ) -> Result<SignedReview, ReviewerError> {
        if op.op.author == self.principal() {
            return Err(ReviewerError::SelfReview);
        }
        let spec_id = match stage {
            ReviewStage::SpecCompliance => Some(spec.verify()?),
            ReviewStage::Quality => None,
        };
        self.sign_review(op.op.op_id()?, stage, spec_id, (ReviewVerdict::RequestChanges, findings))
    }

    fn sign_review(
        &self,
        op_id: joule_code_schema::ContentId,
        stage: ReviewStage,
        spec_id: Option<joule_code_schema::ContentId>,
        verdict: (ReviewVerdict, Vec<String>),
    ) -> Result<SignedReview, ReviewerError> {
        Ok(Review {
            alg: ALG_ED25519,
            op_id,
            stage,
            spec_id,
            verdict: verdict.0,
            findings: verdict.1,
            round: 1,
            reviewer: self.principal(),
        }
        .sign(&self.key)?)
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use joule_code_review::approval;
    use joule_code_cargo::{Anchor, CodingOp, Edit};
    use joule_code_schema::{ContentId, Tier};
    use joule_code_spec::{Clause, Spec, ALG_ED25519 as SALG};

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

    fn spec(author: &SigningKey) -> SignedSpec {
        Spec {
            alg: SALG,
            intent: "a persistent todo list".into(),
            clauses: vec![
                Clause { id: "C1".into(), text: "todos persist".into(), kind: ClauseKind::Requirement },
                Clause { id: "NG1".into(), text: "no multiplayer".into(), kind: ClauseKind::NonGoal },
            ],
            parent: None,
            author: author.verifying_key().to_bytes(),
        }
        .sign(author)
        .unwrap()
    }

    fn op(author: &SigningKey, body: &str) -> SignedOp {
        CodingOp {
            alg: 0x01,
            prior: ContentId::of_bytes(b"t"),
            edits: vec![Edit {
                path: "src/store.rs".into(),
                anchor: Anchor::ByteRange { start: 0, end: 0 },
                replacement: body.as_bytes().to_vec(),
            }],
            author: author.verifying_key().to_bytes(),
            intent: "persist todos".into(),
            tier: Tier::L3,
        }
        .sign(author)
        .unwrap()
    }

    #[test]
    fn a_good_op_gets_a_two_stage_approval_that_satisfies_the_gate() {
        let author = sk(1);
        let reviewer = Reviewer::new(sk(2));
        let s = spec(&author);
        let o = op(&author, "fn save() { write_to_disk(); }");
        let reviews = reviewer.review(&o, &s, &["C1".into()]).unwrap();
        assert_eq!(reviews.len(), 2); // both stages assessed
        assert!(approval(&o, &reviews).is_approved());
    }

    #[test]
    fn citing_a_non_goal_is_sent_back_at_spec_compliance() {
        let author = sk(1);
        let reviewer = Reviewer::new(sk(2));
        let s = spec(&author);
        let o = op(&author, "fn mp() {}");
        let reviews = reviewer.review(&o, &s, &["NG1".into()]).unwrap();
        assert_eq!(reviews.len(), 1); // quality not reached — compliance failed
        assert_eq!(reviews[0].review.verdict, ReviewVerdict::RequestChanges);
        assert!(reviews[0].review.findings[0].contains("non-goal"));
        assert!(!approval(&o, &reviews).is_approved());
    }

    #[test]
    fn an_unfinished_marker_fails_quality() {
        let author = sk(1);
        let reviewer = Reviewer::new(sk(2));
        let s = spec(&author);
        let o = op(&author, "fn save() { todo!() }");
        let reviews = reviewer.review(&o, &s, &["C1".into()]).unwrap();
        assert_eq!(reviews.len(), 2);
        // compliance passed, quality requested changes
        let quality = reviews.iter().find(|r| r.review.stage == ReviewStage::Quality).unwrap();
        assert_eq!(quality.review.verdict, ReviewVerdict::RequestChanges);
        assert!(quality.review.findings[0].contains("unfinished"));
        assert!(!approval(&o, &reviews).is_approved());
    }

    #[test]
    fn a_reviewer_refuses_to_review_its_own_op() {
        let principal = sk(3);
        let reviewer = Reviewer::new(principal.clone());
        let s = spec(&sk(1));
        let own = op(&principal, "fn f() {}"); // authored by the reviewer's principal
        assert!(matches!(reviewer.review(&own, &s, &["C1".into()]), Err(ReviewerError::SelfReview)));
    }
}