feat(cli): argument hint + --auto completion for /rename - #4048
Conversation
Closes QwenLM#4047. The /rename command supports a structured --auto flag (let the fast model generate a sentence-case title from the conversation), but unlike /model — which advertises --fast via argumentHint and a completion entry — /rename's flag was undocumented inline. Users had to either run the command incorrectly or check the docs to learn about --auto. - argumentHint: '[--auto] [<name>]' so the completion menu shows the shape when the user types `/rename` and tabs. - completion: returns null on empty / free-text input (don't shadow the user typing a title) and surfaces --auto when the partial arg is a prefix of it ('-', '--', '--a', '--au', '--auto'). Same shape as /model's --fast handling. Free-text titles intentionally don't auto-complete — there's nothing meaningful to suggest, and offering --auto on every keystroke would feel like noise on `/rename my-feature`. Tests: - pins argumentHint shape - empty partial → null - '-' / '--' / '--a' / '--au' / '--auto' all return the --auto suggestion - 'my-feature' / 'fix bug' / '-x' return null (free-text path) Co-Authored-By: Qwen-Coder <noreply@qwen.ai>
generateJson registers schemas as a respond_in_schema function
declaration and walks parts[].functionCall for the result. When no
tool_choice is set (the OpenAI-compatible converter never sets one) and
the system prompt explicitly asks for text JSON — e.g. session-title
generation's "Return ONLY a JSON object..." — some models honor the
prompt and emit the answer as a plain text part instead of calling the
tool. The answer is semantically correct; we just weren't reading it.
This bottoms out in /rename --auto as "The fast model returned no
usable title" on qwen3.6-max-preview, and likely affects every other
generateJson caller (next-speaker checker, edit corrector, etc.) on
the same class of model.
Add a tolerant fallback: when no function call comes back, parse
getResponseText(result) — which already skips thought parts — with a
JSON-object extractor that strips optional ```json fences and reads
the outermost {...} block. Strictly additive; the function-call path
stays primary.
Closes QwenLM#4057.
Co-Authored-By: Qwen-Coder <noreply@qwen.ai>
Bare /rename (no args) used to call a private generateKebabTitle path that asked the fast model (or main-model fallback) for a 2-4 word kebab-case name via a plain text call. /rename --auto used the schema-enforced tryGenerateSessionTitle path for a 3-7 word sentence- case title. Two code paths, two prompts, two failure-message formats, two sanitizers — with the kebab path consistently lagging on history filtering, surrogate handling, and error specificity. Collapse to a single fast-model schema-enforced pipeline. Both bare /rename and /rename --auto now call tryGenerateSessionTitle and both record titleSource: 'auto' on success. The --auto flag stays as an explicit user-intent marker (preserves the existing argumentHint / completion / parseArgs surface) but no longer diverges semantically. Bare /rename now also hard-requires fastModel; users who relied on the main-model fallback need to either /model --fast <name> or pass a name explicitly (/rename <name>). The new failure message points at both options. Co-Authored-By: Qwen-Coder <noreply@qwen.ai>
…ename-arg-completion
f462f1f to
747a2ed
Compare
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] packages/cli/src/ui/commands/renameCommand.ts titleFailureMessage — the model_error message says "rate limit, auth, or network error," but this reason is now also reachable from SchemaValidator.validate throwing after the new parseLooseJsonObject fallback in BaseLlmClient.generateJson extracts a wrong-shaped JSON object. The model did respond and the network was fine, but the user would be pointed in entirely the wrong direction. Consider either adding a distinct SessionTitleFailureReason for schema-validation failures in tryGenerateSessionTitle, or widening this message to include "…or unexpected response format."
Behavior change note for reviewers: Bare /rename (no args) now hard-requires a fast model via the unified tryGenerateSessionTitle pipeline. Previously it fell back to the main model via the removed kebab-case path. The error message correctly directs users to /model --fast <model> or /rename <name>.
✅ All 48 tests pass. tsc + eslint clean.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| * substring from the first `{` to the matching last `}` and JSON-parses it. | ||
| * Returns the parsed object on success, or `null` if nothing usable is found. | ||
| */ | ||
| function parseLooseJsonObject(text: string): Record<string, unknown> | null { |
There was a problem hiding this comment.
[Critical] parseLooseJsonObject has zero test coverage and no logging for silent degradation.
The new text-based JSON fallback in generateJson is completely untested. The existing test "should return empty object when no function calls are returned" passes because mock text is 'some text' (no {), so parseLooseJsonObject returns null and the method returns {}. The successful text-extraction path, markdown-fenced JSON, malformed JSON, and array rejection are never exercised.
Additionally, when function calling fails silently and this fallback kicks in, there is zero logging (debugLogger.warn/info). This makes production debugging impossible — the system degrades silently.
| function parseLooseJsonObject(text: string): Record<string, unknown> | null { | |
| // In baseLlmClient.test.ts, add tests covering: | |
| // 1. getFunctionCalls returns undefined, text contains valid JSON → parses successfully | |
| // 2. Text contains ```json``` fenced JSON → strips fence and parses | |
| // 3. Text contains JSON array → rejects array, returns {} | |
| // 4. Malformed JSON → returns {} | |
| // 5. No braces in text → returns {} | |
| // In baseLlmClient.ts, add logging on successful fallback: | |
| if (parsed) { | |
| debugLogger.info( | |
| 'generateJson: no respond_in_schema function call, extracted JSON from text fallback.' | |
| ); | |
| return parsed; | |
| } |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| */ | ||
| function autoFailureMessage(reason: SessionTitleFailureReason): string { | ||
| function titleFailureMessage(reason: SessionTitleFailureReason): string { |
There was a problem hiding this comment.
[Suggestion] titleFailureMessage is missing test coverage for 3 branches: empty_result, no_client, and default.
The switch statement handles all 6 SessionTitleFailureReason values plus a default fallback, but tests only cover 4 (no_fast_model, empty_history, aborted, model_error). The untested branches would surface empty or wrong error messages to users if triggered.
| function titleFailureMessage(reason: SessionTitleFailureReason): string { | |
| // Add tests in renameCommand.test.ts: | |
| // - tryGenerateSessionTitle resolves { ok: false, reason: 'empty_result' } | |
| // - tryGenerateSessionTitle resolves { ok: false, reason: 'no_client' } | |
| // - tryGenerateSessionTitle resolves { ok: false, reason: 'bogus_reason' } (default branch) |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| // auto-generated titles; explicit user text stays 'manual'. | ||
| let titleSource: 'auto' | 'manual' = 'manual'; | ||
| // auto-generated titles; only explicit user text stays 'manual'. | ||
| const titleSource: 'auto' | 'manual' = name ? 'manual' : 'auto'; |
There was a problem hiding this comment.
[Suggestion] titleSource semantic change: bare /rename (no args) now records titles as 'auto' instead of 'manual'.
Old code initialized let titleSource = 'manual' and only changed it in the --auto branch. New code uses const titleSource = name ? 'manual' : 'auto', so all LLM-generated titles (including bare /rename without --auto) are now 'auto'. This affects session picker dimming behavior — previously bare /rename titles were undimmed, now they appear dimmed.
The change is logically correct (LLM-generated = auto), but it is a silent behavioral change that will cause user confusion. Consider documenting this in the changelog, or introducing a third source value ('auto_explicit') to distinguish --auto from bare /rename.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| @@ -264,44 +192,10 @@ export const renameCommand: SlashCommand = { | |||
| return { | |||
| type: 'message', | |||
| messageType: 'error', | |||
| content: autoFailureMessage(outcome.reason), | |||
| content: titleFailureMessage(outcome.reason), | |||
| }; | |||
| } | |||
| name = outcome.title; | |||
There was a problem hiding this comment.
[Suggestion] titleSource correctness depends entirely on control-flow ordering — fragile to future refactoring.
const titleSource is computed at declaration time (line 146), before the LLM pipeline runs and reassigns name = outcome.title (line 198). Because titleSource is const, it correctly retains its initial classification. However, this relies on the declaration staying before the LLM block. A future refactor that moves the LLM block above the const titleSource line would silently invert the source attribution.
| name = outcome.title; | |
| // Add a defensive comment above const titleSource: | |
| // Must be computed BEFORE the LLM block below — 'name' is | |
| // reassigned inside the if (!name) block, but titleSource | |
| // must reflect the pre-LLM state (user input vs auto). | |
| const titleSource: 'auto' | 'manual' = name ? 'manual' : 'auto'; |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
|
Verified end-to-end on macOS against
Negative cases also pass: Spinner lifecycle is leak-free across success / One follow-up before merge (low effort)
Outside the test gap, no blockers. Approving from the verification side. Other observations, none blocking:
— @wenshao via Qwen Code |
3 participants
Summary
/renameUX overhaul that started as an argument-hint addition and grew to include a fix for a latent bug we hit while testing the new completion.What changed:
/rename(commit 1, original scope of this PR).argumentHint: '[--auto] [<name>]'shows the shape in the slash-command picker, and acompletionfunction surfaces--autowhen the user starts typing a flag (-,--,--a,--au,--auto). Free-text titles still don't auto-complete.BaseLlmClient.generateJson(commit 2). When the model returns a schema-shaped JSON object as a plaintextpart instead of via a function call (common on OpenAI-compatible providers withouttool_choice: 'required', e.g.qwen3.6-max-preview), the schema-enforced path now recovers the answer. Strictly additive; function-call path stays primary./renameand/rename --auto(commit 3). Both forms now share the schema-enforced sentence-casetryGenerateSessionTitlepipeline. The private kebab-case path (generateKebabTitle+extractConversationText) is removed.--autois retained as an explicit user-intent marker but no longer diverges semantically.Why it changed:
/rename's--autoflag was undocumented inline./rename --autofailed with “The fast model returned no usable title” on tool-capable models that prefer text over the function-call channel. Root cause + repro evidence in the issue.--autoUX divergence (kebab vs sentence case, soft vs hard fast-model requirement) was inconsistent.Behavioral changes worth flagging for reviewers:
/rename(no args) used to fall back to the main model when no fast model was configured. It now hard-requiresfastModel, matching/rename --auto. The failure message points users at both/model --fast <name>and/rename <name>(manual name) as resolutions./rename's output format changes from kebab-case (e.g.fix-login-bug) to sentence case (e.g.Fix login button on mobile)./renameare now recorded withtitleSource: 'auto', so the session picker dims them the same way it dims--autoresults. Only explicit/rename <name>staysmanual.Reviewer focus:
/model: returnnullon empty or non-prefix input, return the suggestion list when the partial argument is a prefix of--auto.parseLooseJsonObjecthelper inbaseLlmClient.ts— strips optional```jsonfences, takes the outermost{...}substring, accepts only plain objects. Worth a careful read for malformed-input handling.titleFailureMessageconsolidates the failure-reason → user-facing-string mapping that was previously split between two locations.Validation
fastModel=qwen3.6-max-preview):/rename→ session renamed to a sentence-case title./rename --auto→ regenerates to a (possibly identical) sentence-case title.fastModelcleared,/renameand/rename --autoboth surface “Auto-generating a title requires a fast model. Configure one with/model --fast <model>, or pass a name:/rename <name>.”Scope / Risk
/renameno longer works out-of-the-box for users without a configured fast model. The error message gives both resolutions; risk is low because the fix is one short command. Users who explicitly want to set a name (/rename my-title) are unaffected.t(...)machinery wraps them for translator pickup./renameoutput format changes (kebab → sentence) and now requiresfastModel. Documented above.Testing Matrix
Testing matrix notes:
qwen3.6-max-preview. Pure TypeScript changes with no platform-specific code paths.Linked Issues / Bugs