Skip to content

filter terminal #157

Merged
Kitenite merged 2 commits intomainfrom
azure-wave-28
Nov 27, 2025
Merged

filter terminal #157
Kitenite merged 2 commits intomainfrom
azure-wave-28

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Nov 27, 2025

Description

Related Issues

Type of Change

  • Bug fix
  • New feature
  • Documentation
  • Refactor
  • Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes

Summary by CodeRabbit

Release Notes

  • New Features

    • Terminal scrollback and history now filter out terminal control sequences for cleaner display, while preserving raw output in live sessions.
  • Tests

    • Added comprehensive test coverage for escape sequence filtering functionality.

✏️ Tip: You can customize this high-level summary in your review settings.

Kitenite and others added 2 commits November 26, 2025 19:16
When reattaching to a terminal, escape sequence responses (CPR, device
attributes, OSC color queries) were being displayed as garbage text.
This filters them from scrollback storage while preserving them for
live xterm.js processing.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Extract filterTerminalQueryResponses to terminal-escape-filter.ts
- Add comprehensive test suite with 54 test cases covering:
  - Cursor Position Reports (CPR) including row-only format
  - Primary/Secondary Device Attributes (DA1/DA2)
  - Device Attributes without ? or > prefix
  - DEC Private Mode Reports (DECRPM)
  - OSC color responses (10-19)
  - Tertiary Device Attributes (DA3)
  - XTVERSION responses
  - Edge cases and mixed content
- Simplify terminal-manager.test.ts to focus on integration

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 27, 2025

Walkthrough

This PR adds terminal escape sequence filtering for PTY output. A new filter module uses regex patterns to remove terminal query responses from terminal data. The filter is applied before appending data to scrollback and history, while preserving unfiltered data for the xterm frontend.

Changes

Cohort / File(s) Summary
Filter Implementation
apps/desktop/src/main/lib/terminal-escape-filter.ts, apps/desktop/src/main/lib/terminal-escape-filter.test.ts
Adds new module with regex-based patterns to filter terminal query responses (cursor position reports, device attributes, OSC sequences, etc.). Exports filterTerminalQueryResponses() function and patterns constant for testing. Comprehensive test suite validates filtering of control sequences while preserving normal output, and handles edge cases.
Terminal Manager Integration
apps/desktop/src/main/lib/terminal-manager.ts, apps/desktop/src/main/lib/terminal-manager.test.ts
Imports and applies filter to PTY data before appending to scrollback and writing to history. Raw unfiltered data continues to be emitted to xterm processor. Integration test verifies filtered data in scrollback while raw data reaches the consumer.

Sequence Diagram

sequenceDiagram
    participant PTY as PTY Process
    participant Manager as Terminal Manager
    participant Filter as Escape Filter
    participant Scrollback as Scrollback/History
    participant XTerm as XTerm Frontend

    PTY->>Manager: data (with escape sequences)
    Manager->>Filter: filterTerminalQueryResponses(data)
    Filter->>Filter: apply regex patterns
    Filter-->>Manager: filtered data
    Manager->>Scrollback: store filtered data
    Manager->>XTerm: emit unfiltered data
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Regex pattern validation: The escape filter uses multiple regex patterns for different terminal sequences (CPR, DA1/DA2/DA3, DECRPM, OSC, XTVERSION)—verify patterns correctly match intended sequences without over-matching legitimate output
  • Filter integration: Confirm the filter is applied at the right point in the data pipeline (before scrollback/history storage) and that unfiltered data still reaches xterm
  • Test coverage: Review comprehensive test suite to ensure edge cases like incomplete sequences, mixed content, and performance scenarios are properly validated

Possibly related PRs

Poem

🐰 A filter so fine, with patterns in place,
Escape codes begone from scrollback's embrace!
Raw data flows free to the frontend's delight,
While history stays clean—a regex-based rite!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is completely unfilled—all template sections are empty with no actual content provided about the changes. Fill in the Description section with details about filtering terminal escape sequences, specify the type of change (New feature), link any related issues, and document the test coverage added.
Title check ❓ Inconclusive The title 'filter terminal' is vague and generic, lacking specificity about what is being filtered or why. Consider a more descriptive title like 'Filter terminal query responses from PTY output' that clarifies the specific functionality being implemented.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch azure-wave-28

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c235eb3 and e50dd83.

📒 Files selected for processing (4)
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts (1 hunks)
  • apps/desktop/src/main/lib/terminal-escape-filter.ts (1 hunks)
  • apps/desktop/src/main/lib/terminal-manager.test.ts (1 hunks)
  • apps/desktop/src/main/lib/terminal-manager.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Apply Biome formatting, linting, and import organization at the root level for all code files

Files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-manager.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Avoid using any type; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck

Files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-manager.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
apps/**/*.{tsx,ts}

📄 CodeRabbit inference engine (AGENTS.md)

apps/**/*.{tsx,ts}: Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export
Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them

Files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-manager.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
apps/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Co-locate tests with their corresponding components (e.g., Component.test.tsx next to Component.tsx)

Files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
apps/desktop/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc

Files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-manager.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: Please use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary

Files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-manager.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
apps/desktop/**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.test.{ts,tsx,js,jsx}: Tests should have one assert per test
Tests should be readable
Tests should be fast
Tests should be independent
Tests should be repeatable

Files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
🧠 Learnings (4)
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be repeatable

Applied to files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be independent

Applied to files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be readable

Applied to files:

  • apps/desktop/src/main/lib/terminal-manager.test.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.test.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/main/lib/*-ipcs.ts : Implement Electron IPC handlers in dedicated files under apps/desktop/src/main/lib/ (e.g., workspace-ipcs.ts, terminal-ipcs.ts)

Applied to files:

  • apps/desktop/src/main/lib/terminal-manager.ts
  • apps/desktop/src/main/lib/terminal-escape-filter.ts
🧬 Code graph analysis (2)
apps/desktop/src/main/lib/terminal-manager.ts (1)
apps/desktop/src/main/lib/terminal-escape-filter.ts (1)
  • filterTerminalQueryResponses (106-108)
apps/desktop/src/main/lib/terminal-escape-filter.test.ts (1)
apps/desktop/src/main/lib/terminal-escape-filter.ts (1)
  • filterTerminalQueryResponses (106-108)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (10)
apps/desktop/src/main/lib/terminal-escape-filter.ts (3)

1-79: LGTM! Comprehensive pattern definitions with excellent documentation.

The escape sequence patterns are well-defined and thoroughly documented. The implementation correctly targets terminal query responses that should not appear in scrollback while preserving normal terminal output.


85-88: LGTM! Efficient pattern combination.

The combined regex pattern is constructed correctly and will match any of the defined query responses. The global flag ensures all occurrences in a data chunk are filtered.


106-111: LGTM! Clean and testable implementation.

The filter function is pure and straightforward. Exporting the patterns for testing is a good practice that allows for targeted pattern verification.

apps/desktop/src/main/lib/terminal-escape-filter.test.ts (4)

1-37: LGTM! Comprehensive preservation tests.

The test suite correctly verifies that normal terminal output (plain text, ANSI colors, cursor movements, styling) is preserved while filtering query responses. This provides confidence that the filter won't accidentally remove valid content.


39-238: LGTM! Thorough pattern coverage.

Each terminal query response pattern is tested comprehensively with various formats, mixed content, and multiple occurrences. The tests align well with the pattern definitions and provide strong validation of the filtering behavior.


240-292: LGTM! Excellent real-world scenario coverage.

The complex content tests validate filtering behavior in realistic scenarios including interleaved content and the exact user-reported issue. This provides confidence that the filter will work correctly in production.


294-333: LGTM! Robust edge case handling.

The edge case tests verify that incomplete sequences are preserved (preventing false positives), performance is acceptable for long strings, and the filter handles unicode and binary-like content correctly. This demonstrates a mature, production-ready implementation.

apps/desktop/src/main/lib/terminal-manager.test.ts (1)

511-560: LGTM! Effective integration test.

This test properly verifies the end-to-end behavior: raw data with escape sequences is emitted to the xterm processor (preserving responses for live processing), while only filtered data is stored in scrollback (preventing garbage on recovery). The test comment clearly documents the integration scope.

apps/desktop/src/main/lib/terminal-manager.ts (2)

6-6: LGTM! Proper module import.

The import follows the project structure and is correctly used in the PTY data handler.


140-148: LGTM! Correct integration of terminal escape filtering.

The implementation properly applies the filter to data before storing in scrollback and history, while emitting the original unfiltered data to xterm.js for live processing. The comments clearly explain the dual-path data flow, making the design intent explicit.


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 and usage tips.

@Kitenite Kitenite changed the title Azure wave 28 filter terminal Nov 27, 2025
@Kitenite Kitenite merged commit 77547c5 into main Nov 27, 2025
3 of 9 checks passed
@Kitenite Kitenite deleted the azure-wave-28 branch November 27, 2025 04:44
@coderabbitai coderabbitai Bot mentioned this pull request Nov 27, 2025
3 tasks
@coderabbitai coderabbitai Bot mentioned this pull request Dec 28, 2025
5 tasks
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.

1 participant