diff --git a/docs/training.md b/docs/training.md index 29ddd26..1301c4c 100644 --- a/docs/training.md +++ b/docs/training.md @@ -243,13 +243,24 @@ against the lint. It is kept for the record of what it measured. visitor will tolerate. 2. **Structure.** Pages, state machines and desks are generated, then revised against the lint, `iris check`, until every need is met: - a path within budget, a desk held by the right group, every - finished state reachable, no record stranding. The page-writing - model sees the needs and never the personas. + a path within budget, a desk held by the right group, every state + of every bucket reachable and none stranding a record. The + page-writing model sees the needs and never the personas. Each + round the lint writes its findings with stable ids (`--findings`, + kept in `/findings/round-N.jsonl`), and the round is logged as + what it fixed, what is still open, what is new and what it + reopened. A fix round is shown only the open findings, each with + the change that closes it, and the files they name (the whole repo + when a finding is about getting around the site). A change that + reopens a closed finding is undone and the model told what it + broke. A round that closes nothing, or leaves as many open as it + found, ends the phase: what is left is written to `run.log` rather + than polished at again. 3. **Wording.** A separate model plays each persona, blind, first with headings only and then with descriptions. A misroute is a wording - defect and goes back to the generator. A rewrite that breaks - structure is reverted. + defect and goes back to the generator; a visitor who chose right + but hesitated is reported and rewrites nothing by itself. A rewrite + that breaks structure is reverted. Model output is only ever written to `aggregates.yaml`, `site.yaml` and `questions/**.yaml` inside the output directory. diff --git a/src/trainer/mod.rs b/src/trainer/mod.rs index 0182eb4..eb8a487 100644 --- a/src/trainer/mod.rs +++ b/src/trainer/mod.rs @@ -132,34 +132,91 @@ pub fn run(argv: Vec) -> anyhow::Result<()> { .to_string(); let mut green = false; let mut report = String::new(); + // What the lint found, by id, round over round (`--findings`): what + // a round closed, what it left, what it broke. `None` while the + // lint is one that predates `--findings`; then the whole report + // goes back, as it always did. + let mut ledger = Ledger::default(); + let mut tracked = true; + let mut undone = String::new(); + std::fs::create_dir_all(opts.out.join("findings"))?; for round in 1..=opts.rounds { let prompt = if round == 1 { format!( "{REFERENCE}\n\n# Task\n\nWrite the content repo that meets this frozen `needs.yaml`: `aggregates.yaml`, `site.yaml`, and every page under `questions/` - the front door, followup pages, role-gated pages, one desk per handling group. Do not return needs.yaml.\n\n## Intake\n\n{intake}\n\n## needs.yaml (frozen)\n\n```yaml\n{needs}\n```\n\n{FORMAT}" ) + } else if tracked { + let (heading, files) = match named_files(&opts.out, ledger.open.values()) { + Some(files) => ("Files the findings name", dump_files(&opts.out, &files)?), + None => ("Current files", dump(&opts.out)?), + }; + format!( + "{REFERENCE}\n\n# Task\n\nThe verifier rejected the repo. Below is every finding still open, each with the change that closes it. Make those changes and nothing else: findings already closed stay closed. Return only the files you change, each complete. To remove a file, return it with the single line DELETE. needs.yaml is frozen; do not return it.\n\n## Open findings\n\n{}\n{undone}## needs.yaml (frozen)\n\n```yaml\n{needs}\n```\n\n## {heading}\n\n{files}\n\n{FORMAT}", + ledger.open_list(round) + ) } else { format!( "{REFERENCE}\n\n# Task\n\nThe verifier rejected the repo below. Fix it. Return only the files you change, each complete. To remove a file, return it with the single line DELETE. needs.yaml is frozen; do not return it.\n\n## Verifier output\n\n```\n{report}\n```\n\n## needs.yaml (frozen)\n\n```yaml\n{needs}\n```\n\n## Current files\n\n{}\n\n{FORMAT}", dump(&opts.out)? ) }; + let before = snapshot(&opts.out)?; match parse_files(&llm(&opts.llm, &prompt)?) { Ok(files) => apply(&opts.out, files)?, Err(_) => eprintln!("structure round {round}: the reply had no file blocks"), } - let (ok, r) = lint(&opts, &[])?; + let (ok, r, found) = lint_findings(&opts, &opts.out.join(format!("findings/round-{round}.jsonl")))?; report = r; - let fails = report.lines().filter(|l| l.starts_with("FAIL")).count(); - eprintln!("structure round {round}: {}", if ok { "green".to_string() } else { format!("{fails} FAIL line(s)") }); - log.push(format!("structure round {round}: ok={ok} fails={fails}")); + let Some(found) = found else { + tracked = false; + let fails = report.lines().filter(|l| l.starts_with("FAIL")).count(); + eprintln!("structure round {round}: {}", if ok { "green".to_string() } else { format!("{fails} FAIL line(s)") }); + log.push(format!("structure round {round}: ok={ok} fails={fails}")); + if ok { + green = true; + break; + } + continue; + }; + let previous = ledger.clone(); + let progress = ledger.update(round, found.into_iter().filter(|f| f.level == "fail").collect()); + let line = format!("structure round {round}: {}", progress.summary(ledger.open.len())); + eprintln!("{line}"); + log.push(line); + undone.clear(); if ok { green = true; break; } + if round == 1 { + continue; + } + // A fix that reopens what an earlier round closed is undone, + // and the model is told what it broke, rather than chasing the + // same findings round after round. + if !progress.reopened.is_empty() { + restore(&opts.out, before)?; + ledger = previous; + undone = format!( + "## Your last change was undone\n\nIt reopened findings an earlier round had closed:\n\n{}\nClose the open findings without touching what made these pass.\n\n", + progress.reopened.iter().map(|f| format!("- [{}] {}\n", f.id, f.message)).collect::() + ); + eprintln!("structure round {round}: reopened {} closed finding(s), undone", progress.reopened.len()); + log.push(format!("structure round {round}: undone (reopened {})", progress.reopened.len())); + continue; + } + // No progress, no more rounds: another pass at the same findings + // is how a site gets over-trained. What is left is reported. + if progress.fixed.is_empty() || ledger.open.len() >= previous.open.len() { + eprintln!("structure round {round}: no finding closed, stopping"); + log.push(format!("structure round {round}: stalled")); + break; + } } if !green { - std::fs::write(opts.out.join("run.log"), format!("{}\n\n{report}", log.join("\n")))?; - bail!("structure never went green in {} round(s); last verifier output is in run.log", opts.rounds); + let open = if tracked { format!("\n\nStill open:\n{}", ledger.open_list(opts.rounds + 1)) } else { String::new() }; + std::fs::write(opts.out.join("run.log"), format!("{}{open}\n\n{report}", log.join("\n")))?; + bail!("structure did not go green; what is still open is in run.log"); } // Phase 3: wording. @@ -191,7 +248,10 @@ pub fn run(argv: Vec) -> anyhow::Result<()> { } eprintln!("wording round {round}: {}", summary.join(" | ")); log.push(format!("wording round {round}: {}", summary.join(" | "))); - if defects.is_empty() || round == opts.wording_rounds { + // A visitor who chose right, hesitantly, is not a defect worth a + // rewrite on its own: only a misroute is. Hesitations ride along + // as context when there is one. + if !defects.contains("MISROUTE") || round == opts.wording_rounds { break; } let personas = needs_full.split("\npersonas:").nth(1).unwrap_or_default(); @@ -311,6 +371,152 @@ fn lint(opts: &Opts, extra: &[String]) -> anyhow::Result<(bool, String)> { Ok((output.status.success(), report)) } +/// One finding as the lint's `--findings` writes it (docs/lint.md). +#[derive(Clone, Debug, serde::Deserialize)] +struct Finding { + id: String, + level: String, + code: String, + #[serde(default)] + need: String, + message: String, + #[serde(default)] + fix: Option, +} + +/// The lint with `--findings` into `out`, and what it found; `None` for +/// the findings when the lint predates the flag (then it is run again +/// the old way, and only its text comes back). +fn lint_findings(opts: &Opts, out: &Path) -> anyhow::Result<(bool, String, Option>)> { + let (ok, report) = lint(opts, &["--findings".to_string(), out.display().to_string()])?; + if report.contains("unknown argument: --findings") { + let (ok, report) = lint(opts, &[])?; + return Ok((ok, report, None)); + } + let raw = std::fs::read_to_string(out).unwrap_or_default(); + let found = raw + .lines() + .filter(|l| !l.trim().is_empty()) + .map(|l| serde_json::from_str::(l).with_context(|| format!("reading {}", out.display()))) + .collect::>>()?; + Ok((ok, report, Some(found))) +} + +/// Every failing finding by id, across rounds. +#[derive(Clone, Default)] +struct Ledger { + open: BTreeMap, + /// The round each open finding was first seen in. + since: BTreeMap, + /// Ids a round has closed, so one that comes back is a regression, + /// not news. + closed: std::collections::BTreeSet, +} + +struct Progress { + fixed: Vec, + new: Vec, + still: usize, + reopened: Vec, +} + +impl Progress { + fn summary(&self, open: usize) -> String { + if open == 0 { + return format!("green (fixed {})", self.fixed.len()); + } + format!( + "{open} open - fixed {}, still open {}, new {}, reopened {}", + self.fixed.len(), + self.still, + self.new.len(), + self.reopened.len() + ) + } +} + +impl Ledger { + fn update(&mut self, round: usize, fails: Vec) -> Progress { + let now: BTreeMap = fails.into_iter().map(|f| (f.id.clone(), f)).collect(); + let fixed: Vec = self.open.values().filter(|f| !now.contains_key(&f.id)).cloned().collect(); + let mut new = Vec::new(); + let mut reopened = Vec::new(); + let mut still = 0; + for (id, f) in &now { + if self.open.contains_key(id) { + still += 1; + } else if self.closed.contains(id) { + reopened.push(f.clone()); + } else { + new.push(f.clone()); + } + self.since.entry(id.clone()).or_insert(round); + } + for f in &fixed { + self.closed.insert(f.id.clone()); + self.since.remove(&f.id); + } + self.open = now; + Progress { fixed, new, still, reopened } + } + + /// The open findings for a prompt or a log: id, how long open, the + /// message and its fix. + fn open_list(&self, round: usize) -> String { + self.open + .values() + .map(|f| { + let age = match self.since.get(&f.id) { + Some(&first) if first + 1 < round => format!(", open since round {first}"), + _ => String::new(), + }; + let scope = if f.need.is_empty() { String::new() } else { format!(" {:?}", f.need) }; + format!("- [{}] {}{scope}{age}: {}\n", f.id, f.code, f.message) + }) + .collect() + } +} + +/// The files open findings point at - every desk page their fixes name, +/// plus aggregates.yaml - when that is enough to act on. `None` when a +/// finding is about getting around the site (a path, an entry page, a +/// missing desk) or names no page: then the model needs to see it all. +fn named_files<'a>(out: &Path, findings: impl Iterator) -> Option> { + const WHOLE_SITE: &[&str] = &["schema", "entry-missing", "entry-gated", "path-missing", "path-closed", "path-over-budget", "desk-missing", "need-duplicate"]; + let mut files = vec![out.join("aggregates.yaml")]; + for f in findings { + if WHOLE_SITE.contains(&f.code.as_str()) { + return None; + } + let text = format!("{} {}", f.message, f.fix.as_deref().unwrap_or_default()); + for page in text.split_whitespace().filter(|w| w.starts_with('/')) { + let page = page.trim_end_matches(|c: char| !(c.is_alphanumeric() || c == '_' || c == '-' || c == '/')); + let rel = page.trim_start_matches('/'); + let candidates = if rel.is_empty() { + vec![out.join("questions/index.yaml")] + } else { + vec![out.join(format!("questions/{rel}.yaml")), out.join(format!("questions/{rel}/index.yaml"))] + }; + if let Some(path) = candidates.into_iter().find(|p| p.exists()) { + if !files.contains(&path) { + files.push(path); + } + } + } + } + Some(files) +} + +/// `dump`, for the named files only. +fn dump_files(out: &Path, files: &[PathBuf]) -> anyhow::Result { + let mut text = String::new(); + for path in files.iter().filter(|p| p.exists()) { + let rel = path.strip_prefix(out).unwrap_or(path).display().to_string(); + text.push_str(&format!("<<>>\n{}<<>>\n\n", std::fs::read_to_string(path)?)); + } + Ok(text) +} + /// `<<>> ... <<>>` blocks. Anything outside a block is /// ignored, so a model that adds a sentence does no harm. fn parse_files(text: &str) -> anyhow::Result> { @@ -591,3 +797,44 @@ mod tests { assert!(!allowed(".gitea/workflows/deploy.yaml")); } } + +#[cfg(test)] +mod ledger_tests { + use super::*; + + fn f(id: &str, code: &str, message: &str) -> Finding { + Finding { id: id.into(), level: "fail".into(), code: code.into(), need: "hire".into(), message: message.into(), fix: None } + } + + #[test] + fn a_round_is_told_what_it_closed_kept_added_and_broke() { + let mut ledger = Ledger::default(); + let p = ledger.update(1, vec![f("a", "strand", "x"), f("b", "strand", "y")]); + assert_eq!((p.fixed.len(), p.new.len(), p.still, p.reopened.len()), (0, 2, 0, 0)); + let p = ledger.update(2, vec![f("b", "strand", "y"), f("c", "edge-unoffered", "z")]); + assert_eq!((p.fixed.len(), p.new.len(), p.still, p.reopened.len()), (1, 1, 1, 0)); + assert_eq!(p.summary(ledger.open.len()), "2 open - fixed 1, still open 1, new 1, reopened 0"); + // "a" was closed in round 2; its return is a regression. + let p = ledger.update(3, vec![f("a", "strand", "x"), f("b", "strand", "y")]); + assert_eq!(p.reopened.iter().map(|f| f.id.as_str()).collect::>(), ["a"]); + assert!(ledger.open_list(4).contains("[b] strand \"hire\", open since round 1: y")); + let p = ledger.update(4, vec![]); + assert_eq!(p.summary(ledger.open.len()), "green (fixed 2)"); + } + + #[test] + fn a_fix_round_sees_the_pages_its_findings_name() { + let dir = std::env::temp_dir().join(format!("ledger-test-{}", std::process::id())); + std::fs::create_dir_all(dir.join("questions/desk")).unwrap(); + std::fs::write(dir.join("aggregates.yaml"), "x").unwrap(); + std::fs::write(dir.join("questions/desk/index.yaml"), "x").unwrap(); + std::fs::write(dir.join("questions/index.yaml"), "x").unwrap(); + let mut strand = f("a", "strand", "records strand in \"quoted\""); + strand.fix = Some("add `{ from: quoted, to: won, label: ... }` to the transitions on /desk".into()); + let files = named_files(&dir, [&strand].into_iter()).unwrap(); + assert_eq!(files, vec![dir.join("aggregates.yaml"), dir.join("questions/desk/index.yaml")]); + // A finding about getting around the site needs all of it. + assert!(named_files(&dir, [&strand, &f("b", "path-missing", "no path")].into_iter()).is_none()); + std::fs::remove_dir_all(&dir).ok(); + } +}