Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion apps/mobile/src/features/threads/git/GitOverviewSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,11 @@ function GitOverviewSheetContent(props: GitOverviewSheetProps) {
</View>
) : null}
{chain.layers.map((link, index) => (
<View key={threadPullRequestKeyOf(link)}>
<View
key={threadPullRequestKeyOf(link)}
// Steps in per layer like the web list, capped so deep stacks keep their width.
style={{ paddingLeft: Math.min(chain.depths[index]!, 3) * 16 }}
>
{index > 0 && Platform.OS !== "android" ? (
<View className="ml-12 h-px bg-border" />
) : null}
Expand Down
16 changes: 16 additions & 0 deletions apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,22 @@ describe("listThreadPullRequests", () => {
]);
expect(result.chains).toEqual([{ kind: "native", numbers: [1, 2] }]);
});

it("gives pull requests built on the same one the same position", () => {
const result = listThreadPullRequests({
pullRequests: [
makeLink(1, { headBranch: "feat-a", baseBranch: "main" }),
makeLink(2, { headBranch: "feat-b", baseBranch: "feat-a" }),
makeLink(3, { headBranch: "feat-c", baseBranch: "feat-a" }),
],
});
expect(result.pullRequests.map((entry) => [entry.number, entry.stack])).toEqual([
[1, { kind: "derived", position: 1, size: 3 }],
[2, { kind: "derived", position: 2, size: 3 }],
[3, { kind: "derived", position: 2, size: 3 }],
]);
expect(result.chains).toEqual([{ kind: "derived", numbers: [1, 2, 3] }]);
});
});

describe("pull request guide tools", () => {
Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/mcp/toolkits/pullRequests/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function entryOf(
if (chain.layers.length < 2) continue;
const index = chain.layers.findIndex((layer) => threadPullRequestKeyOf(layer) === key);
if (index !== -1) {
stack = { kind: chain.kind, position: index + 1, size: chain.layers.length };
stack = { kind: chain.kind, position: chain.depths[index]! + 1, size: chain.layers.length };
break;
}
}
Expand Down
4 changes: 2 additions & 2 deletions apps/server/src/mcp/toolkits/pullRequests/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ export const ThreadPullRequestEntry = Schema.Struct({
stack: Schema.NullOr(
Schema.Struct({
kind: Schema.Literals(["native", "derived"]),
/** 1-based, bottom of the stack first. */
/** 1-based layer, bottom of the stack first. Pull requests built on the same one share it. */
position: Schema.Int,
size: Schema.Int,
}),
Expand All @@ -268,7 +268,7 @@ export const ListThreadPullRequestsResult = Schema.Struct({
chains: Schema.Array(
Schema.Struct({
kind: Schema.Literals(["native", "derived"]),
/** Bottom to top. */
/** Bottom to top; each pull request comes before the ones built on it. */
numbers: Schema.Array(Schema.Int),
}),
),
Expand Down
9 changes: 5 additions & 4 deletions apps/web/src/components/pullRequest/pullRequestListLines.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import type { ThreadPullRequestChain } from "@t3tools/shared/threadPullRequests"
/** One line of a thread's pull-request list: a link plus how deep it sits in its stack. */
export interface PullRequestListLine {
readonly link: ThreadPullRequestLink;
/** 0 for a pull request on the base branch; each layer above steps in by one. */
/** 0 for a pull request on the base branch; each layer above steps in by one. Pull requests
* built on the same parent share a depth. */
readonly depth: number;
/** Which chain the line belongs to, so callers can tell one stack's lines from another's. */
readonly chainKey: string;
Expand Down Expand Up @@ -36,12 +37,12 @@ export function pullRequestListLines(
);
return ordered.flatMap((chain) => {
const chainKey = chainKeyOf(chain);
return chain.layers.map((link, depth) => ({
return chain.layers.map((link, index) => ({
link,
depth,
depth: chain.depths[index]!,
chainKey,
stack:
depth === 0 && chain.layers.length > 1
index === 0 && chain.layers.length > 1
? { kind: chain.kind, size: chain.layers.length }
: null,
}));
Expand Down
27 changes: 27 additions & 0 deletions packages/shared/src/threadPullRequests.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,33 @@ describe("resolveThreadPullRequestChains", () => {
]);
});

it("keeps every link built on the same parent under it", () => {
const links = [
link(3, { snapshot: snapshot({ headBranch: "c", baseBranch: "a" }) }),
link(5, { snapshot: snapshot({ headBranch: "p", baseBranch: "main" }) }),
link(6, { snapshot: snapshot({ headBranch: "q", baseBranch: "p" }) }),
link(1, { snapshot: snapshot({ headBranch: "a", baseBranch: "p" }) }),
link(2, { snapshot: snapshot({ headBranch: "b", baseBranch: "a" }) }),
link(9, { snapshot: snapshot({ headBranch: "solo", baseBranch: "main" }) }),
];
const chains = resolveThreadPullRequestChains(links);
expect(
chains.map((chain) => [chain.layers.map((layer) => layer.number), chain.depths]),
).toEqual([
// #6 and #1 both build on #5; #3 and #2 both build on #1.
[
[5, 6, 1, 3, 2],
[0, 1, 1, 2, 2],
],
[[9], [0]],
]);
expect(resolveThreadPullRequestBadge(links.slice(0, 5))).toEqual({
kind: "stack",
layers: 5,
state: "open",
});
});

it("uses the native stack order when the host provides one", () => {
const stack = {
kind: "native" as const,
Expand Down
59 changes: 38 additions & 21 deletions packages/shared/src/threadPullRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,17 @@ export function legacyLinkedPullRequestOf(

export interface ThreadPullRequestChain {
readonly kind: "native" | "derived";
/** Bottom to top. */
/** Bottom to top. Each link comes before the links built on it. */
readonly layers: ReadonlyArray<ThreadPullRequestLink>;
/** One per layer: 0 for the base, parent's depth + 1 above it. Siblings share a depth. */
readonly depths: ReadonlyArray<number>;
}

/**
* Groups a thread's links into stacks. Native stacks come from the host and win; the rest
* are chained by matching one link's base branch to another's head branch within the same
* repository. A link that chains to nothing is a one-layer chain.
* repository. Several links can build on one parent, so a derived stack is a tree, listed
* depth first. A link that chains to nothing is a one-layer chain.
*/
export function resolveThreadPullRequestChains(
links: ReadonlyArray<ThreadPullRequestLink>,
Expand All @@ -233,7 +236,7 @@ export function resolveThreadPullRequestChains(
const order = new Map(members[0]!.stack!.layers.map((layer, index) => [layer.number, index]));
members.sort((left, right) => (order.get(left.number) ?? 0) - (order.get(right.number) ?? 0));
for (const member of members) placed.add(threadPullRequestKeyOf(member));
chains.push({ kind: "native", layers: members });
chains.push({ kind: "native", layers: members, depths: members.map((_, index) => index) });
}

const remaining = visible.filter((link) => !placed.has(threadPullRequestKeyOf(link)));
Expand All @@ -248,31 +251,45 @@ export function resolveThreadPullRequestChains(
const key = branchKey(link, link.snapshot.headBranch);
byHead.set(key, byHead.has(key) ? null : link);
}
const hasChild = new Set<string>();
for (const link of remaining) {
if (link.snapshot === null) continue;
const parentOf = (link: ThreadPullRequestLink) => {
if (link.snapshot === null) return undefined;
const parent = byHead.get(branchKey(link, link.snapshot.baseBranch));
if (parent != null && parent !== link) hasChild.add(threadPullRequestKeyOf(parent));
return parent == null || parent === link ? undefined : parent;
};
const children = new Map<string, Array<ThreadPullRequestLink>>();
for (const link of remaining) {
const parent = parentOf(link);
if (parent === undefined) continue;
const key = threadPullRequestKeyOf(parent);
const siblings = children.get(key) ?? [];
siblings.push(link);
children.set(key, siblings);
}
// Walk from each top (a link nothing builds on) down its base chain.
for (const top of remaining) {
if (hasChild.has(threadPullRequestKeyOf(top))) continue;
// Walk each base (a link that builds on no other link) depth first, so every link a
// parent has sits under it instead of only the first one.
for (const base of remaining) {
if (parentOf(base) !== undefined) continue;
const layers: Array<ThreadPullRequestLink> = [];
let cursor: ThreadPullRequestLink | undefined = top;
while (cursor !== undefined && !placed.has(threadPullRequestKeyOf(cursor))) {
placed.add(threadPullRequestKeyOf(cursor));
layers.unshift(cursor);
cursor =
cursor.snapshot === null
? undefined
: (byHead.get(branchKey(cursor, cursor.snapshot.baseBranch)) ?? undefined);
const depths: Array<number> = [];
const pending: Array<{ readonly link: ThreadPullRequestLink; readonly depth: number }> = [
{ link: base, depth: 0 },
];
for (let next = pending.pop(); next !== undefined; next = pending.pop()) {
const key = threadPullRequestKeyOf(next.link);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
placed.add(key);
layers.push(next.link);
depths.push(next.depth);
const below = children.get(key) ?? [];
for (let index = below.length - 1; index >= 0; index -= 1) {
pending.push({ link: below[index]!, depth: next.depth + 1 });
}
}
if (layers.length > 0) chains.push({ kind: "derived", layers });
chains.push({ kind: "derived", layers, depths });
}
// Cycles have no top. Keep those links visible without inventing a stack order.
// Cycles have no base. Keep those links visible without inventing a stack order.
for (const link of remaining) {
if (!placed.has(threadPullRequestKeyOf(link))) {
chains.push({ kind: "derived", layers: [link] });
chains.push({ kind: "derived", layers: [link], depths: [0] });
}
}
return chains;
Expand Down
Loading