Skip to content

fix(tui): keep best-value highlights aligned when reversed - #215

Merged
mike1858 merged 1 commit into
Piebald-AI:mainfrom
jimyag:fix/issue-134-reversed-highlight
Jul 26, 2026
Merged

mike1858 merged 1 commit into
Piebald-AI:mainfrom
jimyag:fix/issue-134-reversed-highlight

Conversation

@jimyag

@jimyag jimyag commented Jul 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix the aggregate-table best-value highlights so they stay attached to the correct period when sort order is reversed.

Key changes

  • Track each column's best value by period key instead of the original map index.
  • Preserve existing best-value and cost heatmap behavior.
  • Add a rendered TUI regression test covering reversed sort order.

Validation

  • cargo build --quiet
  • cargo test --quiet (399 passed)
  • cargo clippy --quiet -- -D warnings
  • cargo doc --quiet
  • cargo fmt --all --quiet
  • git diff --check

Closes #134

Summary by CodeRabbit

  • Bug Fixes

    • Corrected best-value highlighting in aggregate statistics tables, including costs, token counts, conversations, and tool calls.
    • Ensured highlighting remains accurate when rows are sorted or displayed in reverse order.
  • Tests

    • Added coverage verifying that the highest relevant metric is highlighted correctly in the rendered table.

Track highlighted maxima by period key instead of display index so reversed aggregate rows retain the correct styling. Add a rendered TUI regression test for issue Piebald-AI#134.

Signed-off-by: jimyag <git@jimyag.com>
@coderabbitai

coderabbitai Bot commented Jul 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a9c93ff2-358b-49b8-ac81-2176c66ff04d

📥 Commits

Reviewing files that changed from the base of the PR and between 76d5327 and 338ddc4.

📒 Files selected for processing (2)
  • src/tui.rs
  • src/tui/tests.rs

📝 Walkthrough

Walkthrough

The aggregate statistics table now tracks best metrics by period key rather than positional row index. Rendering compares each row’s period to the selected best period, and a reversed-sort TUI test verifies correct highlighting.

Changes

Aggregate highlighting

Layer / File(s) Summary
Track best periods
src/tui.rs
Best cost, token, conversation, and tool-call metrics are associated with aggregate period keys during calculation.
Render and validate period highlights
src/tui.rs, src/tui/tests.rs
Cell styling uses period-key comparisons, and a reversed-sort rendering test verifies the higher input-token value receives red styling.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: mike1858, basekevin

Poem

I’m a bunny with a table to mend,
Best rows now follow the period trend.
Reverse the sort, the colors stay true,
Red finds the value it’s meant to pursue.
Hop, hop—tests agree!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: keeping best-value TUI highlights aligned when sort order is reversed.
Linked Issues check ✅ Passed The changes address #134 by keying best-value highlights to the period and adding a regression test for reversed sort mode.
Out of Scope Changes check ✅ Passed The edits are confined to the TUI highlight logic and a related test, which are directly in scope for the reported bug.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@mike1858 mike1858 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops, good catch, I forgot about that issue. Thank you!

@mike1858
mike1858 enabled auto-merge (squash) July 26, 2026 16:03
@mike1858
mike1858 merged commit ca7175f into Piebald-AI:main Jul 26, 2026
6 checks passed
@Sewer56

Sewer56 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Neato 👌

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Best-row (red) highlight markers land on wrong row in reversed sort mode

3 participants