feat: improve performance of subgraph batch-publish#2969
Merged
JivusAyrus merged 12 commits intoJun 23, 2026
Merged
Conversation
…publishing of subgraphs
…rieval in batch processing
…osition-pipeline-for-batch
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughIntroduces a bounded-concurrency batched recomposition/deploy path ( ChangesBatched Composition/Deploy Pipeline
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
controlplane/src/core/services/CompositionService.ts (1)
690-708: ⚡ Quick winType the contract router config instead of letting it become
any.
let contractRouterExecutionConfig;infersanyand then flows intosaveCompositionandcontractBaseCompositionDataByContractId.Proposed change
- let contractRouterExecutionConfig; + let contractRouterExecutionConfig: RouterConfig | undefined;As per coding guidelines,
**/*.{ts,tsx}should “Avoidanytype in TypeScript; use specific types or generics.”🤖 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/src/core/services/CompositionService.ts` around lines 690 - 708, The variable contractRouterExecutionConfig is declared without a type annotation, causing it to be inferred as any type. Add an explicit type annotation to contractRouterExecutionConfig based on the return type of the deserializeRouterExecutionConfig function. Determine the correct type (likely RouterExecutionConfig or similar) from the function's return type definition and use it in the let statement, optionally allowing for null or undefined if the function can return those values.Source: Coding guidelines
controlplane/src/core/repositories/FeatureFlagRepository.ts (1)
68-72: ⚡ Quick winUse an interface for the exported cache shape.
FeatureFlagCollectCachesis an exported object shape, so keep it aligned with the TypeScript style rule.Proposed change
-export type FeatureFlagCollectCaches = { +export interface FeatureFlagCollectCaches { featureFlagsByBaseSubgraphId: Map<string, ReturnType<FeatureFlagRepository['getFeatureFlagsByBaseSubgraphId']>>; matchedFeatureFlagsByLabelKey: Map<string, ReturnType<FeatureFlagRepository['getMatchedFeatureFlags']>>; featureSubgraphsByFlagId: Map<string, ReturnType<FeatureFlagRepository['getFeatureSubgraphsByFeatureFlagId']>>; -}; +}As per coding guidelines,
**/*.{ts,tsx}should “Prefer interfaces over type aliases for object shapes in TypeScript.”🤖 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/src/core/repositories/FeatureFlagRepository.ts` around lines 68 - 72, The FeatureFlagCollectCaches is an exported object shape currently defined as a type alias, but it should be converted to an interface to align with TypeScript coding guidelines that prefer interfaces over type aliases for object shapes. Change the declaration from using `export type FeatureFlagCollectCaches = {` syntax to `export interface FeatureFlagCollectCaches {` syntax, and remove the trailing semicolon after the closing brace to match standard interface declaration patterns.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.
Inline comments:
In `@controlplane/src/core/repositories/FeatureFlagRepository.ts`:
- Around line 1140-1150: The memoization keys for both
getFeatureFlagsByBaseSubgraphId and getMatchedFeatureFlags calls are incomplete.
The first call only includes baseSubgraphId in its key but the query depends on
namespaceId and excludeDisabled as well, and the second call only includes the
sorted label matchers but also depends on namespaceId and excludeDisabled.
Update the memoization key for the first memoizePromise call to include
baseSubgraphId, namespaceId, and excludeDisabled combined into a single key, and
update the second memoizePromise call's key to include the sorted label
matchers, namespaceId, and excludeDisabled combined into a single key. This
ensures cache entries are properly isolated by all query dimensions to prevent
returning stale data across different namespaces or filter modes.
In `@controlplane/src/core/services/CompositionService.ts`:
- Around line 839-850: The saveRouterConfigHash call on line 839 persists the
router configuration hash before validating the router compatibility version on
lines 842-850, which means if the validation fails, the database is left
pointing to a config path that was never uploaded. Move the validation logic
(the check for readyPathOverride obtained from getLatestPath) BEFORE the
saveRouterConfigHash call, so that the hash is only persisted when the router
compatibility version is confirmed to be valid. This ensures the database state
remains consistent with uploaded configurations.
---
Nitpick comments:
In `@controlplane/src/core/repositories/FeatureFlagRepository.ts`:
- Around line 68-72: The FeatureFlagCollectCaches is an exported object shape
currently defined as a type alias, but it should be converted to an interface to
align with TypeScript coding guidelines that prefer interfaces over type aliases
for object shapes. Change the declaration from using `export type
FeatureFlagCollectCaches = {` syntax to `export interface
FeatureFlagCollectCaches {` syntax, and remove the trailing semicolon after the
closing brace to match standard interface declaration patterns.
In `@controlplane/src/core/services/CompositionService.ts`:
- Around line 690-708: The variable contractRouterExecutionConfig is declared
without a type annotation, causing it to be inferred as any type. Add an
explicit type annotation to contractRouterExecutionConfig based on the return
type of the deserializeRouterExecutionConfig function. Determine the correct
type (likely RouterExecutionConfig or similar) from the function's return type
definition and use it in the let statement, optionally allowing for null or
undefined if the function can return those values.
🪄 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: b76b482f-e9e0-4101-805f-f58d43758640
📒 Files selected for processing (4)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraphs.tscontrolplane/src/core/repositories/FeatureFlagRepository.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/services/CompositionService.ts
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (84.30%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2969 +/- ##
===========================================
+ Coverage 42.77% 65.52% +22.74%
===========================================
Files 861 335 -526
Lines 124386 48671 -75715
Branches 9995 5431 -4564
===========================================
- Hits 53204 31890 -21314
+ Misses 70837 16755 -54082
+ Partials 345 26 -319
🚀 New features to boost your workflow:
|
…osition-pipeline-for-batch
…osition-pipeline-for-batch
…28-controlplane-parallelize-composition-pipeline-for-batch
…osition-pipeline-for-batch
JivusAyrus
deleted the
suvij/eng-9728-controlplane-parallelize-composition-pipeline-for-batch
branch
June 23, 2026 08:37
This was referenced Jun 29, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Checklist