Merge pull request #14083 from gitbutlerapp/frontend-sbm

Some plain git mode gui follow-ups

Author: mtsgrd

apps/desktop/src/components/forge/StackedPullRequestCard.svelte

@@ -20,7 +20,7 @@
 		prNumber: number;
 	}
 
-	const { projectId, branchName, parent, child, isPushed, prNumber }: Props = $props();
+	const { projectId, branchName, parent, child, isPushed, poll, prNumber }: Props = $props();
 
 	const baseBranchService = inject(BASE_BRANCH_SERVICE);
 	const forge = inject(DEFAULT_FORGE_FACTORY);
@@ -87,7 +87,7 @@
 	{hasParent}
 	{parentIsPushed}
 	{baseIsTargetBranch}
-	poll
+	{poll}
 >
 	{#snippet button({ pr, mergeStatus, reopenStatus, setDraft })}
 		{#if pr.state === "open"}

apps/desktop/src/components/views/BranchList.svelte

@@ -17,7 +17,7 @@
 	import { REORDER_DROPZONE_FACTORY } from "$lib/dragging/stackingReorderDropzoneManager";
 	import { DEFAULT_FORGE_FACTORY } from "$lib/forge/forgeFactory.svelte";
 	import { createBranchSelection } from "$lib/selection/key";
-	import { segmentContext } from "$lib/stacks/segmentContext";
+	import { precomputeStack, segmentContext } from "$lib/stacks/segmentContext";
 	import { getStackContext } from "$lib/stacks/stackController.svelte";
 	import { STACK_SERVICE } from "$lib/stacks/stackService.svelte";
 	import { ensureValue } from "$lib/utils/validation";
@@ -78,11 +78,15 @@
 	const canPublishPR = $derived(forge.current.authenticated);
 	const baseBranchNameResponse = $derived(baseBranchService.baseBranchShortName(projectId));
 	const baseBranchName = $derived(baseBranchNameResponse.response);
+
+	// Compute stack-wide derived values once per render so the per-iteration
+	// segmentContext() call below stays O(1) instead of O(n) per index.
+	const stackPrecomputed = $derived(precomputeStack(segments));
 </script>
 
 <div class="branches-wrapper">
 	{#each segments as segment, i}
-		{@const ctx = segmentContext(segments, i)}
+		{@const ctx = segmentContext(segments, i, stackPrecomputed)}
 		{@const branchName = segment.refName?.displayName}
 		{@const branchLabel = branchName ?? "Unnamed segment"}
 		{@const remoteTrackingBranch = segment.remoteTrackingRefName

apps/desktop/src/components/views/StackDetails.svelte

@@ -31,7 +31,7 @@
 	} from "$lib/dragging/dropHandlers/commitDropHandler";
 
 	import { UNCOMMITTED_SERVICE } from "$lib/selection/uncommittedService.svelte";
-	import { segmentContext } from "$lib/stacks/segmentContext";
+	import { precomputeStack, segmentContext } from "$lib/stacks/segmentContext";
 	import { getStackContext } from "$lib/stacks/stackController.svelte";
 	import { STACK_SERVICE } from "$lib/stacks/stackService.svelte";
 	import { inject } from "@gitbutler/core/context";
@@ -112,8 +112,9 @@
 		branchName ? segments.findIndex((s) => s.refName?.displayName === branchName) : -1,
 	);
 	const selectedSegment = $derived(selectedIndex >= 0 ? segments[selectedIndex] : undefined);
+	const stackPrecomputed = $derived(precomputeStack(segments));
 	const selectedContext = $derived(
-		selectedIndex >= 0 ? segmentContext(segments, selectedIndex) : undefined,
+		selectedIndex >= 0 ? segmentContext(segments, selectedIndex, stackPrecomputed) : undefined,
 	);
 
 	// If the selected branch is no longer in the stack (e.g. it was renamed
@@ -158,7 +159,9 @@
 						runHooks: $runHooks,
 						okWithForce: true,
 						onCommitIdChange: (newId) => {
-							if (stackId && branchName && selection) {
+							// branchName may be undefined for commits in anonymous segments
+							// (refName === null); the selection state already tolerates that.
+							if (selection) {
 								const previewOpen = selection.previewOpen ?? true;
 								controller.laneState.selection.set({
 									branchName,

apps/desktop/src/lib/dragging/dropHandlers/stackDropHandler.ts

@@ -13,7 +13,6 @@ import { toCommitMovePlacement } from "$lib/stacks/commitMovePlacement";
 import StackMacros from "$lib/stacks/macros";
 import { toMoveBranchWarning } from "$lib/stacks/stack";
 import { withStackBusy } from "$lib/state/uiState.svelte";
-import { ensureValue } from "$lib/utils/validation";
 import { untrack } from "svelte";
 import type { DropResult } from "$lib/dragging/dropResult";
 import type { DropzoneHandler } from "$lib/dragging/handler";
@@ -95,7 +94,7 @@ export class OutsideLaneDzHandler implements DropzoneHandler {
 				if (sourceStackId) {
 					const diffSpec = changesToDiffSpec(await data.treeChanges());
 					await this.macros.moveChangesToNewCommit(
-						ensureValue(stack.id),
+						stack.id,
 						outcome.newCommit,
 						sourceStackId,
 						sourceCommitId,
@@ -122,7 +121,7 @@ export class OutsideLaneDzHandler implements DropzoneHandler {
 					.map((h) => ({
 						hunkHeader: h.hunkHeader,
 						pathBytes: h.pathBytes,
-						target: this.stackTarget(ensureValue(stack.id)),
+						target: this.stackTarget(stack.id),
 					}));
 				await this.diffService.assignHunk({
 					projectId: this.projectId,
@@ -151,7 +150,7 @@ export class OutsideLaneDzHandler implements DropzoneHandler {
 						: null;
 
 				await this.macros.moveChangesToNewCommit(
-					ensureValue(stack.id),
+					stack.id,
 					outcome.newCommit,
 					data.stackId,
 					data.commitId,
@@ -195,7 +194,7 @@ export class OutsideLaneDzHandler implements DropzoneHandler {
 						{
 							hunkHeader: assignment.hunkHeader,
 							pathBytes: assignment.pathBytes,
-							target: this.stackTarget(ensureValue(stack.id)),
+							target: this.stackTarget(stack.id),
 						},
 					],
 				});
@@ -219,8 +218,10 @@ export class OutsideLaneDzHandler implements DropzoneHandler {
 			branch: { name: undefined },
 		});
 
-		const stackId = ensureValue(stack.id);
-		const branchName = ensureValue(stack.heads.at(0)?.name);
+		const stackId = stack.id;
+		// Freshly-created stacks always have at least one head per the
+		// StackEntryNoOpt invariant ("list is never empty").
+		const branchName = stack.heads[0]!.name;
 
 		const { relativeTo, side } = toCommitMovePlacement({
 			targetBranchName: branchName,

apps/desktop/src/lib/stacks/macros.ts

@@ -1,4 +1,3 @@
-import { ensureValue } from "$lib/utils/validation";
 import type { CreateCommitRequestWorktreeChanges } from "$lib/stacks/stackEndpoints";
 import type { StackService } from "$lib/stacks/stackService.svelte";
 import type { RejectionReason } from "$lib/state/uiState.svelte";
@@ -33,7 +32,6 @@ export default class StackMacros {
 			params.branchName,
 			params.commitMessage,
 		);
-		if (!stack.id) return;
 		if (outcome.newCommit) {
 			this.uiState.lane(stack.id).selection.set({
 				branchName,
@@ -55,7 +53,10 @@ export default class StackMacros {
 			projectId: this.projectId,
 			branch: { name },
 		});
-		const branchName = ensureValue(stack.heads.at(0)?.name);
+		// The backend guarantees a non-empty heads array on freshly-created
+		// stacks (see StackEntryNoOpt + the "list is never empty" invariant
+		// on legacy ui::StackEntry).
+		const branchName = stack.heads[0]!.name;
 		const outcome = await this.stackService.createCommitMutation({
 			projectId: this.projectId,
 			stackBranchName: branchName,

apps/desktop/src/lib/stacks/segmentContext.ts

@@ -1,4 +1,4 @@
-import { partialStackRequestsForcePush } from "$lib/stacks/stack";
+import { branchRequiresForcePush } from "$lib/stacks/stack";
 import type { Segment } from "@gitbutler/but-sdk";
 
 /**
@@ -14,13 +14,46 @@ export interface SegmentContext {
 	stackPrNumbers: (number | undefined)[];
 }
 
-export function segmentContext(segments: Segment[], index: number): SegmentContext {
-	const branchName = segments[index]?.refName?.displayName;
+/**
+ * Stack-wide data shared by every SegmentContext in a stack. Compute
+ * this ONCE per stack (e.g. via `$derived`) and pass it to
+ * `segmentContext(...)` for each index — that keeps the per-segment work
+ * O(1) instead of O(N) per call.
+ */
+export interface StackPrecomputed {
+	stackPrNumbers: (number | undefined)[];
+	/**
+	 * `withForceFromIndex[i]` is true when *any* segment at index `>= i`
+	 * needs a force push. Pre-computed via a single suffix-OR pass over
+	 * the stack so per-segment lookup is O(1).
+	 */
+	withForceFromIndex: boolean[];
+}
+
+export function precomputeStack(segments: Segment[]): StackPrecomputed {
+	const stackPrNumbers = segments.map((s) => s.metadata?.review.pullRequest ?? undefined);
+
+	const withForceFromIndex = new Array<boolean>(segments.length);
+	let anyForceFromHere = false;
+	for (let i = segments.length - 1; i >= 0; i--) {
+		const s = segments[i];
+		if (s && branchRequiresForcePush(s)) anyForceFromHere = true;
+		withForceFromIndex[i] = anyForceFromHere;
+	}
+
+	return { stackPrNumbers, withForceFromIndex };
+}
+
+export function segmentContext(
+	segments: Segment[],
+	index: number,
+	precomputed: StackPrecomputed,
+): SegmentContext {
 	return {
 		branchIndex: index,
 		parent: segments[index + 1],
 		child: segments[index - 1],
-		withForce: branchName ? partialStackRequestsForcePush(branchName, segments) : false,
-		stackPrNumbers: segments.map((s) => s.metadata?.review.pullRequest ?? undefined),
+		withForce: precomputed.withForceFromIndex[index] ?? false,
+		stackPrNumbers: precomputed.stackPrNumbers,
 	};
 }

apps/desktop/src/lib/stacks/stack.ts

@@ -117,26 +117,6 @@ export function branchRequiresForcePush(branch: Segment): boolean {
 	return branch.pushStatus === "unpushedCommitsRequiringForce";
 }
 
-/**
- * Does the branch or other branch this depends on require a force push?
- *
- * @param branchName The name of the branch to check
- * @param allBranches Complete list of branches in the stack. The order is expected to be child-to-parent
- */
-export function partialStackRequestsForcePush(branchName: string, allBranches: Segment[]): boolean {
-	let foundBranch = false;
-
-	for (const branch of allBranches) {
-		if (branch.refName?.displayName === branchName && !foundBranch) {
-			foundBranch = true;
-		}
-		if (!foundBranch) continue;
-		if (branchRequiresForcePush(branch)) return true;
-	}
-
-	return false;
-}
-
 export function stackHasConflicts(stack: Stack): boolean {
 	return stack.segments.at(0)?.commits.some((commit) => commit.hasConflicts) ?? false;
 }

apps/desktop/src/lib/stacks/stackEndpoints.ts

@@ -45,7 +45,7 @@ import type {
 	InsertSide,
 	RelativeTo,
 	RefInfo,
-	StackEntry,
+	StackEntryNoOpt,
 } from "@gitbutler/but-sdk";
 
 export type BranchParams = {
@@ -215,7 +215,7 @@ export function buildStackEndpoints(build: BackendEndpointBuilder) {
 				return transformWorkspaceDetails(response);
 			},
 		}),
-		createStack: build.mutation<StackEntry, { projectId: string; branch: BranchParams }>({
+		createStack: build.mutation<StackEntryNoOpt, { projectId: string; branch: BranchParams }>({
 			extraOptions: {
 				command: "create_virtual_branch",
 				actionName: "Create Stack",

crates/but-workspace/src/legacy/ui.rs

@@ -77,22 +77,33 @@ but_schemars::register_sdk_type!(StackEntry);
 /// **Temporary type to help transitioning to the optional version of stack-entry** and ultimately, to [`crate::RefInfo`].
 /// WARNING: for use by parts in the code that can rely on having a non-optional `stack_id`. The goal is to have none of these.
 #[derive(Debug, Clone, Serialize)]
+#[cfg_attr(feature = "export-schema", derive(schemars::JsonSchema))]
 #[serde(rename_all = "camelCase")]
 pub struct StackEntryNoOpt {
     /// The ID of the stack.
+    #[cfg_attr(
+        feature = "export-schema",
+        schemars(schema_with = "but_schemars::stack_id")
+    )]
     pub id: StackId,
     /// The list of the branch information that are part of the stack.
     /// The list is never empty.
     /// The first entry in the list is always the most recent branch on top the stack.
     pub heads: Vec<StackHeadInfo>,
     /// The tip of the top-most branch, i.e., the most recent commit that would become the parent of new commits of the topmost stack branch.
     #[serde(with = "but_serde::object_id")]
+    #[cfg_attr(
+        feature = "export-schema",
+        schemars(schema_with = "but_schemars::object_id")
+    )]
     pub tip: gix::ObjectId,
     /// The zero-based index for sorting stacks.
     pub order: Option<usize>,
     /// If `true`, then any head in this stack is checked directly so `HEAD` points to it, and this is only ever `true` for a single stack.
     pub is_checked_out: bool,
 }
+#[cfg(feature = "export-schema")]
+but_schemars::register_sdk_type!(StackEntryNoOpt);
 
 impl From<StackEntryNoOpt> for crate::commit::Stack {
     fn from(value: StackEntryNoOpt) -> Self {

packages/but-sdk/src/generated/index.d.ts

@@ -1898,6 +1898,27 @@ export type StackEntry = {
   isCheckedOut: boolean;
 };
 
+/**
+ * **Temporary type to help transitioning to the optional version of stack-entry** and ultimately, to [`crate::RefInfo`].
+ * WARNING: for use by parts in the code that can rely on having a non-optional `stack_id`. The goal is to have none of these.
+ */
+export type StackEntryNoOpt = {
+  /** The ID of the stack. */
+  id: string;
+  /**
+   * The list of the branch information that are part of the stack.
+   * The list is never empty.
+   * The first entry in the list is always the most recent branch on top the stack.
+   */
+  heads: Array<StackHeadInfo>;
+  /** The tip of the top-most branch, i.e., the most recent commit that would become the parent of new commits of the topmost stack branch. */
+  tip: string;
+  /** The zero-based index for sorting stacks. */
+  order: number | null;
+  /** If `true`, then any head in this stack is checked directly so `HEAD` points to it, and this is only ever `true` for a single stack. */
+  isCheckedOut: boolean;
+};
+
 /** The information about the branch inside a stack */
 export type StackHeadInfo = {
   /** The name of the branch. */