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.
This commit is contained in:
@@ -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<string, { type: "boolean" | "string" }>): Args {
|
||||
|
||||
@@ -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<string, unknown>) => 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<string, StringSetter> = {
|
||||
result.tools = valid;
|
||||
},
|
||||
"--thinking": (result, value, deps) => {
|
||||
const thinking = deps.parseEffort(value);
|
||||
const thinking = deps.parseThinking(value);
|
||||
if (thinking !== undefined) {
|
||||
result.thinking = thinking;
|
||||
} else {
|
||||
|
||||
@@ -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)",
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user