refactor: technical-debt cleanup from first-principles review

[hakula139]

Summary

A perfectionist-review pass on the crate, executed as 23 atomic commits on a single branch. Findings came from fanning out five review agents (architecture, Rust idioms, style / conventions, tests, usability) across the codebase; fixes were split into this branch (immediate wins) and the deferred follow-ups listed below (items that belong in their own PR).

Zero behavior regressions on the happy path; four of the commits are real correctness fixes for paths tests did not previously exercise.

• Correctness bugs (4 commits). truncate_line gated on bytes when the cap is character-counted, so pure-multibyte input (500 é's) silently returned "... [N chars]" with an empty prefix — data loss; added a regression test. now_millis panicked on pre-epoch clock skew; now returns Option<u64> with expiry predicates defaulting to "refresh" and write_refreshed_credentials propagating a Result. truncate_title with max_len < 4 produced <first-char>... — guarded with debug_assert. tui::app::dispatch_user_action silently dropped UserActions when the channel was closed (agent task dead); now pushes an error block and tears down the TUI.
• Architecture refinement (1 commit). PendingCalls moved from tui:: to agent:: — it coordinates AgentEvent::ToolCallStart/End pairing, so it lives with the contract, not the first consumer. Unblocks StdioSink adopting the same result-header fallback.
• Client hardening (5 commits). Status-aware API error messages — 401 / 429 (with Retry-After) / 529 / generic 5xx now get actionable prefixes instead of one identical string for every failure, raw body preserved as details: {body}. ContextEdit enum-of-one collapsed to a tagged struct. model.to_lowercase() allocations replaced with eq_ignore_ascii_case splits in hot paths. System-section assembly list promoted from a numbered // comment to /// rustdoc. Dead StreamEvent::Unknown match arm removed from a matches! assertion.
• Tool module cleanup (3 commits). bytes_to_mb extracted — three duplicate #[expect(clippy::cast_precision_loss)] suppressions collapse to one. normalize_eol returns Cow<'_, str>, skipping a String allocation on pure-LF input (the common case). tool/bash test bounds tightened — the TRUNCATION_OVERHEAD slop became a named constant the assertion actually references; multibyte-boundary tests now assert the emoji is absent from the truncated head and tail, catching a partial-byte leak the previous starts_with("aaaa") / ends_with('b') pattern missed.
• Style sweeps (3 commits). pub → pub(crate) across the binary-only crate (27 items across config, client, message). Mechanical test rename: *_returns_none, *_returns_err, *_returns_owned_value → scenario form (*_is_absent, *_errors, *_reads_value, etc.) across 15 files. Missing blank lines before section dividers fixed in 8 test modules.
• Test quality (3 commits). tui::wrap had three tests asserting only result.len() >= 2 — replaced with exact-count + per-line content assertions that pin where the word-boundary breaks land. tui::components::status had five contains() substring tests duplicating existing insta snapshots at weaker strength — duplicates dropped, snapshots retained. session::writer substring match on raw JSONL replaced with an Entry::Message parse + exact ContentBlock::Text comparison.
• Docs + infra (4 commits). CLAUDE.md crate tree refreshed (adds agent/pending_calls.rs) and four ambiguous rules clarified: const-group blank-line exception, super:: / crate:: grouped together in imports, top-down ordering is within-section, scenario-vs-mechanism test naming gets a synonym-case clause. Added a PR-convention rule: descriptions must stand on their own and not point at gitignored working docs. .gitignore excludes .claude/worktrees/.

Out of scope

Follow-up PRs for items that either need their own scope or are speculative until a concrete consumer lands:

• Tool-input JSON parse-error surfacing (agent.rs:205). Clean fix plumbs parse errors through stream_response's return type — signature change with cascading test updates. The current behavior (empty input → tool schema error → model self-corrects) works; the concern is diagnostic quality. ~40 LOC follow-up.
• HashSet<&str> in sanitize_resumed_messages. Borrow conflicts with the mutable-iteration path in the current &mut Vec<Message> signature; best tackled after the sanitize-passes extraction splits the function.
• SessionManager std::sync::Mutex swap. record_message is async because of file I/O; the fix is a sync / spawn_blocking restructure, aligned with the write-batching track in docs/research/session-persistence.md.
• client/anthropic.rs split (2761 LOC → wire / sse / betas / completion). Do before MCP client lands so MCP streaming doesn't force a second 3000-line file.
• session/chain extraction + sanitize-passes split. resolve_chain (70% graph walking) moves to its own module; sanitize_resumed_messages decomposes into four testable passes. Groundwork for context compression.
• Per-tool ToolResultView renderer split. Before the next two variants (grep matches, glob list) land.
• AgentEvent::PromptRequest and widened tool content. Speculative until the first interactive feature / MCP result has a concrete shape to design around — "no speculative code" applies.

Changes

Area	Files	Notes
Correctness	tool.rs, config/oauth.rs, session/manager.rs, tui/app.rs	truncate_line char-cap fix + regression test; now_millis → Option<u64>; truncate_title precondition; dispatch_user_action handles Closed
Architecture	agent/pending_calls.rs (moved from tui/), tui.rs, agent.rs, tui/app.rs, tui/components/chat.rs, agent/event.rs	PendingCalls lives with its contract
HTTP client	client/anthropic.rs	Status-aware errors + Retry-After; ContextEdit struct; eq_ignore_ascii_case for family checks; promoted rustdoc; dead Unknown arm dropped; visibility narrowed
Tool modules	tool.rs, tool/{read,edit,grep,bash}.rs	bytes_to_mb helper; normalize_eol returns Cow; TRUNCATION_OVERHEAD promoted; multibyte tests tightened
Session	session/{manager,writer,history,list_view,store}.rs	Title precondition; writer test parses Entry::Message; test renames
Style sweep	config.rs, config/{file,oauth}.rs, client.rs, client/anthropic.rs, message.rs	pub → pub(crate)
Test renames	util/{env,lock,path}.rs, config/{file,oauth}.rs, prompt*.rs, model.rs, session/{history,list_view,store}.rs, tui/markdown/{highlight,render}.rs, tui/components/{chat,input}.rs, agent/pending_calls.rs	Scenario form instead of mechanism form
Test tightening	tui/wrap.rs, tui/components/status.rs	Exact-count assertions; substring-contains duplicates removed
Blank lines	agent.rs, tool/{write,read,grep,glob}.rs, config/oauth.rs, session/list_view.rs, tui/components/input.rs	One blank before first section divider in test modules
Docs	CLAUDE.md	Crate tree + four clarified rules + PR-description rule
Infra	.gitignore	.claude/worktrees/ excluded

Test plan

• cargo fmt --all --check
• cargo build
• cargo clippy --all-targets -- -D warnings — zero warnings
• cargo test — 936 pass (five duplicate status-bar contains() tests dropped vs the 941 before this branch)
• pnpm run lint, pnpm run spellcheck — zero issues

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!