refactor: remove dead _with/extension wrappers - #137
Draft
Ariestar wants to merge 14 commits into
Draft
Conversation
…tection Two review findings on the theme feature: - provider_color returned fixed dark-palette RGB values for every agent, bypassing the active palette: light mode got low-contrast labels (notably Hermes yellow) and the ANSI fallback still emitted RGB sequences. Move provider colors into Theme as a per-palette ProviderPalette with dark, light, and ANSI variants. - light_background accepted every COLORFGBG background >= 8 as light, misclassifying dark 256-color indexes such as 16 or 232. Restrict the check to the documented bright ANSI range 8-15. Also surface config read/parse failures from tui::terminal::init instead of silently falling back to defaults, which made a typo like mode = "ligth" look ignored.
Add the focused [theme] serialization test the theme review asked for, matching the existing per-section config tests, and assert that a mode typo like "ligth" fails serde instead of silently falling back to auto.
Two follow-up review findings: - Without COLORTERM the detect() branch never consulted COLORFGBG, so a light-background terminal always got the same ANSI palette whose light foregrounds (Gray text, Hermes LightYellow, ...) wash out against a light default background. Select a distinct darker ANSI palette when the detected background is light, and verify it emits no RGB like the dark one. - ThemeConfig only had #[serde(default)], so a misspelled key such as [theme] mod = "light" was silently ignored and mode stayed auto — the same "setting looks ignored" failure the typo test guards against. Reject unknown theme keys during config loading and extend the test.
The rebased theme branch dropped the Color import from render.rs while the body-failure marker from the thread-names change still used a hardcoded RGB value. Paint the "[!]" marker with theme::failure() so every piece of TUI chrome follows the active palette.
the preference now decides light vs dark while truecolor support still decides RGB vs ANSI, so a forced mode on a terminal without COLORTERM no longer selects the RGB palette it cannot render
Replace the per-palette ProviderPalette structs (dark/light/ansi/ansi_light literals plus a color() match) with a single provider_colors table: one row per agent holding all four palette variants, selected by the active PaletteMode. Adding an agent now means adding one row, and the exhaustive match makes the compiler enforce completeness - so the palette can never drift out of sync with AgentProvider again (the merge-time breakage caused by main adding Gemini/Goose/QoderCn/Qwen).
Use system appearance detection and route ANSI and content styles through the active palette.
- loading polls no longer trigger appearance detection, so the 100 ms poll cannot hammer the desktop portal's blocking D-Bus call - latch dark_light::detect() failures to stop re-probing the portal - accept TERM -direct names for truecolor alongside COLORTERM - widen the config load error context beyond the theme section - drop redundant structure marker alternatives in markdown
dark_light::detect() fails routinely on headless Linux (no XDG desktop portal), where latching the failure left auto mode permanently dark and broke the polling test on CI. Skipping detection while loading already bounds the call rate, so the latch adds state without value.
Structure channels (<:tool:...:>, skills, thinking) previously shared the amber/blue marker color in read mode and were unstyled in raw mode, while read-mode fold summaries (thinking x7) matched the body foreground. Content now treats structure content uniformly in both modes: body text keeps the default foreground, structure markers (raw) and fold summaries (read) render in muted gray, and raw mode keeps literal markdown layout for tables and code fences. Drops the now-unused theme::structure_style.
- format_blocks / format_blocks_with_text / format_block_with_heading had zero production callers; tests now assert block fields directly - rank / rank_terms / ranked_ids / ranked_ids_with were test-only or uncalled; tests use a local rank_query helper over rank_terms_with - copy_parts_with_prompt merged into its only caller copy_parts - call_with_info dropped from the ipc call pyramid; DEFAULT_READ_TIMEOUT const replaces the hardcoded 30s in call and running() - search_with_scope merged into its only caller search
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Deploying sivtr with
|
| Latest commit: |
bd22943
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://13bc115f.sivtr.pages.dev |
| Branch Preview URL: | https://refactor-drop-wrapper-fns.sivtr.pages.dev |
- copy_text / copy_text_with_prompt merged into one 3-arg copy_text; all call sites pass the optional override explicitly - terminal_input_text: Some(_)/None override branches now fall back to the same prompt rendering when there is no command, so a present-but-unused override no longer drops ANSI coloring - session_title_with_id / content_selection_for_half inlined into their single callers - workspace_search_scope / workspace_search_regex_for_query deleted (single-caller projections)
Collaborator
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd22943e51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+526
to
529
| pub fn copy_text( | ||
| &self, | ||
| mode: RecordTextMode, | ||
| include_prompt: bool, |
There was a problem hiding this comment.
当 sivtr-core 作为独立 crates.io 包发布时（已检查 .github/workflows/release.yml 中的 publish-crates），这里把公开的 copy_text(mode, include_prompt) 改成必须传第三个参数，并同时删除 copy_text_with_prompt；任何按 0.4.x 兼容升级的下游调用都会编译失败。仓库规则明确 PATCH 仅用于向后兼容的变更，因此请保留原签名并另设带 override 的入口，而不要把仅在当前仓库内没有调用的公开方法视为死代码。
AGENTS.md reference: AGENTS.md:L123-L123
Useful? React with 👍 / 👎.
Ariestar
force-pushed
the
tui/theme-config
branch
2 times, most recently
from
August 16, 2026 05:25
e8789f5 to
6263509
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the base+_variant wrapper pairs flagged by ponytail-audit — the same redundancy class as the content _expanded wrappers.
Changes
ank\ /
ank_terms\ (test-only chain over
ank_terms_with) and
anked_ids\ /
anked_ids_with\ (zero callers, even in tests). Tests use a local
ank_query\ helper.
Validation
Net: 8 files, +46 / -131 lines. No behavior change; production entry points (rank_terms_with, copy_text_with_prompt, call_with_read_timeout, resolve_editor_with_config) kept.