majordomus_cli/
intent_review.rs

1//! The two planning records an intent is held to before execution: the gap and the critique.
2//!
3//! Majordomus does not reason about what is missing or what a plan overlooked. A worker does —
4//! a person, Claude, Codex, Gemini — and writes what it found into a typed record, so that
5//! planning starts from a recorded analysis rather than from a prompt, and so that the
6//! analysis can be refused (ADR 0073):
7//!
8//! - a **gap** (`.ai/repo/project/gaps/<intent>.yaml`, `majordomus.gap/v1`) states, at one
9//!   commit, the observations made of the repository, the state of every criterion of the
10//!   intent against them (`satisfied`, `missing`, `conflicting` or `unknown`), the risks, and
11//!   the work it suggests;
12//! - a **critique** (`.ai/repo/project/critiques/<intent>.yaml`, `majordomus.critique/v1`)
13//!   records the adversarial pass over the plan: each finding has a class, a subject, whether
14//!   it blocks execution, and a resolution — `open`, `planned` into an issue that serves the
15//!   intent, or `rejected` with a reason.
16//!
17//! What this module decides is only whether the records hold together and whether the plan
18//! honours them. A gap must answer every criterion and cite an observation for every answer
19//! that is not `unknown`. A criterion the gap observed `satisfied` needs no planned work; every
20//! other criterion still does (that is the coverage's question). A finding planned into an
21//! issue must name an issue that exists and serves the intent. And no issue serving an intent
22//! may have started while the intent has no critique, or while a blocking finding is open.
23//!
24//! Every function here is pure over parsed records, the intent outlines and the plan.
25//!
26//! ```
27//! use majordomus_cli::intent_plan::IntentOutline;
28//! use majordomus_cli::intent_review::{review, GapRecord};
29//! use majordomus_cli::plan::Plan;
30//! # let plan: Plan = serde_json::from_value(serde_json::json!({
31//! #     "project": {"name": "p", "repository": "o/p", "default_branch": "master",
32//! #                 "active_milestone": ""},
33//! #     "statuses": {"issue": [], "milestone": []},
34//! #     "milestones": [], "waves": [], "edges": [], "milestone_edges": [], "findings": [],
35//! #     "issues": []})).unwrap();
36//! let intent = IntentOutline {
37//!     id: "x".into(),
38//!     criteria: vec!["a".into(), "b".into()],
39//!     milestones: vec!["m".into()],
40//!     ..Default::default()
41//! };
42//! // a gap that answers one criterion and leaves the other out is refused, by name
43//! let gap = GapRecord::from_metadata(".ai/repo/project/gaps/x.yaml", &serde_json::json!({
44//!     "intent": "x", "observed_at": "c3f20da",
45//!     "observations": [{"id": "o1", "statement": "nothing there", "source": "file:src"}],
46//!     "conditions": [{"criterion": "a", "state": "missing", "observations": ["o1"]}],
47//! }));
48//! let findings = review(&[intent], &plan, &[gap], &[]);
49//! assert!(findings.iter().any(|f| f.code == "gap_criterion_unanswered" && f.subject == "x#b"));
50//! ```
51
52use std::collections::{BTreeMap, BTreeSet};
53
54use schemars::JsonSchema;
55use serde::{Deserialize, Serialize};
56use serde_json::Value;
57
58use crate::index::Index;
59use crate::intent::{IntentFinding, FAIL, REPRODUCE, WARN};
60use crate::intent_plan::IntentOutline;
61use crate::plan::Plan;
62
63/// The kind of a gap record.
64pub const GAP: &str = "gap";
65/// The kind of a critique record.
66pub const CRITIQUE: &str = "critique";
67
68// ---------------------------------------------------------------- the records as authored
69
70/// The state of one criterion as the gap observed it. Four values, because "not determined"
71/// has to be writable: an omitted criterion is refused, and `Unknown` is never a pass.
72///
73/// ```
74/// use majordomus_cli::intent_review::ConditionState;
75/// assert_eq!(serde_json::to_string(&ConditionState::Conflicting).unwrap(), "\"conflicting\"");
76/// assert_ne!(ConditionState::Unknown, ConditionState::Satisfied);
77/// ```
78#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
79#[serde(rename_all = "snake_case")]
80pub enum ConditionState {
81    /// Already true at the observed commit.
82    Satisfied,
83    /// Not true; work is needed.
84    Missing,
85    /// Something in the repository contradicts it.
86    Conflicting,
87    /// Not determined. Never read as satisfied.
88    Unknown,
89}
90
91impl ConditionState {
92    fn parse(s: &str) -> Option<Self> {
93        Some(match s {
94            "satisfied" => Self::Satisfied,
95            "missing" => Self::Missing,
96            "conflicting" => Self::Conflicting,
97            "unknown" => Self::Unknown,
98            _ => return None,
99        })
100    }
101}
102
103/// One observation of the current state, with the place it was observed in — a file, a
104/// command, a test or a URL — so a reader can go and look at the same thing.
105///
106/// ```
107/// use majordomus_cli::intent_review::GapObservation;
108/// let seen = GapObservation {
109///     id: "no-serves-field".into(),
110///     statement: "the issue schema declares no link to a criterion".into(),
111///     source: "file:share/schemas/majordomus/issue/issue.v1.schema.json".into(),
112/// };
113/// assert!(seen.source.starts_with("file:"));
114/// ```
115#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, JsonSchema)]
116pub struct GapObservation {
117    /// Unique within the gap.
118    pub id: String,
119    /// What was observed.
120    pub statement: String,
121    /// Where: `file:<path>`, `command:<cmd>`, `test:<path>`, `url:<url>`.
122    pub source: String,
123}
124
125/// One criterion's state against the observations, as the record spells it. `state` is `None`
126/// when the file names a state this module does not know, and `state_text` keeps what was
127/// written so the finding can quote it.
128///
129/// ```
130/// use majordomus_cli::intent_review::{ConditionState, GapCondition};
131/// let c = GapCondition {
132///     criterion: "criteria-are-covered".into(),
133///     state: Some(ConditionState::Missing),
134///     state_text: "missing".into(),
135///     because: "nothing relates a criterion to an issue".into(),
136///     observations: vec!["no-serves-field".into()],
137/// };
138/// assert_eq!(c.state, Some(ConditionState::Missing));
139/// assert!(!c.observations.is_empty());
140/// ```
141#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
142pub struct GapCondition {
143    /// The criterion id of the intent.
144    pub criterion: String,
145    /// The state, or `None` when the record spells one this module does not know.
146    pub state: Option<ConditionState>,
147    /// As written, so a finding can quote an unknown state.
148    pub state_text: String,
149    /// Why.
150    pub because: String,
151    /// The observation ids it rests on.
152    pub observations: Vec<String>,
153}
154
155/// One work item the gap suggests: what it would do, the criteria it would make true, and the
156/// issue it became once somebody planned it. Nothing turns this into an issue automatically;
157/// the link is recorded when a worker writes the issue.
158///
159/// ```
160/// use majordomus_cli::intent_review::GapWork;
161/// let w = GapWork {
162///     id: "serves-and-coverage".into(),
163///     title: "an issue names the criterion it serves".into(),
164///     serves: vec!["criteria-are-covered".into()],
165///     issue: "I1901".into(),
166/// };
167/// assert_eq!(w.issue, "I1901");
168/// ```
169#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, JsonSchema)]
170pub struct GapWork {
171    /// Unique within the gap.
172    pub id: String,
173    /// One line.
174    pub title: String,
175    /// The criterion ids it would make true.
176    pub serves: Vec<String>,
177    /// The issue it became, once planned.
178    pub issue: String,
179}
180
181/// A gap record: the observations a worker made of the repository at one commit, the state of
182/// every criterion of the intent against them, the risks, and the work it suggests. One per
183/// intent, named by it.
184///
185/// ```
186/// use majordomus_cli::intent_review::GapRecord;
187/// let g = GapRecord::from_metadata(".ai/repo/project/gaps/x.yaml", &serde_json::json!({
188///     "intent": "x", "observed_at": "c3f20da", "recorded_by": "claude-code",
189///     "conditions": [{"criterion": "a", "state": "unknown"}],
190/// }));
191/// assert_eq!(g.intent, "x");
192/// assert_eq!(g.observed_at, "c3f20da");
193/// ```
194#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
195pub struct GapRecord {
196    /// The intent it analyses; also the file name.
197    pub intent: String,
198    /// Repository-relative path.
199    pub source: String,
200    /// The commit the observations were made on.
201    pub observed_at: String,
202    /// Who or what recorded it.
203    pub recorded_by: String,
204    /// The observations.
205    pub observations: Vec<GapObservation>,
206    /// One per criterion.
207    pub conditions: Vec<GapCondition>,
208    /// Risks, as prose.
209    pub risks: Vec<String>,
210    /// Suggested work.
211    pub work: Vec<GapWork>,
212}
213
214/// The class of a critique finding: the question the adversarial pass asked. Closed, so that a
215/// review's findings can be counted by question rather than read as prose.
216///
217/// ```
218/// use majordomus_cli::intent_review::CritiqueClass;
219/// assert_eq!(
220///     serde_json::to_string(&CritiqueClass::DeliveryVerification).unwrap(),
221///     "\"delivery_verification\""
222/// );
223/// ```
224#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
225#[serde(rename_all = "snake_case")]
226pub enum CritiqueClass {
227    /// Does the plan miss something the intent requires?
228    MissedRequirement,
229    /// Which assumption is unproven?
230    UnprovenAssumption,
231    /// Which criterion lacks sufficient work?
232    InsufficientWork,
233    /// Which work item is unnecessary?
234    UnnecessaryWork,
235    /// What could regress?
236    RegressionRisk,
237    /// Which related surface must also change?
238    SurfaceMissing,
239    /// Which deployment or runtime verification is required?
240    DeliveryVerification,
241}
242
243impl CritiqueClass {
244    fn parse(s: &str) -> Option<Self> {
245        Some(match s {
246            "missed_requirement" => Self::MissedRequirement,
247            "unproven_assumption" => Self::UnprovenAssumption,
248            "insufficient_work" => Self::InsufficientWork,
249            "unnecessary_work" => Self::UnnecessaryWork,
250            "regression_risk" => Self::RegressionRisk,
251            "surface_missing" => Self::SurfaceMissing,
252            "delivery_verification" => Self::DeliveryVerification,
253            _ => return None,
254        })
255    }
256}
257
258/// How a critique finding was resolved: not yet, by planning work that serves the intent, or
259/// by rejecting it with a reason. A blocking finding left `Open` refuses execution.
260///
261/// ```
262/// use majordomus_cli::intent_review::ResolutionState;
263/// assert_eq!(serde_json::to_string(&ResolutionState::Planned).unwrap(), "\"planned\"");
264/// assert_ne!(ResolutionState::Open, ResolutionState::Rejected);
265/// ```
266#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
267#[serde(rename_all = "snake_case")]
268pub enum ResolutionState {
269    /// Not yet resolved.
270    Open,
271    /// Carried by an issue.
272    Planned,
273    /// Judged not to apply, with a reason.
274    Rejected,
275}
276
277impl ResolutionState {
278    fn parse(s: &str) -> Option<Self> {
279        Some(match s {
280            "open" => Self::Open,
281            "planned" => Self::Planned,
282            "rejected" => Self::Rejected,
283            _ => return None,
284        })
285    }
286}
287
288/// One critique finding: what the review found, what it is about, whether it blocks execution,
289/// and how it was resolved. `class` and `resolution` are `None` when the record spells one this
290/// module does not know, and the `_text` fields keep what was written.
291///
292/// ```
293/// use majordomus_cli::intent_review::{CritiqueFinding, CritiqueRecord, ResolutionState};
294/// let c = CritiqueRecord::from_metadata("c.yaml", &serde_json::json!({
295///     "intent": "x",
296///     "findings": [{"id": "f1", "class": "regression_risk", "subject": "x#a",
297///                   "blocking": true, "resolution": {"state": "planned", "issue": "I1"}}],
298/// }));
299/// let finding: &CritiqueFinding = &c.findings[0];
300/// assert!(finding.blocking);
301/// assert_eq!(finding.resolution, Some(ResolutionState::Planned));
302/// assert_eq!(finding.issue, "I1");
303/// ```
304#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
305pub struct CritiqueFinding {
306    /// Unique within the critique.
307    pub id: String,
308    /// The class, or `None` for a class this module does not know.
309    pub class: Option<CritiqueClass>,
310    /// As written.
311    pub class_text: String,
312    /// `<intent>#<criterion>`, an issue id or a milestone id.
313    pub subject: String,
314    /// The finding.
315    pub finding: String,
316    /// Whether execution may not start while it is open.
317    pub blocking: bool,
318    /// The resolution, or `None` for a state this module does not know.
319    pub resolution: Option<ResolutionState>,
320    /// As written.
321    pub resolution_text: String,
322    /// The issue that carries it, when planned.
323    pub issue: String,
324    /// Why, when rejected.
325    pub because: String,
326}
327
328/// A critique record: the adversarial pass over one intent's plan, at the commit it was
329/// reviewed at, with every finding and its resolution. One per intent, named by it.
330///
331/// ```
332/// use majordomus_cli::intent_review::CritiqueRecord;
333/// let c = CritiqueRecord::from_metadata(".ai/repo/project/critiques/x.yaml", &serde_json::json!({
334///     "intent": "x", "reviewed_at": "c3f20da", "reviewed_by": "another session",
335/// }));
336/// assert_eq!(c.intent, "x");
337/// assert!(c.findings.is_empty());
338/// ```
339#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
340pub struct CritiqueRecord {
341    /// The intent whose plan it reviews; also the file name.
342    pub intent: String,
343    /// Repository-relative path.
344    pub source: String,
345    /// The commit the plan was reviewed at.
346    pub reviewed_at: String,
347    /// Who or what reviewed it.
348    pub reviewed_by: String,
349    /// The findings.
350    pub findings: Vec<CritiqueFinding>,
351}
352
353fn text(meta: &Value, key: &str) -> String {
354    match meta.get(key) {
355        Some(Value::String(s)) => s.clone(),
356        Some(Value::Bool(b)) => b.to_string(),
357        Some(Value::Number(n)) => n.to_string(),
358        _ => String::new(),
359    }
360}
361
362fn texts(meta: &Value, key: &str) -> Vec<String> {
363    match meta.get(key) {
364        Some(Value::Array(a)) => a
365            .iter()
366            .filter_map(Value::as_str)
367            .map(str::to_string)
368            .collect(),
369        Some(Value::String(s)) => vec![s.clone()],
370        _ => Vec::new(),
371    }
372}
373
374fn items<'a>(meta: &'a Value, key: &str) -> impl Iterator<Item = &'a Value> {
375    meta.get(key)
376        .and_then(Value::as_array)
377        .into_iter()
378        .flatten()
379}
380
381impl GapRecord {
382    /// The record an object of kind `gap` carries.
383    ///
384    /// ```
385    /// use majordomus_cli::intent_review::{ConditionState, GapRecord};
386    /// let g = GapRecord::from_metadata(".ai/repo/project/gaps/x.yaml", &serde_json::json!({
387    ///     "intent": "x", "observed_at": "abc",
388    ///     "conditions": [{"criterion": "c", "state": "missing", "observations": ["o1"]}]
389    /// }));
390    /// assert_eq!(g.conditions[0].state, Some(ConditionState::Missing));
391    /// ```
392    pub fn from_metadata(source: &str, meta: &Value) -> GapRecord {
393        GapRecord {
394            intent: text(meta, "intent"),
395            source: source.to_string(),
396            observed_at: text(meta, "observed_at"),
397            recorded_by: text(meta, "recorded_by"),
398            observations: items(meta, "observations")
399                .map(|o| GapObservation {
400                    id: text(o, "id"),
401                    statement: text(o, "statement"),
402                    source: text(o, "source"),
403                })
404                .collect(),
405            conditions: items(meta, "conditions")
406                .map(|c| GapCondition {
407                    criterion: text(c, "criterion"),
408                    state: ConditionState::parse(&text(c, "state")),
409                    state_text: text(c, "state"),
410                    because: text(c, "because"),
411                    observations: texts(c, "observations"),
412                })
413                .collect(),
414            risks: texts(meta, "risks"),
415            work: items(meta, "work")
416                .map(|w| GapWork {
417                    id: text(w, "id"),
418                    title: text(w, "title"),
419                    serves: texts(w, "serves"),
420                    issue: text(w, "issue"),
421                })
422                .collect(),
423        }
424    }
425}
426
427/// Every object of `kind` in an index, read by `read`, in identity order.
428fn all_of<T>(index: &Index, kind: &str, read: fn(&str, &Value) -> T) -> Vec<T> {
429    let by_id: BTreeMap<&str, T> = index
430        .objects
431        .iter()
432        .filter(|o| o.kind == kind)
433        .map(|o| (o.identity.as_str(), read(&o.provenance.path, &o.metadata)))
434        .collect();
435    by_id.into_values().collect()
436}
437
438impl GapRecord {
439    /// Every gap record an index holds, in identity order, so the derivation does not depend
440    /// on the order the files were read.
441    ///
442    /// ```
443    /// use majordomus_cli::index::Index;
444    /// use majordomus_cli::intent_review::GapRecord;
445    /// # fn demo(index: &Index) {
446    /// let gaps = GapRecord::all(index);
447    /// assert!(gaps.windows(2).all(|w| w[0].intent <= w[1].intent));
448    /// # }
449    /// ```
450    pub fn all(index: &Index) -> Vec<GapRecord> {
451        all_of(index, GAP, GapRecord::from_metadata)
452    }
453}
454
455impl CritiqueRecord {
456    /// Every critique record an index holds, in identity order, so the derivation does not
457    /// depend on the order the files were read.
458    ///
459    /// ```
460    /// use majordomus_cli::index::Index;
461    /// use majordomus_cli::intent_review::CritiqueRecord;
462    /// # fn demo(index: &Index) {
463    /// let critiques = CritiqueRecord::all(index);
464    /// assert!(critiques.windows(2).all(|w| w[0].intent <= w[1].intent));
465    /// # }
466    /// ```
467    pub fn all(index: &Index) -> Vec<CritiqueRecord> {
468        all_of(index, CRITIQUE, CritiqueRecord::from_metadata)
469    }
470}
471
472impl CritiqueRecord {
473    /// The record an object of kind `critique` carries.
474    ///
475    /// ```
476    /// use majordomus_cli::intent_review::{CritiqueRecord, ResolutionState};
477    /// let c = CritiqueRecord::from_metadata(".ai/repo/project/critiques/x.yaml", &serde_json::json!({
478    ///     "intent": "x", "findings": [{"id": "f1", "class": "regression_risk",
479    ///         "subject": "x#c", "blocking": true, "resolution": {"state": "open"}}]
480    /// }));
481    /// assert!(c.findings[0].blocking);
482    /// assert_eq!(c.findings[0].resolution, Some(ResolutionState::Open));
483    /// ```
484    pub fn from_metadata(source: &str, meta: &Value) -> CritiqueRecord {
485        CritiqueRecord {
486            intent: text(meta, "intent"),
487            source: source.to_string(),
488            reviewed_at: text(meta, "reviewed_at"),
489            reviewed_by: text(meta, "reviewed_by"),
490            findings: items(meta, "findings")
491                .map(|f| {
492                    let res = f.get("resolution").cloned().unwrap_or(Value::Null);
493                    CritiqueFinding {
494                        id: text(f, "id"),
495                        class: CritiqueClass::parse(&text(f, "class")),
496                        class_text: text(f, "class"),
497                        subject: text(f, "subject"),
498                        finding: text(f, "finding"),
499                        blocking: f.get("blocking").and_then(Value::as_bool).unwrap_or(false),
500                        resolution: ResolutionState::parse(&text(&res, "state")),
501                        resolution_text: text(&res, "state"),
502                        issue: text(&res, "issue"),
503                        because: text(&res, "because"),
504                    }
505                })
506                .collect(),
507        }
508    }
509}
510
511// ---------------------------------------------------------------- the judgement
512
513struct Findings(Vec<IntentFinding>);
514
515impl Findings {
516    fn push(&mut self, level: &str, code: &str, subject: &str, message: String) {
517        self.0.push(IntentFinding {
518            level: level.into(),
519            code: code.into(),
520            subject: subject.into(),
521            message,
522            reproduce: REPRODUCE.into(),
523        });
524    }
525}
526
527/// The criteria a gap observed already satisfied, by intent: the ones coverage does not ask
528/// work of. A condition counts only if it is `satisfied` and cites at least one observation
529/// the gap declares.
530///
531/// ```
532/// use majordomus_cli::intent_review::{observed_satisfied, GapRecord};
533/// let g = GapRecord::from_metadata("g", &serde_json::json!({"intent": "x",
534///     "observations": [{"id": "o1", "statement": "s", "source": "file:a"}],
535///     "conditions": [{"criterion": "a", "state": "satisfied", "observations": ["o1"]},
536///                    {"criterion": "b", "state": "satisfied", "observations": []}]}));
537/// assert_eq!(observed_satisfied(&[g])["x"], ["a"]);
538/// ```
539pub fn observed_satisfied(gaps: &[GapRecord]) -> BTreeMap<String, Vec<String>> {
540    let mut out: BTreeMap<String, Vec<String>> = BTreeMap::new();
541    for g in gaps {
542        let known: BTreeSet<&str> = g.observations.iter().map(|o| o.id.as_str()).collect();
543        for c in &g.conditions {
544            if c.state == Some(ConditionState::Satisfied)
545                && !c.observations.is_empty()
546                && c.observations.iter().all(|o| known.contains(o.as_str()))
547            {
548                out.entry(g.intent.clone())
549                    .or_default()
550                    .push(c.criterion.clone());
551            }
552        }
553    }
554    out
555}
556
557/// The findings of the gap and critique records against the intents and the plan: whether each
558/// record holds together, and whether execution waited for the review.
559///
560/// ```
561/// use majordomus_cli::intent_plan::IntentOutline;
562/// use majordomus_cli::intent_review::review;
563/// use majordomus_cli::plan::Plan;
564/// # let plan: Plan = serde_json::from_value(serde_json::json!({
565/// #     "project": {"name": "p", "repository": "o/p", "default_branch": "master",
566/// #                 "active_milestone": ""},
567/// #     "statuses": {"issue": [], "milestone": []},
568/// #     "milestones": [], "waves": [], "edges": [], "milestone_edges": [], "findings": [],
569/// #     "issues": [{"id": "I1", "milestone": "m", "status": "ACTIVE", "wave": 1,
570/// #         "priority": "p1", "profile": "implementation", "parallel_safe": true,
571/// #         "title": "t", "slug": "", "depends_on": [], "blocked_by": [], "dependents": [],
572/// #         "scope": ["src"], "serves": ["x#a"], "objective": "", "evidence_have": 0,
573/// #         "evidence_need": 1, "started_at": "2026-09-16", "verified_at": "",
574/// #         "completed_at": ""}]})).unwrap();
575/// let intent = IntentOutline {
576///     id: "x".into(),
577///     criteria: vec!["a".into()],
578///     milestones: vec!["m".into()],
579///     ..Default::default()
580/// };
581/// // I1 has started, and no critique of this intent's plan was ever recorded
582/// let findings = review(&[intent], &plan, &[], &[]);
583/// assert!(findings.iter().any(|f| f.code == "executing_without_critique"));
584/// ```
585pub fn review(
586    intents: &[IntentOutline],
587    plan: &Plan,
588    gaps: &[GapRecord],
589    critiques: &[CritiqueRecord],
590) -> Vec<IntentFinding> {
591    let mut f = Findings(Vec::new());
592    let by_id: BTreeMap<&str, &IntentOutline> =
593        intents.iter().map(|i| (i.id.as_str(), i)).collect();
594
595    // --- the gap holds together, and answers every criterion
596    for g in gaps {
597        let Some(intent) = by_id.get(g.intent.as_str()) else {
598            f.push(
599                FAIL,
600                "gap_unknown_intent",
601                &g.source,
602                format!("analyses intent {}, which does not exist", g.intent),
603            );
604            continue;
605        };
606        if g.observed_at.is_empty() {
607            f.push(
608                FAIL,
609                "gap_without_commit",
610                &g.intent,
611                "names no observed_at commit; an observation of no tree cannot be checked".into(),
612            );
613        }
614        let known: BTreeSet<&str> = g.observations.iter().map(|o| o.id.as_str()).collect();
615        for o in &g.observations {
616            if o.source.is_empty() {
617                f.push(
618                    FAIL,
619                    "observation_without_source",
620                    &format!("{}:{}", g.intent, o.id),
621                    "names no source it was observed in".into(),
622                );
623            }
624        }
625        let mut answered: BTreeSet<&str> = BTreeSet::new();
626        for c in &g.conditions {
627            let subject = format!("{}#{}", g.intent, c.criterion);
628            if !intent.criteria.contains(&c.criterion) {
629                f.push(
630                    FAIL,
631                    "gap_unknown_criterion",
632                    &subject,
633                    format!(
634                        "answers criterion {}, which intent {} does not declare",
635                        c.criterion, g.intent
636                    ),
637                );
638                continue;
639            }
640            if !answered.insert(c.criterion.as_str()) {
641                f.push(
642                    FAIL,
643                    "gap_duplicate_condition",
644                    &subject,
645                    "is answered more than once".into(),
646                );
647            }
648            match c.state {
649                None => f.push(
650                    FAIL,
651                    "gap_unknown_state",
652                    &subject,
653                    format!(
654                        "has state {:?}; expected satisfied, missing, conflicting or unknown",
655                        c.state_text
656                    ),
657                ),
658                Some(ConditionState::Unknown) => {}
659                Some(_) if c.observations.is_empty() => f.push(
660                    FAIL,
661                    "condition_without_observation",
662                    &subject,
663                    format!("is {} with no observation behind it", c.state_text),
664                ),
665                Some(_) => {}
666            }
667            for o in &c.observations {
668                if !known.contains(o.as_str()) {
669                    f.push(
670                        FAIL,
671                        "gap_unknown_observation",
672                        &subject,
673                        format!("cites observation {o}, which the gap does not declare"),
674                    );
675                }
676            }
677        }
678        for cid in &intent.criteria {
679            if !answered.contains(cid.as_str()) {
680                f.push(
681                    FAIL,
682                    "gap_criterion_unanswered",
683                    &format!("{}#{cid}", g.intent),
684                    "is not answered by the gap; unknown must be written, not left out".into(),
685                );
686            }
687        }
688        for w in &g.work {
689            let subject = format!("{}:{}", g.intent, w.id);
690            if w.serves.is_empty() {
691                f.push(
692                    FAIL,
693                    "gap_work_serves_nothing",
694                    &subject,
695                    "suggests work that serves no criterion".into(),
696                );
697            }
698            for s in &w.serves {
699                if !intent.criteria.contains(s) {
700                    f.push(
701                        FAIL,
702                        "gap_work_unknown_criterion",
703                        &subject,
704                        format!(
705                            "serves criterion {s}, which intent {} does not declare",
706                            g.intent
707                        ),
708                    );
709                }
710            }
711            if !w.issue.is_empty() {
712                match plan.issue(&w.issue) {
713                    None => f.push(
714                        FAIL,
715                        "gap_work_unknown_issue",
716                        &subject,
717                        format!("was planned as {}, which is not an issue", w.issue),
718                    ),
719                    Some(i)
720                        if !w
721                            .serves
722                            .iter()
723                            .any(|s| i.serves.contains(&format!("{}#{s}", g.intent))) =>
724                    {
725                        f.push(
726                            FAIL,
727                            "gap_work_issue_serves_other",
728                            &subject,
729                            format!(
730                                "was planned as {}, which serves none of {}",
731                                w.issue,
732                                w.serves.join(", ")
733                            ),
734                        )
735                    }
736                    Some(_) => {}
737                }
738            }
739        }
740    }
741
742    // --- the critique holds together, and names issues that carry the intent
743    let critiqued: BTreeSet<&str> = critiques.iter().map(|c| c.intent.as_str()).collect();
744    let mut open_blocking: BTreeMap<&str, Vec<&str>> = BTreeMap::new();
745    for c in critiques {
746        if !by_id.contains_key(c.intent.as_str()) {
747            f.push(
748                FAIL,
749                "critique_unknown_intent",
750                &c.source,
751                format!("reviews intent {}, which does not exist", c.intent),
752            );
753            continue;
754        }
755        let mut ids: BTreeSet<&str> = BTreeSet::new();
756        for x in &c.findings {
757            let subject = format!("{}:{}", c.intent, x.id);
758            if !ids.insert(x.id.as_str()) {
759                f.push(
760                    FAIL,
761                    "critique_duplicate_finding",
762                    &subject,
763                    "is declared more than once".into(),
764                );
765            }
766            if x.class.is_none() {
767                f.push(
768                    FAIL,
769                    "critique_unknown_class",
770                    &subject,
771                    format!(
772                        "has class {:?}, which is not a question the critique asks",
773                        x.class_text
774                    ),
775                );
776            }
777            match x.resolution {
778                None => f.push(
779                    FAIL,
780                    "critique_unknown_resolution",
781                    &subject,
782                    format!(
783                        "has resolution {:?}; expected open, planned or rejected",
784                        x.resolution_text
785                    ),
786                ),
787                Some(ResolutionState::Open) if x.blocking => open_blocking
788                    .entry(c.intent.as_str())
789                    .or_default()
790                    .push(x.id.as_str()),
791                Some(ResolutionState::Open) => {}
792                Some(ResolutionState::Rejected) if x.because.trim().is_empty() => {
793                    f.push(FAIL, "rejected_without_reason", &subject, "is rejected with no reason; a dismissal nobody can read is not a resolution".into());
794                }
795                Some(ResolutionState::Rejected) => {}
796                Some(ResolutionState::Planned) => match plan.issue(&x.issue) {
797                    None => f.push(
798                        FAIL,
799                        "planned_into_unknown_issue",
800                        &subject,
801                        format!("is planned into {:?}, which is not an issue", x.issue),
802                    ),
803                    Some(i)
804                        if !i
805                            .serves
806                            .iter()
807                            .any(|s| s.split_once('#').is_some_and(|(iid, _)| iid == c.intent)) =>
808                    {
809                        f.push(
810                            FAIL,
811                            "planned_into_unrelated_issue",
812                            &subject,
813                            format!(
814                                "is planned into {}, which serves no criterion of {}",
815                                x.issue, c.intent
816                            ),
817                        )
818                    }
819                    Some(_) => {}
820                },
821            }
822        }
823    }
824
825    // --- execution does not start before the plan was critiqued and its blockers resolved
826    for intent in intents.iter().filter(|i| !i.retired) {
827        let started: Vec<&str> = plan
828            .issues
829            .iter()
830            .filter(|i| matches!(i.status.as_str(), "ACTIVE" | "VERIFY" | "DONE"))
831            .filter(|i| {
832                i.serves
833                    .iter()
834                    .any(|s| s.split_once('#').is_some_and(|(iid, _)| iid == intent.id))
835            })
836            .map(|i| i.id.as_str())
837            .collect();
838        if started.is_empty() {
839            if !critiqued.contains(intent.id.as_str())
840                && plan.issues.iter().any(|i| {
841                    i.serves
842                        .iter()
843                        .any(|s| s.starts_with(&format!("{}#", intent.id)))
844                })
845            {
846                f.push(WARN, "plan_not_critiqued", &intent.id, "has planned work but no critique; execution will be refused until one is recorded".into());
847            }
848            continue;
849        }
850        if !critiqued.contains(intent.id.as_str()) {
851            f.push(
852                FAIL,
853                "executing_without_critique",
854                &intent.id,
855                format!(
856                    "has started work ({}) but its plan was never critiqued",
857                    started.join(", ")
858                ),
859            );
860        }
861        if let Some(open) = open_blocking.get(intent.id.as_str()) {
862            f.push(
863                FAIL,
864                "executing_with_open_blocker",
865                &intent.id,
866                format!(
867                    "has started work ({}) while blocking critique finding(s) {} are open",
868                    started.join(", "),
869                    open.join(", ")
870                ),
871            );
872        }
873    }
874
875    f.0
876}
877
878#[cfg(test)]
879mod tests {
880    use super::*;
881    use crate::plan::{PlanIssue, PlanProject, PlanVocabulary};
882    use serde_json::json;
883
884    #[test]
885    fn every_word_a_record_spells_is_read_and_an_unknown_one_is_not_guessed() {
886        for (word, state) in [
887            ("satisfied", ConditionState::Satisfied),
888            ("missing", ConditionState::Missing),
889            ("conflicting", ConditionState::Conflicting),
890            ("unknown", ConditionState::Unknown),
891        ] {
892            assert_eq!(ConditionState::parse(word), Some(state), "{word}");
893        }
894        assert_eq!(ConditionState::parse("Satisfied"), None);
895        for (word, class) in [
896            ("missed_requirement", CritiqueClass::MissedRequirement),
897            ("unproven_assumption", CritiqueClass::UnprovenAssumption),
898            ("insufficient_work", CritiqueClass::InsufficientWork),
899            ("unnecessary_work", CritiqueClass::UnnecessaryWork),
900            ("regression_risk", CritiqueClass::RegressionRisk),
901            ("surface_missing", CritiqueClass::SurfaceMissing),
902            ("delivery_verification", CritiqueClass::DeliveryVerification),
903        ] {
904            assert_eq!(CritiqueClass::parse(word), Some(class), "{word}");
905        }
906        assert_eq!(CritiqueClass::parse("nitpick"), None);
907    }
908
909    #[test]
910    fn a_scalar_is_read_as_its_text_and_a_lone_string_as_a_list_of_one() {
911        let meta = json!({
912            "s": "words", "b": true, "n": 42, "nothing": null, "list": ["a", 1, "b"],
913        });
914        assert_eq!(text(&meta, "s"), "words");
915        assert_eq!(text(&meta, "b"), "true");
916        assert_eq!(text(&meta, "n"), "42");
917        assert_eq!(text(&meta, "nothing"), "");
918        assert_eq!(text(&meta, "absent"), "");
919        assert_eq!(texts(&meta, "list"), ["a", "b"]);
920        assert_eq!(texts(&meta, "s"), ["words"]);
921        assert!(texts(&meta, "n").is_empty());
922        assert!(texts(&meta, "absent").is_empty());
923    }
924
925    fn issue(id: &str, serves: &[&str], status: &str) -> PlanIssue {
926        PlanIssue {
927            id: id.into(),
928            milestone: "m1".into(),
929            status: status.into(),
930            wave: 1,
931            priority: "p1".into(),
932            profile: "implementation".into(),
933            parallel_safe: true,
934            title: id.into(),
935            slug: String::new(),
936            depends_on: Vec::new(),
937            blocked_by: Vec::new(),
938            dependents: Vec::new(),
939            scope: vec!["src".into()],
940            serves: serves.iter().map(|s| (*s).into()).collect(),
941            objective: String::new(),
942            evidence_have: 0,
943            evidence_need: 1,
944            started_at: String::new(),
945            verified_at: String::new(),
946            completed_at: String::new(),
947        }
948    }
949
950    fn plan(issues: Vec<PlanIssue>) -> Plan {
951        Plan {
952            project: PlanProject {
953                name: "p".into(),
954                repository: "o/p".into(),
955                default_branch: "master".into(),
956                active_milestone: String::new(),
957            },
958            statuses: PlanVocabulary {
959                issue: Vec::new(),
960                milestone: Vec::new(),
961            },
962            milestones: Vec::new(),
963            issues,
964            waves: Vec::new(),
965            edges: Vec::new(),
966            milestone_edges: Vec::new(),
967            findings: Vec::new(),
968        }
969    }
970
971    fn x() -> Vec<IntentOutline> {
972        vec![IntentOutline {
973            id: "x".into(),
974            criteria: vec!["a".into(), "b".into()],
975            milestones: vec!["m1".into()],
976            ..Default::default()
977        }]
978    }
979
980    fn gap(v: serde_json::Value) -> GapRecord {
981        GapRecord::from_metadata(".ai/repo/project/gaps/x.yaml", &v)
982    }
983
984    fn critique(v: serde_json::Value) -> CritiqueRecord {
985        CritiqueRecord::from_metadata(".ai/repo/project/critiques/x.yaml", &v)
986    }
987
988    fn codes(f: &[IntentFinding]) -> Vec<(&str, &str)> {
989        f.iter()
990            .map(|x| (x.code.as_str(), x.subject.as_str()))
991            .collect()
992    }
993
994    fn good_gap() -> GapRecord {
995        gap(json!({
996            "intent": "x", "observed_at": "c3f20da", "recorded_by": "claude-code",
997            "observations": [{"id": "o1", "statement": "no link exists", "source": "file:src/plan.rs"}],
998            "conditions": [
999                {"criterion": "a", "state": "missing", "because": "no link", "observations": ["o1"]},
1000                {"criterion": "b", "state": "unknown", "because": "not measured"}
1001            ],
1002            "work": [{"id": "w1", "title": "add the link", "serves": ["a"], "issue": "I1"}]
1003        }))
1004    }
1005
1006    #[test]
1007    fn a_complete_gap_and_resolved_critique_are_clean() {
1008        let p = plan(vec![
1009            issue("I1", &["x#a"], "ACTIVE"),
1010            issue("I2", &["x#b"], "READY"),
1011        ]);
1012        let c = critique(
1013            json!({"intent": "x", "reviewed_at": "c3f20da", "findings": [
1014            {"id": "f1", "class": "insufficient_work", "subject": "x#b", "blocking": true,
1015             "resolution": {"state": "planned", "issue": "I2"}},
1016            {"id": "f2", "class": "unnecessary_work", "subject": "I1", "blocking": false,
1017             "resolution": {"state": "rejected", "because": "I1 is the only writer"}}]}),
1018        );
1019        let f = review(&x(), &p, &[good_gap()], &[c]);
1020        assert!(f.is_empty(), "{f:?}");
1021    }
1022
1023    #[test]
1024    fn a_gap_must_answer_every_criterion_and_cite_what_it_saw() {
1025        let g = gap(json!({"intent": "x", "observed_at": "abc",
1026            "observations": [{"id": "o1", "statement": "s", "source": ""}],
1027            "conditions": [{"criterion": "a", "state": "satisfied"},
1028                           {"criterion": "zz", "state": "missing", "observations": ["o1"]},
1029                           {"criterion": "a", "state": "done", "observations": ["o9"]}]}));
1030        let f = review(&x(), &plan(vec![]), &[g], &[]);
1031        let got = codes(&f);
1032        for want in [
1033            ("observation_without_source", "x:o1"),
1034            ("condition_without_observation", "x#a"),
1035            ("gap_unknown_criterion", "x#zz"),
1036            ("gap_duplicate_condition", "x#a"),
1037            ("gap_unknown_state", "x#a"),
1038            ("gap_unknown_observation", "x#a"),
1039            ("gap_criterion_unanswered", "x#b"),
1040        ] {
1041            assert!(got.contains(&want), "missing {want:?} in {got:?}");
1042        }
1043    }
1044
1045    #[test]
1046    fn unknown_is_allowed_but_never_read_as_satisfied() {
1047        let g = good_gap();
1048        assert!(!observed_satisfied(std::slice::from_ref(&g)).contains_key("x"));
1049        let f = review(&x(), &plan(vec![issue("I1", &["x#a"], "READY")]), &[g], &[]);
1050        assert!(
1051            !codes(&f)
1052                .iter()
1053                .any(|(c, _)| c.starts_with("gap_") || c.starts_with("condition_")),
1054            "{f:?}"
1055        );
1056    }
1057
1058    #[test]
1059    fn gap_work_must_serve_a_criterion_and_land_in_an_issue_that_serves_it() {
1060        let g = gap(json!({"intent": "x", "observed_at": "abc",
1061            "conditions": [{"criterion": "a", "state": "unknown"}, {"criterion": "b", "state": "unknown"}],
1062            "work": [{"id": "w1", "title": "t", "serves": []},
1063                     {"id": "w2", "title": "t", "serves": ["q"], "issue": "I404"},
1064                     {"id": "w3", "title": "t", "serves": ["a"], "issue": "I2"}]}));
1065        let p = plan(vec![issue("I2", &["x#b"], "READY")]);
1066        let findings_1 = review(&x(), &p, &[g], &[]);
1067
1068        let got = codes(&findings_1);
1069        for want in [
1070            ("gap_work_serves_nothing", "x:w1"),
1071            ("gap_work_unknown_criterion", "x:w2"),
1072            ("gap_work_unknown_issue", "x:w2"),
1073            ("gap_work_issue_serves_other", "x:w3"),
1074        ] {
1075            assert!(got.contains(&want), "missing {want:?} in {got:?}");
1076        }
1077    }
1078
1079    #[test]
1080    fn a_gap_of_no_intent_or_no_commit_is_refused() {
1081        let got = review(
1082            &x(),
1083            &plan(vec![]),
1084            &[
1085                gap(json!({"intent": "nope"})),
1086                gap(json!({"intent": "x",
1087            "conditions": [{"criterion": "a", "state": "unknown"}, {"criterion": "b", "state": "unknown"}]})),
1088            ],
1089            &[],
1090        );
1091        let got = codes(&got);
1092        assert!(
1093            got.contains(&("gap_unknown_intent", ".ai/repo/project/gaps/x.yaml")),
1094            "{got:?}"
1095        );
1096        assert!(got.contains(&("gap_without_commit", "x")), "{got:?}");
1097    }
1098
1099    #[test]
1100    fn started_work_without_a_critique_is_refused_and_planned_work_is_warned() {
1101        let findings_2 = review(&x(), &plan(vec![issue("I1", &["x#a"], "ACTIVE")]), &[], &[]);
1102
1103        let got = codes(&findings_2);
1104        assert_eq!(got, [("executing_without_critique", "x")]);
1105        let findings_3 = review(&x(), &plan(vec![issue("I1", &["x#a"], "READY")]), &[], &[]);
1106
1107        let got = codes(&findings_3);
1108        assert_eq!(got, [("plan_not_critiqued", "x")]);
1109    }
1110
1111    #[test]
1112    fn an_open_blocking_finding_refuses_execution_but_not_planning() {
1113        let c = critique(json!({"intent": "x", "findings": [
1114            {"id": "f1", "class": "regression_risk", "subject": "x#a", "blocking": true, "resolution": {"state": "open"}},
1115            {"id": "f2", "class": "surface_missing", "subject": "x#a", "blocking": false, "resolution": {"state": "open"}}]}));
1116        let ready = review(
1117            &x(),
1118            &plan(vec![issue("I1", &["x#a"], "READY")]),
1119            &[],
1120            std::slice::from_ref(&c),
1121        );
1122        assert!(ready.is_empty(), "{ready:?}");
1123        let findings_4 = review(
1124            &x(),
1125            &plan(vec![issue("I1", &["x#a"], "VERIFY")]),
1126            &[],
1127            &[c],
1128        );
1129
1130        let active = codes(&findings_4);
1131        assert_eq!(active, [("executing_with_open_blocker", "x")]);
1132    }
1133
1134    #[test]
1135    fn a_resolution_must_be_readable_and_land_in_the_intents_work() {
1136        let c = critique(json!({"intent": "x", "findings": [
1137            {"id": "f1", "class": "vibes", "subject": "x#a", "resolution": {"state": "rejected"}},
1138            {"id": "f1", "class": "regression_risk", "subject": "x#a", "resolution": {"state": "planned", "issue": "I404"}},
1139            {"id": "f3", "class": "regression_risk", "subject": "x#a", "resolution": {"state": "planned", "issue": "I9"}},
1140            {"id": "f4", "class": "regression_risk", "subject": "x#a", "resolution": {"state": "maybe"}}]}));
1141        let p = plan(vec![issue("I9", &[], "READY")]);
1142        let findings_5 = review(&x(), &p, &[], &[c, critique(json!({"intent": "ghost"}))]);
1143
1144        let got = codes(&findings_5);
1145        for want in [
1146            ("critique_unknown_class", "x:f1"),
1147            ("rejected_without_reason", "x:f1"),
1148            ("critique_duplicate_finding", "x:f1"),
1149            ("planned_into_unknown_issue", "x:f1"),
1150            ("planned_into_unrelated_issue", "x:f3"),
1151            ("critique_unknown_resolution", "x:f4"),
1152            (
1153                "critique_unknown_intent",
1154                ".ai/repo/project/critiques/x.yaml",
1155            ),
1156        ] {
1157            assert!(got.contains(&want), "missing {want:?} in {got:?}");
1158        }
1159    }
1160
1161    #[test]
1162    fn a_satisfied_condition_counts_only_with_declared_observations() {
1163        let g = gap(json!({"intent": "x", "observed_at": "abc",
1164            "observations": [{"id": "o1", "statement": "s", "source": "test:t"}],
1165            "conditions": [{"criterion": "a", "state": "satisfied", "observations": ["o1"]},
1166                           {"criterion": "b", "state": "satisfied", "observations": ["o2"]}]}));
1167        assert_eq!(observed_satisfied(&[g])["x"], ["a"]);
1168    }
1169}
majordomus_cli/intent_review.rs

majordomus_cli/
intent_review.rs
