Fixed binary-file crash, unified intent semantics, and flagged empty changes

.gitignore

@@ -4,3 +4,4 @@
 *.env
 !fixtures/repo/head/secrets/.env
 *.DS_Store
+.useit/

TODO.md

@@ -8,4 +8,13 @@ every change. Fixed by aligning the code with its own documented contract:
 only paths *touched* by a change (added, removed, or content-modified vs
 base) are checked. See `test_visibility_policy_only_flags_touched_private_paths`.
 
-No open items.
+## ~~Binary change detection is lossy~~ RESOLVED 2026-08-22
+
+`Snapshot.files` now stores raw bytes (`HashMap<String, Vec<u8>>`). Change
+detection compares bytes exactly, so two distinct binaries never compare
+equal even when their lossy text collapses to the same U+FFFD sequence.
+Text conversion happens only in the structural engine at parse time
+(`as_text`, src/engine/mod.rs). All three ingestion paths (dir, git, jj)
+store unconverted bytes. Pinned by
+`test_cli_distinct_binaries_are_not_collapsed`.
+

src/adapter/git.rs

@@ -1,7 +1,7 @@
 //! Native Git adapter for extracting in-memory snapshots and adjudicating 3-way merges.
 
 use crate::change::{Change, Snapshot, Source};
-use crate::dispute::{Docket, Severity, Verdict};
+use crate::dispute::{finalize_adjudication, Docket, Severity};
 use crate::engine::Engine;
 use crate::policy::MeaningPolicy;
 use crate::visibility::VisibilityPolicy;
@@ -157,10 +157,13 @@ impl GitAdapter {
                         .output()
                         .with_context(|| format!("Failed to fetch blob {blob_sha} for {path}"))?;
 
-                    if blob_output.status.success() {
-                        let content = String::from_utf8_lossy(&blob_output.stdout).into_owned();
-                        files.insert(path.to_string(), content);
+                    if !blob_output.status.success() {
+                        return Err(anyhow!(
+                            "Failed to read blob {blob_sha} for {path}: {}",
+                            String::from_utf8_lossy(&blob_output.stderr).trim()
+                        ));
                     }
+                    files.insert(path.to_string(), blob_output.stdout);
                 }
             }
         }
@@ -222,37 +225,23 @@ impl GitAdapter {
             .any(|d| d.kind == crate::dispute::Kind::Visibility && d.severity == Severity::High);
         disputes.extend(vis_disputes);
 
-        let verdict = if cloaked {
-            Verdict::Cloaked
-        } else if visibility_policy.embargo_until.is_some() {
-            Verdict::Embargoed
-        } else {
-            meaning_policy.evaluate(&disputes)
-        };
-
-        let mut touched_paths: Vec<String> = base_snapshot
-            .files
-            .keys()
-            .chain(head_snapshot.files.keys())
-            .filter(|p| base_snapshot.files.get(*p) != head_snapshot.files.get(*p))
-            .cloned()
-            .collect();
-        touched_paths.sort();
-        touched_paths.dedup();
-
-        let scope = if touched_paths.is_empty() {
-            "no files changed".to_string()
-        } else {
-            touched_paths.join(", ")
-        };
+        let (disputes, intent, verdict) = finalize_adjudication(
+            disputes,
+            &base_snapshot.files,
+            &head_snapshot.files,
+            options.intent.clone(),
+            cloaked,
+            visibility_policy.embargo_until.is_some(),
+            meaning_policy,
+        );
 
         let docket = Docket {
             change: change.name,
             source: format!("git: {merge_base_sha:.7} (base) vs {head_sha:.7} (head)"),
             base: change.base_ref,
             head: change.head_ref,
             disputes,
-            scope,
+            intent,
             authors,
             verdict,
             embargo: visibility_policy.embargo_note(),

src/adapter/jj.rs

@@ -6,7 +6,7 @@
 //! caller's working copy.
 
 use crate::change::{Change, Snapshot, Source};
-use crate::dispute::{Dispute, Docket, Kind, Severity, Verdict};
+use crate::dispute::{finalize_adjudication, Dispute, Docket, Kind, Severity};
 use crate::engine::Engine;
 use crate::policy::MeaningPolicy;
 use crate::visibility::VisibilityPolicy;
@@ -78,6 +78,13 @@ impl JjAdapter {
 
     /// Run a read-only jj command and return its stdout.
     fn run(&self, args: &[&str]) -> Result<String> {
+        Ok(String::from_utf8_lossy(&self.run_bytes(args)?).into_owned())
+    }
+
+    /// Run a read-only jj command and return its raw stdout bytes.
+    ///
+    /// Used for file content so binary files keep exact bytes.
+    fn run_bytes(&self, args: &[&str]) -> Result<Vec<u8>> {
         let mut full: Vec<&str> = vec!["--ignore-working-copy", "--no-pager", "--quiet"];
         full.extend_from_slice(args);
 
@@ -89,13 +96,13 @@ impl JjAdapter {
 
         if !output.status.success() {
             return Err(anyhow!(
-                "jj {:?} failed: {}",
-                args,
+                "jj {} failed: {}",
+                args.join(" "),
                 String::from_utf8_lossy(&output.stderr).trim()
             ));
         }
 
-        Ok(String::from_utf8_lossy(&output.stdout).into_owned())
+        Ok(output.stdout)
     }
 
     /// Resolve a revset to exactly one commit ID.
@@ -204,8 +211,9 @@ impl JjAdapter {
         let mut conflicted = Vec::new();
 
         for path in listing.lines().map(str::trim).filter(|l| !l.is_empty()) {
-            let content = self.run(&["file", "show", "-r", rev, "--", path])?;
-            if content
+            let content = self.run_bytes(&["file", "show", "-r", rev, "--", path])?;
+            let text = String::from_utf8_lossy(&content);
+            if text
                 .lines()
                 .any(|l| l.starts_with("<<<<<<<") && l.contains("conflict"))
             {
@@ -294,29 +302,15 @@ impl JjAdapter {
             .any(|d| d.kind == Kind::Visibility && d.severity == Severity::High);
         disputes.extend(vis_disputes);
 
-        let verdict = if cloaked {
-            Verdict::Cloaked
-        } else if visibility_policy.embargo_until.is_some() {
-            Verdict::Embargoed
-        } else {
-            meaning_policy.evaluate(&disputes)
-        };
-
-        let mut touched_paths: Vec<String> = base_snapshot
-            .files
-            .keys()
-            .chain(head_snapshot.files.keys())
-            .filter(|p| base_snapshot.files.get(*p) != head_snapshot.files.get(*p))
-            .cloned()
-            .collect();
-        touched_paths.sort();
-        touched_paths.dedup();
-
-        let scope = if touched_paths.is_empty() {
-            "no files changed".to_string()
-        } else {
-            touched_paths.join(", ")
-        };
+        let (disputes, intent, verdict) = finalize_adjudication(
+            disputes,
+            &base_snapshot.files,
+            &head_snapshot.files,
+            options.intent.clone(),
+            cloaked,
+            visibility_policy.embargo_until.is_some(),
+            meaning_policy,
+        );
 
         let docket = Docket {
             change: change.name,
@@ -328,7 +322,7 @@ impl JjAdapter {
             base: change.base_ref,
             head: change.head_ref,
             disputes,
-            scope,
+            intent,
             authors,
             verdict,
             embargo: visibility_policy.embargo_note(),

src/change.rs

@@ -43,12 +43,14 @@ impl std::str::FromStr for Source {
 
 /// A snapshot is a mapping of relative file paths to their contents.
 ///
+/// Contents are stored as raw bytes so binary files compare exactly;
+/// text conversion happens only when the structural engine parses a file.
 /// Oot never assumes these files exist on a physical filesystem;
 /// they can be ingested from git, Jujutsu, or an agent's memory isolate.
 #[derive(Debug, Clone, Default)]
 pub struct Snapshot {
-    /// Map of file path (relative to repo root) to UTF-8 file content.
-    pub files: HashMap<String, String>,
+    /// Map of file path (relative to repo root) to raw file content.
+    pub files: HashMap<String, Vec<u8>>,
 }
 
 /// A Change is the core unit Oot adjudicates: a content-addressed delta
@@ -95,7 +97,7 @@ mod tests {
     fn test_change_and_snapshot_creation() {
         let mut snap = Snapshot::default();
         snap.files
-            .insert("src/lib.rs".into(), "pub fn test() {}".into());
+            .insert("src/lib.rs".into(), "pub fn test() {}".as_bytes().to_vec());
 
         let change = Change {
             name: "test-change".into(),

src/dispute.rs

@@ -2,7 +2,7 @@
 //!
 //! A [`Dispute`] represents a point of disagreement (either structural meaning
 //! or visibility violation). A [`Docket`] is the complete, rendered adjudication
-//! record containing disputes, verdict, scope, and embargo metadata.
+//! record containing disputes, verdict, intent, and embargo metadata.
 
 use serde::{Deserialize, Serialize};
 
@@ -54,6 +54,78 @@ pub struct Dispute {
     pub detail: String,
 }
 
+/// Sentinel id for the empty-change notice.
+///
+/// The notice is informational only; policy evaluation and meaning counts
+/// skip it by id so a persisted docket can never re-block on it.
+pub const EMPTY_CHANGE_ID: &str = "D000";
+
+impl Dispute {
+    /// Low-severity notice that a change contains no file differences.
+    ///
+    /// Purely informational: [`finalize_adjudication`] evaluates the verdict
+    /// before appending this notice, and [`MeaningPolicy::evaluate`] plus
+    /// [`Docket::meaning_count`] skip the sentinel, so it never reaches
+    /// blocking or review thresholds regardless of the meaning policy.
+    pub fn empty_change() -> Dispute {
+        Dispute {
+            id: EMPTY_CHANGE_ID.into(),
+            location: "-".into(),
+            kind: Kind::Meaning,
+            severity: Severity::Low,
+            detail: "no file differences between base and head".into(),
+        }
+    }
+}
+
+/// Shared adjudication tail used by every entry point (dir snapshots, git, jj).
+///
+/// Computes touched paths, evaluates the verdict, appends the empty-change
+/// notice, and resolves the docket intent. The verdict is evaluated *before*
+/// the notice is appended so the notice cannot influence thresholds.
+///
+/// Returns `(disputes, intent, verdict)`.
+pub fn finalize_adjudication(
+    mut disputes: Vec<Dispute>,
+    base: &std::collections::HashMap<String, Vec<u8>>,
+    head: &std::collections::HashMap<String, Vec<u8>>,
+    user_intent: Option<String>,
+    cloaked: bool,
+    embargo_active: bool,
+    meaning_policy: &crate::policy::MeaningPolicy,
+) -> (Vec<Dispute>, String, Verdict) {
+    let mut touched_paths: Vec<String> = base
+        .keys()
+        .chain(head.keys())
+        .filter(|p| base.get(*p) != head.get(*p))
+        .cloned()
+        .collect();
+    touched_paths.sort();
+    touched_paths.dedup();
+
+    let verdict = if cloaked {
+        Verdict::Cloaked
+    } else if embargo_active {
+        Verdict::Embargoed
+    } else {
+        meaning_policy.evaluate(&disputes)
+    };
+
+    if touched_paths.is_empty() {
+        disputes.push(Dispute::empty_change());
+    }
+
+    let intent = user_intent.unwrap_or_else(|| {
+        if touched_paths.is_empty() {
+            "no files changed".to_string()
+        } else {
+            touched_paths.join(", ")
+        }
+    });
+
+    (disputes, intent, verdict)
+}
+
 /// The final adjudication verdict for a change.
 #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
 #[serde(rename_all = "lowercase")]
@@ -81,8 +153,9 @@ pub struct Docket {
     pub head: String,
     /// Collection of detected disputes.
     pub disputes: Vec<Dispute>,
-    /// Stated scope or intent of the change.
-    pub scope: String,
+    /// Stated intent of the change, or a summary of touched paths when none was given.
+    #[serde(alias = "scope")]
+    pub intent: String,
     /// Change author handles or agent identifiers.
     pub authors: Vec<String>,
     /// Resulting adjudication verdict.
@@ -93,10 +166,13 @@ pub struct Docket {
 
 impl Docket {
     /// Return the number of meaning-related disputes.
+    ///
+    /// Excludes the empty-change notice, which is informational rather than
+    /// a detected dispute.
     pub fn meaning_count(&self) -> usize {
         self.disputes
             .iter()
-            .filter(|d| d.kind == Kind::Meaning)
+            .filter(|d| d.kind == Kind::Meaning && d.id != EMPTY_CHANGE_ID)
             .count()
     }
 
@@ -144,7 +220,7 @@ impl Docket {
             ));
         }
         out.push('\n');
-        out.push_str(&format!("  scope:      {}\n", self.scope));
+        out.push_str(&format!("  intent:     {}\n", self.intent));
         out.push_str(&format!("  authors:    {}\n", self.authors.join(", ")));
         out.push('\n');
         if self.disputes.is_empty() {
@@ -233,7 +309,7 @@ mod tests {
                     detail: "private path .env touched".into(),
                 },
             ],
-            scope: "auth refactor".into(),
+            intent: "auth refactor".into(),
             authors: vec!["@alice".into(), "@bob".into()],
             verdict: Verdict::Adjudicated,
             embargo: Some("patch held for maintainers until 2026-12-31".into()),

src/docket.rs

@@ -80,7 +80,7 @@ mod tests {
             base: "main".into(),
             head: "feature/test-docket".into(),
             disputes: vec![],
-            scope: "testing save and load".into(),
+            intent: "testing save and load".into(),
             authors: vec!["@tester".into()],
             verdict: Verdict::Adjudicated,
             embargo: None,
@@ -91,7 +91,7 @@ mod tests {
 
         assert_eq!(loaded.change, original.change);
         assert_eq!(loaded.source, original.source);
-        assert_eq!(loaded.scope, original.scope);
+        assert_eq!(loaded.intent, original.intent);
         assert_eq!(loaded.verdict, original.verdict);
 
         let _ = std::fs::remove_file(path);
@@ -105,7 +105,7 @@ mod tests {
             base: "main".into(),
             head: "feature/toml-test".into(),
             disputes: vec![],
-            scope: "toml format".into(),
+            intent: "toml format".into(),
             authors: vec!["@coder".into()],
             verdict: Verdict::Embargoed,
             embargo: Some("patch held for maintainers until 2026-12-31".into()),