From edfe8e46a955056b0b8b5fa6fba1ac44baf06091 Mon Sep 17 00:00:00 2001 From: bootjp Date: Thu, 29 Oct 2026 06:41:30 +0000 Subject: [PATCH 1/3] adapter: give the quorum acceptance snapshot wait room under -race TestMigrationQuorumPartitionAcceptance waits for all four members to persist a native snapshot covering the 64 MiB ACKed write. Every member writes that snapshot at once; it takes about a second without the race detector and regularly overran the 5 s wait under -race in CI, failing main after #1349 and several earlier runs. Wait up to 30 s; the phase's own 60 s budget still bounds the large-write/snapshot step and the assertion is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Fm5mogKjCk6edosPUsnMWZ Signed-off-by: bootjp --- adapter/migration_quorum_partition_acceptance_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/adapter/migration_quorum_partition_acceptance_test.go b/adapter/migration_quorum_partition_acceptance_test.go index dcc0e61d7..d8b4bb996 100644 --- a/adapter/migration_quorum_partition_acceptance_test.go +++ b/adapter/migration_quorum_partition_acceptance_test.go @@ -36,6 +36,7 @@ const ( quorumAcceptanceHeartbeatTicks = 10 quorumAcceptanceElectionWait = 3 * quorumAcceptanceTick * quorumAcceptanceElectionTicks quorumAcceptanceSnapshotEvery = 8 + quorumAcceptanceSnapshotWait = 30 * time.Second ) // Real data-group elections, WAL, transport, ConfChanges and native snapshots. @@ -697,6 +698,10 @@ func (c *quorumAcceptanceCluster) retainedAndRestart(t *testing.T, root quorumAc // All members, including the learner, must have an actual persisted native // snapshot covering the ACKed write itself. Status() is a periodically // refreshed diagnostic and can lag the proposal's returned CommitIndex. + // Each of the four members writes a snapshot holding the 64 MiB value at + // the same time; that takes about a second without the race detector and + // regularly exceeded a 5 s wait under -race in CI. The phase's own 60 s + // budget still bounds the whole large-write/snapshot step. require.Eventually(t, func() bool { for _, n := range c.nodes { if n.engine.Status().LastSnapshotIndex < target { @@ -704,7 +709,7 @@ func (c *quorumAcceptanceCluster) retainedAndRestart(t *testing.T, root quorumAc } } return true - }, 5*time.Second, 20*time.Millisecond) + }, quorumAcceptanceSnapshotWait, 20*time.Millisecond) t.Logf("all four members persisted native snapshots covering ACKed write index %d", target) for _, n := range c.nodes { n.tx.Close() From 3b86130998fb9be3220eb22afc26285044ed2ab2 Mon Sep 17 00:00:00 2001 From: bootjp Date: Thu, 29 Oct 2026 06:46:15 +0000 Subject: [PATCH 2/3] adapter: drop the unreachable all-groups branch from queryLeasePrefix queryLeasePrefix is only reached from queryLeaseKey for a Query without an IndexName, so keySchemaForQuery always returns the primary key and the hash-key-mismatch branch that returned queryLeaseAllGroups could never run (test-suite cleanup WP-5, deviation 1). TestQueryLeasePrefix_ BaseTableClassification pins that a base-table Query resolves to its single hash-key prefix or a validation skip; returning an all-groups plan for a pinned hash key fails it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Fm5mogKjCk6edosPUsnMWZ Signed-off-by: bootjp --- adapter/dynamodb_lease.go | 13 ++--- adapter/dynamodb_lease_validation_test.go | 48 +++++++++++++++++++ .../2026_10_16_partial_test_suite_cleanup.md | 4 +- 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/adapter/dynamodb_lease.go b/adapter/dynamodb_lease.go index 638b5e7b4..395e66bc2 100644 --- a/adapter/dynamodb_lease.go +++ b/adapter/dynamodb_lease.go @@ -297,10 +297,11 @@ func (d *DynamoDBServer) queryLeaseKey(ctx context.Context, in queryInput) ([]by } // queryLeasePrefix resolves the single hash-key prefix a base-table Query -// reads, classifying the read into queryLeaseSingleGroup (resolved prefix), -// queryLeaseAllGroups (whole-table prefix: a valid multi-shard read), or +// reads, classifying the read into queryLeaseSingleGroup (resolved prefix) or // queryLeaseSkip (malformed KeyConditionExpression: a validation error the -// read path rejects identically). The validation error is deliberately not +// read path rejects identically). It is only reached without an IndexName, so +// the key schema is always the primary key; GSI queries take +// gsiQueryLeasePlan instead. The validation error is deliberately not // surfaced — only the routing classification matters here, and the read path // reports the identical error downstream. func queryLeasePrefix(in queryInput, schema *dynamoTableSchema) ([]byte, queryLeasePlan) { @@ -310,12 +311,6 @@ func queryLeasePrefix(in queryInput, schema *dynamoTableSchema) ([]byte, queryLe // ValidationException the read path produces without touching data. return nil, queryLeaseSkip } - // A query whose key schema hash key differs from the primary hash - // key reads the whole-table prefix (see queryScanPrefix), which can - // span multiple shards; let the all-groups check cover them. - if keySchema.HashKey != schema.PrimaryKey.HashKey { - return nil, queryLeaseAllGroups - } prefix, err := queryScanPrefix(schema, in, keySchema, cond.hashValue) if err != nil { // queryScanPrefix only fails on an unparseable hash-key value — a diff --git a/adapter/dynamodb_lease_validation_test.go b/adapter/dynamodb_lease_validation_test.go index 6859fa190..86436888c 100644 --- a/adapter/dynamodb_lease_validation_test.go +++ b/adapter/dynamodb_lease_validation_test.go @@ -545,3 +545,51 @@ func TestDynamoDB_LeaseCheckQuery_InvalidExclusiveStartKeySkipsLease(t *testing. require.Equal(t, 0, coord.allGroupsCalls, "invalid ExclusiveStartKey must not trigger the lease check — the transaction is doomed by validation") } + +// TestQueryLeasePrefix_BaseTableClassification pins the base-table lease plan. +// queryLeasePrefix is reached only for a Query without IndexName, whose key +// schema is always the primary key, so a valid condition resolves to the +// single hash-key prefix and anything else is a validation skip: the +// base-table path never classifies as an all-groups read. +func TestQueryLeasePrefix_BaseTableClassification(t *testing.T) { + t.Parallel() + + schema := leaseGSIFixtureSchema() + pinned := queryInput{ + TableName: "t", + KeyConditionExpression: "pk = :pk", + ExpressionAttributeValues: map[string]attributeValue{":pk": newStringAttributeValue("tenant")}, + } + wantPrefix, err := queryScanPrefix(schema, pinned, schema.PrimaryKey, newStringAttributeValue("tenant")) + require.NoError(t, err) + + for _, tc := range []struct { + name string + in queryInput + wantPlan queryLeasePlan + wantPrefix []byte + }{ + {name: "hash key pinned", in: pinned, wantPlan: queryLeaseSingleGroup, wantPrefix: wantPrefix}, + { + name: "condition on a non-key attribute", + in: queryInput{ + TableName: "t", + KeyConditionExpression: "gk = :gk", + ExpressionAttributeValues: map[string]attributeValue{":gk": newStringAttributeValue("x")}, + }, + wantPlan: queryLeaseSkip, + }, + { + name: "malformed condition", + in: queryInput{TableName: "t", KeyConditionExpression: "pk ="}, + wantPlan: queryLeaseSkip, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + prefix, plan := queryLeasePrefix(tc.in, schema) + require.Equal(t, tc.wantPlan, plan) + require.Equal(t, tc.wantPrefix, prefix) + }) + } +} diff --git a/docs/design/2026_10_16_partial_test_suite_cleanup.md b/docs/design/2026_10_16_partial_test_suite_cleanup.md index a555da0c2..2a33d5d98 100644 --- a/docs/design/2026_10_16_partial_test_suite_cleanup.md +++ b/docs/design/2026_10_16_partial_test_suite_cleanup.md @@ -117,7 +117,7 @@ Behaviour notes for the `LocalCoordinator` move: dispatches are serialized, elem Deviations from the plan, found while implementing: -1. **WP-5 retarget 1 (DynamoDB base-table hash-key mismatch) cannot be written.** `queryLeasePrefix` is only reached from `queryLeaseKey` with an empty `IndexName`, so `keySchemaForQuery` always returns `PrimaryKey` and the `keySchema.HashKey != schema.PrimaryKey.HashKey` branch is unreachable. A base-table key condition that does not pin the hash key fails `buildQueryCondition` and is classified as a skip. The duplicate test was deleted; removing the dead branch is a production change and is listed under Follow-ups. +1. **WP-5 retarget 1 (DynamoDB base-table hash-key mismatch) cannot be written.** `queryLeasePrefix` is only reached from `queryLeaseKey` with an empty `IndexName`, so `keySchemaForQuery` always returns `PrimaryKey` and the `keySchema.HashKey != schema.PrimaryKey.HashKey` branch is unreachable. A base-table key condition that does not pin the hash key fails `buildQueryCondition` and is classified as a skip. The duplicate test was deleted. The dead branch was removed in a follow-up change, with `TestQueryLeasePrefix_BaseTableClassification` pinning that a base-table Query resolves to a single-group prefix or a skip. The twin `keySchema.HashKey != schema.PrimaryKey.HashKey` branches in `queryScanPrefix` are unreachable for the same reason and remain a follow-up. 2. **WP-6 retarget 3 (pprof/metrics).** The live `PreparePprofServer`/`PrepareMetricsServer` moved to `internal/serverwiring`, whose own tests already pin the address-before-token validation. The root wrappers and their tests were deleted rather than retargeted. 3. **WP-4 retarget 6 (EXEC dispatch timeout).** `redisDispatchTimeout` is a 30 s constant, so a blocking-store test would take 30 s or need a production change. The rewritten test records the deadline on the stream-deletion scan instead. The mutation named in the plan (`context.Background()` for the EXEC dispatch) survives because `txnContext.dispatchContext` bounds the commit context a second time. Removing that second bound (`dispatchContext` returning an unbounded context) fails the test. @@ -628,7 +628,7 @@ Record each mutation in the PR as a row: file and line, the change, the test tha ## Follow-ups (out of scope) -- **Dead DynamoDB lease branch.** Remove the unreachable hash-key-mismatch branch in `queryLeasePrefix` (`adapter/dynamodb_lease.go`), or document why it is kept (deviation 1 above). +- **Dead DynamoDB read-path branches.** The `queryLeasePrefix` branch is gone (deviation 1 above); the two matching branches in `queryScanPrefix` (`adapter/dynamodb_query_scan.go`) are still unreachable and can be removed the same way. - **Product bugs found by the audit.** Each needs its own PR with a failing test first. - SQS FIFO messages in one group sent within the same millisecond can be received out of order. The visibility key orders by `(visibleAtMillis, random messageID)`. `TestSQSServer_FifoGroupLockHoldsAcrossVisibilityExpiry` and `TestSQSServer_FifoSequenceNumberMonotonic` fail roughly 7/30 and 2/30 runs without `-race`. From c4703157b8d5ffe2acac751056493a64b3cc108d Mon Sep 17 00:00:00 2001 From: bootjp Date: Thu, 29 Oct 2026 09:50:39 +0000 Subject: [PATCH 3/3] adapter: drop the unreachable whole-table branches from queryScanPrefix Every queryScanPrefix caller derives its key schema from keySchemaForQuery(in.IndexName) on the same input, and a migration source schema copies the target's PrimaryKey. With no IndexName the key schema is the primary key; with one, the GSI prefix is returned first. The two keySchema.HashKey != schema.PrimaryKey.HashKey branches that returned the whole-table prefix therefore never ran. Remove them, the keySchema parameter they were the only use of, and the parameter it left unused in queryItemsByKeyCondition. TestQueryScanPrefix_SelectsHashPrefixPerEncoding pins the base-table and GSI prefixes for both key encodings, and the queryLeasePrefix test now builds its expected prefix from the key helpers instead of queryScanPrefix itself. Returning the whole-table prefix for a base-table Query fails both. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Fm5mogKjCk6edosPUsnMWZ Signed-off-by: bootjp --- adapter/dynamodb_lease.go | 4 +- adapter/dynamodb_lease_validation_test.go | 42 ++++++++++++++++++- adapter/dynamodb_query_scan.go | 22 ++++------ .../2026_10_16_partial_test_suite_cleanup.md | 4 +- 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/adapter/dynamodb_lease.go b/adapter/dynamodb_lease.go index 395e66bc2..9e377f489 100644 --- a/adapter/dynamodb_lease.go +++ b/adapter/dynamodb_lease.go @@ -305,13 +305,13 @@ func (d *DynamoDBServer) queryLeaseKey(ctx context.Context, in queryInput) ([]by // surfaced — only the routing classification matters here, and the read path // reports the identical error downstream. func queryLeasePrefix(in queryInput, schema *dynamoTableSchema) ([]byte, queryLeasePlan) { - keySchema, cond, err := resolveQueryCondition(in, schema) + _, cond, err := resolveQueryCondition(in, schema) if err != nil { // Malformed/unsupported KeyConditionExpression: a deterministic // ValidationException the read path produces without touching data. return nil, queryLeaseSkip } - prefix, err := queryScanPrefix(schema, in, keySchema, cond.hashValue) + prefix, err := queryScanPrefix(schema, in, cond.hashValue) if err != nil { // queryScanPrefix only fails on an unparseable hash-key value — a // ValidationException the read path rejects identically. diff --git a/adapter/dynamodb_lease_validation_test.go b/adapter/dynamodb_lease_validation_test.go index 86436888c..618133a34 100644 --- a/adapter/dynamodb_lease_validation_test.go +++ b/adapter/dynamodb_lease_validation_test.go @@ -560,8 +560,9 @@ func TestQueryLeasePrefix_BaseTableClassification(t *testing.T) { KeyConditionExpression: "pk = :pk", ExpressionAttributeValues: map[string]attributeValue{":pk": newStringAttributeValue("tenant")}, } - wantPrefix, err := queryScanPrefix(schema, pinned, schema.PrimaryKey, newStringAttributeValue("tenant")) + hashSegment, err := attributeValueAsKeySegment(newStringAttributeValue("tenant")) require.NoError(t, err) + wantPrefix := dynamoItemHashPrefixForTable(schema.TableName, schema.Generation, hashSegment) for _, tc := range []struct { name string @@ -593,3 +594,42 @@ func TestQueryLeasePrefix_BaseTableClassification(t *testing.T) { }) } } + +// TestQueryScanPrefix_SelectsHashPrefixPerEncoding pins the prefix a Query +// reads for each key encoding: a base-table Query reads its hash-key item +// prefix and a GSI Query reads its index hash prefix. Every caller derives +// the key schema from the same IndexName, so a base-table Query never falls +// back to the whole-table prefix. +func TestQueryScanPrefix_SelectsHashPrefixPerEncoding(t *testing.T) { + t.Parallel() + + hash := newStringAttributeValue("tenant") + segment, err := attributeValueAsKeySegment(hash) + require.NoError(t, err) + legacyKey, err := attributeValueAsKey(hash) + require.NoError(t, err) + + ordered := leaseGSIFixtureSchema() + legacy := leaseGSIFixtureSchema() + legacy.KeyEncodingVersion = 0 + + for _, tc := range []struct { + name string + schema *dynamoTableSchema + index string + want []byte + }{ + {name: "ordered base table", schema: ordered, want: dynamoItemHashPrefixForTable("t", 1, segment)}, + {name: "ordered gsi", schema: ordered, index: "gsi1", want: dynamoGSIHashPrefixForTable("t", 1, "gsi1", segment)}, + {name: "legacy base table", schema: legacy, want: legacyDynamoItemHashPrefixForTable("t", 1, legacyKey)}, + {name: "legacy gsi", schema: legacy, index: "gsi1", want: legacyDynamoGSIHashPrefixForTable("t", 1, "gsi1", legacyKey)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := queryScanPrefix(tc.schema, queryInput{TableName: "t", IndexName: tc.index}, hash) + require.NoError(t, err) + require.Equal(t, tc.want, got) + require.NotEqual(t, dynamoItemPrefixForTable("t", 1), got) + }) + } +} diff --git a/adapter/dynamodb_query_scan.go b/adapter/dynamodb_query_scan.go index 29328f8b1..fa6a6caf4 100644 --- a/adapter/dynamodb_query_scan.go +++ b/adapter/dynamodb_query_scan.go @@ -371,12 +371,12 @@ func (d *DynamoDBServer) loadQueryItemsWithMigration( cond queryCondition, readTS uint64, ) ([]map[string]attributeValue, error) { - items, err := d.queryItemsByKeyCondition(ctx, in, schema, keySchema, cond, readTS) + items, err := d.queryItemsByKeyCondition(ctx, in, schema, cond, readTS) if err != nil { return nil, err } return d.mergeReadItemsFromSourceSchema(schema, keySchema, items, func(sourceSchema *dynamoTableSchema) ([]map[string]attributeValue, error) { - return d.queryItemsByKeyCondition(ctx, in, sourceSchema, keySchema, cond, readTS) + return d.queryItemsByKeyCondition(ctx, in, sourceSchema, cond, readTS) }) } @@ -861,7 +861,7 @@ func resolveQueryReadBounds( if !schema.usesOrderedKeyEncoding() { return dynamoReadBounds{}, false, nil } - basePrefix, err := queryScanPrefix(schema, in, keySchema, cond.hashValue) + basePrefix, err := queryScanPrefix(schema, in, cond.hashValue) if err != nil { return dynamoReadBounds{}, false, err } @@ -1407,14 +1407,13 @@ func (d *DynamoDBServer) queryItemsByKeyCondition( ctx context.Context, in queryInput, schema *dynamoTableSchema, - keySchema dynamoKeySchema, cond queryCondition, readTS uint64, ) ([]map[string]attributeValue, error) { if strings.TrimSpace(in.IndexName) != "" { return d.queryItemsByGSI(ctx, in, schema, cond, readTS) } - scanPrefix, err := queryScanPrefix(schema, in, keySchema, cond.hashValue) + scanPrefix, err := queryScanPrefix(schema, in, cond.hashValue) if err != nil { return nil, err } @@ -1436,11 +1435,10 @@ func (d *DynamoDBServer) queryItemsByGSI( cond queryCondition, readTS uint64, ) ([]map[string]attributeValue, error) { - keySchema, err := schema.keySchemaForQuery(in.IndexName) - if err != nil { + if _, err := schema.keySchemaForQuery(in.IndexName); err != nil { return nil, newDynamoAPIError(http.StatusBadRequest, dynamoErrValidation, err.Error()) } - prefix, err := queryScanPrefix(schema, in, keySchema, cond.hashValue) + prefix, err := queryScanPrefix(schema, in, cond.hashValue) if err != nil { return nil, err } @@ -1691,7 +1689,7 @@ func sendGSIReadResult(ctx context.Context, results chan<- gsiReadResult, result } } -func queryScanPrefix(schema *dynamoTableSchema, in queryInput, keySchema dynamoKeySchema, hashValue attributeValue) ([]byte, error) { +func queryScanPrefix(schema *dynamoTableSchema, in queryInput, hashValue attributeValue) ([]byte, error) { if !schema.usesOrderedKeyEncoding() { hashKey, err := attributeValueAsKey(hashValue) if err != nil { @@ -1700,9 +1698,6 @@ func queryScanPrefix(schema *dynamoTableSchema, in queryInput, keySchema dynamoK if strings.TrimSpace(in.IndexName) != "" { return legacyDynamoGSIHashPrefixForTable(in.TableName, schema.Generation, in.IndexName, hashKey), nil } - if keySchema.HashKey != schema.PrimaryKey.HashKey { - return dynamoItemPrefixForTable(in.TableName, schema.Generation), nil - } return legacyDynamoItemHashPrefixForTable(in.TableName, schema.Generation, hashKey), nil } hashKey, err := attributeValueAsKeySegment(hashValue) @@ -1712,9 +1707,6 @@ func queryScanPrefix(schema *dynamoTableSchema, in queryInput, keySchema dynamoK if strings.TrimSpace(in.IndexName) != "" { return dynamoGSIHashPrefixForTable(in.TableName, schema.Generation, in.IndexName, hashKey), nil } - if keySchema.HashKey != schema.PrimaryKey.HashKey { - return dynamoItemPrefixForTable(in.TableName, schema.Generation), nil - } return dynamoItemHashPrefixForTable(in.TableName, schema.Generation, hashKey), nil } diff --git a/docs/design/2026_10_16_partial_test_suite_cleanup.md b/docs/design/2026_10_16_partial_test_suite_cleanup.md index 2a33d5d98..e96bf8ed9 100644 --- a/docs/design/2026_10_16_partial_test_suite_cleanup.md +++ b/docs/design/2026_10_16_partial_test_suite_cleanup.md @@ -117,7 +117,7 @@ Behaviour notes for the `LocalCoordinator` move: dispatches are serialized, elem Deviations from the plan, found while implementing: -1. **WP-5 retarget 1 (DynamoDB base-table hash-key mismatch) cannot be written.** `queryLeasePrefix` is only reached from `queryLeaseKey` with an empty `IndexName`, so `keySchemaForQuery` always returns `PrimaryKey` and the `keySchema.HashKey != schema.PrimaryKey.HashKey` branch is unreachable. A base-table key condition that does not pin the hash key fails `buildQueryCondition` and is classified as a skip. The duplicate test was deleted. The dead branch was removed in a follow-up change, with `TestQueryLeasePrefix_BaseTableClassification` pinning that a base-table Query resolves to a single-group prefix or a skip. The twin `keySchema.HashKey != schema.PrimaryKey.HashKey` branches in `queryScanPrefix` are unreachable for the same reason and remain a follow-up. +1. **WP-5 retarget 1 (DynamoDB base-table hash-key mismatch) cannot be written.** `queryLeasePrefix` is only reached from `queryLeaseKey` with an empty `IndexName`, so `keySchemaForQuery` always returns `PrimaryKey` and the `keySchema.HashKey != schema.PrimaryKey.HashKey` branch is unreachable. A base-table key condition that does not pin the hash key fails `buildQueryCondition` and is classified as a skip. The duplicate test was deleted. The dead branch was removed in a follow-up change, with `TestQueryLeasePrefix_BaseTableClassification` pinning that a base-table Query resolves to a single-group prefix or a skip. The twin branches in `queryScanPrefix` were unreachable for the same reason: every caller derives the key schema from the same `IndexName`, and a GSI Query returns the index prefix first. They were removed with the now-unused `keySchema` parameter; `TestQueryScanPrefix_SelectsHashPrefixPerEncoding` pins the base-table and GSI prefix for both key encodings. 2. **WP-6 retarget 3 (pprof/metrics).** The live `PreparePprofServer`/`PrepareMetricsServer` moved to `internal/serverwiring`, whose own tests already pin the address-before-token validation. The root wrappers and their tests were deleted rather than retargeted. 3. **WP-4 retarget 6 (EXEC dispatch timeout).** `redisDispatchTimeout` is a 30 s constant, so a blocking-store test would take 30 s or need a production change. The rewritten test records the deadline on the stream-deletion scan instead. The mutation named in the plan (`context.Background()` for the EXEC dispatch) survives because `txnContext.dispatchContext` bounds the commit context a second time. Removing that second bound (`dispatchContext` returning an unbounded context) fails the test. @@ -628,8 +628,6 @@ Record each mutation in the PR as a row: file and line, the change, the test tha ## Follow-ups (out of scope) -- **Dead DynamoDB read-path branches.** The `queryLeasePrefix` branch is gone (deviation 1 above); the two matching branches in `queryScanPrefix` (`adapter/dynamodb_query_scan.go`) are still unreachable and can be removed the same way. - - **Product bugs found by the audit.** Each needs its own PR with a failing test first. - SQS FIFO messages in one group sent within the same millisecond can be received out of order. The visibility key orders by `(visibleAtMillis, random messageID)`. `TestSQSServer_FifoGroupLockHoldsAcrossVisibilityExpiry` and `TestSQSServer_FifoSequenceNumberMonotonic` fail roughly 7/30 and 2/30 runs without `-race`. - The Redis TTL-inline migration builds zero scan plans in production. `startupGatedCoordinator` does not forward `LocalLeaderGroupIDs`, and `keyVizLabeledCoordinator` returns nil when the inner value lacks it. Regression since `a36dac7f`.