address auto-review

- clarify ChangeIDs - they actually have a hash-independent format.

Author: Byron

crates/but-core/src/change_id.rs

@@ -29,11 +29,7 @@ impl ChangeId {
     }
 
     /// Creates a random length 32 reverse hex ChangeId (SHA-1).
-    ///
-    /// As opposed to [`crate::commit::Headers::synthetic_change_id_from_commit_id`], these
-    /// have 4 bytes more than synthetic ones, which could indicate that this was a newly
-    /// created commit that was imbued with a change-id from the get-go.
-    pub fn generate_sha1() -> Self {
+    pub fn generate() -> Self {
         let mut rng = rand::rng();
         let bytes: [u8; CHANGE_ID_REVERSE_BYTE_LEN] = rng.random();
         ChangeId::from_bytes(&bytes)

crates/but-core/src/commit.rs

@@ -38,10 +38,10 @@ impl Headers {
     ///
     /// Useful for when [`Self::change_id`] is `None`.
     ///
-    /// These synthesized IDs are compatible with Jujutsu's deterministic scheme, including for
-    /// SHA-256 object IDs.
+    /// These synthesized IDs are compatible with Jujutsu's deterministic scheme,
+    /// and JJ would create exactly the same change-id if given the `commit_id`.
     pub fn synthetic_change_id_from_commit_id(commit_id: gix::ObjectId) -> ChangeId {
-        let bytes: Vec<_> = commit_id.as_bytes()[4..commit_id.kind().len_in_bytes()]
+        let bytes: Vec<_> = commit_id.as_bytes()[4..gix::hash::Kind::Sha1.len_in_bytes()]
             .iter()
             .rev()
             .map(|byte| byte.reverse_bits())
@@ -59,10 +59,7 @@ impl Headers {
         if self.change_id.is_none() {
             self.change_id = commit_id
                 .into()
-                .map_or_else(
-                    ChangeId::generate_sha1,
-                    Self::synthetic_change_id_from_commit_id,
-                )
+                .map_or_else(ChangeId::generate, Self::synthetic_change_id_from_commit_id)
                 .into();
         }
         self
@@ -73,7 +70,7 @@ impl Headers {
     #[deprecated = "We want deterministic change-ids, use Headers::synthetic_change_id_from_commit_id() instead."]
     pub fn new_with_random_change_id() -> Self {
         Self {
-            change_id: Some(ChangeId::generate_sha1()),
+            change_id: Some(ChangeId::generate()),
             conflicted: None,
         }
     }
@@ -94,7 +91,7 @@ impl Headers {
                             .ok()
                             .map(ChangeId::from_number_for_testing)
                     })
-                    .unwrap_or_else(ChangeId::generate_sha1),
+                    .unwrap_or_else(ChangeId::generate),
             ),
             conflicted: None,
         }

crates/but-core/tests/core/change_id.rs

@@ -3,9 +3,9 @@ mod generate {
 
     #[test]
     fn returns_a_32_character_random_string() {
-        let a = ChangeId::generate_sha1();
+        let a = ChangeId::generate();
         assert_eq!(a.to_string().len(), 32);
-        let b = ChangeId::generate_sha1();
+        let b = ChangeId::generate();
         assert_ne!(a, b, "these are always different");
     }
 }

crates/but-gerrit/src/lib.rs

@@ -262,7 +262,7 @@ mod tests {
 
     #[test]
     fn output_is_41_characters_long() {
-        let commit_change_id = ChangeId::generate_sha1();
+        let commit_change_id = ChangeId::generate();
         let change_id = GerritChangeId::from(&commit_change_id);
         let output = format!("{change_id}");
         assert_eq!(output.len(), 41); // "I" + 40 hex chars
@@ -271,7 +271,7 @@ mod tests {
 
     #[test]
     fn test_add_trailer_no_existing_trailers() {
-        let commit_change_id = ChangeId::generate_sha1();
+        let commit_change_id = ChangeId::generate();
         let change_id = GerritChangeId::from(&commit_change_id);
         let change_id_line = format!("Change-Id: {change_id}\n");
 
@@ -287,7 +287,7 @@ mod tests {
 
     #[test]
     fn test_add_trailer_already_has_change_id() {
-        let commit_change_id = ChangeId::generate_sha1();
+        let commit_change_id = ChangeId::generate();
         let change_id = GerritChangeId::from(&commit_change_id);
         let change_id_line = format!("Change-Id: {change_id}\n");
 
@@ -298,7 +298,7 @@ mod tests {
 
     #[test]
     fn test_add_trailer_with_signed_off_by() {
-        let commit_change_id = ChangeId::generate_sha1();
+        let commit_change_id = ChangeId::generate();
         let change_id = GerritChangeId::from(&commit_change_id);
         let change_id_line = format!("Change-Id: {change_id}\n");
 

packages/core/src/generated/workspace/index.ts

@@ -131,9 +131,11 @@ export type Commit = {
 	 */
 	author: Author;
 	/**
-	 * The GitButler change-id associated with this commit, if available.
+	 * The GitButler change-id associated with this commit.
+	 * It always exists as we either read it from the [headers][but_core::commit::Headers], or we
+	 * synthesize one from [the commit id][but_core::commit::Headers::synthetic_change_id_from_commit_id()].
 	 */
-	changeId: string | null;
+	changeId: string;
 	/**
 	 * Optional URL to the Gerrit review for this commit, if applicable.
 	 * Only populated if Gerrit mode is enabled and the commit has an associated review.