-
Notifications
You must be signed in to change notification settings - Fork 259
feat: Complete catalog utility implementation with enhanced parameter types and automation support #1371
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
feat: Complete catalog utility implementation with enhanced parameter types and automation support #1371
Conversation
…ion issues Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
…priority tasks Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
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.
In addition to items below, ensure tests for catalog use depict entry quantities per style, there should be more than 80 series types, 2 for buffer, 6+ for hub types. You’ll have to get accurate numbers to pass.
…key comment feedback Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
…lated value and test counts Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
✅ Actions performedReview triggered.
|
Signed-off-by: Dave Skender <8432125+DaveSkender@users.noreply.github.com>
Try again, you failed to initialize |
The catalog test approach is fundamentally flawed. I generated tests that try to use reflection and parameter arrays to call indicator methods, but this doesn't work because method signatures vary significantly. The catalog system already provides proper execution via |
…automation validation Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis update introduces significant enhancements to the indicator catalog system, including new builder and executor classes for dynamic indicator configuration and execution, a robust serialization mechanism for indicator configurations, and comprehensive documentation and testing. Catalog entries now explicitly specify method names and use an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CustomIndicatorBuilder
participant IndicatorListing
participant IndicatorExecutor
participant IndicatorRegistry
participant IndicatorCatalog
User->>CustomIndicatorBuilder: Create builder from IndicatorListing
User->>CustomIndicatorBuilder: Set parameters (WithParamValue/WithParams)
User->>CustomIndicatorBuilder: Set source data (FromSource)
User->>CustomIndicatorBuilder: Execute<TResult>()
CustomIndicatorBuilder->>IndicatorExecutor: Execute with parameters and source
IndicatorExecutor->>IndicatorListing: Retrieve method name and metadata
IndicatorExecutor->>IndicatorRegistry: Lookup indicator method via reflection
IndicatorExecutor->>Indicator: Invoke method with parameters
Indicator->>IndicatorExecutor: Return results
IndicatorExecutor->>CustomIndicatorBuilder: Return typed results
CustomIndicatorBuilder->>User: Results
sequenceDiagram
participant User
participant IndicatorConfig
participant CustomIndicatorBuilder
participant IndicatorListing
participant IndicatorRegistry
User->>IndicatorConfig: Deserialize config from JSON
User->>IndicatorConfig: ToBuilder()
IndicatorConfig->>IndicatorRegistry: Lookup IndicatorListing by ID and style
IndicatorRegistry->>IndicatorConfig: Return IndicatorListing
IndicatorConfig->>CustomIndicatorBuilder: Create builder with parameters
CustomIndicatorBuilder->>User: Ready for execution
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Join our Discord community for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@copilot fix failing tests
|
…d DoNotParallelize for robust registry state management Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
... Fixed in commit 67ef963. The test failure was caused by race conditions during parallel test execution where the real catalog would overwrite test data. Added proper test isolation using |
Summary
This PR completes the remaining tasks from PR #1318 "feat: Catalog utility" by implementing the missing catalog listing definitions, specialized parameter types, and automation support features.
Key Changes
✅ Main Catalog Population (Task 8.9)
IndicatorCatalog
with all 94 individual indicator listings from existing catalog files✅ Enhanced Parameter System
AddDateParameter()
convenience method for DateTime parameters (VWAP use case)AddSeriesParameter()
convenience method for IEnumerable parameters (Beta, Correlation, PRS use cases)AddDateParameter()
for start dateAddSeriesParameter()
for quote series✅ Automation Support
MethodName
property toIndicatorListing
for programmatic discoveryWithMethodName()
builder method for setting method namesTechnical Details
Catalog Architecture
Enhanced Parameter Types
Automation Support
Impact
For Developers
For Automation
For Library Maintenance
Breaking Changes
None. All changes are additive and maintain backward compatibility.
Testing
Related Issues
Completes all high-priority and key medium-priority tasks from PR #1318:
Migration Guide
No migration required. New features are opt-in:
💡 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.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Documentation
Tests