Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
20 changes: 11 additions & 9 deletions cmd/stack_comments.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,12 @@ func updateStackComments(ctx context.Context, st *state.State) {
// updateMergedComments posts a final stack comment on each merged PR showing
// it as merged and displaying the remaining stack. Called from sync after
// merges are processed but before rebasing.
func updateMergedComments(ctx context.Context, st *state.State, mergedData map[string]state.Branch) {
func updateMergedComments(ctx context.Context, st *state.State, mergedData map[string]state.Branch, connectedSets map[string]map[string]dag.BranchInfo) {
repoURL, err := git.RepoWebURL(ctx)
if err != nil {
fmt.Fprintf(os.Stderr, "warning: could not determine repo URL: %v\n", err)
}

dagBranches := stateToDag(st.Branches)
readinessSlice := dag.ComputeReadiness(dagBranches)
readinessMap := make(map[string]dag.ReadinessInfo, len(readinessSlice))
for _, ri := range readinessSlice {
readinessMap[ri.Name] = ri
}

prNumbers := make(map[string]*int, len(st.Branches))
for name, b := range st.Branches {
prNumbers[name] = b.PR
Expand All @@ -87,7 +80,16 @@ func updateMergedComments(ctx context.Context, st *state.State, mergedData map[s
if b.PR == nil {
continue
}
body := dag.RenderMergedStackComment(st.Trunk, dagBranches, prNumbers, readinessMap, name, repoURL)
connected := connectedSets[name]
if connected == nil {
connected = map[string]dag.BranchInfo{}
}
readinessSlice := dag.ComputeReadiness(connected)
readinessMap := make(map[string]dag.ReadinessInfo, len(readinessSlice))
for _, ri := range readinessSlice {
readinessMap[ri.Name] = ri
}
body := dag.RenderMergedStackComment(st.Trunk, connected, prNumbers, readinessMap, name, repoURL)
if err := upsertComment(ctx, *b.PR, body); err != nil {
fmt.Fprintf(os.Stderr, "warning: merged stack comment on PR #%d: %v\n", *b.PR, err)
}
Expand Down
26 changes: 25 additions & 1 deletion cmd/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ func runSync(cmd *cobra.Command, args []string) error {
}
}

// Pre-compute connected stacks for each merged branch before modifying state.
// This captures which branches are in the same stack as each merged branch
// so that merged stack comments only show the connected branches.
preMergeDag := stateToDag(st.Branches)
connectedSets := make(map[string]map[string]dag.BranchInfo, len(mergedBranches))
for _, merged := range mergedBranches {
connectedSets[merged] = dag.ConnectedStack(preMergeDag, merged)
}

// Step 5: Process merged branches.
// reparentedFrom tracks what the old parent was for each reparented child.
reparentedFrom := make(map[string]string)
Expand Down Expand Up @@ -149,7 +158,22 @@ func runSync(cmd *cobra.Command, args []string) error {

// Step 5e: Update stack comments when merges changed the tree structure.
if len(mergedBranches) > 0 {
updateMergedComments(ctx, st, mergedData)
// Compute remaining connected branches for each merged branch.
// The connected set was computed pre-merge; now remove the merged branch
// itself and update parent refs to reflect the post-merge state.
postMergeDag := stateToDag(st.Branches)
postConnected := make(map[string]map[string]dag.BranchInfo, len(mergedBranches))
for _, merged := range mergedBranches {
preSet := connectedSets[merged]
filtered := make(map[string]dag.BranchInfo)
for name := range preSet {
if info, ok := postMergeDag[name]; ok {
filtered[name] = info
}
}
postConnected[merged] = filtered
}
updateMergedComments(ctx, st, mergedData, postConnected)
updateStackComments(ctx, st)
}

Expand Down
69 changes: 68 additions & 1 deletion internal/dag/dag.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,69 @@ func renderChildren(sb *strings.Builder, node string, children map[string][]stri
}
}

// ConnectedStack returns the subset of branches that belong to the same
// connected stack as the target branch. The connected stack is defined as:
// 1. Walk up from target through Parent links to trunk (the "spine").
// 2. Include all descendants of every branch on the spine.
//
// Branches not on the spine or descended from it are excluded.
// Returns an empty map if target is not found in branches.
func ConnectedStack(branches map[string]BranchInfo, target string) map[string]BranchInfo {
if _, ok := branches[target]; !ok {
return map[string]BranchInfo{}
}

// Step 1: Walk up the spine from target to trunk.
spine := make(map[string]bool)
cur := target
for {
spine[cur] = true
info, ok := branches[cur]
if !ok {
break
}
// If parent is not a tracked branch, we've reached trunk.
if _, parentTracked := branches[info.Parent]; !parentTracked {
break
}
cur = info.Parent
}

// Step 2: Build children map and collect all descendants of spine nodes.
children := make(map[string][]string)
for name, info := range branches {
children[info.Parent] = append(children[info.Parent], name)
}

connected := make(map[string]bool)
// Add all spine nodes.
for s := range spine {
connected[s] = true
}
// BFS/DFS from each spine node to collect descendants.
var collect func(node string)
collect = func(node string) {
for _, child := range children[node] {
if !connected[child] {
connected[child] = true
collect(child)
}
}
}
for s := range spine {
collect(s)
}

// Step 3: Filter branches.
result := make(map[string]BranchInfo, len(connected))
for name := range connected {
if info, ok := branches[name]; ok {
result[name] = info
}
}
return result
}

// CommentMarker is the HTML comment used to identify frond stack comments
// on GitHub PRs. Used by both rendering (here) and upsert detection (cmd).
const CommentMarker = "<!-- frond-stack -->"
Expand All @@ -323,7 +386,11 @@ const CommentMarker = "<!-- frond-stack -->"
// tree is wrapped in <pre> tags instead of a code fence.
// Returns a markdown string wrapped with the frond-stack marker.
func RenderStackComment(trunk string, branches map[string]BranchInfo, prNumbers map[string]*int, readiness map[string]ReadinessInfo, highlight string, repoURL string) string {
tree := renderTree(trunk, branches, prNumbers, readiness, renderOpts{highlight: highlight, repoURL: repoURL})
filtered := branches
if highlight != "" {
filtered = ConnectedStack(branches, highlight)
}
tree := renderTree(trunk, filtered, prNumbers, readiness, renderOpts{highlight: highlight, repoURL: repoURL})

var sb strings.Builder
sb.WriteString(CommentMarker + "\n")
Expand Down
183 changes: 183 additions & 0 deletions internal/dag/dag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -846,8 +846,191 @@ func TestRenderStackComment_EmptyRepoURL(t *testing.T) {
}
}

// ─── ConnectedStack Tests ───────────────────────────────────────────────────

func TestConnectedStack_SingleStack(t *testing.T) {
// main → a1 → a2: targeting a1 should include both a1 and a2
branches := map[string]BranchInfo{
"a1": {Parent: "main"},
"a2": {Parent: "a1"},
}
result := ConnectedStack(branches, "a1")
if len(result) != 2 {
t.Fatalf("expected 2 branches, got %d: %v", len(result), keys(result))
}
if _, ok := result["a1"]; !ok {
t.Error("expected a1 in result")
}
if _, ok := result["a2"]; !ok {
t.Error("expected a2 in result")
}
}

func TestConnectedStack_TwoIndependentStacks(t *testing.T) {
// Stack A: main → a1 → a2
// Stack B: main → b1 → b2
// Targeting a1 should only return a1, a2
branches := map[string]BranchInfo{
"a1": {Parent: "main"},
"a2": {Parent: "a1"},
"b1": {Parent: "main"},
"b2": {Parent: "b1"},
}
result := ConnectedStack(branches, "a1")
if len(result) != 2 {
t.Fatalf("expected 2 branches, got %d: %v", len(result), keys(result))
}
if _, ok := result["a1"]; !ok {
t.Error("expected a1 in result")
}
if _, ok := result["a2"]; !ok {
t.Error("expected a2 in result")
}
if _, ok := result["b1"]; ok {
t.Error("b1 should not be in result")
}
if _, ok := result["b2"]; ok {
t.Error("b2 should not be in result")
}
}

func TestConnectedStack_TargetIsLeaf(t *testing.T) {
// main → a1 → a2: targeting a2 should include both a1 and a2
branches := map[string]BranchInfo{
"a1": {Parent: "main"},
"a2": {Parent: "a1"},
}
result := ConnectedStack(branches, "a2")
if len(result) != 2 {
t.Fatalf("expected 2 branches, got %d: %v", len(result), keys(result))
}
if _, ok := result["a1"]; !ok {
t.Error("expected a1 in result")
}
if _, ok := result["a2"]; !ok {
t.Error("expected a2 in result")
}
}

func TestConnectedStack_TargetIsMiddle(t *testing.T) {
// main → a1 → a2 → a3: targeting a2 should include a1, a2, a3
branches := map[string]BranchInfo{
"a1": {Parent: "main"},
"a2": {Parent: "a1"},
"a3": {Parent: "a2"},
}
result := ConnectedStack(branches, "a2")
if len(result) != 3 {
t.Fatalf("expected 3 branches, got %d: %v", len(result), keys(result))
}
}

func TestConnectedStack_BranchingStack(t *testing.T) {
// main → a1 → a2
// → a3
// Targeting a2 should include a1, a2, a3 (a3 is sibling via shared ancestor a1)
branches := map[string]BranchInfo{
"a1": {Parent: "main"},
"a2": {Parent: "a1"},
"a3": {Parent: "a1"},
}
result := ConnectedStack(branches, "a2")
if len(result) != 3 {
t.Fatalf("expected 3 branches, got %d: %v", len(result), keys(result))
}
}

func TestConnectedStack_TargetNotFound(t *testing.T) {
branches := map[string]BranchInfo{
"a1": {Parent: "main"},
}
result := ConnectedStack(branches, "nonexistent")
if len(result) != 0 {
t.Fatalf("expected 0 branches for unknown target, got %d", len(result))
}
}

func TestConnectedStack_SingleBranch(t *testing.T) {
branches := map[string]BranchInfo{
"a1": {Parent: "main"},
}
result := ConnectedStack(branches, "a1")
if len(result) != 1 {
t.Fatalf("expected 1 branch, got %d", len(result))
}
}

func TestRenderStackComment_FiltersToConnectedStack(t *testing.T) {
// Two independent stacks: a1→a2 and b1→b2
// Comment for a1 should NOT show b1 or b2
branches := map[string]BranchInfo{
"a1": {Parent: "main"},
"a2": {Parent: "a1"},
"b1": {Parent: "main"},
"b2": {Parent: "b1"},
}
prNumbers := map[string]*int{
"a1": intPtr(1),
"a2": intPtr(2),
"b1": intPtr(3),
"b2": intPtr(4),
}
readiness := map[string]ReadinessInfo{
"a1": {Name: "a1", Ready: true},
"a2": {Name: "a2", Ready: true},
"b1": {Name: "b1", Ready: true},
"b2": {Name: "b2", Ready: true},
}

result := RenderStackComment("main", branches, prNumbers, readiness, "a1", "")

if !strings.Contains(result, "a1") {
t.Errorf("expected a1 in output:\n%s", result)
}
if !strings.Contains(result, "a2") {
t.Errorf("expected a2 in output:\n%s", result)
}
if strings.Contains(result, "b1") {
t.Errorf("b1 should not appear in a1's stack comment:\n%s", result)
}
if strings.Contains(result, "b2") {
t.Errorf("b2 should not appear in a1's stack comment:\n%s", result)
}
}

func TestRenderMergedStackComment_WithPreFilteredBranches(t *testing.T) {
// Caller should pre-filter to only pass connected branches.
// After merging a1, only a2 remains in the connected stack.
branches := map[string]BranchInfo{
"a2": {Parent: "main"},
}
prNumbers := map[string]*int{
"a2": intPtr(2),
}
readiness := map[string]ReadinessInfo{
"a2": {Name: "a2", Ready: true},
}

result := RenderMergedStackComment("main", branches, prNumbers, readiness, "a1", "")

if !strings.Contains(result, "**a1** has been merged") {
t.Errorf("missing merged message:\n%s", result)
}
if !strings.Contains(result, "a2") {
t.Errorf("expected a2 in remaining stack:\n%s", result)
}
}

// ─── Helpers ────────────────────────────────────────────────────────────────

func keys(m map[string]BranchInfo) []string {
result := make([]string, 0, len(m))
for k := range m {
result = append(result, k)
}
return result
}

func equalSlice(a, b []string) bool {
if len(a) != len(b) {
return false
Expand Down
Loading