feat: add short currency formatting and tooltips in accounts analytics sidepanel

[npv12]

Important

Adds short currency formatting and tooltips to Accounts Analytics Sidepanel for improved UI clarity.

• Behavior:
  • Adds formatShortCurrency to utils.ts for short currency formatting (e.g., 1.2K, 3.4L).
  • Integrates tooltips in AccountsAnalyticsSidepanel.tsx to show full currency values on hover.
  • Adjusts percentageChange calculation for investments in AccountsAnalyticsSidepanel.tsx.
• UI Components:
  • Updates Tooltip in tooltip.tsx to support skipProvider prop for nested tooltips.
  • Adds TooltipProvider wrapping in AccountsAnalyticsSidepanel.tsx for consistent tooltip behavior.
• Misc:
  • Adds getTransactionColor to utils.ts for dynamic text color based on transaction value.

^{This description was created by for daefd7c. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to e2654ad in 1 minute and 57 seconds. Click for details.

• Reviewed 123 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/lib/utils.ts:28

• Draft comment:
  Note that formatShortCurrency uses the 'en-IN' locale to extract a narrow currency symbol, while formatCurrency uses 'en-US'. This may cause inconsistencies for non-INR currencies. Consider aligning or making the locale configurable.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment is pointing out a potential inconsistency between two functions. However, the comment is somewhat speculative ("may cause inconsistencies") and doesn't definitively identify a bug. The use of 'en-IN' locale appears intentional since the function uses Indian numbering conventions (Crore, Lakh). The comment also suggests "consider aligning or making the locale configurable" which is more of a suggestion than identifying a clear issue. According to the rules, I should not keep speculative comments like "If X, then Y is an issue" - only if it's definitely an issue. This seems to fall into that category. The locale difference might actually be intentional design - the new function is specifically for Indian-style short formatting (Cr, L, K), so using 'en-IN' makes sense. The comment could be highlighting a legitimate design consideration that the author should think about, especially if non-INR currencies are passed to these functions. While there might be a design consideration here, the comment is speculative ("may cause inconsistencies") rather than identifying a definite bug. The author clearly chose 'en-IN' deliberately given the Indian numbering system being used. Without evidence of an actual problem, this is more of a "consider this" comment rather than a clear code change requirement. This comment is speculative and doesn't identify a definite issue. It's asking the author to "consider" something rather than pointing out a clear bug. The locale choice appears intentional given the Indian numbering conventions. This violates the rule against speculative comments.

2. frontend/lib/utils.ts:41

• Draft comment:
  The thresholds and suffixes (e.g., 'K', 'L', 'Cr') in formatShortCurrency are specific to the Indian numbering system. Ensure that this behavior is intentional for all supported currencies or consider parameterizing these values.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment falls into several problematic categories: 1) It asks the author to "ensure" something is intentional, which violates the rule about not asking authors to confirm intentions. 2) It uses "consider" which makes it speculative/advisory rather than pointing to a definite bug. 3) The function name is "formatShortCurrency" and defaults to INR, so using Indian numbering conventions appears intentional by design. 4) There's no evidence this is actually broken - it's just pointing out a design choice. The comment doesn't identify a clear bug that needs fixing. Could the author have accidentally hardcoded Indian conventions when they intended to support multiple currency systems? Maybe this is a legitimate internationalization concern that should be addressed before the code goes live? While internationalization could be a concern, the function is clearly designed for Indian currency (defaults to INR, uses en-IN locale, uses Indian abbreviations). If the author wanted multi-currency support, they would need to see that requirement. This comment is asking them to "ensure" and "consider" rather than pointing to a definite problem, which violates the rules. This comment should be deleted. It asks the author to "ensure" behavior is intentional and to "consider" changes, which are explicitly prohibited types of comments. The function appears intentionally designed for Indian currency formatting, and there's no evidence of an actual bug.

Workflow ID: wflow_QoPuGTElXgLNNxGU

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed daefd7c in 29 seconds. Click for details.

• Reviewed 13 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/components/custom/Dashboard/AccountsAnalyticsSidepanel.tsx:70

• Draft comment:
  Minor formatting change: the redundant parentheses were removed from the multiplication expression. This change does not affect the computation, but ensure consistency with your project's style guidelines.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

Workflow ID: wflow_C5Ez98mepyOivJ3n

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 87c5e91 in 2 minutes and 9 seconds. Click for details.

• Reviewed 378 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 8 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/components/custom/Dashboard/AccountsAnalyticsSidepanel.tsx:70

• Draft comment:
  Multiplying analytics.percentage_increase by -1 for investment accounts changes the sign. Confirm if this inversion meets the intended business logic.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is clearly asking the author to "confirm" something about their change. This directly violates the rule that says "Do NOT ask the PR author to confirm their intention, to explain, to double-check things, to ensure the behavior is intended." The comment doesn't point out a definite bug or issue - it's just asking for confirmation. Additionally, I can see that the change is intentional because there are multiple related changes in the diff: the txnBalance calculation also had its sign changed (line 84), and the getTransactionColor function is being called with -1 * account.percentageChange in multiple places (lines 261, 318). This suggests a systematic change to how signs are handled, not an accidental error. Could this be pointing out a real bug where the sign inversion is incorrect? Maybe the automated tool detected an inconsistency that would cause incorrect behavior. However, without seeing the actual data or understanding the full business logic, I can't be certain this is wrong. Even if there might be a bug, the comment is phrased as "Confirm if..." which is explicitly asking for verification rather than stating a definite issue. The rules are clear that we should not keep comments that ask the author to confirm, explain, or double-check. If the tool was certain there was a bug, it should have stated it definitively rather than asking for confirmation. This comment should be deleted because it asks the author to "confirm" their intention, which violates the explicit rule against asking authors to verify, explain, or double-check their changes. The comment doesn't identify a definite bug, just asks for confirmation.

2. frontend/components/custom/Dashboard/AccountsAnalyticsSidepanel.tsx:84

• Draft comment:
  TxnBalance calculation no longer inverts the current_balance value. Verify that removing the '-1 *' multiplier aligns with the correct financial logic.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

3. frontend/components/custom/Dashboard/AccountsAnalyticsSidepanel.tsx:92

• Draft comment:
  Remove the console.log before merging to production.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment is about a console.log statement that was added in the diff. The rules state that comments suggesting code quality refactors are good if they are actionable and clear. However, I need to consider if this is "obvious or unimportant" - removing console.log statements is a very common and well-known practice. The rules also say "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." Removing debug console.log statements is pretty obvious and would likely be caught in any reasonable code review process. This feels like a very minor, obvious issue that most developers would know to handle. While removing console.log is good practice, this is an extremely obvious and common code quality issue that any developer would know about. It's the kind of thing that could be caught by a linter or is so well-known that it doesn't need to be pointed out. The rules specifically say not to make obvious comments. Even though it's obvious, the console.log was explicitly added in this PR and is still present in the code. The comment is actionable (just delete the line) and clear. However, the rules emphasize not making obvious comments, and this is about as obvious as it gets - it's debugging code that was left in. This comment should be deleted. While technically correct, pointing out that a console.log should be removed is too obvious and unimportant to warrant a review comment. This is the type of minor issue that should be caught by linters or is so well-known that it doesn't need explicit mention.

4. frontend/components/custom/Dashboard/AccountsAnalyticsSidepanel.tsx:260

• Draft comment:
  Using getTransactionColor(-1 * account.percentageChange) to determine text color is non-obvious; confirm that negating the percentageChange correctly reflects the desired color semantics.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% This comment is explicitly asking the author to "confirm" something, which is a clear violation of the review rules. The comment doesn't point out a definite bug or provide actionable feedback - it's just expressing uncertainty and asking for verification. The author made deliberate changes to both the percentageChange calculation (line 71) and the color logic (lines 260-261), suggesting this was intentional. Without seeing the getTransactionColor function implementation, I cannot determine if this is actually wrong. The comment is speculative and asks for confirmation rather than identifying a concrete issue. Maybe there is actually a bug here - the percentageChange is negated at calculation time (line 71), then negated again when passed to getTransactionColor (line 261), which could be a double negation issue. However, I don't have access to the getTransactionColor function to know what it expects. Even if there might be a double negation issue, the comment doesn't clearly identify this as a bug - it just asks the author to "confirm" the behavior is correct. Without seeing getTransactionColor's implementation or understanding the full context, this is speculative. The comment should either clearly state what the bug is or be deleted. This comment violates the rule against asking authors to confirm their intentions. It's speculative and doesn't provide actionable feedback or identify a clear bug. It should be deleted.

5. frontend/components/ui/tooltip.tsx:24

• Draft comment:
  The skipProvider flag in the Tooltip component avoids nesting an extra TooltipProvider. Consider adding inline documentation to clarify its purpose.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is suggesting adding documentation, which is a code quality improvement. However, I need to check the rules carefully. The rules say "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." Adding documentation is not strictly a "required" code change - the code works without it. The purpose of skipProvider is relatively self-explanatory from its name and usage. This seems like a "nice to have" rather than a clear code change that's required. The rules emphasize only commenting when there's clearly a code change required, not suggestions for improvements that aren't necessary. The skipProvider flag's purpose might not be immediately obvious to all developers, and documentation could genuinely help maintainability. The comment is actionable and specific, providing exact suggested text. This could be considered a legitimate code quality improvement rather than just a "nice to have." While documentation can help, the rules explicitly state to only comment when there's "clearly a code change required." Documentation for a relatively self-explanatory boolean prop doesn't meet that threshold. The prop name and its usage in the code make its purpose reasonably clear. This is more of a suggestion than a required change. This comment should be deleted. While adding documentation could be helpful, it's not a clearly required code change. The prop name skipProvider and its usage are reasonably self-explanatory, making this suggestion fall into the "nice to have" category rather than a necessary change.

6. frontend/components/ui/tooltip.tsx:60

• Draft comment:
  Ensure the CSS calc expression in the TooltipPrimitive.Arrow class is correctly formatted. It might be clearer as 'calc(-50% - 2px)' with proper spacing.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is about CSS calc() formatting. The diff shows the author intentionally changed from calc(-50%_-_2px) (with underscore, which Tailwind uses for spaces) to calc(-50%-2px) (no spaces). The comment suggests adding spaces back. This is purely a formatting/style preference - both versions are valid CSS. The comment starts with "Ensure" which is a red flag per the rules. It's also not a clear code quality issue - it's just suggesting different spacing. The author made a deliberate change here, and the comment is asking them to reconsider based on "clarity" which is subjective. This violates the rule about not making obvious or unimportant comments, and it's asking the author to verify/ensure something rather than pointing out a clear bug. The calc expression might actually be incorrect without spaces - I should verify if CSS calc() requires spaces around operators. Also, in Tailwind's arbitrary value syntax, spacing rules might be different than regular CSS. While CSS calc() technically should have spaces around operators for best practice, both versions work in practice. More importantly, the author deliberately made this change (removing the space representation), and the comment is just suggesting a style preference with "might be clearer" language, which is not actionable enough. The comment uses "Ensure" which is explicitly called out as a red flag in the rules. This comment should be deleted. It's a stylistic preference about spacing in a calc() expression, uses the red-flag word "Ensure", and the author deliberately made this formatting change. The comment doesn't point to a clear bug or required code change.

7. frontend/lib/utils.ts:42

• Draft comment:
  Formatting for large numbers now returns ${sign}${currencySymbol}${v}Cr without a space. Confirm that omitting the space between the value and 'Cr' adheres to design requirements.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 1% The comment is asking the PR author to confirm if the omission of space between the value and 'Cr' adheres to design requirements. This is a request for confirmation, which violates the rule against asking the PR author to confirm their intention or ensure the behavior is intended.

8. frontend/components/ui/tooltip.tsx:60

• Draft comment:
  The new class string for TooltipPrimitive.Arrow uses translate-y-[calc(-50%-2px)], which removes the underscores from the calc expression. Make sure that this is intended, as CSS calc usually requires spaces around the operator (e.g. calc(-50% - 2px)). If the intent is to include spaces, consider using spaces or the proper syntax to avoid calculation errors.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is technically correct that CSS calc() typically requires spaces around operators for proper parsing. However, I need to consider: 1) This is within a Tailwind CSS arbitrary value (the square brackets), which may have different parsing rules. 2) The original code had underscores, not spaces, so the author intentionally changed from underscores to no spaces. 3) In Tailwind's arbitrary values, spaces need to be replaced with underscores, so calc(-50%_-_2px) would be the correct Tailwind syntax if spaces are needed. 4) The author changed it to remove underscores, which suggests they may have tested this and found it works, or they may have made an error. 5) This is a UI/styling change, and the rules say not to comment on pure frontend UI or styling changes, only logic. The comment might be identifying a legitimate CSS syntax issue that could cause rendering problems. Tailwind CSS arbitrary values do require underscores in place of spaces for calc expressions, so the change from underscores to no spaces could be a bug. This isn't purely a styling preference but could be a functional issue. While this could be a functional issue, the rules explicitly state not to comment on pure frontend UI or styling changes. Additionally, if this were truly broken, it would be immediately visible when testing the tooltip, so the author likely tested it. The comment is also somewhat speculative ("Make sure that this is intended") rather than definitively identifying a bug. This comment should be deleted. It falls under UI/styling changes which the rules say to ignore, and it's speculative rather than definitively identifying a bug. If the calc expression were broken, it would be immediately obvious in testing.

Workflow ID: wflow_vd5eGhwDKQIyRBDb

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}