Repository navigation
fix(tui): v3 system stats — fix garbled rendering - #993
Conversation
- Every line uses <box height={1}> (fixes text overlap / "CPUie23%")
- ASCII bar [===-----] instead of Unicode blocks (fixes width corruption)
- Top-3 hot cores replaces 84-span heatmap (readable, meaningful)
- RAM uses si.mem().active (excludes buffers/cache)
- Load humanized as "28% (24/84 busy)"
- Version renders before stats load
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the SystemStats component by simplifying the system information interfaces and replacing the per-core heatmap with a list of the top three busiest cores. It also transitions the progress bar to a safe ASCII format and updates the layout using fixed-height boxes. Review feedback suggests defining a constant for the hardcoded bar width, using a more compact format for the 'hot cores' string to avoid TUI truncation, and adjusting the swap usage color to maintain a muted visual hierarchy for low usage.
| } | ||
|
|
||
| /** Safe ASCII progress bar: [===-----] */ | ||
| function bar(percent: number, width: number): string { |
There was a problem hiding this comment.
The bar width 8 is used as a hardcoded value in multiple places (lines 133, 146, 153). It would be better to define a constant (e.g., const BAR_WIDTH = 8;) to ensure consistency and make it easier to update the layout globally.
| for (let i = 0; i < cpu.cores.length; i += COLS) { | ||
| coreRows.push(cpu.cores.slice(i, i + COLS).map((load, ci) => ({ ...coreChar(load), id: i + ci }))); | ||
| } | ||
| const hotStr = cpu.hotCores.map((c) => `#${c.id} ${c.load}%`).join(' '); |
There was a problem hiding this comment.
The current hotStr format (e.g., #10 100% #11 100% #12 100%) combined with the ' hot ' prefix can exceed 30 characters. Since the sidebar width is approximately 24-25 characters and the line is constrained by height={1}, it will likely be truncated. A more compact format would ensure the information fits within the TUI layout.
| const hotStr = cpu.hotCores.map((c) => `#${c.id} ${c.load}%`).join(' '); | |
| const hotStr = cpu.hotCores.map((c) => `${c.id}:${c.load}%`).join(' '); |
| <box height={1} width="100%"> | ||
| <text> | ||
| <span fg={palette.textMuted}>SWP </span> | ||
| <span fg={pickColor(swap.percent)}>{`${swap.usedGB}/${swap.totalGB}G ${bar(swap.percent, 8)}`}</span> |
There was a problem hiding this comment.
The use of pickColor for swap usage changes the color from palette.textDim (muted) to palette.emerald (bright green) for low usage. Since swap is ideally empty, using a bright color for 0% usage adds unnecessary visual prominence. Consider using a muted color for low swap usage to maintain the previous visual hierarchy.
| <span fg={pickColor(swap.percent)}>{`${swap.usedGB}/${swap.totalGB}G ${bar(swap.percent, 8)}`}</span> | |
| <span fg={swap.percent > 50 ? pickColor(swap.percent) : palette.textDim}>{`${swap.usedGB}/${swap.totalGB}G ${bar(swap.percent, 8)}`}</span> |
1 participant
Summary
Fixes all rendering bugs from v2 system stats (#990):
<box height={1}>(proven TreeNodeRow pattern)[===-----]bars instead of█░▒▓si.mem().active(excludes buffers/cache)28% (24/84 busy)Test plan