fix(github): give multi-keyspace Strata applies the keyspace-grouped shard layout

[aparajon]

Why this matters

The sharded comment layout required a single namespace, so an apply spanning several keyspaces fell to the per-deployment layout — whose unit is one operation. A wide apply rendered dozens of identical per-operation bullets and empty details blocks, and the operator could not see keyspaces, shards, or ordering at all. This PR gives multi-keyspace applies the same keyspace-grouped shard view a single-keyspace apply already gets.

What it does

isShardedApply admits multi-keyspace applies, and the sharded layout groups by keyspace:

ShardedApplyData
  └─ Keyspaces []ShardedKeyspace   one section per keyspace, resolved order
      ├─ Shards                    per-shard status rows
      └─ Cells                     per-shard DDL cells

One presentation pass derives ordering across all keyspaces. Ordering labels qualify shards with their keyspace (cdb_resolute_lookup/-) only when the apply spans more than one — unsharded keyspaces all use the - shard, so bare names would be ambiguous — while shard names inside each section stay plain.

The layout change itself leaves single-keyspace applies rendering byte-for-byte as before — a one-element keyspace loop emits the same section the old code did.

Divergence group headings adopt the plan comment's shared shard-list helpers: beyond a small inline cap the heading states coverage against the keyspace's shard count (15 of 16 shards) instead of enumerating names — which stay reachable in the group's status table — and shard names in headings and status rows are normalized as inline code. Passing the real keyspace total means a divergent subset can never read as "all" shards. This part does change single-keyspace output: any divergence group wider than the inline cap now states coverage instead of listing names.

Apply Across Multiple Keyspaces

Schema Change Status — Production

Database: cdb_resolute | Type: Strata | Apply ID: apply-a1b2c3d4e5f6

Applied by @jackjackbits at 2026-01-01 00:00:00 UTC

Shards: 1 completed, 1 running table copy, 4 waiting for cdb_resolute_lookup/-

Keyspace cdb_resolute

Shard	Status
-	✅ completed

Keyspace cdb_resolute_lookup

Shard	Status
-	🔄 running table copy

Keyspace cdb_resolute_sharded

Shard	Status
-40	⏳ waiting for cdb_resolute_lookup/-
40-80	⏳ waiting for cdb_resolute_lookup/-
80-c0	⏳ waiting for cdb_resolute_lookup/-
c0-	⏳ waiting for cdb_resolute_lookup/-

VSchema

cdb_resolute_sharded: Pending

Last updated: 2026-01-01 00:00:00 UTC (2026-01-01 00:00:00 UTC)

How it moves us toward the northstar

Real sharded fleets are multi-keyspace — a change routinely touches a sharded keyspace plus its lookup keyspace. Fanning out per shard is only first-class if the PR comment presents that whole graph as one coherent view; this is the base the table-altitude rendering (#1190) and live copy progress (#1191) build on.

Opened by Claude (Fable 5).

[Copilot]

Pull request overview

Fixes the GitHub sharded-apply comment UX for Strata applies that span multiple keyspaces within a single deployment by extending the sharded layout to render one section per keyspace (while preserving existing single-keyspace rendering).

Changes:

• Refactors sharded-apply template data from a single Keyspace + flat shard list to Keyspaces []ShardedKeyspace, enabling keyspace-grouped rendering.
• Updates sharded-apply detection/build logic to treat multi-keyspace, single-deployment shard fan-out as “sharded”, and qualifies ordering-label identities with keyspace/shard when multiple keyspaces are present.
• Adds/updates previews and tests to cover multi-keyspace rendering, ordering, and label qualification.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
TEMPLATES.md	Adds rendered template example for “Apply Across Multiple Keyspaces”.
pkg/webhook/templates/sharded_apply.go	Refactors template input model to keyspace-grouped layout; updates rendering to iterate keyspace sections and flatten for cross-keyspace rollups.
pkg/webhook/templates/sharded_apply_test.go	Updates existing tests for new data shape; adds coverage for multi-keyspace sections and failure lifting across keyspaces.
pkg/webhook/templates/sharded_apply_tenant_test.go	Updates tenant-hint test to new Keyspaces shape.
pkg/webhook/templates/preview_sharded.go	Updates previews to use Keyspaces; adds a new multi-keyspace sharded preview.
pkg/webhook/sharded_apply.go	Allows multi-keyspace sharded applies; builds grouped keyspace data and derives cross-keyspace shard ordering labels.
pkg/webhook/sharded_apply_test.go	Updates sharded-apply behavior tests; adds multi-keyspace rendering and ordering-label qualification tests.
pkg/webhook/multi_apply.go	Updates sharded-apply layout comment to reflect multi-keyspace support.
pkg/cmd/internal/templates/preview_comment.go	Registers the new multi-keyspace sharded preview in the CLI preview output.

Suppressed comments (1)

pkg/webhook/templates/sharded_apply.go:345

• In multi-keyspace sharded applies, the "First failure" callout currently prints only the shard name (e.g. "shard -"), which is ambiguous because shard names can repeat across keyspaces (every unsharded keyspace uses "-"). Consider keyspace-qualifying the shard identifier in this callout when more than one keyspace is present.

	for _, s := range allShardStatuses(data.Keyspaces) {
		if !isShardFailureState(s.State) {
			continue
		}
		shard := html.EscapeString(s.Shard)

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[morgo]

🤖 Approved on Morgan's behalf at e74aa6d0. CI green (37/37). Rendering-only, nothing blocking — findings below are all non-blocking.

Blast radius checked first. isShardedApply has exactly one non-test call site, formatApplyStatusComment, so admitting multi-keyspace applies changes layout selection and nothing else. No apply path, no DDL execution, no gating decision moves.

The one thing I'd like changed, eventually. shardStatusesByKeyspace now pairs derived[i] with groups[i] positionally, where the old shardStatuses read the name back out of d.Deployment and was order-independent. That's a new dependency on presentation.Derive returning Deployments index-parallel to its input.

It holds today — Derive does deployments := make([]Deployment, len(ops)) then deployments[i] = deriveDeployment(ops, i), with no sort or filter — so the code is correct as written. But that invariant is not documented: Derive's comment specifies the input must be in resolved order and says nothing about the output's correspondence. And firstFailure in the same file explicitly anticipates "a caller that later re-slices or sorts Deployments," so reordering is already contemplated in this package. If anyone ever sorts inside Derive — grouping failures first, say — this breaks silently: shard rows land under the wrong keyspace heading with the wrong names, on an operator-facing status comment, with no test necessarily catching it.

Cheapest fix is a sentence on Derive stating that Deployments is index-parallel to ops. Keying by identity would be more robust still, and the qualified identity is unambiguous since shard names contain no /.

Two smaller notes.

The body says "Single-keyspace applies render byte-for-byte as before." That's true of the layout change — I diffed it, the old writeShardKeyspaceSection emitted the same #### Keyspace heading, so a one-element loop is genuinely identical output. But the second commit's divergence-heading coverage change does alter single-keyspace output whenever a group exceeds the inline cap. Both paragraphs are individually accurate; read together the blanket phrasing in the first slightly oversells it. Worth tightening so a reader doesn't take it as a full compatibility guarantee.

On size: GitHubIssueCommentMaxChars documents the logs fold as the elastic section that "spends whatever room the rest of the comment leaves," and this PR grows the non-elastic part for exactly the widest applies that previously fell back to the smaller per-deployment layout. It's much milder than I first assumed, because the apply comment is status-only — no DDL cells — so growth is bounded by shard-row count, and the coverage collapse already caps heading enumeration. But there's no clamp on the shard tables themselves, and the failure mode past the cap is a rejected comment, i.e. the operator sees nothing rather than a degraded view. Not reachable at the fleet widths in the example; worth a bound before it is. The coverage-collapse pattern this PR introduces is the obvious model for it.

Verified clean:

• No map-iteration nondeterminism. keyspaceOrder and groupOrder are slices built in operation order; cellsByKeyspace, groupIndex, and shardsByKeyspace are only ever read through those slices.
• tables[ks.Keyspace+"\x00"+c.Table] uses a NUL separator rather than a bare concat, so the distinct-table count can't collide across keyspaces.
• Keyspace-qualifying the presentation identity only when the apply spans more than one keyspace is the right call, and the reasoning in the comment is correct: every unsharded keyspace's shard is -, so bare names in ordering labels would genuinely be ambiguous. Status rows keeping the plain name under their heading is the right complement.
• The coverage denominator is the keyspace's shard count within the apply, which is the correct denominator for an apply status comment and does prevent a divergent subset reading as "all".
• Three new tests cover multi-keyspace sections, ordering, lifted first-failure, and wide-group coverage; no existing golden assertion was edited, so the single-keyspace regression surface stayed under test.

[aparajon]

🤖 Thanks — all three addressed:

1. Positional pairing — took the robust option in 88b833d: shardStatusesByKeyspace now keys each derived deployment back to its group by presentation identity (unique across groups in both the qualified and bare cases), so a future sort inside Derive can't land shard rows under the wrong keyspace. A missing identity — a contract violation — logs a warning and omits the row rather than rendering another shard's status under this shard's name. Also added the sentence you asked for to Derive's doc: Deployments is index-parallel to ops, one entry per operation, identity preserved.
2. "Byte-for-byte" claim — tightened the PR body: the claim is now scoped to the layout commit, and the divergence-coverage paragraph states explicitly that it changes single-keyspace output for groups wider than the inline cap.
3. Size bound — agreed, deferring to the top of this stack rather than here: the next PR makes the per-shard status tables exception-only, so a clamp added now would be rewritten immediately. The bound (coverage-collapse style, as you suggest) will land with/after that change, where the remaining unbounded surface is the exception tables on wide keyspaces.

— Claude (Fable 5) via Claude Code