From 153e02d6605c36213d6e5524b076a553ce4a0aff Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Sun, 14 Jun 2026 08:40:33 -0700 Subject: [PATCH 1/3] 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}", + ) From c73ff03589bb5b85ad134a248b26e82cd9eb20fc Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Sat, 19 Sep 2026 01:12:59 -0700 Subject: [PATCH 2/3] Score skill evals with partial credit and a no-skill baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every case now runs in two arms — with the skill in context, and with it replaced by a short control prompt — so a run reports what the skill contributed instead of one absolute number. The control prompt names the domain and output format but gives no modeling guidance: a bare baseline would fail every case for the wrong reason and make the delta meaningless. The programmatic asserts now tally each rule as one check and report the fraction satisfied. Binary scoring meant a five-node deployment with one bad node scored the same as unparseable junk, which hid progress while iterating on skill prose. `pass` stays strict — every check must hold — so a partial score never reads as success. --- .../python/skill_evals/README.md | 31 +++++- .../python/skill_evals/assert_deployment.py | 40 ++++---- .../python/skill_evals/assert_node.py | 30 +++--- .../python/skill_evals/node_rules.py | 96 ++++++++++++++----- .../python/skill_evals/promptfooconfig.yaml | 11 +++ .../python/skill_evals/provider.py | 27 +++++- .../python/skill_evals/run.sh | 19 +++- .../python/skill_evals/skill_prompt.py | 11 +++ 8 files changed, 196 insertions(+), 69 deletions(-) diff --git a/datajunction-clients/python/skill_evals/README.md b/datajunction-clients/python/skill_evals/README.md index 81ed8271b..7cd7a6ce8 100644 --- a/datajunction-clients/python/skill_evals/README.md +++ b/datajunction-clients/python/skill_evals/README.md @@ -34,6 +34,31 @@ Each case asserts with some mix of: - **llm-rubric** (LLM judge) — the modeling judgment: did it decompose correctly, resist the shortcut, name things meaningfully. +The programmatic asserts score **partial credit**: every rule is one check, and the +result carries the fraction satisfied (`7/9 checks — …`). A five-node deployment with +one bad node no longer scores the same as unparseable junk, so you can see a skill edit +move the needle before it fully passes. `pass` stays strict — every check must hold — +so a partial score never reads as success. + +## The baseline arm + +Every case runs twice: once with the skill in context, once with it replaced by a short +control prompt that names the domain and output format but gives no modeling guidance +(`skill_prompt.CONTROL_SYSTEM`). The two arms appear side by side in the promptfoo +report as `with-skill` and `baseline-no-skill`. + +The absolute score isn't the interesting number — the **delta** is. A case both arms +pass isn't testing the skill, it's testing the model, and belongs in the set only as a +regression guard. A case both arms fail is either a skill gap or a bad case. What +justifies the skill's tokens is the band in between. + +A control prompt rather than nothing, because a bare baseline fails every case for the +wrong reason (the model doesn't know we wanted DJ YAML), which would make the delta +look large and mean nothing. + +Skip it with `./run.sh --no-baseline` for half the tokens when you just want a quick +check of the with-skill arm. + ## Running it The provider endpoint comes from env — nothing about it is hardcoded. Copy @@ -63,7 +88,9 @@ SKILL_EVAL_MODEL=claude-sonnet-4-6 ./run.sh # any model the endpoint serves - `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. +- `provider.py` — two-turn OpenAI-compatible client; a `followup` var triggers turn 2, + and `skill_mode` selects the with-skill or baseline arm. +- `node_rules.py` — shared node-spec rules mirroring the server deployment schema, plus + the `CheckRun` tally behind partial-credit scoring. - `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 index dbf337e73..568ab73e7 100644 --- a/datajunction-clients/python/skill_evals/assert_deployment.py +++ b/datajunction-clients/python/skill_evals/assert_deployment.py @@ -41,46 +41,42 @@ def get_assert(output, context): "reason": "no authored nodes found in deployment output", } - problems: list[str] = [] + checks = node_rules.CheckRun() for node in nodes: - problems.extend(node_rules.validate_node(node)) + node_rules.validate_node(node, checks) 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( + checks.check( + counts.get(node_type, 0) >= minimum, + f"expected >= {minimum} {node_type} node(s), found {counts.get(node_type, 0)}", + ) + + if variables.get("require_derived_ratio"): + checks.check( + any(node_rules.is_derived_metric(n) for n in nodes), "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( + if variables.get("require_dimension_link"): + checks.check( + any(n.get("dimension_links") for n in nodes), "no node declares `dimension_links` (the join should be a dim link)", ) if variables.get("forbid_join"): + # One check per node, so partial credit shows how many queries are clean. for node in nodes: - if node.get("node_type") in ("metric", "transform") and _JOIN.search( - str(node.get("query") or ""), - ): - problems.append( + if node.get("node_type") in ("metric", "transform"): + checks.check( + not _JOIN.search(str(node.get("query") or "")), 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})"} + return checks.result(f"valid deployment ({summary})") diff --git a/datajunction-clients/python/skill_evals/assert_node.py b/datajunction-clients/python/skill_evals/assert_node.py index 2d31b98cc..9a98166f2 100644 --- a/datajunction-clients/python/skill_evals/assert_node.py +++ b/datajunction-clients/python/skill_evals/assert_node.py @@ -29,21 +29,23 @@ def get_assert(output, context): } data = nodes[0] - problems = node_rules.validate_node(data, expected) + checks = node_rules.CheckRun() + node_rules.validate_node(data, checks, 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}") + if require: + checks.check( + bool(re.search(require, query)), + 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", - } + if forbid: + checks.check( + not re.search(forbid, query), + f"query matches forbidden pattern {forbid!r}", + ) + + return checks.result( + f"valid {data.get('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 index a09cc3ecd..ce8aa1ed9 100644 --- a/datajunction-clients/python/skill_evals/node_rules.py +++ b/datajunction-clients/python/skill_evals/node_rules.py @@ -92,40 +92,86 @@ 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] = [] +class CheckRun: + """Tallies individual checks so an assertion scores partial credit. + + Binary pass/fail hides progress: a five-node deployment with one bad node looks + the same as unparseable junk. Each rule records one check here, and the result + carries the fraction satisfied. ``pass`` stays strict — every check must hold — + so a partial score never reads as success. + """ + + def __init__(self) -> None: + self.total = 0 + self.failed: list[str] = [] + + def check(self, ok: bool, problem: str) -> bool: + """Record one check. ``problem`` is only used when it fails.""" + self.total += 1 + if not ok: + self.failed.append(problem) + return ok + + @property + def passed(self) -> int: + return self.total - len(self.failed) + + @property + def score(self) -> float: + return 1.0 if not self.total else self.passed / self.total + + def result(self, summary: str) -> dict: + """A promptfoo assertion result: fractional score, strict pass.""" + tally = f"{self.passed}/{self.total} checks" + if self.failed: + return { + "pass": False, + "score": round(self.score, 4), + "reason": f"{tally} — {'; '.join(self.failed)}", + } + return {"pass": True, "score": 1.0, "reason": f"{tally} — {summary}"} + + +def validate_node(data: dict, checks: CheckRun, expected: str | None = None) -> None: + """Record the structural checks for a single node spec.""" 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)}", + + if node_type in NODE_TYPES: + problem = "" + elif node_type is not None: + problem = f"{label}: node_type {node_type!r} not one of {sorted(NODE_TYPES)}" + elif data.get("type") in NODE_TYPES: + problem = ( + f"{label}: uses legacy top-level `type: {data['type']}` — the deployment " + f"schema discriminator is `node_type` " + f"(datajunction_server.models.deployment)" + ) + else: + problem = f"{label}: missing `node_type`" + checks.check(not problem, problem) + + if expected: + checks.check( + node_type == expected, + f"{label}: node_type is {node_type!r}, expected {expected!r}", ) - 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`") + checks.check("name" in data, 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( + checks.check( + bool(data.get(field)), + f"{label}: {node_type} node missing `{field}`", + ) + + if node_type == "dimension": + checks.check( + has_primary_key(data), 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: diff --git a/datajunction-clients/python/skill_evals/promptfooconfig.yaml b/datajunction-clients/python/skill_evals/promptfooconfig.yaml index c2727920e..bdcdff00d 100644 --- a/datajunction-clients/python/skill_evals/promptfooconfig.yaml +++ b/datajunction-clients/python/skill_evals/promptfooconfig.yaml @@ -22,10 +22,21 @@ description: DataJunction skill behavioral evals # 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). +# Every case runs in two arms, so a run reports what the skill *contributed* rather +# than one absolute number: with-skill minus baseline is the skill's earned delta. The +# baseline swaps the skill for a short control prompt (skill_prompt.CONTROL_SYSTEM). +# Same id, distinct labels — narrow to one arm with `./run.sh --no-baseline`. providers: - id: file://provider.py + label: with-skill config: temperature: 0 + skill_mode: full + - id: file://provider.py + label: baseline-no-skill + config: + temperature: 0 + skill_mode: none prompts: - file://skill_prompt.py:build_prompt diff --git a/datajunction-clients/python/skill_evals/provider.py b/datajunction-clients/python/skill_evals/provider.py index 6c700b28d..c4e28a1b1 100644 --- a/datajunction-clients/python/skill_evals/provider.py +++ b/datajunction-clients/python/skill_evals/provider.py @@ -14,9 +14,13 @@ - 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). +The ``skill_mode`` config selects the arm: ``full`` (default) keeps the skill in +context, ``none`` is the baseline that replaces it with a control prompt. The config +declares both arms so each run reports the skill's delta. + +Endpoint 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 @@ -25,10 +29,22 @@ import urllib.request import node_rules +import skill_prompt DEFAULT_BASE_URL = "https://api.openai.com/v1" +def _apply_skill_mode(messages, skill_mode): + """``full`` keeps the skill(s) the prompt function injected as the system message. + ``none`` is the baseline arm: drop them for a short control prompt, so the delta + between arms measures the skill's guidance rather than the model's DJ priors.""" + if skill_mode != "none": + return messages + return [{"role": "system", "content": skill_prompt.CONTROL_SYSTEM}] + [ + message for message in messages if message.get("role") != "system" + ] + + 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.""" @@ -67,7 +83,10 @@ def call_api(prompt, options, context): base_url = os.environ.get("OPENAI_BASE_URL", DEFAULT_BASE_URL) api_key = os.environ.get("OPENAI_API_KEY", "") - messages = _messages_from_prompt(prompt) + messages = _apply_skill_mode( + _messages_from_prompt(prompt), + config.get("skill_mode", "full"), + ) variables = (context or {}).get("vars", {}) followup = variables.get("followup") diff --git a/datajunction-clients/python/skill_evals/run.sh b/datajunction-clients/python/skill_evals/run.sh index 45854f148..1a48b2949 100755 --- a/datajunction-clients/python/skill_evals/run.sh +++ b/datajunction-clients/python/skill_evals/run.sh @@ -4,6 +4,10 @@ # .env supplies: OPENAI_BASE_URL, OPENAI_API_KEY, SKILL_EVAL_MODEL # Extra args pass through to promptfoo. # +# By default every case runs twice — once with the skill, once without (the baseline +# arm) — so the report shows what the skill contributed. `--no-baseline` runs only the +# with-skill arm, for half the tokens. +# # 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 @@ -17,8 +21,19 @@ if [ -f .env ]; then set +a fi +ARGS=() +ARMS="with-skill + baseline" +for arg in "$@"; do + if [ "$arg" = "--no-baseline" ]; then + ARGS+=(--filter-providers with-skill) + ARMS="with-skill only" + else + ARGS+=("$arg") + fi +done + 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 "$@" +echo "model: ${MODEL} | base: ${OPENAI_BASE_URL:-} | arms: ${ARMS}" +npx promptfoo@latest eval -c promptfooconfig.local.yaml ${ARGS[@]+"${ARGS[@]}"} diff --git a/datajunction-clients/python/skill_evals/skill_prompt.py b/datajunction-clients/python/skill_evals/skill_prompt.py index eec695504..c50bb60df 100644 --- a/datajunction-clients/python/skill_evals/skill_prompt.py +++ b/datajunction-clients/python/skill_evals/skill_prompt.py @@ -14,6 +14,17 @@ SKILLS_DIR = Path(__file__).resolve().parent.parent / "datajunction" / "skills" +# System prompt for the baseline arm, where the provider drops the skill (see +# provider.py's ``skill_mode``). It names the domain and the output format but gives +# no modeling guidance, so the with-skill minus baseline delta measures what the +# skill's *advice* contributes — not whether the model can guess that we wanted DJ +# YAML. A bare baseline would fail every case for the wrong reason. +CONTROL_SYSTEM = ( + "You are helping a user model data in DataJunction (DJ), an open-source semantic " + "layer. Answer the request directly. When asked for node definitions, output them " + "as YAML in a fenced ```yaml block." +) + def build_prompt(context): variables = context["vars"] From 89f660828732b556fd58e5e05219ae624c491e2e Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Sat, 19 Sep 2026 02:02:37 -0700 Subject: [PATCH 3/3] Adapt skill evals to the /SKILL.md layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main moved skills from a flat `.md` to `/SKILL.md` and added a mirror under plugins/. The merge was textually clean but broke the harness: rename detection carried the `type:` → `node_type:` fixes into the mirror instead of the canonical copy, leaving the source with five legacy examples and the mirror out of sync with it. Apply the fixes to the canonical skill so `sync_plugin_skills.py --check` passes, and point both the prompt function and the drift test at the new layout. --- .../datajunction/skills/datajunction-repo/SKILL.md | 10 +++++----- .../python/skill_evals/skill_prompt.py | 2 +- .../python/tests/test_skill_examples.py | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/datajunction-clients/python/datajunction/skills/datajunction-repo/SKILL.md b/datajunction-clients/python/datajunction/skills/datajunction-repo/SKILL.md index 0776b8fa5..78cab6d0d 100644 --- a/datajunction-clients/python/datajunction/skills/datajunction-repo/SKILL.md +++ b/datajunction-clients/python/datajunction/skills/datajunction-repo/SKILL.md @@ -188,7 +188,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 @@ -232,7 +232,7 @@ mode: published # nodes/dimensions/user.yaml name: finance.user description: User dimension with attributes -type: dimension +node_type: dimension query: | SELECT user_id, @@ -286,7 +286,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( @@ -324,7 +324,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: | @@ -660,7 +660,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/skill_prompt.py b/datajunction-clients/python/skill_evals/skill_prompt.py index c50bb60df..16b4ac3d3 100644 --- a/datajunction-clients/python/skill_evals/skill_prompt.py +++ b/datajunction-clients/python/skill_evals/skill_prompt.py @@ -29,7 +29,7 @@ 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] + skill_docs = [(SKILLS_DIR / name / "SKILL.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 index 620049159..ed584bae6 100644 --- a/datajunction-clients/python/tests/test_skill_examples.py +++ b/datajunction-clients/python/tests/test_skill_examples.py @@ -46,7 +46,7 @@ def _classify(data) -> str | None: def _candidate_examples() -> list[tuple[str, dict, str]]: out: list[tuple[str, dict, str]] = [] - for path in sorted(SKILLS_DIR.glob("*.md")): + for path in sorted(SKILLS_DIR.glob("*/SKILL.md")): for i, match in enumerate(_FENCED_YAML.finditer(path.read_text())): try: data = yaml.safe_load(match.group(1)) @@ -54,7 +54,7 @@ def _candidate_examples() -> list[tuple[str, dict, str]]: continue kind = _classify(data) if kind: - out.append((f"{path.stem}[{i}]", data, kind)) + out.append((f"{path.parent.name}[{i}]", data, kind)) return out