Skip to main content

rto_graph/
authorship.rs

1//! Who wrote the change under review, read from its commit trailers — so a
2//! reviewer can say when it is reviewing its own output (issue #649, part 3).
3//!
4//! A model reviewing code it wrote is the weakest possible reviewer: it shares
5//! the blind spot that produced the defect. The authorship is already recorded,
6//! because commits made by an agent harness carry a trailer:
7//!
8//! ```text
9//! Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
10//! ```
11//!
12//! This module is the pure half — parse the trailers, decide whether one names
13//! the model that is about to review. Reading a commit range and printing a
14//! warning belong to the binary, where git and the engine already are.
15//!
16//! # Warn, never refuse
17//!
18//! Settled by the owner on 27 Aug 2026, and the reason matters for what is here:
19//! a same-model review is weakened, not worthless, and refusing would trade it
20//! for **no** review on exactly the machine most likely to have one model
21//! installed. So nothing in this module returns an error or a veto — the widest
22//! answer it gives is "these two names denote the same model", and the caller's
23//! only move is to say so.
24//!
25//! A commit with no `Co-Authored-By` — a human wrote it — yields no trailer, so
26//! no match, so no warning, and the review proceeds exactly as it did before.
27//! Human-authored changes are never harder to review than machine-authored ones,
28//! which was the thing to avoid.
29//!
30//! # Does the trailer name the model, or the harness that ran it?
31//!
32//! **The trailer is read as naming a model, and the comparison is model-to-model
33//! only.** That is a decision about what can be *compared*, not a claim about
34//! what harnesses write.
35//!
36//! The registry side is unambiguous: [`crate::ModelTask::Review`] resolves to a
37//! model, never to a harness. So a model-to-harness comparison is a category
38//! error whichever way the trailer happens to be written, and there is no rule
39//! that could rescue it. The trailer side, by contrast, is free text with no
40//! schema — a harness may write its product name (`Claude Code`, `Cursor`,
41//! `Aider`) instead of the weights it ran, and nothing in the string says which
42//! it did. Any rule that *decided* "this one is a harness" would be guessing at
43//! the one thing the format does not record.
44//!
45//! So [`names_same_model`] attempts an identity match against the model name and
46//! gives up silently when it fails. A trailer naming a harness normalises onto no
47//! registry model, does not match, and produces nothing.
48//!
49//! ## Why a mismatch is the benign direction
50//!
51//! The two ways to be wrong are not symmetric.
52//!
53//! A **false negative** — a same-model review that goes unwarned — costs exactly
54//! what today costs, because today there is no warning at all. It is a feature
55//! that did not fire, not a regression.
56//!
57//! A **false positive** — warning that the reviewer wrote the change when it
58//! demonstrably did not — is worse than it looks, because the alternative rule
59//! that would "never miss" is *warn whenever any AI trailer is present*. On this
60//! repository, whose commits are Claude-authored and whose local reviewer is a
61//! Qwen GGUF, that rule fires on every single run while being false every single
62//! time. A warning that is always on is a warning nobody reads, and it would take
63//! the true ones down with it.
64//!
65//! Hence exact identity or silence. [`identity_tokens`] absorbs the spelling
66//! differences that are certainly not identity differences — case, the separators
67//! between a family and its size, and a bracketed qualifier like `(1M context)`,
68//! which describes how a model was *configured* rather than which model it is —
69//! and nothing else.
70
71/// One `Co-Authored-By` trailer.
72#[derive(Debug, Clone, PartialEq, Eq)]
73pub struct CoAuthor {
74    /// The display name, exactly as the trailer wrote it — `Claude Opus 5 (1M
75    /// context)`. Kept verbatim so a warning can quote what it actually read
76    /// rather than a normalised form the reader would not find in `git log`.
77    pub name: String,
78    /// The address inside the angle brackets, or the empty string when the
79    /// trailer carried none.
80    pub email: String,
81}
82
83/// Every `Co-Authored-By` trailer in one commit message, in the order it appears.
84///
85/// Deliberately not a full RFC-822-ish trailer parser: this reads the one key it
86/// needs, case-insensitively, from any line of the message. Git's own trailer
87/// rules require the block to be at the end and unbroken, and a message that
88/// slightly violates them still records who wrote the code — which is the fact
89/// wanted here, not a syntactic verdict about the commit.
90///
91/// Duplicates are kept. Two identical trailers are a fact about the message, and
92/// the caller de-duplicates across a range where that is what it wants.
93#[must_use]
94pub fn co_authors(message: &str) -> Vec<CoAuthor> {
95    const KEY: &str = "co-authored-by:";
96    let mut out = Vec::new();
97    for line in message.lines() {
98        let line = line.trim();
99        let Some(head) = line.get(..KEY.len()) else {
100            continue;
101        };
102        if !head.eq_ignore_ascii_case(KEY) {
103            continue;
104        }
105        let value = line[KEY.len()..].trim();
106        // The address is the bracketed tail, if there is one. A trailer with no
107        // `<...>` is still an authorship claim and is kept with an empty email
108        // rather than dropped — the name is the half this module compares.
109        let (name, email) = match value.rfind('<') {
110            Some(at) => {
111                let email = value[at + 1..].trim_end().trim_end_matches('>');
112                (value[..at].trim(), email.trim())
113            }
114            None => (value, ""),
115        };
116        if name.is_empty() && email.is_empty() {
117            continue;
118        }
119        out.push(CoAuthor {
120            name: name.to_owned(),
121            email: email.to_owned(),
122        });
123    }
124    out
125}
126
127/// Normalise a model or trailer name to the tokens that identify it.
128///
129/// Lowercased and split on **every non-alphanumeric character**, so
130/// `claude-opus-5`, `claude_opus_5`, `claude.opus.5` and `Claude Opus 5` all
131/// reach the same answer; and with any bracketed span dropped — `(…)` **and**
132/// `[…]` alike, since a qualifier is written both ways. `(1M context)` says how a
133/// model was *configured*, not which model it is, and two runs of one model at
134/// two context sizes are the same weights with the same blind spot.
135///
136/// Nothing else is removed, and nothing is added. This absorbs spelling, never
137/// meaning — see the module docs for why the rule stops here.
138#[must_use]
139pub fn identity_tokens(name: &str) -> Vec<String> {
140    let mut out = Vec::new();
141    let mut current = String::new();
142    let mut depth = 0usize;
143    for ch in name.chars() {
144        match ch {
145            '(' | '[' => {
146                depth += 1;
147                continue;
148            }
149            ')' | ']' => {
150                depth = depth.saturating_sub(1);
151                continue;
152            }
153            _ => {}
154        }
155        if depth > 0 {
156            continue;
157        }
158        if ch.is_alphanumeric() {
159            current.extend(ch.to_lowercase());
160        } else if !current.is_empty() {
161            out.push(std::mem::take(&mut current));
162        }
163    }
164    if !current.is_empty() {
165        out.push(current);
166    }
167    out
168}
169
170/// Whether a trailer's `name` denotes the same model as `model`.
171///
172/// Exact equality of the [`identity_tokens`] sequence, and nothing looser. A
173/// trailer that names a harness, a person, or a different model returns `false`,
174/// which the caller renders as silence — see the module docs for why that is the
175/// direction to fail in.
176#[must_use]
177pub fn names_same_model(name: &str, model: &str) -> bool {
178    let left = identity_tokens(name);
179    // An empty token list matches nothing, including another empty one: two names
180    // that normalise to nothing are not evidence that they are the same model.
181    !left.is_empty() && left == identity_tokens(model)
182}
183
184/// How much of a commit range the reviewing model wrote — see
185/// [`reviewers_own_work`].
186#[derive(Debug, Clone, Default, PartialEq, Eq)]
187pub struct OwnWork {
188    /// How many of the messages carry at least one matching trailer.
189    ///
190    /// Counted separately from [`OwnWork::names`] because the two answer
191    /// different questions and are easy to conflate into a sentence that is
192    /// quietly false: one model spelled two ways across nine commits is two names
193    /// and nine commits, and a warning that says "2 commits" would be wrong about
194    /// the only number a reader would act on.
195    pub commits: usize,
196    /// The matching trailer names as written, de-duplicated, first-seen order —
197    /// so a warning quotes what `git log` would show rather than a normalised
198    /// form the reader could not search for.
199    pub names: Vec<String>,
200}
201
202impl OwnWork {
203    /// Whether the reviewing model wrote any of the range.
204    #[must_use]
205    pub fn is_empty(&self) -> bool {
206        self.commits == 0
207    }
208}
209
210/// Which commits in a range `model` co-authored, by its trailers.
211///
212/// A default [`OwnWork`] means no commit in the range was written by this model,
213/// which is the ordinary case and is never an error: see the module docs for why
214/// silence is the direction this fails in.
215#[must_use]
216pub fn reviewers_own_work(messages: &[String], model: &str) -> OwnWork {
217    let mut out = OwnWork::default();
218    for message in messages {
219        let mut matched = false;
220        for author in co_authors(message) {
221            if !names_same_model(&author.name, model) {
222                continue;
223            }
224            matched = true;
225            if !out.names.contains(&author.name) {
226                out.names.push(author.name);
227            }
228        }
229        out.commits += usize::from(matched);
230    }
231    out
232}
233
234#[cfg(test)]
235mod tests {
236    use super::{co_authors, identity_tokens, names_same_model, reviewers_own_work};
237
238    #[test]
239    fn a_trailer_is_read_from_a_real_commit_message() {
240        let message = "feat(review): do the thing\n\
241                       \n\
242                       A body paragraph.\n\
243                       \n\
244                       Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>\n\
245                       Claude-Session: https://example.invalid/s\n";
246        let authors = co_authors(message);
247        assert_eq!(authors.len(), 1);
248        assert_eq!(authors[0].name, "Claude Opus 5 (1M context)");
249        assert_eq!(authors[0].email, "noreply@anthropic.com");
250    }
251
252    #[test]
253    fn the_key_is_matched_case_insensitively_and_a_missing_address_is_kept() {
254        let authors = co_authors("x\n\nco-authored-by: qwen3-8b\nCO-AUTHORED-BY: A B <a@b>\n");
255        assert_eq!(authors.len(), 2);
256        assert_eq!(authors[0].name, "qwen3-8b");
257        assert_eq!(
258            authors[0].email, "",
259            "a trailer with no address is still an authorship claim"
260        );
261        assert_eq!(authors[1].email, "a@b");
262    }
263
264    #[test]
265    fn a_human_commit_carries_no_trailer_and_therefore_no_match() {
266        // The case that must never become a reason to refuse: a human-authored
267        // commit yields nothing, so nothing is warned about and the review
268        // proceeds exactly as it did before.
269        let message = "fix: correct the off-by-one\n\nNoticed while reading.\n";
270        assert!(co_authors(message).is_empty());
271        assert!(
272            reviewers_own_work(&[message.to_owned()], "qwen3-8b").is_empty(),
273            "a human-authored commit must never be harder to review"
274        );
275    }
276
277    #[test]
278    fn spelling_differences_that_are_not_identity_differences_are_absorbed() {
279        assert_eq!(
280            identity_tokens("Claude Opus 5 (1M context)"),
281            ["claude", "opus", "5"]
282        );
283        assert_eq!(identity_tokens("claude-opus-5"), ["claude", "opus", "5"]);
284        assert_eq!(identity_tokens("qwen3.8-27b"), ["qwen3", "8", "27b"]);
285        assert!(names_same_model(
286            "Claude Opus 5 (1M context)",
287            "claude-opus-5"
288        ));
289        assert!(
290            names_same_model("QWEN3_8B", "qwen3-8b"),
291            "case and separator are spelling, not identity"
292        );
293    }
294
295    /// The context qualifier is dropped **because it is a configuration, not an
296    /// identity**: the same weights at two window sizes carry the same blind spot,
297    /// which is the whole reason the warning exists.
298    ///
299    /// Both bracket forms, and every separator — the doc comment claims all of
300    /// this, and a claim about normalisation that nothing checks is the
301    /// contract-drift class this reviewer exists to find. Caught as a suppressed
302    /// finding in review of #649, where the docs said "parenthesised" and
303    /// "`-`, `_`, `.` and `/`" while the code took `[…]` and every
304    /// non-alphanumeric character.
305    #[test]
306    fn a_bracketed_qualifier_does_not_make_it_a_different_model() {
307        assert!(names_same_model(
308            "Claude Opus 5 (1M context)",
309            "Claude Opus 5"
310        ));
311        assert!(names_same_model(
312            "Claude Opus 5 (200K context)",
313            "claude opus 5"
314        ));
315        assert!(
316            names_same_model("Claude Opus 5 [1M context]", "claude-opus-5"),
317            "square brackets are dropped too, exactly as the doc comment says"
318        );
319        assert_eq!(
320            identity_tokens("Claude Opus 5 [1M context]"),
321            ["claude", "opus", "5"]
322        );
323        // Every non-alphanumeric character separates, not only the four the doc
324        // comment used to list.
325        for spelling in [
326            "claude-opus-5",
327            "claude_opus_5",
328            "claude.opus.5",
329            "claude/opus/5",
330            "claude:opus:5",
331            "claude+opus+5",
332        ] {
333            assert_eq!(
334                identity_tokens(spelling),
335                ["claude", "opus", "5"],
336                "{spelling} must normalise like every other spelling"
337            );
338        }
339    }
340
341    /// A harness name normalises onto no model name, so it does not match, so
342    /// nothing is printed. This is the mismatch case being benign by
343    /// construction rather than by intention.
344    #[test]
345    fn a_harness_name_matches_no_model_and_is_therefore_silent() {
346        for harness in ["Claude Code", "Cursor", "Aider", "GitHub Copilot"] {
347            assert!(
348                !names_same_model(harness, "claude-opus-5"),
349                "{harness} names a harness, and a harness is not a model"
350            );
351            assert!(!names_same_model(harness, "qwen3-8b"));
352        }
353    }
354
355    /// The realistic negative on this repository, asserted so the feature's
356    /// *silence* here is a measured fact rather than an assumption: commits are
357    /// Claude-authored and the local reviewer is a Qwen GGUF, so nothing fires.
358    #[test]
359    fn a_different_model_is_never_reported_as_the_same_one() {
360        let message = "x\n\nCo-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>\n";
361        assert!(reviewers_own_work(&[message.to_owned()], "qwen3-8b").is_empty());
362        assert!(reviewers_own_work(&[message.to_owned()], "qwen3.8-27b").is_empty());
363    }
364
365    /// The two counts are separate because a sentence that conflates them is
366    /// quietly false: one model spelled two ways over three commits is **two**
367    /// names and **three** commits, and the commit count is the number a reader
368    /// would act on.
369    #[test]
370    fn distinct_names_and_matching_commits_are_counted_separately() {
371        let one = "a\n\nCo-Authored-By: Qwen3 8B <x@y>\n".to_owned();
372        let two = "b\n\nCo-Authored-By: Qwen3 8B <x@y>\n".to_owned();
373        let three = "c\n\nCo-Authored-By: qwen3-8b <x@y>\n".to_owned();
374        let human = "d\n\nnobody else\n".to_owned();
375        let hit = reviewers_own_work(&[one, two, three, human], "qwen3-8b");
376        assert_eq!(
377            hit.names,
378            vec!["Qwen3 8B".to_owned(), "qwen3-8b".to_owned()],
379            "quoted as written, de-duplicated, first-seen order"
380        );
381        assert_eq!(
382            hit.commits, 3,
383            "three commits matched; the fourth is human-authored"
384        );
385    }
386
387    /// One commit naming the model twice is still one commit.
388    #[test]
389    fn a_commit_is_counted_once_however_many_trailers_it_carries() {
390        let message = "a\n\nCo-Authored-By: Qwen3 8B <x@y>\nCo-Authored-By: qwen3-8b <x@y>\n";
391        let hit = reviewers_own_work(&[message.to_owned()], "qwen3-8b");
392        assert_eq!(hit.commits, 1);
393        assert_eq!(hit.names.len(), 2);
394    }
395
396    #[test]
397    fn a_name_that_normalises_to_nothing_matches_nothing() {
398        assert!(
399            !names_same_model("", ""),
400            "two names that say nothing are not evidence of the same model"
401        );
402        assert!(!names_same_model("(1M context)", ""));
403    }
404}