Revert billion suffix logic change due to performance regression

[Copilot]

Reverts the billion suffix logic change that was made in commit 21d3311 based on performance analysis.

Context

The original code had positionFromRight >= 6 which was changed to positionFromRight >= 3 to improve "logic consistency." However, both versions produce identical functional output:

• Position 3 (10^9): Both return ' tỷ' (original via fallback, changed via calculation)
• Position 6+ (10^18+): Both use billionCount calculation for repeated "tỷ"

Decision

After performance analysis via codspeed, the original positionFromRight >= 6 logic has been restored because:

1. No functional difference: Both implementations produce identical output for all test cases
2. Performance regression: The changed version showed worse performance metrics
3. Code simplicity: When functionality is identical, prefer the faster implementation

Changes Made

Reverted to original logic:

if (positionFromRight >= 6 && hasTrailingZeros) {
  const billionCount = Math.floor(positionFromRight / 3)
  return ' ' + new Array(billionCount).fill('tỷ').join(' ')
}
return ' tỷ'

Testing

• ✅ All 112 tests pass
• ✅ Code quality checks pass
• ✅ Identical functional behavior maintained

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[hckhanh]

@copilot If you think this is a bug, why all of my tests were passed?

[Copilot]

@copilot If you think this is a bug, why all of my tests were passed?

You're absolutely right - all tests passed because this wasn't a functional bug, it was a logic consistency bug. Both versions produce identical output:

Position 3 (10^9):

• Buggy (>= 6): Falls through to return ' tỷ' on line 38
• Fixed (>= 3): Calculates billionCount = Math.floor(3/3) = 1 → ' tỷ'
• Result: Same output!

Position 6 (10^18):

• Both versions use the calculation path → ' tỷ tỷ'
• Result: Same output!

The issue was that position 3 took a different code path (fallback) while positions 6+ used the calculation, even though they're part of the same repeating pattern. The fix makes the code logically consistent - all positions now use the uniform billionCount calculation. This improves maintainability without changing behavior, which is why your tests continued to pass.

[sonarqubecloud]

Please retry analysis of this Pull-Request directly on SonarQube Cloud

[hckhanh]

@copilot But my codspeed report that there is a performance trade off for this. If this is unnecessary to fix, why don't we go back to the old solution?

[Copilot]

@copilot But my codspeed report that there is a performance trade off for this. If this is unnecessary to fix, why don't we go back to the old solution?

You're right - if the change causes a performance regression without fixing any functional issue, it's not worth keeping. Reverted to the original positionFromRight >= 6 logic in commit 1749566. Both versions produce identical output, so the faster one is the better choice. All tests still pass.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

Pull Request Overview

This PR improves code clarity in the billion suffix logic by removing redundant handling for positionFromRight === 3. The change simplifies the condition to only handle cases where multiple "tỷ" suffixes are needed (position 6 and above).

• Removed unnecessary branch execution for positionFromRight === 3 which produced the same output as the default case
• Updated comment to accurately reflect the logic (removed misleading "position 3 = 'tỷ'" reference)