Skip to content

fix: scope stack comments to connected branches only#67

Merged
nvandessel merged 1 commit intomainfrom
fix/scope-stack-to-connected-branches
Mar 15, 2026
Merged

fix: scope stack comments to connected branches only#67
nvandessel merged 1 commit intomainfrom
fix/scope-stack-to-connected-branches

Conversation

@nvandessel
Copy link
Copy Markdown
Owner

Summary

  • Add ConnectedStack() function in internal/dag/dag.go that computes the subset of branches belonging to the same connected stack as a target branch (walk up Parent chain to trunk, then include all descendants)
  • RenderStackComment now filters to only connected branches before rendering, so each PR's comment shows only its own stack
  • RenderMergedStackComment receives pre-filtered branches from the caller (sync.go), which computes connected sets before deleting merged branches
  • Fixes the bug where two independent stacks (e.g., main -> a1 -> a2 and main -> b1 -> b2) would both appear in every PR's stack comment

Test plan

  • Added 7 unit tests for ConnectedStack covering: single stack, two independent stacks, target as leaf/middle/root, branching stacks, unknown target, single branch
  • Added integration test TestRenderStackComment_FiltersToConnectedStack verifying that RenderStackComment excludes unrelated stacks
  • Added test for merged comment with pre-filtered branches
  • All existing tests continue to pass (go test ./...)
  • RenderTree (used by frond status) is unchanged and still shows all branches
  • Quality gates pass: go build, go vet, gofmt -l

🤖 Generated with Claude Code

Stack comments on PRs were showing ALL tracked branches instead of only
the branches in the same connected stack. This was noisy and confusing
when multiple independent stacks existed (e.g., main -> a1 -> a2 and
main -> b1 -> b2 would all appear in every PR's comment).

Add ConnectedStack() which computes the subset of branches connected to
a given target by walking up the Parent chain to trunk (the "spine") and
then collecting all descendants of spine nodes. RenderStackComment now
uses this to filter branches before rendering. For merged comments, the
caller (sync.go) pre-computes connected sets before deletion and passes
only the relevant remaining branches.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 15, 2026

Greptile Summary

This PR fixes a bug where independent stacks (e.g., main → a1 → a2 and main → b1 → b2) would all appear in every PR's stack comment, even though they are unrelated. The fix introduces a ConnectedStack function in the DAG package that computes the subset of branches belonging to the same connected stack as a target branch by walking up the parent chain to trunk and then collecting all descendants. RenderStackComment now filters branches through ConnectedStack before rendering, and the sync flow pre-computes connected sets before state mutation so merged PR comments also show only their connected stack.

  • Core algorithm: ConnectedStack walks the parent "spine" to trunk, then collects all descendants of spine nodes via DFS — correctly excluding sibling stacks that share the same trunk
  • Two-phase merge handling: Connected sets are captured pre-merge in sync.go, then intersected with post-merge state to produce accurate remaining-stack data
  • Well-tested: 9 new unit tests for ConnectedStack covering single/multi-stack, leaf/middle/root targets, branching topologies, and unknown targets, plus integration tests for filtered rendering
  • No behavioral changes to RenderTree (used by frond status) which continues to show all branches

Confidence Score: 5/5

  • This PR is safe to merge — it's a well-scoped bug fix with thorough test coverage and no risky changes.
  • The changes are clean, well-structured, and thoroughly tested. The ConnectedStack algorithm is correct — I traced through all edge cases including independent stacks, branching stacks, spine-only walks, and trunk boundary detection. The two-phase pre/post-merge computation in sync.go correctly handles state mutation ordering. No new dependencies or architectural changes. All existing public APIs maintain backward compatibility.
  • No files require special attention.

Important Files Changed

Filename Overview
internal/dag/dag.go Adds ConnectedStack function with correct spine-walk + descendant-collection algorithm; integrates filtering into RenderStackComment. Logic is sound and well-tested.
cmd/stack_comments.go Refactored updateMergedComments to accept pre-filtered connected sets and compute readiness per-stack. Clean signature change with correct per-branch readiness computation.
cmd/sync.go Pre-computes connected stacks before state mutation, then intersects with post-merge DAG to produce accurate remaining-stack data for merged comments. Well-structured two-phase approach.
internal/dag/dag_test.go Adds 9 new tests covering ConnectedStack edge cases (single/multi-stack, leaf/middle/root targets, branching, unknown target) plus integration tests for filtered rendering. Good coverage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["RenderStackComment(trunk, branches, highlight)"] --> B{"highlight != '' ?"}
    B -- Yes --> C["ConnectedStack(branches, highlight)"]
    B -- No --> D["Use all branches"]
    C --> E["Walk up Parent chain to trunk (spine)"]
    E --> F["Collect all descendants of spine nodes (DFS)"]
    F --> G["Filter branches to connected set"]
    G --> H["renderTree(trunk, filtered, ...)"]
    D --> H

    I["sync: runSync"] --> J["Pre-compute connectedSets before merge"]
    J --> K["Process merges: delete branches, reparent children"]
    K --> L["Intersect pre-merge connected sets with post-merge DAG"]
    L --> M["updateMergedComments(ctx, st, mergedData, postConnected)"]
    M --> N["Per merged branch: compute readiness from connected subset"]
    N --> O["RenderMergedStackComment with filtered branches"]
    K --> P["updateStackComments(ctx, st)"]
    P --> A
Loading

Last reviewed commit: d7a9de2

@nvandessel nvandessel merged commit 66ed0ca into main Mar 15, 2026
7 checks passed
@nvandessel nvandessel deleted the fix/scope-stack-to-connected-branches branch March 15, 2026 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant