Fix abbreviation formatting in visualizer

[SmartManoj]

Replaces the previous method of removing trailing '.0' with a more robust approach using rstrip to remove trailing zeros and periods.

---

input 88k to 898k

---

s='89.08'
print(s.replace('.0',''))
print(s.rstrip("0").rstrip("."))
print('===')
s='89.00'
print(s.replace('.0',''))
print(s.rstrip("0").rstrip("."))

Output:

898
89.08
===
890
89

[xingyaoww]

@OpenHands /codereview-roasted

Read the pr description, understand the problem, and see if we can use some existing library / simplify the code to make it more robust

[openhands-ai]

I'm on it! xingyaoww can track my progress at all-hands.dev

[openhands-ai]

🎯 Code Review Complete - PR #989 Updated

I've reviewed and improved the PR fixing the abbreviation formatting bug in visualizer.py.

🔴 Taste Rating: Good → Better

The PR correctly fixed the bug, but I simplified it further using better design.

---

📊 Problem Analysis

Original Bug:

s.replace(".0", "")  # Incorrectly: "89.08K" → "898K" ❌

The .replace(".0", "") removed ".0" from anywhere in the string, breaking numbers like 89.08K.

PR's Fix:

suffix = s[-1]
return s[:-1].rstrip("0").rstrip(".") + suffix  # Works ✓ but complex

This worked by: format → extract suffix → strip → re-add suffix.

---

✨ My Improvement (Applied)

Simplified the data flow - extract value and suffix FIRST, then format:

if n >= 1_000_000_000:
    val, suffix = n / 1_000_000_000, "B"
elif n >= 1_000_000:
    val, suffix = n / 1_000_000, "M"
elif n >= 1_000:
    val, suffix = n / 1_000, "K"
else:
    return str(n)
return f"{val:.2f}".rstrip("0").rstrip(".") + suffix

Why this is better ("Good Taste" principles):

1. ✅ Clearer data structure - Value and suffix are determined separately
2. ✅ Single formatting operation - No format-extract-reformat dance
3. ✅ More maintainable - Suffix logic is centralized in tuple assignments
4. ✅ Same behavior - Produces identical correct results

---

🧪 Testing

Added comprehensive edge case tests covering:

• Below threshold: 999 → "999"
• Exact boundaries: 1000 → "1K", 1000000 → "1M", 1000000000 → "1B"
• Trailing zeros: 89000 → "89K" (not "89.00K")
• The bug case: 89080 → "89.08K" (not "898K") ✓

All 16 tests pass, including the new comprehensive abbreviation test.

---

📚 Library Investigation

Checked if we could use existing libraries like humanize:

• ❌ Not in current dependencies
• ❌ Would add dependency for a simple 11-line function
• ✅ Our implementation is clean, tested, and sufficient

---

✅ Changes Committed & Pushed

• Simplified the abbr() function logic in visualizer.py
• Updated existing test to expect correct output ("1.5K" not "1.50K")
• Added comprehensive edge case test (test_metrics_abbreviation_formatting)
• All pre-commit hooks pass (ruff, pyright, pycodestyle)
• Changes pushed to branch strip updating PR Fix abbreviation formatting in visualizer #989

Verdict: The PR is now simpler, more maintainable, and thoroughly tested. ✨

_{View full conversation}

[xingyaoww]

@SmartManoj lmk if this changes looks good to you. if so we can merge this

[xingyaoww]

merging this now as we are preparing for v1 release. LMK if you have concerns on the changes here @SmartManoj