From 1bdebbdb58c8474c08a4595d466f680f308b14e2 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Fri, 18 Sep 2026 18:12:57 +0800 Subject: [PATCH 01/19] feat(acli): make rule 18 demand cross-reference resolution on transformed output Mirrors agenticCLI a7b1a84. Rule 18 only required verifying code changes, so an agent producing a data document could pass every check it ran and still ship a broken one: Terminal-Bench hard-066c had organization-json-generator write employee *names* into project.members where the grader wants employee IDs, and report the conversion as deliberate -- "member IDs resolved to employee names, per schema's members: string[]". jsonschema agreed with it, because the schema only constrains members to an array of strings, so 3 of 4 tests passed and test_relationships_integrity failed on 'Cristiano Ronaldo' not in {'D001-E001', 'D001-E002', 'D001-E003'}. Names the case in the rule: when output transforms or joins data (ETL, CSV/JSON, migrations), also assert every cross-reference resolves, since schema-valid output with dangling IDs still fails. Measured on Terminal-Bench hard (10 tasks, qwen3.8-max, serial; wheel ae86d994 whose only content delta from the before-wheel ab5c911a is this clause): the task went 3p/1f -> 4p/0f and wrote an explicit dangling-reference check. Batch score unchanged at 6/10 -- two out-of-scope tasks flipped negative and only 4 of the 6 passes are shared between runs, so single-run task-level variance on hard is about +/-2. Full attribution in agenticCLI bench/terminal-bench/RESULTS.md, runs hard-066c / hard-066d. --- dashscope/acli/agent.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dashscope/acli/agent.py b/dashscope/acli/agent.py index e764561..86e025f 100644 --- a/dashscope/acli/agent.py +++ b/dashscope/acli/agent.py @@ -154,7 +154,10 @@ def _budget_burned_slowly(error: BaseException) -> bool: 18. **Verify code changes before reporting done.** Run the tests that cover what you touched; if nothing covers it, add a focused test for the new behaviour and run that. Report the command and its pass/fail result — - never claim a change works without having executed it + never claim a change works without having executed it. When the output + transforms or joins data (ETL, CSV/JSON, migrations), also assert every + cross-reference resolves — schema-valid output with dangling IDs still + fails Reply style: - **Concise**. No filler like "let me see / let me help you / I'll analyze From 600827072f7ab11bb3e7bfb46e8b0eccf64e7d27 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Sun, 20 Sep 2026 17:22:27 +0800 Subject: [PATCH 02/19] fix(cli): route the auth group to typer instead of the expert agent c4e38e7 registered app.add_typer(auth.app) but never added "auth" to _TOP_LEVEL_COMMANDS, so main()'s unknown-command branch forwarded the line to the SDK Expert agent: `dashscope auth whoami` opened a chat session instead of verifying the key, and login/logout were unreachable. `auth --help` kept working, which hid the gap -- the help branch dispatches before routing. Also document auth in the expert's cli.md skill. It claims to be authoritative for CLI usage but had no auth row, so the agent could not answer either. The new test asserts every registered typer group is routable. One-directional on purpose: rl/agentic-rl stay listed when the optional extra is absent. --- .../dashscope-sdk-expert/.acli/skills/cli.md | 12 ++++++-- dashscope/cli/__init__.py | 1 + tests/unit/test_cli_agent_route.py | 29 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md index 5979534..c9ed272 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md @@ -11,6 +11,7 @@ This file is authoritative for answering `dashscope` CLI usage questions (based | Command group | Subcommands | Purpose | Corresponding SDK class | | --- | --- | --- | --- | +| auth | whoami/login/logout | Verify the active API key and persist one to ~/.dashscope/api_key | dashscope.common.api_key (get_default_api_key/save_api_key) | | generation | create | Text generation (including streaming and multi-turn messages) | dashscope.Generation | | ft (hidden alias fine-tunes) | create/get/list/stream/cancel/delete | Fine-tuning job management | dashscope.FineTunes | | files | upload/get/list/delete | Training file upload and management | dashscope.Files | @@ -34,9 +35,12 @@ Note: code-generation and understanding still exist in the source but are deprec ## Common usage -Prerequisite: `export DASHSCOPE_API_KEY=sk-xxx` (or pass the global `-k/--api-key`). +Prerequisite: `export DASHSCOPE_API_KEY=sk-xxx` (or pass the global `-k/--api-key`, or persist one with `dashscope auth login`). ```bash +dashscope auth whoami # verify the active key against the API and show its source +dashscope auth login --key sk-xxx # save a key to ~/.dashscope/api_key +dashscope auth logout # remove the saved key file dashscope generation create -m qwen-plus -p "Hello" -s --temperature 0.7 dashscope ft create -m qwen-turbo -t file-xxx -v file-yyy -e 2 -b 16 -l 1e-5 dashscope files upload -f train.jsonl -p fine-tune -d "training set" @@ -59,6 +63,9 @@ dashscope rl run -c rl_config.yaml -o json -v ``` Key option notes: +- auth whoami: takes no options; validates the resolved key with a lightweight `dashscope.Models.list(page=1, page_size=1)` call and prints `Authenticated key=sk-xxx...yyyy source=...`. The key is always masked (first 6 + last 4), never printed in full. `source` reports where it came from: `environment / --api-key flag`, `file ()`, or `unknown`. Exit codes: 0 = present and accepted, 1 = no key configured, 2 = configured but rejected (401/403) or an unexpected response. +- auth login: `-k/--key` saves to `~/.dashscope/api_key`. Omitted, it prompts interactively with hidden input — that prompt blocks forever in a non-TTY (piped stdin, or a shell tool driven by an agent), so pass `--key` explicitly there. An empty/whitespace key exits 1. Note that `--key sk-xxx` lands in shell history and the process arg list; prefer the interactive prompt when a human is at a terminal. +- auth logout: takes no options; removes `~/.dashscope/api_key` if present, otherwise prints "nothing to remove" and exits 0. It does not unset `DASHSCOPE_API_KEY`, so a key exported in the environment still authenticates afterwards. - generation create: `-p/--prompt`, `--messages` (JSON string), `-s/--stream`, `--temperature/--top-p/--top-k/--max-tokens/--seed/--stop/--repetition-penalty/--presence-penalty/--enable-search/--n/--result-format` - ft create: `-t/--training-file-ids` (repeatable), `-v/--validation-file-ids`, `--mode`, `-e/--n-epochs`, `-b/--batch-size`, `-l/--learning-rate`, `-p/--prompt-loss`; get/stream/cancel/delete all take job_id as a positional argument; list uses `-p/--page`, `-s/--size` - files: upload `-f/--file`, `-p/--purpose` (default fine-tune), `-d/--description`; all commands support `-u/--base-url` @@ -79,7 +86,8 @@ Key option notes: ## Common errors and troubleshooting -- `Error: ... AuthenticationError` (exit code 1): no Key configured; set DASHSCOPE_API_KEY or add `-k`. +- `Error: ... AuthenticationError` (exit code 1): no Key configured; set DASHSCOPE_API_KEY, add `-k`, or persist one with `dashscope auth login`. Diagnose with `dashscope auth whoami` — it reports both validity and which source the key came from. +- `dashscope auth whoami` opens the SDK Expert agent instead of printing key info: `auth` was missing from the CLI's top-level routing table, so the line was treated as an unknown command and forwarded to the agent as a question. Fixed in this repo; if a user hits it, they are on an older install and should upgrade rather than keep retrying. - `Option '--api-key' requires an argument.` (exit code 2): `-k/--api-key` is missing a value or the value starts with `-`. - `--messages must be a valid JSON string`: generation's --messages must be a JSON array string; watch shell quoting/escaping. - `File ... does not exist`: local paths (-f, --image, --audio, etc.) are validated locally first; `~` expansion is supported; URLs must include a scheme. diff --git a/dashscope/cli/__init__.py b/dashscope/cli/__init__.py index 0da6246..2c6c12a 100644 --- a/dashscope/cli/__init__.py +++ b/dashscope/cli/__init__.py @@ -91,6 +91,7 @@ } _TOP_LEVEL_COMMANDS = { + "auth", "generation", "ft", "fine-tunes", diff --git a/tests/unit/test_cli_agent_route.py b/tests/unit/test_cli_agent_route.py index f662fba..8f59c23 100644 --- a/tests/unit/test_cli_agent_route.py +++ b/tests/unit/test_cli_agent_route.py @@ -303,6 +303,35 @@ def test_expert_is_no_longer_a_command(self, monkeypatch): assert routed == ["expert chat"] + def test_registered_groups_are_all_routable(self): + """A group registered with typer but absent from _TOP_LEVEL_COMMANDS + never executes — main() forwards the line to the agent instead. + + Only this direction is asserted: `rl`/`agentic-rl` stay in the set even + when the optional extra is not installed. + """ + registered = { + g.name or g.typer_instance.info.name + for g in cli.app.registered_groups + } + assert registered <= cli._TOP_LEVEL_COMMANDS + + def test_auth_whoami_executes_instead_of_routing_to_agent(self, monkeypatch): + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=False: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr(sys, "argv", ["dashscope", "auth", "whoami"]) + + cli.main() + + assert routed == [] + assert invoked == [True] + class TestBundledExample: def test_expert_example_bundled(self): From afe9141505a6fde87aca03f31e18f215ea0662ca Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Sun, 20 Sep 2026 18:02:09 +0800 Subject: [PATCH 03/19] fix(cli): stop forwarding malformed commands into the expert agent main() sent any line whose first token was not in _TOP_LEVEL_COMMANDS to the agent as a question. That catch-all swallowed real errors: `dashscope -w wsid generation create` opened a chat instead of reporting "No such option: -w", and `dashscope generaton create` hid a typo that typer would have answered with "Did you mean 'generation'". It is also why the missing auth entry fixed in 6008270 silently disabled a working command rather than erroring. Three paths reach the agent now: a bare `dashscope`, an explicit `dashscope expert ...`, and a single unknown token -- the last one keeps the documented `dashscope "question"` one-shot working. Everything else goes to typer. `expert` is a routing keyword, not a typer group, and stays out of _TOP_LEVEL_COMMANDS; that set now only bounds global -k/--api-key parsing. This reverses the routing half of b808fcd -- the typer subcommand and the ~/.acli config sync it deleted stay deleted. README, README_zh and the expert's cli.md skill all advertised the removed catch-all, so their wording is updated to match. --- README.md | 2 +- README_zh.md | 2 +- .../dashscope-sdk-expert/.acli/skills/cli.md | 4 +- dashscope/cli/__init__.py | 78 +++++++--- tests/unit/test_cli_agent_route.py | 141 ++++++++++++++++-- 5 files changed, 195 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index f00a250..10db33d 100644 --- a/README.md +++ b/README.md @@ -175,7 +175,7 @@ A complete runnable example is available in [`samples/set_region_example.py`](sa The SDK ships with an interactive AI assistant, **DashScope SDK Expert**, built on the bundled Agentic CLI (`dashscope/acli`) framework. For DashScope SDK/CLI users it is the recommended way to get development consultation and AI coding help — answering SDK/API questions, generating runnable examples, showing CLI usage, and diagnosing errors, right in your terminal. - Run `dashscope` with no arguments to start the assistant. On first run it offers to install the SDK Expert knowledge pack (per-domain quick-reference skills: text, multimodal, speech, retrieval, fine-tuning, agent, cli), so guidance comes from the SDK's public interfaces — parameters, outputs, error codes — without reading the source -- Ask it instead of reading docs — e.g. `dashscope "how do I stream Generation output"` or `dashscope "CLI command to cancel a fine-tuning job"`. Type `/help` inside the assistant to list available commands (`/setup`, `/skill`, `/stats`, ...); classic SDK subcommands still work, and unrecognized commands are routed to the assistant +- Ask it instead of reading docs — e.g. `dashscope "how do I stream Generation output"` or `dashscope "CLI command to cancel a fine-tuning job"`. Type `/help` inside the assistant to list available commands (`/setup`, `/skill`, `/stats`, ...); classic SDK subcommands still work, and `dashscope expert` enters the assistant explicitly (`dashscope expert "question"` asks once, `dashscope expert --help` shows the forms) - Full walkthrough: [DashScope SDK Expert guide](https://help.aliyun.com/en/model-studio/dashscope-sdk-expert) ## Supported Models diff --git a/README_zh.md b/README_zh.md index 6dd4a96..9ec646a 100644 --- a/README_zh.md +++ b/README_zh.md @@ -174,7 +174,7 @@ print(response) SDK 内置了交互式 AI 助手 **DashScope SDK Expert**，基于随包提供的 Agentic CLI（`dashscope/acli`）框架构建。对于 DashScope SDK/CLI 用户，它是获取开发咨询和 AI 编码帮助的推荐方式——直接在终端中解答 SDK/API 问题、生成可运行示例、展示 CLI 用法、诊断错误。 - 直接运行 `dashscope`（不带参数）即可启动助手。首次运行时会提示安装 SDK Expert 知识包（按领域划分的速查技能：文本、多模态、语音、检索、微调、Agent、CLI），使助手的指导来自 SDK 的公开接口——参数、输出、错误码——而无需阅读源码 -- 直接提问代替翻文档——如 `dashscope "如何流式输出 Generation 结果"` 或 `dashscope "取消微调任务的 CLI 命令"`。在助手内输入 `/help` 可列出可用命令（`/setup`、`/skill`、`/stats` 等）；经典 SDK 子命令依然可用，无法识别的命令会自动转给助手处理 +- 直接提问代替翻文档——如 `dashscope "如何流式输出 Generation 结果"` 或 `dashscope "取消微调任务的 CLI 命令"`。在助手内输入 `/help` 可列出可用命令（`/setup`、`/skill`、`/stats` 等）；经典 SDK 子命令依然可用，也可用 `dashscope expert` 显式进入助手（`dashscope expert "问题"` 为一次性提问，`dashscope expert --help` 查看全部形式） - 完整使用指南：[DashScope SDK Expert 文档](https://help.aliyun.com/zh/model-studio/dashscope-sdk-expert) ## 支持的模型 diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md index c9ed272..5ae168c 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md @@ -82,7 +82,9 @@ Key option notes: - `-k/--api-key`: global API Key; can be placed before or after the command group name. The oss and rl/agentic-rl groups have their own local `--api-key` (with envvar DASHSCOPE_API_KEY), which is parsed locally once the command name appears. - `-h` is automatically converted to `--help`; use `dashscope --help` or `dashscope --help` for the authoritative option list. - Legacy syntax compatibility: `dashscope fine_tunes.call` -> `fine-tunes create`, `generation.call` -> `generation create`, etc.; underscore options like `--training_file_ids` and `--api_key` are automatically mapped to hyphenated forms. -- Bare `dashscope` (no subcommand; a change in this repo): enters the built-in agent "DashScope SDK Expert" interactive mode, TUI by default; `--cli` switches to a plain REPL, `--tui` forces TUI; requires `pip install dashscope[acli]`. When stdin is not a TTY, piped text is treated as a one-shot question. Unrecognized commands do not error; the whole line is forwarded to the agent as a question. On the first interactive run, if there is no ./.acli, it asks whether to download the dashscope-sdk-expert example configuration. +- Bare `dashscope` (no arguments; a change in this repo): enters the built-in agent "DashScope SDK Expert" interactive mode, TUI by default; `--cli` switches to a plain REPL, `--tui` forces TUI; requires `pip install dashscope[acli]`. When stdin is not a TTY, piped text is treated as a one-shot question. On the first interactive run, if there is no ./.acli, it asks whether to download the dashscope-sdk-expert example configuration. +- Entering the agent explicitly: `dashscope expert` (interactive), `dashscope expert --cli|--tui`, `dashscope expert --help` (prints usage), `dashscope expert "question"` (one-shot). A single unrecognized argument is also read as a question — `dashscope "how do I stream Generation output"`. +- Anything longer that is not a known command group is a **typer error, not a question**: `dashscope generaton create` reports `No such command`, so a typo surfaces instead of silently opening a chat. Only `-k/--api-key`, `--cli` and `--tui` may precede the command; group-level options must follow it (`dashscope generation create -w wsid`, not `dashscope -w wsid generation create`). ## Common errors and troubleshooting diff --git a/dashscope/cli/__init__.py b/dashscope/cli/__init__.py index 2c6c12a..38b61b8 100644 --- a/dashscope/cli/__init__.py +++ b/dashscope/cli/__init__.py @@ -21,7 +21,7 @@ from rich.markup import escape # noqa: E402 import dashscope # noqa: E402 -from dashscope.cli.common import err_console # noqa: E402 +from dashscope.cli.common import console, err_console # noqa: E402 from dashscope.common.error import AuthenticationError # noqa: E402 from dashscope.common.utils import set_sdk_client # noqa: E402 from dashscope.cli import ( # noqa: E402 @@ -116,6 +116,29 @@ _COMMANDS_WITH_LOCAL_API_KEY = {"oss", "rl", "agentic-rl"} + +# Routing keyword for the bundled agent, handled in main() before typer sees +# argv. Deliberately NOT a member of _TOP_LEVEL_COMMANDS: that set mirrors the +# typer-registered command groups, and `expert` is not one of them. +_EXPERT_COMMAND = "expert" + +_EXPERT_USAGE = """[bold]DashScope SDK Expert[/bold] — the built-in AI assistant. + +[bold]Usage:[/bold] + dashscope interactive assistant (TUI by default) + dashscope expert the same, spelled explicitly + dashscope expert --cli interactive, plain REPL instead of the TUI + dashscope expert --tui interactive, force the TUI + dashscope expert "question" ask once and exit + dashscope "question" ask once and exit (short form) + +With no argument, piped stdin is read as a one-shot prompt. +SDK subcommands (generation, ft, files, models, ...) dispatch as usual; an +unrecognized [italic]multi-word[/italic] command is reported as an error rather +than treated as a question. + +Requires [bold]pip install dashscope[acli][/bold].""" + _LEGACY_COMMANDS_WITH_SIZE_OPTION = { "files.list", "fine_tunes.list", @@ -326,7 +349,10 @@ def _maybe_offer_example_download(): def _route_to_expert(command, tui=None): - """Run the vendored acli agent (dashscope with no/unknown subcommand).""" + """Run the vendored acli agent (bare ``dashscope`` or ``dashscope expert``). + + ``command`` is a one-shot prompt, or None for the interactive REPL. + """ try: from dashscope.acli.cli.handlers_key import _GUIDE_DOC, _doc_locale from dashscope.acli.ui.embedded import run @@ -512,20 +538,38 @@ def main(): forced_tui = True argv = [a for a in argv if a != "--tui"] + first_cmd = _first_non_option(argv) + # `rl` is an optional extra: report a missing dependency only when the # user asks for that command, never on unrelated invocations. Without # this gate the dispatch below reaches typer, which only knows that `rl` # is in _TOP_LEVEL_COMMANDS but was never registered — "No such command" # does not tell the user how to get it. - if ( - _RL_IMPORT_ERROR is not None - and _first_non_option(argv) in _RL_COMMAND_NAMES - ): + if _RL_IMPORT_ERROR is not None and first_cmd in _RL_COMMAND_NAMES: err_console.print( f"[red]Error:[/red] {_rl_unavailable_detail(_RL_IMPORT_ERROR)}", ) sys.exit(1) + # No arguments at all → the agent, interactively. -k/--api-key and + # --cli/--tui were stripped above, so `dashscope -k sk-x` lands here too. + if len(argv) == 1: + _route_to_expert(None, tui=forced_tui) + return + + # `dashscope expert ...` → the agent, explicitly. Handled before the + # --help gate so `dashscope expert --help` describes the agent instead of + # reaching typer, which has no such command. + if first_cmd == _EXPERT_COMMAND: + idx = argv.index(_EXPERT_COMMAND) + expert_args = argv[idx + 1 :] + if "--help" in expert_args or "-h" in expert_args: + console.print(_EXPERT_USAGE) + return + prompt = " ".join(expert_args).strip() + _route_to_expert(prompt or None, tui=forced_tui) + return + # Top-level --help / -h → show help and exit normally if "--help" in argv or "-h" in argv: argv = _translate_help_shortcut(argv) @@ -533,17 +577,17 @@ def main(): app() return - # No subcommand → enter the agent (acli) directly - first_cmd = _first_non_option(argv) - if first_cmd is None: - _route_to_expert(None, tui=forced_tui) - return - - # Unknown command → route to the agent with the original user input. - # Use the cleaned argv so an extracted -k/--api-key is not leaked into - # the prompt text. - if first_cmd not in _TOP_LEVEL_COMMANDS: - _route_to_expert(" ".join(argv[1:]), tui=forced_tui) + # A lone argument that is not a known command is a question, not a + # malformed one: `dashscope "how do I stream Generation output"`. Anything + # longer falls through to typer, so a typo or a stray option gets a real + # error instead of silently opening a chat. argv is already cleaned, so an + # extracted -k/--api-key never leaks into the prompt. + if ( + len(argv) == 2 + and first_cmd is not None + and first_cmd not in _TOP_LEVEL_COMMANDS + ): + _route_to_expert(first_cmd, tui=forced_tui) return # Direct command execution (backward compatible) diff --git a/tests/unit/test_cli_agent_route.py b/tests/unit/test_cli_agent_route.py index 8f59c23..b043bd3 100644 --- a/tests/unit/test_cli_agent_route.py +++ b/tests/unit/test_cli_agent_route.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -"""Tests for the dashscope → acli agent route (expert subcommand removal). +"""Tests for the dashscope → acli agent route. Covers: - _cleanup_legacy_expert_sync: removes old managed-marker files from ~/.acli @@ -7,7 +7,9 @@ - _route_to_expert: runs embedded acli with the global -k api key passed through, and offers the example download only for interactive no-arg runs. - _maybe_offer_example_download: accept/decline/marker/workspace gating. -- main(): unknown commands route to the agent with cleaned argv (no key leak). +- main(): only a bare `dashscope`, an `expert` keyword, or a single unknown + token reach the agent; everything else is dispatched to typer so typos and + stray options produce a real error. """ # pylint: disable=protected-access,redefined-outer-name,unused-argument @@ -256,12 +258,12 @@ def fail_input(prompt=""): class TestMainRouting: - def test_unknown_command_routes_cleaned_text(self, monkeypatch): + def test_single_token_question_routes_cleaned_text(self, monkeypatch): routed: list = [] monkeypatch.setattr( cli, "_route_to_expert", - lambda command, tui=False: routed.append(command), + lambda command, tui=None: routed.append(command), ) monkeypatch.setattr( sys, @@ -280,7 +282,7 @@ def test_no_args_routes_interactive(self, monkeypatch): monkeypatch.setattr( cli, "_route_to_expert", - lambda command, tui=False: routed.append(command), + lambda command, tui=None: routed.append(command), ) monkeypatch.setattr(sys, "argv", ["dashscope"]) @@ -288,24 +290,136 @@ def test_no_args_routes_interactive(self, monkeypatch): assert routed == [None] - def test_expert_is_no_longer_a_command(self, monkeypatch): + def test_expert_keyword_is_not_a_typer_command(self): + """`expert` is a routing keyword handled inside main(), never a typer + group — it must not appear in any table typer dispatches from.""" assert "expert" not in cli._TOP_LEVEL_COMMANDS assert "expert" not in cli._COMMANDS_WITH_LOCAL_API_KEY + registered = { + g.name or g.typer_instance.info.name + for g in cli.app.registered_groups + } + assert "expert" not in registered + + def test_expert_keyword_forwards_prompt(self, monkeypatch): routed: list = [] monkeypatch.setattr( cli, "_route_to_expert", - lambda command, tui=False: routed.append(command), + lambda command, tui=None: routed.append((command, tui)), ) monkeypatch.setattr(sys, "argv", ["dashscope", "expert", "chat"]) cli.main() - assert routed == ["expert chat"] + # The keyword itself is consumed; only the question is forwarded. + assert routed == [("chat", True)] + + def test_bare_expert_is_interactive(self, monkeypatch): + routed: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append((command, tui)), + ) + monkeypatch.setattr(sys, "argv", ["dashscope", "expert"]) + + cli.main() + + assert routed == [(None, True)] + + def test_expert_cli_flag_selects_plain_repl(self, monkeypatch): + routed: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append((command, tui)), + ) + monkeypatch.setattr(sys, "argv", ["dashscope", "expert", "--cli"]) + + cli.main() + + assert routed == [(None, False)] + + def test_expert_help_prints_usage_without_routing( + self, + monkeypatch, + capsys, + ): + routed: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append((command, tui)), + ) + monkeypatch.setattr(cli, "app", lambda: routed.append("typer")) + monkeypatch.setattr(sys, "argv", ["dashscope", "expert", "--help"]) + + cli.main() + + # Must not fall through to typer, which has no `expert` command. + assert not routed + assert "DashScope SDK Expert" in capsys.readouterr().out + + def test_multiword_unknown_command_reaches_typer(self, monkeypatch): + """A typo'd command group must error in typer, not open a chat.""" + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr(sys, "argv", ["dashscope", "generaton", "create"]) + + cli.main() + + assert not routed + assert invoked == [True] + + def test_stray_leading_option_reaches_typer(self, monkeypatch): + """`-w/--workspace` is a group option, so its value must not be + mistaken for the command name and routed into the agent.""" + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr( + sys, + "argv", + ["dashscope", "-w", "wsid", "generation", "create"], + ) + + cli.main() + + assert not routed + assert invoked == [True] + + def test_unknown_flag_reaches_typer(self, monkeypatch): + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr(sys, "argv", ["dashscope", "--foo"]) + + cli.main() + + assert not routed + assert invoked == [True] def test_registered_groups_are_all_routable(self): """A group registered with typer but absent from _TOP_LEVEL_COMMANDS - never executes — main() forwards the line to the agent instead. + is dispatched to typer anyway now, but the set still drives global + -k/--api-key parsing, so the two must not drift apart. Only this direction is asserted: `rl`/`agentic-rl` stay in the set even when the optional extra is not installed. @@ -316,20 +430,23 @@ def test_registered_groups_are_all_routable(self): } assert registered <= cli._TOP_LEVEL_COMMANDS - def test_auth_whoami_executes_instead_of_routing_to_agent(self, monkeypatch): + def test_auth_whoami_executes_instead_of_routing_to_agent( + self, + monkeypatch, + ): routed: list = [] invoked: list = [] monkeypatch.setattr( cli, "_route_to_expert", - lambda command, tui=False: routed.append(command), + lambda command, tui=None: routed.append(command), ) monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) monkeypatch.setattr(sys, "argv", ["dashscope", "auth", "whoami"]) cli.main() - assert routed == [] + assert not routed assert invoked == [True] From de16112698d3bd8e326d72f75d9bd2fdc019ba28 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Wed, 23 Sep 2026 19:22:45 +0800 Subject: [PATCH 04/19] fix(cli): only bare dashscope and `expert` enter the agent The single-unknown-token short form made a quoted command line indistinguishable from a question: `dashscope "auth whoami"` opened a chat whose first user message was the command itself, which in a session log looks exactly like someone asking about it. Drop the short form so every invocation that is not a bare `dashscope` or `dashscope expert ...` goes to typer, which parses real commands and reports unknown ones with a "Did you mean" suggestion. `dashscope expert "question"` remains the one-shot form. The vendored acli entrypoint had the same flaw in a worse shape: an unrecognized token fell through to the interactive agent and discarded the arguments entirely, so `acli auth whoami` produced no error at all. It now exits 2 with a usage error, matching typer. --- README.md | 4 +- README_zh.md | 4 +- dashscope/acli/cli/__init__.py | 10 +++++ .../dashscope-sdk-expert/.acli/skills/cli.md | 7 ++-- dashscope/cli/__init__.py | 24 ++++------- tests/unit/test_cli_agent_route.py | 42 ++++++++++++++++--- 6 files changed, 61 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 10db33d..12649cd 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ The DashScope Python SDK provides a comprehensive interface to [Alibaba Cloud Mo ## What is New -**v1.27.0 ships an interactive AI assistant — [DashScope SDK Expert](#ai-assistant-dashscope-sdk-expert).** Run `dashscope` with no arguments (or ask directly, e.g. `dashscope "how do I stream Generation output"`) to get SDK/API answers, runnable examples, CLI usage, and error diagnosis right in your terminal. Guidance is drawn from per-domain quick-reference skills (text, multimodal, speech, retrieval, fine-tuning, agent, cli) built on the SDK's public interfaces — parameters, outputs, and error codes — so you can ask instead of reading the docs. Type `/help` inside the assistant to view available commands. +**v1.27.0 ships an interactive AI assistant — [DashScope SDK Expert](#ai-assistant-dashscope-sdk-expert).** Run `dashscope` with no arguments (or ask directly with `dashscope expert "how do I stream Generation output"`) to get SDK/API answers, runnable examples, CLI usage, and error diagnosis right in your terminal. Guidance is drawn from per-domain quick-reference skills (text, multimodal, speech, retrieval, fine-tuning, agent, cli) built on the SDK's public interfaces — parameters, outputs, and error codes — so you can ask instead of reading the docs. Type `/help` inside the assistant to view available commands. ## Installation To install the DashScope Python SDK, simply run: @@ -175,7 +175,7 @@ A complete runnable example is available in [`samples/set_region_example.py`](sa The SDK ships with an interactive AI assistant, **DashScope SDK Expert**, built on the bundled Agentic CLI (`dashscope/acli`) framework. For DashScope SDK/CLI users it is the recommended way to get development consultation and AI coding help — answering SDK/API questions, generating runnable examples, showing CLI usage, and diagnosing errors, right in your terminal. - Run `dashscope` with no arguments to start the assistant. On first run it offers to install the SDK Expert knowledge pack (per-domain quick-reference skills: text, multimodal, speech, retrieval, fine-tuning, agent, cli), so guidance comes from the SDK's public interfaces — parameters, outputs, error codes — without reading the source -- Ask it instead of reading docs — e.g. `dashscope "how do I stream Generation output"` or `dashscope "CLI command to cancel a fine-tuning job"`. Type `/help` inside the assistant to list available commands (`/setup`, `/skill`, `/stats`, ...); classic SDK subcommands still work, and `dashscope expert` enters the assistant explicitly (`dashscope expert "question"` asks once, `dashscope expert --help` shows the forms) +- Ask it instead of reading docs — e.g. `dashscope expert "how do I stream Generation output"` or `dashscope expert "CLI command to cancel a fine-tuning job"`, each of which asks once and exits. Type `/help` inside the assistant to list available commands (`/setup`, `/skill`, `/stats`, ...); classic SDK subcommands still work, plain `dashscope expert` enters the assistant interactively, and `dashscope expert --help` shows all the forms - Full walkthrough: [DashScope SDK Expert guide](https://help.aliyun.com/en/model-studio/dashscope-sdk-expert) ## Supported Models diff --git a/README_zh.md b/README_zh.md index 9ec646a..aadf223 100644 --- a/README_zh.md +++ b/README_zh.md @@ -6,7 +6,7 @@ DashScope Python SDK 提供了访问[阿里云百炼（Model Studio）](https:// ## 最新动态 -**v1.27.0 内置交互式 AI 助手 —— [DashScope SDK Expert](#ai-助手dashscope-sdk-expert)。** 直接运行 `dashscope`（不带任何参数），或直接提问（如 `dashscope "如何流式输出 Generation 结果"`），即可在终端中获得 SDK/API 答疑、可运行示例、CLI 用法和错误诊断。助手基于按领域划分的速查技能（文本、多模态、语音、检索、微调、Agent、CLI），这些技能构建在 SDK 的公开接口——参数、输出、错误码——之上，让你直接提问，无需翻文档。在助手内输入 `/help` 可查看可用命令。 +**v1.27.0 内置交互式 AI 助手 —— [DashScope SDK Expert](#ai-助手dashscope-sdk-expert)。** 直接运行 `dashscope`（不带任何参数），或用 `dashscope expert "如何流式输出 Generation 结果"` 直接提问，即可在终端中获得 SDK/API 答疑、可运行示例、CLI 用法和错误诊断。助手基于按领域划分的速查技能（文本、多模态、语音、检索、微调、Agent、CLI），这些技能构建在 SDK 的公开接口——参数、输出、错误码——之上，让你直接提问，无需翻文档。在助手内输入 `/help` 可查看可用命令。 ## 安装 @@ -174,7 +174,7 @@ print(response) SDK 内置了交互式 AI 助手 **DashScope SDK Expert**，基于随包提供的 Agentic CLI（`dashscope/acli`）框架构建。对于 DashScope SDK/CLI 用户，它是获取开发咨询和 AI 编码帮助的推荐方式——直接在终端中解答 SDK/API 问题、生成可运行示例、展示 CLI 用法、诊断错误。 - 直接运行 `dashscope`（不带参数）即可启动助手。首次运行时会提示安装 SDK Expert 知识包（按领域划分的速查技能：文本、多模态、语音、检索、微调、Agent、CLI），使助手的指导来自 SDK 的公开接口——参数、输出、错误码——而无需阅读源码 -- 直接提问代替翻文档——如 `dashscope "如何流式输出 Generation 结果"` 或 `dashscope "取消微调任务的 CLI 命令"`。在助手内输入 `/help` 可列出可用命令（`/setup`、`/skill`、`/stats` 等）；经典 SDK 子命令依然可用，也可用 `dashscope expert` 显式进入助手（`dashscope expert "问题"` 为一次性提问，`dashscope expert --help` 查看全部形式） +- 直接提问代替翻文档——如 `dashscope expert "如何流式输出 Generation 结果"` 或 `dashscope expert "取消微调任务的 CLI 命令"`，均为一次性提问后退出。在助手内输入 `/help` 可列出可用命令（`/setup`、`/skill`、`/stats` 等）；经典 SDK 子命令依然可用，单独执行 `dashscope expert` 进入交互式助手，`dashscope expert --help` 查看全部形式 - 完整使用指南：[DashScope SDK Expert 文档](https://help.aliyun.com/zh/model-studio/dashscope-sdk-expert) ## 支持的模型 diff --git a/dashscope/acli/cli/__init__.py b/dashscope/acli/cli/__init__.py index 766a631..aeaaf3f 100644 --- a/dashscope/acli/cli/__init__.py +++ b/dashscope/acli/cli/__init__.py @@ -219,6 +219,16 @@ def main(): _mcp_server_main() return + else: + # Only a bare `acli` and the explicit -c/--command one-shot enter + # the agent — the same contract the dashscope CLI enforces. An + # unrecognized token used to fall through and start a chat, which + # discarded the whole command line: `acli auth whoami` opened the + # agent instead of reporting that no such command exists. Exit 2 + # matches the usage-error code typer/click use for the same case. + print(f"Error: No such command '{arg}'.") + print("Try 'acli --help' for help.") + sys.exit(2) config = Config.load() _apply_cli_overrides(config, protocol_override, max_turns_override) diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md index 5ae168c..0493ed6 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/cli.md @@ -83,8 +83,8 @@ Key option notes: - `-h` is automatically converted to `--help`; use `dashscope --help` or `dashscope --help` for the authoritative option list. - Legacy syntax compatibility: `dashscope fine_tunes.call` -> `fine-tunes create`, `generation.call` -> `generation create`, etc.; underscore options like `--training_file_ids` and `--api_key` are automatically mapped to hyphenated forms. - Bare `dashscope` (no arguments; a change in this repo): enters the built-in agent "DashScope SDK Expert" interactive mode, TUI by default; `--cli` switches to a plain REPL, `--tui` forces TUI; requires `pip install dashscope[acli]`. When stdin is not a TTY, piped text is treated as a one-shot question. On the first interactive run, if there is no ./.acli, it asks whether to download the dashscope-sdk-expert example configuration. -- Entering the agent explicitly: `dashscope expert` (interactive), `dashscope expert --cli|--tui`, `dashscope expert --help` (prints usage), `dashscope expert "question"` (one-shot). A single unrecognized argument is also read as a question — `dashscope "how do I stream Generation output"`. -- Anything longer that is not a known command group is a **typer error, not a question**: `dashscope generaton create` reports `No such command`, so a typo surfaces instead of silently opening a chat. Only `-k/--api-key`, `--cli` and `--tui` may precede the command; group-level options must follow it (`dashscope generation create -w wsid`, not `dashscope -w wsid generation create`). +- Entering the agent explicitly: `dashscope expert` (interactive), `dashscope expert --cli|--tui`, `dashscope expert --help` (prints usage), `dashscope expert "question"` (one-shot). The `expert` keyword is the only way to pass a question on the command line. +- Anything else that is not a known command group is a **typer error, not a question**: `dashscope generaton create` reports `No such command`, and so does a quoted question like `dashscope "how do I stream Generation output"`, so a typo surfaces instead of silently opening a chat. Only `-k/--api-key`, `--cli` and `--tui` may precede the command; group-level options must follow it (`dashscope generation create -w wsid`, not `dashscope -w wsid generation create`). ## Common errors and troubleshooting @@ -94,4 +94,5 @@ Key option notes: - `--messages must be a valid JSON string`: generation's --messages must be a JSON array string; watch shell quoting/escaping. - `File ... does not exist`: local paths (-f, --image, --audio, etc.) are validated locally first; `~` expansion is supported; URLs must include a scheme. - `Failed request_id: ..., status_code: ..., code: ..., message: ...` (exit code 1): rejected by the server; troubleshoot by code (InvalidParameter usually means a wrong model name or parameter value); HTTP 200 with a non-empty business code is also treated as a failure. -- Bare dashscope shows `Install dashscope[acli] to enable the agent`: install `dashscope[acli]`; if it reports `Interactive mode requires a terminal.`, use `dashscope "your question"` for a one-shot question instead. +- Bare dashscope shows `Install dashscope[acli] to enable the agent`: install `dashscope[acli]`; if it reports `Interactive mode requires a terminal.`, use `dashscope expert "your question"` for a one-shot question instead. +- `No such command 'how do I ...'`: a question was passed without the `expert` keyword. `dashscope "question"` is no longer a short form — tell the user to write `dashscope expert "question"`. diff --git a/dashscope/cli/__init__.py b/dashscope/cli/__init__.py index 38b61b8..3e6ae91 100644 --- a/dashscope/cli/__init__.py +++ b/dashscope/cli/__init__.py @@ -130,12 +130,11 @@ dashscope expert --cli interactive, plain REPL instead of the TUI dashscope expert --tui interactive, force the TUI dashscope expert "question" ask once and exit - dashscope "question" ask once and exit (short form) With no argument, piped stdin is read as a one-shot prompt. SDK subcommands (generation, ft, files, models, ...) dispatch as usual; an -unrecognized [italic]multi-word[/italic] command is reported as an error rather -than treated as a question. +unrecognized command is reported as an error rather than treated as a +question, so ask one with [bold]dashscope expert "question"[/bold]. Requires [bold]pip install dashscope[acli][/bold].""" @@ -577,20 +576,11 @@ def main(): app() return - # A lone argument that is not a known command is a question, not a - # malformed one: `dashscope "how do I stream Generation output"`. Anything - # longer falls through to typer, so a typo or a stray option gets a real - # error instead of silently opening a chat. argv is already cleaned, so an - # extracted -k/--api-key never leaks into the prompt. - if ( - len(argv) == 2 - and first_cmd is not None - and first_cmd not in _TOP_LEVEL_COMMANDS - ): - _route_to_expert(first_cmd, tui=forced_tui) - return - - # Direct command execution (backward compatible) + # Anything that is not bare `dashscope` and not `dashscope expert ...` + # belongs to typer, so a typo or a stray option gets a real error instead + # of silently opening a chat. Forwarding an unrecognized token as a + # question used to swallow whole command lines -- `dashscope auth whoami` + # opened a session whose first message was the command itself. argv = _translate_help_shortcut(argv) sys.argv = argv diff --git a/tests/unit/test_cli_agent_route.py b/tests/unit/test_cli_agent_route.py index b043bd3..d44dde4 100644 --- a/tests/unit/test_cli_agent_route.py +++ b/tests/unit/test_cli_agent_route.py @@ -7,9 +7,9 @@ - _route_to_expert: runs embedded acli with the global -k api key passed through, and offers the example download only for interactive no-arg runs. - _maybe_offer_example_download: accept/decline/marker/workspace gating. -- main(): only a bare `dashscope`, an `expert` keyword, or a single unknown - token reach the agent; everything else is dispatched to typer so typos and - stray options produce a real error. +- main(): only a bare `dashscope` or an explicit `expert` keyword reach the + agent; everything else is dispatched to typer so typos, stray options and + unrecognized commands produce a real error. """ # pylint: disable=protected-access,redefined-outer-name,unused-argument @@ -258,13 +258,20 @@ def fail_input(prompt=""): class TestMainRouting: - def test_single_token_question_routes_cleaned_text(self, monkeypatch): + def test_unknown_single_token_reaches_typer(self, monkeypatch): + """`dashscope "question"` is not a short form for the agent. + + An unrecognized token must error in typer so a mistyped command stays + visible; asking a question is `dashscope expert "question"`. + """ routed: list = [] + invoked: list = [] monkeypatch.setattr( cli, "_route_to_expert", lambda command, tui=None: routed.append(command), ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) monkeypatch.setattr( sys, "argv", @@ -273,10 +280,33 @@ def test_single_token_question_routes_cleaned_text(self, monkeypatch): cli.main() - # The extracted -k value must not leak into the routed prompt text. - assert routed == ["你好，未知命令"] + assert not routed + assert invoked == [True] + # The global key is still extracted on the typer path. assert dashscope.api_key == "sk-secret" + def test_quoted_command_line_reaches_typer(self, monkeypatch): + """A whole command line passed as one shell word is not a question. + + It used to open a chat whose first user message was the command + itself, which is indistinguishable in the session log from a user who + meant to ask something. + """ + routed: list = [] + invoked: list = [] + monkeypatch.setattr( + cli, + "_route_to_expert", + lambda command, tui=None: routed.append(command), + ) + monkeypatch.setattr(cli, "app", lambda: invoked.append(True)) + monkeypatch.setattr(sys, "argv", ["dashscope", "auth whoami"]) + + cli.main() + + assert not routed + assert invoked == [True] + def test_no_args_routes_interactive(self, monkeypatch): routed: list = [] monkeypatch.setattr( From cd085a02ac2f465c899b00f4118d058b60b35c2f Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Wed, 23 Sep 2026 19:23:03 +0800 Subject: [PATCH 05/19] fix(acli): probe multimodal-generation first and remember the binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Synced from agenticCLI. The gateway binds each model to exactly one aigc service path and answers the other with a 400 "url error", so the probe order decides who pays for an unlearned model. text-generation went first, which made the default model (qwen3.8-max, multimodal-bound) fail the first request of every session — in the prod gateway log that is an InvalidParameter with a fully tokenized body thrown away, then a retry that re-uploads and re-tokenizes it. The learned path was also kept only on the instance, and a provider is built per session start, per model switch and per vision call, so nothing survived between them. Persist the binding next to the global config and share it across instances; drop unreadable or unknown-path entries instead of repairing them, since a lost binding only costs one probe. --- dashscope/acli/providers/tongyi.py | 99 ++++++++++++++++-- tests/unit/conftest.py | 18 ++++ tests/unit/test_tongyi_native.py | 159 ++++++++++++++++++++++++----- 3 files changed, 243 insertions(+), 33 deletions(-) diff --git a/dashscope/acli/providers/tongyi.py b/dashscope/acli/providers/tongyi.py index c3fefce..9b8b1a3 100644 --- a/dashscope/acli/providers/tongyi.py +++ b/dashscope/acli/providers/tongyi.py @@ -4,7 +4,7 @@ import json import os -from typing import AsyncIterator +from typing import AsyncIterator, ClassVar import httpx @@ -15,14 +15,21 @@ # the DashScope native route (NOT the OpenAI-compatible one) so requests # land in the native SLS logstore. DASHSCOPE_BASE_URL = "https://dashscope.aliyuncs.com" -# The gateway binds each model to exactly one aigc service path (newer -# models like qwen3.8-max live on multimodal-generation, classic text -# models on text-generation). The provider tries them in order and -# remembers the one that answers 200 — see _is_url_error. +# The gateway binds each model to exactly one aigc service path and answers +# the other with a 400 "url error" (see _is_url_error), so probing an +# unlearned model always costs one failed request — one that lands in the +# prod gateway log as InvalidParameter. multimodal-generation is probed +# first because acli's default model (qwen3.8-max) and the vl/omni family +# live there; classic text models are the minority and learn their binding +# on the first call. _GENERATION_PATHS = ( - "/api/v1/services/aigc/text-generation/generation", "/api/v1/services/aigc/multimodal-generation/generation", + "/api/v1/services/aigc/text-generation/generation", ) +# Learned model → path bindings, shared across processes so a fresh CLI +# start does not re-probe. Global rather than per-workspace: the binding is +# a property of the gateway, and bench/CI runs execute in a throwaway cwd. +_PATH_CACHE_NAME = "generation-paths.json" # Historical/standard prefixes a persisted config may carry; the provider # normalizes them back to the service root. _LEGACY_PREFIXES = ("/compatible-mode/v1", "/api/v1") @@ -59,6 +66,45 @@ def _is_url_error_message(message: str) -> bool: return "url error" in (message or "") +def _load_path_cache() -> dict[str, str]: + """Read the model → generation-path bindings learned by earlier runs. + + Entries that are unreadable or name an unknown path are dropped rather + than repaired: a dropped binding costs one probe, which the fallback + already survives. + """ + from dashscope.acli.config import CONFIG_DIR + + try: + raw = json.loads( + (CONFIG_DIR / _PATH_CACHE_NAME).read_text(encoding="utf-8"), + ) + except (OSError, ValueError): + return {} + if not isinstance(raw, dict): + return {} + return { + str(model): str(path) + for model, path in raw.items() + if str(path) in _GENERATION_PATHS + } + + +def _save_path_cache(paths: dict[str, str]) -> None: + """Persist learned bindings. An unwritable home must not fail a call.""" + from dashscope.acli.config import CONFIG_DIR + from dashscope.acli.utils.paths import atomic_write_text + + try: + CONFIG_DIR.mkdir(parents=True, exist_ok=True) + atomic_write_text( + CONFIG_DIR / _PATH_CACHE_NAME, + json.dumps(paths, indent=2, sort_keys=True), + ) + except OSError: + pass + + def _to_native_content(content): """Convert OpenAI message content parts to the native shape. @@ -185,6 +231,30 @@ def _native_chunk_to_openai(chunk) -> dict: class TongyiProvider: + # model → generation path, shared by every instance in this process and + # mirrored to disk by _remember_path. A provider is constructed per + # session start, per model switch and per vision call, so without this + # each of those re-probes and re-logs the same 400. + _path_cache: ClassVar[dict[str, str] | None] = None + + @classmethod + def _learned_paths(cls) -> dict[str, str]: + # Bound to a local so the Optional class attribute narrows to a dict + # for both the caller and the type checker. + cache = cls._path_cache + if cache is None: + cache = _load_path_cache() + cls._path_cache = cache + return cache + + @classmethod + def _remember_path(cls, model: str, path: str) -> None: + learned = cls._learned_paths() + if learned.get(model) == path: + return + learned[model] = path + _save_path_cache(learned) + def __init__( self, model: str = "qwen3.8-max", @@ -205,7 +275,16 @@ def __init__( break self.base_url = base self.module = module - self._generation_path = _GENERATION_PATHS[0] + self._generation_path = self._learned_paths().get( + model, + _GENERATION_PATHS[0], + ) + + def _commit_path(self, path: str) -> None: + """Record the path that answered 200, for this instance and every + later one.""" + self._generation_path = path + self._remember_path(self.model, path) def _candidate_paths(self) -> list[str]: others = [p for p in _GENERATION_PATHS if p != self._generation_path] @@ -348,7 +427,7 @@ async def chat( headers=headers, ) if response.status_code == 200: - self._generation_path = path + self._commit_path(path) break if i + 1 < len(paths) and _is_url_error( response.status_code, @@ -487,7 +566,7 @@ async def chat_stream( try: first = await stream.__anext__() except StopAsyncIteration: - self._generation_path = path + self._commit_path(path) return except _StreamAPIError as e: if i + 1 < len(paths) and _is_url_error_message( @@ -495,7 +574,7 @@ async def chat_stream( ): continue raise - self._generation_path = path + self._commit_path(path) yield first async for chunk in stream: yield chunk diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index eb41d5c..7806808 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -10,6 +10,24 @@ from tests.unit.mock_server import create_mock_server, run_server +@pytest.fixture(autouse=True) +def isolated_generation_path_cache(monkeypatch, tmp_path): + """Keep TongyiProvider's learned model→path cache off the real ~/.acli. + + The cache is persisted across processes by design, so without this a test + run would write bindings into the developer's home directory — and would + read back whatever is already there, making the probe-order assertions + depend on machine state instead of on the code. + """ + from dashscope.acli import config as config_module + from dashscope.acli.providers.tongyi import TongyiProvider + + monkeypatch.setattr(config_module, "CONFIG_DIR", tmp_path / "acli-home") + # Class-level state survives across tests; monkeypatch restores the + # previous value on teardown, so each test starts from "nothing learned". + monkeypatch.setattr(TongyiProvider, "_path_cache", None) + + @pytest.fixture def mock_disable_data_inspection_env(monkeypatch): monkeypatch.setenv(DASHSCOPE_DISABLE_DATA_INSPECTION_ENV, "true") diff --git a/tests/unit/test_tongyi_native.py b/tests/unit/test_tongyi_native.py index 2fc4670..36ff2cc 100644 --- a/tests/unit/test_tongyi_native.py +++ b/tests/unit/test_tongyi_native.py @@ -17,10 +17,15 @@ from dashscope.acli.providers.tongyi import ( _GENERATION_PATHS, + _PATH_CACHE_NAME, TongyiProvider, ) -TEXT_GEN, MULTIMODAL_GEN = _GENERATION_PATHS +# Spelled out rather than destructured from _GENERATION_PATHS: a positional +# unpack silently swaps these two names the moment the probe order changes, +# inverting every assertion below while most of them still pass. +TEXT_GEN = "/api/v1/services/aigc/text-generation/generation" +MULTIMODAL_GEN = "/api/v1/services/aigc/multimodal-generation/generation" ROOT = "https://dashscope.aliyuncs.com" MESSAGES = [{"role": "user", "content": "hi"}] @@ -272,7 +277,6 @@ async def test_chat_posts_native_body(fake_http): resp = await _provider().chat(MESSAGES, tools=TOOLS) cap = fake_http.requests[0] - assert cap["url"] == ROOT + TEXT_GEN body = cap["body"] assert body["model"] == "qwen-plus" assert body["input"] == {"messages": MESSAGES} @@ -293,7 +297,6 @@ async def test_chat_stream_posts_native_body_with_sse_headers(fake_http): chunks = [c async for c in provider.chat_stream(MESSAGES, tools=TOOLS)] cap = fake_http.requests[0] - assert cap["url"] == ROOT + TEXT_GEN params = cap["body"]["parameters"] assert params["result_format"] == "message" assert params["stream"] is True @@ -362,27 +365,63 @@ def test_base_url_normalized_to_service_root(given, expected): # --------------------------------------------------------------------------- -# endpoint fallback: text-generation ⇄ multimodal-generation +# endpoint selection: multimodal-generation first, text-generation fallback # --------------------------------------------------------------------------- -async def test_chat_falls_back_to_multimodal_path(fake_http): - fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) +def test_multimodal_path_is_probed_first(): + """Pinned on purpose. The gateway binds each model to one aigc path and + rejects the other with a 400 "url error" that lands in the prod log as + InvalidParameter, so the probe order decides who pays it. acli's default + model (qwen3.8-max) plus the vl/omni family are multimodal-bound; the + classic text models are the minority. Flip this and every fallback + assertion below flips with it.""" + assert _GENERATION_PATHS == (MULTIMODAL_GEN, TEXT_GEN) + + +async def test_chat_multimodal_model_needs_no_probe(fake_http): + """The common case: first request already hits the right path, and the + part-list content multimodal-generation returns is flattened.""" fake_http.enqueue_post(MULTIMODAL_RESPONSE) + resp = await _provider(model="qwen3.8-max").chat(MESSAGES) + + assert [r["url"] for r in fake_http.requests] == [ROOT + MULTIMODAL_GEN] + assert resp.content == "hello there" + assert resp.reasoning_content == "thinking..." + assert resp.usage["cached_tokens"] == 5 + + +async def test_stream_multimodal_model_needs_no_probe(fake_http): + fake_http.enqueue_stream(MULTIMODAL_SSE_LINES) provider = _provider(model="qwen3.8-max") + chunks = [c async for c in provider.chat_stream(MESSAGES)] + + assert [r["url"] for r in fake_http.requests] == [ROOT + MULTIMODAL_GEN] + assert "th" in [c.delta_reasoning_content for c in chunks] + assert [c.delta_content for c in chunks if c.delta_content] == [ + "Hel", + "lo", + ] + assert chunks[-1].usage["total_tokens"] == 9 + + +async def test_chat_falls_back_to_text_path(fake_http): + fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) + fake_http.enqueue_post(NATIVE_RESPONSE) + provider = _provider(model="qwen-plus") resp = await provider.chat(MESSAGES) assert [r["url"] for r in fake_http.requests] == [ - ROOT + TEXT_GEN, ROOT + MULTIMODAL_GEN, + ROOT + TEXT_GEN, ] assert resp.content == "hello there" - assert resp.reasoning_content == "thinking..." - assert resp.usage["cached_tokens"] == 5 + assert resp.usage["total_tokens"] == 18 # working path is cached: the next call skips the 400 round-trip fake_http.enqueue_post(NATIVE_RESPONSE) await provider.chat(MESSAGES) - assert fake_http.requests[-1]["url"] == ROOT + MULTIMODAL_GEN + assert len(fake_http.requests) == 3 + assert fake_http.requests[-1]["url"] == ROOT + TEXT_GEN async def test_chat_no_fallback_on_other_400(fake_http): @@ -395,27 +434,25 @@ async def test_chat_no_fallback_on_other_400(fake_http): assert len(fake_http.requests) == 1 -async def test_stream_falls_back_to_multimodal_path(fake_http): +async def test_stream_falls_back_to_text_path(fake_http): fake_http.enqueue_stream( [], status_code=400, body=json.dumps(URL_ERROR_BODY, ensure_ascii=False), ) - fake_http.enqueue_stream(MULTIMODAL_SSE_LINES) - provider = _provider(model="qwen3.8-max") + fake_http.enqueue_stream(NATIVE_SSE_LINES) + provider = _provider(model="qwen-plus") chunks = [c async for c in provider.chat_stream(MESSAGES)] assert [r["url"] for r in fake_http.requests] == [ - ROOT + TEXT_GEN, ROOT + MULTIMODAL_GEN, + ROOT + TEXT_GEN, ] - reasoning = [c.delta_reasoning_content for c in chunks] - assert "th" in reasoning contents = [c.delta_content for c in chunks if c.delta_content] assert contents == ["Hel", "lo"] last = chunks[-1] assert last.finish_reason == "stop" - assert last.usage["total_tokens"] == 9 + assert last.usage["total_tokens"] == 13 async def test_stream_falls_back_on_sse_error_event(fake_http): @@ -429,27 +466,27 @@ async def test_stream_falls_back_on_sse_error_event(fake_http): "data:" + json.dumps(URL_ERROR_BODY, ensure_ascii=False), ], ) - fake_http.enqueue_stream(MULTIMODAL_SSE_LINES) - provider = _provider(model="qwen3.8-max") + fake_http.enqueue_stream(NATIVE_SSE_LINES) + provider = _provider(model="qwen-plus") chunks = [c async for c in provider.chat_stream(MESSAGES)] assert [r["url"] for r in fake_http.requests] == [ - ROOT + TEXT_GEN, ROOT + MULTIMODAL_GEN, + ROOT + TEXT_GEN, ] assert [c.delta_content for c in chunks if c.delta_content] == [ "Hel", "lo", ] # the working path is cached only after a real chunk arrived - assert provider._generation_path == MULTIMODAL_GEN + assert provider._generation_path == TEXT_GEN async def test_stream_sse_url_error_raises_after_paths_exhausted(fake_http): event = ["data:" + json.dumps(URL_ERROR_BODY, ensure_ascii=False)] fake_http.enqueue_stream(list(event)) fake_http.enqueue_stream(list(event)) - provider = _provider(model="qwen3.8-max") + provider = _provider() with pytest.raises(RuntimeError, match="url error"): async for _ in provider.chat_stream(MESSAGES): pass @@ -461,13 +498,89 @@ async def test_stream_no_fallback_after_content_started(fake_http): "data:" + json.dumps(URL_ERROR_BODY, ensure_ascii=False), ] fake_http.enqueue_stream(lines) - provider = _provider(model="qwen3.8-max") + provider = _provider() with pytest.raises(RuntimeError, match="url error"): async for _ in provider.chat_stream(MESSAGES): pass assert len(fake_http.requests) == 1 +# --------------------------------------------------------------------------- +# learned-path cache: a probe costs a 400 in the prod gateway log, so the +# answer is remembered per model — in-process and on disk +# --------------------------------------------------------------------------- + + +async def test_learned_path_skips_the_probe_in_a_new_instance(fake_http): + """A provider is constructed per session start, per model switch and per + vision call; only the first of those may pay the probe.""" + fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + assert len(fake_http.requests) == 2 + + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + assert len(fake_http.requests) == 3 + assert fake_http.requests[-1]["url"] == ROOT + TEXT_GEN + + +async def test_learned_path_survives_a_new_process(fake_http, monkeypatch): + """Bench and CI runs execute in throwaway containers, so the binding has + to outlive the process to be worth anything.""" + fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + assert len(fake_http.requests) == 2 + + # Simulate a fresh process: empty in-process cache, same home directory. + monkeypatch.setattr(TongyiProvider, "_path_cache", None) + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + assert len(fake_http.requests) == 3 + assert fake_http.requests[-1]["url"] == ROOT + TEXT_GEN + + +async def test_path_cache_is_written_next_to_the_global_config(fake_http): + from dashscope.acli import config as config_module + + fake_http.enqueue_post(URL_ERROR_BODY, status_code=400) + fake_http.enqueue_post(NATIVE_RESPONSE) + await _provider().chat(MESSAGES) + + cache_file = config_module.CONFIG_DIR / _PATH_CACHE_NAME + assert json.loads(cache_file.read_text(encoding="utf-8")) == { + "qwen-plus": TEXT_GEN, + } + + +def test_path_cache_drops_unknown_paths(monkeypatch): + """A model the gateway rebinds, or a hand-edited cache, must fall back to + probing rather than POST to a path that no longer exists.""" + from dashscope.acli import config as config_module + + cache_file = config_module.CONFIG_DIR / _PATH_CACHE_NAME + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_text( + json.dumps({"qwen-plus": "/api/v1/services/aigc/bogus/generation"}), + encoding="utf-8", + ) + monkeypatch.setattr(TongyiProvider, "_path_cache", None) + + assert _provider()._generation_path == MULTIMODAL_GEN + + +def test_path_cache_survives_a_corrupt_file(monkeypatch): + from dashscope.acli import config as config_module + + cache_file = config_module.CONFIG_DIR / _PATH_CACHE_NAME + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_text("not json at all", encoding="utf-8") + monkeypatch.setattr(TongyiProvider, "_path_cache", None) + + assert _provider()._generation_path == MULTIMODAL_GEN + + # --------------------------------------------------------------------------- # non-stream response parsing # --------------------------------------------------------------------------- From 7d28991568b674875aa40e47bcc5f489a5cee546 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Wed, 23 Sep 2026 21:48:20 +0800 Subject: [PATCH 06/19] test(cli): pin dashscope auth whoami and login behaviour Eleven tests over the two commands a user runs first, against a throwaway ~/.dashscope and a stubbed Models.list, so no real key file is touched and no request is sent. whoami: the login guidance and exit 1 when nothing is configured; the masked key with its source on success; an environment key reported as its own source rather than the file; a short key masked as *** instead of half-revealed; exit 2 on a rejected key without leaking it; exit 2 with the reason when the validation call itself fails. login: prompting and saving, --key skipping the prompt, surrounding whitespace stripped, a blank key refused without writing anything, and a round trip proving whoami accepts what login wrote. The key_store fixture has to move both bindings of the api-key path constant: save_api_key/get_default_api_key read it from dashscope.common.api_key while auth reads its own imported copy, so patching one leaves login and whoami silently disagreeing about where the key lives. Messages are compared through a newline-stripping helper because rich hard-wraps at 80 columns under CliRunner and force-breaks long tokens without inserting a space. Wrapping Console.width instead would pin _width on the module-level consoles past teardown. Checked against three deliberate breaks of auth.py: 7 of the 11 failed, then all 11 passed again once it was restored. --- tests/unit/test_cli_auth.py | 238 ++++++++++++++++++++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 tests/unit/test_cli_auth.py diff --git a/tests/unit/test_cli_auth.py b/tests/unit/test_cli_auth.py new file mode 100644 index 0000000..9d39002 --- /dev/null +++ b/tests/unit/test_cli_auth.py @@ -0,0 +1,238 @@ +# -*- coding: utf-8 -*- +# Copyright (c) Alibaba, Inc. and its affiliates. +"""Behaviour of ``dashscope auth whoami`` and ``dashscope auth login``. + +Pins what a user sees on the terminal: the guidance printed when no key is +configured, the masked key and its source on success, and the file ``login`` +writes. Every test runs against a throwaway ``~/.dashscope`` and a stubbed +``Models.list``, so no real key file is touched and no request is sent. +""" +from http import HTTPStatus +from types import SimpleNamespace + +import pytest +from typer.testing import CliRunner + +import dashscope +from dashscope.cli import auth as auth_cli +from dashscope.common import api_key as api_key_mod +from dashscope.common.constants import ( + DASHSCOPE_API_KEY_ENV, + DASHSCOPE_API_KEY_FILE_PATH_ENV, +) + +KEY = "sk-unittest-0123456789" +MASKED = "sk-uni...6789" + + +def _flat(text): + """Undo rich's 80-column hard wrapping so messages compare whole.""" + return "".join(text.splitlines()) + + +def _response(status_code, code=None, message=None): + return SimpleNamespace( + status_code=status_code, + code=code, + message=message, + ) + + +def _stub_models_list(monkeypatch, rsp=None, exc=None): + """Replace the single validation call ``whoami`` makes; record its args.""" + calls = [] + + def fake_list(**kwargs): + calls.append(kwargs) + if exc is not None: + raise exc + return rsp + + monkeypatch.setattr(dashscope.Models, "list", fake_list) + return calls + + +@pytest.fixture +def key_store(monkeypatch, tmp_path): + """Redirect every api-key lookup to an empty throwaway home.""" + monkeypatch.delenv(DASHSCOPE_API_KEY_ENV, raising=False) + monkeypatch.delenv(DASHSCOPE_API_KEY_FILE_PATH_ENV, raising=False) + monkeypatch.setattr(dashscope, "api_key", None) + monkeypatch.setattr(dashscope, "api_key_file_path", None) + + cache_dir = tmp_path / ".dashscope" + key_file = cache_dir / "api_key" + cache_dir.mkdir(parents=True, exist_ok=True) + # ``save_api_key``/``get_default_api_key`` read the constant from + # dashscope.common.api_key, while the printed path and the source check in + # auth read its own imported copy. Both bindings have to move, or login and + # whoami silently disagree about where the key lives. + monkeypatch.setattr( + api_key_mod, + "DEFAULT_DASHSCOPE_CACHE_PATH", + cache_dir, + ) + monkeypatch.setattr( + api_key_mod, + "DEFAULT_DASHSCOPE_API_KEY_FILE_PATH", + key_file, + ) + monkeypatch.setattr( + auth_cli, + "DEFAULT_DASHSCOPE_API_KEY_FILE_PATH", + key_file, + ) + return SimpleNamespace(dir=cache_dir, file=key_file) + + +class TestAuthWhoami: + def test_no_key_prints_login_guidance(self, monkeypatch, key_store): + calls = _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 1 + text = _flat(result.output) + assert "Error: No API key configured." in text + guidance = ( + "Run dashscope auth login or set " + "DASHSCOPE_API_KEY to configure one." + ) + assert guidance in text + assert calls == [], "must not reach the API without a key" + + def test_masks_key_and_reports_the_file_source( + self, + monkeypatch, + key_store, + ): + key_store.file.write_text(KEY, encoding="utf-8") + calls = _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 0 + text = _flat(result.output) + assert f"Authenticated key={MASKED}" in text + assert f"source=file ({key_store.file})" in text + assert KEY not in text + assert calls == [{"page": 1, "page_size": 1}] + + def test_reports_an_environment_key_as_its_own_source( + self, + monkeypatch, + key_store, + ): + monkeypatch.setattr(dashscope, "api_key", KEY) + _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 0 + text = _flat(result.output) + assert f"Authenticated key={MASKED}" in text + assert "source=environment / --api-key flag" in text + + def test_short_key_is_not_partially_revealed( + self, + monkeypatch, + key_store, + ): + short = "sk-abc123" + key_store.file.write_text(short, encoding="utf-8") + _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 0 + text = _flat(result.output) + assert "key=***" in text + assert short not in text + + def test_rejected_key_exits_2(self, monkeypatch, key_store): + key_store.file.write_text(KEY, encoding="utf-8") + _stub_models_list( + monkeypatch, + rsp=_response( + HTTPStatus.UNAUTHORIZED, + "InvalidApiKey", + "Invalid API-key provided.", + ), + ) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 2 + text = _flat(result.output) + assert "Invalid API key" in text + assert f"source=file ({key_store.file})" in text + assert "code=InvalidApiKey" in text + assert KEY not in text + assert MASKED not in text + + def test_api_failure_exits_2_with_the_reason( + self, + monkeypatch, + key_store, + ): + key_store.file.write_text(KEY, encoding="utf-8") + _stub_models_list(monkeypatch, exc=RuntimeError("no route to host")) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 2 + assert "Error: API call failed: no route to host" in _flat( + result.output, + ) + assert KEY not in result.output + + +class TestAuthLogin: + def test_prompts_for_the_key_and_saves_it(self, key_store): + result = CliRunner().invoke(auth_cli.app, ["login"], input=f"{KEY}\n") + + assert result.exit_code == 0 + text = _flat(result.output) + assert "Enter your DashScope API key: " in text + assert f"Saved API key to {key_store.file}" in text + assert key_store.file.read_text(encoding="utf-8") == KEY + assert KEY not in text, "hide_input must keep the key off screen" + + def test_key_flag_skips_the_prompt(self, key_store): + result = CliRunner().invoke(auth_cli.app, ["login", "-k", KEY]) + + assert result.exit_code == 0 + assert "Enter your DashScope API key" not in result.output + assert f"Saved API key to {key_store.file}" in _flat(result.output) + assert key_store.file.read_text(encoding="utf-8") == KEY + + def test_surrounding_whitespace_is_stripped(self, key_store): + result = CliRunner().invoke( + auth_cli.app, + ["login"], + input=f" {KEY} \n", + ) + + assert result.exit_code == 0 + assert key_store.file.read_text(encoding="utf-8") == KEY + + def test_blank_key_is_refused_and_nothing_is_written(self, key_store): + result = CliRunner().invoke(auth_cli.app, ["login", "-k", " "]) + + assert result.exit_code == 1 + assert "Error: API key cannot be empty." in _flat(result.output) + assert not key_store.file.exists() + + def test_whoami_accepts_what_login_wrote(self, monkeypatch, key_store): + """The two commands must agree on the key file location.""" + saved = CliRunner().invoke(auth_cli.app, ["login", "-k", KEY]) + assert saved.exit_code == 0 + calls = _stub_models_list(monkeypatch, rsp=_response(HTTPStatus.OK)) + + result = CliRunner().invoke(auth_cli.app, ["whoami"]) + + assert result.exit_code == 0 + text = _flat(result.output) + assert f"Authenticated key={MASKED}" in text + assert f"source=file ({key_store.file})" in text + assert calls == [{"page": 1, "page_size": 1}] From 6545692c95e313c8a2d866afb2d6c00ff7866ed1 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Wed, 23 Sep 2026 21:48:53 +0800 Subject: [PATCH 07/19] =?UTF-8?q?acli:=20sync=200.6.7=20=E2=80=94=20skills?= =?UTF-8?q?,=20search=5Ffiles,=20platform=20line,=20Windows=20decoding?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vendors agenticCLI 4876f43 plus the 0.6.7 bump. All seven files are byte-identical to the upstream sources modulo the dashscope. import prefix. render_skill no longer runs str.format() over a skill body. A body is Markdown full of literal braces from the JSON and Python it documents, so text-generation.md — which shows messages=[{"role": "user", ...}] and declares no arguments at all — raised KeyError('"role"'), and the old `except KeyError` reported that as "missing args" for a skill that takes none. 6 of the 13 bundled skills were unusable through use_skill while /skill and the catalog kept advertising them. Substitution is now a plain replace of the declared arguments only; use_skill and the scheduler branch on `is None` rather than falsiness, so the missing-args hint can no longer render empty. search_files matches the pattern against paths relative to the search root as well as the basename, and skips dot-directories only unless the pattern names one, so .acli/skills/*.md is reachable without walking every .git in the tree. Its description now says that * crosses separators, which it does under fnmatch and did not under the old basename-only match, and the no-match message states what was actually searched instead of implying the file is absent. EnvironmentSection states the platform and how run_command executes, through a new shell_label() that lives next to _spawn so the prompt cannot drift from what the tool really runs. Nothing used to say which shell this is: on Windows the model wrote `2>/dev/null || find /` and spent a turn on a PowerShell ParserError. Child-process output decoding tries UTF-8 strictly first, then the Windows OEM code page from GetOEMCP() and the stdio/locale encodings. A piped child writes in its console's code page, which need not equal sys.stdout.encoding, and cp936 accepts nearly any byte sequence so it would silently mangle UTF-8 rather than fail. A UnicodeDecodeError in the last four bytes is the byte-budget truncation splitting a character, not evidence of another encoding, so it is repaired instead of re-decoding the whole buffer under a DBCS page. Adds tests/unit/test_acli_skill_render.py: the 13 bundled skill files ship only in this wheel, so they are the real corpus that broke and the one piece of coverage with dashscope-only value. Verified by restoring the pre-fix render_skill — exactly those 6 skills fail, 19 tests — then restoring the fix. The upstream coverage for the other three fixes stays in agenticCLI, which is where those test modules live. dashscope unit: 1011 passed, 7 skipped. pre-commit --all-files green, mypy included. dashscope/acli/examples/ is untouched by the sync. --- dashscope/acli/__init__.py | 2 +- dashscope/acli/prompt_pipeline.py | 14 ++- dashscope/acli/scheduler.py | 4 +- dashscope/acli/skills/base.py | 19 ++-- dashscope/acli/tools/filesystem.py | 39 ++++++-- dashscope/acli/tools/shell.py | 96 +++++++++++++++--- dashscope/acli/tools/skills.py | 5 +- dashscope/version.py | 2 +- tests/unit/test_acli_skill_render.py | 143 +++++++++++++++++++++++++++ 9 files changed, 288 insertions(+), 36 deletions(-) create mode 100644 tests/unit/test_acli_skill_render.py diff --git a/dashscope/acli/__init__.py b/dashscope/acli/__init__.py index 47aaece..79c639e 100644 --- a/dashscope/acli/__init__.py +++ b/dashscope/acli/__init__.py @@ -3,7 +3,7 @@ import uuid -__version__ = "0.6.6" +__version__ = "0.6.7" # Per-process identifier sent as x-dashscope-sdk-session-id so the # backend can group multi-turn requests from one CLI run. diff --git a/dashscope/acli/prompt_pipeline.py b/dashscope/acli/prompt_pipeline.py index 40aa13f..bdba8d2 100644 --- a/dashscope/acli/prompt_pipeline.py +++ b/dashscope/acli/prompt_pipeline.py @@ -200,7 +200,17 @@ class EnvironmentSection: name = "environment" def render(self, ctx: PromptContext) -> str: - env_info: list[str] = [] + import platform + + from dashscope.acli.tools.shell import shell_label + + # The platform is stated because run_command's syntax depends on it and + # a wrong guess costs a whole turn: on Windows the model wrote + # `2>/dev/null || find /` and got back a PowerShell ParserError. + env_info: list[str] = [ + f"Current OS: {platform.system()} {platform.machine()}", + f"run_command executes via: {shell_label()}", + ] if ctx.user_name: env_info.append(f"Current user: {ctx.user_name}") if ctx.provider_name or ctx.model_name: @@ -210,8 +220,6 @@ def render(self, ctx: PromptContext) -> str: "mentioned in history or summaries may be outdated — " "always defer to this line for model questions)", ) - if not env_info: - return "" env_info.append( "Rule: when mentioning this CLI's commands, only cite real " "commands that definitely exist; when unsure, point the user " diff --git a/dashscope/acli/scheduler.py b/dashscope/acli/scheduler.py index a14ad44..52c9ea3 100644 --- a/dashscope/acli/scheduler.py +++ b/dashscope/acli/scheduler.py @@ -591,7 +591,9 @@ async def _execute_job(self, job: CronJob) -> None: continue rendered = render_skill(skill, inv.args) - if not rendered: + # `is None`, not falsy: None is the missing-arguments signal, so + # arg_hint is never empty here. + if rendered is None: arg_hint = " ".join(f"<{a}>" for a in skill.arguments) results.append( f"missing args: /skill {inv.name} {arg_hint}", diff --git a/dashscope/acli/skills/base.py b/dashscope/acli/skills/base.py index e886f24..2cc9fbc 100644 --- a/dashscope/acli/skills/base.py +++ b/dashscope/acli/skills/base.py @@ -200,13 +200,18 @@ def render_skill(skill: Skill, args: list[str]) -> str | None: don't match.""" if len(args) < len(skill.arguments): return None - kwargs = {} + template = skill.prompt_template + # A skill body is Markdown, not a format string: it is full of literal + # braces from the JSON and Python it documents. Running str.format() over + # the whole body raised KeyError on `{"role": ...}`, which the old + # `except KeyError` turned into a bogus "missing args" report for every + # no-argument skill that showed a payload, and let the IndexError from + # `{}` escape entirely. Substitute only the declared arguments and leave + # every other brace exactly as the author wrote it. for i, arg_name in enumerate(skill.arguments): if i == len(skill.arguments) - 1: - kwargs[arg_name] = " ".join(args[i:]) + value = " ".join(args[i:]) else: - kwargs[arg_name] = args[i] - try: - return skill.prompt_template.format(**kwargs) - except KeyError: - return None + value = args[i] + template = template.replace("{" + arg_name + "}", value) + return template diff --git a/dashscope/acli/tools/filesystem.py b/dashscope/acli/tools/filesystem.py index a57d181..04c27d6 100644 --- a/dashscope/acli/tools/filesystem.py +++ b/dashscope/acli/tools/filesystem.py @@ -262,11 +262,22 @@ def delete_directory(path: str) -> str: return f"Deleted directory: {path}" +def _pattern_names_hidden_dir(pattern: str) -> bool: + """True when the caller explicitly asked about a dot-directory.""" + return any( + seg.startswith(".") + for seg in pattern.replace(os.sep, "/").split("/") + if seg + ) + + @tool( name="search_files", description=( - "Search files by name pattern. " - "Supports wildcards like *.py, test_*." + "Search files by name or path pattern, relative to `path` " + "(default: the working directory). Supports wildcards like *.py, " + "test_*, src/*.ts and .github/workflows/*.yml. `*` crosses directory " + "separators, so *.md finds every Markdown file at any depth." ), permission=PermissionLevel.AUTO, ) @@ -279,6 +290,8 @@ def search_files(pattern: str, path: str | None = None) -> str: if not os.path.isdir(search_root): return f"Error: search path not found - {search_root}" + needle = pattern.replace(os.sep, "/") + want_hidden = _pattern_names_hidden_dir(pattern) matches = [] root_depth = search_root.rstrip(os.sep).count(os.sep) for root, dirs, files in os.walk(search_root): @@ -286,16 +299,22 @@ def search_files(pattern: str, path: str | None = None) -> str: current_depth = root.rstrip(os.sep).count(os.sep) - root_depth if current_depth >= _MAX_SEARCH_DEPTH: dirs[:] = [] - # Skip hidden and sensitive directories + # Sensitive directories are always skipped. Other dot-directories are + # skipped unless the pattern asks for one, so `.acli/skills/*.md` is + # searchable without walking every `.git` in the tree. dirs[:] = [ d for d in dirs - if not d.startswith(".") and d not in SENSITIVE_NAMES + if d not in SENSITIVE_NAMES + and (want_hidden or not d.startswith(".")) ] for name in files: - if fnmatch.fnmatch(name, pattern): - if name in SENSITIVE_NAMES: - continue + if name in SENSITIVE_NAMES: + continue + rel = os.path.relpath(os.path.join(root, name), search_root) + if fnmatch.fnmatch(rel.replace(os.sep, "/"), needle) or ( + fnmatch.fnmatch(name, needle) + ): matches.append(os.path.join(root, name)) if len(matches) >= 100: break @@ -303,7 +322,11 @@ def search_files(pattern: str, path: str | None = None) -> str: break if not matches: - return f"No files match '{pattern}'" + return ( + f"No files match '{pattern}' under {search_root}. The pattern is " + "matched against paths relative to that root, and dot-directories " + "are skipped unless the pattern names one." + ) result = f"Found {len(matches)} matching files:\n" result += "\n".join(f" {m}" for m in matches) return result diff --git a/dashscope/acli/tools/shell.py b/dashscope/acli/tools/shell.py index 58897fd..7514047 100644 --- a/dashscope/acli/tools/shell.py +++ b/dashscope/acli/tools/shell.py @@ -42,21 +42,58 @@ def default_timeout() -> int: return value if value > 0 else DEFAULT_TIMEOUT -def _resolve_output_encoding() -> str: - """Encoding for decoding subprocess output. +def _resolve_output_encodings() -> tuple[str, ...]: + """Encodings to try when decoding subprocess output, best guess first. Resolved at import, not per call: the TUI replaces ``sys.stdout`` with textual's capture object, which has no ``.encoding``, so reading it later raises AttributeError and fails every command. + + A captured child writes in its own console's code page, which on Windows is + the OEM page and need not match Python's stdout encoding. Decoding a zh-CN + PowerShell error as UTF-8 handed the model mojibake instead of the one + message that explained its failure. UTF-8 stays first and is tried + strictly: it fails loudly on OEM bytes, whereas a DBCS page like cp936 + accepts almost anything and would silently mangle UTF-8 output. """ - if not IS_WINDOWS: - return "utf-8" - return ( - getattr(sys.stdout, "encoding", None) - or getattr(sys.__stdout__, "encoding", None) - or locale.getpreferredencoding(False) - or "utf-8" - ) + candidates = ["utf-8"] + if IS_WINDOWS: + import ctypes + + kernel32 = getattr(ctypes, "windll", None) + oem = kernel32.kernel32.GetOEMCP() if kernel32 is not None else 0 + if oem: + candidates.append(f"cp{oem}") + for extra in ( + getattr(sys.stdout, "encoding", None), + getattr(sys.__stdout__, "encoding", None), + locale.getpreferredencoding(False), + ): + if extra: + candidates.append(extra) + unique: list[str] = [] + for name in candidates: + if name.lower() not in {u.lower() for u in unique}: + unique.append(name) + return tuple(unique) + + +def _decode_output(data: bytes) -> str: + """Decode child output, falling back across console code pages.""" + for encoding in (OUTPUT_ENCODING,) + OUTPUT_ENCODINGS: + try: + return data.decode(encoding) + except LookupError: + continue + except UnicodeDecodeError as exc: + # Truncation is byte-based, so it can split a trailing multi-byte + # character. That is not evidence the whole buffer is in another + # encoding — repair the tail instead of re-decoding everything. + if exc.start >= len(data) - 4: + return data.decode(encoding, errors="replace") + continue + # Last resort only reachable if every candidate name is unusable. + return data.decode("utf-8", errors="replace") def _resolve_win_shell() -> tuple[str, ...]: @@ -69,9 +106,41 @@ def _resolve_win_shell() -> tuple[str, ...]: return (exe, "-NoProfile", "-NonInteractive", "-Command") -OUTPUT_ENCODING = _resolve_output_encoding() +OUTPUT_ENCODINGS = _resolve_output_encodings() +# The primary candidate keeps its own name because callers and tests pin a +# single encoding; _decode_output honours it before trying the fallbacks. +OUTPUT_ENCODING = OUTPUT_ENCODINGS[0] WIN_SHELL = _resolve_win_shell() + +def shell_label() -> str: + """How ``run_command`` executes a command, for the system prompt. + + Lives next to ``_spawn`` so the prompt cannot drift from what the tool + really runs. Nothing used to say which shell this is: a model that assumes + POSIX spends a turn on ``ls -la ... 2>/dev/null || find /`` before a + PowerShell ParserError tells it otherwise. + """ + if not IS_WINDOWS: + return ( + "POSIX sh via `/bin/sh -c` — not the user's interactive shell, " + "so no aliases, rc files or zsh/bash-only syntax" + ) + exe = os.path.basename(WIN_SHELL[0]) + if exe.lower().startswith("pwsh"): + chaining = "`&&`/`||` work (PowerShell 7+)" + else: + chaining = ( + "`&&`/`||` are not statement separators (Windows PowerShell " + "5.1) — use `;` or separate calls" + ) + return ( + f"PowerShell on Windows ({exe}). Write PowerShell, not POSIX: no " + f"`2>/dev/null`, no `ls -la`/`find`/`grep`; {chaining}. Use " + "`$env:USERPROFILE`, `Get-ChildItem`, `Select-String`." + ) + + BLOCKED_PATTERNS = [ # POSIX "mkfs", @@ -712,10 +781,9 @@ async def run_command(command: str, timeout: int | None = None) -> str: output = "" if stdout: - output += stdout.decode(OUTPUT_ENCODING, errors="replace") + output += _decode_output(stdout) if stderr: - err = stderr.decode(OUTPUT_ENCODING, errors="replace") - output += "\n[stderr]\n" + err + output += "\n[stderr]\n" + _decode_output(stderr) if len(output) > MAX_OUTPUT_LENGTH: output = ( diff --git a/dashscope/acli/tools/skills.py b/dashscope/acli/tools/skills.py index 5c0d479..af5c693 100644 --- a/dashscope/acli/tools/skills.py +++ b/dashscope/acli/tools/skills.py @@ -67,7 +67,10 @@ async def use_skill(name: str, args: list[str] | None = None) -> str: f"{skill.mcp_service}; ask the user to connect first." ) rendered = render_skill(skill, [str(a) for a in (args or [])]) - if not rendered: + # `is None`, not falsy: render_skill only returns None when arguments + # are missing, so the hint below always lists at least one name. An + # empty body is a valid (if useless) skill, not a missing argument. + if rendered is None: hint = " ".join(f"<{a}>" for a in skill.arguments) return f"Error: missing args. '{name}' requires: {hint}" agent = get_agent() diff --git a/dashscope/version.py b/dashscope/version.py index 0ed5fd5..42b1753 100644 --- a/dashscope/version.py +++ b/dashscope/version.py @@ -1,4 +1,4 @@ # -*- coding: utf-8 -*- # Copyright (c) Alibaba, Inc. and its affiliates. -__version__ = "1.27.6" +__version__ = "1.27.7" diff --git a/tests/unit/test_acli_skill_render.py b/tests/unit/test_acli_skill_render.py new file mode 100644 index 0000000..9d8e7a0 --- /dev/null +++ b/tests/unit/test_acli_skill_render.py @@ -0,0 +1,143 @@ +# -*- coding: utf-8 -*- +# Copyright (c) Alibaba, Inc. and its affiliates. +"""Every bundled dashscope-sdk-expert skill must render through ``use_skill``. + +This is the dashscope-only half of the ``render_skill`` regression net: the 13 +skill files under ``examples/dashscope-sdk-expert/.acli/skills/`` ship in this +wheel and exist nowhere else, so they are the real corpus that broke. + +``render_skill`` used to run ``str.format()`` over the whole skill body. A body +is Markdown full of literal JSON and Python braces, so ``text-generation.md`` -- +which documents ``messages=[{"role": "user", ...}]`` and declares no arguments +at all -- raised ``KeyError('"role"')``. The old ``except KeyError`` turned that +into ``Error: missing args`` for a skill that takes none, so 6 of the 13 skills +were unusable through ``use_skill`` while ``/skill`` and the catalog kept +advertising them. + +The assertions here are deliberately about observable output rather than about +the substitution mechanism: declared placeholders disappear, their values +arrive, and every other brace comes back byte-identical. +""" + +# pylint: disable=protected-access + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +import dashscope.acli +from dashscope.acli.skills.base import Skill, _parse_skill_md, render_skill + +SKILLS_DIR = ( + Path(dashscope.acli.__file__).parent + / "examples" + / "dashscope-sdk-expert" + / ".acli" + / "skills" +) + +# Distinct per position so a swapped argument order shows up as a missing +# sentinel instead of quietly passing. +SENTINEL = "ZZSENTINEL{}ZZ" + +_BRACE_SPAN = re.compile(r"\{[^{}]*\}") + + +def _bundled_skill_files() -> list[Path]: + return sorted(SKILLS_DIR.glob("*.md")) + + +def _load(path: Path) -> Skill: + skill = _parse_skill_md(path) + assert skill is not None, f"{path.name} did not parse" + return skill + + +def _args_for(skill: Skill) -> list[str]: + return [SENTINEL.format(i) for i in range(len(skill.arguments))] + + +@pytest.fixture(scope="module") +def bundled_skills() -> list[Skill]: + paths = _bundled_skill_files() + assert paths, f"no skill files found under {SKILLS_DIR}" + return [_load(p) for p in paths] + + +def test_the_bundled_corpus_is_the_one_that_ships(bundled_skills): + """Guard against the test going vacuous if ``examples/`` is ever dropped. + + A silent sync that stops copying the skills directory would otherwise leave + every assertion below passing over an empty corpus. + """ + names = {s.name for s in bundled_skills} + assert len(bundled_skills) == 13 + assert names == {path.stem for path in _bundled_skill_files()} + # Both shapes matter: the bug hit no-argument skills carrying JSON hardest, + # while the substitution path is only exercised by the ones with arguments. + assert any(s.arguments for s in bundled_skills) + assert any(not s.arguments for s in bundled_skills) + assert sum(1 for s in bundled_skills if "{" in s.prompt_template) >= 10 + + +@pytest.mark.parametrize("path", _bundled_skill_files(), ids=lambda p: p.stem) +def test_every_skill_renders_without_raising(path): + skill = _load(path) + rendered = render_skill(skill, _args_for(skill)) + assert rendered is not None, f"{skill.name} reported missing args" + assert rendered.strip() + + +@pytest.mark.parametrize("path", _bundled_skill_files(), ids=lambda p: p.stem) +def test_undeclared_braces_are_left_exactly_as_authored(path): + """A JSON sample in the body is documentation, not a placeholder.""" + skill = _load(path) + rendered = render_skill(skill, _args_for(skill)) + assert rendered is not None + + declared = {f"{{{a}}}" for a in skill.arguments} + literals = [ + span + for span in _BRACE_SPAN.findall(skill.prompt_template) + if span not in declared + ] + for span in literals: + assert span in rendered, f"{skill.name} lost {span!r}" + + +@pytest.mark.parametrize("path", _bundled_skill_files(), ids=lambda p: p.stem) +def test_declared_arguments_are_substituted(path): + skill = _load(path) + args = _args_for(skill) + rendered = render_skill(skill, args) + assert rendered is not None + + for name, value in zip(skill.arguments, args): + assert f"{{{name}}}" not in rendered, f"{skill.name} kept {{{name}}}" + assert value in rendered, f"{skill.name} dropped the value of {name}" + + +def test_a_no_argument_skill_renders_to_its_own_body(bundled_skills): + """The exact regression: ``text-generation`` takes no args and shows JSON.""" + skill = next(s for s in bundled_skills if s.name == "text-generation") + assert skill.arguments == [] + assert '{"role": "user", "content": "Hello"}' in skill.prompt_template + assert render_skill(skill, []) == skill.prompt_template + + +def test_too_few_arguments_still_reports_none(bundled_skills): + """``None`` is the missing-args signal; it must stay reachable. + + Both ``use_skill`` and the scheduler branch on ``rendered is None`` to print + the ``requires: `` hint. If rendering always succeeded, that hint would + become dead code and a genuinely short call would send a half-substituted + prompt to the model. + """ + translate = next(s for s in bundled_skills if s.name == "translate") + assert translate.arguments == ["target_lang", "text"] + assert render_skill(translate, []) is None + assert render_skill(translate, ["English"]) is None + assert render_skill(translate, ["English", "hello there"]) is not None From 13baebda29d53ffc95f27d05764fac58024a3b29 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Wed, 23 Sep 2026 23:54:22 +0800 Subject: [PATCH 08/19] test: silence pylint on the two new cli/acli test modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pytest fixtures are injected by name, so every test taking key_store or bundled_skills redefines it from module scope, and the ones that need only the fixture's side effects never read the parameter. Both are the repo's existing convention for test modules; the assertions are unchanged apart from `calls == []` becoming `not calls`. pre-commit run --all-files only started reporting these once the files were tracked — it lints tracked files, so the untracked originals were skipped. --- tests/unit/test_acli_skill_render.py | 2 +- tests/unit/test_cli_auth.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_acli_skill_render.py b/tests/unit/test_acli_skill_render.py index 9d8e7a0..c53da9f 100644 --- a/tests/unit/test_acli_skill_render.py +++ b/tests/unit/test_acli_skill_render.py @@ -19,7 +19,7 @@ arrive, and every other brace comes back byte-identical. """ -# pylint: disable=protected-access +# pylint: disable=protected-access,redefined-outer-name from __future__ import annotations diff --git a/tests/unit/test_cli_auth.py b/tests/unit/test_cli_auth.py index 9d39002..f3997b4 100644 --- a/tests/unit/test_cli_auth.py +++ b/tests/unit/test_cli_auth.py @@ -7,6 +7,9 @@ writes. Every test runs against a throwaway ``~/.dashscope`` and a stubbed ``Models.list``, so no real key file is touched and no request is sent. """ + +# pylint: disable=redefined-outer-name,unused-argument + from http import HTTPStatus from types import SimpleNamespace @@ -99,7 +102,7 @@ def test_no_key_prints_login_guidance(self, monkeypatch, key_store): "DASHSCOPE_API_KEY to configure one." ) assert guidance in text - assert calls == [], "must not reach the API without a key" + assert not calls, "must not reach the API without a key" def test_masks_key_and_reports_the_file_source( self, From 474abb43ca14a2e03c3226bbc073db08634e04f7 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Thu, 24 Sep 2026 10:22:18 +0800 Subject: [PATCH 09/19] docs(acli): reconcile the bundled expert example with agenticCLI-examples sync_acli.sh deliberately preserves dashscope/acli/examples/, so the bundled example config drifts from its upstream repo in agenticCLI-examples and has to be reconciled by hand. Both files had moved independently: - agent.md: upstream gained the 2026-09 error-code pass (request_too_large, permission_error, billing_error, timeout_error, and the note to match on the exception class rather than .code). Take the upstream version. - system-prompt.md: Bailian docs moved to docs.modelstudio.console.alibabacloud.com. Point the product overview at the new domain and add the API-key, app-id/workspace-id and knowledge-base entries, whose titles were verified against the live pages. The remaining help.aliyun.com links are left alone rather than guessed at. The two trees are now byte-identical. --- .../dashscope-sdk-expert/.acli/skills/agent.md | 13 +++++++++---- .../dashscope-sdk-expert/.acli/system-prompt.md | 5 ++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md index 1140350..11e5e6d 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/skills/agent.md @@ -67,16 +67,21 @@ with client.sessions.events.stream(session.id) as stream: | Error code/exception | HTTP status | Meaning | Handling | | --- | --- | --- | --- | | `InputRequired` / `ModelRequired` / `InvalidInput` | raised locally | missing required params such as app_id/prompt/model | add params per the exception message; Application failures do not raise — check status_code | -| `invalid_request_error` (InvalidRequestError) | 400 | invalid agentstudio request parameters | check parameter names/types, e.g. empty events raises a local ValueError | +| `invalid_request_error` / `request_too_large` (InvalidRequestError) | 400/413 | invalid agentstudio request parameters, or payload too large | check parameter names/types, e.g. empty events raises a local ValueError | | `authentication_error` (AuthenticationError) | 401 | api_key missing or invalid | check api_key or DASHSCOPE_API_KEY | -| `permission_denied_error` (PermissionDeniedError) | 403 | no access to the resource | check workspace/uid and resource ownership | +| `permission_denied_error` / `permission_error` (PermissionDeniedError) | 403 | no access to the resource | check workspace/uid and resource ownership | | `not_found_error` (NotFoundError) | 404 | agent/session and other resources do not exist | verify the id and region/workspace are correct | | `conflict_error` (ConflictError) | 409 | version in agents.update does not match the server | retrieve the latest version first, then update | -| `rate_limit_error` (RateLimitError) | 429 | rate limited | retry with backoff (client default max_retries=2) | -| `api_error` (InternalServerError) / `overloaded_error` (OverloadedError) | 500/502/504 / 503 | server error/overloaded | troubleshoot with `request_id` or retry | +| `rate_limit_error` / `billing_error` (RateLimitError) | 429 | rate limited or out of quota | retry with backoff (client default max_retries=2) | +| `api_error` / `timeout_error` (InternalServerError), `overloaded_error` (OverloadedError) | 500/502/504 / 503 | server error/overloaded | troubleshoot with `request_id` or retry | | `APITimeoutError` / `APIConnectionError` | no response | connection/read/write timeout | increase timeout, check network, then retry | | `StreamError` / `StreamClosedError` | mid-stream | SSE protocol error/stream closed | call `events.stream` again to reopen the stream | +Match on the exception class, not on `.code`: `.code` echoes the server's raw +string, so a 403 may arrive as either `permission_denied_error` or +`permission_error`, and it is normalised to `api_error` when the server omits +it. The class is always the one listed above. + ## Java SDK Java SDK v2.22.23 entry points: diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md index d7e814c..698df09 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md @@ -3,7 +3,10 @@ You are DashScope SDK Expert, an intelligent assistant for the DashScope Python Your knowledge base: - Python SDK: https://github.com/dashscope/dashscope-sdk-python - Java SDK: https://github.com/dashscope/dashscope-sdk-java -- Alibaba Cloud Model Studio (Bailian) Product Overview: https://help.aliyun.com/zh/model-studio/what-is-model-studio +- Alibaba Cloud Model Studio (Bailian) Product Overview: https://docs.modelstudio.console.alibabacloud.com/zh/model-studio/what-is-model-studio +- Get and Configure an API Key: https://docs.modelstudio.console.alibabacloud.com/zh/model-studio/get-api-key +- Get the App ID and Workspace ID: https://docs.modelstudio.console.alibabacloud.com/zh/model-studio/obtain-the-app-id-and-workspace-id +- Build a Knowledge-Base Q&A Application with No Code: https://docs.modelstudio.console.alibabacloud.com/zh/model-studio/build-knowledge-base-qa-assistant-without-coding - Model Studio User Guide: https://help.aliyun.com/zh/model-studio/get-started-with-models - Application User Guide: https://help.aliyun.com/zh/model-studio/start-using - Model API Reference: https://help.aliyun.com/zh/model-studio/preparations From db76344d5a748f01a5cf5d787c1f98a04ed38fb1 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Thu, 24 Sep 2026 10:22:42 +0800 Subject: [PATCH 10/19] fix(cli): point the non-TTY refusal at `dashscope expert "question"` When the agent is invoked without a terminal and nothing is piped in, the error told the user to rerun as `dashscope "your question"`. That stopped working in de16112, which routes everything except a bare `dashscope` and an explicit `expert` keyword to typer. So the one message that could have recovered the command line sent the user straight into `No such command 'your question'`. Cover the whole non-TTY branch, which had no tests: piped text becomes a one-shot prompt, an empty pipe exits 2 with the corrected hint, and an unreadable stdin is refused the same way rather than crashing. The hint assertions name both the corrected form and the stale one, so a revert shows up as a failure instead of passing quietly. --- dashscope/cli/__init__.py | 2 +- tests/unit/test_cli_agent_route.py | 80 ++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/dashscope/cli/__init__.py b/dashscope/cli/__init__.py index 3e6ae91..93de0d1 100644 --- a/dashscope/cli/__init__.py +++ b/dashscope/cli/__init__.py @@ -374,7 +374,7 @@ def _route_to_expert(command, tui=None): if not piped: err_console.print( "[red]Error:[/red] Interactive mode requires a terminal. " - 'Pass a prompt instead, e.g. dashscope "your question".', + 'Pass a prompt instead, e.g. dashscope expert "your question".', ) sys.exit(2) command = piped diff --git a/tests/unit/test_cli_agent_route.py b/tests/unit/test_cli_agent_route.py index d44dde4..7da8d57 100644 --- a/tests/unit/test_cli_agent_route.py +++ b/tests/unit/test_cli_agent_route.py @@ -6,6 +6,8 @@ without touching user-owned files. - _route_to_expert: runs embedded acli with the global -k api key passed through, and offers the example download only for interactive no-arg runs. + On non-TTY stdin, piped text becomes a one-shot prompt and an empty pipe + exits 2 telling the user the `expert` keyword form. - _maybe_offer_example_download: accept/decline/marker/workspace gating. - main(): only a bare `dashscope` or an explicit `expert` keyword reach the agent; everything else is dispatched to typer so typos, stray options and @@ -50,6 +52,33 @@ def _make_tty(monkeypatch): monkeypatch.setattr(sys, "stdin", TtyStdin()) +class NonTtyStdin: + """Piped stdin, optionally empty or unreadable.""" + + def __init__(self, data=""): + self._data = data + + def isatty(self): + return False + + def read(self): + if self._data is None: + raise OSError("stdin unavailable") + return self._data + + +@pytest.fixture() +def captured_err(monkeypatch): + lines: list = [] + + class Recorder: + def print(self, *args, **kwargs): + lines.append(" ".join(str(a) for a in args)) + + monkeypatch.setattr(cli, "err_console", Recorder()) + return lines + + class TestCleanupLegacyExpertSync: def test_removes_managed_files(self, fake_config_dir): skills = fake_config_dir / "skills" @@ -160,6 +189,57 @@ def test_cleanup_runs_before_agent( assert not managed.exists() assert captured_run["command"] == "hi" + def test_piped_stdin_becomes_a_one_shot_prompt( + self, + fake_config_dir, + captured_run, + monkeypatch, + ): + monkeypatch.setattr(cli, "_maybe_offer_example_download", lambda: None) + monkeypatch.setattr(sys, "stdin", NonTtyStdin(" 怎么用 Generation \n")) + + cli._route_to_expert(None) + + assert captured_run["command"] == "怎么用 Generation" + + def test_empty_pipe_names_the_expert_keyword( + self, + fake_config_dir, + captured_run, + captured_err, + monkeypatch, + ): + """The refusal is the user's only recovery instruction. + + It told people to run `dashscope "your question"` for two releases + after that form stopped routing to the agent, so the one message that + could have fixed their command line sent them to `No such command`. + """ + monkeypatch.setattr(sys, "stdin", NonTtyStdin(" \n")) + + with pytest.raises(SystemExit) as exc: + cli._route_to_expert(None) + + assert exc.value.code == 2 + assert not captured_run + message = "\n".join(captured_err) + assert 'dashscope expert "your question"' in message + assert 'e.g. dashscope "' not in message + + def test_unreadable_stdin_refuses_the_same_way( + self, + fake_config_dir, + captured_err, + monkeypatch, + ): + monkeypatch.setattr(sys, "stdin", NonTtyStdin(None)) + + with pytest.raises(SystemExit) as exc: + cli._route_to_expert(None) + + assert exc.value.code == 2 + assert 'dashscope expert "your question"' in "\n".join(captured_err) + class TestOfferExampleDownload: def test_accept_downloads_example( From bf1c084aae8c2266f21e69b4963f496e096fd1a4 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Thu, 24 Sep 2026 10:32:21 +0800 Subject: [PATCH 11/19] acli: sync the write_file temp-dir fix from agenticCLI 749999a MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit write_file refused any path outside the cwd, which made it stricter than every layer around it without protecting anything: run_command can already write to temp because the shell is not path-guarded, delete_file reaches anywhere behind its own prompt, and the opt-in OS sandbox confines writes to "the workspace and temp areas". The refusal also cost more than a failed call — models reach for /tmp for scratch files by instinct, so the retry put the scratch file in the user's repo instead, which is the worse outcome. Writes now land in the cwd or the realpath'd system temp dir. Both sides are resolved with realpath, so a symlink inside either root still cannot escape, and validate_path runs first so SENSITIVE_NAMES keeps blocking e.g. /tmp/.ssh/config. The refusal message names both roots generically rather than echoing absolute paths. Per-file sync only: the two files are byte-identical to agenticCLI modulo the acli. -> dashscope.acli. import rewrite. examples/ was preserved by the sync script, and no version is bumped -- agenticCLI stays at 0.6.7. --- dashscope/acli/tools/filesystem.py | 3 +- dashscope/acli/utils/paths.py | 55 +++++++++++++++++++++++++----- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/dashscope/acli/tools/filesystem.py b/dashscope/acli/tools/filesystem.py index 04c27d6..405407f 100644 --- a/dashscope/acli/tools/filesystem.py +++ b/dashscope/acli/tools/filesystem.py @@ -96,7 +96,8 @@ def read_file( name="write_file", description=( "Write content to a file; overwrites if it exists. " - "The result includes a unified diff." + "The result includes a unified diff. `path` must be inside the " + "working directory or the system temp dir." ), permission=PermissionLevel.CONFIRM, ) diff --git a/dashscope/acli/utils/paths.py b/dashscope/acli/utils/paths.py index 41a32e1..d529987 100644 --- a/dashscope/acli/utils/paths.py +++ b/dashscope/acli/utils/paths.py @@ -61,22 +61,61 @@ def validate_path(path: str) -> str: return path +def _temp_write_roots() -> tuple[str, ...]: + """Realpath'd system temp directories a write is allowed to land in. + + ``run_command`` can already write here — the shell is not path-guarded — + ``delete_file`` reaches anywhere behind its own prompt, and the opt-in OS + sandbox confines writes to "the workspace and temp areas". A cwd-only rule + for write_file was therefore stricter than every layer around it without + protecting anything, and it cost more than a refused call: models reach + for /tmp for scratch files by instinct, so the retry put the scratch file + in the user's repo instead, which is the worse outcome. + + ``tempfile.gettempdir()`` is what makes this portable — it honours + TMPDIR/TEMP/TMP, which on macOS is a per-user /var/folders path rather than + /tmp. /tmp is listed separately because that is what a model actually + writes, and realpath is what ties it to /private/tmp there. + """ + import tempfile + + candidates = [tempfile.gettempdir()] + if os.name != "nt": + candidates += ["/tmp", "/private/tmp"] + roots: list[str] = [] + for candidate in candidates: + if not candidate: + continue + try: + resolved = os.path.realpath(candidate) + except OSError: + continue + if resolved not in roots and os.path.isdir(resolved): + roots.append(resolved) + return tuple(roots) + + def validate_write_path(path: str) -> str: """Validate a path for write operations (stricter than read). - In addition to sensitive-path blocking, prevents writes outside cwd. + In addition to sensitive-path blocking, confines writes to the working + directory and the system temp dir. """ path = validate_path(path) # Resolve both sides with realpath: abspath doesn't follow symlinks, so # a symlink inside cwd pointing outside would otherwise pass the check. abs_path = os.path.realpath(path) - cwd = os.path.realpath(".") - if not abs_path.startswith(cwd + os.sep) and abs_path != cwd: - raise ValueError( - f"Write path '{path}' is outside the current working " - "directory (path traversal guard)", - ) - return path + roots = (os.path.realpath("."),) + _temp_write_roots() + inside = any( + abs_path == root or abs_path.startswith(root + os.sep) + for root in roots + ) + if inside: + return path + raise ValueError( + f"Write path '{path}' is outside the writable roots: the working " + "directory and the system temp dir (path traversal guard)", + ) def atomic_write_text(path: Path, text: str, encoding: str = "utf-8") -> None: From 96bd10dc6facff21c9c1ba36d55527893cb078b3 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Thu, 24 Sep 2026 11:18:42 +0800 Subject: [PATCH 12/19] docs(acli): let the expert example answer questions about itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Response Strategy offered three question types — issue analysis, API question, example generation — and the Workflow said to classify into one of them. A question about the assistant matches none of the three, so it fell through to the only behaviour left: probe the environment until an answer turns up. Observed on a real session: "你现在是什么模型" and "你能干嘛" together burned four run_command calls, two of which failed on bash syntax under PowerShell, before producing an answer that was already sitting in the prompt on the Current model line. Nothing about the assistant is discoverable by running a command. Add the fourth branch and make the Workflow route to it first, rather than relying on the anti-pattern list to be re-read: the model was never classifying these as "explore unrelated files to get oriented", so that line never applied. Vendored from agenticCLI-examples 77c5284; the two trees are byte-identical. --- .../dashscope-sdk-expert/.acli/system-prompt.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md b/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md index 698df09..ead0bb1 100644 --- a/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md +++ b/dashscope/acli/examples/dashscope-sdk-expert/.acli/system-prompt.md @@ -35,6 +35,7 @@ Match your approach to the question type: - **Issue / bug analysis**: Analyze directly from the user's description first. Only inspect SDK source when you need to verify a specific behavior. Go straight to the root cause. - **API questions** (signature, parameters, usage): Load the matching skill under `.acli/skills/` (covers Python and Java interfaces). If the user mentions a specific SDK version, verify the signatures against the installed package. - **Example generation**: Cross-check against the installed version, then write runnable code. +- **Questions about you** (what model you are, what you can do, what a skill or slash command means): Answer from this prompt, with **zero tool calls**. Your model name is on the `Current model:` line, your domains and skill list are above, and your capabilities are in this section. Nothing about yourself is discoverable by running a command — probing the environment to answer "what can you do" only makes the user wait for an answer you already had. ### Version Check @@ -64,10 +65,11 @@ When verification is needed, use `run_command` to inspect the installed `dashsco ### Workflow -1. Identify the question type (issue analysis / API question / example). -2. If issue analysis — answer directly, only verify if something is unclear. -3. If API question — run one targeted `inspect.signature` or `help()` call. -4. Compose a concise, actionable answer. +1. Identify the question type (question about you / issue analysis / API question / example). +2. If it is about you — answer straight from this prompt and stop. No tool call. +3. If issue analysis — answer directly, only verify if something is unclear. +4. If API question — run one targeted `inspect.signature` or `help()` call. +5. Compose a concise, actionable answer. ### SDK Reference From 528f57ee520246abc1e1bdae7e0221675fe3de31 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Thu, 24 Sep 2026 11:19:55 +0800 Subject: [PATCH 13/19] acli: sync the redaction and silence-nudge fixes from agenticCLI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes, both from the same reported transcript: 3f40870 — tool output is now redacted on the way into context. save_session already ran sanitize over the persisted history, so the session file was the only thing protected: a key read out of the environment was scrubbed on disk and handed to the model in full, where it stayed in the prompt prefix for the rest of the turn. It runs before truncate_head_tail rather than after, since a character-budget cut can split a token in half and leave a fragment too short for the sk- pattern to match. tool_result_for_display is untouched — that path renders to the user's own terminal. 39d0781 — one reminder per turn after ACLI_SILENT_TOOL_NUDGE (default 6, 0 disables) consecutive tool calls stream no prose. The turn shape the transcript had has no other backstop: StagnationTracker's read-only classifier counts run_command as mutating and so resets its streak, its repeat counters need byte-identical calls, ReflectionTracker needs failures, and the convergence section is oneshot-only. Fires in interactive mode too, which is where the user is watching a spinner and getting nothing. The agenticCLI unit tests covering both are not vendored by sync_acli.sh, so this repo ships them without coverage; verified against agenticCLI's 2728 passing tests. dashscope suite: 1079 passed, 42 skipped. --- dashscope/acli/agent.py | 85 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 75 insertions(+), 10 deletions(-) diff --git a/dashscope/acli/agent.py b/dashscope/acli/agent.py index 86e025f..128c5f9 100644 --- a/dashscope/acli/agent.py +++ b/dashscope/acli/agent.py @@ -42,6 +42,7 @@ UserSupplement, normalize_for_model, sanitize, + sanitize_text, text_of, tool_result_for_display, tool_result_for_history, @@ -66,14 +67,22 @@ def _classify_outcome(successes: int, failures: int) -> str: def _tool_message(tool_call: ToolCall, result: str) -> dict: """Build the history entry answering ``tool_call``. - The restore note must be attached here, at truncation time: later + Redaction happens here, ahead of truncation. ``save_session`` already ran + ``sanitize`` over the on-disk copy, so before this a key read out of the + environment was scrubbed from the session file and still handed to the + model in full — the persistence path was protecting the wrong reader. + Scrubbing after truncation would not work either: ``truncate_head_tail`` + cuts on a character budget and can split a token in half, leaving a + fragment that matches no pattern. + + The restore note must be attached here for the same reason: later shrinking skips a result that already carries an omission marker, so the largest results would end up with no way to be read back. """ return { "role": "tool", "content": tool_result_for_history( - result, + sanitize_text(result), note=restore_note(tool_call.name, tool_call.arguments), ), "name": tool_call.name, @@ -248,6 +257,20 @@ def __init__( ) except ValueError: self.converge_hard_ratio = 0.85 + # Silence nudge: one reminder per turn after N tool calls have gone by + # without any prose reaching the user. Mode-independent on purpose — + # the shape this exists for is an interactive turn of *distinct* + # run_command calls, which every existing backstop misses: + # StagnationTracker's read-only classifier counts run_command as + # mutating and so resets its streak, its repeat counters need + # byte-identical calls, and _convergence_section is oneshot-only. + # Env: ACLI_SILENT_TOOL_NUDGE (0 disables). + try: + self.silent_tool_nudge = int( + os.environ.get("ACLI_SILENT_TOOL_NUDGE", "6"), + ) + except ValueError: + self.silent_tool_nudge = 6 # Hard acceptance gate (oneshot only): run the project's test command # before accepting a final answer, and send the raw output back when # it fails. Built here, ahead of the policy text below, so the prompt @@ -357,6 +380,10 @@ def __init__( self.turn_subagents = 0 self.turn_mcp_calls = 0 self.turn_skills = 0 + # Consecutive tool calls that streamed no prose, and whether the + # silence nudge has already been spent this turn. + self._silent_tool_calls = 0 + self._silent_nudge_injected = False # Explicit /skill invocations to count on the next turn self._pending_skill_names: list[str] = [] @@ -570,6 +597,33 @@ def _convergence_section(self, loop_index: int) -> str: self.converge_hard_ratio, ) + def _silence_section(self) -> str: + """Inject one reminder when tool calls stop producing prose. + + Spent at most once per turn. Repeating it every iteration teaches the + model to skim past it, which is the failure mode prose instructions + already have — the point of a structural backstop is that it fires on + a count rather than on the model's willingness to re-read advice. + + Deliberately not gated on ``oneshot``: the user watching a spinner for + five minutes with no text is the interactive case, and the oneshot + runs already have the convergence and read-only-cap sections. + """ + if self._silent_nudge_injected or not self.silent_tool_nudge: + return "" + if self._silent_tool_calls < self.silent_tool_nudge: + return "" + self._silent_nudge_injected = True + return ( + "\n## ⏸ You have gone quiet\n" + f"{self._silent_tool_calls} tool calls in a row and not one word " + "of it reached the user.\n" + "Before the next call, say in one or two sentences what you are " + "doing and what you have found so far. If the calls were " + "collecting information to answer a question you can already " + "answer, answer it instead of calling again.\n" + ) + async def _recall_memory(self, user_input) -> str: """Search for relevant profile info and format as context. @@ -652,6 +706,8 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: self.turn_tool_calls = 0 self.turn_subagents = 0 self.turn_mcp_calls = 0 + self._silent_tool_calls = 0 + self._silent_nudge_injected = False try: active = get_skill_manager().active_packages(user_input_text) names = [p.name for p in active] + self._pending_skill_names @@ -728,26 +784,28 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: f"messages → {len(self.messages)}[/dim]", ) - # Reflection / stagnation / convergence hints are volatile, so - # they ride at the tail of the request instead of being folded - # into the system message. Rewriting messages_with_system[0] on - # every iteration changed the whole prompt prefix byte-for-byte and - # defeated provider-side prompt caching, where cached input costs - # roughly a tenth of uncached. The system message and the - # accumulated history below it stay append-only and byte-stable. + # Reflection / stagnation / convergence / silence hints are + # volatile, so they ride at the tail of the request instead of + # being folded into the system message. Rewriting + # messages_with_system[0] on every iteration changed the whole + # prompt prefix byte-for-byte and defeated provider-side prompt + # caching, where cached input costs roughly a tenth of uncached. + # The system message and the accumulated history below it stay + # append-only and byte-stable. # # The hint has to be a user message, not a second system message: # the Anthropic converter keeps only the last system message, so a # second one would replace the real prompt outright, and the Tongyi # converter only reads messages[0] as system. # - # All three sections are rebuilt every iteration because tracker + # All four sections are rebuilt every iteration because tracker # state changes as tools run; each returns "" when its condition # is not met, so an empty hint costs no message at all. hint = ( self._reflection_section() + self._stagnation_section() + self._convergence_section(_loop_i) + + self._silence_section() ).strip() request_messages = ( [*messages_with_system, {"role": "user", "content": hint}] @@ -904,6 +962,13 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: tool_calls = [tc for tc in tool_calls if tc.name] self.turn_tool_calls += len(tool_calls) + # Silence is per-iteration, not cumulative over the turn: an + # iteration that streamed prose did talk to the user, so the run + # of unheard calls starts over from the next one. + if full_content.strip(): + self._silent_tool_calls = 0 + else: + self._silent_tool_calls += len(tool_calls) for tc in tool_calls: if tc.name in ("delegate", "subagent_invoke"): self.turn_subagents += 1 From c404dcc45e12ee5df09fe774ba73fa4c6c118a98 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Thu, 24 Sep 2026 18:11:46 +0800 Subject: [PATCH 14/19] acli: sync the memory/skill-loop stage-0+1 changes from agenticCLI Oneshot tool registrations and memory wiring, prompt-composition and hint/token trace stats, create_plan prompt cleanup with the invisible- tool CI guard, loader-compatible skill generator behind the ACLI_EVOLVE_SKILL gate, runtime skill registration with deletion propagation, and concurrent-write hardening. --- dashscope/acli/agent.py | 65 +-- dashscope/acli/cli/runners.py | 57 ++- dashscope/acli/executor.py | 27 +- dashscope/acli/memory/directives_learning.py | 98 ++-- dashscope/acli/memory/experience.py | 9 +- dashscope/acli/memory/reflection.py | 2 +- dashscope/acli/memory/skill_evolution.py | 58 +-- dashscope/acli/memory/tool_chains.py | 4 +- dashscope/acli/memory/trace.py | 13 +- dashscope/acli/skills/__init__.py | 2 + dashscope/acli/skills/base.py | 86 +++- dashscope/acli/tools/evolution.py | 10 +- dashscope/acli/tools/registry.py | 463 ++++++++++--------- dashscope/acli/utils/paths.py | 8 +- 14 files changed, 555 insertions(+), 347 deletions(-) diff --git a/dashscope/acli/agent.py b/dashscope/acli/agent.py index 128c5f9..afc9c1f 100644 --- a/dashscope/acli/agent.py +++ b/dashscope/acli/agent.py @@ -121,46 +121,41 @@ def _budget_burned_slowly(error: BaseException) -> bool: reuse paths seen in training data (e.g. /Users/xxx/...) 3. For clear-intent requests (git commit, read file, search), call tools directly without stating a plan first. For code changes, go straight to - the edit — but you must verify it afterwards (rule 18). Only for + the edit — but you must verify it afterwards (rule 17). Only for irreversible operations (delete, overwrite, force push) or complex multi-step tasks, briefly explain and wait for user confirmation -4. If a task needs several steps (refactor, new feature, multi-file change), - call create_plan with the goal and its steps first, then execute them one - by one, marking each with complete_step — the plan is echoed back to you - as "## Current plan" on every later turn. For a 2-3 step task, skip the - plan and just do it -5. When a tool call fails, do not retry the same call; report the error to +4. When a tool call fails, do not retry the same call; report the error to the user with a suggestion -6. Tools prefixed [MCP:xxx] come from Bailian MCP services; call them directly -7. If the "user profile" contains info, personalize answers with it (e.g. +5. Tools prefixed [MCP:xxx] come from Bailian MCP services; call them directly +6. If the "user profile" contains info, personalize answers with it (e.g. recommend Python solutions for a Python user) -8. When a request matches an "available Skill template" below, prefer that +7. When a request matches an "available Skill template" below, prefer that flow — if the Skill's MCP is connected, call its tools directly; if not, connect first with mcp_connect, then call -9. When a question may involve personal preferences, tech stack, or work +8. When a question may involve personal preferences, tech stack, or work environment, proactively search the profile with memory_search for context -10. If no suitable tool can complete the task, answer from your knowledge; - never call a nonexistent tool -11. To switch model/Provider → call switch_model or switch_provider directly -12. To use cloud services (time, code execution, doc parsing, etc.) → call +9. If no suitable tool can complete the task, answer from your knowledge; + never call a nonexistent tool +10. To switch model/Provider → call switch_model or switch_provider directly +11. To use cloud services (time, code execution, doc parsing, etc.) → call mcp_connect directly -13. To enable/disable capabilities → call capability_enable / +12. To enable/disable capabilities → call capability_enable / capability_disable directly -14. Never reply "please use the /xxx command" — if you have a tool that can +13. Never reply "please use the /xxx command" — if you have a tool that can do it, just do it -15. **Do not use run_command with python3 -c inline scripts to read/write +14. **Do not use run_command with python3 -c inline scripts to read/write files** — it is inefficient. Read with read_file, write with write_file; shell tools like sed/grep are fine to use -16. **If a question can be answered directly (no files, commands, or external +15. **If a question can be answered directly (no files, commands, or external info needed), answer from your knowledge without calling tools** — e.g. self-introduction, explaining concepts, translating text, small talk -17. **Multiple independent subtasks (e.g. "look at these 2 files", several +16. **Multiple independent subtasks (e.g. "look at these 2 files", several unrelated tasks at once) → call delegate_parallel to fan out subagents**, or send multiple delegate calls in the same turn; a single independent but lengthy subtask (whole-file review, multi-file scan) → call subagent_invoke for isolated execution and take back only the conclusion. Do not grind through them serially yourself -18. **Verify code changes before reporting done.** Run the tests that cover +17. **Verify code changes before reporting done.** Run the tests that cover what you touched; if nothing covers it, add a focused test for the new behaviour and run that. Report the command and its pass/fail result — never claim a change works without having executed it. When the output @@ -801,12 +796,17 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: # All four sections are rebuilt every iteration because tracker # state changes as tools run; each returns "" when its condition # is not met, so an empty hint costs no message at all. - hint = ( - self._reflection_section() - + self._stagnation_section() - + self._convergence_section(_loop_i) - + self._silence_section() - ).strip() + hint_parts = { + "reflection": self._reflection_section(), + "stagnation": self._stagnation_section(), + "convergence": self._convergence_section(_loop_i), + "silence": self._silence_section(), + } + hint = "".join(hint_parts.values()).strip() + # Which behaviour hints fired this call, and their sizes. Kept + # out of the prompt and logged with the llm_call event so hint + # efficacy is measurable from traces instead of guessed. + hint_chars = {k: len(v) for k, v in hint_parts.items() if v} request_messages = ( [*messages_with_system, {"role": "user", "content": hint}] if hint @@ -928,10 +928,18 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: raise # Log LLM call trace - self.executor.record_prompt_composition( + prompt_stats = self.executor.record_prompt_composition( request_messages, tools_schema, ) + # Char-based estimate logged next to the provider's usage in the + # same event, so the two token accounts can be reconciled from + # traces instead of drifting silently apart. + prompt_stats["est_prompt_tokens"] = estimate_message_tokens( + request_messages, + ) + estimate_tokens(json.dumps(tools_schema, ensure_ascii=False)) + if hint_chars: + prompt_stats["hint_chars"] = hint_chars self.trace_logger.log_llm_call( messages=request_messages, tools=tools_schema, @@ -949,6 +957,7 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: }, duration_ms=int((time.monotonic() - llm_start) * 1000), model=self.model_name, + prompt_stats=prompt_stats, ) # If this iteration carried a reflection hint, its answer is the diff --git a/dashscope/acli/cli/runners.py b/dashscope/acli/cli/runners.py index 11cdab8..a388ce3 100644 --- a/dashscope/acli/cli/runners.py +++ b/dashscope/acli/cli/runners.py @@ -148,12 +148,32 @@ async def _run_oneshot(config: Config, prompt: str): auto_approve=config.auto_approve, confirm_mode=config.confirm_mode, ) + + # Memory and the shared hook bus must exist before the Agent does (same + # wiring as cli/repl.py): without them oneshot had no memory provider at + # all, so every memory- or skill-shaped capability scored a constant zero + # in non-interactive runs. + memory = None + if config.memory_enabled: + from dashscope.acli.cli.handlers_profile import set_memory_client + from dashscope.acli.platforms import get_memory_provider + + memory = get_memory_provider(config) + if memory: + set_memory_client(memory) + + from dashscope.acli.hooks import create_hook_bus + + hook_bus = create_hook_bus() + agent = Agent( provider=provider, executor=executor, max_turns=config.max_turns, + memory=memory, provider_name=config.provider, model_name=config.model, + hook_bus=hook_bus, oneshot=True, ) @@ -178,11 +198,46 @@ async def _run_oneshot(config: Config, prompt: str): set_delegate_parent(agent) set_delegate_config(config) - from dashscope.acli.cli.mcp import _connect_mcp + from dashscope.acli.cli.mcp import ( + _connect_mcp, + _disconnect_mcp, + _mcp_clients, + ) from dashscope.acli.tools.platform import register_platform_tools register_platform_tools(config, connect_mcp_fn=_connect_mcp) + # The four registrations below mirror the interactive/TUI paths + # (cli/repl.py, _run_tui_mode). Oneshot used to skip them, which left + # skill packages unloaded and the session/skill/evolution tools + # unregistered in exactly the mode benchmarks run in. + from dashscope.acli.skills import get_skill_manager + + skill_manager = get_skill_manager() + skill_manager._registry_url = getattr(config, "skill_registry", "") + skill_manager._global = False + skill_manager.load(hook_bus=hook_bus) + + from dashscope.acli.tools.session import register_session_tools + + register_session_tools( + config, + get_agent=lambda: agent, + get_provider_fn=get_provider, + connect_mcp_fn=_connect_mcp, + disconnect_mcp_fn=_disconnect_mcp, + list_mcp_services_fn=list_known_services, + get_mcp_clients_fn=lambda: _mcp_clients, + ) + + from dashscope.acli.tools.evolution import register_evolution_tools + + register_evolution_tools(get_agent=lambda: agent) + + from dashscope.acli.tools.skills import register_skill_tools + + register_skill_tools(get_agent=lambda: agent) + expanded, images, audio_clips = _expand_at_references(prompt) if images and not is_vision_model(config.model): console.print( diff --git a/dashscope/acli/executor.py b/dashscope/acli/executor.py index e1b8e16..95079d0 100644 --- a/dashscope/acli/executor.py +++ b/dashscope/acli/executor.py @@ -308,8 +308,21 @@ def record_prompt_composition( self, messages: list[dict], tools_schema: list[dict] | None = None, - ) -> None: - """Accumulate prompt size (chars) by category for one LLM call.""" + ) -> dict[str, int]: + """Accumulate prompt size (chars) by category for one LLM call. + + Also returns the per-call breakdown so the trace can persist what + this specific request was made of; the accumulator feeding /stats + keeps its coarser system/user/assistant/tools buckets. + """ + per_call = { + "system_chars": 0, + "user_chars": 0, + "assistant_chars": 0, + "tool_result_chars": 0, + "tool_call_args_chars": 0, + "tools_schema_chars": 0, + } for msg in messages: role = msg.get("role") content = msg.get("content") @@ -321,16 +334,20 @@ def record_prompt_composition( n = 0 if role in ("system", "user", "assistant"): self._prompt_composition[role] += n + per_call[f"{role}_chars"] += n elif role == "tool": self._prompt_composition["tools"] += n + per_call["tool_result_chars"] += n if role == "assistant": for tc in msg.get("tool_calls") or []: args = (tc.get("function") or {}).get("arguments") or "" self._prompt_composition["tools"] += len(args) + per_call["tool_call_args_chars"] += len(args) if tools_schema: - self._prompt_composition["tools"] += len( - json.dumps(tools_schema, ensure_ascii=False), - ) + n = len(json.dumps(tools_schema, ensure_ascii=False)) + self._prompt_composition["tools"] += n + per_call["tools_schema_chars"] += n + return per_call async def execute(self, tool_def: ToolDefinition, arguments: dict) -> str: """Run one tool call and return its text result. diff --git a/dashscope/acli/memory/directives_learning.py b/dashscope/acli/memory/directives_learning.py index 3595015..cc6cb9c 100644 --- a/dashscope/acli/memory/directives_learning.py +++ b/dashscope/acli/memory/directives_learning.py @@ -17,6 +17,7 @@ from __future__ import annotations import json +import threading from collections import Counter from datetime import datetime, timezone from pathlib import Path @@ -36,6 +37,11 @@ def _empty_patterns() -> dict[str, Any]: # The patterns file is read on every prompt assembly; memoize on mtime. _patterns_cache: tuple[Path, int, dict[str, Any]] | None = None +# Held across the whole load→mutate→save of every writer below: without it +# two threads recording a sequence (or accepting a directive) interleave +# their read-modify-write and silently drop one side's update. +_cache_lock = threading.Lock() + def _load_patterns() -> dict[str, Any]: global _patterns_cache @@ -97,19 +103,20 @@ def record_tool_sequence(tools: list[str]) -> None: if len(tools) < 2: return - patterns = _load_patterns() - patterns["sequences"].append( - { - "tools": tools, - "timestamp": datetime.now(timezone.utc).isoformat(), - }, - ) + with _cache_lock: + patterns = _load_patterns() + patterns["sequences"].append( + { + "tools": tools, + "timestamp": datetime.now(timezone.utc).isoformat(), + }, + ) - # Keep only last 100 sequences - if len(patterns["sequences"]) > 100: - patterns["sequences"] = patterns["sequences"][-100:] + # Keep only last 100 sequences + if len(patterns["sequences"]) > 100: + patterns["sequences"] = patterns["sequences"][-100:] - _save_patterns(patterns) + _save_patterns(patterns) def analyze_patterns() -> list[dict[str, Any]]: @@ -179,18 +186,19 @@ def _generate_directive(tool1: str, tool2: str, count: int) -> str: def propose_directive(directive: str, rationale: str) -> dict[str, Any]: """Create a directive proposal.""" - patterns = _load_patterns() + with _cache_lock: + patterns = _load_patterns() - proposal = { - "id": f"dir_{int(datetime.now(timezone.utc).timestamp())}", - "directive": directive, - "rationale": rationale, - "status": "pending", - "created_at": datetime.now(timezone.utc).isoformat(), - } + proposal = { + "id": f"dir_{int(datetime.now(timezone.utc).timestamp())}", + "directive": directive, + "rationale": rationale, + "status": "pending", + "created_at": datetime.now(timezone.utc).isoformat(), + } - patterns["proposed_directives"].append(proposal) - _save_patterns(patterns) + patterns["proposed_directives"].append(proposal) + _save_patterns(patterns) return proposal @@ -207,38 +215,40 @@ def list_proposed_directives(status: str = "pending") -> list[dict[str, Any]]: def accept_directive(proposal_id: str, config: Any) -> bool: """Accept a proposed directive and add it to user_directives.""" - patterns = _load_patterns() - proposals = patterns.get("proposed_directives", []) + with _cache_lock: + patterns = _load_patterns() + proposals = patterns.get("proposed_directives", []) - for p in proposals: - if p["id"] == proposal_id and p.get("status") == "pending": - directive_text = p["directive"] + for p in proposals: + if p["id"] == proposal_id and p.get("status") == "pending": + directive_text = p["directive"] - # Add to config.user_directives - if hasattr(config, "user_directives"): - if directive_text not in config.user_directives: - config.user_directives.append(directive_text) - config.save_workspace() + # Add to config.user_directives + if hasattr(config, "user_directives"): + if directive_text not in config.user_directives: + config.user_directives.append(directive_text) + config.save_workspace() - p["status"] = "accepted" - p["accepted_at"] = datetime.now(timezone.utc).isoformat() - _save_patterns(patterns) - return True + p["status"] = "accepted" + p["accepted_at"] = datetime.now(timezone.utc).isoformat() + _save_patterns(patterns) + return True return False def reject_directive(proposal_id: str) -> bool: """Reject a proposed directive.""" - patterns = _load_patterns() - proposals = patterns.get("proposed_directives", []) - - for p in proposals: - if p["id"] == proposal_id and p.get("status") == "pending": - p["status"] = "rejected" - p["rejected_at"] = datetime.now(timezone.utc).isoformat() - _save_patterns(patterns) - return True + with _cache_lock: + patterns = _load_patterns() + proposals = patterns.get("proposed_directives", []) + + for p in proposals: + if p["id"] == proposal_id and p.get("status") == "pending": + p["status"] = "rejected" + p["rejected_at"] = datetime.now(timezone.utc).isoformat() + _save_patterns(patterns) + return True return False diff --git a/dashscope/acli/memory/experience.py b/dashscope/acli/memory/experience.py index 781e42b..20f84bf 100644 --- a/dashscope/acli/memory/experience.py +++ b/dashscope/acli/memory/experience.py @@ -7,8 +7,10 @@ from __future__ import annotations import json +import os from pathlib import Path from typing import Any +from uuid import uuid4 from dashscope.acli.utils.keywords import ( expand_scoring_terms, @@ -60,7 +62,12 @@ def _rotate_if_oversize(self) -> None: encoding="utf-8", ).splitlines() keep = [line for line in lines if line.strip()][-self.KEEP_LINES :] - tmp = self.experience_file.with_suffix(".jsonl.tmp") + # Unique temp name: two trackers rotating at once must not + # clobber each other's temp file before the replace. + tmp = self.experience_file.with_name( + f"{self.experience_file.name}.{os.getpid()}." + f"{uuid4().hex[:8]}.tmp", + ) tmp.write_text("\n".join(keep) + "\n", encoding="utf-8") tmp.replace(self.experience_file) except OSError: diff --git a/dashscope/acli/memory/reflection.py b/dashscope/acli/memory/reflection.py index a89019d..7022faf 100644 --- a/dashscope/acli/memory/reflection.py +++ b/dashscope/acli/memory/reflection.py @@ -626,7 +626,7 @@ def _replan_demand(self, hard_cap: int | None = None) -> str: "2. Which approaches are ruled out, and by which output?", ( "3. What is the revised plan? If it is worth writing " - "down, call create_plan." + "down, write it down." ), ( "A structurally different call, or one specific question " diff --git a/dashscope/acli/memory/skill_evolution.py b/dashscope/acli/memory/skill_evolution.py index b9a3f32..cb4dfc2 100644 --- a/dashscope/acli/memory/skill_evolution.py +++ b/dashscope/acli/memory/skill_evolution.py @@ -125,10 +125,6 @@ def _identify_pattern(tool_sequence: list[str]) -> str | None: return "edit-and-test" return "file-edit" - # Code refactoring workflow - if "read_file" in tool_sequence and "edit_file" in tool_sequence: - return "code-refactor" - # Project setup workflow if "run_command" in tool_sequence and "write_file" in tool_sequence: if any("init" in t or "create" in t for t in tool_sequence): @@ -139,10 +135,6 @@ def _identify_pattern(tool_sequence: list[str]) -> str | None: if "read_file" in tool_sequence: return "search-and-analyze" - # Git workflow - if any("git" in t for t in tool_sequence): - return "git-workflow" - return None @@ -176,39 +168,49 @@ def _extract_parameters(user_request: str) -> list[dict[str, str]]: def generate_skill_markdown(analysis: dict[str, Any]) -> str: - """Generate skill markdown from trajectory analysis.""" + """Generate skill markdown from trajectory analysis. + + Frontmatter stays within what the loader's _parse_simple_yaml can read: + one entry per line, lists in ``[a, b]`` form. The previous nested YAML + ``arguments:`` block parsed as garbage keys, and its inner + ``description:`` lines clobbered the real top-level description. + """ name = analysis["pattern_name"] tools = analysis["tools_sequence"] params = analysis["parameters"] - # Build skill markdown + arg_names = [p["name"] for p in params] lines = [ "---", f"name: {name}", f"description: Automated workflow - {analysis['rationale']}", - "arguments:", + f"arguments: [{', '.join(arg_names)}]", + "---", + "", + f"# {name}", + "", + "Automatically run the following workflow:", + "", ] - - for p in params: - lines.append(f" - name: {p['name']}") - lines.append(f" description: {p['description']}") - lines.append(" required: true") - if "example" in p: - lines.append(f" example: {p['example']}") - - lines.append("---") - lines.append("") - lines.append(f"# {name}") - lines.append("") - lines.append("Automatically run the following workflow:") - lines.append("") for i, tool in enumerate(tools, 1): lines.append(f"{i}. `{tool}`") lines.append("") + + if params: + lines.append("## Arguments") + lines.append("") + for p in params: + entry = f"- `{p['name']}` — {p['description']}" + if "example" in p: + entry += f" (e.g. {p['example']})" + lines.append(entry) + lines.append("") + lines.append("## Usage example") lines.append("") lines.append("```") - lines.append(f"/{name} {params[0]['example'] if params else ''}") + example_args = " ".join(p.get("example", p["name"]) for p in params) + lines.append(f"/{name} {example_args}" if example_args else f"/{name}") lines.append("```") lines.append("") lines.append("## Workflow") @@ -218,6 +220,10 @@ def generate_skill_markdown(analysis: dict[str, Any]) -> str: for i, tool in enumerate(tools, 1): lines.append(f"{i}. Call the `{tool}` tool") lines.append("") + if arg_names: + refs = ", ".join(f"`{{{n}}}`" for n in arg_names) + lines.append(f"Apply the workflow to {refs}.") + lines.append("") lines.append("Report the results when done.") return "\n".join(lines) diff --git a/dashscope/acli/memory/tool_chains.py b/dashscope/acli/memory/tool_chains.py index 5fcfa39..f6b4caf 100644 --- a/dashscope/acli/memory/tool_chains.py +++ b/dashscope/acli/memory/tool_chains.py @@ -33,13 +33,13 @@ "steps": [ "1. search_files to find all usages", "2. read_file to read the relevant code", - "3. create_plan for the refactor plan", + "3. plan the order of edits before touching files", "4. write_file to edit files one by one", "5. run_command to run tests and verify", ], "example": ( "User: 'refactor the auth module, extract shared functions'\n" - "→ search_files 'def auth' → read_file → create_plan " + "→ search_files 'def auth' → read_file " "→ write_file → run_command pytest" ), }, diff --git a/dashscope/acli/memory/trace.py b/dashscope/acli/memory/trace.py index ff41e20..fa6f0a3 100644 --- a/dashscope/acli/memory/trace.py +++ b/dashscope/acli/memory/trace.py @@ -7,6 +7,7 @@ from __future__ import annotations import json +import os import time from datetime import datetime from pathlib import Path @@ -19,7 +20,12 @@ class TraceLogger: def __init__(self, workspace_dir: Path): self.traces_dir = workspace_dir / "traces" self.traces_dir.mkdir(parents=True, exist_ok=True) - self.session_id = datetime.now().strftime("%Y%m%d_%H%M%S") + # pid suffix: two acli processes started in the same second (parent + + # subagent, parallel runs) would otherwise share one trace file and + # interleave their JSONL lines. + self.session_id = ( + f"{datetime.now().strftime('%Y%m%d_%H%M%S')}_{os.getpid()}" + ) self.trace_file = self.traces_dir / f"{self.session_id}.jsonl" self._session_start = time.time() @@ -47,11 +53,14 @@ def log_llm_call( response: dict, duration_ms: int = 0, model: str = "", + prompt_stats: dict | None = None, ) -> None: """Log an LLM API call. Lightweight fields (counts, usage, duration) are always recorded; the full prompt payload is included only in debug mode (/debug on). + ``prompt_stats`` carries the per-call section char counts and token + estimate (always on — sizes only, never content). """ data: dict[str, Any] = { "model": model, @@ -62,6 +71,8 @@ def log_llm_call( "tool_calls": response.get("tool_calls", []), "usage": response.get("usage"), } + if prompt_stats: + data["prompt_stats"] = prompt_stats from dashscope.acli import debuglog if debuglog.debug_enabled(): diff --git a/dashscope/acli/skills/__init__.py b/dashscope/acli/skills/__init__.py index 2fac2f6..58c7c5a 100644 --- a/dashscope/acli/skills/__init__.py +++ b/dashscope/acli/skills/__init__.py @@ -9,6 +9,7 @@ list_skills, load_skill_files, register, + register_skill_file, render_skill, skills_summary_for_llm, ) @@ -28,6 +29,7 @@ "list_skills", "load_skill_files", "register", + "register_skill_file", "render_skill", "skills_summary_for_llm", ] diff --git a/dashscope/acli/skills/base.py b/dashscope/acli/skills/base.py index 2cc9fbc..3aead59 100644 --- a/dashscope/acli/skills/base.py +++ b/dashscope/acli/skills/base.py @@ -3,6 +3,8 @@ import json import re +import shutil +import threading from dataclasses import dataclass from pathlib import Path @@ -18,37 +20,89 @@ class Skill: BUILTIN_SKILLS: dict[str, Skill] = {} +# Guards every mutation of BUILTIN_SKILLS. RLock because load_skill_files() +# re-enters register() while holding it. +_skills_lock = threading.RLock() + +# Names that came from .md files, as opposed to register() calls from +# custom-extensions.toml or dev tooling. Only these are cleared on rescan: +# apply_extensions() registers toml skills BEFORE load_skill_files() runs, +# so clearing everything would wipe them. +_FILE_SKILL_NAMES: set[str] = set() + def register(skill: Skill): - BUILTIN_SKILLS[skill.name] = skill + with _skills_lock: + BUILTIN_SKILLS[skill.name] = skill def load_skill_files(): """Load skills from .md files in ~/.acli/skills/ and .acli/skills/. - Global files load first; workspace files override by name. + Global files load first; workspace files override by name. File-sourced + entries from the previous scan are dropped first, so deleting a skill + file propagates without a restart. """ from dashscope.acli.config import CONFIG_DIR, WORKSPACE_DIR global_dir = CONFIG_DIR / "skills" workspace_dir = WORKSPACE_DIR / "skills" - for skills_dir in (global_dir, workspace_dir): - if not skills_dir.is_dir(): - continue - for path in sorted(skills_dir.glob("*.md")): - try: - skill = _parse_skill_md(path) + with _skills_lock: + for name in _FILE_SKILL_NAMES: + BUILTIN_SKILLS.pop(name, None) + _FILE_SKILL_NAMES.clear() + + for skills_dir in (global_dir, workspace_dir): + if not skills_dir.is_dir(): + continue + for path in sorted(skills_dir.glob("*.md")): + try: + skill = _parse_skill_md(path) + except Exception as e: + import logging + + logging.getLogger(__name__).warning( + "Skipping unparseable skill file %s: %s", + path, + e, + ) + continue if skill: register(skill) - except Exception as e: - import logging - - logging.getLogger(__name__).warning( - "Skipping unparseable skill file %s: %s", - path, - e, - ) + _FILE_SKILL_NAMES.add(skill.name) + + +def register_skill_file(path: Path | str) -> bool: + """Register one skill .md at runtime, without waiting for a restart. + + Copies *path* into the workspace skills dir when it lives elsewhere, + then rescans so the catalog, /skill completion, and the prompt summary + all see the change immediately. Returns True when the file parsed and + its skill is registered after the rescan. + """ + from dashscope.acli.config import WORKSPACE_DIR + + src = Path(path) + try: + skill = _parse_skill_md(src) + except Exception: + return False + if skill is None: + return False + + skills_dir = WORKSPACE_DIR / "skills" + target = skills_dir / src.name + try: + if src.resolve() != target.resolve(): + skills_dir.mkdir(parents=True, exist_ok=True) + shutil.copyfile(src, target) + except OSError: + return False + + load_skill_files() + with _skills_lock: + return BUILTIN_SKILLS.get(skill.name) is not None def _parse_skill_md(path: Path) -> Skill | None: diff --git a/dashscope/acli/tools/evolution.py b/dashscope/acli/tools/evolution.py index c50eb80..8cb6c1b 100644 --- a/dashscope/acli/tools/evolution.py +++ b/dashscope/acli/tools/evolution.py @@ -65,11 +65,19 @@ async def evolve_skill(force: bool = False) -> str: "or generation failed" ) + from dashscope.acli.skills import register_skill_file + + registered = register_skill_file(skill_path) + availability = ( + "Registered and available right now" + if registered + else "Saved; will load on next start" + ) return ( f"✓ Generated new skill from this conversation\n" f" Name: {analysis['pattern_name']}\n" f" Path: {skill_path}\n" f" Workflow: {' → '.join(analysis['tools_sequence'][:3])}\n\n" - f"Auto-loaded on next start. " + f"{availability}. " f"Trigger with /{analysis['pattern_name']}." ) diff --git a/dashscope/acli/tools/registry.py b/dashscope/acli/tools/registry.py index 4cc342f..9b8de94 100644 --- a/dashscope/acli/tools/registry.py +++ b/dashscope/acli/tools/registry.py @@ -3,6 +3,7 @@ from __future__ import annotations import inspect +import os import sys import types from dataclasses import dataclass, field @@ -97,6 +98,242 @@ def _build_parameters_schema(func: Callable) -> dict: }, ) +# Core filesystem/shell tools are offered on every call. +_CORE_TOOLS = frozenset( + { + "read_file", + "write_file", + "list_directory", + "search_files", + "create_directory", + "delete_file", + "delete_directory", + "move_file", + "run_command", + "web_search", + }, +) + +# Session tools offered on every call, plus use_skill (model-invocable +# skill templates from .acli/skills/*.md). +_SESSION_ALWAYS_ON = frozenset( + { + "switch_model", + "switch_provider", + "capability_enable", + "capability_disable", + "view_image", + "mcp_connect", + "mcp_disconnect", + "use_skill", + }, +) + +# bailian_* CLI wrappers are numerous (~25 schemas each with full +# parameter definitions); offering them only when the request mentions +# the matching capability keeps every unrelated call much smaller. +_B_ALL = [ + "bailian_text_chat", + "bailian_omni", + "bailian_image_generate", + "bailian_image_edit", + "bailian_video_generate", + "bailian_video_edit", + "bailian_video_ref", + "bailian_video_task_get", + "bailian_video_download", + "bailian_vision_describe", + "bailian_app_call", + "bailian_app_list", + "bailian_memory_add", + "bailian_memory_search", + "bailian_memory_list", + "bailian_memory_update", + "bailian_memory_delete", + "bailian_memory_profile_create", + "bailian_memory_profile_get", + "bailian_knowledge_retrieve", + "bailian_search_web", + "bailian_speech_synthesize", + "bailian_speech_recognize", + "bailian_file_upload", + "bailian_model_list", + "bailian_console_call", + "bailian_usage_free", +] +_B_IMAGE = [ + "bailian_image_generate", + "bailian_image_edit", + "bailian_vision_describe", + "bailian_omni", +] +_B_VIDEO = [ + "bailian_video_generate", + "bailian_video_edit", + "bailian_video_ref", + "bailian_video_task_get", + "bailian_video_download", +] +_B_SPEECH = ["bailian_speech_synthesize", "bailian_speech_recognize"] +_B_MEMORY = [ + "bailian_memory_add", + "bailian_memory_search", + "bailian_memory_list", + "bailian_memory_update", + "bailian_memory_delete", + "bailian_memory_profile_create", + "bailian_memory_profile_get", +] + +# Keyword → tool mapping for conditional inclusion. +# Chinese keywords are kept for CJK input; English equivalents +# are appended alongside them so English prompts match too. +_KEYWORD_TOOLS = { + "memory": ["memory_search", "memory_store"] + _B_MEMORY, + "档案": ["memory_search", "memory_store"], + "profile": ["memory_search", "memory_store"], + "偏好": ["memory_search", "memory_store"], + "preference": ["memory_search", "memory_store"], + "记忆": _B_MEMORY, + "数据": [ + "data_upload", + "data_files", + "data_delete", + "data_categories", + ], + "data": [ + "data_upload", + "data_files", + "data_delete", + "data_categories", + ], + "prompt": [ + "prompt_list", + "prompt_get", + "prompt_create", + "prompt_delete", + "prompt_render", + ], + "模板": [ + "prompt_list", + "prompt_get", + "prompt_create", + "prompt_delete", + "prompt_render", + ], + "template": [ + "prompt_list", + "prompt_get", + "prompt_create", + "prompt_delete", + "prompt_render", + ], + "搜索": ["web_search", "context_search"], + "search": ["web_search", "context_search"], + "模型": ["switch_model", "switch_provider"], + "model": ["switch_model", "switch_provider"], + "能力": ["capability_enable", "capability_disable"], + "capability": ["capability_enable", "capability_disable"], + # Browser / web scraping tools + "scrape": [ + "scrape_web", + "scrape_web_html", + "scrape_web_screenshot", + ], + "抓取": [ + "scrape_web", + "scrape_web_html", + "scrape_web_screenshot", + ], + "网页": [ + "scrape_web", + "scrape_web_html", + "scrape_web_screenshot", + ], + "webpage": [ + "scrape_web", + "scrape_web_html", + "scrape_web_screenshot", + ], + "截图": [ + "scrape_web", + "scrape_web_html", + "scrape_web_screenshot", + ], + "screenshot": [ + "scrape_web", + "scrape_web_html", + "scrape_web_screenshot", + ], + "browser": [ + "scrape_web", + "scrape_web_html", + "scrape_web_screenshot", + ], + "爬取": [ + "scrape_web", + "scrape_web_html", + "scrape_web_screenshot", + ], + "crawl": [ + "scrape_web", + "scrape_web_html", + "scrape_web_screenshot", + ], + # Camera tools + "拍照": ["camera_capture", "camera_record"], + "photo": ["camera_capture", "camera_record"], + "摄像头": ["camera_capture", "camera_record"], + "录屏": ["camera_capture", "camera_record"], + "录制": ["camera_capture", "camera_record"], + "record": ["camera_capture", "camera_record"], + "camera": ["camera_capture", "camera_record"], + # Bailian platform tools (schemas are keyword-gated, see _B_* above) + "图片": _B_IMAGE, + "画图": _B_IMAGE, + "draw": _B_IMAGE, + "image": _B_IMAGE, + "vision": _B_IMAGE, + "多模态": ["bailian_omni"], + "multimodal": ["bailian_omni"], + "omni": ["bailian_omni"], + "视频": _B_VIDEO, + "video": _B_VIDEO, + "语音": _B_SPEECH, + "speech": _B_SPEECH, + "tts": ["bailian_speech_synthesize"], + "朗读": ["bailian_speech_synthesize"], + "read aloud": ["bailian_speech_synthesize"], + "知识库": ["bailian_knowledge_retrieve"], + "knowledge": ["bailian_knowledge_retrieve"], + "rag": ["bailian_knowledge_retrieve"], + "百炼应用": ["bailian_app_call", "bailian_app_list"], + "bailian app": ["bailian_app_call", "bailian_app_list"], + "用量": ["bailian_usage_free"], + "usage": ["bailian_usage_free"], + "quota": ["bailian_usage_free"], + "上传": ["bailian_file_upload"], + "upload": ["bailian_file_upload"], + # Catch-all: mentioning the platform unlocks the full set + "百炼": _B_ALL, + "bailian": _B_ALL, +} + + +def ever_visible_tool_names() -> frozenset[str]: + """Every tool name any to_schema_list call can offer statically. + + Union of the always-on sets and all keyword-gated values. Capability + extension tools ({cap}_{tool}) and mcp_* are dynamic and deliberately + excluded — base prompts never address them by name. Used by the + consistency guard that fails CI when a base prompt instructs the model + to call a tool it can never see. + """ + names = set(_CORE_TOOLS) | _SESSION_ALWAYS_ON | _CAPABILITY_AGENT_TOOLS + for gated in _KEYWORD_TOOLS.values(): + names.update(gated) + return frozenset(names) + Tool = ToolDefinition # alias for backwards-compatibility @@ -154,227 +391,15 @@ def to_schema_list(self, user_input: str = "") -> list[dict]: Specialized tools (platform, MCP) are included if keywords match.""" result = [] - # Core tools always included - core_tools = { - "read_file", - "write_file", - "list_directory", - "search_files", - "create_directory", - "delete_file", - "delete_directory", - "move_file", - "run_command", - "web_search", - } - - # bailian_* CLI wrappers are numerous (~25 schemas each with full - # parameter definitions); offering them only when the request mentions - # the matching capability keeps every unrelated call much smaller. - b_all = [ - "bailian_text_chat", - "bailian_omni", - "bailian_image_generate", - "bailian_image_edit", - "bailian_video_generate", - "bailian_video_edit", - "bailian_video_ref", - "bailian_video_task_get", - "bailian_video_download", - "bailian_vision_describe", - "bailian_app_call", - "bailian_app_list", - "bailian_memory_add", - "bailian_memory_search", - "bailian_memory_list", - "bailian_memory_update", - "bailian_memory_delete", - "bailian_memory_profile_create", - "bailian_memory_profile_get", - "bailian_knowledge_retrieve", - "bailian_search_web", - "bailian_speech_synthesize", - "bailian_speech_recognize", - "bailian_file_upload", - "bailian_model_list", - "bailian_console_call", - "bailian_usage_free", - ] - b_image = [ - "bailian_image_generate", - "bailian_image_edit", - "bailian_vision_describe", - "bailian_omni", - ] - b_video = [ - "bailian_video_generate", - "bailian_video_edit", - "bailian_video_ref", - "bailian_video_task_get", - "bailian_video_download", - ] - b_speech = ["bailian_speech_synthesize", "bailian_speech_recognize"] - b_memory = [ - "bailian_memory_add", - "bailian_memory_search", - "bailian_memory_list", - "bailian_memory_update", - "bailian_memory_delete", - "bailian_memory_profile_create", - "bailian_memory_profile_get", - ] - - # Keyword → tool mapping for conditional inclusion. - # Chinese keywords are kept for CJK input; English equivalents - # are appended alongside them so English prompts match too. - keyword_tools = { - "memory": ["memory_search", "memory_store"] + b_memory, - "档案": ["memory_search", "memory_store"], - "profile": ["memory_search", "memory_store"], - "偏好": ["memory_search", "memory_store"], - "preference": ["memory_search", "memory_store"], - "记忆": b_memory, - "数据": [ - "data_upload", - "data_files", - "data_delete", - "data_categories", - ], - "data": [ - "data_upload", - "data_files", - "data_delete", - "data_categories", - ], - "prompt": [ - "prompt_list", - "prompt_get", - "prompt_create", - "prompt_delete", - "prompt_render", - ], - "模板": [ - "prompt_list", - "prompt_get", - "prompt_create", - "prompt_delete", - "prompt_render", - ], - "template": [ - "prompt_list", - "prompt_get", - "prompt_create", - "prompt_delete", - "prompt_render", - ], - "搜索": ["web_search", "context_search"], - "search": ["web_search", "context_search"], - "模型": ["switch_model", "switch_provider"], - "model": ["switch_model", "switch_provider"], - "能力": ["capability_enable", "capability_disable"], - "capability": ["capability_enable", "capability_disable"], - # Browser / web scraping tools - "scrape": [ - "scrape_web", - "scrape_web_html", - "scrape_web_screenshot", - ], - "抓取": [ - "scrape_web", - "scrape_web_html", - "scrape_web_screenshot", - ], - "网页": [ - "scrape_web", - "scrape_web_html", - "scrape_web_screenshot", - ], - "webpage": [ - "scrape_web", - "scrape_web_html", - "scrape_web_screenshot", - ], - "截图": [ - "scrape_web", - "scrape_web_html", - "scrape_web_screenshot", - ], - "screenshot": [ - "scrape_web", - "scrape_web_html", - "scrape_web_screenshot", - ], - "browser": [ - "scrape_web", - "scrape_web_html", - "scrape_web_screenshot", - ], - "爬取": [ - "scrape_web", - "scrape_web_html", - "scrape_web_screenshot", - ], - "crawl": [ - "scrape_web", - "scrape_web_html", - "scrape_web_screenshot", - ], - # Camera tools - "拍照": ["camera_capture", "camera_record"], - "photo": ["camera_capture", "camera_record"], - "摄像头": ["camera_capture", "camera_record"], - "录屏": ["camera_capture", "camera_record"], - "录制": ["camera_capture", "camera_record"], - "record": ["camera_capture", "camera_record"], - "camera": ["camera_capture", "camera_record"], - # Bailian platform tools (schemas are keyword-gated, see b_* above) - "图片": b_image, - "画图": b_image, - "draw": b_image, - "image": b_image, - "vision": b_image, - "多模态": ["bailian_omni"], - "multimodal": ["bailian_omni"], - "omni": ["bailian_omni"], - "视频": b_video, - "video": b_video, - "语音": b_speech, - "speech": b_speech, - "tts": ["bailian_speech_synthesize"], - "朗读": ["bailian_speech_synthesize"], - "read aloud": ["bailian_speech_synthesize"], - "知识库": ["bailian_knowledge_retrieve"], - "knowledge": ["bailian_knowledge_retrieve"], - "rag": ["bailian_knowledge_retrieve"], - "百炼应用": ["bailian_app_call", "bailian_app_list"], - "bailian app": ["bailian_app_call", "bailian_app_list"], - "用量": ["bailian_usage_free"], - "usage": ["bailian_usage_free"], - "quota": ["bailian_usage_free"], - "上传": ["bailian_file_upload"], - "upload": ["bailian_file_upload"], - # Catch-all: mentioning the platform unlocks the full set - "百炼": b_all, - "bailian": b_all, - } - # Determine which tools to include - include_tools = set(core_tools) + include_tools = set(_CORE_TOOLS) - # Add session tools (always available) - include_tools.update( - [ - "switch_model", - "switch_provider", - "capability_enable", - "capability_disable", - "view_image", - ], - ) - include_tools.update(["mcp_connect", "mcp_disconnect"]) + include_tools.update(_SESSION_ALWAYS_ON) - # Model-invocable skill templates (.acli/skills/*.md) - include_tools.add("use_skill") + # evolve_skill stays env-gated until caps and utility pruning land — + # auto-generated skills without pruning are library noise. + if os.environ.get("ACLI_EVOLVE_SKILL", "0") == "1": + include_tools.add("evolve_skill") # Add capability-gated agent tools (delegate/subagent_invoke) whenever # they are registered — registration itself is the capability gate. @@ -406,7 +431,7 @@ def to_schema_list(self, user_input: str = "") -> list[dict]: ).lower() else: input_lower = user_input.lower() - for keyword, tools in keyword_tools.items(): + for keyword, tools in _KEYWORD_TOOLS.items(): if keyword.lower() in input_lower: include_tools.update(tools) diff --git a/dashscope/acli/utils/paths.py b/dashscope/acli/utils/paths.py index d529987..454d79f 100644 --- a/dashscope/acli/utils/paths.py +++ b/dashscope/acli/utils/paths.py @@ -5,6 +5,7 @@ import os from pathlib import Path +from uuid import uuid4 # Directory / file names considered sensitive. Any path containing these # as a component is blocked for read, write, and delete operations. @@ -123,10 +124,13 @@ def atomic_write_text(path: Path, text: str, encoding: str = "utf-8") -> None: Writes to a temp file in the SAME directory, flush+fsync, then os.replace — a crash mid-write leaves the old file intact instead of - truncating it. + truncating it. The temp name carries pid+uuid: two writers sharing one + fixed temp path would clobber each other's content before the replace. """ path = Path(path) - tmp = path.with_suffix(path.suffix + ".tmp") + tmp = path.with_name( + f"{path.name}.{os.getpid()}.{uuid4().hex[:8]}.tmp", + ) try: with tmp.open("w", encoding=encoding) as f: f.write(text) From 60caebc1e0aeca85a0e75cea1347e1da09e2b553 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Fri, 9 Oct 2026 20:15:19 +0800 Subject: [PATCH 15/19] acli: sync failed-turn tracing, /model and crypto tolerance from agenticCLI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7a1105c — failed/silent turns now leave a "[Turn failed before any response: ...]" assistant trace, an identical resubmit reuses the orphaned trailing user message instead of stacking, markdown-only interrupt partials are dropped, the tongyi native wire strips reasoning_content (it was re-billing the thinking trace every turn), the gateway 403 overdue-payment block is explicitly non-retryable with an account/billing hint at both display layers, unknown slash commands print a dim notice before falling through to the model, the Current user env line is labeled as the configured display name, and an ENC: config value this machine cannot decrypt loads as unset with a stderr note instead of killing startup with a bare UnicodeDecodeError (the "Agent failed to start ('utf-8' codec ...)" report). a3d2a2c — /model switches the model under the current provider without the /provider wizard; bare /model shows the current model. Non-interactive, so both REPL and TUI route it through the synchronous dispatch path. Mirrored tests added to tests/unit (tongyi wire sanitizing, account classification/hint, gateway 403 never retried); the remaining coverage lives in agenticCLI, whose suite passes 2756. dashscope acli suite: 323 passed. --- dashscope/acli/agent.py | 44 +++++++++++++++++-- dashscope/acli/cli/constants.py | 1 + dashscope/acli/cli/dispatch.py | 7 +++ dashscope/acli/cli/handlers_provider.py | 54 ++++++++++++++++++++++++ dashscope/acli/cli/repl.py | 9 ++++ dashscope/acli/cli/runners.py | 5 +++ dashscope/acli/cli/streaming.py | 11 ++++- dashscope/acli/commands.py | 5 +++ dashscope/acli/prompt_pipeline.py | 7 ++- dashscope/acli/providers/hardening.py | 34 +++++++++++++++ dashscope/acli/providers/tongyi.py | 6 +++ dashscope/acli/utils/crypto.py | 27 ++++++++++-- tests/unit/test_acli_provider_retries.py | 31 ++++++++++++++ tests/unit/test_tongyi_native.py | 24 +++++++++++ 14 files changed, 256 insertions(+), 9 deletions(-) diff --git a/dashscope/acli/agent.py b/dashscope/acli/agent.py index afc9c1f..5210c14 100644 --- a/dashscope/acli/agent.py +++ b/dashscope/acli/agent.py @@ -46,6 +46,7 @@ text_of, tool_result_for_display, tool_result_for_history, + truncate, ) console = Console() @@ -654,7 +655,15 @@ async def _store_memory(self, messages: list[dict[str, str]]): console.print(f"\n[dim red](memory error: {e})[/dim red]") async def run_stream(self, user_input) -> AsyncIterator[str]: - self.messages.append({"role": "user", "content": user_input}) + # A failed/interrupted turn can leave its user message as the last + # history entry; resubmitting the same text must not stack a second + # copy on top of it — the orphan is reused as this turn's input. + if not ( + self.messages + and self.messages[-1].get("role") == "user" + and self.messages[-1].get("content") == user_input + ): + self.messages.append({"role": "user", "content": user_input}) from dashscope.acli.audit import get_audit_logger @@ -666,17 +675,42 @@ async def run_stream(self, user_input) -> AsyncIterator[str]: async for chunk in self._run_stream_body(user_input): _partial.append(chunk) yield chunk - except BaseException: + except BaseException as e: # On cancellation / error, save whatever was received so the # user message and partial reply survive the next session load. + # A partial that is pure markdown scaffolding ("**", "```") + # carries no information and is dropped. partial_text = "".join(_partial).strip() - if partial_text: + if partial_text and any(ch.isalnum() for ch in partial_text): self.messages.append( { "role": "assistant", "content": partial_text + " [interrupted]", }, ) + elif not isinstance( + e, + ( + UserAbortedTurn, + KeyboardInterrupt, + SystemExit, + GeneratorExit, + asyncio.CancelledError, + ), + ): + # A provider/turn failure with zero output otherwise leaves + # only the user message behind: the history then shows N + # stacked user prompts with no trace of why each attempt + # failed, and the model cannot learn the cause on resubmit. + self.messages.append( + { + "role": "assistant", + "content": ( + "[Turn failed before any response: " + f"{sanitize_text(str(e))[:500]}]" + ), + }, + ) self.save_session() raise finally: @@ -1224,7 +1258,9 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: # Record experience for learning (only if tools were used) if self._current_turn_tools: - task_summary = text_of(user_input)[:100] # Truncate for storage + # Flatten + ellipsis: raw prompts carry newlines, and a + # mid-word cut breaks the "Past Lessons" list rendering. + task_summary = truncate(" ".join(text_of(user_input).split()), 100) outcome = _classify_outcome( self._turn_tool_successes, self._turn_tool_failures, diff --git a/dashscope/acli/cli/constants.py b/dashscope/acli/cli/constants.py index fc7bb70..6433f9c 100644 --- a/dashscope/acli/cli/constants.py +++ b/dashscope/acli/cli/constants.py @@ -197,6 +197,7 @@ "/tts", "/camera", "/provider", + "/model", "/setup", "/capability", "/subagents", diff --git a/dashscope/acli/cli/dispatch.py b/dashscope/acli/cli/dispatch.py index 9a1d8dc..cadc35c 100644 --- a/dashscope/acli/cli/dispatch.py +++ b/dashscope/acli/cli/dispatch.py @@ -367,6 +367,13 @@ def _handle_slash_command( handle_provider_command(cmd, agent, config) return True + elif cmd.startswith("/model"): + from dashscope.acli.cli.handlers_provider import ( + handle_model_command, + ) + + handle_model_command(cmd, agent, config) + return True elif cmd.startswith("/dev test provider") or cmd.startswith( "/dev debug call", ): diff --git a/dashscope/acli/cli/handlers_provider.py b/dashscope/acli/cli/handlers_provider.py index f52b23a..ae9f588 100644 --- a/dashscope/acli/cli/handlers_provider.py +++ b/dashscope/acli/cli/handlers_provider.py @@ -301,3 +301,57 @@ def handle_provider_command(cmd: str, agent: Agent, config: Config) -> bool: "[dim]/provider is now a Q&A wizard; args ignored[/dim]", ) return _provider_wizard(agent, config) + + +def handle_model_command(cmd: str, agent: Agent, config: Config) -> bool: + """/model [name] — switch the model inside the current provider. + + Users coming from other CLIs type /model reflexively; making them + rediscover it under the /provider wizard costs a turn. Provider, key + and protocol stay with /provider. + """ + parts = cmd.split(maxsplit=1) + if len(parts) == 1 or not parts[1].strip(): + console.print( + f"[cyan]{config.provider}/{config.model}[/cyan] " + f"protocol={config.protocol}", + ) + console.print( + "[dim]Usage: /model switches the model; " + "/provider covers provider/key/protocol.[/dim]", + ) + return True + + model = parts[1].strip() + normalized = normalize_model_name(model) + if normalized != model: + console.print( + f"[dim]Model name normalized to {normalized} " + f"(API model IDs are case-sensitive)[/dim]", + ) + + # Same rule as the wizard's model step: an unlisted name under a + # built-in provider is registered as a custom model; under an + # extension provider it is used as-is. + models = PROVIDER_MODELS.get(config.provider, []) + if not models: + ext = find_provider(config.provider) + if ext is not None: + models = ext.resolved_models() + if ( + models + and normalized not in models + and config.provider in PROVIDER_MODELS + ): + register_custom_model(config, config.provider, normalized) + console.print( + f"[green]✓ Registered {config.provider}/{normalized}[/green]", + ) + config.model = normalized + config.save_global() + config.save_workspace() + if _rebuild_provider(agent, config): + console.print( + f"[green]✓ Switched: {config.provider}/{config.model}[/green]", + ) + return True diff --git a/dashscope/acli/cli/repl.py b/dashscope/acli/cli/repl.py index eab9e5b..f824dc2 100644 --- a/dashscope/acli/cli/repl.py +++ b/dashscope/acli/cli/repl.py @@ -562,6 +562,15 @@ async def _cli_confirm_callback( continue elif result: continue + else: + # Not a registered command: the text goes to the model (an + # agent can legitimately answer slash-command questions), + # but say so — a typo'd command must not fail silently. + console.print( + f"[dim]Unknown command " + f"{user_input.split()[0]}; sending it to the model " + f"instead (/help lists commands).[/dim]", + ) # Expand @path tokens to file contents before sending to the agent. # Done here (post slash-command handling) so slash commands themselves diff --git a/dashscope/acli/cli/runners.py b/dashscope/acli/cli/runners.py index a388ce3..5a63ca6 100644 --- a/dashscope/acli/cli/runners.py +++ b/dashscope/acli/cli/runners.py @@ -280,7 +280,12 @@ async def _run_oneshot(config: Config, prompt: str): if usage_path: _write_usage_file(usage_path, executor) if failure is not None: + from dashscope.acli.providers.hardening import account_error_hint + print(f"Error: {failure}", file=sys.stderr) + hint = account_error_hint(failure) + if hint: + print(hint, file=sys.stderr) sys.exit(1) diff --git a/dashscope/acli/cli/streaming.py b/dashscope/acli/cli/streaming.py index e77fa72..28f6a5a 100644 --- a/dashscope/acli/cli/streaming.py +++ b/dashscope/acli/cli/streaming.py @@ -21,6 +21,7 @@ AsyncSpinner, UserAbortedTurn, message_text_for_compress, + truncate, ) console = Console() @@ -206,7 +207,10 @@ async def _do_summarize( lesson = f"Steps: {'; '.join(steps)}" tracker.record_experience( - task_summary=task[:200], + # Flatten newlines and mark truncation: the task is usually + # the raw prompt, and a mid-word cut with embedded newlines + # breaks the numbered "Past Lessons" list in the system prompt. + task_summary=truncate(" ".join(task.split()), 200), tools_used=tools_used, outcome=outcome, lesson=lesson, @@ -313,6 +317,11 @@ def _sigint_handler(): console.print("\n[dim]Turn aborted[/dim]") except Exception as e: console.print(f"\n[red]Error: {e}[/red]") + from dashscope.acli.providers.hardening import account_error_hint + + hint = account_error_hint(e) + if hint: + console.print(f"[yellow]{hint}[/yellow]") finally: # Restore default SIGINT handler try: diff --git a/dashscope/acli/commands.py b/dashscope/acli/commands.py index 5817f3c..cbacd2c 100644 --- a/dashscope/acli/commands.py +++ b/dashscope/acli/commands.py @@ -80,6 +80,11 @@ "/provider", "Configure Provider / Key / model / protocol (Q&A style)", ), + ( + "/model", + "Switch the model within the current provider " + "(/model )", + ), ( "/trust", "Tool trust/deny cache for this session " diff --git a/dashscope/acli/prompt_pipeline.py b/dashscope/acli/prompt_pipeline.py index bdba8d2..8673855 100644 --- a/dashscope/acli/prompt_pipeline.py +++ b/dashscope/acli/prompt_pipeline.py @@ -212,7 +212,12 @@ def render(self, ctx: PromptContext) -> str: f"run_command executes via: {shell_label()}", ] if ctx.user_name: - env_info.append(f"Current user: {ctx.user_name}") + # Display name from config.toml, NOT the OS login — the model + # once fabricated /Users//... paths from this line. + env_info.append( + f"Current user: {ctx.user_name} " + "(configured display name, not the OS account)", + ) if ctx.provider_name or ctx.model_name: env_info.append( f"Current model: {ctx.provider_name}/{ctx.model_name} " diff --git a/dashscope/acli/providers/hardening.py b/dashscope/acli/providers/hardening.py index 1d42f5a..4b2141b 100644 --- a/dashscope/acli/providers/hardening.py +++ b/dashscope/acli/providers/hardening.py @@ -74,6 +74,13 @@ "model not found", "insufficient_quota", "billing", + # Gateway account blocks, e.g. "Access denied, please make sure your + # account is in good standing ... #overdue-payment" — permanent until + # the account is fixed, so never retry/fallback on them. + "access denied", + "accessdenied", + "good standing", + "overdue", # Chinese patterns "权限", # permission "认证", # authentication @@ -83,6 +90,7 @@ "配额", # quota "账单", # billing "不存在", # does not exist + "欠费", # overdue payment # English equivalents of the Chinese entries not already covered # by the English block above "permission", @@ -91,6 +99,32 @@ "does not exist", ) +# Subset of non-retryable errors where resubmitting the same request is +# pointless until the user fixes their account (overdue payment, service +# not activated). Surfaced as an actionable hint at the display layer. +_ACCOUNT_ERROR_PATTERNS = ( + "access denied", + "accessdenied", + "good standing", + "overdue", + "insufficient_quota", + "billing", + "欠费", +) + + +def account_error_hint(error: BaseException) -> str: + """Actionable hint for account/billing failures, else an empty string.""" + text = str(error).lower() + if any(p in text for p in _ACCOUNT_ERROR_PATTERNS): + return ( + "This is an account/billing error (overdue payment or service " + "not activated); retrying will not help until the account is " + "in good standing." + ) + return "" + + # Hint appended to a retry when the model produced an empty reply. _EMPTY_RECOVERY_HINT = { "role": "user", diff --git a/dashscope/acli/providers/tongyi.py b/dashscope/acli/providers/tongyi.py index 9b8b1a3..f749b14 100644 --- a/dashscope/acli/providers/tongyi.py +++ b/dashscope/acli/providers/tongyi.py @@ -140,6 +140,12 @@ def _to_native_content(content): def _to_native_messages(messages: list[dict]) -> list[dict]: result = [] for m in messages: + # reasoning_content is kept in local history for display/session + # restore, but it must not go back on the wire: the API does not + # accept it as input, and resending it re-bills the whole thinking + # trace on every turn. + if "reasoning_content" in m: + m = {k: v for k, v in m.items() if k != "reasoning_content"} if isinstance(_safe_get(m, "content"), list): m = {**m, "content": _to_native_content(m["content"])} result.append(m) diff --git a/dashscope/acli/utils/crypto.py b/dashscope/acli/utils/crypto.py index e0e8fc5..c4dc813 100644 --- a/dashscope/acli/utils/crypto.py +++ b/dashscope/acli/utils/crypto.py @@ -8,8 +8,10 @@ from __future__ import annotations import base64 +import binascii import hashlib import platform +import sys import uuid _ENC_PREFIX = "ENC:" @@ -36,11 +38,30 @@ def encrypt_value(plaintext: str) -> str: def decrypt_value(stored: str) -> str: - """Reverse :func:`encrypt_value`.""" + """Reverse :func:`encrypt_value`; undecryptable values decode to "". + + The machine key derives from hostname + MAC, so a config.toml copied + from another machine — or a renamed host — holds ENC: values this + machine cannot read. Raising here used to kill agent startup with a + bare UnicodeDecodeError; treating the value as unset routes the user + back through key entry instead. + """ if not stored: return "" if not stored.startswith(_ENC_PREFIX): return stored key = _machine_key() - encrypted = base64.b64decode(stored[len(_ENC_PREFIX) :]) - return _xor_cipher(encrypted, key).decode() + try: + encrypted = base64.b64decode( + stored[len(_ENC_PREFIX) :], + validate=True, + ) + return _xor_cipher(encrypted, key).decode() + except (binascii.Error, UnicodeDecodeError): + print( + "acli: a stored ENC: value could not be decrypted on this " + "machine (hostname or hardware changed?); treating it as " + "unset — re-enter it via /provider", + file=sys.stderr, + ) + return "" diff --git a/tests/unit/test_acli_provider_retries.py b/tests/unit/test_acli_provider_retries.py index ca67650..6b5eda5 100644 --- a/tests/unit/test_acli_provider_retries.py +++ b/tests/unit/test_acli_provider_retries.py @@ -32,6 +32,7 @@ from dashscope.acli.providers.hardening import ( HardenedProvider, RetriesExhausted, + account_error_hint, is_retryable_error, ) from dashscope.acli.providers.profile import ProviderChain, ProviderProfile @@ -207,3 +208,33 @@ async def test_a_failure_after_the_first_chunk_stays_plain(): assert [c.delta_content for c in seen] == ["hi"] assert not isinstance(exc.value, RetriesExhausted) assert inner.stream_calls == 1 + + +# The literal 403 the DashScope gateway raises on an overdue account. +# Permanent until the user fixes billing, so neither the retry layer nor +# the fallback chain may spend a budget on it. +OVERDUE = ( + "DashScope API error: 403 - Access denied, please make sure your " + "account is in good standing. For details, see: " + "https://help.aliyun.com/zh/model-studio/error-code#overdue-payment" +) + + +async def test_a_gateway_account_block_is_never_retried(): + inner = _Scripted(chat=[RuntimeError(OVERDUE)] * 4) + hardened = HardenedProvider(inner, max_retries=3, retry_delay=0.01) + + with pytest.raises(RuntimeError) as exc: + await hardened.chat(MESSAGES) + assert not isinstance(exc.value, RetriesExhausted) + assert inner.chat_calls == 1 + assert is_retryable_error(exc.value) is False + + +def test_account_error_hint_flags_billing_errors_only(): + assert "account/billing" in account_error_hint(RuntimeError(OVERDUE)) + assert account_error_hint(RuntimeError("账户已欠费，请充值")) + # Non-retryable is not the same as an account error: a bad key gets no + # billing hint, and neither does a transient timeout. + assert account_error_hint(RuntimeError("invalid api key")) == "" + assert account_error_hint(RuntimeError("timeout")) == "" diff --git a/tests/unit/test_tongyi_native.py b/tests/unit/test_tongyi_native.py index 36ff2cc..1f4bfea 100644 --- a/tests/unit/test_tongyi_native.py +++ b/tests/unit/test_tongyi_native.py @@ -344,6 +344,30 @@ def test_openai_content_parts_converted_to_native(): assert converted[1]["content"] == "just text" +def test_reasoning_content_is_stripped_from_the_wire(): + """reasoning_content stays in local history for display/session restore, + but it must not go back on the wire: the API does not accept it as input + and resending it re-bills the whole thinking trace every turn.""" + messages = [ + {"role": "user", "content": "hi"}, + { + "role": "assistant", + "content": "hello", + "reasoning_content": "thinking trace", + }, + {"role": "user", "content": "again"}, + ] + body = _provider()._build_request_body( # pylint: disable=protected-access + messages, + None, + ) + converted = body["input"]["messages"] + assert converted[1] == {"role": "assistant", "content": "hello"} + # The caller's history dict keeps the trace — only the wire copy is + # rebuilt without it. + assert messages[1]["reasoning_content"] == "thinking trace" + + # --------------------------------------------------------------------------- # base_url normalization # --------------------------------------------------------------------------- From eb4cbdfad66bfefb9b725b518a763a339acb3ebe Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Mon, 12 Oct 2026 14:42:41 +0800 Subject: [PATCH 16/19] acli: sync thinking-budget, output-cap continuation and oneshot policy from agenticCLI 0.6.12 Source changes arrive via scripts/sync_acli.sh; the reflection and tongyi-native tests are ported by hand with dashscope.acli imports. --- dashscope/acli/__init__.py | 2 +- dashscope/acli/agent.py | 103 ++++++++++++++++++++++++++-- dashscope/acli/memory/reflection.py | 59 ++++++++++++++++ dashscope/acli/providers/tongyi.py | 70 ++++++++++++++++++- tests/unit/test_acli_reflection.py | 56 +++++++++++++++ tests/unit/test_tongyi_native.py | 101 +++++++++++++++++++++++++++ 6 files changed, 384 insertions(+), 7 deletions(-) diff --git a/dashscope/acli/__init__.py b/dashscope/acli/__init__.py index 79c639e..33c797d 100644 --- a/dashscope/acli/__init__.py +++ b/dashscope/acli/__init__.py @@ -3,7 +3,7 @@ import uuid -__version__ = "0.6.7" +__version__ = "0.6.12" # Per-process identifier sent as x-dashscope-sdk-session-id so the # backend can group multi-turn requests from one CLI run. diff --git a/dashscope/acli/agent.py b/dashscope/acli/agent.py index 5210c14..38917a1 100644 --- a/dashscope/acli/agent.py +++ b/dashscope/acli/agent.py @@ -27,6 +27,7 @@ from dashscope.acli.memory.reflection import ( convergence_hint, is_readonly_tool_call, + time_budget_hint, ) from dashscope.acli.platforms.base import MemoryProvider from dashscope.acli.prompt_pipeline import PromptContext, default_pipeline @@ -253,6 +254,34 @@ def __init__( ) except ValueError: self.converge_hard_ratio = 0.85 + # Wall-clock budget nudge (oneshot only): the turn-count convergence + # hint never fires when a run is few-but-huge generations, yet the + # harness kills the run on wall time all the same — and the hard-set + # tasks that died so (dna-assembly, write-compressor) all died with + # their deliverable never written. The bench adapter injects the + # in-container `timeout` value as ACLI_TIME_BUDGET_SEC; unset/<=0 + # disables. Ratios: ACLI_TIME_SOFT / ACLI_TIME_HARD. + try: + self.time_budget_sec = float( + os.environ.get("ACLI_TIME_BUDGET_SEC", "0"), + ) + except ValueError: + self.time_budget_sec = 0.0 + try: + self.time_soft_ratio = float( + os.environ.get("ACLI_TIME_SOFT", "0.5"), + ) + except ValueError: + self.time_soft_ratio = 0.5 + try: + self.time_hard_ratio = float( + os.environ.get("ACLI_TIME_HARD", "0.8"), + ) + except ValueError: + self.time_hard_ratio = 0.8 + # Monotonic anchor for the budget above, set on the first turn: a + # wall clock would credit host-suspend time the process never had. + self._run_started: float | None = None # Silence nudge: one reminder per turn after N tool calls have gone by # without any prose reaching the user. Mode-independent on purpose — # the shape this exists for is an interactive turn of *distinct* @@ -289,6 +318,14 @@ def __init__( if self.oneshot: self.system_prompt = (self.system_prompt or SYSTEM_PROMPT) + ( "\n\nTask execution policy (autonomous mode):\n" + "- Work from a written plan, not from memory: early in the " + "run, write a short plan file (goal, acceptance check, " + "ordered steps); after each step, record in it what is done " + "and what remains; before reporting completion, re-read it " + "and verify each item. Sketch the deliverable itself first, " + "then iterate on it. Never spend a whole turn only " + "thinking — every turn must land something on disk: a file, " + "a measurement, or an updated plan.\n" "- If the request has a measurable acceptance criterion " "(numeric threshold, size/time limit, test suite, " "similarity/accuracy target), iterate until it is met: " @@ -297,10 +334,16 @@ def __init__( "toward the target — but once the criterion is met, lock it " "in and finish rather than risk regressing a passing result. " "Heed any budget-check hint: it tells you when to converge " - "or switch to a different approach.\n" - "- Before reporting completion, self-verify with the same " - "check the requester will apply (run the tests, recompute " - "the metric, validate the format). Fix what fails instead " + "or switch to a different approach. A wall-clock hint means " + "the run is killed when time runs out: get the required " + "deliverable written early, then keep improving it.\n" + "- Before reporting completion, turn every stated " + "requirement into its own executable check and run them " + "all — the measurable criterion, the format rules, and " + "any constraint that mentions components the deliverable " + "must contain. Where a check depends on domain facts, " + "state the fact explicitly first (a wrong assumption is " + "worse than a flagged unknown). Fix what fails instead " "of handing back a near miss.\n" "- When a check fails close to the target, treat it as " "actionable feedback: change the approach based on what " @@ -593,6 +636,17 @@ def _convergence_section(self, loop_index: int) -> str: self.converge_hard_ratio, ) + def _time_budget_section(self) -> str: + """Wall-clock budget nudge; see time_budget_hint for the rationale.""" + if not self.oneshot or self._run_started is None: + return "" + return time_budget_hint( + time.monotonic() - self._run_started, + self.time_budget_sec, + self.time_soft_ratio, + self.time_hard_ratio, + ) + def _silence_section(self) -> str: """Inject one reminder when tool calls stop producing prose. @@ -732,6 +786,8 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: # Clear tool tracking for new turn self._current_turn_tools = [] + if self._run_started is None: + self._run_started = time.monotonic() self.turn_tool_calls = 0 self.turn_subagents = 0 self.turn_mcp_calls = 0 @@ -827,13 +883,14 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: # second one would replace the real prompt outright, and the Tongyi # converter only reads messages[0] as system. # - # All four sections are rebuilt every iteration because tracker + # All five sections are rebuilt every iteration because tracker # state changes as tools run; each returns "" when its condition # is not met, so an empty hint costs no message at all. hint_parts = { "reflection": self._reflection_section(), "stagnation": self._stagnation_section(), "convergence": self._convergence_section(_loop_i), + "time_budget": self._time_budget_section(), "silence": self._silence_section(), } hint = "".join(hint_parts.values()).strip() @@ -869,6 +926,7 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: tool_calls: list[ToolCall] = [] seen_tool_calls: set[tuple[str, str]] = set() last_chunk = None + finish_reason = "" llm_start = time.monotonic() # Turn-level retry around the streaming call, for providers that @@ -888,6 +946,7 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: tool_calls = [] seen_tool_calls = set() last_chunk = None + finish_reason = "" emitted = False try: async for chunk in self.provider.chat_stream( @@ -931,6 +990,8 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: if chunk.usage: self.executor.record_api_call(chunk.usage) last_chunk = chunk + if chunk.finish_reason: + finish_reason = chunk.finish_reason break except Exception as e: if ( @@ -1024,6 +1085,38 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: self.turn_mcp_calls += 1 if not tool_calls: + if finish_reason == "length": + # Output cap cut the generation mid-turn: this is not a + # final answer, so skip the gate and do not break. The + # partial goes into history and the loop continues with + # a shorter-chunks demand — which is also what makes + # every between-iteration mechanism (time budget, + # convergence, reflection) reachable again on the + # monolithic-generation failure shape (2 requests per + # 1800s in run 2026-10-12__time-069). + partial = full_content or ( + "[response cut off by the output-length limit " + "during planning, before any visible output]" + ) + cut_msg: dict = {"role": "assistant", "content": partial} + if full_reasoning: + cut_msg["reasoning_content"] = full_reasoning + self.messages.append(cut_msg) + messages_with_system.append(cut_msg) + nudge = ( + "[Output-length limit reached — your response was " + "cut off. Continue working, but keep each response " + "short: put plans and intermediate results into " + "files with write_file in small pieces instead of " + "one long response; if you were composing the " + "deliverable, write it now in several smaller " + "calls.]" + ) + yield f"\n{nudge}\n" + nudge_msg = {"role": "user", "content": nudge} + self.messages.append(nudge_msg) + messages_with_system.append(nudge_msg) + continue # Acceptance gate. This branch is the loop's only voluntary # exit, so it is the one place where "the model is done" can # be checked against something other than the model's own diff --git a/dashscope/acli/memory/reflection.py b/dashscope/acli/memory/reflection.py index 7022faf..0271f35 100644 --- a/dashscope/acli/memory/reflection.py +++ b/dashscope/acli/memory/reflection.py @@ -725,3 +725,62 @@ def convergence_hint( "passing result." ) return "" + + +def time_budget_hint( + elapsed_sec: float, + budget_sec: float, + soft_ratio: float = 0.5, + hard_ratio: float = 0.8, +) -> str: + """Wall-clock counterpart of ``convergence_hint``, deliverable-first. + + The turn-count nudge never fires on the failure shape this covers: a run + of very few, very long generations (7/150 turns) can still burn 95% of + the harness's wall-clock cap, and every hard-set task that died that way + (dna-assembly, write-compressor) died with its required output file + never written at all. So the soft band does NOT say "converge": it says + "land a best-effort deliverable now, then keep refining" — securing an + artifact early is what the turn nudge's plateaued loop already has and + these runs lack. + + Returns "" below ``soft_ratio`` or when ``budget_sec`` is unset/<=0; + a secure-the-deliverable nudge in the soft band; a best-version-on-disk + finalize nudge at/after ``hard_ratio``. + """ + if budget_sec <= 0 or elapsed_sec < 0: + return "" + frac = elapsed_sec / budget_sec + remaining = max(0.0, budget_sec - elapsed_sec) + elapsed_i = int(elapsed_sec) + budget_i = int(budget_sec) + remaining_i = int(remaining) + if frac >= hard_ratio: + return ( + "\n\n## ⏳ Wall-clock budget nearly exhausted\n" + f"{elapsed_i}s of your ~{budget_i}s budget are gone " + f"({remaining_i}s left).\n" + "Do NOT start a new approach — there is no time left to debug " + "it. Right now:\n" + "1. Make sure the deliverable file on disk is the best version " + "you have produced (overwrite a broken draft with an older " + "working one if needed).\n" + "2. Run ONE final verification against the acceptance " + "criterion.\n" + "3. Finish.\n" + ) + if frac >= soft_ratio: + return ( + "\n\n## ⏳ Wall-clock budget half spent\n" + f"{elapsed_i}s of your ~{budget_i}s budget are gone " + f"({remaining_i}s left); the run is killed when it hits 0.\n" + "If the file/artifact this task requires does not exist yet in " + "complete form, write a best-effort version NOW with your " + "current approach — a rough working deliverable beats a perfect " + "plan that never lands. Then keep refining and overwrite it " + "with better versions.\n" + "If it already exists, keep improving it — but re-save after " + "every improvement, so whatever is on disk when time runs out " + "is your best attempt." + ) + return "" diff --git a/dashscope/acli/providers/tongyi.py b/dashscope/acli/providers/tongyi.py index f749b14..5200e2b 100644 --- a/dashscope/acli/providers/tongyi.py +++ b/dashscope/acli/providers/tongyi.py @@ -2,8 +2,10 @@ # pylint: disable=too-many-branches,too-many-statements from __future__ import annotations +import asyncio import json import os +import time from typing import AsyncIterator, ClassVar import httpx @@ -269,11 +271,47 @@ def __init__( protocol: str = "openai", base_url: str | None = None, module: str = "app", + max_output_tokens: int | None = None, + thinking_budget: int | None = None, ): self.model = model self.api_key = api_key self.request_timeout = request_timeout self.protocol = protocol + # Per-call output cap (reasoning tokens count against it). The + # failure shape this exists for: a run of 15-minute, ~25K-token + # monolithic generations that never lands a deliverable before the + # wall-clock cap — with no cap the agent loop never regains control + # between request boundaries, so every between-turn mechanism (time + # budget, convergence, reflection) is unreachable. Truncation is + # safe: the stream assembly repairs or empties a cut tool call, and + # the agent treats finish_reason=length as "continue", not "done". + # Env: ACLI_MAX_OUTPUT_TOKENS (0/unset disables). + if max_output_tokens is None: + try: + max_output_tokens = int( + os.environ.get("ACLI_MAX_OUTPUT_TOKENS", "0"), + ) + except ValueError: + max_output_tokens = 0 + self.max_output_tokens = max(0, max_output_tokens) + # Reasoning-token budget. Probing qwen3.8-max on the native + # multimodal-generation path showed max_tokens caps only the + # visible text (100 → 284 output tokens incl. 180 reasoning), so + # the monolithic-thinking shape sails past it untouched; + # thinking_budget bounds the reasoning itself (200 → exactly 200). + # When the budget runs out the server wraps up the turn with + # finish_reason=stop, so the agent loop regains control without + # needing the length-continuation branch. Env: + # ACLI_THINKING_BUDGET (0/unset disables). + if thinking_budget is None: + try: + thinking_budget = int( + os.environ.get("ACLI_THINKING_BUDGET", "0"), + ) + except ValueError: + thinking_budget = 0 + self.thinking_budget = max(0, thinking_budget) base = (base_url or DASHSCOPE_BASE_URL).rstrip("/") for prefix in _LEGACY_PREFIXES: if base.endswith(prefix): @@ -360,6 +398,10 @@ def _build_request_body( if stream: parameters["stream"] = True parameters["incremental_output"] = True + if self.max_output_tokens > 0: + parameters["max_tokens"] = self.max_output_tokens + if self.thinking_budget > 0: + parameters["thinking_budget"] = self.thinking_budget ds_tools = self._convert_tools(tools) if ds_tools: parameters["tools"] = ds_tools @@ -600,11 +642,37 @@ async def _iter_stream(self, response) -> AsyncIterator[LLMChunk]: usage_sent = False _json_buf: str | None = None - async for line in response.aiter_lines(): + # SSE keepalive comment lines (": ...") carry no progress but reset + # httpx's read timeout, so a wedged generation can hold the stream + # open indefinitely — measured 882s on a request that never emitted + # a single data: line, surviving the whole max_retries x + # request_timeout budget until the caller's own cap killed it. + # Count only data: lines as progress and bound their gap at one + # request_timeout; a stalled stream must surface as a retryable + # error, not hang the turn. + lines = response.aiter_lines() + last_progress = time.monotonic() + while True: + remaining = self.request_timeout - ( + time.monotonic() - last_progress + ) + try: + line = await asyncio.wait_for( + lines.__anext__(), + timeout=max(remaining, 0), + ) + except StopAsyncIteration: + break + except asyncio.TimeoutError: + raise RuntimeError( + f"stream stalled: no data received for " + f"{self.request_timeout}s (keepalive timeout)", + ) from None # Native SSE sends "data:{...}" with no space; # compatible-mode sends "data: {...}". if not line.startswith("data:"): continue + last_progress = time.monotonic() payload = line[5:].strip() if payload == "[DONE]": break diff --git a/tests/unit/test_acli_reflection.py b/tests/unit/test_acli_reflection.py index baa214f..a99e464 100644 --- a/tests/unit/test_acli_reflection.py +++ b/tests/unit/test_acli_reflection.py @@ -10,6 +10,7 @@ StagnationTracker, convergence_hint, is_readonly_tool_call, + time_budget_hint, ) @@ -717,3 +718,58 @@ def test_soft_hint_contains_switch_advice(self): def test_hard_hint_says_no_new_approach(self): hint = convergence_hint(90, 100) assert "Do NOT start a new approach" in hint + + +# ── time_budget_hint ───────────────────────────────────────────────────────── + + +class TestTimeBudgetHint: + """Wall-clock counterpart of convergence_hint, deliverable-first. + + Fires on elapsed/budget, covering the few-but-huge-generations shape + where the turn-count nudge never triggers (7/150 turns can still burn + 95% of the harness's wall-clock cap). + """ + + def test_below_soft_returns_empty(self): + assert time_budget_hint(100, 900) == "" + # 449/900 = 0.499 < 0.5 + assert time_budget_hint(449, 900) == "" + + def test_at_soft_boundary_is_deliverable_first(self): + # 450/900 = 0.50 → fires soft + hint = time_budget_hint(450, 900) + assert "half spent" in hint + assert "450s of your ~900s budget" in hint + assert "(450s left)" in hint + assert "best-effort version NOW" in hint + # Explicitly not the turn nudge's "converge" wording: the failure + # shape here is a missing artifact, not a plateaued loop. + assert "converge" not in hint + assert "nearly exhausted" not in hint + + def test_soft_band_existing_deliverable_clause(self): + hint = time_budget_hint(600, 900) + assert "re-save after every improvement" in hint + + def test_at_hard_boundary(self): + # 720/900 = 0.80 → fires hard + hint = time_budget_hint(720, 900) + assert "nearly exhausted" in hint + assert "best version" in hint + assert "ONE final verification" in hint + assert "Do NOT start a new approach" in hint + assert "half spent" not in hint + + def test_remaining_clamps_at_zero(self): + hint = time_budget_hint(1000, 900) + assert "(0s left)" in hint + + def test_custom_ratios(self): + assert "half spent" in time_budget_hint(200, 900, soft_ratio=0.2) + hint = time_budget_hint(230, 900, soft_ratio=0.2, hard_ratio=0.25) + assert "nearly exhausted" in hint + + def test_disabled_without_budget(self): + assert time_budget_hint(100, 0) == "" + assert time_budget_hint(100, -5) == "" diff --git a/tests/unit/test_tongyi_native.py b/tests/unit/test_tongyi_native.py index 1f4bfea..70773bb 100644 --- a/tests/unit/test_tongyi_native.py +++ b/tests/unit/test_tongyi_native.py @@ -11,7 +11,9 @@ # pylint: disable=redefined-outer-name,protected-access +import asyncio import json +import time import pytest @@ -368,6 +370,60 @@ def test_reasoning_content_is_stripped_from_the_wire(): assert messages[1]["reasoning_content"] == "thinking trace" +def test_max_output_tokens_sent_when_configured(): + body = _provider(max_output_tokens=4096)._build_request_body( # pylint: disable=protected-access + MESSAGES, + None, + ) + assert body["parameters"]["max_tokens"] == 4096 + + +def test_max_output_tokens_absent_by_default(monkeypatch): + monkeypatch.delenv("ACLI_MAX_OUTPUT_TOKENS", raising=False) + body = _provider()._build_request_body( # pylint: disable=protected-access + MESSAGES, + None, + ) + assert "max_tokens" not in body["parameters"] + + +def test_max_output_tokens_env_override(monkeypatch): + monkeypatch.setenv("ACLI_MAX_OUTPUT_TOKENS", "2048") + assert _provider().max_output_tokens == 2048 + # An explicit constructor argument wins over the env. + assert _provider(max_output_tokens=512).max_output_tokens == 512 + # An unparseable value falls back to disabled, not a crash. + monkeypatch.setenv("ACLI_MAX_OUTPUT_TOKENS", "junk") + assert _provider().max_output_tokens == 0 + + +def test_thinking_budget_sent_when_configured(): + body = _provider(thinking_budget=4096)._build_request_body( # pylint: disable=protected-access + MESSAGES, + None, + ) + assert body["parameters"]["thinking_budget"] == 4096 + + +def test_thinking_budget_absent_by_default(monkeypatch): + monkeypatch.delenv("ACLI_THINKING_BUDGET", raising=False) + body = _provider()._build_request_body( # pylint: disable=protected-access + MESSAGES, + None, + ) + assert "thinking_budget" not in body["parameters"] + + +def test_thinking_budget_env_override(monkeypatch): + monkeypatch.setenv("ACLI_THINKING_BUDGET", "2048") + assert _provider().thinking_budget == 2048 + # An explicit constructor argument wins over the env. + assert _provider(thinking_budget=512).thinking_budget == 512 + # An unparseable value falls back to disabled, not a crash. + monkeypatch.setenv("ACLI_THINKING_BUDGET", "junk") + assert _provider().thinking_budget == 0 + + # --------------------------------------------------------------------------- # base_url normalization # --------------------------------------------------------------------------- @@ -729,3 +785,48 @@ def test_request_body_is_json_serializable(): assert parsed["parameters"]["response_format"] == { "type": "json_object", } + + +class _KeepaliveOnlyResponse(_FakeStreamResponse): + """The wedged-generation signature: keepalive comments flow forever + (resetting httpx's read timeout) while no data: line ever arrives — + the 067b bench failures sat in this state until the harness cap.""" + + async def aiter_lines(self): + while True: + await asyncio.sleep(0.01) + yield ": keep-alive" + + +class _StallAfterContentResponse(_FakeStreamResponse): + """One real chunk, then keepalive silence forever.""" + + async def aiter_lines(self): + yield NATIVE_SSE_LINES[3] + while True: + await asyncio.sleep(0.01) + yield ": keep-alive" + + +async def test_stream_stall_on_keepalives_raises_retryable(fake_http): + fake_http._stream_queue.append(_KeepaliveOnlyResponse([])) + provider = _provider(request_timeout=1) + started = time.monotonic() + with pytest.raises(RuntimeError, match="stream stalled") as excinfo: + async for _ in provider.chat_stream(MESSAGES): + pass + # bounded by the idle timeout (~1s), not by the caller's cap + assert time.monotonic() - started < 10 + from dashscope.acli.providers.hardening import is_retryable_error + + assert is_retryable_error(excinfo.value) + + +async def test_stream_stall_after_content_raises(fake_http): + fake_http._stream_queue.append(_StallAfterContentResponse([])) + provider = _provider(request_timeout=1) + chunks = [] + with pytest.raises(RuntimeError, match="stream stalled"): + async for chunk in provider.chat_stream(MESSAGES): + chunks.append(chunk) + assert any(c.delta_content for c in chunks) From 9e022616e3968a2d8c2e588ff455f9600b3ecb01 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Mon, 12 Oct 2026 16:42:37 +0800 Subject: [PATCH 17/19] acli: sync run_command budget back-pressure and 0.7.0 from agenticCLI Single-command timeout now tracks the remaining wall-clock budget (min 30s, 180s finalize reserve), and the oneshot policy gains the long-command clause; version 0.7.0 marks the Terminal-Bench hard set reaching best-of 10/10 upstream. --- dashscope/acli/__init__.py | 2 +- dashscope/acli/agent.py | 77 +++++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/dashscope/acli/__init__.py b/dashscope/acli/__init__.py index 33c797d..b757291 100644 --- a/dashscope/acli/__init__.py +++ b/dashscope/acli/__init__.py @@ -3,7 +3,7 @@ import uuid -__version__ = "0.6.12" +__version__ = "0.7.0" # Per-process identifier sent as x-dashscope-sdk-session-id so the # backend can group multi-turn requests from one CLI run. diff --git a/dashscope/acli/agent.py b/dashscope/acli/agent.py index 38917a1..cd5072f 100644 --- a/dashscope/acli/agent.py +++ b/dashscope/acli/agent.py @@ -347,7 +347,14 @@ def __init__( "of handing back a near miss.\n" "- When a check fails close to the target, treat it as " "actionable feedback: change the approach based on what " - "the measurement shows, then re-measure." + "the measurement shows, then re-measure.\n" + "- Do not stake a large share of the remaining budget on " + "one long unverified command: before an install, compile " + "or download that could run for minutes, spend seconds " + "probing that the path can succeed at all (package " + "available, toolchain present); give long commands an " + "explicit timeout so a stuck one fails fast and frees " + "the rest of the budget." ) if self._acceptance_gate.active: # An armed gate implies oneshot — from_env hands back an empty @@ -647,6 +654,68 @@ def _time_budget_section(self) -> str: self.time_hard_ratio, ) + # Seconds of wall-clock budget a single run_command may never consume: + # the remainder minus a reserve for writing/finalizing the deliverable. + # Run 2026-10-12__reqenum-0612 lost 720s of an 1800s budget to two + # run_command timeouts (a 600s pip compile that could never succeed + # offline plus a 120s follow-up) and died with the deliverable never + # written; the hint alone can only ask, this enforces. + _CMD_TIMEOUT_RESERVE_SEC = 180 + _CMD_TIMEOUT_MIN_SEC = 30 + + def _command_timeout_ceiling(self) -> int | None: + """Max seconds one run_command may take, or None when unbudgeted.""" + if ( + not self.oneshot + or self.time_budget_sec <= 0 + or self._run_started is None + ): + return None + remaining = self.time_budget_sec - ( + time.monotonic() - self._run_started + ) + return max( + self._CMD_TIMEOUT_MIN_SEC, + int(remaining - self._CMD_TIMEOUT_RESERVE_SEC), + ) + + def _clamp_run_command_timeout(self, tool_call) -> str: + """Clamp run_command's timeout to the budget ceiling. + + Mutates tool_call.arguments and returns a note for the tool + result when it clamped, "" otherwise. The note must ride the + result: the assistant's own tool_call entry in history keeps the + model's original arguments, so without it the model cannot see + why its 600s command died at 120s. + """ + if tool_call.name != "run_command": + return "" + ceiling = self._command_timeout_ceiling() + if ceiling is None: + return "" + from dashscope.acli.tools.shell import default_timeout + + raw = tool_call.arguments.get("timeout") + try: + current = ( + int(raw) if raw not in (None, "", 0) else default_timeout() + ) + except (TypeError, ValueError): + current = default_timeout() + if current <= ceiling: + return "" + tool_call.arguments["timeout"] = ceiling + remaining = self.time_budget_sec - ( + time.monotonic() - self._run_started + ) + return ( + f"[timeout clamped from {current}s to {ceiling}s: " + f"{int(remaining)}s of wall-clock budget remain, " + f"{self._CMD_TIMEOUT_RESERVE_SEC}s reserved for finalizing. " + "Break the work into commands that fit, or pick a path that " + "does not need the long step at all.]\n" + ) + def _silence_section(self) -> str: """Inject one reminder when tool calls stop producing prose. @@ -1400,6 +1469,10 @@ async def _execute_tool(self, tool_call: ToolCall) -> str: if not tool_def: return f"Error: unknown tool '{tool_call.name}'" + # Budget back-pressure on the one tool that can burn minutes of + # wall clock in a single call; see _clamp_run_command_timeout. + budget_note = self._clamp_run_command_timeout(tool_call) + # Track tool usage for experience recording (deduped, keep # first-use order) if tool_call.name not in self._current_turn_tools: @@ -1487,6 +1560,8 @@ async def _execute_tool(self, tool_call: ToolCall) -> str: ) duration_ms = int((time.time() - start_time) * 1000) result = outcome.text + if budget_note: + result = budget_note + result success = outcome.ok self.trace_logger.log_tool_execution( tool_call.name, From ba38588aa4e109781d73f393ec02c55b73fcc477 Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Mon, 12 Oct 2026 17:09:17 +0800 Subject: [PATCH 18/19] fix(acli): narrow _run_started for the repo mypy gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _clamp_run_command_timeout subtracts the monotonic anchor after the ceiling-None guard, but the repo's mypy (pinned --python-version=3.10) cannot infer _run_started is non-None from that guard across methods — CI on PR #206 failed with "Unsupported operand types for - (float and None)". Fold the check into the guard; upstream agenticCLI gets the same line so the next sync stays byte-identical. black also explodes two long calls in the merge-touched tongyi tests. --- dashscope/acli/agent.py | 2 +- tests/unit/test_tongyi_native.py | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/dashscope/acli/agent.py b/dashscope/acli/agent.py index cd5072f..1fd7f75 100644 --- a/dashscope/acli/agent.py +++ b/dashscope/acli/agent.py @@ -691,7 +691,7 @@ def _clamp_run_command_timeout(self, tool_call) -> str: if tool_call.name != "run_command": return "" ceiling = self._command_timeout_ceiling() - if ceiling is None: + if ceiling is None or self._run_started is None: return "" from dashscope.acli.tools.shell import default_timeout diff --git a/tests/unit/test_tongyi_native.py b/tests/unit/test_tongyi_native.py index 70773bb..b25d1d5 100644 --- a/tests/unit/test_tongyi_native.py +++ b/tests/unit/test_tongyi_native.py @@ -371,7 +371,9 @@ def test_reasoning_content_is_stripped_from_the_wire(): def test_max_output_tokens_sent_when_configured(): - body = _provider(max_output_tokens=4096)._build_request_body( # pylint: disable=protected-access + body = _provider( + max_output_tokens=4096, + )._build_request_body( # pylint: disable=protected-access MESSAGES, None, ) @@ -398,7 +400,9 @@ def test_max_output_tokens_env_override(monkeypatch): def test_thinking_budget_sent_when_configured(): - body = _provider(thinking_budget=4096)._build_request_body( # pylint: disable=protected-access + body = _provider( + thinking_budget=4096, + )._build_request_body( # pylint: disable=protected-access MESSAGES, None, ) From d0ea611ad043c5a660787a3b36de1d10ad34e18f Mon Sep 17 00:00:00 2001 From: "zhansheng.lzs" Date: Wed, 14 Oct 2026 13:57:23 +0800 Subject: [PATCH 19/19] acli: sync 0.7.1 from agenticCLI Vision chain (oneshot registration, injection perception, region reads with voting, vision timeout, qwen3-vl-plus default), reasoning stall retry fix, oneshot empty-turn backstop, rules 17/18 and the hard-band best-partial write. vendored suite 1032 passed, 10 skipped. --- dashscope/acli/__init__.py | 2 +- dashscope/acli/agent.py | 61 ++- dashscope/acli/cli/runners.py | 7 + dashscope/acli/config.py | 1 + dashscope/acli/memory/reflection.py | 15 +- dashscope/acli/tools/filesystem.py | 35 ++ dashscope/acli/tools/shell.py | 157 +++++++- dashscope/acli/tools/vision.py | 575 ++++++++++++++++++++++++---- tests/unit/test_acli_vision_tool.py | 4 +- 9 files changed, 762 insertions(+), 95 deletions(-) diff --git a/dashscope/acli/__init__.py b/dashscope/acli/__init__.py index b757291..b5ff1e5 100644 --- a/dashscope/acli/__init__.py +++ b/dashscope/acli/__init__.py @@ -3,7 +3,7 @@ import uuid -__version__ = "0.7.0" +__version__ = "0.7.1" # Per-process identifier sent as x-dashscope-sdk-session-id so the # backend can group multi-turn requests from one CLI run. diff --git a/dashscope/acli/agent.py b/dashscope/acli/agent.py index 1fd7f75..6df31bb 100644 --- a/dashscope/acli/agent.py +++ b/dashscope/acli/agent.py @@ -163,7 +163,22 @@ def _budget_burned_slowly(error: BaseException) -> bool: never claim a change works without having executed it. When the output transforms or joins data (ETL, CSV/JSON, migrations), also assert every cross-reference resolves — schema-valid output with dangling IDs still - fails + fails. When the task names a ground-truth tool or command for a check, + run the final verification with that exact tool and its stated + parameters — a calibrated substitute proves nothing about the grade +18. **Snapshot a fragile artifact before inspecting it.** A tool meant to + read can still mutate what it opens — SQLite deletes an invalid WAL + when the connection closes, and destroyed evidence cannot be + re-downloaded. Copy the file to scratch space first (cp to /tmp), or + open it immutably (e.g. sqlite3 'file:...?immutable=1') +19. **Read images with the vision model, not by proxy.** When a task + hinges on perceiving an image — text or digits in a picture, a chart, + a rendered layout — call view_image on the file itself; reading it + back through terminal-resolution ASCII art or hand-rolled pixel + scripts throws away exactly the detail the answer depends on. If the + artifact is data rather than an image (geometry, toolpaths, a + waveform), render it to a PNG first and then look at it with + view_image Reply style: - **Concise**. No filler like "let me see / let me help you / I'll analyze @@ -236,6 +251,14 @@ def __init__( self.turn_retry_max = 2 self.turn_retry_delay = 5.0 self.turn_retry_backoff_cap = 30.0 + # Empty-turn backstop (oneshot only): a turn that ends with no tool + # calls and no content is a provider failure shape, not "done". An + # autonomous run must not exit on it while budget remains — nudge + # and continue, bounded so a persistently empty endpoint still + # terminates. Interactive keeps the historical break: there the + # user sees the notice and simply speaks again. + self._empty_turns = 0 + self._empty_turn_max = 2 # Budget-aware convergence nudge (oneshot only): as the turn budget is # consumed, tell the agent to switch approach if plateaued, lock in an # already-passing result, then finalize. Covers productive-but-stuck @@ -1005,9 +1028,14 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: # by turn_retry_max, which against a stalled endpoint measured # 819s of silent wall clock from a 60s configured timeout. A # budget spent failing fast is still re-attempted — see - # _budget_burned_slowly. We only retry when nothing has been - # streamed yet; once content is yielded, a retry would duplicate - # output, so that case degrades to a partial turn instead. + # _budget_burned_slowly. We only retry when nothing user-visible + # was streamed yet; once content is yielded, a retry would + # duplicate output, so that case degrades to a partial turn + # instead. Reasoning chunks do not count as emitted: they are + # never yielded to the user, so retrying a stream that carried + # only thinking duplicates nothing. A stalled stream of pure + # reasoning otherwise slipped past both retry layers and ended + # the run as an empty turn (dna-insert, 2026-10-12__fix-071). turn_attempt = 0 while True: full_content = "" @@ -1035,7 +1063,6 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: yield chunk.delta_content if chunk.delta_reasoning_content: - emitted = True full_reasoning += chunk.delta_reasoning_content if chunk.tool_calls: @@ -1142,6 +1169,8 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: self._silent_tool_calls = 0 else: self._silent_tool_calls += len(tool_calls) + if full_content.strip() or tool_calls: + self._empty_turns = 0 for tc in tool_calls: if tc.name in ("delegate", "subagent_invoke"): self.turn_subagents += 1 @@ -1242,6 +1271,28 @@ async def _run_stream_body(self, user_input) -> AsyncIterator[str]: self.messages.append(final_msg) last_content = full_content else: + if ( + self.oneshot + and self._empty_turns < self._empty_turn_max + ): + # A stall that slipped both retry layers (reasoning- + # only stream, then a clean-but-empty stop) must not + # pass for "done": with budget left, exiting here is + # how dna-insert died at 562/1800s with the + # deliverable never written (2026-10-12__fix-071). + self._empty_turns += 1 + nudge = ( + "[Empty response: the model returned no content " + "and no tool calls. The task is not finished — " + "keep working: re-read your plan file and take " + "the next step; if a deliverable file is " + "required, make sure it exists on disk.]" + ) + yield f"\n{nudge}\n" + nudge_msg = {"role": "user", "content": nudge} + self.messages.append(nudge_msg) + messages_with_system.append(nudge_msg) + continue yield ( "\n[Model returned no content; the model name " "may not exist or the service is unresponsive]\n" diff --git a/dashscope/acli/cli/runners.py b/dashscope/acli/cli/runners.py index 5a63ca6..23eac00 100644 --- a/dashscope/acli/cli/runners.py +++ b/dashscope/acli/cli/runners.py @@ -230,6 +230,13 @@ async def _run_oneshot(config: Config, prompt: str): get_mcp_clients_fn=lambda: _mcp_clients, ) + # Vision too: without it, oneshot (every bench run 071c–071k) offered + # no view_image at all — the prompt named a tool the model could not + # see — and read_file/run_command auto-perceive had no context. + from dashscope.acli.tools.vision import register_vision_tools + + register_vision_tools(config, get_provider_fn=get_provider) + from dashscope.acli.tools.evolution import register_evolution_tools register_evolution_tools(get_agent=lambda: agent) diff --git a/dashscope/acli/config.py b/dashscope/acli/config.py index c390a86..5c79cab 100644 --- a/dashscope/acli/config.py +++ b/dashscope/acli/config.py @@ -32,6 +32,7 @@ "qwen-max", "qwen-plus", "qwen-turbo", + "qwen3-vl-plus", "qwen-vl-max", "qwen-vl-plus", "qwen-omni-turbo", diff --git a/dashscope/acli/memory/reflection.py b/dashscope/acli/memory/reflection.py index 0271f35..be6e7fd 100644 --- a/dashscope/acli/memory/reflection.py +++ b/dashscope/acli/memory/reflection.py @@ -745,8 +745,10 @@ def time_budget_hint( these runs lack. Returns "" below ``soft_ratio`` or when ``budget_sec`` is unset/<=0; - a secure-the-deliverable nudge in the soft band; a best-version-on-disk - finalize nudge at/after ``hard_ratio``. + a secure-the-deliverable nudge in the soft band; a finalize nudge + at/after ``hard_ratio`` that forces a partial write when the disk is + still empty — "best version on disk" means nothing to a run that has + written nothing (solve-sudoku in 2026-10-12__medium12 died that way). """ if budget_sec <= 0 or elapsed_sec < 0: return "" @@ -762,9 +764,12 @@ def time_budget_hint( f"({remaining_i}s left).\n" "Do NOT start a new approach — there is no time left to debug " "it. Right now:\n" - "1. Make sure the deliverable file on disk is the best version " - "you have produced (overwrite a broken draft with an older " - "working one if needed).\n" + "1. If the required deliverable file is not on disk yet, write " + "your current best partial answer to it immediately — an " + "incomplete or placeholder version can still score; an empty " + "path scores zero. If one is on disk, make sure it is the best " + "version you have produced (overwrite a broken draft with an " + "older working one if needed).\n" "2. Run ONE final verification against the acceptance " "criterion.\n" "3. Finish.\n" diff --git a/dashscope/acli/tools/filesystem.py b/dashscope/acli/tools/filesystem.py index 405407f..4612789 100644 --- a/dashscope/acli/tools/filesystem.py +++ b/dashscope/acli/tools/filesystem.py @@ -28,6 +28,26 @@ # blow up the context on its own. _READ_MAX_CHARS = 100_000 +# Binary image formats read_file redirects to view_image: decoding them as +# text returns replacement-char noise that is never what the caller wants +# (solve-sudoku, 2026-10-13__fix-071e, read the 23KB jpeg this way and +# perceived none of it). The redirect alone proved ignorable — 071i read +# it at t=136s and was back to pixel scripting 3s later — so read_file now +# auto-perceives the image inline when a vision context is registered, and +# only prints the redirect when perception is unavailable. SVG stays +# readable — it is XML markup, not binary. +_IMAGE_BINARY_EXTS = { + ".png", + ".jpg", + ".jpeg", + ".gif", + ".bmp", + ".webp", + ".tif", + ".tiff", + ".heic", +} + # --------------------------------------------------------------------------- # Tools @@ -56,6 +76,21 @@ def read_file( if not os.path.isfile(path): return f"Error: file not found - {path}" + if os.path.splitext(path)[1].lower() in _IMAGE_BINARY_EXTS: + from dashscope.acli.tools import vision + + perceived = vision.auto_perceive(path) + if perceived is not None: + return perceived + return ( + f"[{path} is an image; reading it as text returns binary " + "noise, not its content. To perceive what the image shows — " + "text, digits, a chart, geometry — call view_image on this " + "path, or run tesseract OCR when exact text matters. For " + "metadata only (format, dimensions), run_command with " + "file/identify instead.]" + ) + with open(path, "r", encoding="utf-8", errors="replace") as f: lines = f.readlines() diff --git a/dashscope/acli/tools/shell.py b/dashscope/acli/tools/shell.py index 7514047..6219703 100644 --- a/dashscope/acli/tools/shell.py +++ b/dashscope/acli/tools/shell.py @@ -28,6 +28,157 @@ # POSIX, hence getattr. _NEW_GROUP_FLAGS = getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0) +# Moment-of-relevance tool discovery: when a command touches an image +# file, or its output is a terminal approximation of something visual +# (ASCII-art render), a tip reminds the model that perception belongs to +# view_image. A static system-prompt rule is invisible once the model +# has framed the task as hand-rolled CV — solve-sudoku and gcode-to-text +# burned five runs between them on terminal ASCII cell/glyph extraction +# with view_image in the schema the whole time (zero mentions of it in +# the transcripts). +_IMAGE_PATH_RE = re.compile( + r"\.(?:png|jpe?g|gif|bmp|webp|tiff?|svg|heic)\b", + re.IGNORECASE, +) +_VISION_TIP_MAX = 3 +_vision_tip_count = 0 + +# Escalating wording by sighting ordinal: a soft pointer first, then a +# directive with the evidence, then blunt. solve-sudoku (071e) saw the +# soft tip at t=6s — before any route commitment — and still spent the +# run on read_file binary dumps and pixel scripts, so repetition alone +# is not the lever; the tone has to change. +# Escalating wording by sighting ordinal — but the first sighting is +# already directive. solve-sudoku (071e/071g) ignored the soft opener at +# t=6s twice and committed to hand-rolled extraction; the directive tone +# that later tips carried moved tooling within minutes (tesseract +# installed), just too late to rebuild the route around it. The verdict +# has to arrive before the route does. +_VISION_TIPS = ( + "this looks like image-perception work, and terminal/pixel " + "approximations lose exactly the detail the answer depends on — " + "use a purpose-built tool from the start: call view_image on the " + "image (for data you rendered, render a PNG first and call it on " + "that); for exact digit-level text, run a local OCR engine such " + "as tesseract on the image file", + "the proxy route has already started, and hand-rolled pixel " + "extraction has produced wrong answers in practice — stop and " + "switch: view_image on the image (or a PNG render), or tesseract " + "OCR for exact text", + "if you keep your own extraction, cross-check it with an " + "independent method (view_image, or tesseract for exact text) — " + "one misread value quietly invalidates everything built on it", +) + + +def _looks_like_ascii_art(output: str) -> bool: + """Grid of low-alphabet lines: the signature of rendering to text. + + A binary numpy matrix print qualifies too — and should, since + eyeballing a bitmap in text form is the same failure shape. Hex + dumps (rich per-line alphabet) and markdown tables (short lines or + rich alphabet) stay out. + + Judged on the grid lines alone: the truncation notice and [stderr] + marker this tool appends are prose, and counting them in the alphabet + silenced the detector on every large render (gcode-to-text, + 2026-10-13__fix-071d). Renderers also rstrip rows, so sparse art + comes in under 40 chars a line — the floor is 30, plus an absolute + grid-line count so progress-bar spam and `seq` output stay out. + """ + lines = [ln for ln in output.split("\n") if ln.strip()] + if len(lines) < 10: + return False + grid = [ln for ln in lines if len(ln) >= 30 and len(set(ln)) <= 8] + if len(grid) < 10 or len(grid) < 0.5 * len(lines): + return False + return len(set("".join(grid))) <= 12 + + +def _vision_inject(command: str) -> str: + """Perception delivered into the command result itself. + + read_file's auto-perceive only fires when the model opens the image + with read_file — but the bench evidence (071j) is that the model's + perception channel is run_command: `file sudoku.jpeg` at t=0s, then + heredoc pixel scripts for the rest of the budget, never a read_file. + When a command references an image file, resolve it (absolute, + process cwd, or a `cd` target earlier in the command) and inject the + auto-perception inline. SVG is excluded: it is XML markup the model + reads fine as text. Falls back to the tip when no image resolves or + the hook is unavailable. + """ + if os.environ.get("ACLI_AUTO_VISION", "1") == "0": + return "" + if not _IMAGE_PATH_RE.search(command): + return "" + from dashscope.acli.tools import vision + + if not vision.perception_available(): + return "" + + tokens = re.split(r"[\s\"'()=<>]+", command) + candidates = [] + for tok in tokens: + tok = tok.strip(";,:") + if ( + _IMAGE_PATH_RE.search(tok) + and not tok.lower().endswith(".svg") + and tok not in candidates + ): + candidates.append(tok) + if not candidates: + return "" + + bases = [os.getcwd()] + bases += re.findall(r"(?:^|&&|\|\||;)\s*cd\s+([^\s;&|]+)", command) + for cand in candidates: + if os.path.isabs(cand): + trials = [cand] + else: + trials = [os.path.join(b, cand) for b in bases] + for p in trials: + if os.path.isfile(p): + perceived = vision.auto_perceive(p, via="command") + if perceived is not None: + return "\n" + perceived + break + return "" + + +def _vision_tip(command: str, output: str) -> str: + """Pointer at view_image, fired at the moment perception is on the + table. Covers reading an existing image, writing one (perceive your + own render), and the ASCII-art fallback shape. ACLI_VISION_TIP=0 + disables. Once perception has actually happened — a view_image call, + or a read_file auto-perception — the tool-choice battle is over: + tips stop instead of nagging a model that already has the image + content (and may be rendering PNGs for it).""" + global _vision_tip_count + if _vision_tip_count >= _VISION_TIP_MAX: + return "" + if os.environ.get("ACLI_VISION_TIP", "1") == "0": + return "" + from dashscope.acli.tools import vision + + if vision.view_image_call_count() > 0 or vision.auto_perceive_count() > 0: + return "" + if _IMAGE_PATH_RE.search(command): + reason = "this command references an image file" + elif _looks_like_ascii_art(output): + reason = "this output is a terminal approximation of something visual" + else: + return "" + text = _VISION_TIPS[min(_vision_tip_count, len(_VISION_TIPS) - 1)] + _vision_tip_count += 1 + return f"\n[tip: {reason} — {text}]" + + +def _reset_vision_tip() -> None: + """Test hook: the tip budget is per-process, suites need a clean slate.""" + global _vision_tip_count + _vision_tip_count = 0 + def default_timeout() -> int: """Seconds a run_command call gets before it is killed. @@ -792,4 +943,8 @@ async def run_command(command: str, timeout: int | None = None) -> str: ) exit_info = f"\n[exit code: {proc.returncode}]" - return output.strip() + exit_info + return ( + output.strip() + + exit_info + + (_vision_inject(command) or _vision_tip(command, output)) + ) diff --git a/dashscope/acli/tools/vision.py b/dashscope/acli/tools/vision.py index 8392e9a..d45e30c 100644 --- a/dashscope/acli/tools/vision.py +++ b/dashscope/acli/tools/vision.py @@ -13,6 +13,14 @@ ``view_image`` auto-slices them with Pillow (when installed) and runs one vision call per slice. Without Pillow it falls back to sending the whole image as-is. + +``auto_perceive`` is the read_file hook: nine bench runs (071c–071i) +showed the model reads an image as text exactly once, receives the +"call view_image / tesseract" redirect, and goes back to pixel +scripting within seconds — advice is ignorable, delivered perception +is not. When read_file hits a binary image it calls ``auto_perceive``, +which runs the same vision pipeline inline and injects the result, +falling back to the text redirect whenever perception is unavailable. """ from __future__ import annotations @@ -21,6 +29,8 @@ import base64 import io import math +import os +import threading from typing import Callable from dashscope.acli.config import Config, is_vision_model @@ -31,7 +41,12 @@ ) _FALLBACK_VISION_MODELS = { - "tongyi": "qwen-vl-max", + # qwen-vl-max is retired on the public DashScope endpoint (404 "Model + # not exist"; qwen-vl-max-latest exists but is 403-gated on accounts + # without access). qwen3-vl-plus is the current generally-available VL + # model — verified against the live endpoint 2026-10-13 after the + # stale fallback silently broke every view_image call. + "tongyi": "qwen3-vl-plus", "openai": "gpt-4o", "anthropic": "claude-sonnet-4-20250514", } @@ -45,6 +60,72 @@ _SLICE_MAX_COUNT = 32 _SLICE_JPEG_QUALITY = 88 +# Region crops (the ``region`` parameter): small crops are enlarged +# before the vision call so dense content stays legible. +_REGION_MIN_SHORT_SIDE = 768 +_REGION_MAX_UPSCALE = 4.0 + +# Independent samples per region read. Region reads exist for +# exact-character verification, where one sample is a coin flip at the +# recognizer's noise floor — gcode-to-text 071r got i/ï/1 for the same +# glyph across identical crops and 071s committed after single reads of +# each region, never seeing the disagreements. Sampling surfaces the +# noise in the result text so it can be adjudicated instead of trusted. +# Runs concurrently: wall-clock stays ~one read. Env-overridable. +_REGION_SAMPLES = int(os.environ.get("ACLI_REGION_SAMPLES", "3")) + +# Adopted-once signal for the run_command vision tip: after the first +# view_image call the tool-choice nudges stop (see shell._vision_tip). +_view_image_calls = 0 + +# Hard cap on one vision call. gcode-to-text (071n) ended with a +# view_image await that produced no output for 700+s — the spinner +# froze, the turn never returned, and the harness watchdog reaped the +# container before the in-container cap could. A vision read that +# takes minutes is indistinguishable from a hang; fail it and let the +# model retry or narrow the ask. Env-overridable for slow endpoints. +_VISION_CALL_TIMEOUT = int(os.environ.get("ACLI_VISION_TIMEOUT", "300")) + + +def view_image_call_count() -> int: + return _view_image_calls + + +def _reset_view_image_calls() -> None: + """Test hook: the counter is per-process, suites need a clean slate.""" + global _view_image_calls + _view_image_calls = 0 + + +# Auto-perception state. _PERCEPTION_CTX is stored by +# register_vision_tools so the read_file hook can reach the active +# provider/config without threading them through the filesystem layer. +_PERCEPTION_CTX = None +_auto_perceive_attempts = 0 +_auto_perceived_paths: set = set() +_AUTO_PERCEIVE_MAX = 5 +# A sliced oversized image runs its calls concurrently; 240s bounds the +# tail without cutting realistic reads. +_AUTO_PERCEIVE_TIMEOUT = 240 + + +def auto_perceive_count() -> int: + return _auto_perceive_attempts + + +def _reset_auto_perceive() -> None: + """Test hook: attempts are per-process, suites need a clean slate.""" + global _auto_perceive_attempts + _auto_perceive_attempts = 0 + _auto_perceived_paths.clear() + + +def _auto_perceive_budget() -> int: + try: + return int(os.environ.get("ACLI_AUTO_VISION_MAX", _AUTO_PERCEIVE_MAX)) + except ValueError: + return _AUTO_PERCEIVE_MAX + def pick_vision_model(config: Config) -> str: """Choose a vision-capable model on the active provider. @@ -61,7 +142,7 @@ def pick_vision_model(config: Config) -> str: ext = find_provider(config.provider) if ext and ext.vision_models: return ext.vision_models[0] - return _FALLBACK_VISION_MODELS.get(config.provider, "qwen-vl-max") + return _FALLBACK_VISION_MODELS.get(config.provider, "qwen3-vl-plus") def plan_slices(width: int, height: int): @@ -108,16 +189,65 @@ def plan_slices(width: int, height: int): def _slice_data_urls(image, boxes) -> list: """Crop ``boxes`` out of a PIL image, JPEG-encode as data URLs.""" - urls = [] - for box in boxes: - crop = image.crop(box) - if crop.mode not in ("RGB", "L"): - crop = crop.convert("RGB") - buf = io.BytesIO() - crop.save(buf, format="JPEG", quality=_SLICE_JPEG_QUALITY) - b64 = base64.b64encode(buf.getvalue()).decode() - urls.append(f"data:image/jpeg;base64,{b64}") - return urls + return [_encode_jpeg(image.crop(box)) for box in boxes] + + +def _encode_jpeg(crop) -> str: + if crop.mode not in ("RGB", "L"): + crop = crop.convert("RGB") + buf = io.BytesIO() + crop.save(buf, format="JPEG", quality=_SLICE_JPEG_QUALITY) + b64 = base64.b64encode(buf.getvalue()).decode() + return f"data:image/jpeg;base64,{b64}" + + +def parse_region(region: str, width: int, height: int) -> tuple: + """Parse ``x0,y0,x1,y1`` fractions into a pixel crop box. + + Fractions keep the parameter resolution-independent: the model can + ask for "the top-left ninth" as 0,0,0.333,0.333 without knowing the + image size. Pixels would invite off-by-one hallucinations. + """ + parts = [p.strip() for p in region.split(",")] + if len(parts) != 4: + raise ValueError( + f'expected "x0,y0,x1,y1" (4 comma-separated fractions), ' + f"got {region!r}", + ) + try: + x0, y0, x1, y1 = (float(p) for p in parts) + except ValueError: + raise ValueError( + f"region values must be numbers, got {region!r}", + ) from None + if not (0 <= x0 < x1 <= 1 and 0 <= y0 < y1 <= 1): + raise ValueError( + f"region must satisfy 0<=x0= _REGION_MIN_SHORT_SIDE: + return crop + factor = min(_REGION_MAX_UPSCALE, _REGION_MIN_SHORT_SIDE / short) + from PIL import Image + + return crop.resize( + (max(1, round(w * factor)), max(1, round(h * factor))), + Image.LANCZOS, + ) async def _ask_vision(provider, data_url: str, text: str) -> str: @@ -130,12 +260,287 @@ async def _ask_vision(provider, data_url: str, text: str) -> str: ], }, ] - response = await provider.chat(messages) + try: + response = await asyncio.wait_for( + provider.chat(messages), + timeout=_VISION_CALL_TIMEOUT, + ) + except (TimeoutError, asyncio.TimeoutError): + # asyncio.TimeoutError is distinct from TimeoutError on 3.10. + return ( + f"Error: vision call timed out after {_VISION_CALL_TIMEOUT}s; " + "retry, or narrow the region/question." + ) return response.content or "(vision model returned no content)" +def _aggregate_region_answers(answers: list[str]) -> str: + """Merge the independent samples of one region read. + + A unanimous vote returns the single answer, indistinguishable from a + one-shot read. Any split shows every sample with its vote count so + the disagreement is visible in the result — hiding it would be the + single-read coin flip all over again (gcode-to-text 071s). + """ + if not answers: + return "(vision model returned no content)" + if len(set(answers)) == 1: + return answers[0] + samples = "\n\n".join( + f"[sample {i + 1}/{len(answers)}]\n{a}" for i, a in enumerate(answers) + ) + return ( + samples + + "\n\n[(these independent reads of the same crop disagree — do " + "not just pick one: re-read a tighter crop of only the contested " + "glyph with a feature question (dot, serif, enclosed counters " + "present or absent), and prefer the candidate that makes the " + "assembled string read as coherent text)]" + ) + + +async def _perceive_region( + config: Config, + get_provider_fn: Callable, + provider, + safe_path: str, + question: str, + slice_mode: str, + region: str, +) -> str: + """Region read: real pixel crop, upscaled, sampled _REGION_SAMPLES×.""" + # A real pixel crop beats asking about a region of the full + # image: solve-sudoku 071l did everything procedurally right + # (whole read, band re-reads, box re-reads) but the crops were + # attention-simulated by the VL and two cells came back wrong — + # the model then "resolved" the inconsistency by guessing and + # solved a different puzzle. + try: + from PIL import Image + except ImportError: + # Pillow is optional and the offline bench bundle is + # pure-python (gcode-to-text 071o: three region calls + # errored in a row). Degrade to the whole-image read with + # working alternatives instead of failing the verification + # loop outright. + whole = await _perceive_image( + config, + get_provider_fn, + safe_path, + question, + slice_mode, + ) + return ( + # Warning FIRST: appended after the read, the note let + # gcode-to-text 071u mistake two whole-image reads for + # two independent region verifications of its answer. + "[REGION READ FAILED — Pillow is not installed in this " + "environment. What follows is a WHOLE-IMAGE read, NOT " + "the requested crop: it is not independent verification " + "of the region, and it must not be treated as " + "confirmation of an earlier whole-image read. " + "`pip install pillow` enables real region crops; or " + "zoom the region into its own image file and read " + "that.]\n\n" + whole + ) + try: + with Image.open(safe_path) as im: + im.load() + box = parse_region(region, *im.size) + crop = _upscale_for_vision(im.crop(box)) + data_url = _encode_jpeg(crop) + except ValueError as e: + return f"Error: invalid region: {e}" + except Exception as e: # pylint: disable=broad-except + return f"Error: failed to crop region: {e}" + try: + answers = await asyncio.gather( + *( + _ask_vision(provider, data_url, question) + for _ in range(max(_REGION_SAMPLES, 1)) + ), + ) + except Exception as e: # pylint: disable=broad-except + return f"Error: vision call failed: {e}" + return _aggregate_region_answers(list(answers)) + + +async def _perceive_image( + config: Config, + get_provider_fn: Callable, + safe_path: str, + question: str, + slice_mode: str, + region: str = "", +) -> str: + """Run one vision read of ``safe_path`` (pre-validated). + + Shared by the view_image tool and the read_file auto-perceive hook; + errors are returned as text, matching the tool contract. + """ + model = pick_vision_model(config) + provider = get_provider_fn( + config.provider, + model, + config.api_key, + base_url=config.base_url or None, + ) + + if region: + return await _perceive_region( + config, + get_provider_fn, + provider, + safe_path, + question, + slice_mode, + region, + ) + + boxes = None + image = None + if slice_mode != "never": + try: + from PIL import Image + + image = Image.open(safe_path) + image.load() + boxes = plan_slices(*image.size) + except Exception: # pylint: disable=broad-except + # No Pillow, or not a decodable image: fall back to the + # whole-image path and let it report the real error. + boxes = None + image = None + + if boxes: + axis = ( + "top-to-bottom" + if image.height >= image.width + else ("left-to-right") + ) + urls = _slice_data_urls(image, boxes) + image.close() + total = len(urls) + tasks = [ + _ask_vision( + provider, + url, + ( + f"Slice {i + 1}/{total} ({axis}) of one large " + "image. " + question + ), + ) + for i, url in enumerate(urls) + ] + results = await asyncio.gather(*tasks, return_exceptions=True) + parts = [ + f"(Image auto-split into {total} slices {axis}; " + "results per slice below.)", + ] + for i, r in enumerate(results, start=1): + if isinstance(r, BaseException): + parts.append(f"--- slice {i}/{total} ---\nError: {r}") + else: + parts.append(f"--- slice {i}/{total} ---\n{r}") + return "\n\n".join(parts) + + try: + from dashscope.acli.utils.images import image_to_data_url + + data_url = image_to_data_url(safe_path) + except (ValueError, OSError) as e: + return f"Error: failed to read image: {e}" + try: + return await _ask_vision(provider, data_url, question) + except Exception as e: # pylint: disable=broad-except + return f"Error: vision call failed: {e}" + + +def perception_available() -> bool: + """Whether an agent has registered a perception context.""" + return _PERCEPTION_CTX is not None + + +def auto_perceive(safe_path: str, *, via: str = "read_file") -> str | None: + """Perceive an image inline and return the read, or None. + + Called by read_file (an image opened as text) and by run_command (a + command referencing an image). ``safe_path`` must already exist. + Returns None — caller then falls back to its text hint — when the + hook is disabled (ACLI_AUTO_VISION=0), unconfigured (no agent + registered a perception context), this same image was already + perceived once (the content is in context; repeats just burn + latency), out of budget (ACLI_AUTO_VISION_MAX successful+failed + attempts per process), or the pipeline itself raises. + + The agent loop runs sync tools inline on the event-loop thread, so + the async pipeline runs on a helper thread with its own loop — the + same blocking shape run_command already has. + """ + global _auto_perceive_attempts + if os.environ.get("ACLI_AUTO_VISION", "1") == "0": + return None + if _PERCEPTION_CTX is None: + return None + real = os.path.realpath(safe_path) + if real in _auto_perceived_paths: + return None + if _auto_perceive_attempts >= _auto_perceive_budget(): + return None + _auto_perceive_attempts += 1 + config, get_provider_fn = _PERCEPTION_CTX + question = ( + "Describe this image in detail. If it contains text, digits or " + "tables, transcribe them verbatim." + ) + + box: list = [] + + def _runner(): + try: + box.append( + asyncio.run( + _perceive_image( + config, + get_provider_fn, + real, + question, + "auto", + ), + ), + ) + except BaseException: # pylint: disable=broad-except + box.append(None) + + worker = threading.Thread(target=_runner, daemon=True) + worker.start() + worker.join(timeout=_AUTO_PERCEIVE_TIMEOUT) + if not box or box[0] is None: + return None + _auto_perceived_paths.add(real) + if via == "command": + prefix = ( + "[this command references an image; its content was " + "perceived with a vision model (auto-perception):]" + ) + else: + prefix = ( + "[read_file cannot decode images as text; this image was " + "perceived with a vision model (auto-perception):]" + ) + return ( + f"{prefix}\n" + f"{box[0]}\n" + "[for exact digit-level text cross-check with tesseract OCR on " + "the file, or call view_image with a focused question or a " + "cropped region.]" + ) + + def register_vision_tools(config: Config, *, get_provider_fn: Callable): """Register ``view_image``; called once after agent construction.""" + global _PERCEPTION_CTX + _PERCEPTION_CTX = (config, get_provider_fn) async def view_image( image_path: str, @@ -144,7 +549,11 @@ async def view_image( "tables, transcribe them verbatim." ), slice_mode: str = "auto", + region: str = "", ) -> str: + global _view_image_calls + _view_image_calls += 1 + from dashscope.acli.utils.paths import validate_path try: @@ -152,81 +561,62 @@ async def view_image( except (ValueError, OSError) as e: return f"Error: failed to read image: {e}" - model = pick_vision_model(config) - provider = get_provider_fn( - config.provider, - model, - config.api_key, - base_url=config.base_url or None, + return await _perceive_image( + config, + get_provider_fn, + safe_path, + question, + slice_mode, + region, ) - boxes = None - image = None - if slice_mode != "never": - try: - from PIL import Image - - image = Image.open(safe_path) - image.load() - boxes = plan_slices(*image.size) - except Exception: # pylint: disable=broad-except - # No Pillow, or not a decodable image: fall back to the - # whole-image path and let it report the real error. - boxes = None - image = None - - if boxes: - axis = ( - "top-to-bottom" - if image.height >= image.width - else ("left-to-right") - ) - urls = _slice_data_urls(image, boxes) - image.close() - total = len(urls) - tasks = [ - _ask_vision( - provider, - url, - ( - f"Slice {i + 1}/{total} ({axis}) of one large " - "image. " + question - ), - ) - for i, url in enumerate(urls) - ] - results = await asyncio.gather(*tasks, return_exceptions=True) - parts = [ - f"(Image auto-split into {total} slices {axis}; " - "results per slice below.)", - ] - for i, r in enumerate(results, start=1): - if isinstance(r, BaseException): - parts.append(f"--- slice {i}/{total} ---\nError: {r}") - else: - parts.append(f"--- slice {i}/{total} ---\n{r}") - return "\n\n".join(parts) - - try: - from dashscope.acli.utils.images import image_to_data_url - - data_url = image_to_data_url(safe_path) - except (ValueError, OSError) as e: - return f"Error: failed to read image: {e}" - try: - return await _ask_vision(provider, data_url, question) - except Exception as e: # pylint: disable=broad-except - return f"Error: vision call failed: {e}" - registry.register( ToolDefinition( name="view_image", description=( - "Analyze a local image file (screenshot, photo, chart) " - "with a vision model: OCR/transcribe text, describe " - "layout or content. Very tall/wide images are split " - "automatically. Use after scrape_web_screenshot or " - "camera_capture, or whenever you need to read an image." + "Analyze a local image file (screenshot, photo, chart, " + "scan, render) with a vision model: OCR/transcribe text " + "or digits, describe layout or content. Very tall/wide " + "images are split automatically. Use whenever a task " + "requires reading an image's content — including images " + "you rendered yourself from data (plots, geometry, " + "toolpaths) — instead of approximating the perception " + "with terminal output or pixel scripting. For grid-like " + "content (tables, digit grids), read the whole image " + "first, then re-read it block by block with the region " + "parameter: region is a real pixel crop (upscaled " + "automatically), far more accurate than asking about a " + "region of the full image. When every character matters " + "(codes, serial numbers, mixed-case text), never trust " + "one whole-image read of small or stylized text: " + "verify it with region reads of a few characters each " + "before committing to an answer, and overlap adjacent " + "crops so no character sits on a crop boundary. When " + "re-reads of one glyph keep disagreeing, more identical " + "re-reads will not converge — the glyph is at the " + "recognizer's noise floor: adjudicate with a feature " + "question (a dot, serif, or enclosed counter present or " + "absent) on a crop of only that glyph, and prefer the " + "candidate that makes the assembled string read as " + "coherent text over the one that does not. Overlapping " + "crops also cross-check each other: characters shared by " + "two crops must read the same in both, and if they do " + "not, re-read the contested span before committing. And " + "when you rendered the image yourself, ambiguous reads " + "are a rendering problem, not a perception limit: " + "re-render larger or higher-contrast and read the new " + "render instead of re-reading the same pixels. Glyphs " + "from confusable sets (0/O/o, 1/l/i/I, 5/S, 8/B, and " + "same-shape letter case like cC, kK, pP) need " + "feature-level confirmation — dot, serif, counter, " + "relative height — not just repeated transcription: " + "stylized fonts make wrong glyphs look unambiguous. " + "Context is what corrupts stylized reads: word-shaped " + "crops get pattern-completed into expected words, while " + "a crop of one isolated glyph can only be read as " + "pixels — settle contested characters glyph by glyph, " + "and never let an expected word override what isolated " + "glyphs clearly show." ), permission=PermissionLevel.AUTO, func=view_image, @@ -256,6 +646,27 @@ async def view_image( "image in one call" ), }, + "region": { + "type": "string", + "description": ( + 'Crop before reading: "x0,y0,x1,y1" as ' + "fractions of width/height (0-1, origin " + "top-left), e.g. the middle horizontal " + 'third is "0,0.333,1,0.667". Overrides ' + "slicing. For dense grids/tables transcribe " + "a few cells per call; for exact-character " + "work (codes, labels, stylized or small " + "text) crop to a few characters per call. " + "When tiling a long string with several " + "crops, overlap adjacent crops: a character " + "at a crop boundary is exactly the one most " + "likely to be dropped or duplicated, so " + "every character must appear fully inside " + "at least one crop. Each region read samples " + "the vision model several times and shows " + "every answer when they disagree." + ), + }, }, "required": ["image_path"], }, diff --git a/tests/unit/test_acli_vision_tool.py b/tests/unit/test_acli_vision_tool.py index d0d0f2e..92e2f2f 100644 --- a/tests/unit/test_acli_vision_tool.py +++ b/tests/unit/test_acli_vision_tool.py @@ -213,7 +213,9 @@ def test_fallback_per_provider(self, monkeypatch): "dashscope.acli.extensions.find_provider", lambda name: None, ) - assert vision.pick_vision_model(_config()) == "qwen-vl-max" + # qwen-vl-max is retired on the public DashScope endpoint (404); + # the tongyi fallback must be a currently-served VL model. + assert vision.pick_vision_model(_config()) == "qwen3-vl-plus" cfg = _config() cfg.provider = "openai" assert vision.pick_vision_model(cfg) == "gpt-4o"