ANSI characters not being displayed correctly in sync display

[gltanaka]

[gltanaka]

serhan
pdd bug #255
target 1/7

[jiaminc-cmu]

Step 1: Duplicate Check

Status: No duplicates found

Search Performed

• Searched for: ANSI, display, sync, characters, escape, terminal, color, formatting, output, text, garbled, corrupt, render
• Issues reviewed: 15+ potentially related issues

Findings

After searching through open and closed issues, no exact duplicates were found. The most related issues are:

• Terminal UI break when terminal is resized during pdd sync #236 (Terminal UI break when terminal is resized during pdd sync) - Different issue: focuses on terminal resize events causing UI corruption, not ANSI character rendering in normal operation
• Right now pdd sync hides all the messages and interactive prompts. Modify this so that it shows the stdout/error messages at the bottom #65 (pdd sync hides messages and interactive prompts) - CLOSED: Was about showing stdout/error messages, not ANSI character rendering issues
• pdd sync --log not showing logs and exits the sync #160 (pdd sync --log not showing logs) - CLOSED: Was about log display functionality, not ANSI rendering
• Display/accessibility issue on Terms of Service and Privacy policy pages #126 (Display/accessibility issue) - CLOSED: Was about website styling, not CLI ANSI output

This appears to be a new issue specifically about ANSI escape sequences not being rendered/stripped correctly in the sync display during normal operation (not triggered by terminal resize).

---

Proceeding to Step 2: Documentation Check

[jiaminc-cmu]

Step 2: Documentation Check

Status: Needs Investigation

Documentation Reviewed

• README.md - Main documentation including sync command and troubleshooting section
• docs/sync_architecture_analysis.md - Technical analysis of sync command architecture
• pdd/sync_tui.py - Source code handling stdout redirection and ANSI conversion
• pdd/sync_animation.py - Source code for animation display rendering
• pyproject.toml - Dependencies (rich==14.0.0, textual)
• docs/faq.md - FAQ documentation

Findings

Code Analysis:
The sync display uses ThreadSafeRedirector class in pdd/sync_tui.py:274-329 which explicitly converts ANSI escape codes using Rich's Text.from_ansi() method:

# sync_tui.py line 308
text = Text.from_ansi(line)

This indicates the system is designed to properly render ANSI codes - displaying them as raw characters is NOT expected behavior.

Documentation Gap:

• The troubleshooting section mentions "Animation display issues: Sync operations work in background; animation is visual feedback only and doesn't affect functionality"
• However, there is no specific documentation about ANSI character rendering expectations or known terminal compatibility issues

Assessment:
Since the code explicitly attempts to convert ANSI sequences to properly formatted Rich Text objects, the display of raw ANSI characters (as shown in the screenshot) represents unintended behavior. This is not documented as expected and not a known limitation.

Next Steps

Further investigation is needed to:

1. Determine which specific ANSI sequences are failing to render
2. Identify whether this is terminal-specific or environment-dependent
3. Check if the issue is with carriage return handling (lines 292-304) or the Text.from_ansi() conversion itself

---

Proceeding to Step 3: Triage

[jiaminc-cmu]

Step 3: Triage

Status: Proceed

Summary

We have sufficient information to attempt reproduction of this ANSI display issue:

What we understand:

• Expected behavior: ANSI escape sequences (colors, formatting) should render properly in the sync TUI display
• Actual behavior: ANSI characters are not displaying correctly (visible in the attached screenshot)
• Code location identified: pdd/sync_tui.py - specifically the ThreadSafeRedirector class which handles output to the RichLog widget

Key code context:

• Line 308 uses Text.from_ansi(line) to convert ANSI escape codes to Rich Text objects
• The RichLog widget is configured with highlight=True, markup=True, wrap=True
• Environment variables FORCE_COLOR=1 and TERM=xterm-256color are set to encourage ANSI output

Reproduction approach:

1. Run pdd sync to trigger the TUI
2. Generate output containing ANSI escape sequences
3. Observe how the sequences are rendered in the RichLog widget

The code explicitly attempts to handle ANSI codes properly, so this appears to be a bug where the intended functionality is not working correctly in certain conditions.

---

Proceeding to Step 4: Reproduction

[jiaminc-cmu]

Step 6: Test Plan

Existing Test Coverage

• Test file: tests/test_thread_safe_redirector.py
• Current coverage:
  • Progress bar handling with \r characters (tests 1-6)
  • Normal newline behavior (tests 3-5)
  • Empty writes and buffering (tests 7-8)
  • Basic ANSI code preservation (test 9 - test_ansi_codes_preserved)
• Gap: The existing ANSI test (test_ansi_codes_preserved) only verifies plain text content is preserved, not that ANSI codes are correctly converted to Rich styles. The MockRichLog captures text.plain which strips all styling information.

Root Cause Analysis

The bug manifests because:

1. ThreadSafeRedirector.write() uses Text.from_ansi(line) at line 308 to convert ANSI codes
2. The current test only verifies text.plain content, not actual style conversion
3. Complex ANSI sequences (256-color, RGB/true color, combined styles) may not be properly handled by Text.from_ansi()
4. The MockRichLog needs to capture full Text objects with styles for proper verification

Proposed Tests

Test 1: ANSI color codes produce correct Rich styles

• Input: "\x1b[32mGreen text\x1b[0m\n" (simple green foreground)
• Expected: Text object with color="green" style applied to "Green text"
• Actual (before fix): Plain text preserved but style not verified

Test 2: ANSI bold/underline styles converted correctly

• Input: "\x1b[1mBold\x1b[0m \x1b[4mUnderline\x1b[0m\n"
• Expected: Text with bold=True on "Bold" and underline=True on "Underline"

Test 3: Combined ANSI styles (bold + color)

• Input: "\x1b[1;31mBold Red\x1b[0m\n"
• Expected: Text with both bold=True and color="red" applied

Test 4: 256-color ANSI codes

• Input: "\x1b[38;5;208mOrange 256-color\x1b[0m\n"
• Expected: Text with appropriate color style (color index 208)

Test 5: RGB/True color ANSI codes

• Input: "\x1b[38;2;255;128;0mOrange RGB\x1b[0m\n"
• Expected: Text with RGB color #ff8000 applied

Test 6: Multiple ANSI sequences in single line

• Input: "\x1b[31mRed\x1b[0m normal \x1b[32mGreen\x1b[0m\n"
• Expected: Text with red style on "Red", no style on " normal ", green style on "Green"

Test 7: Malformed/incomplete ANSI sequences

• Input: "\x1b[Invalid ANSI\n" or "\x1b[31mNo reset\n"
• Expected: Graceful handling without crash, partial styling where possible

Test 8: ANSI sequences split across buffered writes

• Input: Write "\x1b[32mGree" then "n text\x1b[0m\n"
• Expected: Correct green styling applied to complete "Green text"

Test Location

• File: tests/test_thread_safe_redirector.py (append to existing file)
• Framework: pytest with unittest.mock

Notes

1. MockRichLog Enhancement: Need to modify MockRichLog to capture full Text objects with styles instead of just text.plain:

   class MockRichLog:
       def __init__(self):
           self.written_texts: List[Text] = []  # Full Text objects
           self.written_lines: List[str] = []   # Plain text (backward compat)
       
       def write(self, text):
           if isinstance(text, Text):
               self.written_texts.append(text)
               self.written_lines.append(text.plain)
           else:
               self.written_lines.append(str(text))

2. Style Verification Helper: Add helper to extract and verify styles from Rich Text:

   def get_styles_for_text(text: Text) -> dict:
       """Extract style information for each span in the Text object."""
       styles = {}
       for start, end, style in text._spans:
           styles[(start, end)] = style
       return styles

3. Environment Consideration: Tests should verify behavior with FORCE_COLOR=1 and TERM=xterm-256color set (matching TUI runtime environment)

4. Integration Consideration: May also need an integration test that verifies the full pipeline: stdout → ThreadSafeRedirector → RichLog.write() → Textual rendering

---

Proceeding to Step 7: Generate Test