Skip to content

refactor(controlplane): convert # private methods to TypeScript private in CompositionService#2935

Merged
Aenimus merged 2 commits into
mainfrom
private-methods-composition-service
Jun 9, 2026
Merged

refactor(controlplane): convert # private methods to TypeScript private in CompositionService#2935
Aenimus merged 2 commits into
mainfrom
private-methods-composition-service

Conversation

@gausie

@gausie gausie commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Converts all ECMAScript #private methods in CompositionService to TypeScript private methods
  • This makes them visible on the prototype at runtime, allowing the @traced class decorator to instrument them with Sentry spans
  • More files to follow as part of this initiative

Why

ECMAScript # private methods use slot-based access that bypasses prototype reflection, so @traced cannot see or wrap them. TypeScript private provides the same compile-time enforcement while remaining on the prototype at runtime.

Summary by CodeRabbit

  • Refactor
    • Internal refactoring of composition service implementation with no user-visible changes.

@gausie
gausie requested a review from a team as a code owner June 9, 2026 08:59
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR refactors CompositionService.ts by converting all internal methods from ECMAScript #private to TypeScript private, updating all call sites from #methodName to this.methodName syntax. The public API remains unchanged; only internal method definitions and their references are affected across composition, deployment, and feature-flag handling logic.

Changes

CompositionService Private Method Refactoring

Layer / File(s) Summary
Entry point routing to internal methods
controlplane/src/core/services/CompositionService.ts
Entry points composeAndDeployFederatedGraph, composeAndDeployFeatureFlag, deleteFeatureFlag, and recomposeAndDeployAffected now call internal methods via this.getOrganizationFeatures(), this.legacyComposeAndDeploy(), and this.deleteFeatureFlagConfigs() instead of # syntax, routing based on organization feature flags and split-config settings.
Organization feature gating and legacy composition helpers
controlplane/src/core/services/CompositionService.ts
Private methods getOrganizationFeatures(), legacyComposeAndDeploy(), and legacyComposeAndDeployFeatureFlag() are now defined with private syntax, implementing organization feature-flag gating logic and legacy composition orchestration via this.handleCompositionResultsAndDeploy() calls and manifest path helpers getManifestBasePath() and getLatestPath().
Composition result handling and deployment orchestration
controlplane/src/core/services/CompositionService.ts
Private method handleCompositionResult() collects composition errors and router-execution configs, while handleCompositionResultsAndDeploy() orchestrates per-graph/per-feature-flag composition and invokes this.deployGraph() and this.deployFeatureFlags() for deployment, replacing all #handleComposition* references throughout the orchestration loop.
Graph and feature-flag deployment implementation
controlplane/src/core/services/CompositionService.ts
Private methods deployGraph() and deployFeatureFlags() orchestrate schema/contract graph and feature-flag router config uploads to blob storage and call this.saveRouterConfigHash(), this.updateMapperForFederatedGraph(), and manifest path helpers getManifestBasePath() and getLatestPath() to determine CDN storage paths and persist router config state.
Mapper updates, feature-flag deletion, and router config persistence
controlplane/src/core/services/CompositionService.ts
Private helper methods updateMapperForFederatedGraph() updates CDN mapper state via getManifestBasePath(), deleteFeatureFlagConfigs() removes router configs from blob storage and updates mappers via getManifestBasePath() and updateMapperForFederatedGraph(), and saveRouterConfigHash() persists router config hashes to state via blob storage upserts.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: converting ECMAScript # private methods to TypeScript private methods in CompositionService, which aligns with the refactoring work shown in the summary.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Aenimus Aenimus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Aenimus
Aenimus enabled auto-merge (squash) June 9, 2026 09:02

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/services/CompositionService.ts (2)

976-994: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add explicit return type on deployGraph.

deployGraph is async and should declare Promise<void> explicitly to follow the TypeScript guideline.

Suggested patch
   private async deployGraph({
@@
-  }) {
+  }): Promise<void> {

As per coding guidelines, "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/services/CompositionService.ts` around lines 976 - 994,
The async method deployGraph lacks an explicit return type; update its signature
to declare Promise<void> (e.g., change "private async deployGraph({...}: {...})"
to "private async deployGraph(...): Promise<void>" ) so TypeScript has an
explicit return annotation; keep the existing parameter types and body unchanged
and ensure the compiler still type-checks with the new return type.

Source: Coding guidelines


480-492: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add explicit return type on legacyComposeAndDeployFeatureFlag.

This async method currently relies on inference; please annotate its return type explicitly for consistency with the TS guideline.

Suggested patch
   private async legacyComposeAndDeployFeatureFlag({
@@
-  }) {
+  }): Promise<ComposeAndDeployResult> {

As per coding guidelines, "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/services/CompositionService.ts` around lines 480 - 492,
Add an explicit return type to the async method
legacyComposeAndDeployFeatureFlag; update its signature to include the
appropriate Promise return type (e.g., Promise<void> if it does not return a
value) so the function no longer relies on inferred types—modify the declaration
of legacyComposeAndDeployFeatureFlag to append the explicit return type after
the parameter list while keeping the existing parameter types (actorId,
featureFlag, enabled, orgFeatures, prevFederatedGraphs).

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.

Outside diff comments:
In `@controlplane/src/core/services/CompositionService.ts`:
- Around line 976-994: The async method deployGraph lacks an explicit return
type; update its signature to declare Promise<void> (e.g., change "private async
deployGraph({...}: {...})" to "private async deployGraph(...): Promise<void>" )
so TypeScript has an explicit return annotation; keep the existing parameter
types and body unchanged and ensure the compiler still type-checks with the new
return type.
- Around line 480-492: Add an explicit return type to the async method
legacyComposeAndDeployFeatureFlag; update its signature to include the
appropriate Promise return type (e.g., Promise<void> if it does not return a
value) so the function no longer relies on inferred types—modify the declaration
of legacyComposeAndDeployFeatureFlag to append the explicit return type after
the parameter list while keeping the existing parameter types (actorId,
featureFlag, enabled, orgFeatures, prevFederatedGraphs).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 31f44717-bd00-4d1e-a903-b004d3fd8849

📥 Commits

Reviewing files that changed from the base of the PR and between 76a1bbc and 9f538a8.

📒 Files selected for processing (1)
  • controlplane/src/core/services/CompositionService.ts

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.61905% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 65.13%. Comparing base (ccbcb76) to head (df0041f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ntrolplane/src/core/services/CompositionService.ts 97.61% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2935       +/-   ##
===========================================
+ Coverage   41.27%   65.13%   +23.86%     
===========================================
  Files        1045      327      -718     
  Lines      132938    47127    -85811     
  Branches     6359     5239     -1120     
===========================================
- Hits        54867    30697    -24170     
+ Misses      76298    16406    -59892     
+ Partials     1773       24     -1749     
Files with missing lines Coverage Δ
...ntrolplane/src/core/services/CompositionService.ts 90.76% <97.61%> (-0.10%) ⬇️

... and 719 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Aenimus
Aenimus merged commit 9224bc0 into main Jun 9, 2026
12 checks passed
@Aenimus
Aenimus deleted the private-methods-composition-service branch June 9, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants