Skip to content

feat: add volatility to candlestick plot#397

Merged
nk1408 merged 9 commits into
mainfrom
feat/candlestick-volatility
Jul 23, 2025
Merged

feat: add volatility to candlestick plot#397
nk1408 merged 9 commits into
mainfrom
feat/candlestick-volatility

Conversation

@nk1408

@nk1408 nk1408 commented Jul 21, 2025

Copy link
Copy Markdown
Collaborator

Pull Request

Description

Volatility as a First-Class Navigation Segment:

Volatility (high - low) is now included as a dedicated, accessible segment in candlestick chart navigation.
Users can directly navigate to the volatility value for each candle, just like OHLC segments.

Data-Driven Navigation Order:
Navigation order is now [volatility, ...value-sorted OHLC] for each candle.
Volatility is always the first stop when navigating UPWARD from the lower boundary.

Robust Boundary Handling:
When at the lower boundary, pressing UPWARD re-enters at the volatility segment.
When at the upper boundary, pressing DOWNWARD re-enters at the highest value segment (typically 'high').

Related Issues

Closes #394

Changes Made

src/model/candlestick.ts

Added volatility as a computed property for each candle and included it as the first segment in the navigation order.
Updated navigation logic to treat volatility as a first-class, accessible stop: UPWARD from the lower boundary always lands on volatility, and all navigation is data-driven.
Implemented robust boundary handling and re-entry logic, ensuring volatility is always accessible and navigation is symmetric at both boundaries.

src/type/grammar.ts

Updated the CandlestickPoint type to include a required volatility: number property, ensuring type safety and consistent access to volatility values throughout the codebase.

Checklist

  • I have read the Contributor Guidelines.
  • I have performed a self-review of my own code and ensured it follows the project's coding standards.
  • I have tested the changes locally following ManualTestingProcess.md, and all tests related to this pull request pass.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation, if applicable.
  • I have added appropriate unit tests, if applicable.

@github-actions

Copy link
Copy Markdown

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@nk1408

nk1408 commented Jul 21, 2025

Copy link
Copy Markdown
Collaborator Author

braille.ts & navigation.ts changes are a result of linter change

@github-actions

Copy link
Copy Markdown

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@nk1408 nk1408 requested a review from jooyoungseo July 21, 2025 20:22
@jooyoungseo jooyoungseo requested a review from Copilot July 22, 2025 00:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds volatility as a first-class navigation segment in candlestick charts, allowing users to directly navigate to the volatility value (high - low) for each candle. The implementation treats volatility as the primary navigation stop when moving upward from the lower boundary, following a data-driven navigation order of [volatility, ...value-sorted OHLC].

  • Added volatility property to CandlestickPoint interface and computed volatility values
  • Implemented volatility-first navigation ordering with robust boundary handling
  • Enhanced navigation logic to support null segment states for boundary management

Reviewed Changes

Copilot reviewed 2 out of 5 changed files in this pull request and generated 6 comments.

File Description
src/type/grammar.ts Adds required volatility property to CandlestickPoint interface
src/model/candlestick.ts Implements volatility navigation with computed values, updated segment ordering, and enhanced boundary handling logic

Comment thread src/model/candlestick.ts
Comment thread src/model/candlestick.ts Outdated
Comment thread src/model/candlestick.ts Outdated
Comment thread src/model/candlestick.ts
Comment thread src/model/candlestick.ts
Comment thread src/model/candlestick.ts

@jooyoungseo jooyoungseo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Test B (Braille) representation between this branch and main. This PR has broken the B representation.

@github-actions

Copy link
Copy Markdown

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@nk1408

nk1408 commented Jul 22, 2025

Copy link
Copy Markdown
Collaborator Author

@jooyoungseo Professor, I have added row based min/max for candlestick plots, please let me know if the functionality is fine. Thanks

@jooyoungseo jooyoungseo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let’s round the volatility values to two decimal places.

@github-actions

Copy link
Copy Markdown

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@github-actions

Copy link
Copy Markdown

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@nk1408 nk1408 requested review from Copilot and jooyoungseo July 23, 2025 16:41

This comment was marked as outdated.

@nk1408 nk1408 requested a review from Copilot July 23, 2025 17:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds volatility as a first-class navigation segment to candlestick charts, enabling users to directly access volatility values (high - low) alongside traditional OHLC data. The change implements data-driven navigation where volatility becomes the first segment in the navigation order, followed by value-sorted OHLC segments.

  • Added volatility computation and navigation support to candlestick charts
  • Updated type definitions to include required volatility property
  • Modified braille service to support per-row min/max values for improved accessibility

Reviewed Changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 4 comments.

File Description
src/type/grammar.ts Added required volatility property to CandlestickPoint interface
src/service/braille.ts Enhanced braille encoder to support array-based min/max values for better accessibility
src/model/candlestick.ts Implemented volatility as navigable segment with data-driven navigation order
eslint.config.ts Updated ignore patterns for GitHub directory

Comment thread src/model/candlestick.ts Outdated
Comment thread src/model/candlestick.ts Outdated
Comment thread src/model/candlestick.ts Outdated
Comment thread src/model/candlestick.ts Outdated
@github-actions

Copy link
Copy Markdown

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@github-actions

Copy link
Copy Markdown

⚠️ Changes detected in model-related files

This PR contains changes to files that affect the model layer and unit tests. Before merging, please ensure:

  1. Run the unit tests locally:

    npm run test
  2. Update tests if necessary

@nk1408 nk1408 merged commit ce0facf into main Jul 23, 2025
5 checks passed
@nk1408 nk1408 deleted the feat/candlestick-volatility branch July 23, 2025 21:40
@xabilitylab

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 3.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released For issues/features released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add volatility as fifth stop point in candlestick charts

4 participants