Skip to content

Conversation

@jaskfla
Copy link
Contributor

@jaskfla jaskfla commented Feb 11, 2026

No description provided.

ignores locale, though 😔
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @jaskfla, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly upgrades the application's data formatting utility, formatDataValueByType, to provide more user-friendly and accurate numerical representations. The changes primarily focus on introducing thousands separators for better readability of large numbers and refining the logic for percentage formatting to be more robust, especially for negative values and edge cases. Accompanying these functional improvements is a substantial increase in unit test coverage, ensuring the reliability of the formatting utility across all supported data types. Additionally, minor TypeScript enhancements were made to improve type safety.

Highlights

  • Enhanced Number Formatting: Implemented thousands separators for 'number', 'oneDecimalPlace', and default formatting types, improving readability for large numerical values across the application.
  • Improved Percentage Formatting Logic: Refined the percentage formatting to handle negative values correctly and ensure consistent rounding behavior across different magnitudes (e.g., <1%, [1%, 100%), >=100%).
  • Robust Data Type Handling: Replaced isNaN with Number.isNaN for more reliable checks and introduced a specific fix to prevent numeral.js from returning NaN for very small numerical inputs.
  • Expanded Test Coverage: Added comprehensive unit tests for all VALUE_TYPES (boolean, currency, fraction, number, percentage, etc.) to validate the new formatting logic and ensure correctness across various edge cases.
  • TypeScript Type Safety: Applied as const assertions to VALUE_TYPES and VIEWS objects, enhancing type inference and immutability for improved code quality and maintainability.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • packages/tupaia-web/src/features/Visuals/View/View.tsx
    • Added as const assertion to the VIEWS object for improved TypeScript type inference.
  • packages/utils/src/tests/formatDataValue.test.js
    • Expanded test suite to include new test cases for boolean, currency, fraction, number, number and percentage, one decimal place, and text value types.
    • Updated existing percentage tests to cover negative values and various rounding scenarios more thoroughly.
  • packages/utils/src/formatDataValueByType.js
    • Updated VALUE_TYPES and VALUE_TYPE_TO_FORMATTER with as const and Object.freeze for type safety and immutability.
    • Replaced isNaN with Number.isNaN in several functions for more accurate NaN checks.
    • Modified the percentage formatter to correctly handle negative percentages and refine decimal precision logic based on magnitude.
    • Implemented thousands separators (0,0) in the number, oneDecimalPlace, and defaultFormatter functions.
    • Added a hacky fix to the number formatter to prevent numeral.js from returning NaN for extremely small numerical values.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances number formatting by adding thousands separators, which improves readability. The changes are well-tested with a comprehensive suite of new unit tests covering various data types and edge cases, including negative values. The codebase is also improved by using modern JavaScript features like nullish coalescing and const assertions for better type safety.

My main feedback is regarding a workaround for a numeral.js bug, where I've suggested an improvement for better code clarity and maintainability.

@jaskfla jaskfla force-pushed the thousands-separators branch 2 times, most recently from ea0b7ab to 7363be6 Compare February 11, 2026 07:10
@jaskfla jaskfla force-pushed the thousands-separators branch from 26efcf8 to 9718802 Compare February 11, 2026 07:17
@jaskfla jaskfla force-pushed the thousands-separators branch from 9718802 to 3620213 Compare February 11, 2026 07:34
@jaskfla jaskfla force-pushed the thousands-separators branch from 3620213 to 2fe6afb Compare February 11, 2026 07:41
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.


const fraction = (value, { total }) => {
if (isNaN(total)) return 'No data';
if (total === null || Number.isNaN(Number(total))) return 'No data';
Copy link

Choose a reason for hiding this comment

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

Empty string total bypasses fraction's "No data" guard

Medium Severity

The fraction function's guard total === null || Number.isNaN(Number(total)) doesn't catch an empty string total, because Number('') evaluates to 0 (not NaN). When total is '', the function returns "0/" instead of 'No data'. The newly added test expects 'No data' for this case, confirming the intended behavior doesn't match the implementation.

Additional Locations (1)

Fix in Cursor Fix in Web

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant