Extract GenerationOrchestrator from GenerateCommand#1128
Merged
Conversation
Extract core orchestration logic into a dedicated GenerationOrchestrator class with IGenerationOrchestrator interface. GenerateCommand.ExecuteAsync now delegates to the orchestrator, reducing from ~130 lines to ~30 lines. - Create IGenerationOrchestrator interface with RunAsync() method - Create GenerationOrchestrator implementing the full generation pipeline - Make IGenerationReporter and IMultiFileOutputReport public for wider use - Remove WriteSingleFile, WriteMultipleFiles, FormatFileSize, ShowWarnings, ValidateOpenApiSpec from GenerateCommand (moved to orchestrator) - All 1958 existing tests pass without modification
- Test successful single-file generation - Test successful multi-file generation - Test error handling returns non-zero exit code
Contributor
📝 WalkthroughWalkthroughThis PR extracts the orchestration logic from ChangesOrchestrator Extraction and Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 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
🧪 Generate unit tests (beta)
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. 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
🧹 Nitpick comments (1)
src/Refitter.Tests/GenerationOrchestratorTests.cs (1)
23-38: ⚡ Quick winExtract the repeated OpenAPI payload into a shared test constant/helper.
The same inline spec shape is repeated across tests; centralizing it into constants/helpers aligns the suite pattern and reduces maintenance drift.
As per coding guidelines, "Follow the unit testing pattern with OpenAPI specification constants,
[Fact]decorated test methods, FluentAssertions for validation, andBuildHelper.BuildCSharp()for compilation verification in test classes".Also applies to: 88-104
🤖 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 `@src/Refitter.Tests/GenerationOrchestratorTests.cs` around lines 23 - 38, Extract the repeated OpenAPI JSON blob used in GenerationOrchestratorTests into a shared constant or helper (e.g., add a public const string TestOpenApiSpec or static readonly string OpenApiSpec in GenerationOrchestratorTests or a TestConstants helper), then replace the inline triple-quoted payload passed to File.WriteAllText(openApiPath, """...""") with that constant; update all other occurrences of the same inline spec (the other span noted in the comment) to use the new constant/helper so tests still call BuildHelper.BuildCSharp(), remain decorated with [Fact], and validate with FluentAssertions without duplicating the spec.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 `@src/Refitter.Tests/GenerationOrchestratorTests.cs`:
- Around line 61-65: Tests in GenerationOrchestratorTests.cs only assert
generatedCode contains strings but do not verify it compiles; update the
success-path tests so that after reading generatedCode you call
BuildHelper.BuildCSharp(generatedCode) (or the overload used across tests) and
assert the build succeeded (e.g., no diagnostics/error and result is true) using
FluentAssertions. Modify the test cases that set and read the generatedCode
variable in the GenerationOrchestratorTests class (apply the same change to the
other success-path test mentioned) to include constants/OpenAPI spec usage as
needed and a final BuildHelper.BuildCSharp(...) call with an assertion to ensure
the emitted C# compiles.
In `@src/Refitter/GenerationOrchestrator.cs`:
- Around line 10-14: In RunAsync (GenerationOrchestrator.cs file lines 10-14)
ensure the incoming CancellationToken is enforced at stage boundaries by
invoking cancellationToken.ThrowIfCancellationRequested() (or equivalent checks)
before beginning each major generation phase inside RunAsync; in the same file
for the multi-file write loop (lines 125-141) add a cancellation check before
each file write iteration and pass the token into any async I/O calls used there
so writes can be aborted promptly; in GenerateCommand.cs (lines 67-68) propagate
the token by calling File.ReadAllTextAsync(..., cancellationToken) when loading
settings so the file read honors cancellation.
In `@src/Refitter/IGenerationOrchestrator.cs`:
- Around line 5-11: Add missing XML documentation for the newly public API
surfaces: in src/Refitter/IGenerationOrchestrator.cs (lines 5-11) add a summary
on the IGenerationOrchestrator interface and XML docs for the RunAsync method
describing its purpose, each parameter (settings: RefitGeneratorSettings,
cliSettings: Settings, reporter: IGenerationReporter, cancellationToken:
CancellationToken) and the Task<int> return value; in
src/Refitter/IGenerationReporter.cs (lines 14-65) add XML summary/comments for
the IGenerationReporter interface and every public member there explaining
intent, parameters and return values; in src/Refitter/IGenerationReporter.cs
(lines 72-77) add XML docs for the IMultiFileOutputReport members (summaries and
parameter/return descriptions); and in src/Refitter/GenerationOrchestrator.cs
(lines 8-14) add XML docs for the public GenerationOrchestrator class and its
RunAsync method mirroring the interface contract. Ensure tags use <summary>,
<param>, and <returns> consistently and accurately reference the method and
parameter names.
---
Nitpick comments:
In `@src/Refitter.Tests/GenerationOrchestratorTests.cs`:
- Around line 23-38: Extract the repeated OpenAPI JSON blob used in
GenerationOrchestratorTests into a shared constant or helper (e.g., add a public
const string TestOpenApiSpec or static readonly string OpenApiSpec in
GenerationOrchestratorTests or a TestConstants helper), then replace the inline
triple-quoted payload passed to File.WriteAllText(openApiPath, """...""") with
that constant; update all other occurrences of the same inline spec (the other
span noted in the comment) to use the new constant/helper so tests still call
BuildHelper.BuildCSharp(), remain decorated with [Fact], and validate with
FluentAssertions without duplicating the spec.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9db15cb9-369c-441d-a686-40b8534033fd
📒 Files selected for processing (5)
src/Refitter.Tests/GenerationOrchestratorTests.cssrc/Refitter/GenerateCommand.cssrc/Refitter/GenerationOrchestrator.cssrc/Refitter/IGenerationOrchestrator.cssrc/Refitter/IGenerationReporter.cs
Comment on lines
+61
to
+65
| result.Should().Be(0); | ||
| File.Exists(outputPath).Should().BeTrue(); | ||
| var generatedCode = await File.ReadAllTextAsync(outputPath); | ||
| generatedCode.Should().Contain("TestNamespace"); | ||
| generatedCode.Should().Contain("GetPets"); |
Contributor
There was a problem hiding this comment.
Add generated-code compilation checks in the success-path tests.
The tests validate text content but do not verify the generated C# compiles; this misses regressions that produce syntactically invalid output.
As per coding guidelines, "Follow the unit testing pattern with OpenAPI specification constants, [Fact] decorated test methods, FluentAssertions for validation, and BuildHelper.BuildCSharp() for compilation verification in test classes".
Also applies to: 129-134
🤖 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 `@src/Refitter.Tests/GenerationOrchestratorTests.cs` around lines 61 - 65,
Tests in GenerationOrchestratorTests.cs only assert generatedCode contains
strings but do not verify it compiles; update the success-path tests so that
after reading generatedCode you call BuildHelper.BuildCSharp(generatedCode) (or
the overload used across tests) and assert the build succeeded (e.g., no
diagnostics/error and result is true) using FluentAssertions. Modify the test
cases that set and read the generatedCode variable in the
GenerationOrchestratorTests class (apply the same change to the other
success-path test mentioned) to include constants/OpenAPI spec usage as needed
and a final BuildHelper.BuildCSharp(...) call with an assertion to ensure the
emitted C# compiles.
Source: Coding guidelines
Comment on lines
+10
to
+14
| public async Task<int> RunAsync( | ||
| RefitGeneratorSettings refitGeneratorSettings, | ||
| Settings cliSettings, | ||
| IGenerationReporter reporter, | ||
| CancellationToken cancellationToken) |
Contributor
There was a problem hiding this comment.
Cancellation token propagation is incomplete across the generation flow. The token is accepted at the API boundary but not consistently applied where work can block for meaningful durations.
src/Refitter/GenerationOrchestrator.cs#L10-L14: enforce cancellation checks at stage boundaries inRunAsync.src/Refitter/GenerationOrchestrator.cs#L125-L141: check cancellation inside the multi-file write loop before each write.src/Refitter/GenerateCommand.cs#L67-L68: passcancellationTokentoFile.ReadAllTextAsync(...)when loading settings.
📍 Affects 2 files
src/Refitter/GenerationOrchestrator.cs#L10-L14(this comment)src/Refitter/GenerationOrchestrator.cs#L125-L141src/Refitter/GenerateCommand.cs#L67-L68
🤖 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 `@src/Refitter/GenerationOrchestrator.cs` around lines 10 - 14, In RunAsync
(GenerationOrchestrator.cs file lines 10-14) ensure the incoming
CancellationToken is enforced at stage boundaries by invoking
cancellationToken.ThrowIfCancellationRequested() (or equivalent checks) before
beginning each major generation phase inside RunAsync; in the same file for the
multi-file write loop (lines 125-141) add a cancellation check before each file
write iteration and pass the token into any async I/O calls used there so writes
can be aborted promptly; in GenerateCommand.cs (lines 67-68) propagate the token
by calling File.ReadAllTextAsync(..., cancellationToken) when loading settings
so the file read honors cancellation.
Comment on lines
+5
to
+11
| public interface IGenerationOrchestrator | ||
| { | ||
| Task<int> RunAsync( | ||
| RefitGeneratorSettings settings, | ||
| Settings cliSettings, | ||
| IGenerationReporter reporter, | ||
| CancellationToken cancellationToken); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Public API expansion is missing full XML documentation across the new seams. This PR makes several types/members public, but the exposed API surface is not consistently documented.
src/Refitter/IGenerationOrchestrator.cs#L5-L11: add XML docs for the interface andRunAsynccontract/parameters/return value.src/Refitter/IGenerationReporter.cs#L14-L65: add XML docs for all public reporter members now exposed as API.src/Refitter/IGenerationReporter.cs#L72-L77: add XML docs forIMultiFileOutputReportmembers.src/Refitter/GenerationOrchestrator.cs#L8-L14: add XML docs for the public class and publicRunAsyncmethod.
As per coding guidelines, "Include XML documentation for public APIs in C# code".
📍 Affects 3 files
src/Refitter/IGenerationOrchestrator.cs#L5-L11(this comment)src/Refitter/IGenerationReporter.cs#L14-L65src/Refitter/IGenerationReporter.cs#L72-L77src/Refitter/GenerationOrchestrator.cs#L8-L14
🤖 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 `@src/Refitter/IGenerationOrchestrator.cs` around lines 5 - 11, Add missing XML
documentation for the newly public API surfaces: in
src/Refitter/IGenerationOrchestrator.cs (lines 5-11) add a summary on the
IGenerationOrchestrator interface and XML docs for the RunAsync method
describing its purpose, each parameter (settings: RefitGeneratorSettings,
cliSettings: Settings, reporter: IGenerationReporter, cancellationToken:
CancellationToken) and the Task<int> return value; in
src/Refitter/IGenerationReporter.cs (lines 14-65) add XML summary/comments for
the IGenerationReporter interface and every public member there explaining
intent, parameters and return values; in src/Refitter/IGenerationReporter.cs
(lines 72-77) add XML docs for the IMultiFileOutputReport members (summaries and
parameter/return descriptions); and in src/Refitter/GenerationOrchestrator.cs
(lines 8-14) add XML docs for the public GenerationOrchestrator class and its
RunAsync method mirroring the interface contract. Ensure tags use <summary>,
<param>, and <returns> consistently and accurately reference the method and
parameter names.
Source: Coding guidelines
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1128 +/- ##
==========================================
- Coverage 86.27% 83.98% -2.30%
==========================================
Files 40 40
Lines 2551 2628 +77
==========================================
+ Hits 2201 2207 +6
- Misses 260 323 +63
- Partials 90 98 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Jun 13, 2026
Merged
Merged
This was referenced Jun 20, 2026
This was referenced Jul 12, 2026
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
1 participant
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.
Closes #1117
Extract core orchestration logic into a dedicated GenerationOrchestrator
class with IGenerationOrchestrator interface. GenerateCommand.ExecuteAsync
now delegates to the orchestrator, reducing from ~130 lines to ~30 lines.
Summary by CodeRabbit
Release Notes
New Features
IGenerationReporterandIMultiFileOutputReportpublic interfaces for library consumers who need to extend or customize code generation reporting.Refactor
Tests