feat: Add 30 missing indicators to chart options#441
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe pull request introduces a three-part expansion of the indicator implementation: a planning document outlining the missing indicators implementation strategy with completion status and complexity notes; 27 new HTTP GET endpoints for various indicators (AWESOME, BOP, CHAIKIN, DEMA, DPO, FORCE, HMA, HURST, KAMA, KVO, MAMA, MA-ENV, OBV, PMO, PVO, ROCWB, SMMA, STDEV-CH, T3, TEMA, TRIX, TSI, ULTIMATE, VWAP, VWMA, VOL-STOP, WILLIAMS R, WMA) that delegate to existing indicator methods; and metadata configuration entries for these indicators specifying parameters, display properties, and chart rendering details. Possibly related PRs
Comment |
Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
server/WebApi/Endpoints.cs (1)
57-59: Endpoints not in alphabetical order as stated.The comment on line 51 indicates indicators are "sorted alphabetically", but new endpoints like
AWESOMEare inserted out of order (placed between ADL and ADX instead of after ATR-STOP-HL). Consider reordering for consistency.
📜 Review details
Configuration used: Repository: facioquo/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/plans/missing-indicators-implementation.md(1 hunks)server/WebApi/Endpoints.cs(13 hunks)server/WebApi/Services/Service.Metadata.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
{**/*.md,package.json,pnpm-workspace.yaml,Charts.sln}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update documentation when changing project structure
Files:
docs/plans/missing-indicators-implementation.md
**/*
📄 CodeRabbit inference engine (.github/instructions/code-completion.instructions.md)
**/*: Code must pass formatting checks usingpnpm run format:checkwith no issues
All formatting and linting checks must pass before committing code
Files:
docs/plans/missing-indicators-implementation.mdserver/WebApi/Services/Service.Metadata.csserver/WebApi/Endpoints.cs
**/*.md
📄 CodeRabbit inference engine (.github/instructions/code-completion.instructions.md)
**/*.md: README and documentation must be updated when introducing new features
Setup and configuration instructions must be updated when configuration changes are made
All markdown files must pass linting usingpnpm run lint:mdwith no issues
Files:
docs/plans/missing-indicators-implementation.md
⚙️ CodeRabbit configuration file
Check for broken links and formatting according to markdown configuration and instructions
Files:
docs/plans/missing-indicators-implementation.md
**/{*.md,docs/**,server/**/*.cs}
📄 CodeRabbit inference engine (.github/instructions/code-completion.instructions.md)
API endpoint documentation must be updated when API changes are made
Files:
docs/plans/missing-indicators-implementation.mdserver/WebApi/Services/Service.Metadata.csserver/WebApi/Endpoints.cs
**/*.{md,markdown,.markdownlint-cli2.jsonc}
📄 CodeRabbit inference engine (.github/instructions/markdown.instructions.md)
Run
npx markdownlint-cli2 --no-globs {glob} --fixin the terminal, fix any items not auto-fixed, followed bynpx markdownlint-cli2 --no-globs {glob}to verify there are no remaining linting issues, before opening a pull request. Always use--no-globswhen specifying explicit file paths to prevent the tool from expanding globs defined in.markdownlint-cli2.jsonc.
Files:
docs/plans/missing-indicators-implementation.md
**/*.{md,markdown}
📄 CodeRabbit inference engine (.github/instructions/markdown.instructions.md)
**/*.{md,markdown}: Use present tense and imperative mood in Markdown content.
Exclude historical context and migration details from Markdown content; keep each rule as a current directive.
Headings and bold labels in Markdown follow sentence case: first word + proper nouns only.
Maintain a single source of truth per concept in Markdown content; reference existing documents instead of duplicating content.
Give each Markdown document a single purpose; keep conceptual, procedural, and configuration guidance distinct.
Use ATX (#) headers in Markdown with sentence case only, using sequential hierarchy with blank lines around headers.
Always use hyphens (-) for bullets in Markdown lists; indent nested lists with two spaces.
Avoid ordered lists in Markdown unless sequence matters.
Use fenced code blocks with a language identifier (useplaintextif needed) in Markdown; include blank lines around fences.
Avoid inline comments in Markdown code fences.
Increase fence length when nesting Markdown code blocks; outer fences need more backticks than inner fences.
Do not wrap#file:context tokens in backticks; do not place punctuation immediately after the token. Use relative paths.
Minimize cascading hierarchies of#file:references in Markdown files; avoid chains like AGENTS.md → instruction file → context file.
Use@AgentNamesyntax when referencing custom agents. In documentation, wrap in backticks for readability (e.g.,@Planner); in agent files, use plain@AgentNamewithout backticks for invocation.
Use#tool:nameformat for tool references in Markdown; do not wrap in backticks.
Keep Markdown files under ~500 lines when possible; split files >800 lines unless they are cohesive. Prefer refactoring for succinctness before splitting.
Split Markdown files by concepts, workflows, or functional areas when each major section (h2) could stand alone, when document serves multiple distinct audiences, or when navigation becomes difficult due to excessive scrolling....
Files:
docs/plans/missing-indicators-implementation.md
server/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
server/**/*.cs: Use latest C# language features (.NET 9+) with async/await patterns for I/O operations
Use record types for DTOs in .NET backend
Follow Microsoft coding conventions for C# code
Build all projects successfully with 'dotnet build Charts.sln' from root directoryBackend .NET solution must build successfully using
dotnet build Charts.slnwith zero compilation errors or warnings
Files:
server/WebApi/Services/Service.Metadata.csserver/WebApi/Endpoints.cs
**/*.cs
📄 CodeRabbit inference engine (.github/instructions/code-completion.instructions.md)
Backend C# files must pass
pnpm run format:dotnet:checkwith no issues
Files:
server/WebApi/Services/Service.Metadata.csserver/WebApi/Endpoints.cs
**/*.{ts,tsx,cs}
📄 CodeRabbit inference engine (.github/instructions/code-completion.instructions.md)
**/*.{ts,tsx,cs}: Public methods must include JSDoc documentation for TypeScript/JavaScript code or XML documentation for C#
Interface and type definitions must include comprehensive JSDoc/XML documentation
Files:
server/WebApi/Services/Service.Metadata.csserver/WebApi/Endpoints.cs
**/*.{ts,tsx,cs,js,jsx}
📄 CodeRabbit inference engine (.github/instructions/code-completion.instructions.md)
Complex business logic and algorithms must include explanatory comments
Files:
server/WebApi/Services/Service.Metadata.csserver/WebApi/Endpoints.cs
🧠 Learnings (1)
📚 Learning: 2025-12-16T08:43:19.330Z
Learnt from: CR
Repo: facioquo/stock-charts PR: 0
File: .github/instructions/code-completion.instructions.md:0-0
Timestamp: 2025-12-16T08:43:19.330Z
Learning: Applies to **/{*.md,docs/**,server/**/*.cs} : API endpoint documentation must be updated when API changes are made
Applied to files:
server/WebApi/Endpoints.cs
🧬 Code graph analysis (2)
server/WebApi/Services/Service.Metadata.cs (3)
client/src/app/pages/chart/chart.models.ts (3)
IndicatorListing(38-49)ChartConfig(74-78)ChartFill(88-92)server/WebApi/Services/Service.Metadata.Helpers.cs (4)
ChartConfig(6-7)ChartConfig(9-10)ChartConfig(12-13)ChartConfig(15-41)server/WebApi/Models/ChartColors.cs (1)
ChartColors(3-20)
server/WebApi/Endpoints.cs (2)
server/WebApi/Services/Service.Storage.cs (6)
Task(5-5)Task(6-6)Task(7-7)Task(33-33)Task(35-36)Task(44-48)server/WebApi/Services/Service.Quotes.cs (4)
Task(7-7)Task(8-8)Task(22-23)Task(29-69)
⏰ 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 (7)
server/WebApi/Services/Service.Metadata.cs (1)
2603-3978: Well-structured indicator metadata additions.All 28 new indicator entries follow the established patterns consistently. The configurations include appropriate:
- Parameter definitions with sensible defaults and bounds
- Result configurations with correct
DataNamemappings- Chart configurations with relevant thresholds for oscillators
- Color assignments using the
ChartColorsconstantsserver/WebApi/Endpoints.cs (4)
253-255: Same null-handling pattern may be needed for PVO.Similar to the TSI observation,
GetPvopassessignalPeriodsdirectly while the metadata allowsMinimum = 0. Verify whether the library method accepts null for this parameter.
245-247: Good pattern for handling optional SMA periods.The conditional null conversion (
smaPeriods == 0 ? null : smaPeriods) correctly handles the case where users don't want an SMA line on the OBV chart. This pattern is also used for TRIX signal periods.
345-347: VWAP endpoint correctly has no parameters.VWAP is typically calculated from session start, making it parameter-free. The implementation correctly delegates without additional arguments.
329-331: GetTsi does not need the 0→null conversion. Unlike GetTrix (line 327) and GetObv (line 247), which use 0 as a convention to omit optional parameters, GetTsi's signalPeriods allows 0 as a valid value. The Stock.Indicators library accepts signalPeriods >= 0, where 0 explicitly means no signal line. Passing signalPeriods directly is correct.Likely an incorrect or invalid review comment.
docs/plans/missing-indicators-implementation.md (2)
1-80: Well-documented implementation plan.The document clearly:
- Tracks completed vs. remaining indicators with checkbox format
- Explains the implementation pattern for future reference
- Documents why certain complex indicators were deferred (dual-quote comparison, different data structures)
This provides good context for future maintainers.
64-64: The scriptpnpm run generate:backup-indicatorsis properly defined in package.json and exists at line 30.The verification confirms the documented command is correct. The script is defined as
"generate:backup-indicators": "node scripts/generate-backup-indicators.js"and is also referenced in thegenerate:backupscomposite script on line 32.
Co-authored-by: DaveSkender <8432125+DaveSkender@users.noreply.github.com>
docs/plans/missing-indicators-implementation.mdSummary
This PR adds 30 new chart indicators from the Skender.Stock.Indicators library:
Moving Averages (8): DEMA, HMA, KAMA, MAMA, SMMA, T3, TEMA, WMA
Oscillators (9): Awesome, BOP, Chaikin, DPO, PMO, TRIX, TSI, Ultimate, Williams %R
Volume-Based (5): OBV, PVO, VWAP, VWMA, KVO
Price Channels (2): MA Envelopes, Standard Deviation Channels
Other (6): Force Index, Hurst Exponent, ROC with Bands, Volatility Stop
Remaining Indicators Not Implemented
Some indicators require special handling and were not implemented:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.