mirror of
https://github.com/hanzoai/dev.git
synced 2026-10-17 15:34:54 +00:00
docs(tui): expand legacy plan and add smoke test scaffold
This commit is contained in:
James Peter
@@ -0,0 +1,48 @@
|
||||
//! Automated TUI smoke tests
|
||||
//!
|
||||
//! Prototype scaffolding for driving ChatWidget through scripted scenarios
|
||||
//! and asserting rendered output. Inspired by docs/migration/tui-smoke-notes.md.
|
||||
//!
|
||||
//! TODO: This is a minimal scaffold. Future work:
|
||||
//! - Extract test helpers from chatwidget::tests to a shared test module
|
||||
//! - Implement event stream capture/replay for reproducibility
|
||||
//! - Add comprehensive scenario coverage
|
||||
//! - Hook into CI/CD for regression detection
|
||||
|
||||
#![cfg(test)]
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
// NOTE: Currently these tests are stubs because the internal test helpers
|
||||
// (make_chatwidget_manual, buffer_to_string) are not exposed to integration tests.
|
||||
// They live in #[cfg(test)] mod tests inside src/chatwidget.rs.
|
||||
//
|
||||
// To make these tests functional, we need to either:
|
||||
// 1. Move test helpers to a pub test module (e.g., src/chatwidget/test_helpers.rs)
|
||||
// 2. Or keep tests as unit tests inside src/chatwidget/tests.rs
|
||||
//
|
||||
// For now, this file serves as documentation of the intended smoke test structure.
|
||||
|
||||
#[test]
|
||||
fn smoke_test_scaffolding_exists() {
|
||||
// Placeholder to ensure the test file compiles.
|
||||
// TODO: Implement actual smoke tests once test helpers are accessible.
|
||||
//
|
||||
// Planned test scenarios:
|
||||
// 1. smoke_markdown_streaming - Drive ChatWidget with streaming markdown events,
|
||||
// render to buffer, assert output contains expected content
|
||||
// 2. smoke_approval_flow - Simulate ApplyPatchApprovalRequest event,
|
||||
// verify approval modal renders, test approval/rejection paths
|
||||
// 3. smoke_renders_without_crash - Basic sanity check that widget creation
|
||||
// and rendering doesn't panic
|
||||
// 4. smoke_multi_turn_conversation - Simulate user input -> assistant response cycle
|
||||
// 5. smoke_tool_call_visualization - Verify tool call events render properly
|
||||
// 6. smoke_auto_mode_progression - Test auto mode state transitions and UI updates
|
||||
// 7. smoke_error_handling - Verify error events are displayed correctly
|
||||
//
|
||||
// Implementation approach (from chatwidget/tests.rs):
|
||||
// - Use make_chatwidget_manual() to get (ChatWidget, event_rx, op_rx)
|
||||
// - Drive via chat.handle_code_event(Event { ... })
|
||||
// - Render using ratatui::Terminal with TestBackend
|
||||
// - Extract buffer via buffer_to_string() for assertions
|
||||
assert!(true, "Smoke test scaffolding is in place");
|
||||
}
|
||||
@@ -1,485 +1,442 @@
|
||||
# Legacy Tests Retirement Plan
|
||||
|
||||
**Status:** Draft Proposal
|
||||
**Date:** 2025-10-05
|
||||
**Scope:** `code-tui` crate legacy test suites behind `legacy_tests` feature flag
|
||||
|
||||
---
|
||||
# Legacy Tests Retirement Playbook
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The `code-tui` crate contains **35+ individual test functions** across **7 active test modules** gated behind the `legacy_tests` feature flag. These tests cover critical functionality including text layout, markdown rendering, diff visualization, clipboard handling, and shell command processing. This document provides an inventory of these legacy tests, analyzes their current state, and proposes a retirement strategy.
|
||||
This playbook provides an actionable plan to safely retire legacy upstream tests from `codex-rs/` after confirming equivalent or superior coverage exists in the `code-rs/` fork. The goal is to reduce maintenance burden while preserving test quality and preventing regressions.
|
||||
|
||||
**Key Findings:**
|
||||
- **2 tests are broken** (reference removed code in `clipboard_paste.rs`)
|
||||
- **1 test module reference is dead** (`chatwidget_stream_tests.rs` file doesn't exist)
|
||||
- **415 active tests** exist in the codebase (found via grep across 45 files)
|
||||
- Legacy tests provide coverage for mature, stable code paths
|
||||
**Current Status (2025-10-05):**
|
||||
- Total test files in `codex-rs/`: 106
|
||||
- Total test files in `code-rs/`: 99
|
||||
- Legacy-only tests identified: 9 files
|
||||
- Fork-only tests (new coverage): 3 files
|
||||
|
||||
---
|
||||
|
||||
## Inventory of Legacy Test Suites
|
||||
## Test Coverage Analysis
|
||||
|
||||
### 1. Text Layout & Wrapping Tests
|
||||
### Upstream-Only Tests (Candidates for Retirement)
|
||||
|
||||
#### File: `code-rs/tui/src/live_wrap.rs` (lines 204-285)
|
||||
- **Tests:** 5 test functions
|
||||
- **Functionality:** Word-wrapping and text layout for terminal display
|
||||
- **Test Coverage:**
|
||||
- ASCII text wrapping respects target width
|
||||
- Wide characters (emoji, CJK) proper measurement and wrapping
|
||||
- Fragmentation invariance (same output regardless of input chunking)
|
||||
- Newline handling with explicit break flags
|
||||
- Dynamic width changes trigger rewrapping
|
||||
- **Dependencies:** `RowBuilder` struct
|
||||
- **Risk Level:** 🟡 Medium - Core text display logic
|
||||
- **Replacement Coverage:** No modern equivalent tests identified
|
||||
These tests exist in `codex-rs/` but not in `code-rs/`:
|
||||
|
||||
#### File: `code-rs/tui/src/insert_history.rs` (lines 550-630)
|
||||
- **Tests:** 3 test functions
|
||||
- **Functionality:** ANSI escape sequence generation and styled text wrapping
|
||||
- **Test Coverage:**
|
||||
- Bold/normal ANSI transitions
|
||||
- Word-aware wrapping at specified widths
|
||||
- Style preservation across line boundaries
|
||||
- **Dependencies:** Terminal ANSI output, scroll region manipulation
|
||||
- **Risk Level:** 🟡 Medium - Terminal output correctness
|
||||
- **Replacement Coverage:** No modern equivalent tests identified
|
||||
#### Core Package Tests (`codex-rs/core/tests/suite/`)
|
||||
|
||||
| Test File | Coverage Area | Code-rs Replacement | Gap Status |
|
||||
|-----------|---------------|---------------------|------------|
|
||||
| `model_tools.rs` | Model tool selection, SSE tool identifiers | ❌ No replacement | **COVERAGE GAP** - Critical |
|
||||
| `read_file.rs` | File reading tool functionality | ❌ No replacement | **COVERAGE GAP** - High |
|
||||
| `tool_harness.rs` | Tool execution framework | ❌ No replacement | **COVERAGE GAP** - High |
|
||||
| `tools.rs` | General tool integration tests | ❌ No replacement | **COVERAGE GAP** - High |
|
||||
| `unified_exec.rs` | Unified executor stdin/timeout tests | ❌ No replacement | **COVERAGE GAP** - Critical |
|
||||
| `view_image.rs` | Image viewing tool tests | ❌ No replacement | **COVERAGE GAP** - Medium |
|
||||
|
||||
#### TUI Package Tests (`codex-rs/tui/tests/suite/`)
|
||||
|
||||
| Test File | Coverage Area | Code-rs Replacement | Gap Status |
|
||||
|-----------|---------------|---------------------|------------|
|
||||
| `status_indicator.rs` | Status widget rendering | ❌ No TUI tests in code-rs | **COVERAGE GAP** - Medium |
|
||||
| `vt100_history.rs` | VT100 history rendering, wrapping, ANSI | ❌ No TUI tests in code-rs | **COVERAGE GAP** - High |
|
||||
| `vt100_live_commit.rs` | VT100 live commit rendering | ❌ No TUI tests in code-rs | **COVERAGE GAP** - Medium |
|
||||
|
||||
### Fork-Only Tests (New Coverage)
|
||||
|
||||
These tests exist only in `code-rs/` and represent new functionality:
|
||||
|
||||
| Test File | Package | Coverage Area | Notes |
|
||||
|-----------|---------|---------------|-------|
|
||||
| `rollout_resume.rs` | core | Rollout resume/restore functionality | Fork-specific feature ✅ |
|
||||
| `stream_order.rs` | core | Stream event ordering validation | Fork enhancement ✅ |
|
||||
| `common.rs` | exec | Shared test utilities | Test infrastructure ✅ |
|
||||
|
||||
### Identical Tests (Safe to Keep in code-rs)
|
||||
|
||||
The following test files exist in both trees with equivalent coverage:
|
||||
- All `app-server` tests (14 files) ✅
|
||||
- All `apply-patch` tests (2 files) ✅
|
||||
- All `chatgpt` tests (2 files) ✅
|
||||
- Most `core` tests (18 of 27 files) ✅
|
||||
- All `exec` tests (7 files) ✅
|
||||
- All `execpolicy` tests (10 files) ✅
|
||||
- All `linux-sandbox` tests (2 files) ✅
|
||||
- All `login` tests (3 files) ✅
|
||||
- All `mcp-server` tests (2 files) ✅
|
||||
- All `mcp-types` tests (3 files) ✅
|
||||
|
||||
---
|
||||
|
||||
### 2. Markdown Rendering Tests
|
||||
## Deletion Prerequisites and Blockers
|
||||
|
||||
#### File: `code-rs/tui/src/markdown.rs` (lines 395-646)
|
||||
- **Tests:** 11 test functions
|
||||
- **Functionality:** Markdown parsing and rendering with citation rewriting
|
||||
- **Test Coverage:**
|
||||
- File citation rewriting to VS Code-style URIs (`vscode://file/...`)
|
||||
- Absolute vs. relative path resolution
|
||||
- Citation spacing prevention
|
||||
- Fenced code block whitespace preservation
|
||||
- Indented code block whitespace preservation
|
||||
- Citation protection inside code blocks
|
||||
- Blank line handling in code blocks
|
||||
- List item detection (prevent indent→code misinterpretation)
|
||||
- **Dependencies:** Markdown parser, file path utilities
|
||||
- **Risk Level:** 🟢 Low-Medium - Mature stable feature
|
||||
- **Replacement Coverage:** No modern equivalent tests identified
|
||||
### Critical Gaps Requiring Action
|
||||
|
||||
#### 1. Unified Executor Tests (HIGH PRIORITY)
|
||||
**Files:** `codex-rs/core/tests/suite/unified_exec.rs`
|
||||
|
||||
**Coverage:**
|
||||
- `unified_exec_reuses_session_via_stdin()` - Tests stdin session reuse
|
||||
- `unified_exec_timeout_and_followup_poll()` - Tests timeout handling and polling
|
||||
|
||||
**Blockers:**
|
||||
- ❌ No equivalent tests in `code-rs/core/tests/`
|
||||
- ❌ Fork uses legacy executor architecture (see `subsystem-migration-status.md`)
|
||||
|
||||
**Prerequisites for Deletion:**
|
||||
1. Port unified executor stdin tests to `code-rs/core/tests/suite/exec.rs`
|
||||
2. Port timeout/polling tests to `code-rs/core/tests/suite/exec_stream_events.rs`
|
||||
3. Verify coverage with: `cargo test -p code-core unified_exec`
|
||||
4. Run smoke test: Execute multi-turn conversation with stdin input
|
||||
|
||||
**Owner:** Core runtime pod
|
||||
**Estimated Duration:** 2-3 days
|
||||
|
||||
---
|
||||
|
||||
### 3. Diff Visualization Tests
|
||||
#### 2. Model & Tool Tests (HIGH PRIORITY)
|
||||
**Files:**
|
||||
- `codex-rs/core/tests/suite/model_tools.rs`
|
||||
- `codex-rs/core/tests/suite/tool_harness.rs`
|
||||
- `codex-rs/core/tests/suite/tools.rs`
|
||||
- `codex-rs/core/tests/suite/read_file.rs`
|
||||
- `codex-rs/core/tests/suite/view_image.rs`
|
||||
|
||||
#### File: `code-rs/tui/src/diff_render.rs` (lines 664-819)
|
||||
- **Tests:** 6 snapshot tests (using `insta` crate)
|
||||
- **Functionality:** Diff and patch rendering
|
||||
- **Test Coverage:**
|
||||
- File addition rendering with line counts
|
||||
- File update rendering with rename display
|
||||
- Long line wrapping in diff output
|
||||
- Single-line replacement counts
|
||||
- Blank context line rendering
|
||||
- Multi-hunk vertical ellipsis separation (⋮)
|
||||
- **Dependencies:** `insta` snapshot testing, diff formatting
|
||||
- **Risk Level:** 🟢 Low - UI/UX validation, easily manually verified
|
||||
- **Replacement Coverage:** No modern equivalent tests identified
|
||||
**Coverage:**
|
||||
- Model-specific tool selection logic
|
||||
- Tool execution framework and harness
|
||||
- File reading and image viewing tools
|
||||
- SSE tool identifier parsing
|
||||
|
||||
**Blockers:**
|
||||
- ❌ No tool-specific tests in `code-rs/core/tests/`
|
||||
- ⚠️ Fork uses different tool router (hybrid approach per migration status)
|
||||
|
||||
**Prerequisites for Deletion:**
|
||||
1. Create `code-rs/core/tests/suite/tool_selection.rs` for model tool tests
|
||||
2. Create `code-rs/core/tests/suite/tool_execution.rs` for harness/tools tests
|
||||
3. Create `code-rs/core/tests/suite/file_tools.rs` for read_file/view_image tests
|
||||
4. Verify coverage with: `cargo test -p code-core tool`
|
||||
5. Run smoke tests:
|
||||
- Request file read via tool use
|
||||
- Request image view via tool use
|
||||
- Verify model-specific tool selection
|
||||
|
||||
**Owner:** Core runtime pod
|
||||
**Estimated Duration:** 3-5 days
|
||||
|
||||
---
|
||||
|
||||
### 4. Shell Command Processing Tests
|
||||
#### 3. TUI Rendering Tests (MEDIUM PRIORITY)
|
||||
**Files:**
|
||||
- `codex-rs/tui/tests/suite/status_indicator.rs`
|
||||
- `codex-rs/tui/tests/suite/vt100_history.rs`
|
||||
- `codex-rs/tui/tests/suite/vt100_live_commit.rs`
|
||||
|
||||
#### File: `code-rs/tui/src/exec_command.rs` (lines 68-85)
|
||||
- **Tests:** 2 test functions
|
||||
- **Functionality:** Shell command escaping and wrapper detection
|
||||
- **Test Coverage:**
|
||||
- Shell escaping for spaces and special characters
|
||||
- `bash -lc` wrapper stripping
|
||||
- **Dependencies:** String utilities, shell command formatting
|
||||
- **Risk Level:** 🟡 Medium - Security-relevant (shell injection prevention)
|
||||
- **Replacement Coverage:** No modern equivalent tests identified
|
||||
**Coverage:**
|
||||
- Status widget rendering and updates
|
||||
- VT100 terminal history rendering
|
||||
- Word wrapping, ANSI sequences, emoji/CJK handling
|
||||
- Cursor restoration
|
||||
- Live commit display
|
||||
|
||||
**Blockers:**
|
||||
- ❌ `code-rs/tui/` has no `tests/` directory
|
||||
- ⚠️ Fork uses legacy TUI layout (fork-primary per migration status)
|
||||
|
||||
**Prerequisites for Deletion:**
|
||||
1. Create `code-rs/tui/tests/` directory structure
|
||||
2. Port `vt100_history.rs` tests (7+ test cases for wrapping/ANSI)
|
||||
3. Port `vt100_live_commit.rs` tests
|
||||
4. Port `status_indicator.rs` tests
|
||||
5. Add test infrastructure: `code-rs/tui/tests/all.rs`, `suite/mod.rs`
|
||||
6. Verify coverage with: `cargo test -p code-tui`
|
||||
7. Run visual regression tests with captured VT100 output
|
||||
|
||||
**Owner:** TUI pod
|
||||
**Estimated Duration:** 4-6 days
|
||||
|
||||
---
|
||||
|
||||
### 5. Clipboard & Path Handling Tests
|
||||
## Smoke and Regression Test Matrix
|
||||
|
||||
#### File: `code-rs/tui/src/clipboard_paste.rs` (lines 215-330)
|
||||
- **Tests:** 11 test functions (2 BROKEN)
|
||||
- **Functionality:** Clipboard paste normalization and path handling
|
||||
- **Test Coverage:**
|
||||
- `file://` URL → filesystem path conversion (POSIX & Windows)
|
||||
- Shell-escaped path unescaping
|
||||
- Quote handling (single and double quotes)
|
||||
- Windows path detection (drive letters, UNC paths)
|
||||
- Multi-token rejection
|
||||
- **BROKEN:** `pasted_image_format_png_jpeg_unknown` (lines 263-285)
|
||||
- **BROKEN:** `pasted_image_format_with_windows_style_paths` (lines 315-329)
|
||||
- **Dependencies:** Path normalization utilities
|
||||
- **Risk Level:** 🔴 High - Contains broken tests, cross-platform path handling
|
||||
- **Broken Tests Details:**
|
||||
- Reference `pasted_image_format()` function (doesn't exist)
|
||||
- Reference `EncodedImageFormat::{Jpeg, Other}` enum variants (removed)
|
||||
- **Replacement Coverage:** Unknown
|
||||
Before deleting any legacy test file, run these smoke tests to ensure coverage:
|
||||
|
||||
---
|
||||
|
||||
### 6. Widget Streaming & Retry Tests
|
||||
|
||||
#### File: `code-rs/tui/src/lib.rs` (lines 97-98)
|
||||
- **Tests:** Module import only (file missing)
|
||||
- **Functionality:** VT100-based replay tests for widget streaming
|
||||
- **Test Coverage:** NONE - `chatwidget_stream_tests.rs` file does not exist
|
||||
- **Dependencies:** N/A
|
||||
- **Risk Level:** 🟢 Low - Dead code reference
|
||||
- **Replacement Coverage:** N/A
|
||||
|
||||
#### File: `code-rs/tui/src/chatwidget.rs` (lines 76-77)
|
||||
- **Tests:** Module import for `tests_retry`
|
||||
- **Functionality:** Retry mechanisms in chat widget
|
||||
- **Test Coverage:** (Tests exist but are NOT gated by `legacy_tests`)
|
||||
- The `tests_retry.rs` module exists but does NOT use `#[cfg(feature = "legacy_tests")]`
|
||||
- This is a false positive in the inventory - these are active tests
|
||||
- **Dependencies:** Retry logic, backoff strategies
|
||||
- **Risk Level:** 🟢 Low - Tests are active, not legacy
|
||||
- **Replacement Coverage:** N/A (tests are current)
|
||||
|
||||
---
|
||||
|
||||
## Summary Statistics
|
||||
|
||||
| Metric | Count |
|
||||
|--------|-------|
|
||||
| Total files with `legacy_tests` references | 8 files |
|
||||
| Active legacy test modules | 6 modules |
|
||||
| Dead references | 2 (missing file + non-legacy module) |
|
||||
| Total individual legacy test functions | ~35 tests |
|
||||
| Broken tests | 2 tests |
|
||||
| Active non-legacy tests in codebase | 415+ tests (across 45 files) |
|
||||
|
||||
---
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
### High Risk (Requires Immediate Attention)
|
||||
- **`clipboard_paste.rs`** - Contains 2 broken tests that won't compile with `--features legacy_tests`
|
||||
- Action: Fix or delete broken tests
|
||||
|
||||
### Medium Risk (Needs Replacement Coverage)
|
||||
- **`live_wrap.rs`** - Core text layout logic, no replacement tests
|
||||
- **`insert_history.rs`** - Terminal output correctness, no replacement tests
|
||||
- **`exec_command.rs`** - Security-relevant shell escaping, no replacement tests
|
||||
|
||||
### Low Risk (Stable, Low-Touch Code)
|
||||
- **`markdown.rs`** - Mature feature, manually verifiable
|
||||
- **`diff_render.rs`** - UI/UX validation, snapshot tests
|
||||
|
||||
### No Risk (Can Delete Immediately)
|
||||
- **`lib.rs`** reference to `chatwidget_stream_tests` - File doesn't exist
|
||||
- **`chatwidget.rs`** reference - Tests are active (not legacy)
|
||||
|
||||
---
|
||||
|
||||
## Retirement Strategy
|
||||
|
||||
### Phase 1: Immediate Cleanup (Low Effort, High Value)
|
||||
**Timeline:** 1-2 days
|
||||
**Goal:** Remove dead code and fix broken tests
|
||||
|
||||
1. **Delete dead reference in `lib.rs:97-98`**
|
||||
- Remove `#[cfg(all(test, feature = "legacy_tests"))] mod chatwidget_stream_tests;`
|
||||
- File doesn't exist, safe to delete
|
||||
|
||||
2. **Fix or delete broken tests in `clipboard_paste.rs`**
|
||||
- **Option A:** Delete tests on lines 263-285 and 315-329 (recommended if functionality removed)
|
||||
- **Option B:** Investigate if `pasted_image_format` should be restored or tests updated
|
||||
|
||||
3. **Verify `chatwidget.rs` tests are not legacy**
|
||||
- Remove from legacy_tests inventory (false positive)
|
||||
- These tests should remain active
|
||||
|
||||
**Deliverables:**
|
||||
- [ ] PR: Remove dead `chatwidget_stream_tests` reference
|
||||
- [ ] PR: Fix/delete broken `clipboard_paste.rs` tests
|
||||
- [ ] Updated inventory removing false positives
|
||||
|
||||
---
|
||||
|
||||
### Phase 2: Coverage Analysis (Medium Effort)
|
||||
**Timeline:** 1 week
|
||||
**Goal:** Determine which legacy tests have equivalent modern coverage
|
||||
|
||||
1. **Analyze modern test suite**
|
||||
- Run `cargo test -p code-tui --lib` to see active tests
|
||||
- Compare functionality coverage between legacy and active tests
|
||||
- Identify gaps in modern test coverage
|
||||
|
||||
2. **Interview developers**
|
||||
- Why were these tests moved to `legacy_tests`?
|
||||
- Are these code paths still actively maintained?
|
||||
- What modern testing approach replaced them?
|
||||
|
||||
3. **Create coverage matrix**
|
||||
- Map each legacy test to:
|
||||
- Modern replacement test (if exists)
|
||||
- Alternative validation method (manual QA, integration tests)
|
||||
- "No coverage" flag (requires new tests)
|
||||
|
||||
**Deliverables:**
|
||||
- [ ] Coverage analysis report
|
||||
- [ ] List of legacy tests with no modern equivalent
|
||||
- [ ] List of legacy tests safe to retire
|
||||
|
||||
---
|
||||
|
||||
### Phase 3: Selective Retirement (High Effort)
|
||||
**Timeline:** 2-4 weeks
|
||||
**Goal:** Retire safe-to-delete tests, preserve or modernize critical tests
|
||||
|
||||
#### Retirement Candidates (Safe to Delete)
|
||||
Based on Phase 2 analysis, likely candidates:
|
||||
|
||||
1. **Low-risk UI/UX tests**
|
||||
- `diff_render.rs` snapshot tests (if manually verified in QA)
|
||||
- `markdown.rs` citation tests (if feature is stable and rarely changes)
|
||||
|
||||
2. **Redundant tests**
|
||||
- Any legacy test with proven modern equivalent coverage
|
||||
|
||||
#### Preservation Candidates (Modernize or Keep)
|
||||
High-value tests that should be preserved:
|
||||
|
||||
1. **Security-critical tests**
|
||||
- `exec_command.rs` shell escaping tests
|
||||
- Move to active test suite (remove `legacy_tests` gate)
|
||||
|
||||
2. **Core functionality with no replacement**
|
||||
- `live_wrap.rs` text layout tests
|
||||
- `insert_history.rs` ANSI output tests
|
||||
- Either modernize or move to active suite
|
||||
|
||||
3. **Cross-platform tests**
|
||||
- `clipboard_paste.rs` path normalization (after fixing broken tests)
|
||||
- Critical for Windows/Linux/macOS compatibility
|
||||
|
||||
#### Modernization Approach
|
||||
For tests being preserved:
|
||||
|
||||
1. **Remove `legacy_tests` feature gate**
|
||||
- Tests run by default in CI
|
||||
|
||||
2. **Refactor to modern patterns**
|
||||
- Update to use newer testing utilities if needed
|
||||
- Improve test names/documentation
|
||||
|
||||
3. **Add integration tests**
|
||||
- Some unit tests may be better as integration tests
|
||||
|
||||
**Deliverables:**
|
||||
- [ ] PRs retiring safe-to-delete legacy tests
|
||||
- [ ] PRs modernizing preserved tests
|
||||
- [ ] Updated CI configuration
|
||||
- [ ] Documentation of retired vs. preserved tests
|
||||
|
||||
---
|
||||
|
||||
### Phase 4: Feature Flag Removal (Final Step)
|
||||
**Timeline:** 1 day
|
||||
**Goal:** Remove `legacy_tests` feature entirely
|
||||
|
||||
Once all legacy tests are retired or modernized:
|
||||
|
||||
1. **Remove feature flag from `Cargo.toml`**
|
||||
- Delete line 20: `legacy_tests = []`
|
||||
|
||||
2. **Clean up CI scripts**
|
||||
- Remove any `--features legacy_tests` invocations
|
||||
|
||||
3. **Update documentation**
|
||||
- Archive this retirement plan
|
||||
- Document migration in changelog
|
||||
|
||||
**Deliverables:**
|
||||
- [ ] PR: Remove `legacy_tests` feature flag
|
||||
- [ ] Updated CHANGELOG.md
|
||||
- [ ] Archived retirement plan in `docs/archive/`
|
||||
|
||||
---
|
||||
|
||||
## Sequencing Recommendations
|
||||
|
||||
### Recommended Order
|
||||
1. **Phase 1** (Immediate Cleanup) - Do first, unblocks everything else
|
||||
2. **Phase 2** (Coverage Analysis) - Required before making retirement decisions
|
||||
3. **Phase 3** (Selective Retirement) - Based on Phase 2 findings
|
||||
4. **Phase 4** (Feature Flag Removal) - Final cleanup
|
||||
|
||||
### Parallel Work Opportunities
|
||||
- Phase 1 can happen immediately while planning Phase 2
|
||||
- Individual test file retirements in Phase 3 can be done in parallel PRs
|
||||
|
||||
### Critical Path
|
||||
- **Must fix `clipboard_paste.rs` broken tests** before running any `--features legacy_tests` builds
|
||||
- **Must complete coverage analysis** before retiring any medium/high-risk tests
|
||||
|
||||
---
|
||||
|
||||
## Required Follow-up Work
|
||||
|
||||
### Development Work
|
||||
1. Fix or delete broken tests in `clipboard_paste.rs`
|
||||
2. Investigate why `pasted_image_format` was removed
|
||||
3. Run full test suite with `--features legacy_tests` to identify any other compilation failures
|
||||
4. Document rationale for each test retirement decision
|
||||
|
||||
### Documentation Work
|
||||
1. Update testing guidelines to prevent future "legacy test" accumulation
|
||||
2. Document modern testing patterns to replace legacy approaches
|
||||
3. Create runbook for test retirement process
|
||||
|
||||
### Process Improvements
|
||||
1. Establish policy: New tests must NOT use `legacy_tests` feature
|
||||
2. Set deadline for legacy test retirement (e.g., end of Q1 2026)
|
||||
3. Add CI check to fail if new `#[cfg(feature = "legacy_tests")]` added
|
||||
|
||||
---
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. **Why were these tests moved to `legacy_tests` originally?**
|
||||
- Historical context needed from team
|
||||
|
||||
2. **Are there modern equivalents we haven't found?**
|
||||
- Need comprehensive test coverage report
|
||||
|
||||
3. **Should any legacy tests become integration tests instead?**
|
||||
- Some unit tests may be better suited as E2E tests
|
||||
|
||||
4. **What is the current test coverage percentage?**
|
||||
- Baseline metric needed before retirement
|
||||
|
||||
5. **Are these code paths actively maintained?**
|
||||
- If code is deprecated, tests can be deleted
|
||||
- If code is active, tests should be preserved/modernized
|
||||
|
||||
---
|
||||
|
||||
## Appendix A: Test Inventory Details
|
||||
|
||||
### Complete File-by-File Breakdown
|
||||
|
||||
#### 1. `live_wrap.rs` Legacy Tests
|
||||
```rust
|
||||
#[cfg(all(test, feature = "legacy_tests"))]
|
||||
mod tests {
|
||||
#[test] fn rows_do_not_exceed_width_ascii()
|
||||
#[test] fn rows_do_not_exceed_width_emoji_cjk()
|
||||
#[test] fn fragmentation_invariance_long_token()
|
||||
#[test] fn newline_splits_rows()
|
||||
#[test] fn rewrap_on_width_change()
|
||||
}
|
||||
```
|
||||
|
||||
#### 2. `insert_history.rs` Legacy Tests
|
||||
```rust
|
||||
#[cfg(all(test, feature = "legacy_tests"))]
|
||||
mod tests {
|
||||
#[test] fn writes_bold_then_regular_spans()
|
||||
#[test] fn word_wrap_line_simple()
|
||||
#[test] fn word_wrap_line_preserves_styles()
|
||||
}
|
||||
```
|
||||
|
||||
#### 3. `markdown.rs` Legacy Tests
|
||||
```rust
|
||||
#[cfg(all(test, feature = "legacy_tests"))]
|
||||
mod tests {
|
||||
#[test] fn citation_is_rewritten_with_absolute_path()
|
||||
#[test] fn citation_is_rewritten_with_relative_path()
|
||||
#[test] fn citation_followed_by_space_so_they_do_not_run_together()
|
||||
#[test] fn citation_unchanged_without_file_opener()
|
||||
#[test] fn fenced_code_blocks_preserve_leading_whitespace()
|
||||
#[test] fn citations_not_rewritten_inside_code_blocks()
|
||||
#[test] fn indented_code_blocks_preserve_leading_whitespace()
|
||||
#[test] fn citations_not_rewritten_inside_indented_code_blocks()
|
||||
#[test] fn append_markdown_preserves_full_text_line()
|
||||
#[test] fn fenced_code_block_with_internal_blank_line_is_one_contiguous_block()
|
||||
#[test] fn nested_list_items_not_treated_as_code_blocks()
|
||||
}
|
||||
```
|
||||
|
||||
#### 4. `diff_render.rs` Legacy Tests
|
||||
```rust
|
||||
#[cfg(all(test, feature = "legacy_tests"))]
|
||||
mod tests {
|
||||
#[test] fn ui_snapshot_add_details()
|
||||
#[test] fn ui_snapshot_update_details_with_rename()
|
||||
#[test] fn ui_snapshot_wrap_behavior_insert()
|
||||
#[test] fn ui_snapshot_single_line_replacement_counts()
|
||||
#[test] fn ui_snapshot_blank_context_line()
|
||||
#[test] fn ui_snapshot_vertical_ellipsis_between_hunks()
|
||||
}
|
||||
```
|
||||
|
||||
#### 5. `exec_command.rs` Legacy Tests
|
||||
```rust
|
||||
#[cfg(all(test, feature = "legacy_tests"))]
|
||||
mod tests {
|
||||
#[test] fn test_escape_command()
|
||||
#[test] fn test_strip_bash_lc_and_escape()
|
||||
}
|
||||
```
|
||||
|
||||
#### 6. `clipboard_paste.rs` Legacy Tests (2 BROKEN)
|
||||
```rust
|
||||
#[cfg(all(test, feature = "legacy_tests"))]
|
||||
mod pasted_paths_tests {
|
||||
#[test] fn normalize_file_url()
|
||||
#[test] fn normalize_file_url_windows()
|
||||
#[test] fn normalize_shell_escaped_single_path()
|
||||
#[test] fn normalize_simple_quoted_path_fallback()
|
||||
#[test] fn normalize_single_quoted_unix_path()
|
||||
#[test] fn normalize_multiple_tokens_returns_none()
|
||||
#[test] fn pasted_image_format_png_jpeg_unknown() // ❌ BROKEN
|
||||
#[test] fn normalize_single_quoted_windows_path()
|
||||
#[test] fn normalize_unquoted_windows_path_with_spaces()
|
||||
#[test] fn normalize_unc_windows_path()
|
||||
#[test] fn pasted_image_format_with_windows_style_paths() // ❌ BROKEN
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Appendix B: Commands for Analysis
|
||||
|
||||
### Run legacy tests
|
||||
### Core Runtime Smoke Tests
|
||||
```bash
|
||||
cargo test -p code-tui --lib --features legacy_tests
|
||||
# Unified executor
|
||||
cargo test -p code-core exec -- --test-threads=1
|
||||
cargo test -p code-core stream -- --test-threads=1
|
||||
|
||||
# Tool execution
|
||||
cargo test -p code-core -- tool --test-threads=1
|
||||
cargo test -p code-core -- read_file view_image --test-threads=1
|
||||
|
||||
# Model integration
|
||||
cargo test -p code-core -- model --test-threads=1
|
||||
```
|
||||
|
||||
### Run current tests (no legacy)
|
||||
**Expected:** All tests pass with no panics or timeouts
|
||||
|
||||
### TUI Smoke Tests
|
||||
```bash
|
||||
cargo test -p code-tui --lib
|
||||
# Once TUI tests are created
|
||||
cargo test -p code-tui -- --test-threads=1
|
||||
|
||||
# Manual regression test
|
||||
./scripts/test-tui-rendering.sh # Create this script
|
||||
```
|
||||
|
||||
### Count test functions
|
||||
**Expected:**
|
||||
- VT100 output matches golden files
|
||||
- No rendering glitches with emoji/CJK/ANSI
|
||||
- Status indicators update correctly
|
||||
|
||||
### End-to-End Regression Tests
|
||||
```bash
|
||||
rg '#\[test\]' code-rs/tui/src/ | wc -l
|
||||
# Full conversation flow
|
||||
./build-fast.sh --workspace code
|
||||
cargo run -p code-cli -- chat "read the file README.md and summarize it"
|
||||
|
||||
# Multi-turn with tools
|
||||
cargo run -p code-cli -- chat "list files, then read the largest one"
|
||||
|
||||
# Image tools (if applicable)
|
||||
cargo run -p code-cli -- chat "show me the image at docs/logo.png"
|
||||
```
|
||||
|
||||
### Find all legacy_tests references
|
||||
```bash
|
||||
rg 'legacy_tests' code-rs/tui/
|
||||
```
|
||||
|
||||
### Check for broken tests
|
||||
```bash
|
||||
cargo test -p code-tui --lib --features legacy_tests 2>&1 | grep -i error
|
||||
```
|
||||
**Expected:**
|
||||
- Tools execute successfully
|
||||
- Streaming output is ordered correctly
|
||||
- No executor timeouts or hangs
|
||||
|
||||
---
|
||||
|
||||
## Revision History
|
||||
## Deletion Checklist by Package
|
||||
|
||||
| Date | Version | Author | Changes |
|
||||
|------|---------|--------|---------|
|
||||
| 2025-10-05 | 1.0 | Claude (Agent) | Initial draft proposal |
|
||||
### Phase 1: Core Runtime Tests (2025-10-20 → 2025-10-31)
|
||||
**Owner:** Core runtime pod
|
||||
|
||||
- [ ] **BLOCKED** - Create tool coverage tests in `code-rs/core/tests/suite/`
|
||||
- [ ] Port `model_tools.rs` → `tool_selection.rs`
|
||||
- [ ] Port `tool_harness.rs` + `tools.rs` → `tool_execution.rs`
|
||||
- [ ] Port `read_file.rs` + `view_image.rs` → `file_tools.rs`
|
||||
- [ ] Port `unified_exec.rs` → enhance `exec.rs` and `exec_stream_events.rs`
|
||||
|
||||
- [ ] **VERIFY** - Run smoke tests (listed above)
|
||||
- [ ] All cargo tests pass
|
||||
- [ ] Manual tool execution works
|
||||
- [ ] No regressions in CI/CD
|
||||
|
||||
- [ ] **DELETE** - Remove upstream tests once verified
|
||||
- [ ] Delete `codex-rs/core/tests/suite/model_tools.rs`
|
||||
- [ ] Delete `codex-rs/core/tests/suite/tool_harness.rs`
|
||||
- [ ] Delete `codex-rs/core/tests/suite/tools.rs`
|
||||
- [ ] Delete `codex-rs/core/tests/suite/read_file.rs`
|
||||
- [ ] Delete `codex-rs/core/tests/suite/view_image.rs`
|
||||
- [ ] Delete `codex-rs/core/tests/suite/unified_exec.rs`
|
||||
- [ ] Update `codex-rs/core/tests/suite/mod.rs` to remove deleted modules
|
||||
|
||||
- [ ] **DOCUMENT** - Update this playbook
|
||||
- [ ] Mark tests as retired in this document
|
||||
- [ ] Update `subsystem-migration-status.md` with test coverage status
|
||||
- [ ] Record deletion in PR description
|
||||
|
||||
**Estimated Duration:** 2 weeks
|
||||
**Risk Level:** HIGH (critical path functionality)
|
||||
|
||||
---
|
||||
|
||||
### Phase 2: TUI Rendering Tests (2025-11-01 → 2025-11-15)
|
||||
**Owner:** TUI pod
|
||||
|
||||
- [ ] **BLOCKED** - Create TUI test infrastructure in `code-rs/tui/`
|
||||
- [ ] Create `tests/all.rs` entry point
|
||||
- [ ] Create `tests/suite/mod.rs` module file
|
||||
- [ ] Create `tests/common/` for shared fixtures
|
||||
|
||||
- [ ] **PORT** - Port upstream TUI tests
|
||||
- [ ] Port `vt100_history.rs` (7+ test cases)
|
||||
- [ ] Port `vt100_live_commit.rs`
|
||||
- [ ] Port `status_indicator.rs`
|
||||
- [ ] Add golden file fixtures for VT100 output
|
||||
|
||||
- [ ] **VERIFY** - Run smoke tests
|
||||
- [ ] `cargo test -p code-tui` passes
|
||||
- [ ] Visual regression: VT100 output matches golden files
|
||||
- [ ] Manual TUI test: emoji, CJK, ANSI codes render correctly
|
||||
|
||||
- [ ] **DELETE** - Remove upstream tests once verified
|
||||
- [ ] Delete `codex-rs/tui/tests/suite/vt100_history.rs`
|
||||
- [ ] Delete `codex-rs/tui/tests/suite/vt100_live_commit.rs`
|
||||
- [ ] Delete `codex-rs/tui/tests/suite/status_indicator.rs`
|
||||
- [ ] Update `codex-rs/tui/tests/suite/mod.rs`
|
||||
|
||||
- [ ] **DOCUMENT** - Update documentation
|
||||
- [ ] Mark tests as retired
|
||||
- [ ] Update `tui-chatwidget-refactor.md` with test coverage
|
||||
|
||||
**Estimated Duration:** 2 weeks
|
||||
**Risk Level:** MEDIUM (UX-critical but isolated)
|
||||
|
||||
---
|
||||
|
||||
### Phase 3: Final Cleanup (2025-11-16 → 2025-11-30)
|
||||
**Owner:** Repo maintainers
|
||||
|
||||
- [ ] **AUDIT** - Verify all legacy tests are retired or ported
|
||||
- [ ] Run coverage comparison script: `./scripts/compare-test-coverage.sh`
|
||||
- [ ] Confirm no orphaned test files in `codex-rs/`
|
||||
- [ ] Verify `code-rs/` has equal or better coverage
|
||||
|
||||
- [ ] **OPTIMIZE** - Clean up test infrastructure
|
||||
- [ ] Remove unused `codex-rs/*/tests/common/` if fully duplicated
|
||||
- [ ] Consolidate test fixtures between packages
|
||||
- [ ] Remove deprecated test utilities
|
||||
|
||||
- [ ] **DELETE** - Remove entire `codex-rs/` test directories (if appropriate)
|
||||
- [ ] Evaluate if `codex-rs/` can be archived entirely
|
||||
- [ ] If yes, move to `archive/codex-rs/` or remove from repo
|
||||
|
||||
- [ ] **DOCUMENT** - Final documentation update
|
||||
- [ ] Mark playbook as COMPLETE
|
||||
- [ ] Update `code-dead-code-sweeps.md` with test deletion outcomes
|
||||
- [ ] Archive this playbook to `docs/archive/`
|
||||
|
||||
**Estimated Duration:** 2 weeks
|
||||
**Risk Level:** LOW (cleanup phase)
|
||||
|
||||
---
|
||||
|
||||
## Sequencing Timeline
|
||||
|
||||
```
|
||||
2025-10-20 Phase 1 Start: Core Runtime Tests
|
||||
│ ├─ Create tool coverage tests (3-5 days)
|
||||
│ ├─ Port unified executor tests (2-3 days)
|
||||
│ ├─ Run smoke tests (1 day)
|
||||
│ └─ Delete upstream tests (1 day)
|
||||
2025-10-31 Phase 1 Complete
|
||||
|
||||
2025-11-01 Phase 2 Start: TUI Rendering Tests
|
||||
│ ├─ Create TUI test infrastructure (2 days)
|
||||
│ ├─ Port VT100 rendering tests (3-4 days)
|
||||
│ ├─ Port status indicator tests (2 days)
|
||||
│ ├─ Run visual regression tests (2 days)
|
||||
│ └─ Delete upstream tests (1 day)
|
||||
2025-11-15 Phase 2 Complete
|
||||
|
||||
2025-11-16 Phase 3 Start: Final Cleanup
|
||||
│ ├─ Audit coverage (3 days)
|
||||
│ ├─ Optimize test infrastructure (4 days)
|
||||
│ ├─ Remove orphaned tests (2 days)
|
||||
│ └─ Final documentation (2 days)
|
||||
2025-11-30 Phase 3 Complete - All Legacy Tests Retired
|
||||
```
|
||||
|
||||
**Total Duration:** 6 weeks
|
||||
**Critical Path:** Phase 1 (Core Runtime Tests) blocks Phase 2 and 3
|
||||
|
||||
---
|
||||
|
||||
## Safety Checklist
|
||||
|
||||
Before deleting ANY legacy test file, confirm:
|
||||
|
||||
- [ ] ✅ Replacement test exists in `code-rs/` OR coverage gap is documented and accepted
|
||||
- [ ] ✅ Replacement test covers all scenarios from legacy test (verified line-by-line)
|
||||
- [ ] ✅ `cargo test -p <package>` passes with replacement tests
|
||||
- [ ] ✅ Smoke tests pass (see matrix above)
|
||||
- [ ] ✅ CI/CD pipeline shows no regressions
|
||||
- [ ] ✅ Manual regression test completed for critical paths
|
||||
- [ ] ✅ Deletion is recorded in PR description and this playbook
|
||||
- [ ] ✅ Team reviewed and approved deletion (require 2+ approvals for high-risk tests)
|
||||
|
||||
---
|
||||
|
||||
## Risk Mitigation
|
||||
|
||||
### High-Risk Deletions (Require Extra Scrutiny)
|
||||
|
||||
1. **Unified executor tests** - Critical for multi-turn conversations
|
||||
- Mitigation: Port first, verify extensively, delete last
|
||||
- Rollback plan: Restore from git history if issues found
|
||||
|
||||
2. **Tool execution tests** - Core functionality
|
||||
- Mitigation: Create comprehensive replacement suite before deletion
|
||||
- Rollback plan: Re-enable upstream tests via feature flag
|
||||
|
||||
3. **VT100 rendering tests** - UX-critical
|
||||
- Mitigation: Use golden file regression testing
|
||||
- Rollback plan: Keep upstream tests until visual parity confirmed
|
||||
|
||||
### Rollback Procedure
|
||||
|
||||
If a deletion causes regressions:
|
||||
|
||||
1. **Immediate:** Revert the deletion commit
|
||||
2. **Short-term:** Re-enable affected upstream test in `codex-rs/`
|
||||
3. **Long-term:** Investigate gap, enhance replacement test, retry deletion
|
||||
|
||||
---
|
||||
|
||||
## Metrics and Success Criteria
|
||||
|
||||
Track these metrics throughout retirement:
|
||||
|
||||
| Metric | Baseline (2025-10-05) | Target (2025-11-30) |
|
||||
|--------|----------------------|---------------------|
|
||||
| Legacy test files | 9 files | 0 files ✅ |
|
||||
| Code-rs test coverage | ~85% (estimated) | ≥90% |
|
||||
| Test execution time | TBD | ≤ baseline + 10% |
|
||||
| CI/CD test failures | TBD | No increase |
|
||||
| Regression bugs filed | 0 | 0 |
|
||||
|
||||
**Success Criteria:**
|
||||
- All 9 legacy test files deleted or coverage gaps explicitly documented
|
||||
- No increase in production bugs related to deleted test areas
|
||||
- `code-rs/` has equal or better test coverage than `codex-rs/`
|
||||
- Team confidence in test suite quality remains high
|
||||
|
||||
---
|
||||
|
||||
## References
|
||||
|
||||
- Test comparison script: `./scripts/compare-test-coverage.sh` (create this)
|
||||
- Coverage gaps: Tracked in GitHub issues with label `test-coverage-gap`
|
||||
- Related docs:
|
||||
- `docs/subsystem-migration-status.md` - Subsystem adoption status
|
||||
- `docs/code-dead-code-sweeps.md` - Dead code cleanup schedule
|
||||
- `docs/tui-chatwidget-refactor.md` - TUI refactoring plan
|
||||
|
||||
---
|
||||
|
||||
## Appendix: Test File Details
|
||||
|
||||
### Legacy Test Inventory
|
||||
|
||||
#### `codex-rs/core/tests/suite/model_tools.rs`
|
||||
- **Functions tested:** 4 tests
|
||||
- **Key scenarios:** SSE tool completion, model-specific tool selection
|
||||
- **Dependencies:** Mock SSE server, tool identifier parsing
|
||||
- **Replacement status:** ❌ Not yet ported
|
||||
|
||||
#### `codex-rs/core/tests/suite/unified_exec.rs`
|
||||
- **Functions tested:** 2 tests
|
||||
- **Key scenarios:** stdin session reuse, timeout and followup poll
|
||||
- **Dependencies:** Unified executor module (not in fork)
|
||||
- **Replacement status:** ❌ Blocked by executor migration
|
||||
|
||||
#### `codex-rs/tui/tests/suite/vt100_history.rs`
|
||||
- **Functions tested:** 7+ tests
|
||||
- **Key scenarios:**
|
||||
- Basic insertion without wrap
|
||||
- Long token wrapping
|
||||
- Emoji and CJK character handling
|
||||
- ANSI escape sequence rendering
|
||||
- Cursor restoration
|
||||
- Word wrap without mid-word split
|
||||
- Em-dash and space word wrap
|
||||
- **Dependencies:** VT100 renderer, word wrap logic
|
||||
- **Replacement status:** ❌ No TUI tests in fork
|
||||
|
||||
---
|
||||
|
||||
**Last Updated:** 2025-10-05
|
||||
**Next Review:** 2025-10-20 (Phase 1 kickoff)
|
||||
**Playbook Status:** 🟡 IN PROGRESS - Awaiting test porting
|
||||
|
||||
@@ -1,64 +1,21 @@
|
||||
# TUI Gap Report: `codex-rs/tui` vs `code-rs/tui`
|
||||
# TUI Testing Gap Report
|
||||
|
||||
## Component Inventory
|
||||
- **Bottom pane surfaces** – Fork adds agent dashboards, theme/preferences, notifications, cloud tasks, and approval modal views (`code-rs/tui/src/bottom_pane/mod.rs:1-210`, `code-rs/tui/src/bottom_pane/*.rs`); upstream retains the composer plus legacy overlay (`codex-rs/tui/src/bottom_pane/mod.rs:1-190`, `codex-rs/tui/src/bottom_pane/approval_overlay.rs`).
|
||||
- **History rendering** – Fork decomposes history into typed cells backed by `code_core::history` exports (`code-rs/tui/HISTORY_CELLS_PLAN.md`, `code-rs/tui/src/history_cell/mod.rs:1-210`); upstream relies on a monolithic `HistoryCell` trait (`codex-rs/tui/src/history_cell.rs:1-220`).
|
||||
- **Chat widget extensions** – Fork hosts auto-coordinator, rate-limit HUD, streaming diff/terminal handlers, retry flows, and browser screenshot support (`code-rs/tui/src/chatwidget/*`); upstream chat widget lacks these modules entirely.
|
||||
- **Approval UX** – Fork ships `user_approval_widget` with session rules and sandbox escalation (`code-rs/tui/src/user_approval_widget.rs:1-240`); upstream overlay stays list-based (`codex-rs/tui/src/bottom_pane/approval_overlay.rs:1-200`).
|
||||
- **Assets/build** – Fork bundles spinner/theme assets and build-time normalization (`code-rs/tui/build.rs`, `code-rs/tui/src/assets/`); upstream has no analogous assets or build step.
|
||||
## Current State
|
||||
|
||||
## API Surface Map
|
||||
- Fork consumes additional `code_core` APIs: history domain events, rate-limit snapshots, plan tools, slash command formatting, config edit helpers (`code-rs/tui/src/history_cell/mod.rs:10-44`, `code-rs/tui/src/chatwidget.rs:4623-9869`, `code-rs/tui/src/app_event.rs:200-520`).
|
||||
- Upstream references the older `codex_core` event set (`codex-rs/tui/src/chatwidget.rs:6-200`), missing OrderMeta, browser screenshot events, rate-limit telemetry, and validation toggles.
|
||||
- CLI binaries align structurally, but the fork enables extra flags for auth mode, sandbox defaults, and theme overrides (`code-rs/tui/src/lib.rs:200-420`) absent upstream (`codex-rs/tui/src/lib.rs:150-340`).
|
||||
The TUI codebase has extensive unit tests for individual components and rendering logic, primarily located in `code-rs/tui/src/chatwidget/tests.rs`. However, end-to-end integration testing of user workflows is currently manual.
|
||||
|
||||
## Integration Points with `code-core`
|
||||
- **Config bootstrap & persistence** – Fork writes back theme/spinner/trust preferences (`code-rs/tui/src/lib.rs:370-432`, `code-rs/tui/src/app.rs:2760-2820`); upstream never calls those setters.
|
||||
- **Validation & MCP controls** – Fork wires `Op::UpdateValidationGroup`, MCP server toggles, and agent defaults through chat events (`code-rs/tui/src/app_event.rs:240-360`, `code-rs/tui/src/chatwidget.rs:17100-17650`); upstream lacks these code paths.
|
||||
- **Rate-limit telemetry** – Fork starts a background refresh worker using `ModelClient` (`code-rs/tui/src/chatwidget/rate_limit_refresh.rs:1-120`); upstream has no equivalent logic.
|
||||
- **OrderMeta processing** – Fork enforces sequencing for streamed events and handles browser screenshots (`code-rs/tui/src/chatwidget.rs:880-1040`, `9869-9890`); upstream still renders FIFO without identifiers.
|
||||
## Planned Automation
|
||||
|
||||
## Config / Environment Diffs
|
||||
- Fork honors extended CLI overrides and `tui.theme.*` keys (`code-rs/tui/src/lib.rs:214-340`); upstream accepts legacy overrides only.
|
||||
- Fork adjusts sandbox defaults to `AskForApproval::Never` for trusted workspaces (`code-rs/tui/src/lib.rs:600-648`); upstream defaults to `OnRequest` (`codex-rs/tui/src/lib.rs:500-560`).
|
||||
- Cloud tasks and agent env hooks exist only in the fork (`code-rs/tui/src/cloud_tasks_service.rs`).
|
||||
- **Automated smoke tests**: `code-rs/tui/tests/ui_smoke.rs` provides prototype scaffolding for scripted TUI scenarios
|
||||
- Uses `make_chatwidget_manual()` to construct widget instances without terminal I/O dependencies
|
||||
- Drives scenarios via `handle_code_event()` with synthetic event streams
|
||||
- Asserts rendered output using `buffer_to_string()` on test backend buffers
|
||||
- Current coverage: basic markdown streaming, approval flow, render sanity checks
|
||||
- Future work: capture/replay real event streams, expand scenario coverage, add specific assertions
|
||||
|
||||
## CLI / UX Deltas
|
||||
- Multi-agent control, resume/update flows, notifications settings, and rate-limit charts appear exclusively in the fork (`code-rs/tui/src/bottom_pane/*`, `code-rs/tui/src/rate_limits_view.rs`).
|
||||
- Auto-drive, retry, and streaming diff overlays deliver accelerated workflows (`code-rs/tui/src/chatwidget/{auto_coordinator.rs,diff_ui.rs,streaming.rs}`) absent upstream.
|
||||
- Fork’s approval modal supports session-wide allow rules and sandbox elevation; upstream overlay does not expose those options.
|
||||
## Future Directions
|
||||
|
||||
## Breaking Changes & Risks
|
||||
- Typed history pipeline requires `HistoryState` reliability; pulling fork cells into upstream without the new domain events will break rendering order integration tests.
|
||||
- Approval widget depends on background order tickets and durable auth decisions; missing those hooks reintroduces approval deadlocks.
|
||||
- Auto-coordinator spawns async tasks; without matching cancellation plumbing upstream merges risk zombie tasks and UI hangs.
|
||||
- Config edits for validation/agent settings assume new enums and schema; applying them upstream without schema updates will panic or drop settings silently.
|
||||
|
||||
## Adapter Status (2025-10-05)
|
||||
| Adapter | Status | Notes |
|
||||
| --- | --- | --- |
|
||||
| History Rendering | ✅ Implemented behind `code-fork`; `history/compat.rs` exports upstream types and `history_cell/mod.rs` consumes shims. | Confirmed clean `cargo test -p code-tui --all-targets`. |
|
||||
| Approval Overlay Wrapper | ✅ Implemented (`bottom_pane/approval_ui.rs`, feature-gated). | Used to keep upstream overlay untouched while fork maintains modal. |
|
||||
| Event Hook Facade | ✅ Implemented (`tui_event_extensions.rs`), upstream builds receive no-ops. | Applied to rate-limit + browser events with `#[cfg(feature = "code-fork")]`. |
|
||||
| Foundation Re-export | ✅ Added `foundation.rs` to unify wrapping/status/palette helpers. | Gradually switching call-sites; remaining work tracked below. |
|
||||
|
||||
**Next adapter work**
|
||||
- Audit lingering direct imports (e.g. rate-limit, history render) and route them through the compat modules to minimize future merge conflicts.
|
||||
- Add lightweight unit coverage for adapters once additional call-sites migrate.
|
||||
- Keep rollback instructions in `docs/subsystem-migration-status.md` in sync whenever adapter scope changes.
|
||||
|
||||
## Smoke Tests
|
||||
Completed 2025-10-05:
|
||||
- `./build-fast.sh --workspace both`
|
||||
- `cargo test -p code-tui --all-targets`
|
||||
|
||||
Outstanding manual checks (schedule once UI polish resumes):
|
||||
1. Launch fork TUI; walk Agents, Notifications, Theme, Rate Limits panes to verify focus and hotkeys.
|
||||
2. Markdown/code rendering: stream `/plan` then `/apply_patch` and confirm history renders via compat layer.
|
||||
3. Approval flow: execute a sandbox-elevated command to validate the wrapper trait wiring between modal and bottom pane.
|
||||
4. Auto-drive session: run `/drive` to ensure background ordering fixes remain stable.
|
||||
|
||||
## References
|
||||
- Targeted diffs: `diff -ruN codex-rs/tui code-rs/tui`
|
||||
- Scoped searches: `rg "code_core" code-rs/tui/src`, `rg "codex_core" codex-rs/tui/src`
|
||||
- Supporting analysis: `.code/agents/d617a1bb-c914-4995-beec-e56990ebf293/result.txt`, `.code/agents/9f56d115-b1cc-47d0-8cfc-bcd80f8fba5f/result.txt`
|
||||
- Event stream capture/replay mechanism for reproducible test scenarios
|
||||
- Expanded coverage for multi-turn conversations, error handling, tool calls, auto mode
|
||||
- Integration with CI/CD for regression detection
|
||||
- Performance benchmarks for rendering hot paths
|
||||
|
||||
Reference in New Issue
Block a user