feat: Add extensible ReusableObserver implementing IStreamObserver<IReusable> #1894
Conversation
📝 WalkthroughWalkthroughAdds a new public generic class 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 400000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new ReusableObserver class that implements IStreamObserver<IReusable> with a flexible callback-based design pattern, allowing users to provide custom implementations for observer lifecycle methods through constructor-injected Action delegates.
Changes:
- Added a new
ReusableObserverclass implementingIStreamObserver<IReusable> - Provides optional callback configuration for all observer lifecycle methods
- Manages subscription state internally with a boolean field
| Action<DateTime>? onRebuildTimestamp = null, | ||
| Action<int>? onRebuildIndex = null) | ||
| { | ||
| _onNext = onNext ?? throw new ArgumentNullException(nameof(onNext)); |
There was a problem hiding this comment.
The constructor incorrectly validates the onNext parameter which is not part of the IStreamObserver interface. Remove this validation line.
| _onNext = onNext ?? throw new ArgumentNullException(nameof(onNext)); | |
| _onNext = onNext; |
| public void OnPrune(DateTime toTimestamp) => _onPrune?.Invoke(toTimestamp); | ||
|
|
||
| // Reinitialize is called to reset the observer state | ||
| public void Reinitialize() { _isSubscribed = true; _onReinitialize?.Invoke(); } |
There was a problem hiding this comment.
The Reinitialize method sets _isSubscribed to true, but this may not be accurate since the subscription state is managed externally. Looking at the StreamHub implementation, IsSubscribed checks Provider.HasSubscriber(this). Setting this field to true without actual subscription could lead to an incorrect state. Consider whether this class should manage its own subscription state or if IsSubscribed should be updated by external callers.
| public void Reinitialize() { _isSubscribed = true; _onReinitialize?.Invoke(); } | |
| public void Reinitialize() { _onReinitialize?.Invoke(); } |
| Action? onCompleted = null, | ||
| Action? onUnsubscribe = null, | ||
| Action<IReusable, bool, int?>? onAdd = null, | ||
| Action<DateTime>? onRebuildFromTimestamp = null, |
There was a problem hiding this comment.
The parameter name onRebuildFromTimestamp is inconsistent with the other timestamp-based parameters. The corresponding field _onRebuildFromTimestamp and the OnRebuild method name suggest this should be called onRebuild to match the interface method name OnRebuild(DateTime fromTimestamp).
| private readonly Action? _onCompleted; | ||
| private readonly Action? _onUnsubscribe; | ||
| private readonly Action<IReusable, bool, int?>? _onAdd; | ||
| private readonly Action<DateTime>? _onRebuildFromTimestamp; |
There was a problem hiding this comment.
The field name _onRebuildFromTimestamp doesn't align well with the interface method OnRebuild(DateTime fromTimestamp). Consider renaming to _onRebuildFromTimestamp or just _onRebuildTimestamp for consistency with the overload pattern (Rebuild(), Rebuild(DateTime), Rebuild(int)).
|
|
||
| /// <summary> | ||
| /// Flexible IStreamObserver implementation with customizable callbacks for all lifecycle methods. | ||
| /// Most callbacks are optional (nullable) except onNext which is required. |
There was a problem hiding this comment.
The documentation states "Most callbacks are optional (nullable) except onNext which is required." However, onNext is not part of the IStreamObserver interface. This documentation should be updated or removed since all callbacks in the actual interface are optional (all actions are nullable in the constructor).
| /// Most callbacks are optional (nullable) except onNext which is required. | |
| /// Most callbacks in this implementation are optional (nullable); see constructor parameters for details. |
| public void Unsubscribe() { _isSubscribed = false; _onUnsubscribe?.Invoke(); } | ||
|
|
||
| // OnAdd is called when a new item is added to the hub's cache | ||
| public void OnAdd(IReusable item, bool notify, int? indexHint) => _onAdd?.Invoke(item, notify, indexHint); | ||
|
|
||
| // OnRebuild is called when the hub recalculates from a specific timestamp | ||
| public void OnRebuild(DateTime fromTimestamp) => _onRebuildFromTimestamp?.Invoke(fromTimestamp); | ||
|
|
||
| // OnPrune is called when old items are removed from the hub's cache | ||
| public void OnPrune(DateTime toTimestamp) => _onPrune?.Invoke(toTimestamp); | ||
|
|
||
| // Reinitialize is called to reset the observer state | ||
| public void Reinitialize() { _isSubscribed = true; _onReinitialize?.Invoke(); } |
There was a problem hiding this comment.
The Unsubscribe and Reinitialize methods use block braces with multiple statements on one line. For better readability and consistency, consider placing each statement on its own line or using separate lines for state updates and callback invocations.
| _onRebuildIndex = onRebuildIndex; | ||
| } | ||
|
|
||
| public void OnNext(IReusable value) { _onNext(value); } |
There was a problem hiding this comment.
The OnNext method does not exist in the IStreamObserver interface. This will cause a compilation error. The interface defines OnAdd, OnError, OnCompleted, OnRebuild, OnPrune, Reinitialize, and three Rebuild overloads, but does not include an OnNext method. This method should be removed.
| private readonly Action<IReusable> _onNext; | ||
| private readonly Action<Exception>? _onError; | ||
| private readonly Action? _onCompleted; | ||
| private readonly Action? _onUnsubscribe; | ||
| private readonly Action<IReusable, bool, int?>? _onAdd; | ||
| private readonly Action<DateTime>? _onRebuildFromTimestamp; | ||
| private readonly Action<DateTime>? _onPrune; | ||
| private readonly Action? _onReinitialize; | ||
| private readonly Action? _onRebuild; | ||
| private readonly Action<DateTime>? _onRebuildTimestamp; | ||
| private readonly Action<int>? _onRebuildIndex; | ||
| private bool _isSubscribed = false; | ||
|
|
||
| public bool IsSubscribed => _isSubscribed; | ||
|
|
||
| public ReusableObserver( | ||
| Action<IReusable> onNext, |
There was a problem hiding this comment.
The _onNext field and corresponding constructor parameter are not needed since OnNext is not part of the IStreamObserver interface. Remove this field and the onNext parameter from the constructor.
| private bool _isSubscribed = false; | ||
|
|
||
| public bool IsSubscribed => _isSubscribed; |
There was a problem hiding this comment.
The _isSubscribed field is initialized to false but set to true in Reinitialize and false in Unsubscribe. However, there's no mechanism to set it to true when the observer is actually subscribed via the Subscribe method. This could lead to IsSubscribed returning false even when the observer is subscribed, or true when it's not. Consider adding a method to update subscription state or remove internal state management entirely if it's managed externally.
| public class ReusableObserver : IStreamObserver<IReusable> | ||
| { | ||
| private readonly Action<IReusable> _onNext; | ||
| private readonly Action<Exception>? _onError; | ||
| private readonly Action? _onCompleted; | ||
| private readonly Action? _onUnsubscribe; | ||
| private readonly Action<IReusable, bool, int?>? _onAdd; | ||
| private readonly Action<DateTime>? _onRebuildFromTimestamp; | ||
| private readonly Action<DateTime>? _onPrune; | ||
| private readonly Action? _onReinitialize; | ||
| private readonly Action? _onRebuild; | ||
| private readonly Action<DateTime>? _onRebuildTimestamp; | ||
| private readonly Action<int>? _onRebuildIndex; | ||
| private bool _isSubscribed = false; | ||
|
|
||
| public bool IsSubscribed => _isSubscribed; | ||
|
|
||
| public ReusableObserver( | ||
| Action<IReusable> onNext, | ||
| Action<Exception>? onError = null, | ||
| Action? onCompleted = null, | ||
| Action? onUnsubscribe = null, | ||
| Action<IReusable, bool, int?>? onAdd = null, | ||
| Action<DateTime>? onRebuildFromTimestamp = null, | ||
| Action<DateTime>? onPrune = null, | ||
| Action? onReinitialize = null, | ||
| Action? onRebuild = null, | ||
| Action<DateTime>? onRebuildTimestamp = null, | ||
| Action<int>? onRebuildIndex = null) | ||
| { | ||
| _onNext = onNext ?? throw new ArgumentNullException(nameof(onNext)); | ||
| _onError = onError; | ||
| _onCompleted = onCompleted; | ||
| _onUnsubscribe = onUnsubscribe; | ||
| _onAdd = onAdd; | ||
| _onRebuildFromTimestamp = onRebuildFromTimestamp; | ||
| _onPrune = onPrune; | ||
| _onReinitialize = onReinitialize; | ||
| _onRebuild = onRebuild; | ||
| _onRebuildTimestamp = onRebuildTimestamp; | ||
| _onRebuildIndex = onRebuildIndex; | ||
| } | ||
|
|
||
| public void OnNext(IReusable value) { _onNext(value); } | ||
|
|
||
| public void OnError(Exception error) => _onError?.Invoke(error); | ||
|
|
||
| public void OnCompleted() => _onCompleted?.Invoke(); | ||
|
|
||
| public void Unsubscribe() { _isSubscribed = false; _onUnsubscribe?.Invoke(); } | ||
|
|
||
| // OnAdd is called when a new item is added to the hub's cache | ||
| public void OnAdd(IReusable item, bool notify, int? indexHint) => _onAdd?.Invoke(item, notify, indexHint); | ||
|
|
||
| // OnRebuild is called when the hub recalculates from a specific timestamp | ||
| public void OnRebuild(DateTime fromTimestamp) => _onRebuildFromTimestamp?.Invoke(fromTimestamp); | ||
|
|
||
| // OnPrune is called when old items are removed from the hub's cache | ||
| public void OnPrune(DateTime toTimestamp) => _onPrune?.Invoke(toTimestamp); | ||
|
|
||
| // Reinitialize is called to reset the observer state | ||
| public void Reinitialize() { _isSubscribed = true; _onReinitialize?.Invoke(); } | ||
|
|
||
| // Rebuild methods trigger recalculation | ||
| public void Rebuild() => _onRebuild?.Invoke(); | ||
|
|
||
| public void Rebuild(DateTime fromTimestamp) => _onRebuildTimestamp?.Invoke(fromTimestamp); | ||
|
|
||
| public void Rebuild(int fromIndex) => _onRebuildIndex?.Invoke(fromIndex); | ||
| } |
There was a problem hiding this comment.
The new ReusableObserver class lacks test coverage. Given that other StreamHub components in this directory have comprehensive test coverage, this class should have tests validating: callback invocation, subscription state management, null callback handling, and proper implementation of all IStreamObserver interface methods.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/_common/StreamHub/ReusableObserver.cs:
- Around line 3-7: Add XML documentation comments to every public member of the
ReusableObserver class: the methods OnNext, OnError, OnCompleted, Unsubscribe,
OnAdd, OnRebuild, OnPrune, Reinitialize, Rebuild and the IsSubscribed property.
For each member, add a <summary> describing its purpose (e.g., what the callback
does or what the method controls), document parameters with <param> where
applicable (e.g., the IReusable parameter for OnNext, Exception for OnError),
and document return values with <returns> when non-void; include any important
remarks or thread-safety/usage notes in <remarks> where relevant. Ensure the XML
tags are placed immediately above each public member declaration in the
ReusableObserver class so the project coding guidelines and tooling pick them
up.
- Line 52: The OnError implementation in ReusableObserver currently discards
exceptions when the _onError delegate is null; update the OnError method to
check if _onError is non-null and invoke it, otherwise do not swallow the
exception — either log it via an existing logger (e.g., _logger) or rethrow the
passed Exception (throw error;) so errors are not silently ignored; change the
single-line expression-bodied OnError to an explicit block that performs this
null check and either calls _onError(error) or logs/rethrows the error.
🧹 Nitpick comments (2)
src/_common/StreamHub/ReusableObserver.cs (2)
14-18: Clarify naming distinction between rebuild timestamp callbacks.There are two timestamp-related rebuild callbacks with confusingly similar names:
onRebuildFromTimestamp→ used byOnRebuild(DateTime)(interface method)onRebuildTimestamp→ used byRebuild(DateTime)(separate method)This naming could easily lead to developer confusion. Consider more distinct names that clarify the semantic difference, such as
onObservableRebuildvsonTriggerRebuild, or consolidate if they serve the same purpose.Also applies to: 30-34
50-50: Consider consistent method body style.
OnNextuses a block body while similar single-statement methods (OnError,OnCompleted) use expression bodies. For consistency:Style consistency suggestion
- public void OnNext(IReusable value) { _onNext(value); } + public void OnNext(IReusable value) => _onNext(value);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/_common/StreamHub/ReusableObserver.cs
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.cs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.cs: Implement single-pass O(n) algorithms unless mathematically impossible (justify exceptions in PR)
Provide deterministic WarmupPeriod helper or documented rule for each indicator
Usedoublefor speed; escalate todecimalonly when rounding affects financial correctness (>0.5 tick at 4-decimal pricing)
Allocate only result list and minimal working buffers; avoid temporary per-step Lists or LINQ in loops
Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
UseArgumentOutOfRangeExceptionfor invalid numeric parameter ranges
UseArgumentExceptionfor semantic misuse (e.g., insufficient history)
Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Use non-nullabledoubletypes internally with intentional NaN propagation; convert NaN tonullonly at the final result boundary via.NaN2Null()
Guard variable denominators with ternary checks (e.g.,denom != 0 ? num / denom : double.NaN) to prevent division by zero
Never use epsilon values (e.g.,1e-8,1e-9) for zero checks in division guards; use exact zero comparison (!= 0or== 0)
Accept NaN inputs and allow natural propagation through calculations; never reject or filter NaN values
Only validate quotes for null/missing quotes, not for NaN values in quote properties (High/Low/Close/etc.)
Usedouble.NaNto represent uninitialized state instead of sentinel values like0or-1
Avoid off-by-one errors when calculating lookback or warmup periods
Favordoublefor performance, switching todecimalonly when business accuracy demands it; avoid chained calculations that cause precision loss
Avoid performance regressions from unnecessary allocations or LINQ; prefer span-friendly loops and avoid boxing
All public methods must have XML documentation
Validate all user inputs in indicator implementations
Use consistent formatting following.editorconfigUse Roslynator for ...
Files:
src/_common/StreamHub/ReusableObserver.cs
⚙️ CodeRabbit configuration file
src/**/*.cs: Focus on:
- Mathematical accuracy: Verify financial calculations against reference implementations
- Performance: Flag unnecessary allocations, excessive LINQ, boxing operations
- Precision: Ensure appropriate use of
doublevsdecimalfor financial calculations- Null safety: Validate handling of insufficient data scenarios
- Input validation: Check parameter validation and error handling
- Lookback periods: Verify off-by-one errors in window calculations
- Streaming support: Ensure stateful indicators handle edge cases properly
- Memory efficiency: Check for proper span usage and buffer management
- XML documentation: Ensure all public APIs have complete documentation
Files:
src/_common/StreamHub/ReusableObserver.cs
**/*.{cs,md}
📄 CodeRabbit inference engine (AGENTS.md)
Treat all warnings and errors as failures that must be fixed; do not suppress warnings or ignore them
Files:
src/_common/StreamHub/ReusableObserver.cs
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T10:07:54.602Z
Learning: Applies to src/**/Stream*.cs : Consult open specs before modifying stateful pipelines in StreamHub indicators for streaming support
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
📚 Learning: 2026-01-10T10:07:54.602Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T10:07:54.602Z
Learning: Applies to src/**/Stream*.cs : Consult open specs before modifying stateful pipelines in StreamHub indicators for streaming support
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-09T09:37:31.805Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-08T01:06:34.843Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1497
File: src/a-d/Alligator/Alligator.BufferList.cs:335-351
Timestamp: 2025-10-08T01:06:34.843Z
Learning: In the Stock.Indicators codebase, the IQuote interface extends IReusable (public interface IQuote : IReusable), which means any Quote or IQuote implementation automatically implements IReusable and can be used wherever IReusable is required, enabling both direct quote usage and indicator chaining scenarios.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: For repaint-by-design indicators like ZigZag that require full history recalculation: Override Rebuild(DateTime fromTimestamp) to use the Series implementation instead of incremental processing. Call NotifyObserversOnRebuild(fromTimestamp) after updating Cache. Do not manipulate Cache directly in ToIndicator or custom helper methods.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
⏰ Context from checks skipped due to timeout of 400000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
src/_common/StreamHub/ReusableObserver.cs (2)
20-20: Verify intended subscription lifecycle behavior.The observer starts with
_isSubscribed = falseand only becomes subscribed viaReinitialize(). This is counterintuitive for an observer pattern where subscription typically begins at construction or attachment time.Additionally,
OnNextdoesn't checkIsSubscribedbefore invoking the callback—events will be processed regardless of subscription state. Is this the intended design, or should the observer either:
- Start subscribed (
_isSubscribed = true), or- Guard
OnNextwith anIsSubscribedcheck?Also applies to: 50-50, 68-68
20-22: No changes needed. The_isSubscribedfield is instance-scoped and does not violate the coding guideline requiring no static mutable fields. There is no evidence in the codebase thatReusableObserveris designed or used in concurrent scenarios; unlikeStreamHub(which manages shared observer state and appropriately uses_unsubscribeLock),ReusableObserveris a standalone observer with instance-local state. TheIsSubscribedproperty is never read anywhere in the codebase, confirming single-context usage.Likely an incorrect or invalid review comment.
| /// <summary> | ||
| /// Flexible IStreamObserver implementation with customizable callbacks for all lifecycle methods. | ||
| /// Most callbacks are optional (nullable) except onNext which is required. | ||
| /// </summary> | ||
| public class ReusableObserver : IStreamObserver<IReusable> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add XML documentation to all public members.
The class-level XML documentation is present, but all public methods (OnNext, OnError, OnCompleted, Unsubscribe, OnAdd, OnRebuild, OnPrune, Reinitialize, Rebuild) and the IsSubscribed property lack XML documentation. Per coding guidelines, all public methods must have XML documentation.
🤖 Prompt for AI Agents
In @src/_common/StreamHub/ReusableObserver.cs around lines 3 - 7, Add XML
documentation comments to every public member of the ReusableObserver class: the
methods OnNext, OnError, OnCompleted, Unsubscribe, OnAdd, OnRebuild, OnPrune,
Reinitialize, Rebuild and the IsSubscribed property. For each member, add a
<summary> describing its purpose (e.g., what the callback does or what the
method controls), document parameters with <param> where applicable (e.g., the
IReusable parameter for OnNext, Exception for OnError), and document return
values with <returns> when non-void; include any important remarks or
thread-safety/usage notes in <remarks> where relevant. Ensure the XML tags are
placed immediately above each public member declaration in the ReusableObserver
class so the project coding guidelines and tooling pick them up.
|
|
||
| public void OnNext(IReusable value) { _onNext(value); } | ||
|
|
||
| public void OnError(Exception error) => _onError?.Invoke(error); |
There was a problem hiding this comment.
Silently ignoring errors may violate exception handling guidelines.
If _onError is null, exceptions passed to OnError are silently discarded. Per coding guidelines, exceptions should not be swallowed. Consider logging or rethrowing when no error handler is provided:
Suggested approach
- public void OnError(Exception error) => _onError?.Invoke(error);
+ public void OnError(Exception error)
+ {
+ if (_onError is not null)
+ _onError.Invoke(error);
+ else
+ throw error; // Or use a logging mechanism
+ }🤖 Prompt for AI Agents
In @src/_common/StreamHub/ReusableObserver.cs at line 52, The OnError
implementation in ReusableObserver currently discards exceptions when the
_onError delegate is null; update the OnError method to check if _onError is
non-null and invoke it, otherwise do not swallow the exception — either log it
via an existing logger (e.g., _logger) or rethrow the passed Exception (throw
error;) so errors are not silently ignored; change the single-line
expression-bodied OnError to an explicit block that performs this null check and
either calls _onError(error) or logs/rethrows the error.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In @src/_common/StreamHub/ReusableObserver.cs:
- Around line 62-63: Convert the inline comment above the public Reinitialize
method into proper XML documentation: add a /// <summary> describing that
Reinitialize resets the observer state, and document any side effects such as
setting _isSubscribed = true and invoking _onReinitialize; include any <remarks>
or <returns> if applicable and ensure the public method Reinitialize has the XML
doc immediately above its declaration in ReusableObserver class so it complies
with coding guidelines.
- Line 21: The public property IsSubscribed on class ReusableObserver lacks XML
documentation; add a concise XML doc comment (/// <summary>...</summary>) above
the IsSubscribed property describing what the boolean indicates (e.g., whether
the observer is currently subscribed), and include any remarks or value
description if relevant so it meets the project's XML documentation guideline
for public members.
- Line 49: Add XML documentation for the public method OnCompleted in the
ReusableObserver class: add a /// <summary> describing that this method signals
completion to the observer and calls the internal _onCompleted action, and
include any remarks or exceptions if relevant; ensure the XML comment is placed
immediately above the public void OnCompleted() => _onCompleted?.Invoke();
declaration and follows the project's XML documentation style (summary tag at
minimum, plus <remarks> or <seealso> if applicable).
- Around line 59-60: Add XML documentation for the public method
OnPrune(DateTime toTimestamp): replace the inline comment with a /// XML doc
containing a <summary> that explains that OnPrune is invoked when old items are
removed from the hub's cache and an <param name="toTimestamp"> describing the
meaning of the timestamp parameter (e.g., cutoff time up to which items were
pruned). Optionally include <remarks> or <seealso> if there are related members,
but ensure the public method OnPrune has at minimum <summary> and <param> tags
per coding guidelines.
- Around line 56-57: Convert the inline comment into XML documentation for the
public method OnRebuild: add a /// <summary> describing that the method is
called when the hub recalculates from a specific timestamp and a /// <param
name="fromTimestamp"> explaining that this parameter is the timestamp from which
the rebuild is calculated; place the XML doc immediately above the OnRebuild
declaration (method name OnRebuild) and keep it consistent with other public
method docs in ReusableObserver.
- Around line 53-54: Add XML documentation for the public method OnAdd: replace
the inline comment with a /// <summary> describing that OnAdd is called when a
new IReusable item is added to the hub's cache, and add /// <param name="item">,
/// <param name="notify"> and /// <param name="indexHint"> tags documenting each
parameter and intended behavior; keep the implementation as-is (calls
_onAdd?.Invoke(item, notify, indexHint)).
- Around line 65-70: Add XML documentation comments to all three public Rebuild
overloads (Rebuild(), Rebuild(DateTime fromTimestamp), Rebuild(int fromIndex)).
For each method provide a summary describing the purpose (that it triggers a
rebuild/recalculation), document parameters where applicable (fromTimestamp and
fromIndex) and mention any side effects (invokes the delegates _rebuild,
_rebuildTimestamp, _rebuildIndex respectively). Keep the XML tags <summary> and
<param> and ensure the comments are placed immediately above each method
declaration.
- Line 47: Add XML documentation to the public OnError method describing its
purpose and parameter, and validate the exception parameter for null per coding
guidelines: in ReusableObserver.OnError check if exception is null and throw an
ArgumentNullException for the "exception" parameter before invoking the
delegate, then call _onError?.Invoke(exception); ensure the XML doc includes
summary and param tags to satisfy tooling.
- Around line 23-45: The ReusableObserver constructor is missing XML
documentation and the subscription state (_isSubscribed) isn't explicitly
initialized; add XML doc comments for the public constructor describing each
parameter and the initial subscription behavior, and either add an optional bool
parameter (e.g., isInitiallySubscribed = false) to the ReusableObserver
constructor or explicitly assign _isSubscribed = false inside the constructor
(and document that choice in the XML summary/param tags); update references to
the constructor signature (and the XML docs) so callers know the initial
subscription state.
- Line 51: Add XML doc comments for the public Unsubscribe() method and the
IsSubscribed property per coding guidelines, and make access to the mutable
_isSubscribed field thread-safe: introduce a private readonly object _sync (or
reuse an existing sync object) and wrap writes to _isSubscribed in lock(_sync)
inside Unsubscribe() and Reinitialize(), and wrap the IsSubscribed getter in the
same lock to avoid races; update calls to _onUnsubscribe?.Invoke() to be invoked
outside the lock if needed to avoid reentrancy issues.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/_common/StreamHub/ReusableObserver.cs
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.cs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.cs: Implement single-pass O(n) algorithms unless mathematically impossible (justify exceptions in PR)
Provide deterministic WarmupPeriod helper or documented rule for each indicator
Usedoublefor speed; escalate todecimalonly when rounding affects financial correctness (>0.5 tick at 4-decimal pricing)
Allocate only result list and minimal working buffers; avoid temporary per-step Lists or LINQ in loops
Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
UseArgumentOutOfRangeExceptionfor invalid numeric parameter ranges
UseArgumentExceptionfor semantic misuse (e.g., insufficient history)
Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Use non-nullabledoubletypes internally with intentional NaN propagation; convert NaN tonullonly at the final result boundary via.NaN2Null()
Guard variable denominators with ternary checks (e.g.,denom != 0 ? num / denom : double.NaN) to prevent division by zero
Never use epsilon values (e.g.,1e-8,1e-9) for zero checks in division guards; use exact zero comparison (!= 0or== 0)
Accept NaN inputs and allow natural propagation through calculations; never reject or filter NaN values
Only validate quotes for null/missing quotes, not for NaN values in quote properties (High/Low/Close/etc.)
Usedouble.NaNto represent uninitialized state instead of sentinel values like0or-1
Avoid off-by-one errors when calculating lookback or warmup periods
Favordoublefor performance, switching todecimalonly when business accuracy demands it; avoid chained calculations that cause precision loss
Avoid performance regressions from unnecessary allocations or LINQ; prefer span-friendly loops and avoid boxing
All public methods must have XML documentation
Validate all user inputs in indicator implementations
Use consistent formatting following.editorconfigUse Roslynator for ...
Files:
src/_common/StreamHub/ReusableObserver.cs
⚙️ CodeRabbit configuration file
src/**/*.cs: Focus on:
- Mathematical accuracy: Verify financial calculations against reference implementations
- Performance: Flag unnecessary allocations, excessive LINQ, boxing operations
- Precision: Ensure appropriate use of
doublevsdecimalfor financial calculations- Null safety: Validate handling of insufficient data scenarios
- Input validation: Check parameter validation and error handling
- Lookback periods: Verify off-by-one errors in window calculations
- Streaming support: Ensure stateful indicators handle edge cases properly
- Memory efficiency: Check for proper span usage and buffer management
- XML documentation: Ensure all public APIs have complete documentation
Files:
src/_common/StreamHub/ReusableObserver.cs
**/*.{cs,md}
📄 CodeRabbit inference engine (AGENTS.md)
Treat all warnings and errors as failures that must be fixed; do not suppress warnings or ignore them
Files:
src/_common/StreamHub/ReusableObserver.cs
🧠 Learnings (12)
📓 Common learnings
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
📚 Learning: 2026-01-10T10:07:54.602Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T10:07:54.602Z
Learning: Applies to src/**/Stream*.cs : Consult open specs before modifying stateful pipelines in StreamHub indicators for streaming support
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: For repaint-by-design indicators like ZigZag that require full history recalculation: Override Rebuild(DateTime fromTimestamp) to use the Series implementation instead of incremental processing. Call NotifyObserversOnRebuild(fromTimestamp) after updating Cache. Do not manipulate Cache directly in ToIndicator or custom helper methods.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/tests/**/*.Tests.cs : Provide comprehensive unit test coverage for all code paths in indicators
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-12-31T23:59:09.102Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1821
File: tests/indicators/m-r/Prs/Prs.Regression.Tests.cs:16-17
Timestamp: 2025-12-31T23:59:09.102Z
Learning: Applies to tests/indicators/**/*.Regression.Tests.cs: When abstract base class RegressionTestBase requires override methods (Series, Buffer, Stream), keep inconclusive test stubs (Assert.Inconclusive("Test not yet implemented")) as placeholders during feature removal or temporary lack of implementation, rather than removing the method, as they serve as reminders for future work and avoid compilation errors.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : All public methods must have XML documentation
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Keep documentation accurate and synchronized between code comments, XML docs, and the published docs site
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-09-30T03:06:24.792Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1423
File: src/e-k/Kama/Kama.StreamHub.cs:80-144
Timestamp: 2025-09-30T03:06:24.792Z
Learning: In C# code within the Stock.Indicators repository, never use the null-forgiving operator (!) as it is considered a code smell. Use alternative approaches such as `.Value` property access on nullable types when null checks are already in place, or proper null-conditional operators.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-09T09:37:31.805Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-08T01:06:34.843Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1497
File: src/a-d/Alligator/Alligator.BufferList.cs:335-351
Timestamp: 2025-10-08T01:06:34.843Z
Learning: In the Stock.Indicators codebase, the IQuote interface extends IReusable (public interface IQuote : IReusable), which means any Quote or IQuote implementation automatically implements IReusable and can be used wherever IReusable is required, enabling both direct quote usage and indicator chaining scenarios.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: ZigZagHub must override Rebuild() to use ProviderCache.ToZigZag() for full recalculation and call NotifyObserversOnRebuild() to notify chain observers. ToIndicator should simply return cached results. RollbackState should call Rebuild(DateTime.MinValue) to trigger full recalculation.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
🔇 Additional comments (1)
src/_common/StreamHub/ReusableObserver.cs (1)
3-7: Class documentation looks good.The XML documentation clearly describes the purpose and flexibility of the observer implementation.
| _rebuildIndex = rebuildIndex; | ||
| } | ||
|
|
||
| public void OnError(Exception exception) => _onError?.Invoke(exception); |
There was a problem hiding this comment.
Add XML documentation and validate exception parameter.
Two issues:
- Missing XML documentation required per coding guidelines.
- The
exceptionparameter is not validated for null. Consider validating user inputs per coding guidelines.
📝 Proposed fix
+ /// <summary>
+ /// Invoked when an error occurs in the stream.
+ /// </summary>
+ /// <param name="exception">The exception that occurred.</param>
- public void OnError(Exception exception) => _onError?.Invoke(exception);
+ public void OnError(Exception exception)
+ {
+ ArgumentNullException.ThrowIfNull(exception);
+ _onError?.Invoke(exception);
+ }As per coding guidelines, all public methods require XML documentation.
🤖 Prompt for AI Agents
In @src/_common/StreamHub/ReusableObserver.cs at line 47, Add XML documentation
to the public OnError method describing its purpose and parameter, and validate
the exception parameter for null per coding guidelines: in
ReusableObserver.OnError check if exception is null and throw an
ArgumentNullException for the "exception" parameter before invoking the
delegate, then call _onError?.Invoke(exception); ensure the XML doc includes
summary and param tags to satisfy tooling.
|
|
||
| public void OnCompleted() => _onCompleted?.Invoke(); | ||
|
|
||
| public void Unsubscribe() { _isSubscribed = false; _onUnsubscribe?.Invoke(); } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for concurrent/multi-threaded usage patterns in StreamHub
# and examine the ReusableObserver implementation
# First, find and examine the ReusableObserver.cs file
echo "=== ReusableObserver.cs content ==="
cat -n src/_common/StreamHub/ReusableObserver.cs
echo ""
echo "=== Threading patterns in StreamHub ==="
rg -n -C3 --type=cs 'Thread|Task|async|Parallel|lock|Interlocked|ConcurrentBag|ConcurrentDictionary' src/_common/StreamHub/
echo ""
echo "=== IStreamObserver interface usage ==="
rg -n -C3 --type=cs 'IStreamObserver' src/_common/StreamHub/Repository: DaveSkender/Stock.Indicators
Length of output: 15345
🏁 Script executed:
#!/bin/bash
# Check the complete IStreamObserver interface definition and concurrent usage patterns
echo "=== IStreamObserver interface definition ==="
cat -n src/_common/StreamHub/IStreamObserver.cs
echo ""
echo "=== StreamHub.Observer.cs lock usage context ==="
sed -n '50,100p' src/_common/StreamHub/StreamHub.Observer.cs | cat -nRepository: DaveSkender/Stock.Indicators
Length of output: 5790
🏁 Script executed:
#!/bin/bash
# Verify other public methods in ReusableObserver for XML documentation
sed -n '47,71p' src/_common/StreamHub/ReusableObserver.cs | grep -E '^\s*(public|///)' | cat -nRepository: DaveSkender/Stock.Indicators
Length of output: 939
Add XML documentation and ensure thread-safe access to _isSubscribed.
- Missing XML documentation required per coding guidelines for all public methods. The
Unsubscribe()method and theIsSubscribedproperty must have XML doc comments. - The
_isSubscribedfield is modified without synchronization in bothUnsubscribe()andReinitialize(). In streaming scenarios with potential concurrent access, reads via theIsSubscribedproperty and writes could race. Since this observer operates in a streaming context where thread-safety is required, protect mutable state access (considerlock,Interlocked, or other synchronization).
📝 Proposed documentation for Unsubscribe
+ /// <summary>
+ /// Unsubscribes from the stream and invokes the unsubscribe callback.
+ /// </summary>
public void Unsubscribe() { _isSubscribed = false; _onUnsubscribe?.Invoke(); }🤖 Prompt for AI Agents
In @src/_common/StreamHub/ReusableObserver.cs at line 51, Add XML doc comments
for the public Unsubscribe() method and the IsSubscribed property per coding
guidelines, and make access to the mutable _isSubscribed field thread-safe:
introduce a private readonly object _sync (or reuse an existing sync object) and
wrap writes to _isSubscribed in lock(_sync) inside Unsubscribe() and
Reinitialize(), and wrap the IsSubscribed getter in the same lock to avoid
races; update calls to _onUnsubscribe?.Invoke() to be invoked outside the lock
if needed to avoid reentrancy issues.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/_common/StreamHub/ReusableObserver.cs:
- Line 21: Add XML documentation for the public property IsSubscribed: add a
triple-slash XML comment immediately above the IsSubscribed property summarizing
its purpose (e.g., indicates whether the observer is currently subscribed),
describe the return value with a <returns> tag, and document any thread-safety
or nullability considerations; reference the IsSubscribed property and the
backing delegate _isSubscribed() so reviewers can locate and validate the
comment.
- Around line 23-47: Add XML documentation to the ReusableObserver constructor
describing the purpose and each parameter, and validate the required
isSubscribed parameter for null (throw ArgumentNullException) before assigning
to the _isSubscribed field; keep other optional delegates nullable as-is and
mention in the docs which parameters are optional. Ensure the validation occurs
at the top of the ReusableObserver constructor and reference the constructor and
the _isSubscribed field in the doc/comment.
- Around line 49-72: Public methods in ReusableObserver (OnError, OnCompleted,
Unsubscribe, OnAdd, OnRebuild, OnPrune, Reinitialize, Rebuild() overloads) lack
XML documentation; replace the inline comments with /// XML docs: add a
<summary> describing each method’s purpose, and for methods with parameters
(OnError(Exception exception), OnAdd(IReusable item, bool notify, int?
indexHint), OnRebuild(DateTime fromTimestamp), OnPrune(DateTime toTimestamp),
Rebuild(DateTime fromTimestamp), Rebuild(int fromIndex)) include <param
name="..."> entries explaining each parameter and any nullable semantics, and
include a <remarks> or <returns> if relevant for the overloads to clarify
behavior; ensure the XML tags are properly formatted above each public method
signature for consistent tooling and style.
🧹 Nitpick comments (1)
src/_common/StreamHub/ReusableObserver.cs (1)
14-19: Consider renaming for clarity between notification and action callbacks.The naming distinction between
_onRebuild(notification callback, line 14) and the three_rebuild*(action callbacks, lines 17-19) could cause confusion. The_onRebuildis invoked byOnRebuild(DateTime)(a notification from upstream), while_rebuild,_rebuildTimestamp, and_rebuildIndexare invoked by the threeRebuild()overloads (actions triggered by this observer).Consider renaming to make the distinction clearer, e.g.,
_onUpstreamRebuildor_onRebuildNotificationvs._onRebuild(no params),_onRebuildFromTimestamp,_onRebuildFromIndex.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/_common/StreamHub/ReusableObserver.cs
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.cs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.cs: Implement single-pass O(n) algorithms unless mathematically impossible (justify exceptions in PR)
Provide deterministic WarmupPeriod helper or documented rule for each indicator
Usedoublefor speed; escalate todecimalonly when rounding affects financial correctness (>0.5 tick at 4-decimal pricing)
Allocate only result list and minimal working buffers; avoid temporary per-step Lists or LINQ in loops
Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
UseArgumentOutOfRangeExceptionfor invalid numeric parameter ranges
UseArgumentExceptionfor semantic misuse (e.g., insufficient history)
Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Use non-nullabledoubletypes internally with intentional NaN propagation; convert NaN tonullonly at the final result boundary via.NaN2Null()
Guard variable denominators with ternary checks (e.g.,denom != 0 ? num / denom : double.NaN) to prevent division by zero
Never use epsilon values (e.g.,1e-8,1e-9) for zero checks in division guards; use exact zero comparison (!= 0or== 0)
Accept NaN inputs and allow natural propagation through calculations; never reject or filter NaN values
Only validate quotes for null/missing quotes, not for NaN values in quote properties (High/Low/Close/etc.)
Usedouble.NaNto represent uninitialized state instead of sentinel values like0or-1
Avoid off-by-one errors when calculating lookback or warmup periods
Favordoublefor performance, switching todecimalonly when business accuracy demands it; avoid chained calculations that cause precision loss
Avoid performance regressions from unnecessary allocations or LINQ; prefer span-friendly loops and avoid boxing
All public methods must have XML documentation
Validate all user inputs in indicator implementations
Use consistent formatting following.editorconfigUse Roslynator for ...
Files:
src/_common/StreamHub/ReusableObserver.cs
⚙️ CodeRabbit configuration file
src/**/*.cs: Focus on:
- Mathematical accuracy: Verify financial calculations against reference implementations
- Performance: Flag unnecessary allocations, excessive LINQ, boxing operations
- Precision: Ensure appropriate use of
doublevsdecimalfor financial calculations- Null safety: Validate handling of insufficient data scenarios
- Input validation: Check parameter validation and error handling
- Lookback periods: Verify off-by-one errors in window calculations
- Streaming support: Ensure stateful indicators handle edge cases properly
- Memory efficiency: Check for proper span usage and buffer management
- XML documentation: Ensure all public APIs have complete documentation
Files:
src/_common/StreamHub/ReusableObserver.cs
**/*.{cs,md}
📄 CodeRabbit inference engine (AGENTS.md)
Treat all warnings and errors as failures that must be fixed; do not suppress warnings or ignore them
Files:
src/_common/StreamHub/ReusableObserver.cs
🧠 Learnings (13)
📓 Common learnings
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
📚 Learning: 2026-01-10T10:07:54.602Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T10:07:54.602Z
Learning: Applies to src/**/Stream*.cs : Consult open specs before modifying stateful pipelines in StreamHub indicators for streaming support
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: For repaint-by-design indicators like ZigZag that require full history recalculation: Override Rebuild(DateTime fromTimestamp) to use the Series implementation instead of incremental processing. Call NotifyObserversOnRebuild(fromTimestamp) after updating Cache. Do not manipulate Cache directly in ToIndicator or custom helper methods.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/tests/**/*.Tests.cs : Provide comprehensive unit test coverage for all code paths in indicators
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-12-31T23:59:09.102Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1821
File: tests/indicators/m-r/Prs/Prs.Regression.Tests.cs:16-17
Timestamp: 2025-12-31T23:59:09.102Z
Learning: Applies to tests/indicators/**/*.Regression.Tests.cs: When abstract base class RegressionTestBase requires override methods (Series, Buffer, Stream), keep inconclusive test stubs (Assert.Inconclusive("Test not yet implemented")) as placeholders during feature removal or temporary lack of implementation, rather than removing the method, as they serve as reminders for future work and avoid compilation errors.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : All public methods must have XML documentation
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Keep documentation accurate and synchronized between code comments, XML docs, and the published docs site
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-09-30T03:06:24.792Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1423
File: src/e-k/Kama/Kama.StreamHub.cs:80-144
Timestamp: 2025-09-30T03:06:24.792Z
Learning: In C# code within the Stock.Indicators repository, never use the null-forgiving operator (!) as it is considered a code smell. Use alternative approaches such as `.Value` property access on nullable types when null checks are already in place, or proper null-conditional operators.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: ZigZagHub must override Rebuild() to use ProviderCache.ToZigZag() for full recalculation and call NotifyObserversOnRebuild() to notify chain observers. ToIndicator should simply return cached results. RollbackState should call Rebuild(DateTime.MinValue) to trigger full recalculation.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-09T09:37:31.805Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-08T01:06:34.843Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1497
File: src/a-d/Alligator/Alligator.BufferList.cs:335-351
Timestamp: 2025-10-08T01:06:34.843Z
Learning: In the Stock.Indicators codebase, the IQuote interface extends IReusable (public interface IQuote : IReusable), which means any Quote or IQuote implementation automatically implements IReusable and can be used wherever IReusable is required, enabling both direct quote usage and indicator chaining scenarios.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
⏰ Context from checks skipped due to timeout of 400000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
src/_common/StreamHub/ReusableObserver.cs (1)
49-49: Verify exception handling design when callback is null.The
OnErrormethod silently ignores exceptions when_onErrorcallback is null. While this aligns with the flexible callback design where all handlers are optional, it differs from the coding guideline "Never swallow exceptions."Consider whether:
- The current design is intentional (observers opt-in to error handling), or
- A default behavior should be provided (e.g., logging or re-throwing) when no callback is registered
Based on coding guidelines emphasizing proper exception handling.
- Add XML docs for IsSubscribed property with thread-safety notes - Add XML docs for constructor with parameter descriptions - Add null validation for required isSubscribed parameter - Add XML docs for all public methods (OnError, OnCompleted, Unsubscribe, OnAdd, OnRebuild, OnPrune, Reinitialize, Rebuild overloads) - Add test for ArgumentNullException validation Co-authored-by: JGronholz <17935969+JGronholz@users.noreply.github.com>
…reamhub docs: Add XML documentation to ReusableObserver
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/_common/StreamHub/ReusableObserver.cs (4)
21-32: Thread-safety claim may be misleading.The remarks state "This property is thread-safe" but the property simply invokes the provided delegate. Thread-safety depends entirely on the implementation of the delegate passed by the caller. Consider revising the documentation to clarify that thread-safety is the caller's responsibility.
📝 Suggested documentation revision
/// <remarks> -/// This property is thread-safe and returns the current subscription state -/// by invoking the backing delegate <see cref="_isSubscribed"/>. +/// This property returns the current subscription state by invoking the +/// backing delegate <see cref="_isSubscribed"/>. Thread-safety depends on +/// the delegate implementation provided by the caller. /// The value is determined by the provider at the time of invocation. /// </remarks>
107-116: Consider validating the exception parameter.The
OnErrormethod accepts anExceptionparameter but does not validate it before invocation. If the interface contract expects a non-null exception, consider adding validation.Additionally, when
_onErroris null, exceptions are silently ignored. This is by design per the class documentation, but consumers should be aware that unhandled errors will be swallowed.🛠️ Optional: Add exception parameter validation
-public void OnError(Exception exception) => _onError?.Invoke(exception); +public void OnError(Exception exception) +{ + ArgumentNullException.ThrowIfNull(exception); + _onError?.Invoke(exception); +}
118-136: Minor: Inconsistent formatting style.Line 135 uses block syntax
{ ... }while surrounding methods (lines 116, 127) use expression-body syntax. Consider using consistent formatting throughout.♻️ Suggested fix for consistency
-public void Unsubscribe() { _onUnsubscribe?.Invoke(); } +public void Unsubscribe() => _onUnsubscribe?.Invoke();
167-195: Minor: Inconsistent formatting style on Reinitialize.Line 195 uses block syntax while other single-statement methods use expression bodies.
♻️ Suggested fix for consistency
-public void Reinitialize() { _onReinitialize?.Invoke(); } +public void Reinitialize() => _onReinitialize?.Invoke();tests/indicators/_common/StreamHub/StreamHub.Observer.Tests.cs (1)
63-72: LGTM: Constructor null validation test.The test correctly verifies that
ArgumentNullExceptionis thrown with the appropriate parameter name whenisSubscribedis null. The use ofnull!is acceptable here to intentionally bypass the nullable check for testing purposes.However, per the coding guidelines for test coverage, consider adding additional tests to verify:
IsSubscribedproperty returns the delegate's value correctly- Each lifecycle method invokes its corresponding callback
- Methods with null callbacks do not throw
Do you want me to help generate additional test cases for comprehensive coverage of the
ReusableObserverclass?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/_common/StreamHub/ReusableObserver.cstests/indicators/_common/StreamHub/StreamHub.Observer.Tests.cs
🧰 Additional context used
📓 Path-based instructions (3)
tests/**/*.cs
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/**/*.cs: Write tests for all public methods
Cover edge cases: empty input, minimum/maximum values, boundary conditions in tests
Use descriptive test names that explain the scenario
Keep tests focused on single behaviors
Files:
tests/indicators/_common/StreamHub/StreamHub.Observer.Tests.cs
⚙️ CodeRabbit configuration file
tests/**/*.cs: Review for:
- Test coverage: Ensure edge cases and boundary conditions are tested
- Mathematical validation: Verify tests use manually calculated results
- Performance tests: Check benchmark tests for regression detection
- Integration tests: Validate API compatibility and chaining scenarios
- Streaming tests: Ensure real-time scenarios are properly tested
- Error scenarios: Verify exception handling and invalid input tests
Files:
tests/indicators/_common/StreamHub/StreamHub.Observer.Tests.cs
**/*.{cs,md}
📄 CodeRabbit inference engine (AGENTS.md)
Treat all warnings and errors as failures that must be fixed; do not suppress warnings or ignore them
Files:
tests/indicators/_common/StreamHub/StreamHub.Observer.Tests.cssrc/_common/StreamHub/ReusableObserver.cs
src/**/*.cs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.cs: Implement single-pass O(n) algorithms unless mathematically impossible (justify exceptions in PR)
Provide deterministic WarmupPeriod helper or documented rule for each indicator
Usedoublefor speed; escalate todecimalonly when rounding affects financial correctness (>0.5 tick at 4-decimal pricing)
Allocate only result list and minimal working buffers; avoid temporary per-step Lists or LINQ in loops
Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
UseArgumentOutOfRangeExceptionfor invalid numeric parameter ranges
UseArgumentExceptionfor semantic misuse (e.g., insufficient history)
Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Use non-nullabledoubletypes internally with intentional NaN propagation; convert NaN tonullonly at the final result boundary via.NaN2Null()
Guard variable denominators with ternary checks (e.g.,denom != 0 ? num / denom : double.NaN) to prevent division by zero
Never use epsilon values (e.g.,1e-8,1e-9) for zero checks in division guards; use exact zero comparison (!= 0or== 0)
Accept NaN inputs and allow natural propagation through calculations; never reject or filter NaN values
Only validate quotes for null/missing quotes, not for NaN values in quote properties (High/Low/Close/etc.)
Usedouble.NaNto represent uninitialized state instead of sentinel values like0or-1
Avoid off-by-one errors when calculating lookback or warmup periods
Favordoublefor performance, switching todecimalonly when business accuracy demands it; avoid chained calculations that cause precision loss
Avoid performance regressions from unnecessary allocations or LINQ; prefer span-friendly loops and avoid boxing
All public methods must have XML documentation
Validate all user inputs in indicator implementations
Use consistent formatting following.editorconfigUse Roslynator for ...
Files:
src/_common/StreamHub/ReusableObserver.cs
⚙️ CodeRabbit configuration file
src/**/*.cs: Focus on:
- Mathematical accuracy: Verify financial calculations against reference implementations
- Performance: Flag unnecessary allocations, excessive LINQ, boxing operations
- Precision: Ensure appropriate use of
doublevsdecimalfor financial calculations- Null safety: Validate handling of insufficient data scenarios
- Input validation: Check parameter validation and error handling
- Lookback periods: Verify off-by-one errors in window calculations
- Streaming support: Ensure stateful indicators handle edge cases properly
- Memory efficiency: Check for proper span usage and buffer management
- XML documentation: Ensure all public APIs have complete documentation
Files:
src/_common/StreamHub/ReusableObserver.cs
🧠 Learnings (14)
📓 Common learnings
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
📚 Learning: 2025-12-23T09:47:28.113Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1745
File: tests/indicators/s-z/StochRsi/StochRsi.BufferList.Tests.cs:51-57
Timestamp: 2025-12-23T09:47:28.113Z
Learning: For bounded indicators (e.g., Stoch, StochRsi, Williams %R), prefer algebraic stability with explicit boundary checks instead of post-calculation clamping or ToPrecision rounding. Implement checks such as if value >= high then set to 100, if value <= low then set to 0. This prevents floating-point precision errors at boundaries and ensures exact 0/100 values. Apply this guidance to test and implementation files under tests/ indicators where these calculations are validated to ensure consistency across edge cases.
Applied to files:
tests/indicators/_common/StreamHub/StreamHub.Observer.Tests.cs
📚 Learning: 2026-01-10T10:07:54.602Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T10:07:54.602Z
Learning: Applies to src/**/Stream*.cs : Consult open specs before modifying stateful pipelines in StreamHub indicators for streaming support
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: For repaint-by-design indicators like ZigZag that require full history recalculation: Override Rebuild(DateTime fromTimestamp) to use the Series implementation instead of incremental processing. Call NotifyObserversOnRebuild(fromTimestamp) after updating Cache. Do not manipulate Cache directly in ToIndicator or custom helper methods.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/tests/**/*.Tests.cs : Provide comprehensive unit test coverage for all code paths in indicators
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-12-31T23:59:09.102Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1821
File: tests/indicators/m-r/Prs/Prs.Regression.Tests.cs:16-17
Timestamp: 2025-12-31T23:59:09.102Z
Learning: Applies to tests/indicators/**/*.Regression.Tests.cs: When abstract base class RegressionTestBase requires override methods (Series, Buffer, Stream), keep inconclusive test stubs (Assert.Inconclusive("Test not yet implemented")) as placeholders during feature removal or temporary lack of implementation, rather than removing the method, as they serve as reminders for future work and avoid compilation errors.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : All public methods must have XML documentation
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Keep documentation accurate and synchronized between code comments, XML docs, and the published docs site
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-09-30T03:06:24.792Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1423
File: src/e-k/Kama/Kama.StreamHub.cs:80-144
Timestamp: 2025-09-30T03:06:24.792Z
Learning: In C# code within the Stock.Indicators repository, never use the null-forgiving operator (!) as it is considered a code smell. Use alternative approaches such as `.Value` property access on nullable types when null checks are already in place, or proper null-conditional operators.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: ZigZagHub must override Rebuild() to use ProviderCache.ToZigZag() for full recalculation and call NotifyObserversOnRebuild() to notify chain observers. ToIndicator should simply return cached results. RollbackState should call Rebuild(DateTime.MinValue) to trigger full recalculation.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-27T21:10:12.886Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1627
File: src/m-r/ParabolicSar/ParabolicSar.StreamHub.cs:12-12
Timestamp: 2025-10-27T21:10:12.886Z
Learning: Inline comments (e.g., "// State variables") used for organizational purposes within classes are acceptable in the Stock.Indicators codebase. The Roslynator RCS1181 diagnostic for converting comments to documentation comments should be suppressed, as it's considered unnecessary noise.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-09T09:37:31.805Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
🧬 Code graph analysis (1)
tests/indicators/_common/StreamHub/StreamHub.Observer.Tests.cs (1)
src/_common/StreamHub/ReusableObserver.cs (2)
ReusableObserver(7-231)ReusableObserver(79-105)
⏰ Context from checks skipped due to timeout of 400000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
src/_common/StreamHub/ReusableObserver.cs (5)
1-7: LGTM: Class declaration and XML documentation.The class declaration with XML documentation is clear and follows the coding guidelines. The summary accurately describes the purpose of this flexible observer implementation.
9-19: LGTM: Private callback fields.All callback fields are appropriately declared as
private readonlywith nullable types for optional callbacks. The field naming is consistent.
79-105: LGTM: Constructor implementation with proper validation.The constructor correctly validates the required
isSubscribedparameter usingArgumentNullException.ThrowIfNull()and properly assigns all callback parameters. The use of optional parameters withnulldefaults provides a clean API.
137-165: LGTM: OnAdd and OnRebuild implementations.Both methods correctly delegate to their respective optional callbacks with proper null-conditional invocation. XML documentation accurately describes the purpose and parameters.
197-231: LGTM: Rebuild method overloads.All three
Rebuildoverloads (parameterless,DateTime,int) correctly delegate to their respective optional callbacks. The XML documentation clearly distinguishes between each variant's purpose.
|
If you don't consider this useless, it still needs proper integration into the library (catalog?/etc). Example usage: |
|
This is basically the observer third of StreamHub: except not hard-coded to do anything specific. We can't produce observable TOut's without first observing TIn's. I like the idea of making |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/_common/StreamHub/ReusableObserver.cs:
- Around line 21-32: The XML doc for the IsSubscribed property incorrectly
claims thread-safety even though it simply calls the user-supplied delegate
_isSubscribed; update the summary/remarks to remove the unconditional
"thread-safe" guarantee and instead state that IsSubscribed invokes the backing
delegate _isSubscribed and that any thread-safety guarantees depend on the
delegate's implementation (mention ReusableObserver and the IsSubscribed
property so reviewers can find the code).
- Around line 233-260: Public API members lack XML docs: add
summary/param/remarks as required for the generic class
ReusableObserver<TResult> and its public constructor and method OnAdd to satisfy
the "All public methods must have XML documentation" guideline. Specifically,
add XML comments for the class ReusableObserver<TResult> (describe purpose and
type parameter TResult), for the public constructor (describe each parameter:
isSubscribed, onError, onCompleted, onUnsubscribe, onAdd, onRebuild, onPrune,
onReinitialize, rebuild, rebuildTimestamp, rebuildIndex), and for the public
method OnAdd(TResult item, bool notify, int? indexHint) (describe parameters and
behavior), and include <typeparam> for TResult and appropriate <returns> or
<remarks> where applicable; also add XML docs for the public readonly delegate
field _onAdd if exposed, otherwise ensure any other public members in this file
have similar documentation.
🧹 Nitpick comments (2)
src/_common/StreamHub/ReusableObserver.cs (2)
107-230: Lifecycle methods correctly delegate to callbacks.All lifecycle methods properly delegate to their respective optional callbacks using the null-conditional operator. The XML documentation is comprehensive and accurately describes each method's behavior.
🎨 Minor style consistency suggestion (optional)
For consistency with other methods that use expression-body syntax, consider:
- public void Unsubscribe() { _onUnsubscribe?.Invoke(); } + public void Unsubscribe() => _onUnsubscribe?.Invoke();- public void Reinitialize() { _onReinitialize?.Invoke(); } + public void Reinitialize() => _onReinitialize?.Invoke();
253-253: Consider moving field declaration before constructor.The field
_onAddis declared after the constructor and method that use it. While valid C#, convention suggests declaring fields at the top of the class for better readability.📐 Suggested field reordering
public class ReusableObserver<TResult> : ReusableObserver, IStreamObserver<TResult> where TResult : IReusable { + private readonly Action<TResult, bool, int?>? _onAdd; + public ReusableObserver( Func<bool> isSubscribed, Action<Exception>? onError = null, Action? onCompleted = null, Action? onUnsubscribe = null, Action<TResult, bool, int?>? onAdd = null, Action<DateTime>? onRebuild = null, Action<DateTime>? onPrune = null, Action? onReinitialize = null, Action? rebuild = null, Action<DateTime>? rebuildTimestamp = null, Action<int>? rebuildIndex = null) : base(isSubscribed, onError, onCompleted, onUnsubscribe, null, onRebuild, onPrune, onReinitialize, rebuild, rebuildTimestamp, rebuildIndex) { _onAdd = onAdd; } - private readonly Action<TResult, bool, int?>? _onAdd; - public void OnAdd(TResult item, bool notify, int? indexHint)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/_common/StreamHub/ReusableObserver.cs
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.cs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.cs: Implement single-pass O(n) algorithms unless mathematically impossible (justify exceptions in PR)
Provide deterministic WarmupPeriod helper or documented rule for each indicator
Usedoublefor speed; escalate todecimalonly when rounding affects financial correctness (>0.5 tick at 4-decimal pricing)
Allocate only result list and minimal working buffers; avoid temporary per-step Lists or LINQ in loops
Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
UseArgumentOutOfRangeExceptionfor invalid numeric parameter ranges
UseArgumentExceptionfor semantic misuse (e.g., insufficient history)
Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Use non-nullabledoubletypes internally with intentional NaN propagation; convert NaN tonullonly at the final result boundary via.NaN2Null()
Guard variable denominators with ternary checks (e.g.,denom != 0 ? num / denom : double.NaN) to prevent division by zero
Never use epsilon values (e.g.,1e-8,1e-9) for zero checks in division guards; use exact zero comparison (!= 0or== 0)
Accept NaN inputs and allow natural propagation through calculations; never reject or filter NaN values
Only validate quotes for null/missing quotes, not for NaN values in quote properties (High/Low/Close/etc.)
Usedouble.NaNto represent uninitialized state instead of sentinel values like0or-1
Avoid off-by-one errors when calculating lookback or warmup periods
Favordoublefor performance, switching todecimalonly when business accuracy demands it; avoid chained calculations that cause precision loss
Avoid performance regressions from unnecessary allocations or LINQ; prefer span-friendly loops and avoid boxing
All public methods must have XML documentation
Validate all user inputs in indicator implementations
Use consistent formatting following.editorconfigUse Roslynator for ...
Files:
src/_common/StreamHub/ReusableObserver.cs
⚙️ CodeRabbit configuration file
src/**/*.cs: Focus on:
- Mathematical accuracy: Verify financial calculations against reference implementations
- Performance: Flag unnecessary allocations, excessive LINQ, boxing operations
- Precision: Ensure appropriate use of
doublevsdecimalfor financial calculations- Null safety: Validate handling of insufficient data scenarios
- Input validation: Check parameter validation and error handling
- Lookback periods: Verify off-by-one errors in window calculations
- Streaming support: Ensure stateful indicators handle edge cases properly
- Memory efficiency: Check for proper span usage and buffer management
- XML documentation: Ensure all public APIs have complete documentation
Files:
src/_common/StreamHub/ReusableObserver.cs
**/*.{cs,md}
📄 CodeRabbit inference engine (AGENTS.md)
Treat all warnings and errors as failures that must be fixed; do not suppress warnings or ignore them
Files:
src/_common/StreamHub/ReusableObserver.cs
🧠 Learnings (14)
📓 Common learnings
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1497
File: src/a-d/Alligator/Alligator.BufferList.cs:335-351
Timestamp: 2025-10-08T01:06:34.843Z
Learning: In the Stock.Indicators codebase, the IQuote interface extends IReusable (public interface IQuote : IReusable), which means any Quote or IQuote implementation automatically implements IReusable and can be used wherever IReusable is required, enabling both direct quote usage and indicator chaining scenarios.
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: For repaint-by-design indicators like ZigZag that require full history recalculation: Override Rebuild(DateTime fromTimestamp) to use the Series implementation instead of incremental processing. Call NotifyObserversOnRebuild(fromTimestamp) after updating Cache. Do not manipulate Cache directly in ToIndicator or custom helper methods.
📚 Learning: 2026-01-10T10:07:54.602Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T10:07:54.602Z
Learning: Applies to src/**/Stream*.cs : Consult open specs before modifying stateful pipelines in StreamHub indicators for streaming support
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: For repaint-by-design indicators like ZigZag that require full history recalculation: Override Rebuild(DateTime fromTimestamp) to use the Series implementation instead of incremental processing. Call NotifyObserversOnRebuild(fromTimestamp) after updating Cache. Do not manipulate Cache directly in ToIndicator or custom helper methods.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-09T09:37:31.805Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/tests/**/*.Tests.cs : Provide comprehensive unit test coverage for all code paths in indicators
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-12-31T23:59:09.102Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1821
File: tests/indicators/m-r/Prs/Prs.Regression.Tests.cs:16-17
Timestamp: 2025-12-31T23:59:09.102Z
Learning: Applies to tests/indicators/**/*.Regression.Tests.cs: When abstract base class RegressionTestBase requires override methods (Series, Buffer, Stream), keep inconclusive test stubs (Assert.Inconclusive("Test not yet implemented")) as placeholders during feature removal or temporary lack of implementation, rather than removing the method, as they serve as reminders for future work and avoid compilation errors.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : All public methods must have XML documentation
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Keep documentation accurate and synchronized between code comments, XML docs, and the published docs site
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-09-30T03:06:24.792Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1423
File: src/e-k/Kama/Kama.StreamHub.cs:80-144
Timestamp: 2025-09-30T03:06:24.792Z
Learning: In C# code within the Stock.Indicators repository, never use the null-forgiving operator (!) as it is considered a code smell. Use alternative approaches such as `.Value` property access on nullable types when null checks are already in place, or proper null-conditional operators.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-27T21:10:12.886Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1627
File: src/m-r/ParabolicSar/ParabolicSar.StreamHub.cs:12-12
Timestamp: 2025-10-27T21:10:12.886Z
Learning: Inline comments (e.g., "// State variables") used for organizational purposes within classes are acceptable in the Stock.Indicators codebase. The Roslynator RCS1181 diagnostic for converting comments to documentation comments should be suppressed, as it's considered unnecessary noise.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: ZigZagHub must override Rebuild() to use ProviderCache.ToZigZag() for full recalculation and call NotifyObserversOnRebuild() to notify chain observers. ToIndicator should simply return cached results. RollbackState should call Rebuild(DateTime.MinValue) to trigger full recalculation.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-08T01:06:34.843Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1497
File: src/a-d/Alligator/Alligator.BufferList.cs:335-351
Timestamp: 2025-10-08T01:06:34.843Z
Learning: In the Stock.Indicators codebase, the IQuote interface extends IReusable (public interface IQuote : IReusable), which means any Quote or IQuote implementation automatically implements IReusable and can be used wherever IReusable is required, enabling both direct quote usage and indicator chaining scenarios.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
🧬 Code graph analysis (1)
src/_common/StreamHub/ReusableObserver.cs (2)
src/_common/Math/Numerical.cs (1)
DateTime(105-110)src/_common/Reusable/Reusable.Utilities.cs (1)
IReusable(66-67)
⏰ Context from checks skipped due to timeout of 400000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
src/_common/StreamHub/ReusableObserver.cs (2)
3-19: Well-structured class with appropriate field declarations.The class correctly implements
IStreamObserver<ISeries>with configurable callbacks stored as readonly fields, ensuring immutability after construction.
79-105: LGTM! Proper validation and initialization.The constructor correctly validates the required
isSubscribedparameter and properly initializes all callback fields. The use of optional parameters with default null values provides good flexibility.
| /// <summary> | ||
| /// Gets a value indicating whether the observer is currently subscribed to the data provider. | ||
| /// </summary> | ||
| /// <value> | ||
| /// <c>true</c> if the observer is subscribed; otherwise, <c>false</c>. | ||
| /// </value> | ||
| /// <remarks> | ||
| /// This property is thread-safe and returns the current subscription state | ||
| /// by invoking the backing delegate <see cref="_isSubscribed"/>. | ||
| /// The value is determined by the provider at the time of invocation. | ||
| /// </remarks> | ||
| public bool IsSubscribed => _isSubscribed(); |
There was a problem hiding this comment.
Clarify the thread-safety guarantee in documentation.
The documentation claims "This property is thread-safe" (line 28), but the property simply delegates to the user-provided _isSubscribed function. Thread-safety depends entirely on the caller's implementation of that delegate, not on this property. This could mislead users into assuming a guarantee that doesn't exist.
📝 Suggested documentation revision
/// <remarks>
- /// This property is thread-safe and returns the current subscription state
- /// by invoking the backing delegate <see cref="_isSubscribed"/>.
+ /// Returns the current subscription state by invoking the backing delegate
+ /// <see cref="_isSubscribed"/>. Thread-safety depends on the implementation
+ /// of the provided delegate.
/// The value is determined by the provider at the time of invocation.
/// </remarks>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// <summary> | |
| /// Gets a value indicating whether the observer is currently subscribed to the data provider. | |
| /// </summary> | |
| /// <value> | |
| /// <c>true</c> if the observer is subscribed; otherwise, <c>false</c>. | |
| /// </value> | |
| /// <remarks> | |
| /// This property is thread-safe and returns the current subscription state | |
| /// by invoking the backing delegate <see cref="_isSubscribed"/>. | |
| /// The value is determined by the provider at the time of invocation. | |
| /// </remarks> | |
| public bool IsSubscribed => _isSubscribed(); | |
| /// <summary> | |
| /// Gets a value indicating whether the observer is currently subscribed to the data provider. | |
| /// </summary> | |
| /// <value> | |
| /// <c>true</c> if the observer is subscribed; otherwise, <c>false</c>. | |
| /// </value> | |
| /// <remarks> | |
| /// Returns the current subscription state by invoking the backing delegate | |
| /// <see cref="_isSubscribed"/>. Thread-safety depends on the implementation | |
| /// of the provided delegate. | |
| /// The value is determined by the provider at the time of invocation. | |
| /// </remarks> | |
| public bool IsSubscribed => _isSubscribed(); |
🤖 Prompt for AI Agents
In @src/_common/StreamHub/ReusableObserver.cs around lines 21 - 32, The XML doc
for the IsSubscribed property incorrectly claims thread-safety even though it
simply calls the user-supplied delegate _isSubscribed; update the
summary/remarks to remove the unconditional "thread-safe" guarantee and instead
state that IsSubscribed invokes the backing delegate _isSubscribed and that any
thread-safety guarantees depend on the delegate's implementation (mention
ReusableObserver and the IsSubscribed property so reviewers can find the code).
| public class ReusableObserver<TResult> : ReusableObserver, IStreamObserver<TResult> | ||
| where TResult : IReusable | ||
| { | ||
| public ReusableObserver( | ||
| Func<bool> isSubscribed, | ||
| Action<Exception>? onError = null, | ||
| Action? onCompleted = null, | ||
| Action? onUnsubscribe = null, | ||
| Action<TResult, bool, int?>? onAdd = null, | ||
| Action<DateTime>? onRebuild = null, | ||
| Action<DateTime>? onPrune = null, | ||
| Action? onReinitialize = null, | ||
| Action? rebuild = null, | ||
| Action<DateTime>? rebuildTimestamp = null, | ||
| Action<int>? rebuildIndex = null) | ||
| : base(isSubscribed, onError, onCompleted, onUnsubscribe, null, onRebuild, onPrune, onReinitialize, rebuild, rebuildTimestamp, rebuildIndex) | ||
| { | ||
| _onAdd = onAdd; | ||
| } | ||
|
|
||
| private readonly Action<TResult, bool, int?>? _onAdd; | ||
|
|
||
| public void OnAdd(TResult item, bool notify, int? indexHint) | ||
| { | ||
| //Don't invoke base.OnAdd to avoid boxing | ||
| _onAdd?.Invoke(item, notify, indexHint); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add required XML documentation for public members.
The generic class and all its public members lack XML documentation, which violates the coding guideline: "All public methods must have XML documentation."
📚 Suggested XML documentation
+/// <summary>
+/// Generic implementation of <see cref="ReusableObserver"/> that provides type-safe callbacks
+/// for <typeparamref name="TResult"/> items, avoiding boxing overhead.
+/// </summary>
+/// <typeparam name="TResult">
+/// The type of result items, constrained to <see cref="IReusable"/>.
+/// </typeparam>
public class ReusableObserver<TResult> : ReusableObserver, IStreamObserver<TResult>
where TResult : IReusable
{
+ /// <summary>
+ /// Initializes a new instance of the <see cref="ReusableObserver{TResult}"/> class
+ /// with type-safe callbacks for <typeparamref name="TResult"/> items.
+ /// </summary>
+ /// <param name="isSubscribed">
+ /// A function that returns the current subscription state. This parameter is required.
+ /// </param>
+ /// <param name="onError">
+ /// Optional callback invoked when an error occurs in the data provider.
+ /// </param>
+ /// <param name="onCompleted">
+ /// Optional callback invoked when the data provider has finished sending notifications.
+ /// </param>
+ /// <param name="onUnsubscribe">
+ /// Optional callback invoked when unsubscribing from the data provider.
+ /// </param>
+ /// <param name="onAdd">
+ /// Optional type-safe callback invoked when a new <typeparamref name="TResult"/> item is added.
+ /// </param>
+ /// <param name="onRebuild">
+ /// Optional callback invoked when the hub recalculates from a specific timestamp.
+ /// </param>
+ /// <param name="onPrune">
+ /// Optional callback invoked when old items are removed from the hub's cache.
+ /// </param>
+ /// <param name="onReinitialize">
+ /// Optional callback invoked to reset the observer state.
+ /// </param>
+ /// <param name="rebuild">
+ /// Optional callback invoked to trigger a full recalculation of the cache.
+ /// </param>
+ /// <param name="rebuildTimestamp">
+ /// Optional callback invoked to trigger a recalculation from a specific timestamp.
+ /// </param>
+ /// <param name="rebuildIndex">
+ /// Optional callback invoked to trigger a recalculation from a specific index position.
+ /// </param>
+ /// <exception cref="ArgumentNullException">
+ /// Thrown when <paramref name="isSubscribed"/> is <c>null</c>.
+ /// </exception>
public ReusableObserver(And for the OnAdd method:
+ /// <summary>
+ /// Provides the observer with new data when a <typeparamref name="TResult"/> item
+ /// is added to the hub's cache.
+ /// </summary>
+ /// <param name="item">
+ /// The strongly-typed result item to add.
+ /// </param>
+ /// <param name="notify">
+ /// Notify subscribers of the new item.
+ /// </param>
+ /// <param name="indexHint">
+ /// Provider index hint, if known. This parameter is nullable.
+ /// </param>
+ /// <remarks>
+ /// This override avoids boxing by directly invoking the type-safe <c>_onAdd</c> callback
+ /// instead of delegating to the base class.
+ /// </remarks>
public void OnAdd(TResult item, bool notify, int? indexHint)As per coding guidelines.
🤖 Prompt for AI Agents
In @src/_common/StreamHub/ReusableObserver.cs around lines 233 - 260, Public API
members lack XML docs: add summary/param/remarks as required for the generic
class ReusableObserver<TResult> and its public constructor and method OnAdd to
satisfy the "All public methods must have XML documentation" guideline.
Specifically, add XML comments for the class ReusableObserver<TResult> (describe
purpose and type parameter TResult), for the public constructor (describe each
parameter: isSubscribed, onError, onCompleted, onUnsubscribe, onAdd, onRebuild,
onPrune, onReinitialize, rebuild, rebuildTimestamp, rebuildIndex), and for the
public method OnAdd(TResult item, bool notify, int? indexHint) (describe
parameters and behavior), and include <typeparam> for TResult and appropriate
<returns> or <remarks> where applicable; also add XML docs for the public
readonly delegate field _onAdd if exposed, otherwise ensure any other public
members in this file have similar documentation.
|
Latest commit added the generic! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/_common/StreamHub/ReusableObserver.cs:
- Line 36: The XML doc for the constructor incorrectly references the
non-generic type; update the <see> reference to point to the generic class by
changing the tag to reference ReusableObserver{TResult} so the documentation
link resolves correctly (e.g., replace ReusableObserver with
ReusableObserver{TResult} in the constructor's XML summary for the
ReusableObserver class).
🧹 Nitpick comments (4)
src/_common/StreamHub/ReusableObserver.cs (4)
10-20: Consider the implications of multiple rebuild callbacks.The class stores three separate rebuild-related callbacks (
_rebuild,_rebuildTimestamp,_rebuildIndex) corresponding to threeRebuildoverloads. If a user provides multiple rebuild callbacks, it's unclear which takes precedence or how they interact. While this flexibility might be intentional, consider documenting the expected usage pattern or validating that only one rebuild callback variant is provided.Would you like me to generate a validation check to ensure only one rebuild callback is provided, or is it intentional to allow multiple?
22-33: Clarify thread-safety guarantees in documentation.The documentation states "This property is thread-safe" but doesn't clarify that thread-safety depends on the
isSubscribeddelegate provided by the caller. Consider updating the remarks to note that the property access is thread-safe, but the delegate itself must be thread-safe if called from multiple threads.
136-136: Use consistent expression-body syntax.The
Unsubscribemethod uses a block body{ }while other single-statement methods in this class use expression bodies=>. For consistency, consider using an expression body here as well.♻️ Proposed fix
- public void Unsubscribe() { _onUnsubscribe?.Invoke(); } + public void Unsubscribe() => _onUnsubscribe?.Invoke();
196-196: Use consistent expression-body syntax.The
Reinitializemethod uses a block body{ }while other single-statement methods use expression bodies=>. Apply the same style for consistency with the rest of the class.♻️ Proposed fix
- public void Reinitialize() { _onReinitialize?.Invoke(); } + public void Reinitialize() => _onReinitialize?.Invoke();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/_common/StreamHub/ReusableObserver.cs
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.cs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.cs: Implement single-pass O(n) algorithms unless mathematically impossible (justify exceptions in PR)
Provide deterministic WarmupPeriod helper or documented rule for each indicator
Usedoublefor speed; escalate todecimalonly when rounding affects financial correctness (>0.5 tick at 4-decimal pricing)
Allocate only result list and minimal working buffers; avoid temporary per-step Lists or LINQ in loops
Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
UseArgumentOutOfRangeExceptionfor invalid numeric parameter ranges
UseArgumentExceptionfor semantic misuse (e.g., insufficient history)
Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Use non-nullabledoubletypes internally with intentional NaN propagation; convert NaN tonullonly at the final result boundary via.NaN2Null()
Guard variable denominators with ternary checks (e.g.,denom != 0 ? num / denom : double.NaN) to prevent division by zero
Never use epsilon values (e.g.,1e-8,1e-9) for zero checks in division guards; use exact zero comparison (!= 0or== 0)
Accept NaN inputs and allow natural propagation through calculations; never reject or filter NaN values
Only validate quotes for null/missing quotes, not for NaN values in quote properties (High/Low/Close/etc.)
Usedouble.NaNto represent uninitialized state instead of sentinel values like0or-1
Avoid off-by-one errors when calculating lookback or warmup periods
Favordoublefor performance, switching todecimalonly when business accuracy demands it; avoid chained calculations that cause precision loss
Avoid performance regressions from unnecessary allocations or LINQ; prefer span-friendly loops and avoid boxing
All public methods must have XML documentation
Validate all user inputs in indicator implementations
Use consistent formatting following.editorconfigUse Roslynator for ...
Files:
src/_common/StreamHub/ReusableObserver.cs
⚙️ CodeRabbit configuration file
src/**/*.cs: Focus on:
- Mathematical accuracy: Verify financial calculations against reference implementations
- Performance: Flag unnecessary allocations, excessive LINQ, boxing operations
- Precision: Ensure appropriate use of
doublevsdecimalfor financial calculations- Null safety: Validate handling of insufficient data scenarios
- Input validation: Check parameter validation and error handling
- Lookback periods: Verify off-by-one errors in window calculations
- Streaming support: Ensure stateful indicators handle edge cases properly
- Memory efficiency: Check for proper span usage and buffer management
- XML documentation: Ensure all public APIs have complete documentation
Files:
src/_common/StreamHub/ReusableObserver.cs
**/*.{cs,md}
📄 CodeRabbit inference engine (AGENTS.md)
Treat all warnings and errors as failures that must be fixed; do not suppress warnings or ignore them
Files:
src/_common/StreamHub/ReusableObserver.cs
🧠 Learnings (13)
📓 Common learnings
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
📚 Learning: 2026-01-10T10:07:54.602Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T10:07:54.602Z
Learning: Applies to src/**/Stream*.cs : Consult open specs before modifying stateful pipelines in StreamHub indicators for streaming support
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-09T09:37:31.805Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1503
File: src/a-d/Beta/Beta.BufferList.cs:6-11
Timestamp: 2025-10-09T09:37:31.805Z
Learning: The IBufferReusable interface is designed for single-input indicators only. Dual-input indicators like Beta, Correlation, and PRS cannot implement IBufferReusable because they require synchronized pairs of inputs (e.g., evaluated asset and market benchmark) rather than single values, making them incompatible with IBufferReusable's single-input method signatures (Add(DateTime, double), Add(IReusable), Add(IReadOnlyList<IReusable>)).
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: For repaint-by-design indicators like ZigZag that require full history recalculation: Override Rebuild(DateTime fromTimestamp) to use the Series implementation instead of incremental processing. Call NotifyObserversOnRebuild(fromTimestamp) after updating Cache. Do not manipulate Cache directly in ToIndicator or custom helper methods.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/tests/**/*.Tests.cs : Provide comprehensive unit test coverage for all code paths in indicators
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-12-31T23:59:09.102Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1821
File: tests/indicators/m-r/Prs/Prs.Regression.Tests.cs:16-17
Timestamp: 2025-12-31T23:59:09.102Z
Learning: Applies to tests/indicators/**/*.Regression.Tests.cs: When abstract base class RegressionTestBase requires override methods (Series, Buffer, Stream), keep inconclusive test stubs (Assert.Inconclusive("Test not yet implemented")) as placeholders during feature removal or temporary lack of implementation, rather than removing the method, as they serve as reminders for future work and avoid compilation errors.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : All public methods must have XML documentation
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Keep documentation accurate and synchronized between code comments, XML docs, and the published docs site
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : Never swallow exceptions; wrap only to add context with parameter name and offending value when relevant
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-09-30T03:06:24.792Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1423
File: src/e-k/Kama/Kama.StreamHub.cs:80-144
Timestamp: 2025-09-30T03:06:24.792Z
Learning: In C# code within the Stock.Indicators repository, never use the null-forgiving operator (!) as it is considered a code smell. Use alternative approaches such as `.Value` property access on nullable types when null checks are already in place, or proper null-conditional operators.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2026-01-01T06:10:39.665Z
Learnt from: CR
Repo: DaveSkender/Stock.Indicators PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-01-01T06:10:39.665Z
Learning: Applies to src/**/*.cs : Ensure stateless calculations are thread-safe; streaming hubs must isolate instance state (no static mutable fields)
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-27T21:10:12.886Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 1627
File: src/m-r/ParabolicSar/ParabolicSar.StreamHub.cs:12-12
Timestamp: 2025-10-27T21:10:12.886Z
Learning: Inline comments (e.g., "// State variables") used for organizational purposes within classes are acceptable in the Stock.Indicators codebase. The Roslynator RCS1181 diagnostic for converting comments to documentation comments should be suppressed, as it's considered unnecessary noise.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
📚 Learning: 2025-10-31T04:56:01.231Z
Learnt from: DaveSkender
Repo: DaveSkender/Stock.Indicators PR: 0
File: :0-0
Timestamp: 2025-10-31T04:56:01.231Z
Learning: ZigZagHub must override Rebuild() to use ProviderCache.ToZigZag() for full recalculation and call NotifyObserversOnRebuild() to notify chain observers. ToIndicator should simply return cached results. RollbackState should call Rebuild(DateTime.MinValue) to trigger full recalculation.
Applied to files:
src/_common/StreamHub/ReusableObserver.cs
⏰ Context from checks skipped due to timeout of 400000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
src/_common/StreamHub/ReusableObserver.cs (3)
1-9: LGTM: Class declaration is well-structured.The generic constraint and interface implementation are correct. The XML documentation clearly describes the purpose and flexibility of the class.
80-106: LGTM: Constructor implementation is correct.The constructor properly validates the required
isSubscribedparameter usingArgumentNullException.ThrowIfNulland assigns all fields correctly. As per coding guidelines, this follows the principle of validating all required user inputs.
108-231: LGTM: All lifecycle methods correctly delegate to callbacks.The implementation properly delegates all
IStreamObserver<TResult>members to their corresponding callbacks. The use of null-conditional operators (?.) ensures callbacks are only invoked when provided, and no exceptions are swallowed, which aligns with the coding guidelines.
|
If I’m interpreting your intent correctly, you want:
To be honest, I’m struggling to see the value in either of these (pun intended). I’d probably need to see more of what this would improve outside the library to truly see it (e.g. before this vs. with this). |
|
I challenged copilot to put an RsiHub and MacdHub in the same List<> and IChainProvider was THE shared interface it found. Everything else doesn't work due to invariance in the interfaces. This does work, and gets me access to Value via So, now I can repeatedly poll all my hubs and copy Value somewhere... If only there was some kind of universal thing I could subscribe to the ends of all my hub chains to actually DO something with the calculated indicator value(s)... invoke UI callbacks... draw candles... persist... hello world... anything. Quotes > Indicators > X I am struggling to figure out what X is, for you. I think I'm on the right track--- Current option: New option: Datagrid only cares about Results.Last().Value: All for this: |
I'm working up some better examples to demonstrate how to use Hubs in an event-driven streaming architecture. In a nutshell, if you're polling hubs ... you're doing it wrong! → you really do need a WebSocket or SSE implementation for quote feeds, The referenced PR will also include a |
|
As a utility class, I think if anything, we might consider something like: public abstract class HubObserver<TIn> : IStreamObserver<TIn>
where TIn : ISeries
{
…
// all the implementations, some as `virtual`
// from src/_common/StreamHub/StreamHub.Observer.cs
// e.g. has clean standard stuff like Subscribe()
}then users can do whatever they want in implementations outside the library, like public class MyObserver<T> : HubObserver<T>
where T : IReusable
{
// whatevers or overrides
}// usage
QuoteHub quoteHub = new();
SmaHub smaHub = quoteHub.ToSma(); // subscribes to quotes
MyObserver myObserver = new();
smaHub.Subscribe(myObserver): // subscribes to SMA (long form)
HubCollection<IReusable> = [
quoteHub,
smaHub,
myObserver,
…
]; |
|
|
I've realized what I was trying to implement was just a SomethingChanged notification: The indicators update when you Add() to the quotehub... The upgrades this provoked are appreciated, but my original use-case was/is dumb. 👍 |
|
"I have not failed. I've just found 10,000 ways that won't work." It's all helpful! Appreciated, for sure. |
2 participants
Everything I was trying to gain access to by modifying the base interfaces (observables) was already available at the other end (observer).
This class generically implements a specific run-time configuration of
IStreamObserver<IReusable>.TODO: It could probably be made generic to provide type-safety to hard-wired instances.