diff --git a/.claude/skills/ci-gates/SKILL.md b/.claude/skills/ci-gates/SKILL.md index 3329364c73..d681326a5d 100644 --- a/.claude/skills/ci-gates/SKILL.md +++ b/.claude/skills/ci-gates/SKILL.md @@ -6347,3 +6347,53 @@ read by exactly the person who does not yet know the answer, which is the worst audience for a stale number. Grep your own help strings for digits whenever the thing they describe grows. + +## 202. Three spellings of a declaration, then three of a field + +The type scanner learned that a type is declared three ways — `struct X {`, +`pub struct X {`, `const X = struct {`. One iteration later, an agent checking +*coverage* found that a FIELD is also written two ways, and the second was being +thrown away: + + pub struct HealthStatus { + pub is_healthy: bool, <- name reads as "pub is_healthy", rejected + ... + } + +Five fields dropped, and the struct then read as empty. **An empty field list +compares equal to any other empty field list**, so a five-field type and an +unrelated placeholder of the same name were reported as *the same fields written +twice*. + +The lesson repeats one level down and is worth stating both times: **when a +scanner recognises a construct, enumerate the ways that construct is spelled — +and then do it again for the constructs nested inside it.** + +## 203. The riskiest sample, not the easiest + +The coverage verifier's set comparison came back exact — nothing missing, nothing +spurious. It could have stopped there and reported `sound: true`. + +Instead it hand-verified nine conflicts, *chosen as the riskiest rather than the +easiest*: the five whose conflict rests on a side the reader could not parse, and +the two same-file pairs. `HealthStatus` came out of exactly that choice — a name +in the wrong bucket, invisible to a set comparison because the set was right and +the *classification* was wrong. + +**A verifier that samples the easy cases confirms the tool's happy path.** Ask +for the rows where the tool had least to work with. + +## 204. A ratchet one day old, catching a real change + +`tri types ratchet` was written in one iteration and fired in the next, on a real +fix with nothing planted: + + ledger 79 name(s), observed 80 + + HealthStatus NEW conflict + +Identity-keyed, so it would also have caught a swap at a constant count — one +name resolved while another appears, which is the case a count cannot see and +which was tested on purpose before the real one arrived. + +**Write the ratchet before the work it will police, not after.** The one that +already exists is the one that reports the change you did not predict. diff --git a/cli/tri/src/types_dup.rs b/cli/tri/src/types_dup.rs index a8847d267b..34aa0664ab 100644 --- a/cli/tri/src/types_dup.rs +++ b/cli/tri/src/types_dup.rs @@ -271,7 +271,15 @@ pub fn defs_in(file: &str, src: &str) -> Vec<(String, Def)> { } if !l.starts_with("//") { if let Some((n, ty)) = l.split_once(':') { - let n = n.trim(); + // W707: a field may be `pub name: T`. Splitting on `:` then + // rejecting a name containing a space threw every such field + // away, so `pub struct HealthStatus { pub is_healthy: bool, + // ... }` parsed as a struct with NO fields -- and compared + // equal to an unrelated empty definition of the same name, + // which the detector then called DUPLICATED instead of + // CONFLICTED. Found by an agent asked to check coverage, + // not by me. + let n = n.trim().strip_prefix("pub ").unwrap_or(n.trim()).trim(); let ty = ty.trim().trim_end_matches(',').trim(); // A field is `name: Type`. Anything with a space in the name // is a line this does not understand, and is skipped rather @@ -540,6 +548,28 @@ mod tests { assert!(new.is_empty() && gone.is_empty()); } + /// A `pub` field is a field. Dropping them made a five-field struct read as + /// empty, and an empty struct compares equal to any other empty one. + #[test] + fn a_pub_field_is_read() { + let d = parse("pub struct S {\n pub is_healthy: bool,\n pub code: u16,\n}\n"); + assert_eq!( + d[0].1.fields, + vec![ + ("is_healthy".to_string(), "bool".to_string()), + ("code".to_string(), "u16".to_string()) + ] + ); + } + + /// And the consequence: five fields versus none is a CONFLICT, not a match. + #[test] + fn a_populated_struct_conflicts_with_an_empty_one_of_the_same_name() { + let a = parse("pub struct S {\n pub a: bool,\n}\n")[0].1.clone(); + let b = parse("pub const S = struct {\n};\n")[0].1.clone(); + assert_eq!(verdict(&[a, b]), "CONFLICTED"); + } + #[test] fn a_struct_with_no_parseable_fields_is_still_a_definition() { let d = parse("struct Empty {\n}\n"); diff --git a/docs/now/2026-08-29-a-pub-field-is-a-field-and-the-ratchet-caught-the-consequenc.md b/docs/now/2026-08-29-a-pub-field-is-a-field-and-the-ratchet-caught-the-consequenc.md new file mode 100644 index 0000000000..25fe25a784 --- /dev/null +++ b/docs/now/2026-08-29-a-pub-field-is-a-field-and-the-ratchet-caught-the-consequenc.md @@ -0,0 +1,8 @@ +# NOW -- A pub field is a field, and the ratchet caught the consequence (2026-08-29) + +## A pub field is a field, and the ratchet caught the consequence (Refs #2774) + +- pub struct HealthStatus { pub is_healthy: bool, ... } parsed as a struct with NO fields: the field parser split on : and rejected any name with a space +- an empty struct compares equal to any other empty one, so a five-field type and an unrelated placeholder of the same name were called DUPLICATED +- found by an agent asked to check COVERAGE of the classification, not by me; the ratchet then reported + HealthStatus NEW conflict on a real change, one day after being written +- 79 -> 80 conflicted, definitions with unreadable fields 9 -> 6 diff --git a/docs/reports/type_conflicts.json b/docs/reports/type_conflicts.json index ea9256d4e3..26a29504da 100644 --- a/docs/reports/type_conflicts.json +++ b/docs/reports/type_conflicts.json @@ -22,6 +22,7 @@ "FFNConfig", "FileInfo", "Graph", + "HealthStatus", "HttpRequest", "HttpResponse", "HttpStatus",