From 153e02d6605c36213d6e5524b076a553ce4a0aff Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Sun, 14 Jun 2026 08:40:33 -0700 Subject: [PATCH] Add skills eval setup with promptfoo --- .../datajunction/skills/datajunction-repo.md | 10 +- .../python/skill_evals/.env.example | 16 ++ .../python/skill_evals/.gitignore | 4 + .../python/skill_evals/README.md | 69 ++++++ .../python/skill_evals/assert_deployment.py | 86 +++++++ .../python/skill_evals/assert_node.py | 49 ++++ .../python/skill_evals/node_rules.py | 142 +++++++++++ .../python/skill_evals/promptfooconfig.yaml | 231 ++++++++++++++++++ .../python/skill_evals/provider.py | 99 ++++++++ .../python/skill_evals/run.sh | 24 ++ .../python/skill_evals/skill_prompt.py | 25 ++ .../python/tests/test_skill_examples.py | 90 +++++++ 12 files changed, 840 insertions(+), 5 deletions(-) create mode 100644 datajunction-clients/python/skill_evals/.env.example create mode 100644 datajunction-clients/python/skill_evals/.gitignore create mode 100644 datajunction-clients/python/skill_evals/README.md create mode 100644 datajunction-clients/python/skill_evals/assert_deployment.py create mode 100644 datajunction-clients/python/skill_evals/assert_node.py create mode 100644 datajunction-clients/python/skill_evals/node_rules.py create mode 100644 datajunction-clients/python/skill_evals/promptfooconfig.yaml create mode 100644 datajunction-clients/python/skill_evals/provider.py create mode 100755 datajunction-clients/python/skill_evals/run.sh create mode 100644 datajunction-clients/python/skill_evals/skill_prompt.py create mode 100644 datajunction-clients/python/tests/test_skill_examples.py diff --git a/datajunction-clients/python/datajunction/skills/datajunction-repo.md b/datajunction-clients/python/datajunction/skills/datajunction-repo.md index f987ac2d0..2bb0893ff 100644 --- a/datajunction-clients/python/datajunction/skills/datajunction-repo.md +++ b/datajunction-clients/python/datajunction/skills/datajunction-repo.md @@ -183,7 +183,7 @@ query: | # nodes/sources/transactions.yaml name: finance.transactions description: Raw transaction data from payment system -type: source +node_type: source catalog: prod_catalog schema_: finance table: transactions_table @@ -227,7 +227,7 @@ mode: published # nodes/dimensions/user.yaml name: finance.user description: User dimension with attributes -type: dimension +node_type: dimension query: | SELECT user_id, @@ -281,7 +281,7 @@ mode: published # nodes/metrics/revenue.yaml name: finance.total_revenue description: Total revenue from completed transactions -type: metric +node_type: metric query: | SELECT SUM( @@ -319,7 +319,7 @@ mode: published # nodes/transforms/clean_transactions.yaml name: finance.clean_transactions description: Cleaned transaction data with standardized status -type: transform +node_type: transform primary_key: - transaction_id query: | @@ -598,7 +598,7 @@ git checkout feature-add-churn-metric cat > nodes/metrics/churn_rate.yaml <<'EOF' name: finance.churn_rate description: Monthly user churn rate -type: metric +node_type: metric query: | SELECT CAST(SUM(CASE WHEN churned = true THEN 1 ELSE 0 END) AS DOUBLE) / diff --git a/datajunction-clients/python/skill_evals/.env.example b/datajunction-clients/python/skill_evals/.env.example new file mode 100644 index 000000000..d5fa1cad8 --- /dev/null +++ b/datajunction-clients/python/skill_evals/.env.example @@ -0,0 +1,16 @@ +# Copy to .env and fill in (real .env is gitignored). The harness reads everything +# from env — nothing about the provider is hardcoded. +# +# Bring your own key (default — uses the public OpenAI API): +OPENAI_API_KEY=sk-... +SKILL_EVAL_MODEL=gpt-4o +# +# OpenAI-compatible gateway/proxy instead of api.openai.com: point the base URL at it +# and pick a model it serves. (If your org runs a billing proxy, its specifics go in a +# local note, not here.) +# OPENAI_BASE_URL=http://your-openai-compatible-endpoint/v1 +# OPENAI_API_KEY=... # whatever the endpoint expects +# SKILL_EVAL_MODEL=claude-sonnet-4-6 # any model the endpoint serves +# +# The python assertions need pyyaml. Point promptfoo at a Python that has it: +# PROMPTFOO_PYTHON=/path/to/a/venv/bin/python # a venv with `pip install pyyaml` diff --git a/datajunction-clients/python/skill_evals/.gitignore b/datajunction-clients/python/skill_evals/.gitignore new file mode 100644 index 000000000..eb05a27d8 --- /dev/null +++ b/datajunction-clients/python/skill_evals/.gitignore @@ -0,0 +1,4 @@ +# generated by run.sh (local model override) +promptfooconfig.local.yaml +__pycache__/ +.env diff --git a/datajunction-clients/python/skill_evals/README.md b/datajunction-clients/python/skill_evals/README.md new file mode 100644 index 000000000..81ed8271b --- /dev/null +++ b/datajunction-clients/python/skill_evals/README.md @@ -0,0 +1,69 @@ +# Skill evals (behavioral) + +Behavioral evals for the bundled DJ Claude skills: load a `SKILL.md` into a model's +context, give it a realistic request, and grade what it produces. + +**This is a local / on-demand tool, not CI.** It makes real LLM calls, so it isn't +wired into GitHub Actions (public runners have no provider key). The free, every-PR +safety net is the *programmatic* tier — +`tests/test_skill_examples.py`, which validates the example YAML in the skills against +the deployment schema with no model. This dir is the deliberate, occasional quality +check on top of that. + +## What it checks + +Cases fall in two groups. **Authoring** cases (single turn) ask for a node and grade the +YAML. **Decomposition** cases (two turns) test the semantic-model skill's "propose, don't +produce" workflow: turn 1 should return a *structured decomposition proposal* (no YAML), +turn 2 produces the deployment YAML after a follow-up. `provider.py` drives both turns in +one test row so there's no fragile cross-row ordering. + +The semantic-modeling traps under test: ratio → named base metrics + a derived metric +(not one inlined blob); filters via `CASE WHEN`, not a `WHERE`; slice-by-dimension via a +dimension link, not a baked-in `JOIN`; the reusability rule holding even when the user +asks for "just one metric"; mixed grains split into separate transforms; business- +meaningful naming. + +Each case asserts with some mix of: + +- **programmatic, no LLM** — `assert_node.py` (single node) / `assert_deployment.py` + (multi-node): nodes parse, use `node_type` (not legacy `type:`), have required fields, + and satisfy case-specific shape (min nodes per type, a derived ratio metric exists, a + dimension link exists, no JOIN baked into a query, a query matches/avoids a pattern). + Rules are shared in `node_rules.py` so the two asserts can't drift; +- **llm-rubric** (LLM judge) — the modeling judgment: did it decompose correctly, resist + the shortcut, name things meaningfully. + +## Running it + +The provider endpoint comes from env — nothing about it is hardcoded. Copy +`.env.example` to `.env` (gitignored) and fill it in, then run `./run.sh`. + +**Bring your own key (default — public OpenAI API):** + +```bash +export OPENAI_API_KEY="sk-..." # your OpenAI key +./run.sh # defaults to gpt-4o +npx promptfoo@latest view # browse results +``` + +**Any OpenAI-compatible endpoint (e.g. a gateway/proxy):** point the base URL at it and +pick a model it serves. `run.sh` generates a gitignored `promptfooconfig.local.yaml` +with `SKILL_EVAL_MODEL` (the committed config stays `gpt-4o`, so BYO-key runs and +gateway runs don't fight over one line): + +```bash +export OPENAI_BASE_URL="http://your-endpoint/v1" +export OPENAI_API_KEY="..." # whatever the endpoint expects +SKILL_EVAL_MODEL=claude-sonnet-4-6 ./run.sh # any model the endpoint serves +``` + +## Files + +- `promptfooconfig.yaml` — providers, prompts, golden cases + assertions. +- `skill_prompt.py` — prompt function: injects the SKILL.md(s) as system + the request. + `vars.skill` is a comma-separated string (multiple skills compose). +- `provider.py` — two-turn OpenAI-compatible client; a `followup` var triggers turn 2. +- `node_rules.py` — shared node-spec rules mirroring the server deployment schema. +- `assert_node.py` — structural check on a single produced node. +- `assert_deployment.py` — structural check on a multi-node decomposition / deployment. diff --git a/datajunction-clients/python/skill_evals/assert_deployment.py b/datajunction-clients/python/skill_evals/assert_deployment.py new file mode 100644 index 000000000..dbf337e73 --- /dev/null +++ b/datajunction-clients/python/skill_evals/assert_deployment.py @@ -0,0 +1,86 @@ +"""promptfoo python assertion for multi-node decomposition outputs (the turn-2 YAML of +a two-turn case, or any answer that emits several nodes / a deployment doc). + +It validates every node structurally (via ``node_rules``) and then checks the +case-specific modeling shape from test vars: + - ``min_nodes_by_type`` — mapping like ``{metric: 3, transform: 1}``: at least + this many nodes of each type must be present; + - ``require_derived_ratio`` — at least one metric must be a derived ratio (an + expression composing other metrics, no raw aggregate); + - ``require_dimension_link``— at least one node must declare ``dimension_links``; + - ``forbid_join`` — no metric/transform query may contain a ``JOIN`` (joins + belong in dimension links, not baked into queries). + +If the output contains the turn-2 marker, only the part after it (the YAML turn) is +validated — the proposal turn is graded by the llm-rubric, not here. +""" + +import re + +import node_rules + +_JOIN = re.compile(r"\bJOIN\b", re.IGNORECASE) + + +def get_assert(output, context): + variables = context.get("vars", {}) + + yaml_part = output + if node_rules.TURN2_MARKER in output: + yaml_part = output.split(node_rules.TURN2_MARKER, 1)[1] + + nodes = [ + n + for n in node_rules.parse_nodes(yaml_part) + if not node_rules.is_reference_stub(n) + ] + if not nodes: + return { + "pass": False, + "score": 0, + "reason": "no authored nodes found in deployment output", + } + + problems: list[str] = [] + for node in nodes: + problems.extend(node_rules.validate_node(node)) + + counts: dict[str, int] = {} + for node in nodes: + counts[node.get("node_type")] = counts.get(node.get("node_type"), 0) + 1 + + for node_type, minimum in (variables.get("min_nodes_by_type") or {}).items(): + if counts.get(node_type, 0) < minimum: + problems.append( + f"expected >= {minimum} {node_type} node(s), found {counts.get(node_type, 0)}", + ) + + if variables.get("require_derived_ratio") and not any( + node_rules.is_derived_metric(n) for n in nodes + ): + problems.append( + "no derived ratio metric (expected a metric composing other metrics by name, " + "not a single query with the whole ratio inlined)", + ) + + if variables.get("require_dimension_link") and not any( + n.get("dimension_links") for n in nodes + ): + problems.append( + "no node declares `dimension_links` (the join should be a dim link)", + ) + + if variables.get("forbid_join"): + for node in nodes: + if node.get("node_type") in ("metric", "transform") and _JOIN.search( + str(node.get("query") or ""), + ): + problems.append( + f"{node_rules.node_label(node)}: query contains a JOIN — joins belong " + f"in dimension links, not baked into the query", + ) + + summary = ", ".join(f"{v}×{k}" for k, v in sorted(counts.items())) or "0 nodes" + if problems: + return {"pass": False, "score": 0, "reason": "; ".join(problems)} + return {"pass": True, "score": 1, "reason": f"valid deployment ({summary})"} diff --git a/datajunction-clients/python/skill_evals/assert_node.py b/datajunction-clients/python/skill_evals/assert_node.py new file mode 100644 index 000000000..2d31b98cc --- /dev/null +++ b/datajunction-clients/python/skill_evals/assert_node.py @@ -0,0 +1,49 @@ +"""promptfoo python assertion: the model produced a single node conforming to the +server deployment schema (``datajunction_server.models.deployment``). For multi-node +decomposition outputs use ``assert_deployment.py``. + +Rules live in ``node_rules`` (shared with the deployment assert). Per-case expectations +come from test vars: + - ``expected_node_type`` — the node_type the case asked for; + - ``require_in_query`` — regex the node's ``query`` MUST match (e.g. ``CASE\\s+WHEN``); + - ``forbid_in_query`` — regex the node's ``query`` must NOT match (e.g. a ``WHERE``). +""" + +import re + +import node_rules + + +def get_assert(output, context): + variables = context.get("vars", {}) + expected = variables.get("expected_node_type") + + nodes = node_rules.parse_nodes(output) + if not nodes: + return {"pass": False, "score": 0, "reason": "no YAML node found in output"} + if len(nodes) > 1: + return { + "pass": False, + "score": 0, + "reason": f"expected a single node, found {len(nodes)} (use a deployment case)", + } + + data = nodes[0] + problems = node_rules.validate_node(data, expected) + + query = str(data.get("query") or "") + require = variables.get("require_in_query") + if require and not re.search(require, query): + problems.append(f"query does not match required pattern {require!r}") + forbid = variables.get("forbid_in_query") + if forbid and re.search(forbid, query): + problems.append(f"query matches forbidden pattern {forbid!r}") + + if problems: + return {"pass": False, "score": 0, "reason": "; ".join(problems)} + node_type = data.get("node_type") + return { + "pass": True, + "score": 1, + "reason": f"valid {node_type} node per deployment schema", + } diff --git a/datajunction-clients/python/skill_evals/node_rules.py b/datajunction-clients/python/skill_evals/node_rules.py new file mode 100644 index 000000000..a09cc3ecd --- /dev/null +++ b/datajunction-clients/python/skill_evals/node_rules.py @@ -0,0 +1,142 @@ +"""Shared structural rules for DataJunction node specs, mirroring the server +deployment schema (``datajunction_server.models.deployment``). + +We validate structurally rather than importing the server models: importing +``datajunction_server.models.deployment`` standalone currently triggers a circular +import outside the full pytest harness, and promptfoo runs the asserts in a bare +Python. The rules below mirror that schema and match real deployed nodes in ads-dj: + + - the node-type discriminator is ``node_type`` (NOT a top-level ``type:``); + - SourceSpec needs ``catalog`` + ``table``; transform/dimension/metric need ``query``; + - a dimension declares a primary key (top-level ``primary_key`` OR a column carrying + ``primary_key`` in its ``attributes`` — the schema treats these as equivalent). + +Both ``assert_node`` (single node) and ``assert_deployment`` (multi-node) build on +these helpers so a rule change lands in one place. +""" + +import re + +import yaml + +# Separates turn 1 (the decomposition proposal) from turn 2 (the deployment YAML) in +# a two-turn provider response. Shared by provider.py and assert_deployment.py. +TURN2_MARKER = "<<>>" + +NODE_TYPES = {"source", "transform", "dimension", "metric", "cube"} +REQUIRED_FIELDS: dict[str, list[str]] = { + "source": ["catalog", "table"], + "transform": ["query"], + "dimension": ["query"], + "metric": ["query"], + "cube": [], +} + +# Aggregate functions a *base* metric uses. A derived/ratio metric composes other +# metrics and should contain none of these — it references metric names instead. +_AGG = re.compile( + r"\b(SUM|COUNT|AVG|MIN|MAX|APPROX_COUNT_DISTINCT|VAR_POP|STDDEV_POP|" + r"PERCENTILE_APPROX)\s*\(", + re.IGNORECASE, +) + + +def extract_yaml_blocks(output: str) -> list[str]: + """All fenced ```yaml blocks, in order. Falls back to the whole output if it has + no fences but looks like YAML (a bare node).""" + blocks = re.findall(r"```ya?ml\n(.*?)\n```", output, re.DOTALL) + if blocks: + return blocks + return [output] if ":" in output else [] + + +def parse_nodes(output: str) -> list[dict]: + """Flatten every node spec in the output. Handles both a deployment doc + (``nodes: [...]``) and one-or-more standalone node blocks.""" + nodes: list[dict] = [] + for block in extract_yaml_blocks(output): + try: + doc = yaml.safe_load(block) + except yaml.YAMLError: + continue + if isinstance(doc, dict) and isinstance(doc.get("nodes"), list): + nodes.extend(n for n in doc["nodes"] if isinstance(n, dict)) + elif isinstance(doc, dict): + nodes.append(doc) + return nodes + + +_CONTENT_FIELDS = ("query", "catalog", "table", "columns", "dimension_links") + + +def is_reference_stub(data: dict) -> bool: + """A bare pointer to an existing node — name + node_type and no content fields. The + model may list these in a deployment to reference parents it was told already exist; + they aren't nodes we're authoring, so the deployment assert ignores them (and does + NOT count them toward min-nodes, so a genuinely incomplete node still fails).""" + if not data.get("name") or not data.get("node_type"): + return False + return not any(data.get(field) for field in _CONTENT_FIELDS) + + +def has_primary_key(data: dict) -> bool: + if data.get("primary_key"): + return True + return any( + isinstance(col, dict) and "primary_key" in (col.get("attributes") or []) + for col in (data.get("columns") or []) + ) + + +def node_label(data: dict) -> str: + return data.get("name") or data.get("node_type") or "" + + +def validate_node(data: dict, expected: str | None = None) -> list[str]: + """Structural problems with a single node spec (empty list == valid).""" + problems: list[str] = [] + label = node_label(data) + + node_type = data.get("node_type") + if node_type is None: + if data.get("type") in NODE_TYPES: + problems.append( + f"{label}: uses legacy top-level `type: {data['type']}` — the deployment " + f"schema discriminator is `node_type` " + f"(datajunction_server.models.deployment)", + ) + else: + problems.append(f"{label}: missing `node_type`") + elif node_type not in NODE_TYPES: + problems.append( + f"{label}: node_type {node_type!r} not one of {sorted(NODE_TYPES)}", + ) + elif expected and node_type != expected: + problems.append(f"{label}: node_type is {node_type!r}, expected {expected!r}") + + if "name" not in data: + problems.append(f"{label}: missing `name`") + if isinstance(node_type, str): + for field in REQUIRED_FIELDS.get(node_type, []): + if not data.get(field): + problems.append(f"{label}: {node_type} node missing `{field}`") + if node_type == "dimension" and not has_primary_key(data): + problems.append( + f"{label}: dimension has no primary key (top-level `primary_key` or a " + f"column with `attributes: [primary_key]`)", + ) + return problems + + +def is_derived_metric(data: dict) -> bool: + """A metric that composes other metrics: it's a ratio/expression (has an operator) + and contains no raw aggregate of its own.""" + if data.get("node_type") != "metric": + return False + query = str(data.get("query") or "") + has_operator = any(op in query for op in ("/", "+", "-", "*")) + return has_operator and not _AGG.search(query) + + +def query_has_aggregate(data: dict) -> bool: + return bool(_AGG.search(str(data.get("query") or ""))) diff --git a/datajunction-clients/python/skill_evals/promptfooconfig.yaml b/datajunction-clients/python/skill_evals/promptfooconfig.yaml new file mode 100644 index 000000000..c2727920e --- /dev/null +++ b/datajunction-clients/python/skill_evals/promptfooconfig.yaml @@ -0,0 +1,231 @@ +# Behavioral evals for the bundled DJ Claude skills. +# +# This is a LOCAL / on-demand tool — it makes real LLM calls, so it is NOT wired into +# CI (public runners have no provider key). Run it when you want a quality check. +# +# Tokens — point the provider at any OpenAI-compatible endpoint via env (see .env.example): +# export OPENAI_API_KEY="sk-..." # your key (default base: api.openai.com) +# # or an OpenAI-compatible gateway/proxy: +# export OPENAI_BASE_URL="http://your-endpoint/v1" +# +# Pick a model your endpoint serves via SKILL_EVAL_MODEL (default gpt-4o). +# +# Run: +# ./run.sh # sources .env, applies SKILL_EVAL_MODEL +# npx promptfoo@latest view + +description: DataJunction skill behavioral evals + +# The test provider is a small two-turn orchestrator (provider.py): some cases need a +# proposal turn followed by a "now produce the YAML" turn (the semantic-model skill says +# "propose, don't produce"). It reads the model from SKILL_EVAL_MODEL and the endpoint +# from OPENAI_BASE_URL / OPENAI_API_KEY — so unlike the built-in openai provider it does +# NOT need its id templated. The llm-rubric grader below stays a native openai:chat +# provider (gpt-4o committed default — run.sh swaps it to SKILL_EVAL_MODEL locally). +providers: + - id: file://provider.py + config: + temperature: 0 + +prompts: + - file://skill_prompt.py:build_prompt + +defaultTest: + options: + # Grade llm-rubric with the same model (run.sh keeps this in sync via SKILL_EVAL_MODEL). + provider: openai:chat:gpt-4o + +tests: + # --- Authoring discipline (single turn → node YAML) ------------------------------- + + - description: Author a dimension node from a source table (datajunction-repo) + vars: + skill: datajunction-repo + expected_node_type: dimension + request: | + Author a DataJunction dimension node named `default.users` over the source + table `prod.analytics.users_table`, keyed by `user_id`, exposing `user_id`, + `country`, and `signup_date`. Output only the node YAML. Do not invent owners + or fields that weren't requested. + assert: + - type: python + value: file://assert_node.py + - type: llm-rubric + value: > + The output is a single DataJunction dimension node in YAML that a user could + deploy as-is: node_type is dimension, it declares a primary_key of user_id, + and its query selects user_id, country, and signup_date from + prod.analytics.users_table. It should not invent unrelated fields. + + - description: Author a metric (semantic-model composed with repo for YAML) + vars: + skill: "datajunction-semantic-model,datajunction-repo" + expected_node_type: metric + request: | + Author a DataJunction metric node `default.num_users` that counts distinct + users from the node `default.users`. Output only the node YAML. Do not invent + owners or fields that weren't requested. + assert: + - type: python + value: file://assert_node.py + - type: llm-rubric + value: > + The output is a single DataJunction metric node in YAML: node_type is metric + and the query is a single aggregating SELECT over default.users (e.g. + COUNT(DISTINCT user_id)). It should be a metric, not a transform or dimension. + + - description: Conditional filter uses CASE WHEN, not a WHERE clause + vars: + skill: "datajunction-semantic-model,datajunction-repo" + expected_node_type: metric + require_in_query: '(?i)CASE\s+WHEN' + forbid_in_query: '(?i)\bWHERE\b' + request: | + Author a DataJunction metric `finance.completed_revenue` that sums `amount_usd` + but only for rows where `status = 'completed'`, over the node + `finance.transactions`. Output only the node YAML. Do not invent owners or + fields that weren't requested. + assert: + - type: python + value: file://assert_node.py + - type: llm-rubric + value: > + A single DataJunction metric node. Crucially the filter is expressed as a + CASE WHEN inside the aggregate (SUM(CASE WHEN status = 'completed' THEN + amount_usd ELSE 0 END)) — NOT as a WHERE clause, which DJ metrics forbid. The + name should reflect the scoped population (e.g. completed_revenue). + + - description: Metric naming is business-meaningful, not a column transformation + vars: + skill: "datajunction-semantic-model,datajunction-repo" + expected_node_type: metric + request: | + Author a DataJunction metric over the node `finance.transactions` that sums + `amount_usd` — the revenue in USD booked on each transaction. Output only the + node YAML. Do not invent owners or fields that weren't requested. + assert: + - type: python + value: file://assert_node.py + - type: llm-rubric + value: > + A single DataJunction metric node summing amount_usd. The metric name must be + readable and business-meaningful — what a stakeholder would call it, e.g. + total_revenue — NOT a name that just describes the column transformation like + sum_amount_usd or amount_usd_sum. + + # --- Decomposition judgment (two-turn: propose, then produce YAML) ---------------- + + - description: Ratio query decomposes into named base metrics + a derived metric + vars: + skill: "datajunction-semantic-model,datajunction-repo" + require_derived_ratio: true + min_nodes_by_type: + metric: 3 + followup: > + Looks good. Assume `finance.transactions` already exists as a transform and a + `region` dimension already exists. Produce the final deployment YAML for ONLY the + new metric nodes now (do not redefine finance.transactions or the region + dimension), as a single ```yaml block with a top-level `nodes:` list. Make + reasonable assumptions instead of asking questions. + request: | + I have this analytics query and want to express it as DataJunction nodes: + + SELECT + region_id, + SUM(amount_usd) + / SUM(CASE WHEN status = 'completed' THEN 1 ELSE 0 END) AS avg_order_value + FROM finance.transactions + GROUP BY region_id + + How should I model this in DJ? Walk me through the decomposition first. + assert: + - type: python + value: file://assert_deployment.py + - type: llm-rubric + value: > + Two turns separated by a "<<>>" marker. Turn 1 (before the marker) + is a STRUCTURED DECOMPOSITION PROPOSAL in prose/list form — it should NOT dump + deployable node YAML on this first pass. It must decompose the ratio into two + named base metrics (one per aggregate: total revenue = SUM(amount_usd), and a + count of completed orders) plus ONE derived metric (avg_order_value) that + references those base metrics by name rather than re-inlining the aggregates, + and it should treat region_id as a dimension, not a baked-in column. Turn 2 + (after the marker) is the deployment YAML for that decomposition. Fail if turn 1 + collapses everything into a single metric with the whole ratio inlined. + + - description: Slice-by-dimension is modeled as a dimension link, not a JOIN + vars: + skill: "datajunction-semantic-model,datajunction-repo" + require_dimension_link: true + forbid_join: true + min_nodes_by_type: + dimension: 1 + metric: 1 + followup: > + Looks good. Assume the raw tables exist. Produce the final deployment YAML for + those nodes now (the dimension, the transform with its dimension link, and the + metric), as a single ```yaml block with a top-level `nodes:` list. Make + reasonable assumptions instead of asking questions. + request: | + I want total revenue sliced by customer country. Revenue lives in the transform + `finance.transactions` (one row per transaction; has `customer_id` and + `amount_usd`). Country lives in `core.customers` (keyed by `customer_id`, has a + `country` column). How should I model this in DJ so I can slice revenue by + country? Walk me through the decomposition first. + assert: + - type: python + value: file://assert_deployment.py + - type: llm-rubric + value: > + Two turns separated by a "<<>>" marker. The recommended model is: + country modeled as a DIMENSION node over core.customers, connected to + finance.transactions via a DIMENSION LINK on the transform, and total_revenue + as a plain aggregate metric over the transaction grain. It must NOT bake a JOIN + to core.customers into the metric or transform query — joins to dimensions + belong in dimension_links so the join is optional/slice-time. Turn 2 is the + deployment YAML reflecting that. + + - description: Reusability rule holds even when the user asks for one metric + vars: + skill: "datajunction-semantic-model,datajunction-repo" + require_derived_ratio: true + min_nodes_by_type: + metric: 3 + followup: > + Okay. Assume the transform `web.sessions` already exists. Produce the final + deployment YAML for whatever you'd recommend now, as a single ```yaml block with + a top-level `nodes:` list. Make reasonable assumptions instead of asking + questions. + request: | + I just want our checkout conversion rate — completed checkouts divided by + sessions, both countable from the transform `web.sessions`. Keep it simple, I + only care about the final rate, so just give me the one metric. + assert: + - type: python + value: file://assert_deployment.py + - type: llm-rubric + value: > + Despite the user asking for "just one metric", the skill should hold the + reusability line: it decomposes into two named base metrics (a count of + completed checkouts and a count of sessions) plus a derived conversion_rate + metric that references them — rather than a single metric with the whole ratio + as an anonymous SQL blob. It may briefly explain why named base metrics are + worth it. Fail if it simply produces one inlined ratio metric. + + # --- Grain (single-turn judgment) ------------------------------------------------ + + - description: Mixed grains in one transform should be split into two + vars: + skill: datajunction-semantic-model + request: | + I'm planning to put order line items (one row per order line) AND daily account + balance snapshots (one row per account per day) into a single transform called + `finance.account_activity`. Is that a good idea, and how should I model it? + assert: + - type: llm-rubric + value: > + The answer identifies that these are TWO DIFFERENT GRAINS (order-line grain vs + account-per-day grain) and that grains must not mix in one fact/transform. It + recommends SPLITTING into two separate transforms (one per grain) rather than + combining them, consistent with "grain is the most important decision in a + fact". It should not endorse putting both grains in one node. diff --git a/datajunction-clients/python/skill_evals/provider.py b/datajunction-clients/python/skill_evals/provider.py new file mode 100644 index 000000000..6c700b28d --- /dev/null +++ b/datajunction-clients/python/skill_evals/provider.py @@ -0,0 +1,99 @@ +"""promptfoo custom provider: a thin OpenAI-compatible chat client that can run a +case as one turn or two. + +Why a custom provider instead of the built-in ``openai:chat``: the semantic-model +skill's decomposition workflow says *propose, don't produce* — on a raw-query request +the model should first return a structured decomposition, and only emit node YAML when +asked. To eval that faithfully we need the model's turn-1 reply fed back as context for +a turn-2 "now produce the YAML" follow-up. A single combined prompt can't test the +"no YAML on the first pass" discipline; chaining across promptfoo rows is order/ +concurrency-fragile. So one test row drives both turns here. + +Behaviour: + - turn 1: call the model with the rendered prompt (system = skill(s), user = request); + - if the case sets a ``followup`` var, turn 2: replay [system, user, assistant=turn1, + user=followup] and append the reply after ``node_rules.TURN2_MARKER``. + +Config is all env (nothing about the provider is hardcoded): ``OPENAI_BASE_URL`` +(default the public OpenAI API), ``OPENAI_API_KEY``, and ``SKILL_EVAL_MODEL`` (default +gpt-4o). +""" + +import json +import os +import urllib.error +import urllib.request + +import node_rules + +DEFAULT_BASE_URL = "https://api.openai.com/v1" + + +def _messages_from_prompt(prompt): + """promptfoo hands the rendered prompt as a string; our prompt function returns a + chat message list, so it arrives JSON-encoded. Fall back to a single user turn.""" + try: + parsed = json.loads(prompt) + except (TypeError, json.JSONDecodeError): + return [{"role": "user", "content": str(prompt)}] + if isinstance(parsed, list): + return parsed + return [{"role": "user", "content": str(prompt)}] + + +def _chat(messages, model, temperature, base_url, api_key): + """One chat-completions call. Returns (content, usage_dict).""" + body = json.dumps( + {"model": model, "messages": messages, "temperature": temperature}, + ).encode() + request = urllib.request.Request( + f"{base_url.rstrip('/')}/chat/completions", + data=body, + headers={ + "Content-Type": "application/json", + "Authorization": f"Bearer {api_key}", + }, + ) + with urllib.request.urlopen(request, timeout=120) as response: + payload = json.loads(response.read()) + content = payload["choices"][0]["message"]["content"] + return content, payload.get("usage", {}) + + +def call_api(prompt, options, context): + config = options.get("config", {}) if isinstance(options, dict) else {} + temperature = config.get("temperature", 0) + model = os.environ.get("SKILL_EVAL_MODEL", "gpt-4o") + base_url = os.environ.get("OPENAI_BASE_URL", DEFAULT_BASE_URL) + api_key = os.environ.get("OPENAI_API_KEY", "") + + messages = _messages_from_prompt(prompt) + variables = (context or {}).get("vars", {}) + followup = variables.get("followup") + + try: + turn1, usage1 = _chat(messages, model, temperature, base_url, api_key) + if not followup: + return {"output": turn1, "tokenUsage": _usage(usage1)} + + messages = messages + [ + {"role": "assistant", "content": turn1}, + {"role": "user", "content": followup}, + ] + turn2, usage2 = _chat(messages, model, temperature, base_url, api_key) + combined = f"{turn1}\n\n{node_rules.TURN2_MARKER}\n\n{turn2}" + return {"output": combined, "tokenUsage": _usage(usage1, usage2)} + except urllib.error.HTTPError as exc: + detail = exc.read().decode(errors="replace") + return {"error": f"HTTP {exc.code} from {base_url}: {detail}"} + except Exception as exc: # noqa: BLE001 - surface any provider failure to promptfoo + return {"error": f"{type(exc).__name__}: {exc}"} + + +def _usage(*usages): + total = {"prompt": 0, "completion": 0, "total": 0} + for usage in usages: + total["prompt"] += usage.get("prompt_tokens", 0) + total["completion"] += usage.get("completion_tokens", 0) + total["total"] += usage.get("total_tokens", 0) + return total diff --git a/datajunction-clients/python/skill_evals/run.sh b/datajunction-clients/python/skill_evals/run.sh new file mode 100755 index 000000000..45854f148 --- /dev/null +++ b/datajunction-clients/python/skill_evals/run.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# Run the skill evals with config from a local .env (gitignored). See .env.example. +# +# .env supplies: OPENAI_BASE_URL, OPENAI_API_KEY, SKILL_EVAL_MODEL +# Extra args pass through to promptfoo. +# +# We source .env and export it here so it's authoritative for this run — that avoids +# a stale OPENAI_BASE_URL in your shell silently overriding it. promptfoo doesn't +# template the provider id from env, so the model is applied by generating a +# gitignored promptfooconfig.local.yaml. +set -euo pipefail +cd "$(dirname "$0")" + +if [ -f .env ]; then + set -a + . ./.env + set +a +fi + +MODEL="${SKILL_EVAL_MODEL:-gpt-4o}" +sed "s|openai:chat:gpt-4o|openai:chat:${MODEL}|g" promptfooconfig.yaml > promptfooconfig.local.yaml + +echo "model: ${MODEL} | base: ${OPENAI_BASE_URL:-}" +npx promptfoo@latest eval -c promptfooconfig.local.yaml "$@" diff --git a/datajunction-clients/python/skill_evals/skill_prompt.py b/datajunction-clients/python/skill_evals/skill_prompt.py new file mode 100644 index 000000000..eec695504 --- /dev/null +++ b/datajunction-clients/python/skill_evals/skill_prompt.py @@ -0,0 +1,25 @@ +"""promptfoo prompt function: load the skill(s) into context, then the request. + +This simulates how Claude uses a skill — the SKILL.md is injected as the system +message and the test case's request is the user turn. The model's response is what +the assertions in promptfooconfig.yaml grade. + +`vars.skill` is a comma-separated string of skill names — multiple compose them (in +real use a skill like datajunction-semantic-model is loaded alongside the skills it +defers to, e.g. datajunction-repo for the actual YAML authoring). It's a string, not +a YAML list, because promptfoo expands a list var into separate test cases. +""" + +from pathlib import Path + +SKILLS_DIR = Path(__file__).resolve().parent.parent / "datajunction" / "skills" + + +def build_prompt(context): + variables = context["vars"] + names = [n.strip() for n in variables["skill"].split(",") if n.strip()] + skill_docs = [(SKILLS_DIR / f"{name}.md").read_text() for name in names] + return [ + {"role": "system", "content": "\n\n---\n\n".join(skill_docs)}, + {"role": "user", "content": variables["request"]}, + ] diff --git a/datajunction-clients/python/tests/test_skill_examples.py b/datajunction-clients/python/tests/test_skill_examples.py new file mode 100644 index 000000000..620049159 --- /dev/null +++ b/datajunction-clients/python/tests/test_skill_examples.py @@ -0,0 +1,90 @@ +"""Validate that deployable node-spec examples in the bundled skills conform to the +current server-side deployment schema (``datajunction_server.models.deployment``). + +Why: the skills hand users example node YAML. As the orchestrator's deployment schema +evolves, those examples silently drift. This "compiles" each example against the real +Pydantic spec models — no running server needed (the spec models are imported directly, +and the client test suite already depends on ``datajunction_server``). + +Each fenced ```yaml block is classified: + + - **spec** — a single node (top-level ``node_type``) or a deployment (``nodes:``). + Validated against the server schema; a mismatch fails the build. + - **legacy** — the deprecated client shape (top-level ``type: source|metric|…`` + instead of ``node_type``). Failed with a migration message, so drifted examples + can't silently dodge validation by not matching the new schema. + - **fragment** — anything else (a bare ``query:`` / ``columns:`` snippet). Skipped. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest +import yaml +from pydantic import TypeAdapter, ValidationError + +from datajunction_server.models.deployment import DeploymentSpec, NodeUnion + +SKILLS_DIR = Path(__file__).parent.parent / "datajunction" / "skills" +_NODE_ADAPTER = TypeAdapter(NodeUnion) +_FENCED_YAML = re.compile(r"```ya?ml\n(.*?)\n```", re.DOTALL) +_LEGACY_TYPES = {"source", "transform", "dimension", "metric", "cube"} + + +def _classify(data) -> str | None: + """Return 'spec' | 'legacy' for a deployable block, or None to skip it.""" + if not isinstance(data, dict): + return None + if "node_type" in data or "nodes" in data: + return "spec" + if data.get("type") in _LEGACY_TYPES: # top-level node type the old way + return "legacy" + return None + + +def _candidate_examples() -> list[tuple[str, dict, str]]: + out: list[tuple[str, dict, str]] = [] + for path in sorted(SKILLS_DIR.glob("*.md")): + for i, match in enumerate(_FENCED_YAML.finditer(path.read_text())): + try: + data = yaml.safe_load(match.group(1)) + except yaml.YAMLError: + continue + kind = _classify(data) + if kind: + out.append((f"{path.stem}[{i}]", data, kind)) + return out + + +_EXAMPLES = _candidate_examples() + + +@pytest.mark.parametrize( + "ident,data,kind", + _EXAMPLES or [("__none__", {}, "spec")], + ids=[e[0] for e in _EXAMPLES] or ["no-deployable-examples"], +) +def test_skill_node_examples_conform_to_deployment_schema(ident, data, kind) -> None: + if ident == "__none__": + pytest.skip("No deployable node-spec examples found in the skills.") + + if kind == "legacy": + pytest.fail( + f"{ident}: uses the deprecated client node shape (top-level `type: " + f"{data.get('type')}`). Migrate to the server deployment schema " + f"(`node_type:` + the current spec fields) so it matches what the " + f"orchestrator accepts.", + ) + + try: + if "nodes" in data: + DeploymentSpec.model_validate(data) + else: + _NODE_ADAPTER.validate_python(data) + except ValidationError as exc: + pytest.fail( + f"{ident}: skill example does not conform to the current deployment schema " + f"(datajunction_server.models.deployment):\n{exc}", + )