mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(skills): preserve source and SQL semantics across data analysis (#6739)
* fix(skills): preserve source and SQL semantics across data analysis * test(skills): use a portable filename for quoted SQL aliases * test(guidance): register data-analysis scoped instructions * test(guidance): keep skill registration independent of new examples * fix(skills): require explicit data analysis dependency setup --------- Co-authored-by: xyi74976-del <290725798+xyi74976-del@users.noreply.github.com>
This commit is contained in:
parent
a2fcfe0d9b
commit
a3c65c72e5
@ -1486,6 +1486,8 @@ File reads accept raw repository-relative paths and preserve spaces, Unicode,
|
||||
|
||||
Users can explicitly activate an enabled skill for a single turn by starting the request with `/skill-name`, for example `/data-analysis analyze uploads/foo.csv`. DeerFlow loads that skill's `SKILL.md` as hidden current-turn context while leaving the base prompt limited to skill metadata. Slash activation respects disabled skills, custom-agent skill whitelists, and existing channel commands such as `/new` and `/help`.
|
||||
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
An enabled skill's `allowed-tools` policy applies only after that skill is explicitly slash-activated or captured in the agent's active skill context after a `read_file` load. Merely enabling, advertising, or listing a skill in a custom agent or subagent `skills` allowlist does not reduce that agent's normal toolset; subagents use the same progressive discovery and activation policy as the lead agent. During a slash-activated run, that explicit skill's policy is authoritative: reading another `SKILL.md` may provide instructions but cannot widen the slash skill's tools. Without slash activation, policies from skills actually loaded into active context retain their union semantics. Once active, the policy filters both model-visible tool schemas and tool execution. Framework discovery tools (`tool_search` and `describe_skill`) remain available so an allowed deferred tool or installed skill can still be discovered, but discovery and promotion never grant permission to execute a business tool omitted from `allowed-tools`. `task` is not framework-exempt; a restrictive skill must list it explicitly to delegate to a subagent. Per-step policy decisions are internal runtime context and are removed from observable or persisted context copies. Registry failures and an active set with no remaining valid skill fail closed to framework-safe tools; individual stale paths are ignored only when another valid active skill remains. This is best-effort behavioral scoping, not a hard security boundary: loading skill instructions through another tool is not captured, and active-skill entries can be evicted from bounded context.
|
||||
|
||||
@ -9,6 +9,7 @@ REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
CHECKER_PATH = REPO_ROOT / "scripts" / "check_agent_guidance.py"
|
||||
EXPECTED_GUIDANCE_PATHS = {
|
||||
"AGENTS.md",
|
||||
"skills/public/data-analysis/AGENTS.md",
|
||||
"backend/AGENTS.md",
|
||||
"backend/tests/AGENTS.md",
|
||||
"frontend/AGENTS.md",
|
||||
|
||||
25
skills/public/data-analysis/AGENTS.md
Normal file
25
skills/public/data-analysis/AGENTS.md
Normal file
@ -0,0 +1,25 @@
|
||||
# Data-analysis source and SQL correctness
|
||||
|
||||
`analyze.py` allocates lookup aliases and SQL table names together. Neither may
|
||||
shadow an existing alias or SQL name (case-insensitively). Both CSV files and Excel
|
||||
sheets use this allocator; reserve a name only after CREATE TABLE succeeds.
|
||||
|
||||
Cache identity includes input order, filesystem-encoded basenames, per-file content
|
||||
digests, and a namespace version. Directory relocation alone does not change a
|
||||
readable input's identity. Bump the namespace if table-mapping semantics change.
|
||||
|
||||
Regressions live under the repository's root tests directory. They use
|
||||
real DuckDB. Excel and CLI integration additionally run with an already installed
|
||||
official spatial extension via `DEERFLOW_TEST_SPATIAL_DIRECTORY`; default tests
|
||||
never download that extension. Keep table lookup and persisted-cache checks tied
|
||||
to distinct source values, rather than only inspecting generated names.
|
||||
|
||||
Bind file paths and Excel sheet names as SQL values. Quote generated table and
|
||||
column identifiers by doubling embedded double quotes. Never rewrite user SQL
|
||||
with regular expressions: unique original names resolve through temporary views
|
||||
over canonical tables in the main schema, including on read-only cache hits.
|
||||
Keep these views connection-local and preserve normal CTE shadowing.
|
||||
|
||||
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.
|
||||
@ -9,6 +9,16 @@ description: Use this skill when the user uploads Excel (.xlsx/.xls) or CSV file
|
||||
|
||||
This skill analyzes user-uploaded Excel/CSV files using DuckDB — an in-process analytical SQL engine. It supports schema inspection, SQL-based querying, statistical summaries, and result export, all through a single Python script.
|
||||
|
||||
## Environment setup
|
||||
|
||||
Prepare the Python environment before running analyses. Install the required packages explicitly using the same interpreter that will run the script:
|
||||
|
||||
```bash
|
||||
python -m pip install duckdb openpyxl
|
||||
```
|
||||
|
||||
If either package is missing, the script exits with a setup instruction; it does not invoke an installer or modify the environment. DuckDB's existing spatial-extension setup for loading files is unchanged.
|
||||
|
||||
## Core Capabilities
|
||||
|
||||
- Inspect Excel/CSV file structure (sheets, columns, types, row counts)
|
||||
@ -103,7 +113,8 @@ Supported output formats (auto-detected from extension):
|
||||
- **Excel files**: Each sheet becomes a table named after the sheet (e.g., `Sheet1`, `Sales`, `Revenue`)
|
||||
- **CSV files**: Table name is the filename without extension (e.g., `data.csv` → `data`)
|
||||
- **Multiple files**: All tables from all files are available in the same query context, enabling cross-file joins
|
||||
- **Special characters**: Sheet/file names with spaces or special characters are auto-sanitized (spaces → underscores). Use double quotes for names that start with numbers or contain special characters, e.g., `"2024_Sales"`
|
||||
- **Collisions**: Repeated names (including case-only differences) receive unique SQL names in input order, such as `report`, `report_1`, `report_2`. This also applies to repeated Excel sheet names. Use the SQL names shown by `inspect` for queries and summaries. An original-name alias never overrides an earlier SQL name.
|
||||
- **Special characters**: Sheet/file names are sanitized into SQL names (for example, `sales-2026` → `sales_2026`). Use the SQL names printed by `inspect`, or double-quote a unique original name, e.g., `SELECT * FROM "sales-2026"`. Queries execute unchanged: string literals, column aliases, comments, and CTEs retain normal DuckDB semantics. Names containing a double quote escape it by doubling it, e.g., `"net""amount"`
|
||||
|
||||
## Analysis Patterns
|
||||
|
||||
@ -233,8 +244,8 @@ After analysis:
|
||||
|
||||
The script automatically caches loaded data to avoid re-parsing files on every call:
|
||||
|
||||
- On first load, files are parsed and stored in a persistent DuckDB database under `/mnt/user-data/workspace/.data-analysis-cache/`
|
||||
- The cache key is a SHA256 hash of all input file contents — if files change, a new cache is created
|
||||
- On first load, files are parsed and stored in a persistent DuckDB database under the system temporary directory (`.data-analysis-cache/`)
|
||||
- The cache key hashes the ordered input basenames and a separate SHA256 digest of each file. Renaming, reordering, or changing inputs creates a new cache; moving unchanged, identically named inputs to another directory preserves reuse. The updated key namespace ignores older cached table maps, which are rebuilt on first use
|
||||
- Subsequent calls with the same files will use the cached database directly (near-instant startup)
|
||||
- Cache is transparent — no extra parameters needed
|
||||
|
||||
|
||||
@ -11,7 +11,6 @@ import json
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
|
||||
@ -21,14 +20,16 @@ logger = logging.getLogger(__name__)
|
||||
try:
|
||||
import duckdb
|
||||
except ImportError:
|
||||
logger.error("duckdb is not installed. Installing...")
|
||||
subprocess.run([sys.executable, "-m", "pip", "install", "duckdb", "openpyxl", "-q"], check=True)
|
||||
import duckdb
|
||||
raise SystemExit(
|
||||
"Missing duckdb. Prepare this Python environment with: python -m pip install duckdb openpyxl"
|
||||
) from None
|
||||
|
||||
try:
|
||||
import openpyxl # noqa: F401
|
||||
except ImportError:
|
||||
subprocess.run([sys.executable, "-m", "pip", "install", "openpyxl", "-q"], check=True)
|
||||
raise SystemExit(
|
||||
"Missing openpyxl. Prepare this Python environment with: python -m pip install duckdb openpyxl"
|
||||
) from None
|
||||
|
||||
# Cache directory for persistent DuckDB databases
|
||||
CACHE_DIR = os.path.join(tempfile.gettempdir(), ".data-analysis-cache")
|
||||
@ -36,16 +37,26 @@ TABLE_MAP_SUFFIX = ".table_map.json"
|
||||
|
||||
|
||||
def compute_files_hash(files: list[str]) -> str:
|
||||
"""Compute a combined SHA256 hash of all input files for cache key."""
|
||||
hasher = hashlib.sha256()
|
||||
for file_path in sorted(files):
|
||||
"""Hash the ordered names and contents that determine the loaded table map."""
|
||||
# A new namespace also prevents reuse of maps built by the old allocator.
|
||||
hasher = hashlib.sha256(b"data-analysis-inputs-v2\0")
|
||||
for file_path in files:
|
||||
name = os.fsencode(os.path.basename(file_path))
|
||||
hasher.update(len(name).to_bytes(8, "big"))
|
||||
hasher.update(name)
|
||||
file_hasher = hashlib.sha256()
|
||||
try:
|
||||
with open(file_path, "rb") as f:
|
||||
while chunk := f.read(8192):
|
||||
hasher.update(chunk)
|
||||
file_hasher.update(chunk)
|
||||
except OSError:
|
||||
# Include path as fallback if file can't be read
|
||||
hasher.update(file_path.encode())
|
||||
# Keep unreadable paths distinct, including POSIX surrogate escapes.
|
||||
hasher.update(b"missing\0")
|
||||
file_hasher = hashlib.sha256(os.fsencode(file_path))
|
||||
else:
|
||||
hasher.update(b"readable\0")
|
||||
# Fixed-width digests preserve file boundaries, even for empty files.
|
||||
hasher.update(file_hasher.digest())
|
||||
return hasher.hexdigest()
|
||||
|
||||
|
||||
@ -87,11 +98,30 @@ def sanitize_table_name(name: str) -> str:
|
||||
return sanitized
|
||||
|
||||
|
||||
def _quote_identifier(name: str) -> str:
|
||||
"""Quote a SQL identifier, including embedded double quotes."""
|
||||
return '"' + name.replace('"', '""') + '"'
|
||||
|
||||
|
||||
def _allocate_table_names(name: str, table_map: dict[str, str]) -> tuple[str, str]:
|
||||
"""Keep lookup aliases and SQL names unique, including case-only collisions."""
|
||||
reserved = {n.casefold() for n in (*table_map.keys(), *table_map.values())}
|
||||
base = sanitize_table_name(name)
|
||||
table_name = base
|
||||
counter = 1
|
||||
while table_name.casefold() in reserved:
|
||||
table_name = f"{base}_{counter}"
|
||||
counter += 1
|
||||
# An original name must never redirect an earlier canonical SQL name.
|
||||
alias = table_name if name.casefold() in reserved else name
|
||||
return alias, table_name
|
||||
|
||||
|
||||
def load_files(con: duckdb.DuckDBPyConnection, files: list[str]) -> dict[str, str]:
|
||||
"""
|
||||
Load Excel/CSV files into DuckDB tables.
|
||||
|
||||
Returns a mapping of original_name -> sanitized_table_name.
|
||||
Returns unique lookup aliases mapped to their canonical SQL table names.
|
||||
"""
|
||||
con.execute("INSTALL spatial; LOAD spatial;")
|
||||
table_map: dict[str, str] = {}
|
||||
@ -124,30 +154,24 @@ def _load_excel(
|
||||
wb.close()
|
||||
|
||||
for sheet_name in sheet_names:
|
||||
table_name = sanitize_table_name(sheet_name)
|
||||
|
||||
# Handle duplicate table names
|
||||
original_table_name = table_name
|
||||
counter = 1
|
||||
while table_name in table_map.values():
|
||||
table_name = f"{original_table_name}_{counter}"
|
||||
counter += 1
|
||||
alias, table_name = _allocate_table_names(sheet_name, table_map)
|
||||
|
||||
try:
|
||||
con.execute(
|
||||
f"""
|
||||
CREATE TABLE "{table_name}" AS
|
||||
CREATE TABLE {_quote_identifier(table_name)} AS
|
||||
SELECT * FROM st_read(
|
||||
'{file_path}',
|
||||
layer = '{sheet_name}',
|
||||
?,
|
||||
layer = ?,
|
||||
open_options = ['HEADERS=FORCE', 'FIELD_TYPES=AUTO']
|
||||
)
|
||||
"""
|
||||
""",
|
||||
[file_path, sheet_name],
|
||||
)
|
||||
table_map[sheet_name] = table_name
|
||||
row_count = con.execute(f'SELECT COUNT(*) FROM "{table_name}"').fetchone()[
|
||||
0
|
||||
]
|
||||
table_map[alias] = table_name
|
||||
row_count = con.execute(
|
||||
f"SELECT COUNT(*) FROM {_quote_identifier(table_name)}"
|
||||
).fetchone()[0]
|
||||
logger.info(
|
||||
f" Loaded sheet '{sheet_name}' -> table '{table_name}' ({row_count} rows)"
|
||||
)
|
||||
@ -160,24 +184,20 @@ def _load_csv(
|
||||
) -> None:
|
||||
"""Load a CSV file into a DuckDB table."""
|
||||
base_name = os.path.splitext(os.path.basename(file_path))[0]
|
||||
table_name = sanitize_table_name(base_name)
|
||||
|
||||
# Handle duplicate table names
|
||||
original_table_name = table_name
|
||||
counter = 1
|
||||
while table_name in table_map.values():
|
||||
table_name = f"{original_table_name}_{counter}"
|
||||
counter += 1
|
||||
alias, table_name = _allocate_table_names(base_name, table_map)
|
||||
|
||||
try:
|
||||
con.execute(
|
||||
f"""
|
||||
CREATE TABLE "{table_name}" AS
|
||||
SELECT * FROM read_csv_auto('{file_path}')
|
||||
"""
|
||||
CREATE TABLE {_quote_identifier(table_name)} AS
|
||||
SELECT * FROM read_csv_auto(?)
|
||||
""",
|
||||
[file_path],
|
||||
)
|
||||
table_map[base_name] = table_name
|
||||
row_count = con.execute(f'SELECT COUNT(*) FROM "{table_name}"').fetchone()[0]
|
||||
table_map[alias] = table_name
|
||||
row_count = con.execute(
|
||||
f"SELECT COUNT(*) FROM {_quote_identifier(table_name)}"
|
||||
).fetchone()[0]
|
||||
logger.info(
|
||||
f" Loaded CSV '{base_name}' -> table '{table_name}' ({row_count} rows)"
|
||||
)
|
||||
@ -195,11 +215,13 @@ def action_inspect(con: duckdb.DuckDBPyConnection, table_map: dict[str, str]) ->
|
||||
output_parts.append(f"{'=' * 60}")
|
||||
|
||||
# Get row count
|
||||
row_count = con.execute(f'SELECT COUNT(*) FROM "{table_name}"').fetchone()[0]
|
||||
row_count = con.execute(
|
||||
f"SELECT COUNT(*) FROM {_quote_identifier(table_name)}"
|
||||
).fetchone()[0]
|
||||
output_parts.append(f"Rows: {row_count}")
|
||||
|
||||
# Get column info
|
||||
columns = con.execute(f'DESCRIBE "{table_name}"').fetchall()
|
||||
columns = con.execute(f"DESCRIBE {_quote_identifier(table_name)}").fetchall()
|
||||
output_parts.append(f"\nColumns ({len(columns)}):")
|
||||
output_parts.append(f"{'Name':<30} {'Type':<15} {'Nullable'}")
|
||||
output_parts.append(f"{'-' * 30} {'-' * 15} {'-' * 8}")
|
||||
@ -211,8 +233,10 @@ def action_inspect(con: duckdb.DuckDBPyConnection, table_map: dict[str, str]) ->
|
||||
col_names = [col[0] for col in columns]
|
||||
non_null_parts = []
|
||||
for c in col_names:
|
||||
non_null_parts.append(f'COUNT("{c}") as "{c}"')
|
||||
non_null_sql = f'SELECT {", ".join(non_null_parts)} FROM "{table_name}"'
|
||||
non_null_parts.append(f"COUNT({_quote_identifier(c)})")
|
||||
non_null_sql = (
|
||||
f"SELECT {', '.join(non_null_parts)} FROM {_quote_identifier(table_name)}"
|
||||
)
|
||||
try:
|
||||
non_null_counts = con.execute(non_null_sql).fetchone()
|
||||
output_parts.append("\nNon-null counts:")
|
||||
@ -224,10 +248,14 @@ def action_inspect(con: duckdb.DuckDBPyConnection, table_map: dict[str, str]) ->
|
||||
# Sample data (first 5 rows)
|
||||
output_parts.append("\nSample data (first 5 rows):")
|
||||
try:
|
||||
sample = con.execute(f'SELECT * FROM "{table_name}" LIMIT 5').fetchdf()
|
||||
sample = con.execute(
|
||||
f"SELECT * FROM {_quote_identifier(table_name)} LIMIT 5"
|
||||
).fetchdf()
|
||||
output_parts.append(sample.to_string(index=False))
|
||||
except Exception:
|
||||
sample = con.execute(f'SELECT * FROM "{table_name}" LIMIT 5').fetchall()
|
||||
sample = con.execute(
|
||||
f"SELECT * FROM {_quote_identifier(table_name)} LIMIT 5"
|
||||
).fetchall()
|
||||
header = [col[0] for col in columns]
|
||||
output_parts.append(" " + " | ".join(header))
|
||||
for row in sample:
|
||||
@ -245,27 +273,22 @@ def action_query(
|
||||
output_file: str | None = None,
|
||||
) -> str:
|
||||
"""Execute a SQL query and return/export results."""
|
||||
# Replace original sheet/file names with sanitized table names in SQL
|
||||
modified_sql = sql
|
||||
for original_name, table_name in sorted(
|
||||
table_map.items(), key=lambda x: len(x[0]), reverse=True
|
||||
):
|
||||
if original_name != table_name:
|
||||
# Replace occurrences not already quoted
|
||||
modified_sql = re.sub(
|
||||
rf"\b{re.escape(original_name)}\b",
|
||||
f'"{table_name}"',
|
||||
modified_sql,
|
||||
)
|
||||
|
||||
try:
|
||||
result = con.execute(modified_sql)
|
||||
# Let DuckDB resolve identifiers without rewriting SQL literals, column
|
||||
# names, comments, or CTEs. Temporary views also work on read-only caches.
|
||||
for original_name, table_name in table_map.items():
|
||||
if original_name != table_name:
|
||||
con.execute(
|
||||
f"CREATE OR REPLACE TEMP VIEW {_quote_identifier(original_name)} "
|
||||
f"AS SELECT * FROM main.{_quote_identifier(table_name)}"
|
||||
)
|
||||
result = con.execute(sql)
|
||||
columns = [desc[0] for desc in result.description]
|
||||
rows = result.fetchall()
|
||||
except Exception as e:
|
||||
error_msg = f"SQL Error: {e}\n\nAvailable tables:\n"
|
||||
for orig, tbl in table_map.items():
|
||||
cols = con.execute(f'DESCRIBE "{tbl}"').fetchall()
|
||||
cols = con.execute(f"DESCRIBE {_quote_identifier(tbl)}").fetchall()
|
||||
col_names = [c[0] for c in cols]
|
||||
error_msg += f' "{tbl}" ({orig}): {", ".join(col_names)}\n'
|
||||
print(error_msg)
|
||||
@ -370,17 +393,17 @@ def action_summary(
|
||||
) -> str:
|
||||
"""Generate statistical summary for a table."""
|
||||
# Resolve table name
|
||||
resolved = table_map.get(table_name, table_name)
|
||||
resolved = _quote_identifier(table_map.get(table_name, table_name))
|
||||
|
||||
try:
|
||||
columns = con.execute(f'DESCRIBE "{resolved}"').fetchall()
|
||||
columns = con.execute(f"DESCRIBE {resolved}").fetchall()
|
||||
except Exception:
|
||||
available = ", ".join(f'"{t}" ({o})' for o, t in table_map.items())
|
||||
msg = f"Table '{table_name}' not found. Available tables: {available}"
|
||||
print(msg)
|
||||
return msg
|
||||
|
||||
row_count = con.execute(f'SELECT COUNT(*) FROM "{resolved}"').fetchone()[0]
|
||||
row_count = con.execute(f"SELECT COUNT(*) FROM {resolved}").fetchone()[0]
|
||||
|
||||
output_parts = []
|
||||
output_parts.append(f"\nStatistical Summary: {table_name}")
|
||||
@ -402,6 +425,7 @@ def action_summary(
|
||||
|
||||
for col in columns:
|
||||
col_name, col_type = col[0], col[1].upper()
|
||||
quoted_col = _quote_identifier(col_name)
|
||||
output_parts.append(f"\n--- {col_name} ({col[1]}) ---")
|
||||
|
||||
# Check base type (strip parameterized parts)
|
||||
@ -411,16 +435,16 @@ def action_summary(
|
||||
try:
|
||||
stats = con.execute(f"""
|
||||
SELECT
|
||||
COUNT("{col_name}") as count,
|
||||
AVG("{col_name}")::DOUBLE as mean,
|
||||
STDDEV("{col_name}")::DOUBLE as std,
|
||||
MIN("{col_name}") as min,
|
||||
QUANTILE_CONT("{col_name}", 0.25) as q25,
|
||||
MEDIAN("{col_name}") as median,
|
||||
QUANTILE_CONT("{col_name}", 0.75) as q75,
|
||||
MAX("{col_name}") as max,
|
||||
COUNT(*) - COUNT("{col_name}") as null_count
|
||||
FROM "{resolved}"
|
||||
COUNT({quoted_col}) as count,
|
||||
AVG({quoted_col})::DOUBLE as mean,
|
||||
STDDEV({quoted_col})::DOUBLE as std,
|
||||
MIN({quoted_col}) as min,
|
||||
QUANTILE_CONT({quoted_col}, 0.25) as q25,
|
||||
MEDIAN({quoted_col}) as median,
|
||||
QUANTILE_CONT({quoted_col}, 0.75) as q75,
|
||||
MAX({quoted_col}) as max,
|
||||
COUNT(*) - COUNT({quoted_col}) as null_count
|
||||
FROM {resolved}
|
||||
""").fetchone()
|
||||
labels = [
|
||||
"count",
|
||||
@ -444,11 +468,11 @@ def action_summary(
|
||||
try:
|
||||
stats = con.execute(f"""
|
||||
SELECT
|
||||
COUNT("{col_name}") as count,
|
||||
COUNT(DISTINCT "{col_name}") as unique_count,
|
||||
MODE("{col_name}") as mode_val,
|
||||
COUNT(*) - COUNT("{col_name}") as null_count
|
||||
FROM "{resolved}"
|
||||
COUNT({quoted_col}) as count,
|
||||
COUNT(DISTINCT {quoted_col}) as unique_count,
|
||||
MODE({quoted_col}) as mode_val,
|
||||
COUNT(*) - COUNT({quoted_col}) as null_count
|
||||
FROM {resolved}
|
||||
""").fetchone()
|
||||
output_parts.append(f" count : {stats[0]}")
|
||||
output_parts.append(f" unique : {stats[1]}")
|
||||
@ -457,10 +481,10 @@ def action_summary(
|
||||
|
||||
# Show top 5 values
|
||||
top_vals = con.execute(f"""
|
||||
SELECT "{col_name}", COUNT(*) as freq
|
||||
FROM "{resolved}"
|
||||
WHERE "{col_name}" IS NOT NULL
|
||||
GROUP BY "{col_name}"
|
||||
SELECT {quoted_col}, COUNT(*) as freq
|
||||
FROM {resolved}
|
||||
WHERE {quoted_col} IS NOT NULL
|
||||
GROUP BY {quoted_col}
|
||||
ORDER BY freq DESC
|
||||
LIMIT 5
|
||||
""").fetchall()
|
||||
|
||||
35
tests/skills/test_data_analysis_dependency_setup.py
Normal file
35
tests/skills/test_data_analysis_dependency_setup.py
Normal file
@ -0,0 +1,35 @@
|
||||
"""Missing optional dependencies must not launch package installers."""
|
||||
|
||||
import builtins
|
||||
import runpy
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
SCRIPT = (
|
||||
Path(__file__).resolve().parents[2]
|
||||
/ "skills/public/data-analysis/scripts/analyze.py"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("dependency", ["duckdb", "openpyxl"])
|
||||
def test_missing_dependency_reports_explicit_setup(monkeypatch, dependency):
|
||||
if dependency == "openpyxl":
|
||||
pytest.importorskip("duckdb")
|
||||
original_import = builtins.__import__
|
||||
|
||||
def missing(name, *args, **kwargs):
|
||||
if name == dependency:
|
||||
raise ModuleNotFoundError(f"No module named '{dependency}'")
|
||||
return original_import(name, *args, **kwargs)
|
||||
|
||||
def forbidden_install(*args, **kwargs):
|
||||
pytest.fail("analysis must not launch dependency installers")
|
||||
|
||||
monkeypatch.setattr(builtins, "__import__", missing)
|
||||
monkeypatch.setattr(subprocess, "run", forbidden_install)
|
||||
with pytest.raises(
|
||||
SystemExit, match=f"Missing {dependency}.*pip install duckdb openpyxl"
|
||||
):
|
||||
runpy.run_path(str(SCRIPT))
|
||||
307
tests/skills/test_data_analysis_input_identity.py
Normal file
307
tests/skills/test_data_analysis_input_identity.py
Normal file
@ -0,0 +1,307 @@
|
||||
"""Real DuckDB regressions for multi-file loading, lookup, and cache identity."""
|
||||
|
||||
import os
|
||||
import runpy
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
# Real-engine regressions require explicitly prepared optional dependencies.
|
||||
duckdb = pytest.importorskip("duckdb")
|
||||
pytest.importorskip("openpyxl")
|
||||
SCRIPT = (
|
||||
Path(__file__).resolve().parents[2]
|
||||
/ "skills/public/data-analysis/scripts/analyze.py"
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def analysis():
|
||||
return runpy.run_path(str(SCRIPT))
|
||||
|
||||
|
||||
def csv_file(root, name, value):
|
||||
root.mkdir(parents=True, exist_ok=True)
|
||||
path = root / name
|
||||
path.write_text(f"value\n{value}\n", encoding="utf-8")
|
||||
return str(path)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"names, expected",
|
||||
[
|
||||
(
|
||||
["report.csv", "report.csv", "report_1.csv"],
|
||||
["report", "report_1", "report_1_1"],
|
||||
),
|
||||
(["Report.csv", "report.csv"], ["Report", "report_1"]),
|
||||
(["sales-2026.csv", "sales_2026.csv"], ["sales_2026", "sales_2026_1"]),
|
||||
(["a-b.csv", "a b.csv", "a_b.csv"], ["a_b", "a_b_1", "a_b_2"]),
|
||||
],
|
||||
)
|
||||
def test_each_input_keeps_a_distinct_queryable_table(
|
||||
analysis, tmp_path, names, expected
|
||||
):
|
||||
table_map = {}
|
||||
with duckdb.connect() as con:
|
||||
for i, name in enumerate(names, 1):
|
||||
analysis["_load_csv"](con, csv_file(tmp_path / str(i), name, i), table_map)
|
||||
assert list(table_map.values()) == expected
|
||||
assert len(table_map) == len(names)
|
||||
# Map-driven SQL queries and summaries must select the same data as the
|
||||
# canonical SQL name printed by inspect; no alias may redirect that name.
|
||||
for i, table in enumerate(expected, 1):
|
||||
output = analysis["action_query"](
|
||||
con, f'SELECT value FROM "{table}"', table_map
|
||||
)
|
||||
assert output.splitlines()[2].strip() == str(i)
|
||||
summary = analysis["action_summary"](con, table, table_map)
|
||||
assert f" mean : {float(i)}" in summary
|
||||
|
||||
|
||||
def test_cache_reopen_keeps_all_duplicate_inputs(analysis, tmp_path):
|
||||
files = [csv_file(tmp_path / str(i), "report.csv", i) for i in (1, 2)]
|
||||
db = tmp_path / "loaded.duckdb"
|
||||
table_map = {}
|
||||
with duckdb.connect(str(db)) as con:
|
||||
for file in files:
|
||||
analysis["_load_csv"](con, file, table_map)
|
||||
# Exercise the actual JSON persistence used on a cache hit.
|
||||
analysis["save_table_map"].__globals__["CACHE_DIR"] = str(tmp_path)
|
||||
key = analysis["compute_files_hash"](files)
|
||||
analysis["save_table_map"](key, table_map)
|
||||
restored = analysis["load_table_map"](key)
|
||||
assert restored == {"report": "report", "report_1": "report_1"}
|
||||
with duckdb.connect(str(db), read_only=True) as con:
|
||||
output = analysis["action_query"](
|
||||
con, "SELECT a.value, b.value FROM report a CROSS JOIN report_1 b", restored
|
||||
)
|
||||
assert output.splitlines()[2].replace(" ", "") == "1|2"
|
||||
|
||||
|
||||
def test_cache_identity_changes_with_duplicate_input_order(analysis, tmp_path):
|
||||
files = [csv_file(tmp_path / str(i), "report.csv", i) for i in (1, 2)]
|
||||
assert analysis["compute_files_hash"](files) != analysis["compute_files_hash"](
|
||||
files[::-1]
|
||||
)
|
||||
|
||||
|
||||
def test_cache_identity_changes_when_file_is_renamed(analysis, tmp_path):
|
||||
original = csv_file(tmp_path, "before.csv", 1)
|
||||
renamed = csv_file(tmp_path, "after.csv", 1)
|
||||
assert analysis["compute_files_hash"]([original]) != analysis["compute_files_hash"](
|
||||
[renamed]
|
||||
)
|
||||
|
||||
|
||||
def test_cache_identity_preserves_file_boundaries(analysis, tmp_path):
|
||||
# Both streams concatenate to b"abc", but their per-file data differs.
|
||||
files = [tmp_path / "a.csv", tmp_path / "b.csv"]
|
||||
files[0].write_bytes(b"a")
|
||||
files[1].write_bytes(b"bc")
|
||||
before = analysis["compute_files_hash"]([str(p) for p in files])
|
||||
files[0].write_bytes(b"ab")
|
||||
files[1].write_bytes(b"c")
|
||||
assert analysis["compute_files_hash"]([str(p) for p in files]) != before
|
||||
|
||||
|
||||
def test_relocating_the_same_named_inputs_reuses_cache(analysis, tmp_path):
|
||||
first = csv_file(tmp_path / "first", "report.csv", 1)
|
||||
relocated = csv_file(tmp_path / "other", "report.csv", 1)
|
||||
assert analysis["compute_files_hash"]([first]) == analysis["compute_files_hash"](
|
||||
[relocated]
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name == "nt", reason="POSIX surrogateescaped filenames")
|
||||
def test_cache_hash_accepts_non_utf8_filesystem_names(analysis):
|
||||
# Even an unreadable POSIX argv path must reach load_files' warning, rather
|
||||
# than crashing in the hash function's fallback encoding.
|
||||
path = os.fsdecode(b"/missing/report-\xff.csv")
|
||||
assert len(analysis["compute_files_hash"]([path])) == 64
|
||||
|
||||
|
||||
def test_excel_duplicate_sheet_names_keep_their_data(analysis, tmp_path):
|
||||
# Optional real-spatial integration: no download from the default test run.
|
||||
# Supply an already installed official extension directory to opt in.
|
||||
extension_dir = os.environ.get("DEERFLOW_TEST_SPATIAL_DIRECTORY")
|
||||
if not extension_dir:
|
||||
pytest.skip(
|
||||
"set DEERFLOW_TEST_SPATIAL_DIRECTORY for real Excel/spatial integration"
|
||||
)
|
||||
import openpyxl
|
||||
|
||||
table_map = {}
|
||||
with duckdb.connect(config={"extension_directory": extension_dir}) as con:
|
||||
con.execute("LOAD spatial")
|
||||
for i in (1, 2):
|
||||
workbook = openpyxl.Workbook()
|
||||
sheet = workbook.active
|
||||
sheet.title = "Sheet1"
|
||||
sheet.append(["value"])
|
||||
sheet.append([i])
|
||||
path = tmp_path / f"book{i}.xlsx"
|
||||
workbook.save(path)
|
||||
workbook.close()
|
||||
analysis["_load_excel"](con, str(path), table_map)
|
||||
assert table_map == {"Sheet1": "Sheet1", "Sheet1_1": "Sheet1_1"}
|
||||
output = analysis["action_query"](
|
||||
con,
|
||||
"SELECT a.value, b.value FROM Sheet1 a CROSS JOIN Sheet1_1 b",
|
||||
table_map,
|
||||
)
|
||||
assert output.splitlines()[2].replace(" ", "") == "1|2"
|
||||
|
||||
|
||||
def test_cli_cache_reopen_and_reorder_select_the_requested_sources(
|
||||
analysis, tmp_path, monkeypatch, capsys, caplog
|
||||
):
|
||||
extension_dir = os.environ.get("DEERFLOW_TEST_SPATIAL_DIRECTORY")
|
||||
if not extension_dir:
|
||||
pytest.skip(
|
||||
"set DEERFLOW_TEST_SPATIAL_DIRECTORY for real CLI/spatial integration"
|
||||
)
|
||||
import logging
|
||||
import sys
|
||||
|
||||
connect = duckdb.connect
|
||||
# Redirect only the official extension location. All loading, INSTALL/LOAD,
|
||||
# SQL, file hashing, database reopening, and CLI actions remain production code.
|
||||
monkeypatch.setattr(
|
||||
duckdb,
|
||||
"connect",
|
||||
lambda *a, **kw: connect(
|
||||
*a, **kw, config={"extension_directory": extension_dir}
|
||||
),
|
||||
)
|
||||
analysis["main"].__globals__["CACHE_DIR"] = str(tmp_path / "cache")
|
||||
files = [
|
||||
csv_file(tmp_path / "owner's uploads" / str(i), "sales-2026.csv", i)
|
||||
for i in (1, 2)
|
||||
]
|
||||
sql = 'SELECT a.value, b.value FROM "sales-2026" a CROSS JOIN sales_2026_1 b'
|
||||
|
||||
def query(paths):
|
||||
monkeypatch.setattr(
|
||||
sys,
|
||||
"argv",
|
||||
[str(SCRIPT), "--files", *paths, "--action", "query", "--sql", sql],
|
||||
)
|
||||
caplog.clear()
|
||||
with caplog.at_level(logging.INFO):
|
||||
analysis["main"]()
|
||||
return capsys.readouterr().out.splitlines()[2].replace(" ", "")
|
||||
|
||||
assert query(files) == "1|2"
|
||||
assert query(files[::-1]) == "2|1"
|
||||
assert "Cache hit!" not in caplog.text
|
||||
assert query(files[::-1]) == "2|1"
|
||||
assert "Cache hit!" in caplog.text
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"sql, expected",
|
||||
[
|
||||
("SELECT 'sales-2026' AS label FROM sales_2026", {"label": "sales-2026"}),
|
||||
('SELECT value FROM "sales-2026"', {"value": 7}),
|
||||
('SELECT value AS "sales-2026" FROM sales_2026', {"sales-2026": 7}),
|
||||
(
|
||||
'WITH "sales-2026" AS (SELECT 9 AS value) SELECT * FROM "sales-2026"',
|
||||
{"value": 9},
|
||||
),
|
||||
(
|
||||
'SELECT value FROM "sales-2026" -- sales-2026 is a source alias',
|
||||
{"value": 7},
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_queries_preserve_sql_semantics(analysis, tmp_path, sql, expected):
|
||||
import json
|
||||
|
||||
table_map = {}
|
||||
with duckdb.connect() as con:
|
||||
analysis["_load_csv"](con, csv_file(tmp_path, "sales-2026.csv", 7), table_map)
|
||||
output = tmp_path / "result.json"
|
||||
analysis["action_query"](con, sql, table_map, str(output))
|
||||
assert output.exists(), "query must succeed without rewriting identifiers"
|
||||
assert json.loads(output.read_text()) == [expected]
|
||||
|
||||
|
||||
def test_original_aliases_work_on_read_only_cache(analysis, tmp_path):
|
||||
import json
|
||||
|
||||
db = tmp_path / "cached.duckdb"
|
||||
table_map = {}
|
||||
with duckdb.connect(str(db)) as con:
|
||||
analysis["_load_csv"](con, csv_file(tmp_path, "sales-2026.csv", 7), table_map)
|
||||
# Double quotes are valid SQL alias characters, but Windows forbids them
|
||||
# in disk filenames. Exercise alias escaping independently of the CSV path.
|
||||
table_map['sales-"2026'] = table_map.pop("sales-2026")
|
||||
before = db.read_bytes()
|
||||
with duckdb.connect(str(db), read_only=True) as con:
|
||||
for i in range(2):
|
||||
output = tmp_path / f"result{i}.json"
|
||||
analysis["action_query"](
|
||||
con, 'SELECT value FROM "sales-""2026"', table_map, str(output)
|
||||
)
|
||||
assert output.exists(), "original names must work on cache hits too"
|
||||
assert json.loads(output.read_text()) == [{"value": 7}]
|
||||
assert db.read_bytes() == before
|
||||
with duckdb.connect(str(db), read_only=True) as con:
|
||||
assert (
|
||||
con.execute(
|
||||
"SELECT view_name FROM duckdb_views() WHERE NOT internal"
|
||||
).fetchall()
|
||||
== []
|
||||
)
|
||||
|
||||
|
||||
def test_csv_paths_are_bound_as_values(analysis, tmp_path):
|
||||
table_map = {}
|
||||
with duckdb.connect() as con:
|
||||
path = csv_file(tmp_path / "owner's uploads", "sales'2026.csv", 7)
|
||||
analysis["_load_csv"](con, path, table_map)
|
||||
assert table_map == {"sales'2026": "sales_2026"}
|
||||
assert con.execute("SELECT value FROM sales_2026").fetchall() == [(7,)]
|
||||
|
||||
|
||||
def test_inspect_and_summary_handle_quoted_headers(analysis, tmp_path):
|
||||
import csv
|
||||
|
||||
path = tmp_path / "report.csv"
|
||||
with path.open("w", newline="", encoding="utf-8") as handle:
|
||||
writer = csv.writer(handle)
|
||||
writer.writerow(['net"amount', 'customer"name'])
|
||||
writer.writerows([(10, "Alice"), (20, "Alice"), (None, "Bob")])
|
||||
table_map = {}
|
||||
with duckdb.connect() as con:
|
||||
analysis["_load_csv"](con, str(path), table_map)
|
||||
inspected = analysis["action_inspect"](con, table_map)
|
||||
assert ' net"amount: 2 / 3' in inspected
|
||||
assert ' customer"name: 3 / 3' in inspected
|
||||
summary = analysis["action_summary"](con, "report", table_map)
|
||||
assert "Error computing stats" not in summary
|
||||
assert " mean : 15.0000" in summary
|
||||
assert " unique : 2" in summary
|
||||
assert " Alice: 2 (66.7%)" in summary
|
||||
|
||||
|
||||
def test_excel_paths_and_sheet_names_are_bound_as_values(analysis, tmp_path):
|
||||
extension_dir = os.environ.get("DEERFLOW_TEST_SPATIAL_DIRECTORY")
|
||||
if not extension_dir:
|
||||
pytest.skip("set DEERFLOW_TEST_SPATIAL_DIRECTORY for real Excel integration")
|
||||
import openpyxl
|
||||
|
||||
path = tmp_path / "owner's report.xlsx"
|
||||
workbook = openpyxl.Workbook()
|
||||
workbook.active.title = "owner's sales"
|
||||
workbook.active.append(["value"])
|
||||
workbook.active.append([7])
|
||||
workbook.save(path)
|
||||
workbook.close()
|
||||
table_map = {}
|
||||
with duckdb.connect(config={"extension_directory": extension_dir}) as con:
|
||||
con.execute("LOAD spatial")
|
||||
analysis["_load_excel"](con, str(path), table_map)
|
||||
assert table_map == {"owner's sales": "owner_s_sales"}
|
||||
assert con.execute("SELECT value FROM owner_s_sales").fetchall() == [(7,)]
|
||||
Loading…
x
Reference in New Issue
Block a user