feat: improve performance for subgraph batch publishing#2949
Merged
wilsonrivera merged 20 commits intoJun 10, 2026
Merged
Conversation
…composition-queries
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)
WalkthroughBulk subgraph lookup replaces per-item resolution in publishFederatedSubgraphs, publish authorization switches to RBAC span checks, SubgraphRepository gains batched name queries and concurrency improvements, and composition/schema-version repository methods run core DB operations inside explicit transactions. ChangesSubgraph Publishing Flow and Repository Data-Path Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraphs.ts (1)
117-160:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing subgraph detection is broken after bulk fetch refactor.
The old per-entry loop populated
notFoundwhen a subgraph didn't exist. The new code iterates over results fromgetSubgraphsByNames, which only returns existing subgraphs. If a requested subgraph doesn't exist in the database, it simply won't appear in the results—but nothing detects this.
notFoundis initialized at line 119 but never populated, making the check at line 149 dead code. Users who request a non-existent subgraph will silently succeed with a partial publish instead of receiving anERR_NOT_FOUNDerror.🐛 Proposed fix to detect missing subgraphs
// Resolve every requested subgraph; all of them must already exist. let now = performance.now(); const resolved: { subgraph: SubgraphDTO; schema: string }[] = []; const notFound: string[] = []; const typeErrors: string[] = []; - // for (const entry of requestedEntries) { - for (const subgraph of await subgraphRepo.getSubgraphsByNames(requestedEntries.map((e) => e.name), namespace.id)) { - // const subgraph = await subgraphRepo.byName(entry.name, req.namespace); - // if (!subgraph) { - // notFound.push(entry.name); - // continue; - // } + const requestedNames = requestedEntries.map((e) => e.name); + const subgraphsByName = new Map( + (await subgraphRepo.getSubgraphsByNames(requestedNames, namespace.id)).map((s) => [s.name, s]), + ); + + for (const entry of requestedEntries) { + const subgraph = subgraphsByName.get(entry.name); + if (!subgraph) { + notFound.push(entry.name); + continue; + } if (subgraph.type === 'grpc_plugin') { typeErrors.push( `Subgraph "${subgraph.name}" is a plugin. Please use the 'wgc router plugin publish' command to publish it.`, ); continue; } if (subgraph.type === 'grpc_service') { typeErrors.push( `Subgraph "${subgraph.name}" is a grpc service. Please use the 'wgc grpc-service publish' command to publish it.`, ); continue; } - const schema = requestedEntries.find((re) => re.name === subgraph.name)!.schema; - // const schema = entry.schema; + const schema = entry.schema; resolved.push({ subgraph, schema }); }🤖 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/bufservices/subgraph/publishFederatedSubgraphs.ts` around lines 117 - 160, The bulk-fetch removed detection of missing subgraphs; after calling subgraphRepo.getSubgraphsByNames(...) compute the set of fetched names and compare it to requestedEntries (use requestedEntries.map(e=>e.name)) to find any missing names, populate the existing notFound array with those missing names, and early-return the ERR_NOT_FOUND response as the code already expects; also ensure when building resolved you only call requestedEntries.find(...) for names that exist in the fetched results (or use a map from requested name to schema) so you don't rely on a non-null assertion.
🧹 Nitpick comments (1)
controlplane/src/core/repositories/SubgraphRepository.ts (1)
1199-1201: ⚡ Quick winUse linear-time deduplication in the hot path.
filter(...findIndex(...))is O(n²). This path is now used by bulk fetching, so switching to aSet/Map-based dedupe avoids quadratic growth.♻️ Suggested refactor
- return subgraphs - .filter((sg, index, self) => self.findIndex((x) => x.targetId === sg.targetId) === index) - .map((sg) => { + const uniqueByTargetId = new Map<string, (typeof subgraphs)[number]>(); + for (const sg of subgraphs) { + if (!uniqueByTargetId.has(sg.targetId)) { + uniqueByTargetId.set(sg.targetId, sg); + } + } + + return [...uniqueByTargetId.values()].map((sg) => {🤖 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/SubgraphRepository.ts` around lines 1199 - 1201, The current dedupe uses filter(...findIndex(...)) which is O(n²); replace it with a linear-time Set/Map-based approach: iterate over the subgraphs array once, track seen targetIds (e.g., const seen = new Set<string>() or a Map<string, Subgraph>), collect only the first occurrence per sg.targetId into a new array, then .map over that array; update the code around the subgraphs variable (the chain using .filter(...findIndex(...)).map(...)) so it uses the single-pass dedupe before mapping, preserving the original order and behavior.
🤖 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/bufservices/subgraph/publishFederatedSubgraphs.ts`:
- Around line 147-148: Replace all raw console.log timing instrumentation in
publishFederatedSubgraphs.ts (e.g., the statement at the shown diff and the
other occurrences around lines referenced) with calls to the module's structured
logger (logger), using template literals and including structured fields (e.g.,
elapsedMs: <value>, operation: 'loadSubgraphs' or other descriptive operation
names, and any request/context IDs available). Locate the timing calculations in
functions such as publishFederatedSubgraphs and the surrounding helper blocks
where performance.now() is used and replace console.log('load subgraphs: ' +
(performance.now() - now)) and similar prints with logger.debug or logger.info
as appropriate, passing both a template literal message and/or an object with
elapsedMs and context to preserve filtering and correlation. If the timing is
only temporary debug instrumentation and not needed, remove the statements
entirely instead of logging. Ensure all replacements use template literals
rather than string concatenation.
In `@controlplane/src/core/repositories/FederatedGraphRepository.ts`:
- Line 2: The file imports only KeyObject from 'node:crypto' but later calls
randomUUID(), causing an unresolved symbol; update the import to also bring in
randomUUID from 'node:crypto' (so the import line that currently imports
KeyObject in FederatedGraphRepository includes randomUUID) so calls to
randomUUID() in the FederatedGraphRepository class or related functions resolve
correctly.
In `@controlplane/src/core/repositories/SubgraphRepository.ts`:
- Around line 1092-1109: getSubgraphsByNames currently returns only existing
SubgraphDTOs which hides missing names; change it to preserve cardinality by
returning both found and missing names (e.g. { found: SubgraphDTO[], missing:
string[] }) so callers like publishFederatedSubgraphs can detect absent entries
and raise ERR_NOT_FOUND. Implement this by keeping the original uniqueNames
input list, collecting subgraphs as now via getSubgraphsMatching, then computing
missing = uniqueInputNames.filter(n => !found.map(s => s.name).includes(n));
update the method signature and all callers (notably publishFederatedSubgraphs)
to handle the new return shape and throw or abort when missing.length > 0.
---
Outside diff comments:
In `@controlplane/src/core/bufservices/subgraph/publishFederatedSubgraphs.ts`:
- Around line 117-160: The bulk-fetch removed detection of missing subgraphs;
after calling subgraphRepo.getSubgraphsByNames(...) compute the set of fetched
names and compare it to requestedEntries (use requestedEntries.map(e=>e.name))
to find any missing names, populate the existing notFound array with those
missing names, and early-return the ERR_NOT_FOUND response as the code already
expects; also ensure when building resolved you only call
requestedEntries.find(...) for names that exist in the fetched results (or use a
map from requested name to schema) so you don't rely on a non-null assertion.
---
Nitpick comments:
In `@controlplane/src/core/repositories/SubgraphRepository.ts`:
- Around line 1199-1201: The current dedupe uses filter(...findIndex(...)) which
is O(n²); replace it with a linear-time Set/Map-based approach: iterate over the
subgraphs array once, track seen targetIds (e.g., const seen = new Set<string>()
or a Map<string, Subgraph>), collect only the first occurrence per sg.targetId
into a new array, then .map over that array; update the code around the
subgraphs variable (the chain using .filter(...findIndex(...)).map(...)) so it
uses the single-pass dedupe before mapping, preserving the original order and
behavior.
🪄 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: c07a2be7-b663-4fb0-836d-a94a90855a30
📒 Files selected for processing (4)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraphs.tscontrolplane/src/core/repositories/FederatedGraphRepository.tscontrolplane/src/core/repositories/GraphCompositionRepository.tscontrolplane/src/core/repositories/SubgraphRepository.ts
wilsonrivera
changed the base branch from
main
to
wilson/eng-9669-controlplane-improve-composition-queries
June 10, 2026 15:51
…s' into wilson/eng-9716-controlplane-optimize-publishfederatedsubgraphs-performance # Conflicts: # controlplane/src/core/repositories/GraphCompositionRepository.ts # controlplane/src/core/repositories/SubgraphRepository.ts
Base automatically changed from
wilson/eng-9669-controlplane-improve-composition-queries
to
main
June 10, 2026 16:09
wilsonrivera
marked this pull request as ready for review
June 10, 2026 18:25
…hfederatedsubgraphs-performance # Conflicts: # controlplane/src/core/repositories/SubgraphRepository.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraphs.ts (1)
120-159:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCompute missing names from the fetched rows, not the type-filtered
resolvedlist.Entries rejected as
grpc_pluginorgrpc_serviceare skipped beforeresolvedSubgraphNamesis built, so existing subgraphs of those types get reported as missing and the method returnsERR_NOT_FOUNDbefore it ever surfacestypeErrors. Track the fetched names separately and use those for the missing-name check.💡 Suggested fix
- for (const subgraph of await subgraphRepo.getSubgraphsByNames( + const fetchedSubgraphs = await subgraphRepo.getSubgraphsByNames( requestedEntries.map((e) => e.name), namespace.id, - )) { + ); + const fetchedSubgraphNames = new Set(fetchedSubgraphs.map((sg) => sg.name)); + + for (const subgraph of fetchedSubgraphs) { if (subgraph.type === 'grpc_plugin') { typeErrors.push( `Subgraph "${subgraph.name}" is a plugin. Please use the 'wgc router plugin publish' command to publish it.`, @@ - const resolvedSubgraphNames = new Set(resolved.map((re) => re.subgraph.name)); const requestedSubgraphNames = new Set(requestedEntries.map((re) => re.name)); - const notFoundSubgraphNames = [...requestedSubgraphNames.difference(resolvedSubgraphNames)]; + const notFoundSubgraphNames = [...requestedSubgraphNames.difference(fetchedSubgraphNames)];🤖 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/bufservices/subgraph/publishFederatedSubgraphs.ts` around lines 120 - 159, The missing-name check uses resolvedSubgraphNames (which excludes entries filtered out as 'grpc_plugin' or 'grpc_service'), causing existing plugin/service subgraphs to be reported as missing; instead collect the fetched rows' names from the result of subgraphRepo.getSubgraphsByNames (e.g., build a fetchedSubgraphNames Set from the looped `subgraph` values) and compute notFoundSubgraphNames = requestedSubgraphNames.difference(fetchedSubgraphNames); keep the existing typeErrors accumulation and only use resolved (with schema) for later processing (resolved, resolvedSubgraphNames etc.).controlplane/src/core/repositories/SubgraphRepository.ts (1)
1119-1135:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign bulk name lookup with the caller's case-insensitive matching.
publishFederatedSubgraphsnow associates request entries withre.name.toLowerCase() === subgraph.name.toLowerCase(), but this helper still does an exacttargets.namelookup. A request forproductswill therefore still miss an existingProductssubgraph and come back asERR_NOT_FOUND, so the new case-insensitive publish path does not work end-to-end.🤖 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/SubgraphRepository.ts` around lines 1119 - 1135, getSubgraphsByNames currently does an exact match on schema.targets.name which breaks the new case-insensitive publish flow; change the lookup to compare lowercase names: transform chunkOfNames to all-lowercase and update the condition to use a lowercase DB-side expression (e.g. lower(schema.targets.name)) with inArray so the SQL compares lower(schema.targets.name) IN chunkOfNamesLowercase; keep other conditions and still call getSubgraphsMatching with the updated conditions (referencing getSubgraphsByNames, schema.targets.name, inArray, and getSubgraphsMatching).
🧹 Nitpick comments (2)
controlplane/src/core/repositories/GraphCompositionRepository.ts (1)
50-52: ⚡ Quick winDrop the
indexOflookups from subgraph diffing.
subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)]is justsubgraph.schemaVersionId, but it adds extra linear scans in both diff passes on the publish path.♻️ Proposed fix
- const subgraphSchemaVersionIds = composedSubgraphs.map((subgraph) => subgraph.schemaVersionId); - const updatedSubgraphs = composedSubgraphs.filter((subgraph) => { const prevSubgraph = prevCompositionSubgraphs.find((prevSubgraph) => prevSubgraph.id === subgraph.id); - return ( - prevSubgraph && prevSubgraph.schemaVersionId !== subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)] - ); + return prevSubgraph && prevSubgraph.schemaVersionId !== subgraph.schemaVersionId; }); const unchangedSubgraphs = composedSubgraphs.filter((subgraph) => prevCompositionSubgraphs.some( (prevSubgraph) => prevSubgraph.id === subgraph.id && - prevSubgraph.schemaVersionId === subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)], + prevSubgraph.schemaVersionId === subgraph.schemaVersionId, ), );Also applies to: 117-129
🤖 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/GraphCompositionRepository.ts` around lines 50 - 52, The diff uses composedSubgraphs.indexOf(subgraph) to look up schemaVersionIds (e.g., subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)]) which causes unnecessary O(n) scans; replace those indexOf-based lookups with direct access to subgraph.schemaVersionId wherever they occur (for example in the code paths around previousComposition and the second diff pass noted at lines ~117-129). Update all occurrences (composedSubgraphs.indexOf(subgraph) usages) in GraphCompositionRepository to use the subgraph object directly (subgraph.schemaVersionId) to eliminate extra linear scans.controlplane/src/core/repositories/FederatedGraphRepository.ts (1)
688-710: ⚡ Quick winAdd an explicit return type to this public method.
This signature just changed from returning a DTO to returning a narrower result object, so keeping it inferred makes the public contract easier to widen accidentally later.
♻️ Proposed fix
public async addSchemaVersion({ targetId, composedSDL, clientSchema, compositionErrors, compositionWarnings, composedSubgraphs, composedById, schemaVersionId, isFeatureFlagComposition, featureFlagId, }: { targetId: string; schemaVersionId: string; composedSDL?: string; clientSchema?: string; compositionErrors?: Error[]; compositionWarnings?: Warning[]; composedSubgraphs: CompositionSubgraphRecord[]; composedById: string; isFeatureFlagComposition: boolean; featureFlagId: string; - }) { + }): Promise<{ composedSchemaVersionId: string; routerCompatibilityVersion: string } | undefined> {As per coding guidelines,
**/*.{ts,tsx}: Use explicit type annotations for function parameters and return types 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/FederatedGraphRepository.ts` around lines 688 - 710, The public method addSchemaVersion in FederatedGraphRepository currently relies on inferred returns—add an explicit return type annotation (e.g., Promise<AddSchemaVersionResult> or the existing DTO type used elsewhere) to the method signature so the contract is fixed; locate the addSchemaVersion declaration and replace the inferred return with the appropriate exported type (Promise<...>), ensuring the chosen type matches the narrower result object now returned and update any imports/usages if necessary.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/GraphCompositionRepository.ts`:
- Around line 43-159: The addComposition() flow is not atomic: graphCompositions
is inserted separately from graphCompositionSubgraphs so a later failure can
leave inconsistent state (and FederatedGraphRepository.addSchemaVersion() may
have already advanced composedSchemaVersionId). Fix by making addComposition()
accept and use a transaction-scoped DB handle (or accept a Transaction/tx
parameter) and perform the insert into graphCompositions and the subsequent
insert into graphCompositionSubgraphs within the same transaction so both commit
or roll back together; update callers (e.g.,
FederatedGraphRepository.addSchemaVersion) to pass the transaction through and
use the transaction when calling addComposition().
In `@controlplane/src/core/repositories/SubgraphRepository.ts`:
- Around line 1159-1167: The helper getSubgraphNameByIds removes organization
scoping and needs to re-add the organization filter to prevent cross-tenant data
leaks: in the query that selects from targets and innerJoin(subgraphs...) add
the condition eq(targets.organizationId, this.organizationId) to the where
clause (alongside eq(targets.type, 'subgraph') and inArray(subgraphs.id,
chunkOfIds)), ensuring the method (getSubgraphNameByIds) only returns names for
the current organization when resolving via targets and subgraphs.
---
Outside diff comments:
In `@controlplane/src/core/bufservices/subgraph/publishFederatedSubgraphs.ts`:
- Around line 120-159: The missing-name check uses resolvedSubgraphNames (which
excludes entries filtered out as 'grpc_plugin' or 'grpc_service'), causing
existing plugin/service subgraphs to be reported as missing; instead collect the
fetched rows' names from the result of subgraphRepo.getSubgraphsByNames (e.g.,
build a fetchedSubgraphNames Set from the looped `subgraph` values) and compute
notFoundSubgraphNames = requestedSubgraphNames.difference(fetchedSubgraphNames);
keep the existing typeErrors accumulation and only use resolved (with schema)
for later processing (resolved, resolvedSubgraphNames etc.).
In `@controlplane/src/core/repositories/SubgraphRepository.ts`:
- Around line 1119-1135: getSubgraphsByNames currently does an exact match on
schema.targets.name which breaks the new case-insensitive publish flow; change
the lookup to compare lowercase names: transform chunkOfNames to all-lowercase
and update the condition to use a lowercase DB-side expression (e.g.
lower(schema.targets.name)) with inArray so the SQL compares
lower(schema.targets.name) IN chunkOfNamesLowercase; keep other conditions and
still call getSubgraphsMatching with the updated conditions (referencing
getSubgraphsByNames, schema.targets.name, inArray, and getSubgraphsMatching).
---
Nitpick comments:
In `@controlplane/src/core/repositories/FederatedGraphRepository.ts`:
- Around line 688-710: The public method addSchemaVersion in
FederatedGraphRepository currently relies on inferred returns—add an explicit
return type annotation (e.g., Promise<AddSchemaVersionResult> or the existing
DTO type used elsewhere) to the method signature so the contract is fixed;
locate the addSchemaVersion declaration and replace the inferred return with the
appropriate exported type (Promise<...>), ensuring the chosen type matches the
narrower result object now returned and update any imports/usages if necessary.
In `@controlplane/src/core/repositories/GraphCompositionRepository.ts`:
- Around line 50-52: The diff uses composedSubgraphs.indexOf(subgraph) to look
up schemaVersionIds (e.g.,
subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)]) which causes
unnecessary O(n) scans; replace those indexOf-based lookups with direct access
to subgraph.schemaVersionId wherever they occur (for example in the code paths
around previousComposition and the second diff pass noted at lines ~117-129).
Update all occurrences (composedSubgraphs.indexOf(subgraph) usages) in
GraphCompositionRepository to use the subgraph object directly
(subgraph.schemaVersionId) to eliminate extra linear scans.
🪄 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: 638623c9-7071-45cc-b62e-6f20c0c4901c
📒 Files selected for processing (7)
controlplane/src/core/bufservices/subgraph/publishFederatedSubgraphs.tscontrolplane/src/core/composition/composeGraphs.worker.tscontrolplane/src/core/repositories/FeatureFlagRepository.tscontrolplane/src/core/repositories/FederatedGraphRepository.tscontrolplane/src/core/repositories/GraphCompositionRepository.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/tracing.ts
✅ Files skipped from review due to trivial changes (2)
- controlplane/src/core/tracing.ts
- controlplane/src/core/composition/composeGraphs.worker.ts
thisisnithin
approved these changes
Jun 10, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
controlplane/src/core/repositories/GraphCompositionRepository.ts (1)
111-132: ⚡ Quick winSimplify schema version comparison by using the property directly.
subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)]is equivalent tosubgraph.schemaVersionIdsincesubgraphSchemaVersionIdsis derived fromcomposedSubgraphs.map(s => s.schemaVersionId). The index lookup adds unnecessary complexity and O(n) overhead per iteration.♻️ Proposed simplification
const updatedSubgraphs = composedSubgraphs.filter((subgraph) => { const prevSubgraph = prevCompositionSubgraphs.find((prevSubgraph) => prevSubgraph.id === subgraph.id); - return ( - prevSubgraph && - prevSubgraph.schemaVersionId !== subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)] - ); + return prevSubgraph && prevSubgraph.schemaVersionId !== subgraph.schemaVersionId; }); const unchangedSubgraphs = composedSubgraphs.filter((subgraph) => prevCompositionSubgraphs.some( (prevSubgraph) => - prevSubgraph.id === subgraph.id && - prevSubgraph.schemaVersionId === subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)], + prevSubgraph.id === subgraph.id && prevSubgraph.schemaVersionId === subgraph.schemaVersionId, ), );🤖 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/GraphCompositionRepository.ts` around lines 111 - 132, The comparison uses subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)] which is redundant and O(n) per check; update the filters in GraphCompositionRepository (the updatedSubgraphs and unchangedSubgraphs computations) to compare prevSubgraph.schemaVersionId directly to subgraph.schemaVersionId (e.g., use prevCompositionSubgraphs.find(...) and compare prevSubgraph.schemaVersionId !== subgraph.schemaVersionId for updated, and === for unchanged) to remove the indexOf lookups and simplify the logic.
🤖 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/FederatedGraphRepository.ts`:
- Around line 758-772: The code inserts a row into
federatedGraphsToFeatureFlagSchemaVersions using
federatedGraph.composedSchemaVersionId despite that field being nullable; add a
guard in the block where isFeatureFlagComposition is true to verify
federatedGraph.composedSchemaVersionId is present before calling tx.insert on
federatedGraphsToFeatureFlagSchemaVersions (or otherwise handle the missing base
composition by creating it or returning a controlled error), and if missing,
throw a clear error or create the base composition so
baseCompositionSchemaVersionId (target column) never receives null; update the
logic around isFeatureFlagComposition, federatedGraph.composedSchemaVersionId,
and the tx.insert(...) arguments accordingly.
---
Nitpick comments:
In `@controlplane/src/core/repositories/GraphCompositionRepository.ts`:
- Around line 111-132: The comparison uses
subgraphSchemaVersionIds[composedSubgraphs.indexOf(subgraph)] which is redundant
and O(n) per check; update the filters in GraphCompositionRepository (the
updatedSubgraphs and unchangedSubgraphs computations) to compare
prevSubgraph.schemaVersionId directly to subgraph.schemaVersionId (e.g., use
prevCompositionSubgraphs.find(...) and compare prevSubgraph.schemaVersionId !==
subgraph.schemaVersionId for updated, and === for unchanged) to remove the
indexOf lookups and simplify the 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: d49fd99a-b482-4034-916c-28128144c83a
📒 Files selected for processing (3)
controlplane/src/core/repositories/FederatedGraphRepository.tscontrolplane/src/core/repositories/GraphCompositionRepository.tscontrolplane/src/core/repositories/SubgraphRepository.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- controlplane/src/core/repositories/SubgraphRepository.ts
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2949 +/- ##
==========================================
+ Coverage 65.13% 65.56% +0.43%
==========================================
Files 327 327
Lines 47138 46918 -220
Branches 5241 5250 +9
==========================================
+ Hits 30703 30763 +60
+ Misses 16411 16131 -280
Partials 24 24
🚀 New features to boost your workflow:
|
wilsonrivera
deleted the
wilson/eng-9716-controlplane-optimize-publishfederatedsubgraphs-performance
branch
June 10, 2026 20:59
This was referenced Jun 12, 2026
6 tasks
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
Improvements
Bug Fixes
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.