From 6ff37e346a53623f9bd9727d3e39aa9d3038369e Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 23 Jun 2026 01:46:36 +0200 Subject: [PATCH] feat(coding-agent): extended --thinking CLI flag options - Added `off` and `auto` as valid inputs for the `--thinking` CLI flag. - Centralized thinking level definitions in `CLI_THINKING_LEVELS` to keep flag options, shell completions, and validation in sync. - Configured CLI parsing to reject `inherit` as an explicit input to prevent unintended configuration suppression. --- packages/coding-agent/src/cli/args.ts | 9 ++++----- packages/coding-agent/src/cli/flag-tables.ts | 6 +++--- packages/coding-agent/src/commands/launch.ts | 6 +++--- packages/coding-agent/src/thinking.ts | 20 +++++++++++++++++++ .../test/auto-thinking-classifier.test.ts | 9 +++++++++ .../test/cli-hide-thinking-flag.test.ts | 20 +++++++++++++++++++ .../coding-agent/test/cli/completions.test.ts | 2 +- 7 files changed, 60 insertions(+), 12 deletions(-) diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index 488826561..364bb94ac 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -1,10 +1,9 @@ /** * CLI argument parsing and help display */ -import { type Effort, THINKING_EFFORTS } from "@oh-my-pi/pi-catalog/effort"; import { APP_NAME, CONFIG_DIR_NAME, logger } from "@oh-my-pi/pi-utils"; import chalk from "chalk"; -import { parseEffort } from "../thinking"; +import { CLI_THINKING_LEVELS, type ConfiguredThinkingLevel, parseCliThinkingLevel } from "../thinking"; import { BUILTIN_TOOL_NAMES } from "../tools/builtin-names"; import { OPTIONAL_FLAGS, @@ -32,7 +31,7 @@ export interface Args { apiKey?: string; systemPrompt?: string; appendSystemPrompt?: string; - thinking?: Effort; + thinking?: ConfiguredThinkingLevel; hideThinking?: boolean; advisor?: boolean; continue?: boolean; @@ -89,9 +88,9 @@ export interface Args { */ const PARSE_DEPS: ParseDeps = { logger, - parseEffort, + parseThinking: parseCliThinkingLevel, builtinToolNames: BUILTIN_TOOL_NAMES, - thinkingEfforts: THINKING_EFFORTS, + thinkingEfforts: CLI_THINKING_LEVELS, }; export function parseArgs(inputArgs: string[], extensionFlags?: Map): Args { diff --git a/packages/coding-agent/src/cli/flag-tables.ts b/packages/coding-agent/src/cli/flag-tables.ts index 63dcb13ec..fd315145e 100644 --- a/packages/coding-agent/src/cli/flag-tables.ts +++ b/packages/coding-agent/src/cli/flag-tables.ts @@ -30,7 +30,7 @@ * real implementations at the dispatch site. */ -import type { Effort } from "@oh-my-pi/pi-ai"; +import type { ConfiguredThinkingLevel } from "../thinking"; import type { Args } from "./args"; /** @@ -44,7 +44,7 @@ import type { Args } from "./args"; */ export interface ParseDeps { logger: { warn: (message: string, meta?: Record) => void }; - parseEffort: (value: string | null | undefined) => Effort | undefined; + parseThinking: (value: string | null | undefined) => ConfiguredThinkingLevel | undefined; builtinToolNames: readonly string[]; thinkingEfforts: readonly string[]; } @@ -165,7 +165,7 @@ export const STRING_SETTERS: Record = { result.tools = valid; }, "--thinking": (result, value, deps) => { - const thinking = deps.parseEffort(value); + const thinking = deps.parseThinking(value); if (thinking !== undefined) { result.thinking = thinking; } else { diff --git a/packages/coding-agent/src/commands/launch.ts b/packages/coding-agent/src/commands/launch.ts index d0a623bf1..5c559d6ea 100644 --- a/packages/coding-agent/src/commands/launch.ts +++ b/packages/coding-agent/src/commands/launch.ts @@ -2,12 +2,12 @@ * Root command for the coding agent CLI. */ -import { THINKING_EFFORTS } from "@oh-my-pi/pi-catalog/effort"; import { APP_NAME } from "@oh-my-pi/pi-utils"; import { Args, Command, Flags } from "@oh-my-pi/pi-utils/cli"; import { parseArgs } from "../cli/args"; import { runRootCommand } from "../main"; import { prepareAcpTerminalAuthArgs } from "../modes/acp/terminal-auth"; +import { CLI_THINKING_LEVELS } from "../thinking"; export default class Index extends Command { static description = "AI coding assistant"; @@ -100,8 +100,8 @@ export default class Index extends Command { description: "Comma-separated list of tools to enable (default: all)", }), thinking: Flags.string({ - description: `Set thinking level: ${THINKING_EFFORTS.join(", ")}`, - options: [...THINKING_EFFORTS], + description: `Set thinking level: ${CLI_THINKING_LEVELS.join(", ")}`, + options: [...CLI_THINKING_LEVELS], }), "hide-thinking": Flags.boolean({ description: "Hide thinking blocks in TUI output (display only, does not disable model thinking)", diff --git a/packages/coding-agent/src/thinking.ts b/packages/coding-agent/src/thinking.ts index 261e61be0..2e17f7fa5 100644 --- a/packages/coding-agent/src/thinking.ts +++ b/packages/coding-agent/src/thinking.ts @@ -153,6 +153,26 @@ export function getConfiguredThinkingLevelMetadata(level: ConfiguredThinkingLeve return level === AUTO_THINKING ? AUTO_THINKING_METADATA : getThinkingLevelMetadata(level); } +/** + * Thinking selectors accepted by the `--thinking` CLI flag, in display order: + * `off`, every concrete effort (`minimal`..`xhigh`), then `auto`. Single source + * for the flag's `options` list, shell completions, and the "invalid level" + * warning so all three stay in sync. + */ +export const CLI_THINKING_LEVELS: readonly string[] = [ThinkingLevel.Off, ...THINKING_EFFORTS, AUTO_THINKING]; + +/** + * Parses a `--thinking` CLI value. Accepts every {@link parseConfiguredThinkingLevel} + * selector (`off`, `auto`, `minimal`..`xhigh`, plus the `max` alias) but rejects + * `inherit`: an explicit `inherit` on the command line would suppress the + * settings/scoped-model fallback during startup resolution only to resolve back + * to the provider default, which is never what the user means. + */ +export function parseCliThinkingLevel(value: string | null | undefined): ConfiguredThinkingLevel | undefined { + const level = parseConfiguredThinkingLevel(value); + return level === ThinkingLevel.Inherit ? undefined : level; +} + /** * Resolves an auto-classified effort against the active model's supported * range. Unlike {@link clampThinkingLevelForModel}, `auto` never resolves below diff --git a/packages/coding-agent/test/auto-thinking-classifier.test.ts b/packages/coding-agent/test/auto-thinking-classifier.test.ts index 1e8141eba..a51e70d22 100644 --- a/packages/coding-agent/test/auto-thinking-classifier.test.ts +++ b/packages/coding-agent/test/auto-thinking-classifier.test.ts @@ -14,6 +14,7 @@ import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { AUTO_THINKING, clampAutoThinkingEffort, + parseCliThinkingLevel, parseConfiguredThinkingLevel, parseEffort, parseThinkingLevel, @@ -65,6 +66,14 @@ describe("auto thinking classifier helpers", () => { expect(parseThinkingLevel(ThinkingLevel.Off)).toBe(ThinkingLevel.Off); }); + it("parses CLI --thinking selectors while rejecting inherit", () => { + expect(parseCliThinkingLevel(ThinkingLevel.Off)).toBe(ThinkingLevel.Off); + expect(parseCliThinkingLevel(AUTO_THINKING)).toBe(AUTO_THINKING); + expect(parseCliThinkingLevel("max")).toBe(ThinkingLevel.XHigh); + expect(parseCliThinkingLevel(ThinkingLevel.Inherit)).toBeUndefined(); + expect(parseCliThinkingLevel("bogus")).toBeUndefined(); + }); + it("maps online 4-way classifier labels to effort levels", () => { expect(parseDifficultyLevel("x-high")).toBe(Effort.XHigh); expect(parseDifficultyLevel("The answer is HIGH.")).toBe(Effort.High); diff --git a/packages/coding-agent/test/cli-hide-thinking-flag.test.ts b/packages/coding-agent/test/cli-hide-thinking-flag.test.ts index a58634a1c..40c4d548b 100644 --- a/packages/coding-agent/test/cli-hide-thinking-flag.test.ts +++ b/packages/coding-agent/test/cli-hide-thinking-flag.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from "bun:test"; +import { ThinkingLevel } from "@oh-my-pi/pi-agent-core"; import { Effort } from "@oh-my-pi/pi-ai"; import { parseArgs } from "@oh-my-pi/pi-coding-agent/cli/args"; +import { AUTO_THINKING } from "@oh-my-pi/pi-coding-agent/thinking"; describe("parseArgs — --hide-thinking flag", () => { it("parses --hide-thinking as a boolean flag", () => { @@ -43,3 +45,21 @@ describe("parseArgs — --hide-thinking flag", () => { expect(result.messages).toEqual([]); }); }); + +describe("parseArgs — --thinking flag", () => { + it("accepts off so reasoning can be disabled from the CLI", () => { + expect(parseArgs(["--thinking", "off"]).thinking).toBe(ThinkingLevel.Off); + expect(parseArgs(["--thinking=off"]).thinking).toBe(ThinkingLevel.Off); + }); + + it("accepts auto, concrete efforts, and the max alias", () => { + expect(parseArgs(["--thinking", "auto"]).thinking).toBe(AUTO_THINKING); + expect(parseArgs(["--thinking", "medium"]).thinking).toBe(Effort.Medium); + expect(parseArgs(["--thinking", "max"]).thinking).toBe(ThinkingLevel.XHigh); + }); + + it("ignores invalid levels and the internal inherit selector", () => { + expect(parseArgs(["--thinking", "bogus"]).thinking).toBeUndefined(); + expect(parseArgs(["--thinking", "inherit"]).thinking).toBeUndefined(); + }); +}); diff --git a/packages/coding-agent/test/cli/completions.test.ts b/packages/coding-agent/test/cli/completions.test.ts index 2dcf3a17c..4da7f19ef 100644 --- a/packages/coding-agent/test/cli/completions.test.ts +++ b/packages/coding-agent/test/cli/completions.test.ts @@ -211,7 +211,7 @@ describe("omp completions (integration / drift)", () => { } expect(stdout).toContain("{-r,--resume}"); // Real enum option sets flow through unchanged. - expect(stdout).toContain(":value:(minimal low medium high xhigh)"); + expect(stdout).toContain(":value:(off minimal low medium high xhigh auto)"); expect(stdout).toContain(":value:(always-ask write yolo)"); // Real subcommands present; dynamic callbacks wired. expect(stdout).toContain("_omp_cmd_commit");