feat(github): withhold a plan body rather than overflow the comment

pkg/webhook/plan_drift.go

@@ -64,7 +64,7 @@ func (h *Handler) reviewTimeDrift(ctx context.Context, planReq api.PlanRequest,
 			summary: "drift check failed; see logs",
 		}, &templates.DeploymentDriftData{Computed: false}
 	}
-	preview := deploymentDriftPreview(rollup)
+	preview := deploymentDriftPreview(rollup, primaryPlan.GetPlanId())
 	if rollup.Clean {
 		return reviewDriftOutcome{state: driftClean}, preview
 	}
@@ -83,7 +83,7 @@ const erroredDriftDetail = "diff failed; see server logs"
 // deploymentDriftPreview turns a computed rollup into the PR-preview rendering
 // data. It returns nil for a single-deployment database: with one deployment
 // there is nothing to compare, so a "same plan everywhere" line would be noise.
-func deploymentDriftPreview(rollup api.PlanRollup) *templates.DeploymentDriftData {
+func deploymentDriftPreview(rollup api.PlanRollup, reviewedPlanID string) *templates.DeploymentDriftData {
 	if len(rollup.Entries) <= 1 {
 		return nil
 	}
@@ -124,7 +124,7 @@ func deploymentDriftPreview(rollup api.PlanRollup) *templates.DeploymentDriftDat
 	// vocabulary of a fleet that may diverge, would suggest the agreement was an
 	// outcome rather than the requirement that let the check pass.
 	if rollup.Clean && independent {
-		data.Plans = deploymentPlanGroups(rollup)
+		data.Plans = deploymentPlanGroups(rollup, reviewedPlanID)
 	}
 	return data
 }
@@ -139,16 +139,23 @@ func deploymentDriftPreview(rollup api.PlanRollup) *templates.DeploymentDriftDat
 // the reviewed plan is the one an operator has already seen, and a fixed order
 // keeps a comment that is re-rendered on a later push from reshuffling under a
 // reader who is looking for what changed.
-func deploymentPlanGroups(rollup api.PlanRollup) []templates.DeploymentPlanGroup {
+func deploymentPlanGroups(rollup api.PlanRollup, reviewedPlanID string) []templates.DeploymentPlanGroup {
 	names := rollupMemberNames(rollup)
 	var groups []templates.DeploymentPlanGroup
 	byPlan := make(map[string]int, len(rollup.Entries))
 	for i, e := range rollup.Entries {
 		at, ok := byPlan[e.PlanFingerprint]
 		if !ok {
+			// The primary runs the reviewed plan itself, so it has no member plan
+			// row of its own and its identifier is the reviewed plan's.
+			planID := e.PlanIdentifier
+			if i == 0 {
+				planID = reviewedPlanID
+			}
 			groups = append(groups, templates.DeploymentPlanGroup{
 				Primary: i == 0,
 				Changes: memberPlanChanges(e.ChangeSet),
+				PlanID:  planID,
 			})
 			at = len(groups) - 1
 			byPlan[e.PlanFingerprint] = at

pkg/webhook/plan_drift_test.go

@@ -128,7 +128,7 @@ func TestDeploymentDriftPreview_NilForSingleDeployment(t *testing.T) {
 		Entries: []api.DeploymentRollupEntry{{Deployment: "primary", Class: api.DeploymentMatch}},
 		Clean:   true,
 	}
-	assert.Nil(t, deploymentDriftPreview(rollup))
+	assert.Nil(t, deploymentDriftPreview(rollup, "plan_reviewed"))
 }
 
 // A clean multi-deployment rollup becomes preview data flagged clean and
@@ -141,7 +141,7 @@ func TestDeploymentDriftPreview_CleanMultiDeployment(t *testing.T) {
 		},
 		Clean: true,
 	}
-	preview := deploymentDriftPreview(rollup)
+	preview := deploymentDriftPreview(rollup, "plan_reviewed")
 	assert.NotNil(t, preview)
 	assert.True(t, preview.Computed)
 	assert.True(t, preview.Clean)
@@ -166,7 +166,7 @@ func TestDeploymentDriftPreview_DivergedAndErroredDetails(t *testing.T) {
 		},
 		Clean: false,
 	}
-	preview := deploymentDriftPreview(rollup)
+	preview := deploymentDriftPreview(rollup, "plan_reviewed")
 	assert.False(t, preview.Clean)
 	assert.Equal(t, "diverged", preview.Deployments[1].Class)
 	assert.Contains(t, preview.Deployments[1].Detail, "1 unexpected")
@@ -250,13 +250,37 @@ func TestDeploymentPlanGroups_SameWorkGroupsTogether(t *testing.T) {
 		},
 	}
 
-	groups := deploymentPlanGroups(rollup)
+	groups := deploymentPlanGroups(rollup, "plan_reviewed")
 	assert.Equal(t, [][]string{{"primary/testapp_1", "primary/testapp_2", "primary/testapp_3"}}, groupMembers(groups))
 	assert.True(t, groups[0].Primary)
 	assert.Equal(t, []string{email}, groups[0].Changes[0].Statements)
 	assert.False(t, groups[0].Empty())
 }
 
+// A withheld plan is only reachable if the comment can name it. The primary
+// runs the reviewed plan itself and has no member plan row of its own, so its
+// group carries the reviewed plan's identifier; every other group carries the
+// identifier of the member plan its members were stored with.
+func TestDeploymentPlanGroups_GroupsCarryThePlanToPrintThem(t *testing.T) {
+	email := "ALTER TABLE users ADD COLUMN email VARCHAR(255)"
+	index := "ALTER TABLE users ADD INDEX idx_email (email)"
+	member := plannedMember("eu", "testapp_2", email, index)
+	member.PlanIdentifier = "plan_eu"
+	rollup := api.PlanRollup{
+		Clean:    true,
+		Planning: api.PlanIndependent,
+		Entries: []api.DeploymentRollupEntry{
+			plannedMember("primary", "testapp_1", email),
+			member,
+		},
+	}
+
+	groups := deploymentPlanGroups(rollup, "plan_reviewed")
+	require.Len(t, groups, 2)
+	assert.Equal(t, "plan_reviewed", groups[0].PlanID)
+	assert.Equal(t, "plan_eu", groups[1].PlanID)
+}
+
 // Targets that hold their own schemas can need different work. Each distinct
 // plan is its own group, so the comment describes every plan the apply would
 // run rather than the reviewed one alone.
@@ -273,7 +297,7 @@ func TestDeploymentPlanGroups_DifferentWorkSplits(t *testing.T) {
 		},
 	}
 
-	groups := deploymentPlanGroups(rollup)
+	groups := deploymentPlanGroups(rollup, "plan_reviewed")
 	assert.Equal(t, [][]string{
 		{"primary/testapp_1", "primary/testapp_3"},
 		{"primary/testapp_2"},
@@ -299,7 +323,7 @@ func TestDeploymentPlanGroups_ConvergedTargetsAreTheirOwnGroup(t *testing.T) {
 		},
 	}
 
-	groups := deploymentPlanGroups(rollup)
+	groups := deploymentPlanGroups(rollup, "plan_reviewed")
 	assert.Equal(t, [][]string{
 		{"primary/testapp_1", "primary/testapp_3"},
 		{"primary/testapp_2", "primary/testapp_4"},
@@ -322,7 +346,7 @@ func TestDeploymentPlanGroups_PrimaryGroupComesFirst(t *testing.T) {
 		},
 	}
 
-	groups := deploymentPlanGroups(rollup)
+	groups := deploymentPlanGroups(rollup, "plan_reviewed")
 	assert.True(t, groups[0].Primary)
 	assert.Equal(t, []string{"primary/testapp_1"}, groups[0].Members)
 	assert.True(t, groups[0].Empty(), "the primary having nothing to apply does not move its group")
@@ -346,7 +370,7 @@ func TestDeploymentDriftPreview_BlockedRollupIsNotGrouped(t *testing.T) {
 		},
 	}
 
-	preview := deploymentDriftPreview(rollup)
+	preview := deploymentDriftPreview(rollup, "plan_reviewed")
 	assert.Empty(t, preview.Plans)
 	assert.Len(t, preview.Deployments, 2)
 }
@@ -367,7 +391,7 @@ func TestDeploymentDriftPreview_MirroredMembersAreNotGrouped(t *testing.T) {
 		Entries:  []api.DeploymentRollupEntry{eu, au},
 	}
 
-	preview := deploymentDriftPreview(rollup)
+	preview := deploymentDriftPreview(rollup, "plan_reviewed")
 	assert.Empty(t, preview.Plans)
 }
 
@@ -385,7 +409,7 @@ func TestDeploymentDriftPreview_CleanIndependentRollupCarriesGroups(t *testing.T
 		},
 	}
 
-	preview := deploymentDriftPreview(rollup)
+	preview := deploymentDriftPreview(rollup, "plan_reviewed")
 	assert.Len(t, preview.Plans, 2)
 }
 

pkg/webhook/templates/plan.go

@@ -227,6 +227,17 @@ type DeploymentPlanGroup struct {
 	// the comment renders the reviewed plan itself. Empty for a group whose
 	// members are already at the desired schema.
 	Changes []KeyspaceChangeData
+	// PlanID names the stored plan the group's members would run, so a reader
+	// can print it in full when the comment has no room to show it. Every member
+	// of the group has a stored plan of its own and they are identical, so any
+	// one of them answers the question; this is the first member's.
+	PlanID string
+
+	// clamped marks a group whose DDL body this comment gave up to stay inside
+	// GitHub's size cap. It is set while rendering, never by the caller: how much
+	// room a plan needs is a fact about the comment it lands in, not about the
+	// group.
+	clamped bool
 }
 
 // Empty reports that the group's members are already at the desired schema and
@@ -316,8 +327,88 @@ type KeyspaceShardChange struct {
 	Satisfied bool
 }
 
+// planCommentChromeHeadroom reserves room under GitHub's cap for the markup a
+// caller adds to the body after this renders — the tracking marker and the
+// support-channel footer — plus margin, so an assembled comment never lands
+// exactly at the limit.
+const planCommentChromeHeadroom = 1024
+
+// planCommentBudget is how large a plan comment may render. A comment over
+// GitHub's cap is rejected outright, which would leave the PR with no plan at
+// all rather than a long one.
+const planCommentBudget = GitHubIssueCommentMaxChars - planCommentChromeHeadroom
+
 // RenderPlanComment renders the plan comment markdown.
+//
+// A rollout of N targets against M tables can render past what GitHub will
+// accept, so an oversized comment gives up DDL bodies until it fits, largest
+// plan first. What it never gives up is the summary lines: the comment still
+// names every target and says how much each one would run, and each withheld
+// plan is replaced by the command that prints it in full. A reader who cannot
+// see one plan's statements can still see that it exists, which target runs it,
+// and where to read it.
 func RenderPlanComment(data PlanCommentData) string {
+	out := renderPlanComment(data)
+	if len(out) <= planCommentBudget || len(data.planGroups()) == 0 {
+		return out
+	}
+
+	// Clamping works on a copy so the caller's groups are not rewritten by the
+	// act of rendering them.
+	groups := slices.Clone(data.DeploymentDrift.Plans)
+	drift := *data.DeploymentDrift
+	drift.Plans = groups
+	data.DeploymentDrift = &drift
+
+	for range groups {
+		i := largestUnclampedGroup(groups)
+		if i < 0 {
+			break
+		}
+		groups[i].clamped = true
+		out = renderPlanComment(data)
+		if len(out) <= planCommentBudget {
+			break
+		}
+	}
+	return out
+}
+
+// largestUnclampedGroup is the group whose DDL body would free the most room,
+// or -1 when every group's body has already been given up. Taking the largest
+// first withholds the fewest plans for the room it recovers.
+func largestUnclampedGroup(groups []DeploymentPlanGroup) int {
+	best, bestSize := -1, 0
+	for i, g := range groups {
+		if g.clamped || g.Empty() {
+			continue
+		}
+		if size := renderedDDLSize(g.Changes); size > bestSize {
+			best, bestSize = i, size
+		}
+	}
+	return best
+}
+
+// renderedDDLSize approximates how much of a comment a group's statements
+// occupy. It is used only to order the groups against each other, so it counts
+// the statements rather than rendering them.
+func renderedDDLSize(changes []KeyspaceChangeData) int {
+	size := 0
+	for _, ks := range changes {
+		for _, s := range ks.Statements {
+			size += len(s)
+		}
+		for _, shard := range ks.Shards {
+			for _, s := range shard.Statements {
+				size += len(s)
+			}
+		}
+	}
+	return size
+}
+
+func renderPlanComment(data PlanCommentData) string {
 	var sb strings.Builder
 
 	// Header
@@ -1294,7 +1385,7 @@ func writePlanGroups(sb *strings.Builder, data PlanCommentData, groups []Deploym
 
 		if !collapse {
 			fmt.Fprintf(sb, "**%s** — %s\n\n", heading, label)
-			writeKeyspaceChanges(sb, scoped)
+			writePlanGroupBody(sb, scoped, g)
 			continue
 		}
 
@@ -1303,7 +1394,7 @@ func writePlanGroups(sb *strings.Builder, data PlanCommentData, groups []Deploym
 			open = " open"
 		}
 		fmt.Fprintf(sb, "<details%s>\n<summary><b>%s — %s</b></summary>\n\n", open, heading, label)
-		writeKeyspaceChanges(sb, scoped)
+		writePlanGroupBody(sb, scoped, g)
 		sb.WriteString("</details>\n\n")
 	}
 
@@ -1315,6 +1406,27 @@ func writePlanGroups(sb *strings.Builder, data PlanCommentData, groups []Deploym
 	}
 }
 
+// writePlanGroupBody renders a group's statements, or the pointer that replaces
+// them when the comment had no room for this plan.
+//
+// The pointer names the stored plan rather than summarizing it. A summary of
+// withheld DDL is the one thing worse than withholding it: a reader would take
+// it for the plan and stop looking.
+func writePlanGroupBody(sb *strings.Builder, scoped PlanCommentData, g DeploymentPlanGroup) {
+	if !g.clamped {
+		writeKeyspaceChanges(sb, scoped)
+		return
+	}
+	sb.WriteString("This plan is too large to render here. To read it in full:\n\n")
+	if g.PlanID == "" {
+		// Without an identifier there is nothing to point at, so the comment says
+		// so plainly rather than printing a command that cannot work.
+		sb.WriteString("SchemaBot has no stored identifier for this plan. Re-plan the pull request to store one.\n\n")
+		return
+	}
+	fmt.Fprintf(sb, "```\nschemabot list-plans %s\n```\n\n", g.PlanID)
+}
+
 // writePlanGroupSummary states what the apply runs across the whole rollout,
 // standing in for the reviewed plan's own summary. Counting the primary's tables
 // would understate an apply that runs different work on the other targets, and