Conversation
The streaming tokens/sec row now respects display.show_tps, checked at the three data-feed sites (usage info, TuiState::output_tps, runtime panel feed), so setting false hides live throughput in the info panel and status line at once. Default keeps today's behavior.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| let tokens_per_second = if matches!(self.status, ProcessingStatus::Streaming) { | ||
| let tokens_per_second = if matches!(self.status, ProcessingStatus::Streaming) | ||
| && crate::config::config().display.show_tps |
There was a problem hiding this comment.
With show_tps = false, the live status line and info panel hide TPS, but the completed-turn transcript footer still shows it. The new guard does not apply when that footer is built, so users who disable TPS continue to see a throughput value after each turn. This display inconsistency is non-blocking; apply the setting to the footer too, or clarify that it controls only live values.
Artifacts
Executed Rust test source for live and completed TPS
- This is the focused test executed in both runs; it checks live feeds and the appended completed-turn footer.
Command script for both TPS configuration runs
- This script supplies an isolated config value and runs the focused Cargo test; it makes the two checks reproducible.
- Ran the focused test with `show_tps=true`, showing TPS in both live feeds and the completed footer.
- Ran the same test with `show_tps=false`, showing empty live TPS feeds but an unchanged completed footer containing TPS.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-tui/src/tui/app/tui_state.rs
Line: 1559
Comment:
**Completed footer still shows TPS**
With `show_tps = false`, the live status line and info panel hide TPS, but the completed-turn transcript footer still shows it. The new guard does not apply when that footer is built, so users who disable TPS continue to see a throughput value after each turn. This display inconsistency is non-blocking; apply the setting to the footer too, or clarify that it controls only live values.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| "freshly created user config should request model reasoning" | ||
| ); | ||
| assert!( | ||
| parsed.display.show_tps, |
There was a problem hiding this comment.
This assertion checks the enabled default, but no committed test sets show_tps = false and checks both live TPS outputs. Both paths currently hide TPS when disabled. The non-blocking cost is that either path could stop honoring the setting while this test still passes; add a disabled-setting test for the status line and info panel.
Artifacts
Temporary live TPS test source
- This is the source executed in the TUI test binary for both settings, asserting the live status and info-panel values.
- This script wrote an isolated config with the requested setting and ran the focused TUI test.
Live TUI check with TPS enabled
- The focused test ran with show_tps=true and observed TPS in both live outputs; it passed.
Live TUI check with TPS disabled
- The same test ran with show_tps=false and observed no TPS in either live output; it passed.
- This authored Rust probe parsed omitted and explicit show_tps values using the compiled config type.
Config parsing with TPS enabled
- The compiled probe parsed the omitted and explicit true values as true and exited 0.
Config parsing with TPS disabled
- The same probe parsed explicit false as false and exited 0.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-base/src/config_tests.rs
Line: 806
Comment:
**Disabled setting lacks coverage**
This assertion checks the enabled default, but no committed test sets `show_tps = false` and checks both live TPS outputs. Both paths currently hide TPS when disabled. The non-blocking cost is that either path could stop honoring the setting while this test still passes; add a disabled-setting test for the status line and info panel.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Adds
[display] show_tps(default true). Set false to hide the live tokens/sec row from the info panel and status line. Matches the existingshow_thinkingpattern: serde default, generated default config documents it, and the streaming TPS computation is skipped entirely when off.Tests:
test_generated_default_config_has_expected_user_defaultsassertsshow_tps: trueon fresh configs, and the existing streaming TPS tests (13) pass unchanged since the default keeps current behavior. 22 insertions. Single commit rebased onto current master.