Skip to content

feat: improve ov chat UX with rustyline and markdown rendering#466

Merged
zhoujh01 merged 1 commit intomainfrom
feature/ov_chat_cli_v2
Mar 6, 2026
Merged

feat: improve ov chat UX with rustyline and markdown rendering#466
zhoujh01 merged 1 commit intomainfrom
feature/ov_chat_cli_v2

Conversation

@chenjw
Copy link
Collaborator

@chenjw chenjw commented Mar 6, 2026

  • Use rustyline for proper line editing (no ^[[D characters)
  • Add markdown rendering with termimad
  • Add command history support (~/.ov_chat_history)
  • Add --no-format option to disable markdown
  • Add --no-history option to disable command history

Description

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

Testing

  • 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
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

- Use rustyline for proper line editing (no ^[[D characters)
- Add markdown rendering with termimad
- Add command history support (~/.ov_chat_history)
- Add --no-format option to disable markdown
- Add --no-history option to disable command history
@github-actions
Copy link

github-actions bot commented Mar 6, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Regression in reasoning output

The "reasoning" event output always appends "..." now, even when content is <= 100 characters, which is a change from the old behavior.

"reasoning" => {
    let content = data.as_str().unwrap_or("");
    println!(
        "  \x1b[2mThink: {}...\x1b[0m",
        truncate_utf8(content, 100)
    );
Missing import check

The local truncate_utf8 function is used, but verify that it behaves the same as the removed crate::utils::truncate_utf8 to avoid unintended changes.

fn truncate_utf8(s: &str, max_chars: usize) -> &str {
    if s.chars().count() <= max_chars {
        return s;
    }
    if let Some((idx, _)) = s.char_indices().nth(max_chars) {
        &s[..idx]
    } else {
        s
    }
}

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix tool result truncation check

The "tool_result" event checks byte length (content.len()) instead of character
count to decide truncation. This can incorrectly truncate strings with multi-byte
Unicode characters. Use content.chars().count() instead.

crates/ov_cli/src/commands/chat.rs [315-323]

 "tool_result" => {
     let content = data.as_str().unwrap_or("");
-    let truncated = if content.len() > 150 {
+    let truncated = if content.chars().count() > 150 {
         format!("{}...", truncate_utf8(content, 150))
     } else {
         content.to_string()
     };
     println!("  \x1b[2m└─ Result: {}\x1b[0m", truncated);
 }
Suggestion importance[1-10]: 6

__

Why: The suggestion addresses a correctness issue where the tool result truncation used byte length instead of character count, which could break with multi-byte Unicode. This is a solid improvement for robustness.

Low
Fix reasoning event ellipsis logic

The "reasoning" event always appends "..." even when the content is not truncated.
Check if the content exceeds 100 characters before adding an ellipsis, and only
truncate when necessary.

crates/ov_cli/src/commands/chat.rs [304-310]

 "reasoning" => {
     let content = data.as_str().unwrap_or("");
-    println!(
-        "  \x1b[2mThink: {}...\x1b[0m",
-        truncate_utf8(content, 100)
-    );
+    if content.chars().count() > 100 {
+        println!(
+            "  \x1b[2mThink: {}...\x1b[0m",
+            truncate_utf8(content, 100)
+        );
+    } else {
+        println!("  \x1b[2mThink: {}\x1b[0m", content);
+    }
 }
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly fixes the minor issue where the reasoning event always appends "..." even when content isn't truncated. This improves output readability without changing core functionality.

Low

@zhoujh01 zhoujh01 merged commit 06e24fc into main Mar 6, 2026
11 of 12 checks passed
@zhoujh01 zhoujh01 deleted the feature/ov_chat_cli_v2 branch March 6, 2026 09:41
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants