refactor(LLMs): improve generator tools #889
Conversation
Reviewer's GuideRefactors the LLM docs generator into a simpler, static, root-folder–driven tool with a new namespace, removes incremental/checking logic and CLI options, and centralizes argument parsing and execution through a Runner and ArgumentsHelper while making Markdown generation static and allocation-local. Sequence diagram for new CLI execution flow of the docs generatorsequenceDiagram
actor Developer
participant Program
participant Runner
participant ArgumentsHelper
participant RootCommand
participant DocsGenerator
participant ComponentAnalyzer
participant MarkdownBuilder
participant FileSystem
Developer->>Program: llms-docs-generator --root path
Program->>Runner: Run(args)
activate Runner
Runner->>ArgumentsHelper: Parse(args)
activate ArgumentsHelper
ArgumentsHelper->>RootCommand: configure options and action
ArgumentsHelper-->>Runner: ParseResult
deactivate ArgumentsHelper
Runner->>RootCommand: InvokeAsync()
activate RootCommand
RootCommand->>DocsGenerator: GenerateAllAsync(rootFolder)
deactivate RootCommand
activate DocsGenerator
DocsGenerator->>FileSystem: build _sourcePath, _outputPath
DocsGenerator->>FileSystem: Directory.Exists(_sourcePath)?
alt source path exists
DocsGenerator->>FileSystem: CreateDirectory(_outputPath)
DocsGenerator->>FileSystem: CreateDirectory(_componentsOutputPath)
DocsGenerator->>ComponentAnalyzer: AnalyzeAllComponentsAsync()
activate ComponentAnalyzer
ComponentAnalyzer-->>DocsGenerator: List<ComponentInfo>
deactivate ComponentAnalyzer
DocsGenerator->>MarkdownBuilder: BuildIndexDoc(components)
activate MarkdownBuilder
MarkdownBuilder-->>DocsGenerator: indexContent
deactivate MarkdownBuilder
DocsGenerator->>FileSystem: WriteAllTextAsync(llms.txt, indexContent)
loop for each component
DocsGenerator->>MarkdownBuilder: BuildComponentDoc(component)
activate MarkdownBuilder
MarkdownBuilder-->>DocsGenerator: componentContent
deactivate MarkdownBuilder
DocsGenerator->>FileSystem: WriteAllTextAsync(componentFile, componentContent)
end
else source path missing
DocsGenerator-->>Runner: return (no files generated)
end
deactivate DocsGenerator
Runner-->>Program: completed
deactivate Runner
Program-->>Developer: process exit
Class diagram for refactored LLMs docs generator toolclassDiagram
direction LR
class Runner {
<<static>>
+Task Run(string[] args)
}
class ArgumentsHelper {
<<static>>
+ParseResult Parse(string[] args)
}
class DocsGenerator {
<<static>>
+Task GenerateAllAsync(string rootFolder)
-static void Logger(string message)
}
class MarkdownBuilder {
<<static>>
-const string GitHubBaseUrl
+string BuildIndexDoc(List~ComponentInfo~ components)
+string BuildComponentDoc(ComponentInfo component)
-static void BuildComponentSection(StringBuilder sb, ComponentInfo component, bool includeHeader)
-static string EscapeMarkdownCell(string text)
-static string EscapeMarkdownCode(string code)
-static string TruncateSummary(string summary, int maxLength)
}
class ComponentAnalyzer {
+Task List~ComponentInfo~ AnalyzeAllComponentsAsync()
+Task ComponentInfo AnalyzeComponentAsync(string componentName)
}
class ComponentInfo {
+string Name
+string FilePath
+DateTime LastModified
+string Summary
+List~ParameterInfo~ Parameters
+List~EventInfo~ Events
}
class ParseResult {
+Task InvokeAsync()
}
class RootCommand
class Option
Runner ..> ArgumentsHelper : uses
Runner ..> ParseResult : receives
ArgumentsHelper ..> RootCommand : configures
ArgumentsHelper ..> Option : defines
ArgumentsHelper ..> DocsGenerator : sets action
DocsGenerator ..> ComponentAnalyzer : creates and uses
DocsGenerator ..> MarkdownBuilder : uses
ComponentAnalyzer ..> ComponentInfo : produces
MarkdownBuilder ..> ComponentInfo : reads
NoteForStaticClasses <|.. Runner : static
NoteForStaticClasses <|.. ArgumentsHelper : static
NoteForStaticClasses <|.. DocsGenerator : static
NoteForStaticClasses <|.. MarkdownBuilder : static
class NoteForStaticClasses {
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The new
ArgumentsHelper.Parsepath handling is a bit opaque:--rootis required but silently ignored when missing, andDocsGenerator.GenerateAllAsyncassumesrootFolderis the server project directory; consider validating the argument and failing fast with a clear error if it’s null/invalid so callers know why nothing was generated. - With
DocsGenerator.Loggernow always writing toConsole, the tool will be much noisier than before (no--debugflag); consider either reintroducing a verbosity switch or routing logs through a configurable logger to avoid excessive output in automated environments. - In
MarkdownBuilder, the commented-outBuildParameterTablemethod is now dead code; if it’s not planned for near-term use, removing it (or moving it to a separate utility) would keep the helper cleaner and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `ArgumentsHelper.Parse` path handling is a bit opaque: `--root` is required but silently ignored when missing, and `DocsGenerator.GenerateAllAsync` assumes `rootFolder` is the server project directory; consider validating the argument and failing fast with a clear error if it’s null/invalid so callers know why nothing was generated.
- With `DocsGenerator.Logger` now always writing to `Console`, the tool will be much noisier than before (no `--debug` flag); consider either reintroducing a verbosity switch or routing logs through a configurable logger to avoid excessive output in automated environments.
- In `MarkdownBuilder`, the commented-out `BuildParameterTable` method is now dead code; if it’s not planned for near-term use, removing it (or moving it to a separate utility) would keep the helper cleaner and easier to maintain.
## Individual Comments
### Comment 1
<location> `tools/BootstrapBlazor.LLMsDocsGenerator/Runner.cs:13` </location>
<code_context>
+ {
+ var result = ArgumentsHelper.Parse(args);
+
+ await result.InvokeAsync();
+ }
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** The process exit code from System.CommandLine is ignored, which can hide failures from callers/CI.
Previously `Program.cs` returned `await rootCommand.Parse(args).InvokeAsync();`, so the process exit code reflected the command result. Now `Runner.Run` awaits `result.InvokeAsync()` but discards the `int`, so the process always exits with 0 and CI/scripts can’t detect failures. Please capture the return value and either set `Environment.ExitCode` or return it so callers can observe failures.
</issue_to_address>
### Comment 2
<location> `tools/BootstrapBlazor.LLMsDocsGenerator/DocsGenerator.cs:23-25` </location>
<code_context>
+ if (!Directory.Exists(_sourcePath))
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Silently returning when the source path is missing makes it hard to diagnose misconfiguration.
If `_sourcePath` doesn’t exist, `GenerateAllAsync` effectively no‑ops with no log or non‑zero exit code, so misconfigured `--root` values look like successful runs. Consider logging a clear error and, if appropriate, signalling failure (exception or non‑zero exit code) instead of returning silently.
Suggested implementation:
```csharp
if (!Directory.Exists(_sourcePath))
{
// Misconfigured --root should be treated as a failure, not a silent no-op
Console.Error.WriteLine($"[DocsGenerator] Source path '{_sourcePath}' does not exist. Check the --root configuration.");
throw new DirectoryNotFoundException($"Source path '{_sourcePath}' does not exist.");
```
Depending on how `GenerateAllAsync` is currently used, you may want to:
1. Replace the `throw` with returning a non-zero exit code if `GenerateAllAsync` is part of your `Main`/CLI pipeline and already uses exit codes.
2. If there is an existing logging abstraction (e.g., `ILogger<DocsGenerator>` or a custom logger), replace `Console.Error.WriteLine` with that logger to be consistent with the rest of the codebase.
3. Ensure the caller of `GenerateAllAsync` either lets the exception bubble up to the process boundary (so the process exits non-zero) or catches it and converts it into an appropriate exit code.
</issue_to_address>
### Comment 3
<location> `tools/BootstrapBlazor.LLMsDocsGenerator/ArgumentsHelper.cs:23-29` </location>
<code_context>
+
+ rootCommand.SetAction(async result =>
+ {
+ var rootFolder = result.GetValue(rootFolderOption);
+ if (string.IsNullOrEmpty(rootFolder))
+ {
+ return;
+ }
+
+ await DocsGenerator.GenerateAllAsync(rootFolder);
+ });
+
</code_context>
<issue_to_address>
**suggestion:** Handling of missing `--root` is very silent and may confuse users.
Right now, omitting `--root` makes the command exit silently. Since `rootFolder` is required for any useful work, consider either marking the option as required or displaying usage/help or an explicit error when it’s missing so users understand why nothing happened.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| { | ||
| var result = ArgumentsHelper.Parse(args); | ||
|
|
||
| await result.InvokeAsync(); |
There was a problem hiding this comment.
issue (bug_risk): The process exit code from System.CommandLine is ignored, which can hide failures from callers/CI.
Previously Program.cs returned await rootCommand.Parse(args).InvokeAsync();, so the process exit code reflected the command result. Now Runner.Run awaits result.InvokeAsync() but discards the int, so the process always exits with 0 and CI/scripts can’t detect failures. Please capture the return value and either set Environment.ExitCode or return it so callers can observe failures.
| var rootFolder = result.GetValue(rootFolderOption); | ||
| if (string.IsNullOrEmpty(rootFolder)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| await DocsGenerator.GenerateAllAsync(rootFolder); |
There was a problem hiding this comment.
suggestion: Handling of missing --root is very silent and may confuse users.
Right now, omitting --root makes the command exit silently. Since rootFolder is required for any useful work, consider either marking the option as required or displaying usage/help or an explicit error when it’s missing so users understand why nothing happened.
There was a problem hiding this comment.
Pull request overview
This PR refactors the LLMs documentation generator tool by simplifying its architecture and removing several command-line options that were previously available. The refactoring consolidates the code into a more streamlined structure.
Key changes:
- Simplified command-line interface by removing
--component,--index-only,--check, and--debugoptions, keeping only--root - Converted instance-based classes to static classes for simpler architecture
- Removed dynamic source path discovery and replaced with hard-coded path patterns
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| Runner.cs | New entry point that delegates to ArgumentsHelper for parsing and execution |
| ArgumentsHelper.cs | New file that handles command-line argument parsing with simplified options |
| Program.cs | Simplified to a single line that calls Runner.Run |
| DocsGenerator.cs | Converted to static class, removed CheckAsync and component-specific generation methods, hard-coded output paths |
| MarkdownBuilder.cs | Converted from instance to static class, renamed BuildIndex to BuildIndexDoc, commented out BuildParameterTable method |
| ComponentInfo.cs | Namespace change only |
| ComponentAnalyzer.cs | Namespace change only |
| BootstrapBlazor.LLMsDocsGenerator.csproj | Version bump to 10.0.1 and added SatelliteResourceLanguages property |
Comments suppressed due to low confidence (4)
tools/BootstrapBlazor.LLMsDocsGenerator/ComponentAnalyzer.cs:242
- This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
foreach (var method in methodDeclarations)
{
// Skip property accessors, overrides of base class methods
if (method.Modifiers.Any(m => m.IsKind(SyntaxKind.OverrideKeyword)))
continue;
var methodInfo = new MethodInfo
{
Name = method.Identifier.Text,
ReturnType = SimplifyTypeName(method.ReturnType.ToString()),
Description = ExtractXmlSummary(method),
IsJSInvokable = HasAttribute(method, "JSInvokable"),
Parameters = method.ParameterList.Parameters
.Select(p => (SimplifyTypeName(p.Type?.ToString() ?? ""), p.Identifier.Text))
.ToList()
};
methods.Add(methodInfo);
}
tools/BootstrapBlazor.LLMsDocsGenerator/ComponentAnalyzer.cs:52
- This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
foreach (var file in files)
{
var component = await AnalyzeFileAsync(file);
if (component != null && component.Parameters.Count > 0)
{
components.Add(component);
}
}
tools/BootstrapBlazor.LLMsDocsGenerator/ComponentAnalyzer.cs:63
- This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
foreach (var file in csFiles)
{
var component = await AnalyzeFileAsync(file);
if (component != null && component.Parameters.Count > 0)
{
components.Add(component);
}
}
tools/BootstrapBlazor.LLMsDocsGenerator/ComponentAnalyzer.cs:151
- Redundant call to 'ToString' on a String object.
className += classDeclaration.TypeParameterList.ToString();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Maintainer: Argo Zhang([email protected]) Website: https://www.blazor.zone | ||
|
|
||
| namespace LlmsDocsGenerator; | ||
| namespace BootstrapBlazorLLMsDocsGenerator; |
There was a problem hiding this comment.
There is a namespace inconsistency in this file. Line 6 declares the namespace as BootstrapBlazorLLMsDocsGenerator (without dots), but the proper format used in ArgumentsHelper.cs line 8 is BootstrapBlazor.LLMsDocsGenerator (with dots). All files should use a consistent namespace pattern to avoid confusion and potential compilation issues.
| namespace BootstrapBlazorLLMsDocsGenerator; | |
| namespace BootstrapBlazor.LLMsDocsGenerator; |
| // Maintainer: Argo Zhang([email protected]) Website: https://www.blazor.zone | ||
|
|
||
| namespace LlmsDocsGenerator; | ||
| namespace BootstrapBlazorLLMsDocsGenerator; |
There was a problem hiding this comment.
There is a namespace inconsistency in this file. Line 6 declares the namespace as BootstrapBlazorLLMsDocsGenerator (without dots), but the proper format used in ArgumentsHelper.cs line 8 is BootstrapBlazor.LLMsDocsGenerator (with dots). All files should use a consistent namespace pattern to avoid confusion and potential compilation issues.
| namespace BootstrapBlazorLLMsDocsGenerator; | |
| namespace BootstrapBlazor.LLMsDocsGenerator; |
| using BootstrapBlazorLLMsDocsGenerator; | ||
| using System.CommandLine; | ||
|
|
||
| namespace BootstrapBlazor.LLMsDocsGenerator; |
There was a problem hiding this comment.
There's a namespace inconsistency: line 5 uses BootstrapBlazorLLMsDocsGenerator (no dots between words) while line 8 declares the namespace as BootstrapBlazor.LLMsDocsGenerator (with dots). This mismatch will cause a compilation error. The using statement should match the actual namespace of the DocsGenerator class, which appears to be BootstrapBlazorLLMsDocsGenerator based on DocsGenerator.cs line 6.
| namespace BootstrapBlazor.LLMsDocsGenerator; | |
| namespace BootstrapBlazorLLMsDocsGenerator; |
| public static async Task GenerateAllAsync(string rootFolder) | ||
| { | ||
| var _sourcePath = Path.Combine(rootFolder, "..", "BootstrapBlazor"); | ||
| var _outputPath = Path.Combine(rootFolder, "bin", "Release", "net10.0", "publish", "wwwroot", "llms"); |
There was a problem hiding this comment.
The target framework version "net10.0" is hard-coded in the output path. This makes the tool brittle - if the framework version changes or if the tool needs to support multiple framework versions, this path will break. Consider making this configurable or dynamically detecting the framework version.
| var _outputPath = Path.Combine(rootFolder, "bin", "Release", "net10.0", "publish", "wwwroot", "llms"); | |
| var releaseDir = Path.Combine(rootFolder, "bin", "Release"); | |
| string targetFrameworkDir; | |
| if (Directory.Exists(releaseDir)) | |
| { | |
| targetFrameworkDir = Path.Combine(releaseDir, "net10.0"); | |
| foreach (var dir in Directory.GetDirectories(releaseDir)) | |
| { | |
| var frameworkName = Path.GetFileName(dir); | |
| if (frameworkName.StartsWith("net", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| targetFrameworkDir = dir; | |
| break; | |
| } | |
| } | |
| } | |
| else | |
| { | |
| targetFrameworkDir = Path.Combine(releaseDir, "net10.0"); | |
| } | |
| var _outputPath = Path.Combine(targetFrameworkDir, "publish", "wwwroot", "llms"); |
| using System.Text; | ||
|
|
||
| namespace LlmsDocsGenerator; | ||
| namespace BootstrapBlazorLLMsDocsGenerator; |
There was a problem hiding this comment.
There is a namespace inconsistency in this file. Line 8 declares the namespace as BootstrapBlazorLLMsDocsGenerator (without dots), but the proper format used in other files like ArgumentsHelper.cs line 8 is BootstrapBlazor.LLMsDocsGenerator (with dots). This inconsistency will cause namespace resolution issues. All namespaces should follow a consistent pattern.
| namespace BootstrapBlazorLLMsDocsGenerator; | |
| namespace BootstrapBlazor.LLMsDocsGenerator; |
| <TargetFramework>net10.0</TargetFramework> | ||
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <Nullable>enable</Nullable> | ||
| <SatelliteResourceLanguages>false</SatelliteResourceLanguages> |
There was a problem hiding this comment.
The SatelliteResourceLanguages property should be a semicolon-separated list of language codes (e.g., "en;zh") or an empty string, not a boolean value. Setting it to "false" is invalid. If the intention is to disable satellite resource assembly generation, the correct value should be an empty string or the property should be removed entirely.
| <SatelliteResourceLanguages>false</SatelliteResourceLanguages> | |
| <SatelliteResourceLanguages></SatelliteResourceLanguages> |
| using System.Text.RegularExpressions; | ||
|
|
||
| namespace LlmsDocsGenerator; | ||
| namespace BootstrapBlazorLLMsDocsGenerator; |
There was a problem hiding this comment.
There is a namespace inconsistency in this file. Line 11 declares the namespace as BootstrapBlazorLLMsDocsGenerator (without dots), but the proper format used in ArgumentsHelper.cs line 8 is BootstrapBlazor.LLMsDocsGenerator (with dots). All files should use a consistent namespace pattern to avoid confusion and potential compilation issues.
| namespace BootstrapBlazorLLMsDocsGenerator; | |
| namespace BootstrapBlazor.LLMsDocsGenerator; |
| if (string.IsNullOrEmpty(rootFolder)) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
When the root folder is not provided, the command silently exits without any error message or indication of what went wrong. This makes it difficult for users to understand that the --root parameter is required. Consider adding a descriptive error message or making the option required.
| Logger("Generating individual component documentation..."); | ||
| foreach (var component in components) | ||
| if (!Directory.Exists(_sourcePath)) | ||
| { |
There was a problem hiding this comment.
When the source path doesn't exist, the method silently returns without any error message or logging. This makes it difficult to debug why documentation generation failed. Consider adding an error message or throwing an exception to inform the user that the source path was not found.
| { | |
| { | |
| Logger($"Source path not found: {_sourcePath}. Documentation generation aborted."); |
| // _sb.Clear(); | ||
|
|
||
| // _sb.AppendLine("| Parameter | Type | Default | Description |"); | ||
| // _sb.AppendLine("|-----------|------|---------|-------------|"); | ||
|
|
||
| // var sortedParams = parameters | ||
| // .OrderByDescending(p => p.IsRequired) | ||
| // .ThenBy(p => p.IsEventCallback) | ||
| // .ThenBy(p => p.Name); | ||
|
|
||
| // foreach (var param in sortedParams) | ||
| // { | ||
| // var required = param.IsRequired ? " **[Required]**" : ""; | ||
| // var description = EscapeMarkdownCell(param.Description ?? "") + required; | ||
| // var defaultVal = param.DefaultValue ?? "-"; | ||
| // var type = EscapeMarkdownCell(param.Type); | ||
|
|
||
| // _sb.AppendLine($"| {param.Name} | `{type}` | {defaultVal} | {description} |"); | ||
| // } | ||
|
|
||
| // return _sb.ToString(); | ||
| //} | ||
|
|
There was a problem hiding this comment.
Large blocks of commented-out code should be removed rather than left in the codebase. If this functionality (BuildParameterTable) might be needed in the future, it should either be kept as working code or removed entirely and retrieved from version control if needed later. Commented-out code reduces code readability and maintainability.
| // _sb.Clear(); | |
| // _sb.AppendLine("| Parameter | Type | Default | Description |"); | |
| // _sb.AppendLine("|-----------|------|---------|-------------|"); | |
| // var sortedParams = parameters | |
| // .OrderByDescending(p => p.IsRequired) | |
| // .ThenBy(p => p.IsEventCallback) | |
| // .ThenBy(p => p.Name); | |
| // foreach (var param in sortedParams) | |
| // { | |
| // var required = param.IsRequired ? " **[Required]**" : ""; | |
| // var description = EscapeMarkdownCell(param.Description ?? "") + required; | |
| // var defaultVal = param.DefaultValue ?? "-"; | |
| // var type = EscapeMarkdownCell(param.Type); | |
| // _sb.AppendLine($"| {param.Name} | `{type}` | {defaultVal} | {description} |"); | |
| // } | |
| // return _sb.ToString(); | |
| //} |
Link issues
fixes #888
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Refactor the LLMs documentation generator tool to simplify its CLI interface, adjust paths to work from the published server output, and streamline documentation generation.
Enhancements: