feat: decouple feature flag composition when split config is enabled - #3034
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds feature-flag composition fields to proto, generated code, and DTOs. Updates the feature-flag mapping table and migration to allow nullable base schema version IDs and a new key layout. Threads ChangesFeature Flag Composition Metadata and Split-Config Wiring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
Router image scan passed✅ No security vulnerabilities found in image: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@studio/src/pages/`[organizationSlug]/[namespace]/graph/[slug]/compositions/[compositionId]/index.tsx:
- Around line 647-648: Normalize the active tab in the composition details page
when feature-flag compositions are hidden: the current `Tabs` value can still
point to `ffCompostions` even though `featureFlagCompositions` is being omitted,
leaving no valid panel selected. Update the tab selection logic in the
composition page component (`Tabs` / selected tab state around
`isFeatureFlagComposition` and `featureFlagCompositions`) so that when
`isFeatureFlagComposition` is true, the selected tab is coerced to a valid
default tab instead of preserving the invalid feature-flag tab value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb91eff0-aa70-45ce-9f88-e9414dd6ee7b
⛔ Files ignored due to path filters (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**
📒 Files selected for processing (15)
connect/src/wg/cosmo/platform/v1/platform_pb.tscontrolplane/migrations/0145_wild_lady_bullseye.sqlcontrolplane/migrations/meta/0145_snapshot.jsoncontrolplane/migrations/meta/_journal.jsoncontrolplane/src/bin/cleanup-old-data.tscontrolplane/src/core/composition/composer.tscontrolplane/src/core/repositories/FederatedGraphRepository.tscontrolplane/src/core/repositories/GraphCompositionRepository.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/services/CompositionService.tscontrolplane/src/db/schema.tscontrolplane/src/types/index.tsproto/wg/cosmo/platform/v1/platform.protostudio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/[compositionId]/index.tsxstudio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/index.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/[compositionId]/index.tsx (1)
211-213: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
constfor the normalized tab value.This is a single derived value, so keeping both bindings immutable is simpler and matches the repo rule.
♻️ Proposed refactor
- let tab = router.query.tab as string; - tab = isFeatureFlagComposition && tab === 'ffCompostions' ? 'output' : tab; + const requestedTab = router.query.tab as string; + const tab = isFeatureFlagComposition && requestedTab === 'ffCompostions' ? 'output' : requestedTab;As per coding guidelines,
**/*.{js,jsx,ts,tsx}: Preferconstoverletandletovervar.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio/src/pages/`[organizationSlug]/[namespace]/graph/[slug]/compositions/[compositionId]/index.tsx around lines 211 - 213, The tab normalization in the composition page uses a mutable binding even though it is only derived once; update the logic around router.query.tab so the normalized tab value is assigned with const instead of let. Keep the same behavior for the isFeatureFlagComposition and ffCompostions check, but make both bindings immutable to match the repo’s const-over-let guideline.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@studio/src/pages/`[organizationSlug]/[namespace]/graph/[slug]/compositions/[compositionId]/index.tsx:
- Around line 211-213: The tab normalization in the composition page uses a
mutable binding even though it is only derived once; update the logic around
router.query.tab so the normalized tab value is assigned with const instead of
let. Keep the same behavior for the isFeatureFlagComposition and ffCompostions
check, but make both bindings immutable to match the repo’s const-over-let
guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d39c7abe-3648-4a76-9607-f80fd1c3db30
📒 Files selected for processing (1)
studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/[compositionId]/index.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controlplane/src/core/repositories/FeatureFlagRepository.ts`:
- Around line 1397-1402: The split-config lookup in FeatureFlagRepository should
stay scoped to the base federated graph instead of filtering only by
featureFlagId. Update the conditional branch in the query that uses
splitConfigFeature?.enabled to also derive the federated graph from
baseSchemaVersionId and add
federatedGraphsToFeatureFlagSchemaVersions.federatedGraphId to that filter,
matching the scope used by the non-split branch.
In `@controlplane/src/core/repositories/GraphCompositionRepository.ts`:
- Around line 403-408: The child subgraph query in GraphCompositionRepository
currently joins all feature-flag schema-version rows for a base schema version,
which can include stale recomposition entries. Update the query that builds the
composed subgraphs to first select only the latest row per feature flag/base
composition pair, then join that filtered set to graphCompositions so
hasMultipleChangedSubgraphs and triggeredBySubgraphName are based on the most
recent composition only. Use the existing latest-per-feature-flag selection
pattern already used elsewhere in GraphCompositionRepository as the reference
for the join/filter logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aa1adba5-e5b1-4065-876a-b8d7b6e52399
📒 Files selected for processing (5)
controlplane/src/core/bufservices/federated-graph/getCompositionDetails.tscontrolplane/src/core/repositories/FeatureFlagRepository.tscontrolplane/src/core/repositories/GraphCompositionRepository.tscontrolplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.tsstudio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/index.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts (1)
106-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStray line-continuation backslash in comment.
The comment at line 107 ends with a trailing
\before the line break, which is a leftover artifact (likely from copy/paste) and serves no purpose inside a//comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts` around lines 106 - 108, Remove the stray trailing line-continuation backslash from the comment in get-feature-flags-in-latest-composition-by-federated-graph.test.ts so the explanatory text around recomposeFeatureFlag reads cleanly; keep the comment content the same, just delete the unnecessary backslash artifact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts`:
- Around line 123-132: The inline comment in
getFeatureFlagsInLatestCompositionByFederatedGraph test is stale and contradicts
the assertion. Update the comment near withSecondFlag in the test to match the
current split-config-loading behavior: the second flag may be composed, but it
is not surfaced in featureFlags, so the expected length remains 0. Keep the
assertion unchanged and make the comment align with the actual behavior of
client.getFeatureFlagsInLatestCompositionByFederatedGraph and createFeatureFlag.
---
Nitpick comments:
In
`@controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts`:
- Around line 106-108: Remove the stray trailing line-continuation backslash
from the comment in
get-feature-flags-in-latest-composition-by-federated-graph.test.ts so the
explanatory text around recomposeFeatureFlag reads cleanly; keep the comment
content the same, just delete the unnecessary backslash artifact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d505270-c761-4bb6-a1e8-0bb3b0db1ebe
📒 Files selected for processing (2)
controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.tsstudio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/compositions/index.tsx
…e-flag-composition-when-split
…e-flag-composition-when-split
…e-flag-composition-when-split # Conflicts: # controlplane/src/core/services/CompositionService.ts
…e-flag-composition-when-split
…e-flag-composition-when-split
…e-flag-composition-when-split
…e-flag-composition-when-split # Conflicts: # connect/src/wg/cosmo/platform/v1/platform_pb.ts
…e-flag-composition-when-split
Summary by CodeRabbit
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.