Revert "feat: add --sessions/--list-sessions CLI options & fix CJK shorten"#1608
Revert "feat: add --sessions/--list-sessions CLI options & fix CJK shorten"#1608
Conversation
…orten (#…" This reverts commit 8654a09.
![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: f0b9ec4b0f
ℹ️ 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".
| user_text = user_message.strip() | ||
| user_text = " ".join(user_text.split()) | ||
| fallback_title = shorten(user_text, width=50) or "Untitled" | ||
| fallback_title = shorten(user_text, width=50, placeholder="...") or "Untitled" |
There was a problem hiding this comment.
Using textwrap.shorten here regresses title generation for inputs without whitespace (common in Chinese/Japanese): for long strings it returns only the placeholder ("...") instead of a readable prefix. In generate_session_title, that placeholder can be persisted as the final title after retry exhaustion, causing many sessions to collapse to identical names and become hard to distinguish in the session list.
Useful? React with 👍 / 👎.
| val = args.get(key) | ||
| if isinstance(val, str) and val.strip(): | ||
| return shorten(val, width=60) | ||
| return shorten(val, width=60, placeholder="…") |
There was a problem hiding this comment.
This now relies on textwrap.shorten, which truncates on word boundaries only; when the argument is a long no-space string (e.g., CJK text, long tokenized commands/queries), the hint degrades to just …. That removes the identifying context from exported tool-call headings, making transcript exports much less diagnosable for these inputs.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| import uuid | ||
| from dataclasses import dataclass | ||
| from pathlib import Path | ||
| from textwrap import shorten |
There was a problem hiding this comment.
🔴 Replacing custom shorten with textwrap.shorten breaks CJK text truncation in session titles
The custom shorten function (removed from src/kimi_cli/utils/string.py) was specifically designed to fall back to a hard character cut for CJK text without word boundaries (spaces). Python's textwrap.shorten only breaks at whitespace, so when CJK text exceeds the width and contains no ASCII spaces, it returns only the placeholder instead of truncated content.
Demonstration of the regression
from textwrap import shorten
text = '请帮我分析一下这段代码中存在的所有潜在安全漏洞并且给出详细的修复方案和最佳实践建议我需要确保生产环境的安全性' # 54 chars
shorten(text, width=50) # => '[...]' (just the placeholder!)
shorten(text, width=50, placeholder='...') # => '...'The old custom function would return the first 49 characters + "…" placeholder.
At src/kimi_cli/session.py:101-104, shorten(text, width=50) uses the default ' [...]' placeholder (6 chars). For a CJK user prompt like "请帮我分析这段代码的安全问题并给出修复方案和最佳实践建议确保上线安全" (>50 chars, no spaces), the session title becomes ' [...] (session-id)' — conveying zero information about the session content. The same regression affects src/kimi_cli/web/store/sessions.py:149, src/kimi_cli/web/api/sessions.py:959, src/kimi_cli/web/api/sessions.py:1021, src/kimi_cli/utils/export.py:69, and src/kimi_cli/utils/export.py:234.
Prompt for agents
The custom `shorten` function was removed from `src/kimi_cli/utils/string.py` and all call sites were migrated to `textwrap.shorten`. However, `textwrap.shorten` does not handle CJK text correctly — it only breaks at whitespace, so for CJK text without ASCII spaces, it returns just the placeholder instead of truncated content.
Restore the custom `shorten` function in `src/kimi_cli/utils/string.py` and revert the import changes in these files:
1. `src/kimi_cli/session.py:10` — change `from textwrap import shorten` back to `from kimi_cli.utils.string import shorten`
2. `src/kimi_cli/utils/export.py:7` — change `from textwrap import shorten` back to `from kimi_cli.utils.string import shorten`
3. `src/kimi_cli/web/store/sessions.py:132` — change `from textwrap import shorten` back to `from kimi_cli.utils.string import shorten`
4. `src/kimi_cli/web/api/sessions.py:955` — change `from textwrap import shorten` back to `from kimi_cli.utils.string import shorten`
Also restore the CHANGELOG entries about the custom `shorten` function in CHANGELOG.md, docs/en/release-notes/changelog.md, and docs/zh/release-notes/changelog.md.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pull request overview
This PR reverts the previously introduced CLI session selection/listing options and the custom CJK-friendly shorten helper, restoring earlier behavior and updating docs/changelogs accordingly.
Changes:
- Remove
--pick-session/--list-sessionsCLI options and their associated e2e tests. - Remove
kimi_cli.utils.string.shortenand switch call sites back totextwrap.shorten. - Update changelogs and CLI reference docs (EN/ZH) to reflect the reverted functionality.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/test_cli_error_output.py | Removes e2e coverage for reverted CLI flags/conflicts. |
| src/kimi_cli/cli/init.py | Removes CLI option wiring/validation for session picking/listing. |
| src/kimi_cli/utils/string.py | Deletes the custom CJK-safe shorten helper. |
| src/kimi_cli/session.py | Switches session title derivation to textwrap.shorten. |
| src/kimi_cli/utils/export.py | Switches export truncation to textwrap.shorten with explicit placeholder. |
| src/kimi_cli/web/api/sessions.py | Switches web title generation fallback truncation to textwrap.shorten. |
| src/kimi_cli/web/store/sessions.py | Switches wire-derived title truncation to textwrap.shorten. |
| docs/en/release-notes/changelog.md | Removes release note entries for reverted features. |
| docs/zh/release-notes/changelog.md | Removes release note entries for reverted features. |
| docs/en/reference/kimi-command.md | Removes CLI reference for reverted flags and updates mutual-exclusion note. |
| docs/zh/reference/kimi-command.md | Removes CLI reference for reverted flags and updates mutual-exclusion note. |
| CHANGELOG.md | Removes release note entries for reverted features. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import json | ||
| from textwrap import shorten | ||
|
|
There was a problem hiding this comment.
Swapping to textwrap.shorten changes truncation semantics: default placeholder becomes " [...]", and for long single-token inputs (e.g., CJK text without spaces) it can return only the placeholder. Since this function later uses shorten(text, width=300), consider passing an explicit placeholder (and adding a hard-cut fallback if you need to avoid placeholder-only titles).
| from pathlib import Path | ||
| from textwrap import shorten | ||
|
|
There was a problem hiding this comment.
Now that Session.refresh() uses textwrap.shorten, note that textwrap.shorten(..., width=50) defaults to the placeholder " [...]" and may return only the placeholder for long single-token inputs (common with CJK text). That can yield titles like " [...] (<id>)" and differs from other truncation call sites that use an ellipsis placeholder explicitly; consider passing an explicit placeholder (and a hard-cut fallback if you want non-empty content for no-space strings).
| # Fallback title from user message (used if AI generation fails) | ||
| from textwrap import shorten | ||
|
|
||
| user_text = user_message.strip() | ||
| user_text = " ".join(user_text.split()) | ||
| fallback_title = shorten(user_text, width=50) or "Untitled" | ||
| fallback_title = shorten(user_text, width=50, placeholder="...") or "Untitled" |
There was a problem hiding this comment.
The truncation placeholder is set to "..." here, while other truncation in the codebase (e.g., export helpers) uses the single-character ellipsis "…". Consider standardizing on one placeholder (possibly via a shared constant) to keep UI/export output consistent and avoid snapshot churn.
Reverts #1376