Order PR stack tables by base chain

src/app/ManualRebase.tsx

@@ -56,11 +56,10 @@ async function run() {
     let commit_range = await CommitMetadata.range(commit_map);
 
     // The first revise pass rewrites commits from the merge base upward, so it
-    // must walk groups in commit application order rather than stack display
-    // order. We intentionally do not apply the master_base-first reshuffle
-    // here; that only matters for the follow-up pass that chooses a restart
-    // point for dirty groups.
-    commit_range.group_list = CommitMetadata.stack_order(commit_range).reverse();
+    // must walk groups in commit application order. We intentionally do not
+    // apply the master_base-first reshuffle here; that only matters for the
+    // follow-up pass that chooses a restart point for dirty groups.
+    commit_range.group_list = CommitMetadata.stack_order(commit_range);
 
     for (const commit of commit_range.commit_list) {
       const group_from_map = commit_map[commit.sha];

src/app/SyncGithub.test.ts

@@ -0,0 +1,79 @@
+import { expect, test } from "bun:test";
+
+import { stack_table_data } from "~/app/SyncGithub";
+import * as StackSummaryTable from "~/core/StackSummaryTable";
+
+test("stack table rows follow the base chain even when group_list is reversed", () => {
+  const pr_url_by_group_id = {
+    A: "https://github.com/magus/git-multi-diff-playground/pull/43",
+    B: "https://github.com/magus/git-multi-diff-playground/pull/47",
+    C: "https://github.com/magus/git-multi-diff-playground/pull/61",
+  };
+
+  const commit_range = range(["A", "B", "C"], pr_url_by_group_id);
+  commit_range.group_list.reverse();
+
+  const { pr_url_list } = stack_table_data({
+    commit_range,
+    pr_url_by_group_id: {},
+  });
+
+  const output = StackSummaryTable.write({
+    body: "Summary of problem",
+    pr_url_list,
+    selected_url: pr_url_by_group_id.B,
+  });
+
+  expect(output.split("\n")).toEqual([
+    "Summary of problem",
+    "",
+    "#### [git stack](https://github.com/magus/git-stack-cli)",
+    "- ⏳ `1` https://github.com/magus/git-multi-diff-playground/pull/43",
+    "- 👉 `2` https://github.com/magus/git-multi-diff-playground/pull/47",
+    "- ⏳ `3` https://github.com/magus/git-multi-diff-playground/pull/61",
+  ]);
+});
+
+function range(group_id_list: Array<string>, pr_url_by_group_id: Record<string, string>) {
+  return {
+    invalid: false,
+    group_list: group_id_list.map((group_id, index) => {
+      return {
+        id: group_id,
+        title: group_id,
+        pr: pull_request(pr_url_by_group_id[group_id]!),
+        base: index === 0 ? "master" : group_id_list[index - 1]!,
+        dirty: false,
+        commits: [
+          {
+            sha: group_id,
+            full_message: group_id,
+            subject_line: group_id,
+            branch_id: group_id,
+            title: group_id,
+            master_base: false,
+          },
+        ],
+        master_base: false,
+      };
+    }),
+    commit_list: [],
+    pr_lookup: {},
+    UNASSIGNED: "unassigned",
+  };
+}
+
+function pull_request(url: string) {
+  return {
+    id: "id",
+    number: 1,
+    state: "OPEN" as const,
+    baseRefName: "master",
+    headRefName: "branch",
+    commits: [],
+    title: "title",
+    body: "",
+    url,
+    isDraft: false,
+  };
+}

src/app/SyncGithub.tsx

@@ -137,30 +137,9 @@ async function run() {
     // finally, ensure all prs have the updated stack table from updated pr_url_by_group_id
     // this step must come after the after_push since that step may create new PRs
     // we need the urls for all prs at this step so we run it after the after_push
-    const all_pr_groups: Array<CommitMetadataGroup> = [];
-
-    // Stack tables are rendered directly from this list, so use the canonical
-    // stack presentation order instead of depending on whatever local
-    // iteration order a caller might otherwise use.
-    const stack_group_list = CommitMetadata.stack_order(commit_range);
-
-    // collect all groups and existing pr urls
-    for (const group of stack_group_list) {
-      if (group.id !== commit_range.UNASSIGNED) {
-        // collect all groups
-        all_pr_groups.push(group);
-
-        if (group.pr) {
-          pr_url_by_group_id[group.id] = group.pr.url;
-        }
-      }
-    }
-
-    // get pr url list for all pr groups
-    const pr_url_list = all_pr_groups.map((group) => {
-      const pr_url = pr_url_by_group_id[group.id];
-      invariant(pr_url, "pr_url must exist");
-      return pr_url;
+    const { all_pr_groups, pr_url_list } = stack_table_data({
+      commit_range,
+      pr_url_by_group_id,
     });
 
     // update PR body for all pr groups (not just push_group_list)
@@ -367,3 +346,35 @@ async function run() {
 }
 
 type CommitMetadataGroup = CommitMetadata.CommitRange["group_list"][number];
+
+export function stack_table_data(args: {
+  commit_range: CommitMetadata.CommitRange;
+  pr_url_by_group_id: Record<string, string>;
+}) {
+  const all_pr_groups: Array<CommitMetadataGroup> = [];
+
+  // Stack tables are rendered directly from this list, so use the canonical
+  // stack presentation order instead of depending on whatever local
+  // iteration order a caller might otherwise use.
+  const stack_group_list = CommitMetadata.stack_order(args.commit_range);
+
+  for (const group of stack_group_list) {
+    if (group.id === args.commit_range.UNASSIGNED) {
+      continue;
+    }
+
+    all_pr_groups.push(group);
+
+    if (group.pr) {
+      args.pr_url_by_group_id[group.id] = group.pr.url;
+    }
+  }
+
+  const pr_url_list = all_pr_groups.map((group) => {
+    const pr_url = args.pr_url_by_group_id[group.id];
+    invariant(pr_url, "pr_url must exist");
+    return pr_url;
+  });
+
+  return { all_pr_groups, pr_url_list };
+}

src/core/CommitMetadata.order.test.ts

@@ -10,18 +10,36 @@ test("stack_order preserves stack group order", () => {
   expect(ids(actual)).toEqual(["A", "B", "C"]);
 });
 
-test("rebase_order reverses stack order", () => {
+test("stack_order derives stack group order from base chain", () => {
+  const commit_range = range(["A", "B", "C"]);
+  commit_range.group_list.reverse();
+
+  const actual = CommitMetadata.stack_order(commit_range);
+
+  expect(ids(actual)).toEqual(["A", "B", "C"]);
+});
+
+test("stack_order does not drop groups with incomplete base links", () => {
+  const commit_range = range(["A", "B", "C"]);
+  commit_range.group_list[1]!.base = "missing";
+
+  const actual = CommitMetadata.stack_order(commit_range);
+
+  expect(ids(actual)).toEqual(["A", "B", "C"]);
+});
+
+test("rebase_order preserves stack group order", () => {
   const commit_range = range(["A", "B", "C"]);
   const actual = CommitMetadata.rebase_order(commit_range);
 
-  expect(ids(actual)).toEqual(["C", "B", "A"]);
+  expect(ids(actual)).toEqual(["A", "B", "C"]);
 });
 
 test("rebase_order moves master_base groups to the front", () => {
   const commit_range = range(["A", { id: "B", master_base: true }, "C"]);
 
   const actual = CommitMetadata.rebase_order(commit_range);
-  expect(ids(actual)).toEqual(["B", "C", "A"]);
+  expect(ids(actual)).toEqual(["B", "A", "C"]);
 
   const [first_group] = actual;
   invariant(first_group, "first_group must exist");

src/core/CommitMetadata.ts

@@ -331,19 +331,58 @@ export async function range(commit_group_map?: CommitGroupMap) {
 }
 
 export function stack_order(commit_range: CommitRange): CommitGroupList {
-  return [...commit_range.group_list];
+  const group_by_id = new Map(commit_range.group_list.map((group) => [group.id, group]));
+  const children_by_base = new Map<string, CommitGroupList>();
+
+  for (const group of commit_range.group_list) {
+    if (!group.base || !group_by_id.has(group.base)) {
+      continue;
+    }
+
+    const children = children_by_base.get(group.base) || [];
+    children.push(group);
+    children_by_base.set(group.base, children);
+  }
+
+  const ordered_group_list: CommitGroupList = [];
+  const visited = new Set<string>();
+
+  function visit(group: CommitGroupList[number]) {
+    if (visited.has(group.id)) {
+      return;
+    }
+
+    visited.add(group.id);
+    ordered_group_list.push(group);
+
+    for (const child of children_by_base.get(group.id) || []) {
+      visit(child);
+    }
+  }
+
+  for (const group of commit_range.group_list) {
+    if (!group.base || !group_by_id.has(group.base)) {
+      visit(group);
+    }
+  }
+
+  for (const group of commit_range.group_list) {
+    visit(group);
+  }
+
+  return ordered_group_list;
 }
 
 export function rebase_order(commit_range: CommitRange): CommitGroupList {
   const state = Store.getState();
   const actions = state.actions;
 
-  const reversed_group_list = stack_order(commit_range).reverse();
+  const stack_group_list = stack_order(commit_range);
 
   // order groups with group.master_base first
   const group_list_master: CommitGroupList = [];
   const group_list_others: CommitGroupList = [];
-  for (const group of reversed_group_list) {
+  for (const group of stack_group_list) {
     if (group.master_base) {
       group_list_master.push(group);
     } else {
@@ -354,8 +393,8 @@ export function rebase_order(commit_range: CommitRange): CommitGroupList {
   const ordered_group_list = [...group_list_master, ...group_list_others];
 
   // detect if group list order differs
-  for (let i = 0; i < reversed_group_list.length; i++) {
-    const original_group = reversed_group_list[i];
+  for (let i = 0; i < stack_group_list.length; i++) {
+    const original_group = stack_group_list[i];
     const ordered_group = ordered_group_list[i];
     invariant(original_group, "original_group must exist");
     invariant(ordered_group, "ordered_group must exist");