From dcb3a70fbf728bb5ab711798382ee466c399340b Mon Sep 17 00:00:00 2001 From: theater <180920469+Theater-ahyeon@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:48:57 +0800 Subject: [PATCH] fix(skills): support basename data-analysis exports (#7132) * fix(skills): support basename data-analysis exports Use the current directory when export output has no parent directory. * test(skills): cover data-analysis export paths Exercise real DuckDB queries, filesystem exports and cached CLI execution across CSV, JSON and Markdown. * docs(skills): record export path regression guidance Document basename and nested output expectations and the regression test command. * docs(skills): explain data-analysis export paths Clarify bare filenames, relative parent directories and absolute output paths. * docs: note supported data-analysis export paths Add a concise README explanation while retaining existing upstream documentation. * docs(skills): clarify export overwrite behavior Document that existing export targets are overwritten without warning, as suggested in PR #7132. Verified real CSV, JSON and Markdown overwrites; 27 export regression tests pass. --------- Co-authored-by: Willem Jiang --- README.md | 2 +- skills/public/data-analysis/AGENTS.md | 5 + skills/public/data-analysis/SKILL.md | 5 + .../public/data-analysis/scripts/analyze.py | 2 +- .../skills/test_data_analysis_export_paths.py | 106 ++++++++++++++++++ 5 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 tests/skills/test_data_analysis_export_paths.py diff --git a/README.md b/README.md index e8ed793f87..cd6788a71f 100644 --- a/README.md +++ b/README.md @@ -1725,7 +1725,7 @@ values nested in lists or structs, before opening the destination. Filter them or replace them with `NULL` in SQL; the CLI reports an error and preserves any existing output rather than writing invalid JSON. -The **data-analysis** skill preserves each CSV file or Excel sheet when names collide across inputs. [Prepare its Python environment](skills/public/data-analysis/SKILL.md#environment-setup) explicitly with DuckDB and openpyxl; missing packages report setup instructions instead of installing themselves during analysis. Use the SQL names printed by `inspect` (such as `report`, `report_1`) for queries and summaries. Its cache follows ordered input basenames and contents, so renaming or reordering inputs rebuilds their table map instead of silently selecting a different source. Quoted original names also resolve without rewriting query text; apostrophes in upload paths and double quotes in column names are preserved through loading, inspection, queries, and summaries. +The **data-analysis** skill preserves each CSV file or Excel sheet when names collide across inputs. [Prepare its Python environment](skills/public/data-analysis/SKILL.md#environment-setup) explicitly with DuckDB and openpyxl; missing packages report setup instructions instead of installing themselves during analysis. Use the SQL names printed by `inspect` (such as `report`, `report_1`) for queries and summaries. Its cache follows ordered input basenames and contents, so renaming or reordering inputs rebuilds their table map instead of silently selecting a different source. Quoted original names also resolve without rewriting query text; apostrophes in upload paths and double quotes in column names are preserved through loading, inspection, queries, and summaries. CSV, JSON, and Markdown exports accept bare filenames such as `report.csv` in the current working directory as well as nested relative and absolute paths. After an answer loads skills, its toolbar includes **Skills used**. Hover over or click the icon to see the skills and their sources; hovering a skill name underlines it. Select a skill to inspect its `SKILL.md` in the resizable side panel (a drawer on mobile). The view uses snapshots captured during successful configured read-tool loads or explicit slash activation, so later edits or removal of a skill do not rewrite its history. Repeated loads appear once per run, in first-load order. Range reads and bounded snapshots are labeled as partial; older conversations without captured evidence do not show this menu. Copy returns the captured Markdown, including YAML frontmatter. Package-relative links and images remain readable references rather than navigating away from the conversation. Skill loads performed through other tools, such as shell commands, are not inferred from answer text. diff --git a/skills/public/data-analysis/AGENTS.md b/skills/public/data-analysis/AGENTS.md index dde75f7be5..1453814e79 100644 --- a/skills/public/data-analysis/AGENTS.md +++ b/skills/public/data-analysis/AGENTS.md @@ -24,6 +24,11 @@ Prepare DuckDB and openpyxl explicitly in the execution environment. Missing packages produce setup instructions without launching installers; keep those import-failure regressions offline in the root skill tests. +Export paths resolve relative to the caller's working directory, including bare +filenames. Keep real DuckDB regressions for CSV/JSON/Markdown, empty results, +existing and missing parent directories, and the cache-hit CLI path in +`tests/skills/test_data_analysis_export_paths.py`. + Markdown exports use the same cell escaping for headers and values. Escape backslashes before pipes and normalize CRLF, CR, and LF to inline `
` breaks; raw newlines must not split one query-result row across Markdown table rows. diff --git a/skills/public/data-analysis/SKILL.md b/skills/public/data-analysis/SKILL.md index f0daa233ee..90a398c683 100644 --- a/skills/public/data-analysis/SKILL.md +++ b/skills/public/data-analysis/SKILL.md @@ -95,6 +95,11 @@ Supported output formats (auto-detected from extension): - `.json` — JSON array of records - `.md` — Markdown table +Relative output paths are resolved from the current working directory. A bare +filename such as `report.csv` writes there directly; missing parent directories +in paths such as `reports/report.csv` are created automatically. +An existing file at the target path is overwritten without warning. + JSON result column names must be unique. Use explicit SQL aliases for repeated names, for example `SELECT a.id AS left_id, b.id AS right_id FROM ...`. Duplicate names cause an error and leave any existing output file untouched. diff --git a/skills/public/data-analysis/scripts/analyze.py b/skills/public/data-analysis/scripts/analyze.py index 67e08d1756..acc46e9662 100644 --- a/skills/public/data-analysis/scripts/analyze.py +++ b/skills/public/data-analysis/scripts/analyze.py @@ -352,7 +352,7 @@ def _markdown_cell(value: object) -> str: def _export_results(columns: list[str], rows: list[tuple], output_file: str) -> str: """Export query results to a file (CSV, JSON, or Markdown).""" - os.makedirs(os.path.dirname(output_file), exist_ok=True) + os.makedirs(os.path.dirname(output_file) or ".", exist_ok=True) ext = os.path.splitext(output_file)[1].lower() if ext == ".csv": diff --git a/tests/skills/test_data_analysis_export_paths.py b/tests/skills/test_data_analysis_export_paths.py new file mode 100644 index 0000000000..3c29eaf0f2 --- /dev/null +++ b/tests/skills/test_data_analysis_export_paths.py @@ -0,0 +1,106 @@ +"""Exercise output paths with real DuckDB queries and filesystem exports.""" + +import csv +import json +import os +import runpy +import subprocess +import sys +from contextlib import chdir +from pathlib import Path + +import pytest + +duckdb = pytest.importorskip("duckdb") +pytest.importorskip("openpyxl") +SCRIPT = ( + Path(__file__).resolve().parents[2] + / "skills/public/data-analysis/scripts/analyze.py" +) + + +def assert_export(path, extension, empty=False): + if extension == "csv": + with path.open(encoding="utf-8", newline="") as handle: + assert list(csv.reader(handle)) == ( + [["answer", "label"]] + if empty + else [["answer", "label"], ["42", "分析"]] + ) + elif extension == "json": + assert json.loads(path.read_text(encoding="utf-8")) == ( + [] if empty else [{"answer": 42, "label": "分析"}] + ) + else: + assert path.read_text(encoding="utf-8") == ( + "| answer | label |\n| --- | --- |\n" + ("" if empty else "| 42 | 分析 |\n") + ) + + +@pytest.mark.parametrize("extension", ["csv", "json", "md"]) +@pytest.mark.parametrize("path_kind", ["basename", "dot", "nested", "absolute"]) +@pytest.mark.parametrize("empty", [False, True]) +def test_query_exports_to_requested_path(tmp_path, extension, path_kind, empty): + analysis = runpy.run_path(str(SCRIPT)) + name = f"report.{extension}" + output = { + "basename": name, + "dot": f"./{name}", + "nested": f"new directory/reports/{name}", + "absolute": str(tmp_path / "absolute" / name), + }[path_kind] + sql = "SELECT 42 AS answer, '分析' AS label" + (" WHERE false" if empty else "") + with chdir(tmp_path), duckdb.connect() as con: + result = analysis["action_query"](con, sql, {}, output) + assert result == f"Results exported to {output} ({0 if empty else 1} rows)" + assert_export(Path(output), extension, empty) + + +@pytest.mark.parametrize("extension", ["csv", "json", "md"]) +def test_cli_exports_basename_from_real_read_only_cache(tmp_path, extension): + # Build the real CSV cache without requiring the optional spatial extension. + # The subprocess then exercises main's normal cache-hit and export branches. + temp_dir = tmp_path / "runtime" + cache_dir = temp_dir / ".data-analysis-cache" + cache_dir.mkdir(parents=True) + source = tmp_path / "source.csv" + source.write_text("answer,label\n42,分析\n", encoding="utf-8") + analysis = runpy.run_path(str(SCRIPT)) + key = analysis["compute_files_hash"]([str(source)]) + table_map = {} + with duckdb.connect(str(cache_dir / f"{key}.duckdb")) as con: + analysis["_load_csv"](con, str(source), table_map) + (cache_dir / f"{key}.table_map.json").write_text( + json.dumps(table_map), encoding="utf-8" + ) + output = f"report.{extension}" + completed = subprocess.run( + [ + sys.executable, + "-B", + str(SCRIPT), + "--files", + str(source), + "--action", + "query", + "--sql", + "SELECT * FROM source", + "--output-file", + output, + ], + cwd=tmp_path, + env={ + **os.environ, + "TMPDIR": str(temp_dir), + "TEMP": str(temp_dir), + "TMP": str(temp_dir), + }, + capture_output=True, + text=True, + encoding="utf-8", + check=False, + ) + assert completed.returncode == 0, completed.stderr + assert "Cache hit!" in completed.stderr + assert f"Results exported to {output} (1 rows)" in completed.stdout + assert_export(tmp_path / output, extension)