Skip to content

Conversation

@AClerbois
Copy link
Collaborator

  • Added "mcp" output format to DocApiGen for MCP-compatible JSON documentation, consumable by McpServer.
  • Implemented McpDocumentationGenerator and new model classes for components, enums, and metadata.
  • Updated Program.cs to support "mcp" format and improved CLI help.
  • Added FluentUI.Demo.DocApiGen.Tests project with xUnit tests for all new MCP documentation models.
  • All new files are MIT licensed and follow consistent style.

Pull Request

📖 Description

🎫 Issues

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new component
  • I have modified an existing component
  • I have validated the Unit Tests for an existing component

⏭ Next Steps

AClerbois and others added 8 commits December 12, 2025 12:54
- Added "mcp" output format to DocApiGen for MCP-compatible JSON documentation, consumable by McpServer.
- Implemented McpDocumentationGenerator and new model classes for components, enums, and metadata.
- Updated Program.cs to support "mcp" format and improved CLI help.
- Added FluentUI.Demo.DocApiGen.Tests project with xUnit tests for all new MCP documentation models.
- All new files are MIT licensed and follow consistent style.
Moved FluentUI.Demo.DocApiGen.Tests and FluentUI.Demo.DocViewer.Tests
projects from /examples/tools/ to a new /examples/tools/Tests/ folder
in the solution file. This improves project organization by clearly
separating test projects from other tool projects.
- Introduce GenerationMode enum (Summary/All) for doc generation
- Update ApiClassGenerator, CLI, and options to support mode param
- Refactor models: add AllMode (ComponentInfo, etc.), move SummaryMode types
- Remove old Mcp* models/tests; add new AllMode unit tests
- Update McpDocumentationGenerator to use AllMode and always generate full docs
- Add integration tests for output, file writing, and real FluentUI XML
- Update solution file to include new test projects
- Add GENERATION_MODES.md for usage and architecture documentation
- Improve code organization and separation between summary/full logic
Removed expected-api-comments.json, eliminating all API documentation metadata from the project. Updated IsValidType to exclude nested, interface, and non-public types. Refactored ApiClass Properties to always include all enum values, regardless of PropertyParameterOnly, while maintaining filtering for class properties.
- Add ApiClassPerformanceTests to ensure robust handling of abstract, interface, and complex types without exceptions
- Add progress reporting and warnings to documentation generators
- Refactor instance creation logic with CanCreateInstance method
- Catch and log member processing errors instead of failing entire runs
- Cache valid types/enums for better performance and reporting
- Only attempt default value extraction when safe to instantiate
- Overall, make documentation generation more robust and informative
AllMode model classes now use JsonIgnore attributes to skip serializing properties with default or null values, resulting in smaller and cleaner JSON. Default values for strings and collections are set to null, and the generator only assigns non-empty lists. Tests are updated to match the new defaults. Adds a launch profile for "all" mode and removes outdated GENERATION_MODES.md documentation.
Replaced manual null checks with ArgumentNullException.ThrowIfNull for clarity and consistency. Updated unit tests to use object initializers. Simplified dictionary initialization with collection expressions and improved code style with var usage. Removed unnecessary comments, whitespace, and the <NoWarn> property from the project file. No significant documentation changes.
@AClerbois
Copy link
Collaborator Author

@dvoituron I fixed your comments :-) I push the PR in Ready for review

@AClerbois AClerbois marked this pull request as ready for review December 16, 2025 11:25
Copilot AI review requested due to automatic review settings December 16, 2025 11:25
@AClerbois AClerbois requested a review from vnbaaij as a code owner December 16, 2025 11:25
@AClerbois AClerbois requested a review from dvoituron December 16, 2025 11:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the DocApiGen tool to support multiple generation modes and output formats. It introduces a cleaner architecture with abstractions for generators and formatters, adds comprehensive model classes for both Summary and All documentation modes, and includes extensive test coverage.

Key Changes:

  • Adds Summary and All generation modes with factory pattern support
  • Implements JSON (compact/structured) and C# output formatters
  • Improves error handling and performance optimizations in ApiClass
  • Adds comprehensive unit and integration tests for new models

Reviewed changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
Program.cs Enhanced CLI with mode/format validation and improved error handling
Abstractions/IDocumentationGenerator.cs New abstraction defining generator contract with GenerationMode enum
Abstractions/IOutputFormatter.cs New abstraction for output formatting
Generators/* New generator classes implementing Summary and All modes with factories
Formatters/* JSON and C# output formatters with compact/structured format support
Models/SummaryMode/* Refactored models moved to SummaryMode namespace
Models/AllMode/* New models for complete documentation (ComponentInfo, EnumInfo, etc.)
ApiClass.cs Performance optimizations: better error handling, instance creation checks
ReflectionExtensions.cs Enhanced type filtering (nested types, interfaces, public check)
ReadMe.md Updated documentation explaining modes and formats
*.Tests.csproj New test projects with xUnit for unit and integration testing
.sln/.slnx Solution updates to include new test projects
Comments suppressed due to low confidence (6)

examples/Tools/FluentUI.Demo.DocApiGen/Models/SummaryMode/ApiClass.cs:145

  • The change on line 144 returns null instead of throwing an InvalidCastException when InstanceTypes is null for a generic type. However, the InstanceTypes property (lines 41-52) has a getter that always returns a non-null array with typeof(string) as the default. This means InstanceTypes can never be null, making this null check redundant.

Consider either:

  1. Removing this null check since it can never be true, OR
  2. If there's a scenario where InstanceTypes could be null, update the property to allow null returns

The current implementation creates dead code that will never execute.
examples/Tools/FluentUI.Demo.DocApiGen/Models/SummaryMode/ApiClass.cs:278

  • The error handling improvement on lines 275-279 is good - catching exceptions and continuing with the next member instead of rethrowing. However, the error message includes ex.Message but no logging of the exception type or stack trace. For better debugging, consider including the exception type at minimum, e.g., $"[ApiDocumentation] ERROR: Cannot process {_component.FullName} -> {memberInfo.Name}: {ex.GetType().Name}: {ex.Message}".

This would help developers quickly identify whether it's an ArgumentException, NullReferenceException, etc., without losing the non-rethrowing behavior.
examples/Tools/FluentUI.Demo.DocApiGen/Models/SummaryMode/ApiClass.cs:318

  • The comment on line 298 says "constructor with nullable LibraryConfiguration", but the check on line 318 verifies parameters[0].IsOptional == false, which means the parameter is NOT optional (and therefore NOT nullable in the C# sense). This is inconsistent.

The comment should be updated to clarify the actual intent. Based on the code, it appears you're checking for a constructor with a single required LibraryConfiguration parameter that can be passed as null at runtime (line 124 shows new object?[] { null }), not an optional/nullable parameter in the method signature.
examples/Tools/FluentUI.Demo.DocApiGen/Models/SummaryMode/ApiClass.cs:322

  • This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
    examples/Tools/FluentUI.Demo.DocApiGen/Models/SummaryMode/ApiClass.cs:318
  • The expression 'A == false' can be simplified to '!A'.
    examples/Tools/FluentUI.Demo.DocApiGen/Models/SummaryMode/ApiClass.cs:80
  • The expression 'A == false' can be simplified to '!A'.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@AClerbois AClerbois removed the request for review from vnbaaij December 16, 2025 17:42
@dvoituron dvoituron requested a review from vnbaaij December 16, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants