diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs
index 0cb9e597a..9181026ef 100644
--- a/lib/src/rewrite.rs
+++ b/lib/src/rewrite.rs
@@ -19,6 +19,7 @@ use itertools::Itertools;
 use crate::backend::CommitId;
 use crate::commit::Commit;
 use crate::commit_builder::CommitBuilder;
+use crate::dag_walk;
 use crate::op_store::RefTarget;
 use crate::repo::{MutableRepo, RepoRef};
 use crate::repo_path::RepoPath;
@@ -165,19 +166,38 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
         let to_visit_revset = to_visit_expression
             .evaluate(mut_repo.as_repo_ref())
             .unwrap();
-        let to_visit = to_visit_revset.iter().commit_ids().collect_vec();
+        let to_visit_entries = to_visit_revset.iter().collect_vec();
         drop(to_visit_revset);
-
-        let new_commits_expression =
-            RevsetExpression::commits(rewritten.values().flatten().cloned().collect());
-        let ancestors_expression =
-            to_visit_expression.intersection(&new_commits_expression.ancestors());
-        let ancestors_revset = ancestors_expression
-            .evaluate(mut_repo.as_repo_ref())
-            .unwrap();
-        let mut to_skip = abandoned;
-        to_skip.extend(ancestors_revset.iter().commit_ids());
-        drop(ancestors_revset);
+        let index = mut_repo.index();
+        let to_visit_set: HashSet<CommitId> = to_visit_entries
+            .iter()
+            .map(|entry| entry.commit_id())
+            .collect();
+        let mut visited = HashSet::new();
+        // Calculate an order where we rebase parents first, but if the parents were
+        // rewritten, make sure we rebase the rewritten parent first.
+        let to_visit = dag_walk::topo_order_reverse(
+            to_visit_entries,
+            Box::new(|entry| entry.commit_id()),
+            Box::new(|entry| {
+                visited.insert(entry.commit_id());
+                let mut dependents = vec![];
+                for parent in entry.parents() {
+                    if let Some(targets) = rewritten.get(&parent.commit_id()) {
+                        for target in targets {
+                            if to_visit_set.contains(target) && !visited.contains(target) {
+                                dependents.push(index.entry_by_id(target).unwrap());
+                            }
+                        }
+                    }
+                    if to_visit_set.contains(&parent.commit_id()) {
+                        dependents.push(parent);
+                    }
+                }
+                dependents
+            }),
+        );
+        let to_visit = to_visit.iter().map(|entry| entry.commit_id()).collect_vec();
 
         let new_commits = rewritten.values().flatten().cloned().collect();
 
@@ -219,7 +239,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
             new_parents,
             divergent,
             to_visit,
-            to_skip,
+            to_skip: abandoned,
             new_commits,
             rebased: Default::default(),
             branches,
@@ -239,7 +259,14 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
         let mut new_ids = vec![];
         for old_id in old_ids {
             if let Some(new_parent_ids) = self.new_parents.get(old_id) {
-                new_ids.extend(new_parent_ids.clone());
+                for new_parent_id in new_parent_ids {
+                    // The new parent may itself have been rebased earlier in the process
+                    if let Some(newer_parent_id) = self.rebased.get(new_parent_id) {
+                        new_ids.push(newer_parent_id.clone());
+                    } else {
+                        new_ids.push(new_parent_id.clone());
+                    }
+                }
             } else if let Some(new_parent_id) = self.rebased.get(old_id) {
                 new_ids.push(new_parent_id.clone());
             } else {
@@ -308,7 +335,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
         }
 
         self.heads_to_add.remove(&old_commit_id);
-        if !self.new_commits.contains(&old_commit_id) {
+        if !self.new_commits.contains(&old_commit_id) || self.rebased.contains_key(&old_commit_id) {
             self.heads_to_remove.push(old_commit_id);
         }
     }
@@ -358,8 +385,9 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
                 .map(|new_parent_id| self.mut_repo.store().get_commit(new_parent_id).unwrap())
                 .collect_vec();
             let new_commit = rebase_commit(self.settings, self.mut_repo, &old_commit, &new_parents);
-            self.update_references(old_commit_id.clone(), vec![new_commit.id().clone()]);
-            self.rebased.insert(old_commit_id, new_commit.id().clone());
+            self.rebased
+                .insert(old_commit_id.clone(), new_commit.id().clone());
+            self.update_references(old_commit_id, vec![new_commit.id().clone()]);
             return Some(RebasedDescendant {
                 old_commit,
                 new_commit,
diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs
index 97adbec31..b29ed8a9d 100644
--- a/lib/tests/test_rewrite.rs
+++ b/lib/tests/test_rewrite.rs
@@ -81,6 +81,11 @@ fn test_rebase_descendants_forward(use_git: bool) {
     // Commit D does not get rebased because it's an ancestor of the
     // destination. Commit G does not get replaced because it's already in
     // place.
+    // TODO: The above is not what actually happens! The test below shows what
+    // actually happens: D and F also get rebased onto F, so we end up with
+    // duplicates. Consider if it's worth supporting the case above better or if
+    // that decision belongs with the caller (as we currently force it to do by
+    // not supporting it in DescendantRebaser).
     //
     // G
     // F E
@@ -107,18 +112,74 @@ fn test_rebase_descendants_forward(use_git: bool) {
         },
         hashset! {},
     );
-    let new_commit_c = assert_rebased(rebaser.rebase_next(), &commit_c, &[&commit_f]);
-    let new_commit_e = assert_rebased(rebaser.rebase_next(), &commit_e, &[&commit_f]);
+    let new_commit_d = assert_rebased(rebaser.rebase_next(), &commit_d, &[&commit_f]);
+    let new_commit_f = assert_rebased(rebaser.rebase_next(), &commit_f, &[&new_commit_d]);
+    let new_commit_c = assert_rebased(rebaser.rebase_next(), &commit_c, &[&new_commit_f]);
+    let new_commit_e = assert_rebased(rebaser.rebase_next(), &commit_e, &[&new_commit_d]);
+    let new_commit_g = assert_rebased(rebaser.rebase_next(), &commit_g, &[&new_commit_f]);
     assert!(rebaser.rebase_next().is_none());
-    assert_eq!(rebaser.rebased().len(), 2);
+    assert_eq!(rebaser.rebased().len(), 5);
 
     assert_eq!(
         *tx.mut_repo().view().heads(),
         hashset! {
             repo.view().checkout().clone(),
-            commit_g.id().clone(),
             new_commit_c.id().clone(),
-            new_commit_e.id().clone()
+            new_commit_e.id().clone(),
+            new_commit_g.id().clone(),
+        }
+    );
+}
+
+#[test_case(false ; "local backend")]
+#[test_case(true ; "git backend")]
+fn test_rebase_descendants_reorder(use_git: bool) {
+    let settings = testutils::user_settings();
+    let test_workspace = testutils::init_repo(&settings, use_git);
+    let repo = &test_workspace.repo;
+
+    // Commit E was replaced by commit D, and commit C was replaced by commit F
+    // (attempting to to reorder C and E), and commit G was replaced by commit
+    // H.
+    //
+    // I
+    // G H
+    // E F
+    // C D
+    // |/
+    // B
+    // A
+    let mut tx = repo.start_transaction("test");
+    let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
+    let commit_a = graph_builder.initial_commit();
+    let commit_b = graph_builder.commit_with_parents(&[&commit_a]);
+    let commit_c = graph_builder.commit_with_parents(&[&commit_b]);
+    let commit_d = graph_builder.commit_with_parents(&[&commit_b]);
+    let commit_e = graph_builder.commit_with_parents(&[&commit_c]);
+    let commit_f = graph_builder.commit_with_parents(&[&commit_d]);
+    let commit_g = graph_builder.commit_with_parents(&[&commit_e]);
+    let commit_h = graph_builder.commit_with_parents(&[&commit_f]);
+    let commit_i = graph_builder.commit_with_parents(&[&commit_g]);
+
+    let mut rebaser = DescendantRebaser::new(
+        &settings,
+        tx.mut_repo(),
+        hashmap! {
+            commit_e.id().clone() => hashset!{commit_d.id().clone()},
+            commit_c.id().clone() => hashset!{commit_f.id().clone()},
+            commit_g.id().clone() => hashset!{commit_h.id().clone()},
+        },
+        hashset! {},
+    );
+    let new_commit_i = assert_rebased(rebaser.rebase_next(), &commit_i, &[&commit_h]);
+    assert!(rebaser.rebase_next().is_none());
+    assert_eq!(rebaser.rebased().len(), 1);
+
+    assert_eq!(
+        *tx.mut_repo().view().heads(),
+        hashset! {
+            repo.view().checkout().clone(),
+            new_commit_i.id().clone(),
         }
     );
 }
@@ -161,6 +222,55 @@ fn test_rebase_descendants_backward(use_git: bool) {
     );
 }
 
+#[test_case(false ; "local backend")]
+#[test_case(true ; "git backend")]
+fn test_rebase_descendants_chain_becomes_branchy(use_git: bool) {
+    let settings = testutils::user_settings();
+    let test_workspace = testutils::init_repo(&settings, use_git);
+    let repo = &test_workspace.repo;
+
+    // Commit B was replaced by commit E and commit C was replaced by commit F.
+    // Commit F should get rebased onto E, and commit D should get rebased onto
+    // the rebased F.
+    //
+    // D
+    // C F
+    // |/
+    // B E
+    // |/
+    // A
+    let mut tx = repo.start_transaction("test");
+    let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
+    let commit_a = graph_builder.initial_commit();
+    let commit_b = graph_builder.commit_with_parents(&[&commit_a]);
+    let commit_c = graph_builder.commit_with_parents(&[&commit_b]);
+    let commit_d = graph_builder.commit_with_parents(&[&commit_c]);
+    let commit_e = graph_builder.commit_with_parents(&[&commit_a]);
+    let commit_f = graph_builder.commit_with_parents(&[&commit_b]);
+
+    let mut rebaser = DescendantRebaser::new(
+        &settings,
+        tx.mut_repo(),
+        hashmap! {
+            commit_b.id().clone() => hashset!{commit_e.id().clone()},
+            commit_c.id().clone() => hashset!{commit_f.id().clone()},
+        },
+        hashset! {},
+    );
+    let new_commit_f = assert_rebased(rebaser.rebase_next(), &commit_f, &[&commit_e]);
+    let new_commit_d = assert_rebased(rebaser.rebase_next(), &commit_d, &[&new_commit_f]);
+    assert!(rebaser.rebase_next().is_none());
+    assert_eq!(rebaser.rebased().len(), 2);
+
+    assert_eq!(
+        *tx.mut_repo().view().heads(),
+        hashset! {
+            repo.view().checkout().clone(),
+            new_commit_d.id().clone(),
+        }
+    );
+}
+
 #[test_case(false ; "local backend")]
 #[test_case(true ; "git backend")]
 fn test_rebase_descendants_internal_merge(use_git: bool) {
@@ -596,63 +706,6 @@ fn test_rebase_descendants_multiple_no_descendants(use_git: bool) {
     );
 }
 
-#[test_case(false ; "local backend")]
-#[test_case(true ; "git backend")]
-fn test_rebase_descendants_multiple_forward_and_backward(use_git: bool) {
-    let settings = testutils::user_settings();
-    let test_workspace = testutils::init_repo(&settings, use_git);
-    let repo = &test_workspace.repo;
-
-    // Commit B was replaced by commit D. Commit F was replaced by commit C.
-    // Commit G should be rebased onto commit C. Commit H should be rebased onto
-    // commit D. Commits C-D should be left alone since they're ancestors of D.
-    // Commit E should be left alone since its already in place (as a descendant of
-    // D).
-    //
-    // G
-    // F
-    // E
-    // D
-    // C H
-    // |/
-    // B
-    // A
-    let mut tx = repo.start_transaction("test");
-    let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
-    let commit_a = graph_builder.initial_commit();
-    let commit_b = graph_builder.commit_with_parents(&[&commit_a]);
-    let commit_c = graph_builder.commit_with_parents(&[&commit_b]);
-    let commit_d = graph_builder.commit_with_parents(&[&commit_c]);
-    let commit_e = graph_builder.commit_with_parents(&[&commit_d]);
-    let commit_f = graph_builder.commit_with_parents(&[&commit_e]);
-    let commit_g = graph_builder.commit_with_parents(&[&commit_f]);
-    let commit_h = graph_builder.commit_with_parents(&[&commit_b]);
-
-    let mut rebaser = DescendantRebaser::new(
-        &settings,
-        tx.mut_repo(),
-        hashmap! {
-            commit_b.id().clone() => hashset!{commit_d.id().clone()},
-            commit_f.id().clone() => hashset!{commit_c.id().clone()},
-        },
-        hashset! {},
-    );
-    let new_commit_g = assert_rebased(rebaser.rebase_next(), &commit_g, &[&commit_c]);
-    let new_commit_h = assert_rebased(rebaser.rebase_next(), &commit_h, &[&commit_d]);
-    assert!(rebaser.rebase_next().is_none());
-    assert_eq!(rebaser.rebased().len(), 2);
-
-    assert_eq!(
-        *tx.mut_repo().view().heads(),
-        hashset! {
-            repo.view().checkout().clone(),
-            commit_e.id().clone(),
-            new_commit_g.id().clone(),
-            new_commit_h.id().clone()
-        }
-    );
-}
-
 #[test_case(false ; "local backend")]
 #[test_case(true ; "git backend")]
 fn test_rebase_descendants_divergent_rewrite(use_git: bool) {