From 706c8108df2d49c52faf74c55680e2283c1157f0 Mon Sep 17 00:00:00 2001 From: Bendik Aagaard Lynghaug Date: Sat, 3 Oct 2026 10:48:09 +0200 Subject: [PATCH] iris check --findings: every finding as JSON with a stable id The id hashes scope, need, code and subject, nothing of the wording, so the same gap keeps its id across runs. Every message that knows its fix says it after "fix:". Codes listed in docs/lint.md. Co-Authored-By: Claude Opus 5.5 --- docs/lint.md | 22 ++++++++ src/lint/check.rs | 44 ++++++++++++++- src/lint/needs.rs | 139 +++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 187 insertions(+), 18 deletions(-) diff --git a/docs/lint.md b/docs/lint.md index 39c860e..4be9756 100644 --- a/docs/lint.md +++ b/docs/lint.md @@ -50,6 +50,28 @@ already has the button, or the edge aggregates.yaml lacks. A schema error no longer hides the needs findings - both are printed, so one round can fix them together. +`--findings ` writes the same findings one JSON object per +line: `id`, `level` (`fail`/`warn`), `code`, `scope` (`need`, +`bucket`, or `repo` for a schema error), `need` (the need's id, or the +bucket's name), `subject` (the state, the move as `from>to`, or the +page), `message` and `fix`. The file is written empty when the content +is clean. `id` hashes scope, need, code and subject and nothing of the +wording, so the same gap keeps its id from one run to the next: a +trainer can tell what a round closed, what is still open and what it +broke. The codes: + +| code | | +|---|---| +| `schema` | content the site would refuse to load | +| `need-duplicate`, `persona-unknown-need` | needs.yaml itself | +| `entry-missing`, `entry-gated`, `entry-no-responsible` | the page a visitor arrives on | +| `path-missing`, `path-closed`, `path-over-budget`, `path-no-action` | getting the answer recorded | +| `machine-missing`, `desk-missing`, `desk-gated-twice` | who works the bucket | +| `done-unknown`, `done-unreachable`, `success-not-done` | finishing a case | +| `stage-unknown`, `stage-unreachable`, `stage-order` | the business's own stages | +| `state-unreached`, `strand`, `edge-unoffered` | every state and move of the bucket | +| `mail-unreachable`, `mail-unknown-field` | what a state mails | + A need marked `planned` warns instead of failing. Exit status is the verdict, so a content repo's CI is one line. diff --git a/src/lint/check.rs b/src/lint/check.rs index 0030ac6..1c70723 100644 --- a/src/lint/check.rs +++ b/src/lint/check.rs @@ -24,6 +24,7 @@ pub async fn run(argv: Vec) -> anyhow::Result<()> { let mut skim = false; let mut sim_out: Option = None; let mut show_access = false; + let mut findings_out: Option = None; while let Some(arg) = args.next() { match arg.as_str() { @@ -35,6 +36,7 @@ pub async fn run(argv: Vec) -> anyhow::Result<()> { "--skim" => skim = true, "--needs-sim" => sim_out = args.next(), "--access" => show_access = true, + "--findings" => findings_out = args.next(), "--needs-score" => answers_in = args.next(), "--min-accuracy" => { min_accuracy = args.next().and_then(|v| v.parse().ok()).unwrap_or(min_accuracy) @@ -66,13 +68,14 @@ pub async fn run(argv: Vec) -> anyhow::Result<()> { (questions, aggregates_map, site, needs_raw) } else { eprintln!( - "usage: iris check (--repo [--branch main] [--subdir questions] | --path ) [--needs-tasks [--skim]] [--needs-sim ] [--access] [--needs-score [--min-accuracy 0.8]]" + "usage: iris check (--repo [--branch main] [--subdir questions] | --path ) [--findings ] [--needs-tasks [--skim]] [--needs-sim ] [--access] [--needs-score [--min-accuracy 0.8]]" ); std::process::exit(2); }; if let Err(e) = site.validate() { eprintln!("FAIL: {e}"); + write_findings(findings_out.as_deref(), &[schema_finding(&e.to_string())]); std::process::exit(1); } let aggregates_map = content::with_builtin_aggregates(aggregates_map); @@ -114,6 +117,7 @@ pub async fn run(argv: Vec) -> anyhow::Result<()> { println!(); } check_needs( + findings_out.as_deref(), needs_raw.as_deref(), &questions, &aggregates_map, @@ -129,14 +133,43 @@ pub async fn run(argv: Vec) -> anyhow::Result<()> { // One schema error is no reason to hide the rest: the needs // pass reads the same content and says what else is wrong, // so a fix round can take them all at once. + let mut all = vec![schema_finding(&e.to_string())]; if let Some(file) = needs_raw.as_deref().and_then(|raw| needs::parse(raw).ok()) { - print_findings(&needs::check(&file, &questions, &aggregates_map)); + let findings = needs::check(&file, &questions, &aggregates_map); + print_findings(&findings); + all.extend(findings); } + write_findings(findings_out.as_deref(), &all); std::process::exit(1); } } } +/// A content error the schema pass stopped at, as a finding: the repo +/// is its scope and the message its subject, since it has no other. +fn schema_finding(message: &str) -> needs::Finding { + needs::Finding { + need: String::new(), + level: needs::Level::Fail, + message: message.to_string(), + scope: "repo", + code: "schema", + subject: message.to_string(), + } +} + +/// `--findings`: every finding as one JSON line (id, level, code, +/// scope, need, subject, message, fix), the same ones the text output +/// prints - an empty file when there are none, so a reader can tell +/// "clean" from "not run". +fn write_findings(out: Option<&str>, findings: &[needs::Finding]) { + let Some(out) = out else { return }; + let lines: String = findings.iter().map(|f| format!("{}\n", f.to_json())).collect(); + if let Err(e) = std::fs::write(out, lines) { + eprintln!("WARN: could not write the findings to {out}: {e}"); + } +} + /// Each finding on its own line, FAIL or WARN first; the count of FAILs. fn print_findings(findings: &[needs::Finding]) -> usize { let mut failed = 0; @@ -157,6 +190,7 @@ fn print_findings(findings: &[needs::Finding]) -> usize { /// then the wording tasks written out and/or an engine's answers /// scored, when asked. A repo without `needs.yaml` skips all of it. fn check_needs( + findings_out: Option<&str>, raw: Option<&str>, questions: &std::collections::HashMap, aggregates_map: &std::collections::HashMap, @@ -167,6 +201,7 @@ fn check_needs( min_accuracy: f64, ) -> anyhow::Result<()> { let Some(raw) = raw else { + write_findings(findings_out, &[]); if tasks_out.is_some() || answers_in.is_some() { eprintln!("FAIL: --needs-tasks/--needs-score given, but the repo has no needs.yaml"); std::process::exit(1); @@ -177,10 +212,13 @@ fn check_needs( Ok(file) => file, Err(e) => { eprintln!("FAIL: {e}"); + write_findings(findings_out, &[schema_finding(&e.to_string())]); std::process::exit(1); } }; - let failed = print_findings(&needs::check(&file, questions, aggregates_map)); + let findings = needs::check(&file, questions, aggregates_map); + let failed = print_findings(&findings); + write_findings(findings_out, &findings); for need in &file.needs { if let Some(path) = needs::path_for(questions, need) { let steps: Vec = path diff --git a/src/lint/needs.rs b/src/lint/needs.rs index 9d0f016..c538815 100644 --- a/src/lint/needs.rs +++ b/src/lint/needs.rs @@ -144,6 +144,48 @@ pub struct Finding { /// "need" for a need's own path; "bucket" for what holds for every /// record in a bucket whichever need brought it in. pub scope: &'static str, + /// What kind of gap it is, stable across releases: `strand`, + /// `edge-unoffered`, `done-unreachable`, ... (docs/lint.md). + pub code: &'static str, + /// What in the content it is about: a state, a move (`from>to`), a + /// page. With the scope, need and code it makes the finding's id. + pub subject: String, +} + +impl Finding { + /// The same gap keeps the same id however its message is worded, + /// so a fix round can tell what it closed, what is still open and + /// what it broke. FNV-1a over scope, need, code and subject. + pub fn id(&self) -> String { + let mut hash: u64 = 0xcbf2_9ce4_8422_2325; + for part in [self.scope, &self.need, self.code, &self.subject] { + for byte in part.bytes().chain(std::iter::once(0)) { + hash ^= u64::from(byte); + hash = hash.wrapping_mul(0x0100_0000_01b3); + } + } + format!("{:08x}", hash >> 32) + } + + /// The change that closes it, when the lint knows one: what follows + /// "fix:" in the message. + pub fn fix(&self) -> Option<&str> { + self.message.split_once(" - fix: ").map(|(_, fix)| fix) + } + + /// One line of `--findings` JSONL. + pub fn to_json(&self) -> serde_json::Value { + serde_json::json!({ + "id": self.id(), + "level": match self.level { Level::Fail => "fail", Level::Warn => "warn" }, + "code": self.code, + "scope": self.scope, + "need": self.need, + "subject": self.subject, + "message": self.message, + "fix": self.fix(), + }) + } } #[derive(Clone, Debug, PartialEq, Eq)] @@ -354,10 +396,12 @@ pub fn check( let mut strands_reported: HashSet<(String, String)> = HashSet::new(); let mut unreached_reported: HashSet<(String, String)> = HashSet::new(); for need in &file.needs { - let mut report = |level: Level, message: String| { + let mut report = |level: Level, code: &'static str, subject: String, message: String| { findings.push(Finding { need: need.id.clone(), scope: "need", + code, + subject, level: if need.planned || need.paused { Level::Warn } else { level }, message: match (need.paused, need.planned, level == Level::Fail) { (true, _, true) => format!("paused, not open now: {message}"), @@ -367,15 +411,17 @@ pub fn check( }) }; if !seen.insert(need.id.as_str()) { - report(Level::Fail, "declared more than once".to_string()); + report(Level::Fail, "need-duplicate", String::new(), "declared more than once".to_string()); continue; } // The visitor's side: can they get their answer recorded? match questions.get(&need.enters) { - None => report(Level::Fail, format!("entry page {:?} does not exist", need.enters)), + None => report(Level::Fail, "entry-missing", need.enters.clone(), format!("entry page {:?} does not exist", need.enters)), Some(page) if !visible(page, need.enters_as.as_deref()) => report( Level::Fail, + "entry-gated", + need.enters.clone(), format!( "entry page {:?} is gated on {:?}, which this visitor does not hold", need.enters, page.qualifies @@ -385,6 +431,8 @@ pub fn check( if page.responsible.is_none() { report( Level::Warn, + "entry-no-responsible", + need.enters.clone(), format!("entry page {:?} names no one responsible to turn to", need.enters), ); } @@ -412,6 +460,8 @@ pub fn check( .unwrap_or_default(); report( Level::Fail, + "path-closed", + at.clone(), format!( "the path exists but is closed: {at} is `disabled`, so the answer is advertised and refused" ), @@ -419,6 +469,8 @@ pub fn check( } None => report( Level::Fail, + "path-missing", + String::new(), format!( "no path from {:?}{} records into {:?} within {} submission(s)", need.enters, @@ -435,6 +487,8 @@ pub fn check( if best.fields > budget { report( Level::Fail, + "path-over-budget", + String::new(), format!( "the path asks for {} required field(s), over the budget of {budget}", best.fields @@ -445,6 +499,8 @@ pub fn check( if best.records_with_action == Some(false) { report( Level::Warn, + "path-no-action", + String::new(), "the recording alternative has no `action` - the visitor is never told what happens next".to_string(), ); } @@ -457,6 +513,8 @@ pub fn check( let Some(schema) = aggregates.get(&need.lands_in) else { report( Level::Fail, + "machine-missing", + need.lands_in.clone(), format!("aggregates.yaml declares no state machine for {:?}", need.lands_in), ); continue; @@ -475,6 +533,8 @@ pub fn check( .collect(); report( Level::Fail, + "desk-missing", + need.handled_by.clone(), format!( "no desk gated on {:?} reads {:?}{}", need.handled_by, @@ -556,7 +616,7 @@ pub fn check( match ways_in(state).first() { Some((from, to)) => format!(" - fix: {}", fix_for(from, to)), None => format!( - " - and no state a record reaches has a declared edge into it: add one in aggregates.yaml under {:?}", + " - fix: no state a record reaches has a declared edge into it; add one in aggregates.yaml under {:?}", need.lands_in ), } @@ -565,12 +625,16 @@ pub fn check( if !schema.has_state(state) { report( Level::Fail, + "done-unknown", + state.clone(), format!("done_when names {state:?}, which {:?} has no such state", need.lands_in), ); } else if !reached.contains(state) { unreached_reported.insert((need.lands_in.clone(), state.clone())); report( Level::Fail, + "done-unreachable", + state.clone(), format!( "{:?} cannot move a record to {state:?} - no desk of theirs offers the moves{}", need.handled_by, @@ -583,6 +647,8 @@ pub fn check( if !schema.has_state(stage) { report( Level::Fail, + "stage-unknown", + stage.clone(), format!( "the business describes a stage {stage:?} that {:?} has no state for", need.lands_in @@ -592,6 +658,8 @@ pub fn check( unreached_reported.insert((need.lands_in.clone(), stage.clone())); report( Level::Fail, + "stage-unreachable", + stage.clone(), format!( "stage {stage:?} exists but {:?} is offered no moves that reach it{}", need.handled_by, @@ -613,11 +681,13 @@ pub fn check( if reached.contains(before) && reached.contains(after) && !reachable(before, &moves).contains(after) { report( Level::Fail, + "stage-order", + format!("{before}>{after}"), format!( "stage {after:?} comes after {before:?} for the business, but no move leads on from {before:?} to it{}", match schema.allowed(before).iter().find(|to| reachable(to, &moves).contains(after) || *to == after) { Some(to) => format!(" - fix: {}", fix_for(before, to)), - None => format!(" - add an edge from {before:?} toward {after:?} in aggregates.yaml under {:?}", need.lands_in), + None => format!(" - fix: add an edge from {before:?} toward {after:?} in aggregates.yaml under {:?}", need.lands_in), } ), ); @@ -627,6 +697,8 @@ pub fn check( if !need.done_when.contains(state) { report( Level::Fail, + "success-not-done", + state.clone(), format!("success names {state:?}, which is not one of done_when"), ); } @@ -639,12 +711,14 @@ pub fn check( let fix = match schema.allowed(&state).first() { Some(to) => format!(" - fix: {}", fix_for(&state, to)), None => format!( - " - and {:?} declares no way out of it: add {state:?} to done_when if it is finished, or an edge out in aggregates.yaml", + " - fix: {:?} declares no way out of it; add {state:?} to done_when if it is finished, or an edge out in aggregates.yaml", need.lands_in ), }; report( Level::Fail, + "strand", + state.clone(), format!("records strand in {state:?}: {movers} is offered no move out of it{fix}"), ); } @@ -658,6 +732,8 @@ pub fn check( findings.push(Finding { need: persona.need.clone(), scope: "need", + code: "persona-unknown-need", + subject: persona.id.clone(), level: Level::Fail, message: format!("persona {:?} points at a need that is not declared", persona.id), }); @@ -690,10 +766,12 @@ fn operations( // Only open needs make a gap fail; a bucket that only planned // or paused needs use reports it as a warning. let open = needs.iter().any(|n| !n.planned && !n.paused); - let mut report = |level: Level, message: String| { + let mut report = |level: Level, code: &'static str, subject: String, message: String| { findings.push(Finding { need: bucket.to_string(), scope: "bucket", + code, + subject, level: if open { level } else { Level::Warn }, message, }) @@ -728,6 +806,8 @@ fn operations( ways.sort(); report( Level::Fail, + "state-unreached", + (*state).clone(), format!( "state {state:?} is never reached: no desk, link or clock moves a record there - fix: {}", match ways.first() { @@ -747,6 +827,8 @@ fn operations( if !strands_reported.contains(&(bucket.to_string(), (*state).clone())) { report( Level::Fail, + "strand", + (*state).clone(), format!( "records strand in {state:?}: {bucket:?} declares moves on to {} but no desk, link or clock offers one - fix: {}", out.join(", "), @@ -759,8 +841,10 @@ fn operations( for to in out.iter().filter(|to| !offered(state, to)) { report( Level::Warn, + "edge-unoffered", + format!("{state}>{to}"), format!( - "the move {state:?} -> {to:?} is declared but no desk, link or clock offers it - {}, or drop the edge from aggregates.yaml", + "the move {state:?} -> {to:?} is declared but no desk, link or clock offers it - fix: {}, or drop the edge from aggregates.yaml", button(state, to) ), ); @@ -771,8 +855,10 @@ fn operations( if page_group != group { report( Level::Warn, + "desk-gated-twice", + desk.page.clone(), format!( - "the desk on {} is gated twice: the page qualifies {page_group:?}, its list requires {group:?} - only members of both see the buttons; make them the same group", + "the desk on {} is gated twice: the page qualifies {page_group:?}, its list requires {group:?}, so only members of both see the buttons - fix: make them the same group", desk.page ), ); @@ -793,7 +879,7 @@ fn operations( mail_states.sort_by(|a, b| a.0.cmp(b.0)); for (state, mail) in mail_states { if !reached.contains(state) { - report(Level::Warn, format!("the mail on entering {state:?} never goes out: no record reaches {state:?}")); + report(Level::Warn, "mail-unreachable", state.clone(), format!("the mail on entering {state:?} never goes out: no record reaches {state:?} - fix: make {state:?} reachable, or drop its mail")); } let mut unknown: Vec = Vec::new(); for key in placeholders(&mail.subject).into_iter().chain(placeholders(&mail.body)) { @@ -811,8 +897,10 @@ fn operations( names.dedup(); report( Level::Fail, + "mail-unknown-field", + state.clone(), format!( - "the mail on entering {state:?} uses {}, which no form recording into {bucket:?} collects - it would go out blank there; the forms collect: {}", + "the mail on entering {state:?} uses {}, which no form recording into {bucket:?} collects, so it would go out blank there - fix: use one of the fields the forms collect ({}), or ask for it on the form", unknown.iter().map(|k| format!("{{{k}}}")).collect::>().join(", "), if names.is_empty() { "nothing".to_string() } else { names.join(", ") } ), @@ -1407,7 +1495,7 @@ mod tests { assert!(fails(&findings).is_empty(), "{findings:?}"); assert_eq!( warns(&findings), - vec!["the move \"open\" -> \"lost\" is declared but no desk, link or clock offers it - add `{ from: open, to: lost, label: ... }` to the transitions on /desk, or drop the edge from aggregates.yaml"] + vec!["the move \"open\" -> \"lost\" is declared but no desk, link or clock offers it - fix: add `{ from: open, to: lost, label: ... }` to the transitions on /desk, or drop the edge from aggregates.yaml"] ); } @@ -1422,7 +1510,7 @@ mod tests { let findings = check(&needs(" stages: [open, surveyed, quoted, won, lost]\n"), &questions, &aggregates); assert_eq!( fails(&findings), - vec!["stage \"quoted\" comes after \"surveyed\" for the business, but no move leads on from \"surveyed\" to it - add an edge from \"surveyed\" toward \"quoted\" in aggregates.yaml under \"projects\""] + vec!["stage \"quoted\" comes after \"surveyed\" for the business, but no move leads on from \"surveyed\" to it - fix: add an edge from \"surveyed\" toward \"quoted\" in aggregates.yaml under \"projects\""] ); } @@ -1435,7 +1523,7 @@ mod tests { let findings = check(&needs(""), &questions, &aggregates); assert_eq!( fails(&findings), - vec!["the mail on entering \"open\" uses {refnr}, which no form recording into \"projects\" collects - it would go out blank there; the forms collect: brief, budget, email"] + vec!["the mail on entering \"open\" uses {refnr}, which no form recording into \"projects\" collects, so it would go out blank there - fix: use one of the fields the forms collect (brief, budget, email), or ask for it on the form"] ); } @@ -1445,8 +1533,29 @@ mod tests { questions.get_mut("/desk").unwrap().qualifies = Some("staff".into()); let findings = check(&needs(""), &questions, &aggregates); assert!( - warns(&findings).contains(&"the desk on /desk is gated twice: the page qualifies \"staff\", its list requires \"owners\" - only members of both see the buttons; make them the same group"), + warns(&findings).contains(&"the desk on /desk is gated twice: the page qualifies \"staff\", its list requires \"owners\", so only members of both see the buttons - fix: make them the same group"), "{findings:?}" ); } + + #[test] + fn a_finding_keeps_its_id_when_reworded_and_says_its_fix() { + let (questions, aggregates) = site(" - { to: quoted, label: Quote }\n - { to: lost, label: Lose }\n"); + let findings = check(&needs(""), &questions, &aggregates); + let strand = findings.iter().find(|f| f.code == "strand").unwrap(); + assert_eq!(strand.subject, "quoted"); + assert_eq!(strand.fix(), Some("add `{ from: quoted, to: won, label: ... }` to the transitions on /desk")); + let reworded = Finding { message: "something else entirely".into(), ..clone_of(strand) }; + assert_eq!(reworded.id(), strand.id()); + let elsewhere = Finding { subject: "open".into(), ..clone_of(strand) }; + assert_ne!(elsewhere.id(), strand.id()); + let json = strand.to_json(); + assert_eq!(json["code"], "strand"); + assert_eq!(json["level"], "fail"); + assert_eq!(json["id"], strand.id()); + } + + fn clone_of(f: &Finding) -> Finding { + Finding { need: f.need.clone(), level: f.level, message: f.message.clone(), scope: f.scope, code: f.code, subject: f.subject.clone() } + } }