-
Notifications
You must be signed in to change notification settings - Fork 3
⚙️ Implement YAML and XML Configuration Format Support #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: jongalloway <[email protected]>
Co-authored-by: jongalloway <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
@copilot Please review and fix if necessary:
IgnoreWhitespace = true
});
var result = (ToolDefinitions?)_serializer.Deserialize(xmlReader);After deserializing in LoadFromXml, run ValidateToolDefinitions and throw on errors (like in LoadFromStream) to keep validation consistent across both methods. |
Co-authored-by: jongalloway <[email protected]>
Fixed in commit 8f086ab. Added validation logic to |
There was a problem hiding this 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 adds support for YAML configuration files and XML-based tool definitions to the NLWebNet library, preserving existing JSON compatibility.
- Introduces
AddYamlFile,AddYamlStream, and auto-detection extensions for YAML inConfigurationExtensions. - Implements
IToolDefinitionLoaderand related models for XML tool definitions with validation. - Updates tests, documentation, and demo to showcase new formats.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/.../ConfigurationFormatSupportTests.cs | New tests for YAML parsing, XML loading, service reg. |
| src/NLWebNet/Services/ToolDefinitionLoader.cs | XML tool definitions loader with validation logic |
| src/NLWebNet/Extensions/ConfigurationExtensions.cs | Extension methods for YAML/XML config support |
| src/NLWebNet/Models/ToolDefinition.cs | XML models for tool definitions |
| src/NLWebNet/Models/ConfigurationFormatOptions.cs | Options class to control YAML/XML behavior |
| src/NLWebNet/Models/NLWebOptions.cs | Added ConfigurationFormat property |
| src/NLWebNet/NLWebNet.csproj | Added YamlDotNet and XML configuration package refs |
| samples/Demo/* | Example YAML/XML files and demo program updates |
| doc/configuration-format-updates.md | Detailed guide on new config formats |
| README.md | Quickstart examples for YAML and XML |
Comments suppressed due to low confidence (3)
tests/NLWebNet.Tests/Extensions/ConfigurationFormatSupportTests.cs:75
- Commented-out assertions reduce test coverage for YAML binding. Uncomment or add appropriate assertions to verify all intended option values are correctly bound.
// Assert.IsTrue(options.Value.EnableStreaming);
tests/NLWebNet.Tests/Extensions/ConfigurationFormatSupportTests.cs:85
- There’s no test covering
LoadFromStreamorLoadFromFileAsyncforToolDefinitionLoader. Consider adding tests for those methods to ensure full coverage of the XML loader API.
public void XmlToolDefinitions_BasicLoading_ShouldWork()
src/NLWebNet/Extensions/ConfigurationExtensions.cs:87
- [nitpick] This method only registers
IToolDefinitionLoader. It might be helpful to also bindConfigurationFormatOptionsfromIConfigurationso consumers can control format features via DI.
public static IServiceCollection AddNLWebConfigurationFormats(this IServiceCollection services,
tests/NLWebNet.Tests/Extensions/ConfigurationFormatSupportTests.cs
Outdated
Show resolved
Hide resolved
…s.cs Co-authored-by: Copilot <[email protected]>
This PR implements comprehensive configuration format support to address the NLWeb June 2025 release requirements, adding YAML-style multi-backend configuration and XML-based tool definitions while maintaining full backward compatibility.
🎯 Features Implemented
YAML Configuration Support
AddYamlFile()andAddYamlStream()extension methodsconfig_retrieval.yaml,nlweb.yaml, and environment-specific variantsExample YAML configuration:
XML Tool Definitions
ToolDefinition,ToolParameters,CustomPropertymodelsIToolDefinitionLoaderwith async loading and comprehensive validationExample XML tool definition:
Enhanced Configuration Options
ConfigurationFormatproperty for format-specific settingsAddNLWebConfigurationFormats()helper for easy DI setup🔧 Usage
✅ Backward Compatibility
All existing JSON configuration continues to work unchanged. No breaking changes to existing APIs or functionality.
🧪 Testing
📚 Documentation
doc/configuration-format-updates.mdwith examples and migration instructionssamples/Demo/🔗 Dependencies
📁 Files Changed
This implementation fully satisfies the requirements for supporting new configuration formats introduced in the NLWeb June 2025 release while maintaining complete backward compatibility and providing a clear migration path.
Fixes #36.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.