Files
telegram-tui/docs/REFACTOR_PLAN.md
2026-05-17 18:55:36 +03:00

251 lines
9.4 KiB
Markdown

# tele-tui Refactor Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use `superpowers:subagent-driven-development` (recommended) or `superpowers:executing-plans` to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Finish the next review/refactor layer after the TDLib facade split, keeping behavior stable while making the code easier to test, review, and change.
**Architecture:** The current working tree already introduces scoped TDLib traits, removes the local `build.rs`, switches message formatting to the system local timezone, moves media chat handlers into a submodule, and makes fake TDLib state more explicit. The remaining work should continue in small vertical slices with focused tests after each slice.
**Tech Stack:** Rust 2021, Tokio, tdlib-rs, ratatui, crossterm, insta, criterion, Woodpecker CI.
---
## Current Baseline
The current uncommitted layer should be treated as the baseline before starting the next refactor tasks.
- TDLib facade is split into scoped traits in `src/tdlib/trait.rs`.
- `src/tdlib/client_impl.rs` implements the scoped traits for `TdClient`.
- `current_chat_messages()` returns `Cow<'_, [MessageInfo]>`; mutation goes through `update_current_chat_messages`.
- Runtime date formatting uses the system local timezone; tests can inject deterministic time through `FixedLocalTime`.
- Media/image/voice chat handling is moved from `src/input/handlers/chat.rs` into `src/input/handlers/chat/media.rs`.
- The repository no longer uses the local `build.rs` that tried to link `tdlib-rs` during build-script execution.
Verification already used for this baseline:
```bash
cargo check --all-targets --all-features
cargo clippy --all-targets --all-features -- -D warnings
cargo test --all-features
git diff --check
```
## Task 0: Commit Current Layer
Goal: preserve the completed facade/timezone/media/test-cleanup work before deeper refactors.
Files to review before commit:
- `CONTEXT.md`
- `Cargo.toml`
- `src/tdlib/trait.rs`
- `src/tdlib/mod.rs`
- `src/tdlib/client_impl.rs`
- `src/utils/formatting.rs`
- `src/input/handlers/chat.rs`
- `src/input/handlers/chat/media.rs`
- `tests/helpers/fake_tdclient.rs`
- `tests/helpers/fake_tdclient_impl.rs`
- touched tests and benches
Steps:
- [x] Review `git diff --stat` and `git diff --check`.
- [x] Run the full verification commands from the baseline section.
- [x] Commit this layer separately from the follow-up refactors.
## Task 1: Split `FakeTdClient`
Goal: reduce `tests/helpers/fake_tdclient.rs` from one large mixed helper into smaller modules with clear responsibilities.
Target files:
- `tests/helpers/fake_tdclient.rs`
- `tests/helpers/fake_tdclient_impl.rs`
- `tests/helpers/mod.rs`
- new `tests/helpers/fake_tdclient/state.rs`
- new `tests/helpers/fake_tdclient/builders.rs`
- new `tests/helpers/fake_tdclient/operations.rs`
- new `tests/helpers/fake_tdclient/inspect.rs`
Steps:
- [x] Move state aliases and shared storage fields into `state.rs`.
- [x] Move fixture construction helpers such as `with_chat`, `with_messages`, and account setup helpers into `builders.rs`.
- [x] Move behavior helpers such as send/edit/delete/reaction operations into `operations.rs`.
- [x] Move read/assertion helpers such as sent-message inspection and viewed-message inspection into `inspect.rs`.
- [x] Keep the public test API stable unless a call site becomes simpler and safer.
- [x] Remove direct test access to internal `Arc<Mutex<...>>` fields where helper methods are clearer.
- [x] Run `cargo test --all-features`.
Acceptance criteria:
- `FakeTdClient` remains easy to construct in integration tests.
- No test loses behavior coverage.
- `tests/helpers/fake_tdclient.rs` becomes a small module entry point instead of the main implementation body.
## Task 2: Tighten Internal TDLib Mutation API
Goal: limit raw mutable access to TDLib client internals and replace cross-module state poking with domain-specific methods.
Target files:
- `src/tdlib/client.rs`
- `src/tdlib/chat_helpers.rs`
- `src/tdlib/update_handlers.rs`
- `src/tdlib/message_converter.rs`
- `src/tdlib/client_impl.rs`
Search command:
```bash
rg -n "current_chat_messages_mut|chats_mut|folders_mut|pending_user_ids_mut|user_cache_mut" src/tdlib
```
Steps:
- [x] Add focused methods on `TdClient` for common mutations: update chat, update message by id, queue pending user, update user cache, update folders.
- [x] Replace raw `*_mut()` usage in helper/update modules with those methods.
- [x] Keep raw mutable access private to `TdClient` implementation where it is still needed.
- [x] Add or update tests around message updates, user-cache updates, and chat-list updates.
- [x] Run `cargo test --all-features`.
Acceptance criteria:
- External and helper modules express intent through domain methods.
- Raw state access is either gone or contained in a small internal area.
## Task 3: Split Remaining Large Input and UI Files
Goal: make modal, message rendering, and app/input code easier to review independently.
Target files:
- `src/input/handlers/modal.rs`
- `src/input/handlers/chat.rs`
- `src/app/mod.rs`
- `src/ui/messages.rs`
- new `src/input/handlers/modal/account.rs`
- new `src/input/handlers/modal/delete.rs`
- new `src/input/handlers/modal/profile.rs`
- new `src/input/handlers/modal/reactions.rs`
- new `src/input/handlers/modal/pinned.rs`
- new `src/ui/messages/header.rs`
- new `src/ui/messages/list.rs`
- new `src/ui/messages/pinned.rs`
Steps:
- [x] Split modal handlers by modal type and keep `modal.rs` as the dispatcher/module entry point.
- [x] Split message UI rendering into header, pinned-message, and list rendering modules.
- [x] Keep public function names stable until each split is covered by tests.
- [x] Avoid mixing behavior changes with file movement.
- [x] Run focused modal/navigation/message tests after each split.
- [x] Run `cargo test --all-features` after the full split.
Acceptance criteria:
- Large files are reduced to dispatch/orchestration roles.
- The split does not change key handling or rendering behavior.
- Module names match user-facing concepts instead of implementation accidents.
## Task 4: Remove Production `unwrap()` Risk
Goal: keep test unwraps where useful, but remove production unwraps where runtime data can be absent.
Target files:
- `src/input/handlers/chat/media.rs`
- `src/input/handlers/chat.rs`
- `src/ui/components/message_bubble.rs`
- `src/utils/tdlib.rs`
- `src/audio/player.rs`
Search command:
```bash
rg -n "unwrap\\(|expect\\(|panic!\\(" src
```
Steps:
- [x] Replace `photo_info().unwrap()` and `voice_info().unwrap()` with `let Some(...) else { ... }`.
- [x] Replace `selected_chat_id.unwrap()` with an early return or status message.
- [x] Review playback/message unwraps in `message_bubble.rs` and convert absent data into graceful UI fallback.
- [x] Audit mutex unwraps separately; leave only cases where poisoning should be fatal and documented by context.
- [x] Add tests for missing media metadata and absent selected chat.
- [x] Run `cargo clippy --all-targets --all-features -- -D warnings`.
Acceptance criteria:
- Malformed or partial TDLib data does not panic in normal UI paths.
- Error handling stays local and does not add noisy user-facing text.
## Task 5: Resolve TODO and Compatibility Paths
Goal: make unfinished behavior explicit: either implement it, test it, or remove stale comments.
Target files:
- `src/input/key_handler.rs`
- `src/tdlib/reactions.rs`
- `src/tdlib/messages/operations.rs`
Steps:
- [x] Review every TODO in `src/`.
- [x] Convert active TODOs into tests or tracked plan items.
- [x] Remove stale TODOs whose behavior is already implemented.
- [x] For pinned-message compatibility in `messages/operations.rs`, decide whether the fallback is still needed and document the decision in code or tests.
- [x] Run `cargo test --all-features`.
Acceptance criteria:
- Remaining TODOs point to real unresolved behavior.
- No stale TODO describes behavior that no longer exists.
## Task 6: Add CI Quality Gate
Goal: make local quality checks reproducible in CI.
Target files:
- `.woodpecker/check.yml`
- `DEVELOPMENT.md`
- `AGENT.md`
Steps:
- [x] Add CI steps for `cargo check --all-targets --all-features`.
- [x] Add CI steps for `cargo clippy --all-targets --all-features -- -D warnings`.
- [x] Add CI steps for `cargo test --all-features`.
- [x] Document the same commands in `DEVELOPMENT.md` or `AGENT.md`.
- [x] Keep CI commands aligned with the commands used by agents and humans locally.
Acceptance criteria:
- CI catches compile, lint, and test failures before merge.
- Local documentation and CI use the same command set.
## Global Acceptance Criteria
Before considering the refactor layer complete:
- [x] `cargo check --all-targets --all-features` passes.
- [x] `cargo clippy --all-targets --all-features -- -D warnings` passes.
- [x] `cargo test --all-features` passes.
- [x] `git diff --check` passes.
- [x] No unexpected `*.snap.new` files remain.
- [x] `rg -n "current_chat_messages_mut|chats_mut|folders_mut|pending_user_ids_mut|user_cache_mut" src/tdlib` shows only intentionally contained internal access.
- [x] `rg -n "unwrap\\(|expect\\(|panic!\\(" src` has no risky production UI or TDLib data-path panics left.
## Recommended Commit Order
1. Baseline commit for already completed facade/timezone/media/test cleanup.
2. `FakeTdClient` split.
3. TDLib internal mutation API cleanup.
4. Modal and message UI file splits.
5. Production unwrap cleanup.
6. TODO cleanup.
7. CI quality gate.