codeant-ai-for-open-source[bot] commented on code in PR #36644:
URL: https://github.com/apache/superset/pull/36644#discussion_r2655887295
##########
superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx:
##########
@@ -34,6 +34,19 @@ export function convertToNumWithSpaces(num: number) {
return num.toString().replace(/(\d)(?=(\d{3})+(?!\d))/g, '$1 ');
}
+export function convertToShortNum(num: number) {
+ if (num < 1000) {
+ return num;
+ }
+ if (num < 1_000_000) {
+ return `${num / 1000}K`;
+ }
+ if (num < 1_000_000_000) {
+ return `${num / 1000_000}M`;
+ }
+ return num;
Review Comment:
**Suggestion:** The function `convertToShortNum` returns numbers in some
branches and strings in others, producing an inconsistent return type
(number|string). This can cause unexpected behavior in callers that expect a
string (rendering, concatenation, or downstream formatting) and defeats
TypeScript's type-safety; make the function always return a string. [type error]
**Severity Level:** Minor ⚠️
```suggestion
export function convertToShortNum(num: number): string {
if (num < 1000) {
return `${num}`;
}
if (num < 1_000_000) {
return `${num / 1000}K`;
}
if (num < 1_000_000_000) {
return `${num / 1_000_000}M`;
}
return `${num}`;
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The current implementation returns numbers for small and very large inputs
and strings for K/M branches, which is inconsistent and can surprise callers
and renderers. Making the function always return a string fixes the
type/formatting inconsistency and is the correct, minimal change for a UI
formatting helper.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx
**Line:** 37:47
**Comment:**
*Type Error: The function `convertToShortNum` returns numbers in some
branches and strings in others, producing an inconsistent return type
(number|string). This can cause unexpected behavior in callers that expect a
string (rendering, concatenation, or downstream formatting) and defeats
TypeScript's type-safety; make the function always return a string.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
