From ec87cb9a215ac11864d150bc80a63ec25140e40e Mon Sep 17 00:00:00 2001 From: wannT-1 <321008868@qq.com> Date: Sun, 25 Oct 2026 09:57:13 +0800 Subject: [PATCH] fix(chart-visualization): wire generate_spreadsheet and fail CLI nonzero (#7395) * fix(chart-visualization): wire generate_spreadsheet and fail CLI nonzero SKILL.md already selects generate_spreadsheet and the reference file documents it, but CHART_TYPE_MAP had no entry, so the CLI reported Unknown tool. Map it to the AntV spreadsheet type and exit non-zero after any unknown tool, missing tool, or HTTP failure. Successful chart URLs stay on stdout. Co-authored-by: wannT-1 <200890097+wannT-1@users.noreply.github.com> * docs(agents): fold chart CLI failures into the shared stderr/stdout note The sandbox guidance chain was over the AG002 hard limit. Share one nonzero-exit rule for video, podcast, and chart-visualization CLIs. Co-authored-by: wannT-1 --------- Co-authored-by: wannT-1 <200890097+wannT-1@users.noreply.github.com> Co-authored-by: Willem Jiang Co-authored-by: wannT-1 --- AGENTS.md | 4 +- README.md | 5 + skills/public/chart-visualization/SKILL.md | 3 +- .../chart-visualization/scripts/generate.js | 9 + tests/skills/test_chart_visualization.py | 166 ++++++++++++++++++ 5 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 tests/skills/test_chart_visualization.py diff --git a/AGENTS.md b/AGENTS.md index 9cfdc99173..f4ab91ba4c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -226,8 +226,8 @@ Repo-wide conventions: Public skill script regressions live in `tests/skills/` and run in Skill Review CI independently of package review. Exercise dependency fallbacks with the optional import unavailable and mock HTTP transport to keep tests offline. - Video- and podcast-generation CLI failures must exit non-zero and write diagnostics to stderr; - keep success output on stdout and test the CLI boundary with offline transport. + Video, podcast, and chart-visualization CLI failures exit non-zero; + diagnostics on stderr, success URLs on stdout; test offline. - **Before pushing** — backend `make format`, frontend `pnpm check`. Backend CI enforces `ruff format --check`. - **Skill text encoding** — use explicit UTF-8 for textual skill resources. diff --git a/README.md b/README.md index 414d5c6acd..d70421ff9d 100644 --- a/README.md +++ b/README.md @@ -1662,6 +1662,11 @@ The bundled podcast-generation command returns a nonzero exit status and writes diagnostics to stderr when generation fails. Callers can check the status before presenting audio; an existing MP3 or generated transcript is not proof of success. +The bundled chart-visualization command does the same for an unknown chart tool, +a spec missing `tool`, or a chart-service HTTP failure. Successful chart URLs, +including spreadsheets (`generate_spreadsheet`), stay on stdout. A mixed batch +still prints URLs for the charts that succeeded, then exits with status 1. + When deferred skill discovery is enabled, `describe_skill` ranks installed skills by bounded, Unicode-normalized intent-term coverage across names and descriptions. Natural multi-term requests can therefore find a relevant skill without requiring one exact phrase, while exact `select:` and required-name `+prefix` lookups remain available. Ranked searches use up to 256 characters and return up to five results; exact `select:` lists are not truncated and return all requested catalog matches. A skill directory is a package boundary: once DeerFlow finds its `SKILL.md`, nested `SKILL.md` files under that package (for example evaluation fixtures) remain supporting data and are not registered as runtime skills. This applies to managed integration packs as well as public and custom skills. Namespace directories without their own `SKILL.md` can still group nested skills. diff --git a/skills/public/chart-visualization/SKILL.md b/skills/public/chart-visualization/SKILL.md index d7c6358d89..661add59fd 100644 --- a/skills/public/chart-visualization/SKILL.md +++ b/skills/public/chart-visualization/SKILL.md @@ -59,7 +59,8 @@ node ./scripts/generate.js '' ``` ### 4. Result Return -The script will output the URL of the generated chart image. +The script prints each generated chart's result on stdout (the image URL for chart types; text or JSON for map tools). +A missing `tool`, an unknown tool, or a failed chart-service request is written to stderr. The process exits with status 1 if any item in the batch fails, after printing URLs for the items that succeeded. Present a URL only when the command exits 0. Return the following to the user: - The image URL. - The complete `args` (specification) used for generation. diff --git a/skills/public/chart-visualization/scripts/generate.js b/skills/public/chart-visualization/scripts/generate.js index 7d5bc96511..ad46560341 100644 --- a/skills/public/chart-visualization/scripts/generate.js +++ b/skills/public/chart-visualization/scripts/generate.js @@ -25,6 +25,7 @@ const CHART_TYPE_MAP = { generate_radar_chart: "radar", generate_sankey_chart: "sankey", generate_scatter_chart: "scatter", + generate_spreadsheet: "spreadsheet", generate_treemap_chart: "treemap", generate_venn_chart: "venn", generate_violin_chart: "violin", @@ -116,6 +117,7 @@ async function main() { } const specs = Array.isArray(spec) ? spec : [spec]; + let failed = false; for (const item of specs) { const tool = item.tool; @@ -125,12 +127,14 @@ async function main() { console.error( `Error: 'tool' field missing in spec: ${JSON.stringify(item)}`, ); + failed = true; continue; } const chartType = CHART_TYPE_MAP[tool]; if (!chartType) { console.error(`Error: Unknown tool '${tool}'`); + failed = true; continue; } @@ -158,8 +162,13 @@ async function main() { } } catch (e) { console.error(`Error generating chart for ${tool}: ${e.message}`); + failed = true; } } + + if (failed) { + process.exit(1); + } } if (require.main === module) { diff --git a/tests/skills/test_chart_visualization.py b/tests/skills/test_chart_visualization.py new file mode 100644 index 0000000000..a7b1f03bfa --- /dev/null +++ b/tests/skills/test_chart_visualization.py @@ -0,0 +1,166 @@ +"""Chart-visualization CLI regressions: spreadsheet mapping and nonzero failures. + +The AntV request is mocked with a Node ``--require`` preload so the test stays +offline. Successful chart URLs stay on stdout; diagnostics stay on stderr. +""" + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT = REPO_ROOT / "skills" / "public" / "chart-visualization" / "scripts" / "generate.js" +SKILL = REPO_ROOT / "skills" / "public" / "chart-visualization" / "SKILL.md" +NODE = shutil.which("node") + +FETCH_PRELOAD = """\ +const fs = require("fs"); + +global.fetch = async (url, options) => { + const capture = process.env.CHART_VIS_CAPTURE; + if (capture) { + fs.appendFileSync( + capture, + JSON.stringify({ url: String(url), body: options && options.body }) + "\\n", + ); + } + const status = Number(process.env.CHART_VIS_HTTP_STATUS || 200); + if (status >= 400) { + return { + ok: false, + status, + text: async () => "upstream failed", + json: async () => ({}), + }; + } + return { + ok: true, + status: 200, + text: async () => "", + json: async () => ({ + success: true, + resultObj: "https://example.test/sheet.png", + }), + }; +}; +""" + + +pytestmark = pytest.mark.skipif(NODE is None, reason="node is required to run generate.js") + + +def _run(tmp_path, spec, *, http_status=None): + preload = tmp_path / "mock_fetch.js" + preload.write_text(FETCH_PRELOAD, encoding="utf-8") + capture = tmp_path / "capture.jsonl" + env = os.environ.copy() + env["CHART_VIS_CAPTURE"] = str(capture) + env["VIS_REQUEST_SERVER"] = "http://127.0.0.1:9/api/gpt-vis" + env.pop("SERVICE_ID", None) + if http_status is None: + env.pop("CHART_VIS_HTTP_STATUS", None) + else: + env["CHART_VIS_HTTP_STATUS"] = str(http_status) + + result = subprocess.run( + [NODE, "--require", str(preload), str(SCRIPT), json.dumps(spec)], + capture_output=True, + text=True, + check=False, + timeout=15, + env=env, + ) + calls = [] + if capture.exists(): + calls = [json.loads(line) for line in capture.read_text(encoding="utf-8").splitlines() if line] + return result, calls + + +def _chart_type_map(): + probe = subprocess.run( + [ + NODE, + "-e", + "const {CHART_TYPE_MAP}=require(process.argv[1]); process.stdout.write(JSON.stringify(CHART_TYPE_MAP));", + str(SCRIPT), + ], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + assert probe.returncode == 0, probe.stderr + return json.loads(probe.stdout) + + +def test_generate_spreadsheet_is_mapped_and_prints_url(tmp_path): + chart_types = _chart_type_map() + skill = SKILL.read_text(encoding="utf-8-sig") + assert "26 available options" in skill + assert len(chart_types) == 26 + assert chart_types["generate_spreadsheet"] == "spreadsheet" + + spec = { + "tool": "generate_spreadsheet", + "args": {"data": [{"name": "Jane", "age": 25}], "title": "People"}, + } + result, calls = _run(tmp_path, spec) + + assert result.returncode == 0 + assert result.stdout == "https://example.test/sheet.png\n" + assert result.stderr == "" + assert "Unknown tool" not in result.stderr + assert len(calls) == 1 + body = json.loads(calls[0]["body"]) + assert body["type"] == "spreadsheet" + assert body["source"] == "chart-visualization-creator" + assert body["data"] == [{"name": "Jane", "age": 25}] + assert body["title"] == "People" + assert calls[0]["url"] == "http://127.0.0.1:9/api/gpt-vis" + + +def test_unknown_tool_exits_nonzero_without_calling_the_service(tmp_path): + result, calls = _run(tmp_path, {"tool": "generate_not_a_chart", "args": {}}) + + assert result.returncode == 1 + assert result.stdout == "" + assert "Unknown tool 'generate_not_a_chart'" in result.stderr + assert calls == [] + + +def test_missing_tool_exits_nonzero(tmp_path): + result, calls = _run(tmp_path, {"args": {"data": []}}) + + assert result.returncode == 1 + assert result.stdout == "" + assert "tool" in result.stderr + assert calls == [] + + +def test_http_failure_exits_nonzero(tmp_path): + spec = {"tool": "generate_spreadsheet", "args": {"data": [{"name": "Jane"}]}} + result, calls = _run(tmp_path, spec, http_status=502) + + assert result.returncode == 1 + assert result.stdout == "" + assert "HTTP 502" in result.stderr + assert len(calls) == 1 + assert json.loads(calls[0]["body"])["type"] == "spreadsheet" + + +def test_batch_keeps_successful_urls_and_still_exits_nonzero(tmp_path): + spec = [ + {"tool": "generate_not_a_chart"}, + {"tool": "generate_spreadsheet", "args": {"data": [{"name": "Ada"}]}}, + ] + result, calls = _run(tmp_path, spec) + + assert result.returncode == 1 + assert result.stdout == "https://example.test/sheet.png\n" + assert "Unknown tool 'generate_not_a_chart'" in result.stderr + assert len(calls) == 1 + assert json.loads(calls[0]["body"])["type"] == "spreadsheet"