test: finops formatting — formatBytes and truncateQuery edge cases

[anandgupta42]

Summary

What does this PR do?

1. formatBytes and truncateQuery — src/altimate/tools/finops-formatting.ts (12 new tests)

These pure utility functions power user-facing output in finops tools (finops-expensive-queries.ts, finops-query-history.ts). Zero tests existed. Testing revealed 5 confirmed bugs:

formatBytes bugs:

• Negative bytes → "NaN undefined" — users see garbage when finops tools compute negative deltas (e.g. comparing periods where usage decreased)
• NaN input → "NaN undefined" — propagated NaN from upstream calculations crashes display
• Fractional bytes (0.5) → "512.00 undefined" — Math.floor(Math.log(0.5) / Math.log(1024)) = -1, and units[-1] is undefined

truncateQuery bugs:

• Whitespace-only input → "" (empty string) instead of "(empty)" — the !text guard doesn't catch " " (truthy), but after trim() it becomes "" which is returned directly by the length check
• maxLen < 3 → output exceeds maxLen — slice(0, 2-3) = slice(0, -1) keeps most of the string, then "..." is appended, producing 13+ chars for a 2-char limit

New coverage includes:

• Normal cases: zero bytes, exact unit boundaries (B/KB/MB/GB), non-boundary values
• Normal cases: empty input, short text passthrough, long text truncation, multiline collapse
• Bug-documenting tests: all 5 bugs above with explanatory comments for future fixers

Type of change

• New feature (non-breaking change which adds functionality)

Issue for this PR

N/A — proactive test coverage via /test-discovery

How did you verify your code works?

bun test test/altimate/tools/finops-formatting.test.ts    # 12 pass, 0 fail

Checklist

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

https://claude.ai/code/session_01AfyN3rkDU7fn9G5SpZSM77

Summary by CodeRabbit

• Bug Fixes

  • Improved byte formatting display for non-finite, negative, fractional and boundary values; units now format consistently.
  • Query truncation behavior refined: whitespace-only input shows "(empty)", multiline SQL is collapsed to one line, and very small/non‑positive length limits are handled predictably.

• Tests

  • Added comprehensive tests covering normal and edge-case scenarios for both formatting behaviors.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a Bun test suite for finops-formatting and small logic updates to formatBytes and truncateQuery in the finops-formatting utility; tests validate normal and edge-case behavior for both functions.

Changes

Cohort / File(s)	Summary
Tests packages/opencode/test/altimate/tools/finops-formatting.test.ts	New Bun test suite covering formatBytes and truncateQuery across normal cases and multiple edge cases (NaN/Infinity, negative and fractional bytes, whitespace-only input, small/zero/negative maxLen, multiline SQL, truncation length constraints).
Utility Implementation packages/opencode/src/altimate/tools/finops-formatting.ts	Small logic changes: formatBytes returns "0 B" for non-finite inputs, uses absolute value and clamps unit index; truncateQuery treats whitespace-only strings as "(empty)", handles non-positive maxLen and very small maxLen (<4) specially before applying truncation/ellipsis. Relevant areas to review: numeric edge handling, index clamping, and truncation boundary logic.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hopping through bytes and SQL with cheer,

I round and truncate without fear,
NaN tamed, long queries made neat,
Tests patter quick feet to repeat,
A rabbit's stamp — code checks complete.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main changes: adding tests for edge cases in finops formatting utilities.
Description check	✅ Passed	The description provides comprehensive coverage: detailed summary of what changed and why (5 confirmed bugs documented), test plan confirming tests pass locally, and checklist items completed. All template sections are addressed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/test-finops-formatting-01AfyN3rkDU7fn9G5SpZSM77

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/opencode/test/altimate/tools/finops-formatting.test.ts (1)

51-56: Consider asserting exact length for the truncation test.

The truncation logic for maxLen=20 should produce exactly 20 characters (17 from slice + 3 for "..."). While toBeLessThanOrEqual(20) is correct, a stricter assertion would catch if the implementation under-truncates.

💡 Optional: Use exact length assertion

   test("long text truncated with ellipsis", () => {
     const long = "SELECT * FROM very_long_table_name WHERE id = 1"
     const result = truncateQuery(long, 20)
-    expect(result.length).toBeLessThanOrEqual(20)
+    expect(result.length).toBe(20)
     expect(result).toEndWith("...")
   })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/altimate/tools/finops-formatting.test.ts` around lines
51 - 56, The test "long text truncated with ellipsis" currently uses a
non-strict length check; change the assertion for truncateQuery(long, 20) to
expect exact length 20 (e.g., expect(result.length).toBe(20)) and keep the
expect(result).toEndWith("...") to ensure the function returns the 17-char slice
plus the 3-char ellipsis; update the test for truncateQuery accordingly so it
fails if the function under- or over-truncates.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/opencode/test/altimate/tools/finops-formatting.test.ts`:
- Around line 51-56: The test "long text truncated with ellipsis" currently uses
a non-strict length check; change the assertion for truncateQuery(long, 20) to
expect exact length 20 (e.g., expect(result.length).toBe(20)) and keep the
expect(result).toEndWith("...") to ensure the function returns the 17-char slice
plus the 3-char ellipsis; update the test for truncateQuery accordingly so it
fails if the function under- or over-truncates.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 60b5d1e2-5b90-4795-bcaa-09bc81c63501

📥 Commits

Reviewing files that changed from the base of the PR and between 8d5804f and 5fd6316.

📒 Files selected for processing (1)

• packages/opencode/test/altimate/tools/finops-formatting.test.ts

[coderabbitai]

🧹 Nitpick comments (1)

packages/opencode/src/altimate/tools/finops-formatting.ts (1)

14-16: Consider the interaction between whitespace-only input and small maxLen.

The current check order means whitespace-only input returns "(empty)" (7 characters) regardless of maxLen, which could exceed the requested length. For example, truncateQuery(" ", 2) returns "(empty)" instead of respecting the 2-character limit.

Given the production callers use maxLen values of 80 and 100 (per the relevant snippets), this is unlikely to cause issues in practice. If this behavior is intentional—treating "(empty)" as a semantic marker that takes precedence over length constraints—it would be helpful to add a brief comment documenting this design choice.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/src/altimate/tools/finops-formatting.ts` around lines 14 -
16, Reorder the early-return checks in truncateQuery so maxLen constraints are
enforced before returning the "(empty)" marker: move the maxLen checks (maxLen
<= 0 and maxLen < 4) above the oneLine emptiness check (the if (!oneLine) return
"(empty)") so whitespace-only inputs are truncated to respect maxLen;
alternatively, if the "(empty)" marker is intended to take precedence, add a
short comment next to the if (!oneLine) branch explaining that design choice.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/opencode/src/altimate/tools/finops-formatting.ts`:
- Around line 14-16: Reorder the early-return checks in truncateQuery so maxLen
constraints are enforced before returning the "(empty)" marker: move the maxLen
checks (maxLen <= 0 and maxLen < 4) above the oneLine emptiness check (the if
(!oneLine) return "(empty)") so whitespace-only inputs are truncated to respect
maxLen; alternatively, if the "(empty)" marker is intended to take precedence,
add a short comment next to the if (!oneLine) branch explaining that design
choice.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6c43f44a-b1d0-4688-b98a-9fa09bbecf6b

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd6316 and a3fe7ef.

📒 Files selected for processing (2)

• packages/opencode/src/altimate/tools/finops-formatting.ts
• packages/opencode/test/altimate/tools/finops-formatting.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/opencode/test/altimate/tools/finops-formatting.test.ts

[anandgupta42]

Consolidated into #403