From 3e30f714d35c990e000c0ec2c0692bb6c95d02ce Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 4 Sep 2026 19:22:13 +0000 Subject: [PATCH 1/2] fix(git): classify declaration-language comments so prose-only can see them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `without_comments` knew four extensions — `.md`, `.rs`, the shell family, and extensionless `mise-tasks/` programs. Those are the languages a consumer writes PROGRAMS in, which is the set CLOUD-827's shell predecessor classified. A declarative consumer keeps most of its prose somewhere else: policy modules, config, pipeline documents, workflows. Every one of those took the unrecognised-extension fall-through, whose remainder is the whole file, so a comment-only change to any of them read as a code change and `prose-only` could not fire over it. Measured 2026-09-04, on this branch's own first commit: a 121-to-71 comment trim of one policy module, nothing else in the diff, `batten check --rule prose-only` exit 0, and a full required matrix spent on a change CI does not judge. That is precisely the instance CLOUD-827 exists to price. `.rego`, `.toml`, `.yml`/`.yaml` are `#`; `.pkl` is `//` — Java-family, where `#` is not a comment marker at all, so one table for everything `#`-ish would have kept real code in the remainder and dropped commented lines from it. The fall-through's direction is unchanged and still right for a genuinely unknown extension: it admits the branch rather than blocking it, and only blocking is unrecoverable by waiting. What was wrong was calling these unknown. Both tiers, because the load-time one cannot see this class: * `git::tests` pins the table, one assertion per extension, so one arm's presence cannot stand in for the rest; * `crates/batten/tests/it/prose_only.rs` drives the compiled engine over a real `origin/main` for all four, which is the tier that proves the remainder comparison the ENGINE builds now reaches them. Every existing case in that file is a `.rs`, `.md`, `.bats` or extensionless program — the same blind spot one layer up, and the reason the gap survived. Refs: CLOUD-827 --- crates/batten/src/git.rs | 67 ++++++++++++++++++++++++++++ crates/batten/tests/it/prose_only.rs | 66 +++++++++++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/crates/batten/src/git.rs b/crates/batten/src/git.rs index 7f636255e..749ea8448 100644 --- a/crates/batten/src/git.rs +++ b/crates/batten/src/git.rs @@ -2572,6 +2572,22 @@ fn without_comments(path: &str, text: &str) -> String { Some("md") => return String::new(), Some("rs") => "//", Some("sh" | "bash" | "bats") => "#", + // THE DECLARATION LANGUAGES — policy modules, config, workflow and + // pipeline documents. Absent from CLOUD-827's original table, which + // named only the languages a consumer writes PROGRAMS in. A declarative + // consumer keeps most of its prose here instead, and every one of these + // took the fall-through below, so a comment-only change to one read as a + // code change and the whole prose-only class could not fire over them. + // + // The fall-through's direction is still right for a genuinely unknown + // extension — it admits rather than blocks, and only blocking is + // unrecoverable by waiting. What was wrong is calling these unknown. + // + // `pkl` is `//` and NOT `#`: it is Java-family, and `#` is not a comment + // marker there at all. One table for everything `#`-ish is how a + // remainder silently keeps its real code. + Some("rego" | "toml" | "yml" | "yaml") => "#", + Some("pkl") => "//", // `mise-tasks/` programs carry no extension (CLOUD-865 renamed most to // `.sh`, but the pattern stays so a re-added extensionless task is still // read). The check is on the DIRECTORY, so it cannot claim a file @@ -4091,6 +4107,57 @@ mod tests { use super::*; + /// The declaration languages have their comments stripped, so a rule over + /// the remainder can see a comment-only change to one of them. + /// + /// This is the case CLOUD-827's table was missing: `.rego`, `.toml`, + /// `.yml`/`.yaml` and `.pkl` all took the unrecognised-extension + /// fall-through, whose remainder is the whole file, so every prose change to + /// a policy module, a config document or a workflow read as a code change. + /// + /// Fails by: removing an arm from `without_comments`'s table. Each assertion + /// below reddens for exactly the extension whose arm went, which is what + /// stops one arm's presence standing in for the others'. + #[test] + fn a_declaration_language_has_its_comments_stripped() { + for path in ["a/x.rego", "x.toml", "a/x.yml", "a/x.yaml"] { + assert_eq!( + without_comments(path, "# a comment\nreal = 1\n"), + "real = 1", + "`#` is the comment marker for {path}, so a comment-only change \ + must leave the remainder identical" + ); + } + // pkl is Java-family: `//`, and `#` is not a comment marker there at all. + // Reading it with the `#` table would keep a commented line in the + // remainder and drop a real one. + assert_eq!( + without_comments("x.pkl", "// a comment\nreal = 1\n"), + "real = 1", + "pkl comments are `//`, so the `#` table would misread every line" + ); + } + + /// And a genuinely unknown extension still takes the admitting direction. + /// + /// The paired half of the case above: widening the table must not turn the + /// fall-through into a guess. An extension nobody has classified has no + /// comments, so its whole content is the remainder and any change to it is a + /// code change — which ADMITS the branch rather than blocking it, and only + /// blocking is unrecoverable by waiting. + /// + /// Fails by: making the fall-through strip `#`, which would read a `.conf` + /// or `.ini` prose change as prose-only on a guess. + #[test] + fn an_unclassified_extension_still_reads_as_code() { + assert_eq!( + without_comments("thing.conf", "# a comment\nreal = 1\n"), + "# a comment\nreal = 1\n", + "an unrecognised extension has no comments, so the remainder is the \ + whole file and the gate admits rather than blocks" + ); + } + /// THE WINDOWS REGRESSION, TESTED AS A DECISION RATHER THAN A CONDITION. /// /// The failing condition — `canonicalize` returning a verbatim path — is one diff --git a/crates/batten/tests/it/prose_only.rs b/crates/batten/tests/it/prose_only.rs index a3ebc1aa5..e0dfbf665 100644 --- a/crates/batten/tests/it/prose_only.rs +++ b/crates/batten/tests/it/prose_only.rs @@ -370,6 +370,72 @@ fn an_unresolvable_base_says_nothing_rather_than_refusing() { admitted(&root); } +/// A comment-only change to a DECLARATION language is refused. +/// +/// The case this suite was missing for its whole life, and the reason it went +/// unnoticed is worth the paragraph. Every arm above is a `.rs`, a `.md`, a +/// `.bats` or an extensionless shell program — the languages a consumer writes +/// PROGRAMS in, which is the set the shell predecessor classified. This +/// repository writes most of its prose somewhere else entirely: `.rego` policy +/// modules, the config authority, `hk.pkl`, and the workflows. All four took +/// `without_comments`'s unrecognised-extension arm, whose remainder is the WHOLE +/// file, so a comment-only change to any of them read as a code change and this +/// gate could not fire over it. +/// +/// Measured 2026-09-04: a 121→71-comment trim of a policy module, no other +/// change in the branch, `batten check --rule prose-only` exit 0, a full +/// required matrix spent. That is exactly the instance CLOUD-827 exists to +/// price, and it walked past because the classifier had never been told what a +/// `.rego` comment looks like. +/// +/// One case per extension rather than one representative, because the arms are +/// independent table entries and `.pkl` in particular is `//` where the other +/// three are `#` — a single case would let one arm's presence stand in for the +/// rest. +/// +/// Fails by: removing the matching arm from `without_comments`'s table, which +/// returns each file's whole text as its remainder and admits the branch. +#[test] +fn a_comment_only_change_to_a_declaration_language_is_refused() { + for (path, before, after) in [ + ( + "policy/a.rego", + "# a note\nviolation contains 1 if { true }\n", + "# a different note\nviolation contains 1 if { true }\n", + ), + ( + "conf.toml", + "# a note\nversion = 1\n", + "# a different note\nversion = 1\n", + ), + ( + ".github/workflows/w.yml", + "# a note\non: push\n", + "# a different note\non: push\n", + ), + ( + "gate.pkl", + "// a note\nx = 1\n", + "// a different note\nx = 1\n", + ), + ] { + let stem = path.rsplit('/').next().unwrap_or(path); + let root = repo( + &format!("declaration-{stem}"), + &[(path, before)], + &Head { + written: &[(path, after)], + removed: &[], + }, + ); + assert_eq!( + findings(&root), + vec!["prose-only".to_owned()], + "a comment-only change to {path} must be priced as prose-only" + ); + } +} + // subsumed: "a Rust block comment is NOT read as prose" crates/batten/tests/it/prose_only.rs #[test] fn a_shell_program_carrying_no_extension_is_read_as_shell() { From 1105c216360eed74781ea7a5748d8765e2ac5ee8 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 4 Sep 2026 19:42:27 +0000 Subject: [PATCH 2/2] fix(git): merge the comment-marker arms clippy refuses to see twice `clippy::match_same_arms` is `-D warnings` here, so `Some("pkl") => "//"` beside `Some("rs") => "//"` does not compile, and neither does the declaration-language arm beside the shell family's. Two arms now, one per marker. The grouping a reader wants is not the grouping the lint permits, so it moves into the comments: `.pkl` sits with Rust because it is Java-family and `#` is not a comment marker there at all, which is the one thing about this table worth getting wrong slowly. Both test doc comments said "removing an arm" where the removable unit is now a token, and the per-extension cases matter more for it: seven extensions share two arms, so a case per extension is the only granularity the table itself no longer provides. Refs: CLOUD-827 --- crates/batten/src/git.rs | 40 ++++++++++++++++------------ crates/batten/tests/it/prose_only.rs | 5 ++-- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/crates/batten/src/git.rs b/crates/batten/src/git.rs index 749ea8448..742b7a93f 100644 --- a/crates/batten/src/git.rs +++ b/crates/batten/src/git.rs @@ -2570,24 +2570,28 @@ fn without_comments(path: &str, text: &str) -> String { let prefix = match path.rsplit_once('.').map(|(_, ext)| ext) { // Markdown is prose end to end; there is no non-comment remainder. Some("md") => return String::new(), - Some("rs") => "//", - Some("sh" | "bash" | "bats") => "#", - // THE DECLARATION LANGUAGES — policy modules, config, workflow and - // pipeline documents. Absent from CLOUD-827's original table, which - // named only the languages a consumer writes PROGRAMS in. A declarative - // consumer keeps most of its prose here instead, and every one of these - // took the fall-through below, so a comment-only change to one read as a - // code change and the whole prose-only class could not fire over them. + // `pkl` SITS WITH RUST RATHER THAN WITH THE OTHER DECLARATION + // LANGUAGES: it is Java-family, and `#` is not a comment marker there at + // all. One table for everything that merely looks `#`-ish is how a + // remainder silently keeps its real code and drops a commented line. + // + // The two `//` and the four `#` groups are one arm each because + // `clippy::match_same_arms` refuses a table that repeats a body, so the + // grouping a reader wants lives in these comments rather than in the + // arms. + Some("rs" | "pkl") => "//", + // The shell family, and THE DECLARATION LANGUAGES beside it — policy + // modules, config, pipeline and workflow documents. Only the shell half + // was in CLOUD-827's original table, which named the languages a + // consumer writes PROGRAMS in. A declarative consumer keeps most of its + // prose in the other half, and every one of those took the fall-through + // below, so a comment-only change to one read as a code change and the + // whole prose-only class could not fire over them. // // The fall-through's direction is still right for a genuinely unknown // extension — it admits rather than blocks, and only blocking is // unrecoverable by waiting. What was wrong is calling these unknown. - // - // `pkl` is `//` and NOT `#`: it is Java-family, and `#` is not a comment - // marker there at all. One table for everything `#`-ish is how a - // remainder silently keeps its real code. - Some("rego" | "toml" | "yml" | "yaml") => "#", - Some("pkl") => "//", + Some("sh" | "bash" | "bats" | "rego" | "toml" | "yml" | "yaml") => "#", // `mise-tasks/` programs carry no extension (CLOUD-865 renamed most to // `.sh`, but the pattern stays so a re-added extensionless task is still // read). The check is on the DIRECTORY, so it cannot claim a file @@ -4115,9 +4119,11 @@ mod tests { /// fall-through, whose remainder is the whole file, so every prose change to /// a policy module, a config document or a workflow read as a code change. /// - /// Fails by: removing an arm from `without_comments`'s table. Each assertion - /// below reddens for exactly the extension whose arm went, which is what - /// stops one arm's presence standing in for the others'. + /// Fails by: removing an extension token from `without_comments`'s table. + /// Each assertion below reddens for exactly the token that went, which is + /// what stops one extension's presence standing in for its neighbours' — + /// they share two arms, so a per-extension case is the only granularity the + /// table itself does not provide. #[test] fn a_declaration_language_has_its_comments_stripped() { for path in ["a/x.rego", "x.toml", "a/x.yml", "a/x.yaml"] { diff --git a/crates/batten/tests/it/prose_only.rs b/crates/batten/tests/it/prose_only.rs index e0dfbf665..d1d1dac39 100644 --- a/crates/batten/tests/it/prose_only.rs +++ b/crates/batten/tests/it/prose_only.rs @@ -393,8 +393,9 @@ fn an_unresolvable_base_says_nothing_rather_than_refusing() { /// three are `#` — a single case would let one arm's presence stand in for the /// rest. /// -/// Fails by: removing the matching arm from `without_comments`'s table, which -/// returns each file's whole text as its remainder and admits the branch. +/// Fails by: removing the matching extension token from `without_comments`'s +/// table, which returns that file's whole text as its remainder and admits the +/// branch. #[test] fn a_comment_only_change_to_a_declaration_language_is_refused() { for (path, before, after) in [