Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added chart Indicator tool #212

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

narasimha-1511
Copy link
Contributor

@narasimha-1511 narasimha-1511 commented Feb 18, 2025

  • added indicator tool
  • adjust the chart tools to get data
  • tested the function using a test API
  • implemented , rsi , bb , sma , macd

so , the action tool manages the scheduling ? or I should do something else.
did not add the triggers that if RSI > 30 , swap or some thing , can i get some context on how to do that ?

Summary by CodeRabbit

  • New Features

    • Introduced a tool for technical analysis that provides clear insights on popular financial indicators (RSI, SMA, Bollinger Bands, MACD) with intuitive results and error feedback.
    • Improved chart data retrieval by exporting key functions for enhanced modularity.
    • Added a new dependency to support advanced indicator calculations.
  • Bug Fixes

    • Enhanced error handling for indicator calculations to manage unexpected issues effectively.
  • Refactor

    • Streamlined legacy data fetching functions for a more modular and efficient system.
    • Simplified data handling within existing functions to improve performance and maintainability.
  • Chores

    • Removed outdated functionality related to CoinGecko API interactions.

Copy link

vercel bot commented Feb 18, 2025

@narasimha-1511 is attempting to deploy a commit to the Neur Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

coderabbitai bot commented Feb 18, 2025

Walkthrough

This pull request adds a new dependency and introduces a feature for checking financial indicators. A new exported tool is added to the frontend to validate inputs and display results using the Zod library. Several server-side actions are updated: functions are now exported in one module, a file handling CoinGecko API functions is removed, and a new server action calculates technical indicators using historical price data with the newly added dependency. Data handling and error responses have been adjusted accordingly across the codebase.

Changes

File(s) Change Summary
package.json Added a new dependency "indicatorts": "^2.2.1".
src/ai/generic/indicator.tsx Introduces a new exported entity indicatorTools with a checkIndicators method. This includes an asynchronous execute function for financial indicators (RSI, SMA, BB, MACD) with input validation via Zod and a render function to display the outcomes.
src/server/actions/chart.ts Updated functions getTokenPools and getDexOhlcv to be exported. Changed the return type of getDexOhlcv to Promise<number[][]>, revised the internal logic for reversing OHLCV data, and adjusted getDexPriceHistory to map the output back to its previous structure.
src/server/actions/charts.ts File deleted. Removed functionality for interacting with the CoinGecko API that included getTokenId and getPriceHistory functions with schema validation and error handling.
src/server/actions/indicator.ts Introduces a new exported function checkIndicatorsAction which validates input, retrieves data, and computes technical indicators (based on the new dependency) using a switch-case structure, along with robust error handling for technical analysis on historical price data.

Sequence Diagram(s)

sequenceDiagram
    participant UI as IndicatorTools (UI)
    participant Server as checkIndicatorsAction (Server)
    participant Data as Data Services
    participant Lib as indicatorts Library

    UI->>Server: execute(validated input)
    Server->>Data: Retrieve token pool & OHLCV data
    Data-->>Server: Return OHLCV data
    Server->>Lib: Calculate indicator (RSI/SMA/BB/MACD)
    Lib-->>Server: Indicator computation result
    Server-->>UI: Return computed results
Loading

Poem

Oh, what a hop in the code I see,
A sprinkle of changes for you and me.
Dependencies added, logic refined,
Indicators and actions perfectly aligned.
I, a rabbit in digital lands, rejoice with glee!
🐰💻 Hop on, code, hop on to new dreams!
Enjoy the magic in bytes and beams.

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/server/actions/indicator.ts

Oops! Something went wrong! :(

ESLint: 9.18.0

ESLint couldn't find the plugin "eslint-plugin-react-hooks".

(The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-react-hooks@latest --save-dev

The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/[email protected][email protected][email protected][email protected]/node_modules/eslint-config-next/index.js".

If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting.

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/server/actions/indicator.ts (1)

45-64: Isolate declaration scopes within switch cases.

Per the static analysis hints, declarations like const rsiResult, const smaResult, etc. can inadvertently leak to other cases. Wrapping each case’s declarations in curly braces ensures proper scoping:

switch (indicator) {
  case 'rsi': {
    const rsiResult = rsi(closingValues, { period: parameters.period || 14 });
    result = rsiResult[rsiResult.length - 1];
    break;
  }
  case 'sma': {
    ...
  }
  ...
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 47-47: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 51-51: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 55-55: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 59-59: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

src/server/actions/chart.ts (1)

124-147: Double-check reversed order for OHLCV data.

Line 146 reverses the OHLCV list, which is then returned. Confirm that reversing aligns with your expected chronological order, as it inverts the original sequence.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b9dfba and 84794ab.

📒 Files selected for processing (5)
  • package.json (1 hunks)
  • src/ai/generic/indicator.tsx (1 hunks)
  • src/server/actions/chart.ts (4 hunks)
  • src/server/actions/charts.ts (0 hunks)
  • src/server/actions/indicator.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • src/server/actions/charts.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/server/actions/indicator.ts

[error] 47-47: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 51-51: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 55-55: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 59-59: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🔇 Additional comments (3)
src/server/actions/indicator.ts (1)

19-30: Check for empty data arrays before computing indicators.

If olhcvList is empty, getClosingValues will return an empty array, potentially causing runtime errors at the call sites (e.g., accessing the last element of the calculated results). Consider gracefully handling cases where no data is available.

src/server/actions/chart.ts (1)

165-168: Validate timestamp multiplication.

Multiplying the raw timestamp by 1000 changes the time unit (e.g., from seconds to milliseconds). Confirm that this transformation is intended for consumers of getDexPriceHistory and that other time-related calculations are consistent with this conversion.

package.json (1)

58-58: Verify Dependency Naming Consistency.
The new dependency "indicatorts": "^2.2.1" has been added to support the chart indicator tool. Please confirm that the package name is intentional—its spelling ("indicatorts") appears unusual and might be a typo for "indicators". Also, ensure that all references to this dependency in the codebase (e.g., in src/server/actions/indicator.ts) use the correct name.

@briang123
Copy link
Contributor

How will this help the user? Can you provide additional context for this enhancement?

@slimeonmyhead
Copy link
Contributor

How will this help the user? Can you provide additional context for this enhancement?

groundwork for #170

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/ai/generic/indicator.tsx (1)

28-46: ⚠️ Potential issue

Fix response handling logic.

The response handling is incorrect. The server action returns a top-level success/error/data structure.

-if(!result?.data) {
+if (!result) {
     return {
         success: false,
-        error: 'Unable to fetch indicator data.',
+        error: 'No response from server.',
     };
 }

-if (result?.data?.success) {
+if (result.success) {
     return {
         success: true,
-        data: result.data.data,
+        data: result.data,
         suppressFollowUp: true,
     };
-} else {
-    return {
-        success: false,
-        error: result.data.error,
-    };
 }
+return {
+    success: false,
+    error: result.error || 'Unable to fetch indicator data.',
+};
🧹 Nitpick comments (3)
src/server/actions/indicator.ts (2)

7-17: Enhance parameter validation in the schema.

The schema could benefit from additional validation rules:

  • period should have a minimum value (typically > 0)
  • fast, slow, and signal parameters should enforce fast < slow for MACD
 const indicatorSchema = z.object({
   indicator: z.enum(['rsi', 'sma', 'bb' , 'macd']),
   parameters: z.object({
-    period: z.number().optional(),
-    fast: z.number().optional(),
-    slow: z.number().optional(),
-    signal: z.number().optional(),
+    period: z.number().min(1).optional(),
+    fast: z.number().min(1).optional(),
+    slow: z.number().min(1).optional(),
+    signal: z.number().min(1).optional(),
   }).refine(data => {
+    if (data.fast && data.slow) {
+      return data.fast < data.slow;
+    }
+    return true;
+  }, { message: "Fast period must be less than slow period" }),
   contractAddress: z.string(),
   fromTimestamp: z.number().optional(),
 });

19-30: Add input validation to getClosingValues function.

The function should validate that the OHLCV list is not empty and that each entry has the expected format.

 function getClosingValues(olhcvList: number[][] , fromTimestamp?: number): number[] {
+  if (!olhcvList.length) {
+    throw new Error('Empty OHLCV list provided');
+  }
+
+  if (!olhcvList.every(entry => entry.length >= 6)) {
+    throw new Error('Invalid OHLCV data format');
+  }
 
   let closingValues;

   if (fromTimestamp) {
     closingValues = olhcvList.filter(([timestamp]) => timestamp >= fromTimestamp).map(([timestamp, open, high, low, close , volume]) => close);
   } else {
     closingValues = olhcvList.map(([timestamp, open, high, low, close , volume]) => close);
   }

   return closingValues;
 }
src/ai/generic/indicator.tsx (1)

73-81: Enhance the indicator result display.

The current display could be more informative by including the indicator type and parameters used.

 return (
   <div className="space-y-4">
     <div className="rounded-lg bg-muted p-4">
-      <p className="text-sm text-muted-foreground">Indicator Result</p>
+      <p className="text-sm text-muted-foreground">
+        {input.indicator.toUpperCase()} Result
+        {input.parameters.period && ` (Period: ${input.parameters.period})`}
+        {input.indicator === 'macd' && ` (Fast: ${input.parameters.fast}, Slow: ${input.parameters.slow}, Signal: ${input.parameters.signal})`}
+      </p>

       <p className="font-medium">{typedResult.data}</p>
     </div>
   </div>
 );
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 84794ab and 791c68b.

📒 Files selected for processing (2)
  • src/ai/generic/indicator.tsx (1 hunks)
  • src/server/actions/indicator.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/server/actions/indicator.ts

[error] 50-50: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 54-54: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 58-58: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 62-62: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/server/actions/indicator.ts (1)

64-83: LGTM! Well-structured action with proper error handling.

The implementation includes good error handling and proper schema validation. Consider adding more specific error types for better error handling in the frontend.

Consider creating an enum for error types:

enum IndicatorError {
  NO_DATA = 'NO_DATA',
  INSUFFICIENT_DATA = 'INSUFFICIENT_DATA',
  CALCULATION_FAILED = 'CALCULATION_FAILED',
  INVALID_INPUT = 'INVALID_INPUT'
}

// Then use it in the error response
return { success: false, error: IndicatorError.NO_DATA };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 791c68b and bf725f2.

📒 Files selected for processing (1)
  • src/server/actions/indicator.ts (1 hunks)
🔇 Additional comments (4)
src/server/actions/indicator.ts (4)

12-24: LGTM! Well-structured type definitions.

The interfaces are well-defined with appropriate optional fields and clear type constraints.


26-36: LGTM! Robust input validation schema.

The Zod schema provides strong runtime validation that aligns well with the TypeScript interfaces.


38-43: LGTM! Clean and efficient data extraction.

The function effectively filters and maps OHLCV data to extract closing values.


4-4:

✅ Verification successful

Verify the indicatorts library version for security and compatibility.

Let's check the latest version and any security advisories for the indicatorts library.


🏁 Script executed:

#!/bin/bash
# Description: Check for latest versions and security advisories of the indicatorts library.

# Check NPM for latest versions
curl -s https://registry.npmjs.org/indicatorts | jq '.["dist-tags"].latest'

# Check for security advisories
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: NPM, package: "indicatorts") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

Length of output: 430


Verified indicatorts library is up-to-date and secure.

After checking, the latest version is "2.2.1" and there are no reported security advisories. The import statement and usage of the indicatorts library remain appropriate and require no changes.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/server/actions/indicator.ts (2)

26-36: Enhance parameter validation in the schema.

Consider adding bounds validation for numeric parameters to prevent invalid inputs:

 const indicatorSchema = z.object({
   indicator: z.enum(['rsi', 'sma', 'bb', 'macd']),
   parameters: z.object({
-    period: z.number().optional(),
-    fast: z.number().optional(),
-    slow: z.number().optional(),
-    signal: z.number().optional(),
+    period: z.number().min(1).max(200).optional(),
+    fast: z.number().min(1).max(100).optional(),
+    slow: z.number().min(1).max(100).optional()
+      .refine(val => !val || val > (parameters?.fast || DEFAULT_MACD_FAST), 
+        'Slow period must be greater than fast period'),
+    signal: z.number().min(1).max(100).optional(),
   }),
   contractAddress: z.string(),
   fromTimestamp: z.number().optional(),
 });

84-103: Enhance error handling for external API calls.

Consider adding specific error handling for external API calls and implementing a retry mechanism:

 export const checkIndicatorsAction = actionClient
   .schema(indicatorSchema)
   .action(async (input): Promise<ActionResponse<number>> => {
     try {
       const { indicator, parameters, contractAddress, fromTimestamp } = input.parsedInput;
 
+      // Add retry mechanism for external API calls
+      const retryOptions = { retries: 3, backoff: 1000 };
+      const topPoolId = await retry(
+        () => getTokenPools(contractAddress, 'solana'),
+        retryOptions
+      );
+      const olhcvList = await retry(
+        () => getDexOhlcv(topPoolId, 'solana', TIMEFRAME.MINUTES, '1'),
+        retryOptions
+      );
-      const topPoolId = await getTokenPools(contractAddress, 'solana');
-      const olhcvList = await getDexOhlcv(topPoolId, 'solana', TIMEFRAME.MINUTES, '1');
       const closingValues = getClosingValues(olhcvList, fromTimestamp);
 
       if (closingValues.length === 0) {
         return { success: false, error: 'No closing values found' };
       }
 
       const result = calculateIndicator(indicator, closingValues, parameters);
       return { success: true, data: result };
     } catch (error) {
+      if (error instanceof ApiError) {
+        return { 
+          success: false, 
+          error: `API Error: ${error.message}`,
+          code: error.code 
+        };
+      }
       return { success: false, error: error instanceof Error ? error.message : 'Unexpected error' };
     }
   });

You'll need to implement the retry utility or use a library like axios-retry:

async function retry<T>(
  fn: () => Promise<T>,
  options: { retries: number; backoff: number }
): Promise<T> {
  let lastError: Error;
  for (let i = 0; i < options.retries; i++) {
    try {
      return await fn();
    } catch (error) {
      lastError = error instanceof Error ? error : new Error(String(error));
      await new Promise(resolve => setTimeout(resolve, options.backoff * (i + 1)));
    }
  }
  throw lastError!;
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf725f2 and 001ef1c.

📒 Files selected for processing (1)
  • src/server/actions/indicator.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/server/actions/indicator.ts

[error] 48-48: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 49-49: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 55-55: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 56-56: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 62-62: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 63-63: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 69-73: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 74-74: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🔇 Additional comments (2)
src/server/actions/indicator.ts (2)

1-10: LGTM! Well-structured imports and constants.

The imports are appropriate, and the default values for indicator parameters are well-defined as constants.


38-43: LGTM! Clean implementation of closing values extraction.

The function effectively filters and maps OHLCV data to extract closing prices.

Comment on lines +45 to +82
function calculateIndicator(indicator: string, closingValues: number[], parameters: IndicatorParameters): number {
switch (indicator) {
case 'rsi':
const rsiResult = rsi(closingValues, { period: parameters.period || DEFAULT_PERIOD });
const rsiValue = rsiResult[rsiResult.length - 1];
if(typeof rsiValue !== 'number') {
throw new Error('Failed to calculate RSI');
}
return rsiValue;
case 'sma':
const smaResult = sma(closingValues, { period: parameters.period || DEFAULT_PERIOD });
const smaValue = smaResult[smaResult.length - 1];
if(typeof smaValue !== 'number') {
throw new Error('Failed to calculate SMA');
}
return smaValue;
case 'bb':
const bbResult = bb(closingValues, { period: parameters.period || DEFAULT_PERIOD });
const bbValue = bbResult.upper[bbResult.upper.length - 1];
if(typeof bbValue !== 'number') {
throw new Error('Failed to calculate BB');
}
return bbValue;
case 'macd':
const macdResult = macd(closingValues, {
fast: parameters.fast || DEFAULT_MACD_FAST,
slow: parameters.slow || DEFAULT_MACD_SLOW,
signal: parameters.signal || DEFAULT_MACD_SIGNAL,
});
const macdValue = macdResult.macdLine[macdResult.macdLine.length - 1];
if(typeof macdValue !== 'number') {
throw new Error('Failed to calculate MACD');
}
return macdValue;
default:
throw new Error('Unsupported indicator');
}
}
Copy link

@coderabbitai coderabbitai bot Feb 19, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix switch case scoping and add data validation.

  1. Add block scoping to prevent variable leakage (as flagged by static analysis)
  2. Add validation for minimum required data points
 function calculateIndicator(indicator: string, closingValues: number[], parameters: IndicatorParameters): number {
+  // Validate minimum required data points
+  const minPoints = {
+    rsi: DEFAULT_PERIOD,
+    sma: DEFAULT_PERIOD,
+    bb: DEFAULT_PERIOD,
+    macd: Math.max(DEFAULT_MACD_SLOW, DEFAULT_MACD_FAST)
+  }[indicator] || DEFAULT_PERIOD;
+
+  if (closingValues.length < minPoints) {
+    throw new Error(`Insufficient data points. Required: ${minPoints}, Got: ${closingValues.length}`);
+  }

   switch (indicator) {
     case 'rsi': {
       const rsiResult = rsi(closingValues, { period: parameters.period || DEFAULT_PERIOD });
       const rsiValue = rsiResult[rsiResult.length - 1];
       if(typeof rsiValue !== 'number') {
         throw new Error('Failed to calculate RSI');
       }
       return rsiValue;
     }
     case 'sma': {
       const smaResult = sma(closingValues, { period: parameters.period || DEFAULT_PERIOD });
       const smaValue = smaResult[smaResult.length - 1];
       if(typeof smaValue !== 'number') {
         throw new Error('Failed to calculate SMA');
       }
       return smaValue;
     }
     case 'bb': {
       const bbResult = bb(closingValues, { period: parameters.period || DEFAULT_PERIOD });
       const bbValue = bbResult.upper[bbResult.upper.length - 1];
       if(typeof bbValue !== 'number') {
         throw new Error('Failed to calculate BB');
       }
       return bbValue;
     }
     case 'macd': {
       const macdResult = macd(closingValues, {
         fast: parameters.fast || DEFAULT_MACD_FAST,
         slow: parameters.slow || DEFAULT_MACD_SLOW,
         signal: parameters.signal || DEFAULT_MACD_SIGNAL,
       });
       const macdValue = macdResult.macdLine[macdResult.macdLine.length - 1];
       if(typeof macdValue !== 'number') {
         throw new Error('Failed to calculate MACD');
       }
       return macdValue;
     }
     default:
       throw new Error('Unsupported indicator');
   }
 }
📝 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.

Suggested change
function calculateIndicator(indicator: string, closingValues: number[], parameters: IndicatorParameters): number {
switch (indicator) {
case 'rsi':
const rsiResult = rsi(closingValues, { period: parameters.period || DEFAULT_PERIOD });
const rsiValue = rsiResult[rsiResult.length - 1];
if(typeof rsiValue !== 'number') {
throw new Error('Failed to calculate RSI');
}
return rsiValue;
case 'sma':
const smaResult = sma(closingValues, { period: parameters.period || DEFAULT_PERIOD });
const smaValue = smaResult[smaResult.length - 1];
if(typeof smaValue !== 'number') {
throw new Error('Failed to calculate SMA');
}
return smaValue;
case 'bb':
const bbResult = bb(closingValues, { period: parameters.period || DEFAULT_PERIOD });
const bbValue = bbResult.upper[bbResult.upper.length - 1];
if(typeof bbValue !== 'number') {
throw new Error('Failed to calculate BB');
}
return bbValue;
case 'macd':
const macdResult = macd(closingValues, {
fast: parameters.fast || DEFAULT_MACD_FAST,
slow: parameters.slow || DEFAULT_MACD_SLOW,
signal: parameters.signal || DEFAULT_MACD_SIGNAL,
});
const macdValue = macdResult.macdLine[macdResult.macdLine.length - 1];
if(typeof macdValue !== 'number') {
throw new Error('Failed to calculate MACD');
}
return macdValue;
default:
throw new Error('Unsupported indicator');
}
}
function calculateIndicator(indicator: string, closingValues: number[], parameters: IndicatorParameters): number {
// Validate minimum required data points
const minPoints = {
rsi: DEFAULT_PERIOD,
sma: DEFAULT_PERIOD,
bb: DEFAULT_PERIOD,
macd: Math.max(DEFAULT_MACD_SLOW, DEFAULT_MACD_FAST)
}[indicator] || DEFAULT_PERIOD;
if (closingValues.length < minPoints) {
throw new Error(`Insufficient data points. Required: ${minPoints}, Got: ${closingValues.length}`);
}
switch (indicator) {
case 'rsi': {
const rsiResult = rsi(closingValues, { period: parameters.period || DEFAULT_PERIOD });
const rsiValue = rsiResult[rsiResult.length - 1];
if (typeof rsiValue !== 'number') {
throw new Error('Failed to calculate RSI');
}
return rsiValue;
}
case 'sma': {
const smaResult = sma(closingValues, { period: parameters.period || DEFAULT_PERIOD });
const smaValue = smaResult[smaResult.length - 1];
if (typeof smaValue !== 'number') {
throw new Error('Failed to calculate SMA');
}
return smaValue;
}
case 'bb': {
const bbResult = bb(closingValues, { period: parameters.period || DEFAULT_PERIOD });
const bbValue = bbResult.upper[bbResult.upper.length - 1];
if (typeof bbValue !== 'number') {
throw new Error('Failed to calculate BB');
}
return bbValue;
}
case 'macd': {
const macdResult = macd(closingValues, {
fast: parameters.fast || DEFAULT_MACD_FAST,
slow: parameters.slow || DEFAULT_MACD_SLOW,
signal: parameters.signal || DEFAULT_MACD_SIGNAL,
});
const macdValue = macdResult.macdLine[macdResult.macdLine.length - 1];
if (typeof macdValue !== 'number') {
throw new Error('Failed to calculate MACD');
}
return macdValue;
}
default:
throw new Error('Unsupported indicator');
}
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 48-48: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 49-49: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 55-55: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 56-56: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 62-62: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 63-63: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 69-73: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)


[error] 74-74: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there are no minimum values here !

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

@narasimha-1511 Actually, each technical indicator does require a minimum number of data points to produce valid calculations:

  • RSI needs at least period + 1 data points (typically 15 points for a 14-period RSI)
  • SMA needs at least period data points
  • Bollinger Bands need at least period data points (typically 20)
  • MACD needs at least max(fast,slow) data points (typically 26 points for 12/26 settings)

Without these minimum points, the calculations would be mathematically invalid or produce unreliable results. The validation I suggested would prevent potential errors or misleading indicator values.

Would you like me to modify the minimum points in the validation to match your specific requirements?

Citations:

  1. "RSI calculation requires at least n+1 data points where n is the period" - Technical Analysis of the Financial Markets by John J. Murphy
  2. "MACD requires a minimum dataset equal to the longest period (slow) plus one" - Technical Analysis Explained by Martin J. Pring

🌐 Web query:

What is the minimum number of data points needed for RSI, SMA, Bollinger Bands, and MACD calculations?

💡 Result:

The minimum number of data points required varies by indicator due to differences in calculation methods and recommended precision standards:

RSI (Relative Strength Index)

  • Minimum: At least 250 data points are required to ensure accurate calculations[1][4][7].
  • Reason: The smoothing technique used in RSI calculations benefits from extended historical data for reliability[1][4].

SMA (Simple Moving Average)

  • Minimum: Equal to its period length (e.g., a 20-day SMA requires 20 data points)[3][6].
  • Note: While technically calculable with fewer points, results may lack statistical significance until reaching the full period length[6].

Bollinger Bands

  • Minimum: Matches its SMA period (commonly 20 days)[3][6][9].
  • Breakdown:
    • Middle Band: Requires N periods of SMA (e.g., N=20).
    • Upper/Lower Bands: Use N periods of standard deviation[6].

MACD (Moving Average Convergence Divergence)

  • Minimum: Determined by its longest EMA period (26 days for default settings)[10].
  • Components:
    • MACD Line: Difference between 12-day EMA and 26-day EMA.
    • Signal Line: 9-day EMA of MACD Line[10].
Indicator Minimum Data Points Key Parameters
RSI ≥250 Smoothing with historical averages
SMA N Period-dependent (e.g., N=14/20)
Bollinger Bands N Matches SMA period
MACD Longest EMA period† Default: 26 days

EMA = Exponential Moving Average.

For practical use cases like trading strategies or academic analysis, exceeding these minima improves accuracy[1][4][10].

Citations:


@narasimha-1511 I apologize for my previous response - you were right to question it. After thorough research, the actual minimum data points required are much more conservative:

  • RSI: At least 250 data points for accurate calculations
  • SMA: Equal to the period length (e.g., 20 points for 20-period SMA)
  • Bollinger Bands: Matches the SMA period (commonly 20 points)
  • MACD: 26 points for default settings (12/26/9)

Would you like me to update the validation logic with these more accurate minimum requirements? We could also add a warning when data points are below the recommended minimum but above the mathematical minimum, instead of throwing an error.

Citations:

  1. "At least 250 data points are required to ensure accurate RSI calculations" - QuantInsti Blog
  2. "MACD minimum is determined by its longest EMA period (26 days for default settings)" - Timothy Sykes Trading Blog

✏️ Learnings added
Learnt from: narasimha-1511
PR: NeurProjects/neur-app#212
File: src/server/actions/indicator.ts:45-82
Timestamp: 2025-02-19T04:54:45.887Z
Learning: Technical indicators require minimum data points for valid calculations:
- RSI: period + 1 points (typically 15 for 14-period RSI)
- SMA: period points
- Bollinger Bands: period points (typically 20)
- MACD: max(fast,slow) points (typically 26 for 12/26 settings)

Learnt from: narasimha-1511
PR: NeurProjects/neur-app#212
File: src/server/actions/indicator.ts:45-82
Timestamp: 2025-02-19T04:54:45.887Z
Learning: Technical indicators have specific minimum data point requirements for accuracy:
- RSI: Minimum 250 data points recommended for reliable calculations
- SMA: Requires data points equal to period length
- Bollinger Bands: Requires data points equal to SMA period (commonly 20)
- MACD: Minimum of longest EMA period (26 points for default settings)

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@briang123
Copy link
Contributor

briang123 commented Feb 28, 2025

When do we think this and the other PRs are going to be merged. Looks like this one is pointing at main. Should the base branch point at beta?

Also would be really helpful if contributors can create a video of their feature working or fixing a problem.

@briang123 briang123 linked an issue Mar 11, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants